Changeset 44722 in vbox for trunk/src/VBox/Devices

Timestamp:

Feb 15, 2013 6:19:53 PM (12 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

83823

Message:

VMMDev: More cleanups.

Location:

trunk/src/VBox/Devices

Files:

• VMMDev/VMMDev.cpp (modified) (15 diffs)
• VMMDev/VMMDevState.h (modified) (1 diff)
• testcase/tstDeviceStructSizeRC.cpp (modified) (1 diff)

trunk/src/VBox/Devices/VMMDev/VMMDev.cpp

@@ -60 +60 @@
 *   Defined Constants And Macros                                               *
 *******************************************************************************/
-#define VMMDEVSTATE_2_DEVINS(pVMMDevState)         ( (pVMMDevState)->pDevIns )
-
 #define VBOX_GUEST_INTERFACE_VERSION_1_03(s) \
     (   RT_HIWORD((s)->guestInfo.interfaceVersion) == 1 \
@@ -131 +129 @@
  */
 
-static void vmmdevSetIRQ_Legacy_EMT (VMMDevState *pVMMDevState)
+static void vmmdevSetIRQ_Legacy_EMT(VMMDevState *pVMMDevState)
 {
     if (!pVMMDevState->fu32AdditionsOk)
@@ -167 +165 @@
     /* Set IRQ level for pin 0 */
     /** @todo make IRQ pin configurable, at least a symbolic constant */
-    PPDMDEVINS pDevIns = VMMDEVSTATE_2_DEVINS(pVMMDevState);
+    PPDMDEVINS pDevIns = pVMMDevState->pDevIns;
     PDMDevHlpPCISetIrqNoWait(pDevIns, 0, u32IRQLevel);
     Log(("vmmdevSetIRQ: IRQ set %d\n", u32IRQLevel));
 }
 
-static void vmmdevMaybeSetIRQ_EMT (VMMDevState *pVMMDevState)
-{
-    PPDMDEVINS pDevIns = VMMDEVSTATE_2_DEVINS (pVMMDevState);
-
+static void vmmdevMaybeSetIRQ_EMT(VMMDevState *pVMMDevState)
+{
     Log3(("vmmdevMaybeSetIRQ_EMT: u32HostEventFlags = 0x%08X, u32GuestFilterMask = 0x%08X.\n",
           pVMMDevState->u32HostEventFlags, pVMMDevState->u32GuestFilterMask));
@@ -182 +178 @@
     {
         pVMMDevState->pVMMDevRAMR3->V.V1_04.fHaveEvents = true;
-        PDMDevHlpPCISetIrqNoWait (pDevIns, 0, 1);
+        PDMDevHlpPCISetIrqNoWait(pVMMDevState->pDevIns, 0, 1);
         Log3(("vmmdevMaybeSetIRQ_EMT: IRQ set.\n"));
     }
 }
 
-static void vmmdevNotifyGuest_EMT (VMMDevState *pVMMDevState, uint32_t u32EventMask)
+static void vmmdevNotifyGuest_EMT(VMMDevState *pVMMDevState, uint32_t u32EventMask)
 {
     Log3(("VMMDevNotifyGuest_EMT: u32EventMask = 0x%08X.\n", u32EventMask));
 
-    if (VBOX_GUEST_INTERFACE_VERSION_1_03 (pVMMDevState))
+    if (VBOX_GUEST_INTERFACE_VERSION_1_03(pVMMDevState))
     {
         Log3(("VMMDevNotifyGuest_EMT: Old additions detected.\n"));
 
         pVMMDevState->u32HostEventFlags |= u32EventMask;
-        vmmdevSetIRQ_Legacy_EMT (pVMMDevState);
+        vmmdevSetIRQ_Legacy_EMT(pVMMDevState);
     }
     else
@@ -252 +248 @@
 void VMMDevNotifyGuest (VMMDevState *pVMMDevState, uint32_t u32EventMask)
 {
-    PPDMDEVINS pDevIns = VMMDEVSTATE_2_DEVINS(pVMMDevState);
+    PPDMDEVINS pDevIns = pVMMDevState->pDevIns;
 
     Log3(("VMMDevNotifyGuest: u32EventMask = 0x%08X.\n", u32EventMask));
@@ -1710 +1706 @@
             else
             {
-                if (VBOX_GUEST_INTERFACE_VERSION_1_03 (pThis))
+                if (VBOX_GUEST_INTERFACE_VERSION_1_03(pThis))
                 {
-                    vmmdevSetIRQ_Legacy_EMT (pThis);
+                    vmmdevSetIRQ_Legacy_EMT(pThis);
                 }
                 else
@@ -2478 +2474 @@
 
 /**
- * Callback function for mapping an PCI I/O region.
- *
- * @return VBox status code.
- * @param   pPciDev         Pointer to PCI device. Use pPciDev->pDevIns to get the device instance.
- * @param   iRegion         The region number.
- * @param   GCPhysAddress   Physical address of the region. If iType is PCI_ADDRESS_SPACE_IO, this is an
- *                          I/O port, else it's a physical address.
- *                          This address is *NOT* relative to pci_mem_base like earlier!
- * @param   enmType         One of the PCI_ADDRESS_SPACE_* values.
- */
-static DECLCALLBACK(int) vmmdevIORAMRegionMap(PPCIDEVICE pPciDev, /*unsigned*/ int iRegion, RTGCPHYS GCPhysAddress, uint32_t cb, PCIADDRESSSPACE enmType)
+ * @interface_method_impl{FNPCIIOREGIONMAP, MMIO/MMIO2 regions}
+ */
+static DECLCALLBACK(int)
+vmmdevIORAMRegionMap(PPCIDEVICE pPciDev, int iRegion, RTGCPHYS GCPhysAddress, uint32_t cb, PCIADDRESSSPACE enmType)
 {
     LogFlow(("vmmdevR3IORAMRegionMap: iRegion=%d GCPhysAddress=%RGp cb=%#x enmType=%d\n", iRegion, GCPhysAddress, cb, enmType));
@@ -2552 +2541 @@
 
 /**
- * Callback function for mapping a PCI I/O region.
- *
- * @return VBox status code.
- * @param   pPciDev         Pointer to PCI device. Use pPciDev->pDevIns to get the device instance.
- * @param   iRegion         The region number.
- * @param   GCPhysAddress   Physical address of the region. If iType is PCI_ADDRESS_SPACE_IO, this is an
- *                          I/O port, else it's a physical address.
- *                          This address is *NOT* relative to pci_mem_base like earlier!
- * @param   enmType         One of the PCI_ADDRESS_SPACE_* values.
- */
-static DECLCALLBACK(int) vmmdevIOPortRegionMap(PPCIDEVICE pPciDev, /*unsigned*/ int iRegion, RTGCPHYS GCPhysAddress, uint32_t cb, PCIADDRESSSPACE enmType)
+ * @interface_method_impl{FNPCIIOREGIONMAP, I/O Port Region}
+ */
+static DECLCALLBACK(int)
+vmmdevIOPortRegionMap(PPCIDEVICE pPciDev, int iRegion, RTGCPHYS GCPhysAddress, uint32_t cb, PCIADDRESSSPACE enmType)
 {
     PVMMDEV pThis = RT_FROM_MEMBER(pPciDev, VMMDEV, PciDev);
-    int         rc = VINF_SUCCESS;
 
     Assert(enmType == PCI_ADDRESS_SPACE_IO);
@@ -2572 +2553 @@
 
     /*
-     * Save the base port address to simplify Port offset calculations.
-     */
-    pThis->PortBase = (RTIOPORT)GCPhysAddress;
-
-    /*
      * Register our port IO handlers.
      */
-    rc = PDMDevHlpIOPortRegister(pPciDev->pDevIns,
-                                 (RTIOPORT)GCPhysAddress + VMMDEV_PORT_OFF_REQUEST, 1,
-                                 (void*)pThis, vmmdevRequestHandler,
-                                 NULL, NULL, NULL, "VMMDev Request Handler");
+    int rc = PDMDevHlpIOPortRegister(pPciDev->pDevIns, (RTIOPORT)GCPhysAddress + VMMDEV_PORT_OFF_REQUEST, 1,
+                                     pThis, vmmdevRequestHandler, NULL, NULL, NULL, "VMMDev Request Handler");
     AssertRC(rc);
     return rc;
@@ -2632 +2606 @@
 /* -=-=-=-=-=- PDMIVMMDEVPORT (VMMDEV::IPort) -=-=-=-=-=- */
 
-/** Converts a VMMDev port interface pointer to a VMMDev state pointer. */
-#define IVMMDEVPORT_2_VMMDEVSTATE(pInterface) ( (VMMDevState*)((uintptr_t)pInterface - RT_OFFSETOF(VMMDevState, IPort)) )
-
-
 /**
  * @interface_method_impl{PDMIVMMDEVPORT, pfnQueryAbsoluteMouse}
@@ -2643 +2613 @@
     PVMMDEV pThis = RT_FROM_MEMBER(pInterface, VMMDEV, IPort);
 
+    /** @todo at the first sign of trouble in this area, just enter the critsect.
+     * As indicated by the comment below, the atomic reads serves no real purpose
+     * here since we can assume cache coherency protocoles and int32_t alignment
+     * rules making sure we won't see a halfwritten value. */
     if (pxAbs)
         *pxAbs = ASMAtomicReadS32(&pThis->mouseXAbs); /* why the atomic read? */
     if (pyAbs)
         *pyAbs = ASMAtomicReadS32(&pThis->mouseYAbs);
+
     return VINF_SUCCESS;
 }
@@ -2717 +2692 @@
 
     if (idxDisplay >= RT_ELEMENTS(pThis->displayChangeData.aRequests))
-    {
         return VERR_INVALID_PARAMETER;
-    }
 
     PDMCritSectEnter(&pThis->CritSect, VERR_SEM_BUSY);
@@ -2921 +2894 @@
     Log(("vmmdevIPort_VBVAChange: fEnabled = %d\n", fEnabled));
 
+    /* Only used by saved state, which I guess is why we don't bother with locking here. */
     pThis->u32VideoAccelEnabled = fEnabled;
 }
@@ -2929 +2903 @@
 static DECLCALLBACK(int) vmmdevIPort_CpuHotUnplug(PPDMIVMMDEVPORT pInterface, uint32_t idCpuCore, uint32_t idCpuPackage)
 {
-    int rc = VINF_SUCCESS;
     PVMMDEV pThis = RT_FROM_MEMBER(pInterface, VMMDEV, IPort);
+    int     rc    = VINF_SUCCESS;
 
     Log(("vmmdevIPort_CpuHotUnplug: idCpuCore=%u idCpuPackage=%u\n", idCpuCore, idCpuPackage));
@@ -2955 +2929 @@
 static DECLCALLBACK(int) vmmdevIPort_CpuHotPlug(PPDMIVMMDEVPORT pInterface, uint32_t idCpuCore, uint32_t idCpuPackage)
 {
-    int rc = VINF_SUCCESS;
     PVMMDEV pThis = RT_FROM_MEMBER(pInterface, VMMDEV, IPort);
+    int     rc    = VINF_SUCCESS;
 
     Log(("vmmdevCpuPlug: idCpuCore=%u idCpuPackage=%u\n", idCpuCore, idCpuPackage));

trunk/src/VBox/Devices/VMMDev/VMMDevState.h

@@ -158 +158 @@
     /** message buffer index. */
     uint32_t iMsg;
-    /** Base port in the assigned I/O space. */
-    RTIOPORT PortBase;
-    /** Alignment padding.  */
-    RTIOPORT PortAlignment2;
+    /** Alignment padding. */
+    uint32_t u32Alignment2;
 
     /** IRQ number assigned to the device */

trunk/src/VBox/Devices/testcase/tstDeviceStructSizeRC.cpp

@@ -1427 +1427 @@
     GEN_CHECK_OFF(VMMDEV, szMsg);
     GEN_CHECK_OFF(VMMDEV, iMsg);
-    GEN_CHECK_OFF(VMMDEV, PortBase);
     GEN_CHECK_OFF(VMMDEV, irq);
     GEN_CHECK_OFF(VMMDEV, u32HostEventFlags);