Changeset 44574 in vbox

Timestamp:

Feb 6, 2013 5:48:18 PM (12 years ago)

Author:

vboxsync

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

83644

Message:

DevAHCI: Let IOM do the complicated reads. Let IOM filter out unaligned and odd sized writes as well, though we still need to split QWORD writes our selves.

File:

• trunk/src/VBox/Devices/Storage/DevAHCI.cpp (modified) (16 diffs)

trunk/src/VBox/Devices/Storage/DevAHCI.cpp

@@ -1 +1 @@
 /* $Id$ */
 /** @file
- * VBox storage devices: AHCI controller device (disk and cdrom).
- *                       Implements the AHCI standard 1.1
+ * DevAHCI - AHCI controller device (disk and cdrom).
+ *
+ * Implements the AHCI standard 1.1
  */
 
@@ -2156 +2157 @@
  *
  * @param   pAhci       The AHCI instance.
- * @param   uReg        The register to write.
- * @param   pv          Where to fetch the result.
- * @param   cb          Number of bytes to write.
- */
-static int ahciRegisterWrite(PAHCI pAhci, uint32_t uReg, void const *pv, unsigned cb)
-{
-    int rc = VINF_SUCCESS;
+ * @param   offReg      The offset of the register to write to.
+ * @param   u32Value    The value to write.
+ */
+static int ahciRegisterWrite(PAHCI pAhci, uint32_t offReg, uint32_t u32Value)
+{
+    int rc;
     uint32_t iReg;
 
-    if (uReg < AHCI_HBA_GLOBAL_SIZE)
+    /*
+     * If the access offset is smaller than 100h the guest accesses the global registers.
+     * Otherwise it accesses the registers of a port.
+     */
+    if (offReg < AHCI_HBA_GLOBAL_SIZE)
     {
         Log3(("Write global HBA register\n"));
-        iReg = uReg >> 2;
+        iReg = offReg >> 2;
         if (iReg < RT_ELEMENTS(g_aOpRegs))
         {
             const AHCIOPREG *pReg = &g_aOpRegs[iReg];
-            rc = pReg->pfnWrite(pAhci, iReg, *(uint32_t *)pv);
+            rc = pReg->pfnWrite(pAhci, iReg, u32Value);
         }
         else
@@ -2185 +2189 @@
         Log3(("Write Port register\n"));
         /* Calculate accessed port. */
-        uReg -= AHCI_HBA_GLOBAL_SIZE;
-        iPort = uReg / AHCI_PORT_REGISTER_SIZE;
-        iReg  = (uReg % AHCI_PORT_REGISTER_SIZE) >> 2;
+        offReg -= AHCI_HBA_GLOBAL_SIZE;
+        iPort   =  offReg / AHCI_PORT_REGISTER_SIZE;
+        iReg    = (offReg % AHCI_PORT_REGISTER_SIZE) >> 2;
         Log3(("%s: Trying to write to port %u and register %u\n", __FUNCTION__, iPort, iReg));
         if (RT_LIKELY(   iPort < pAhci->cPortsImpl
@@ -2193 +2197 @@
         {
             const AHCIPORTOPREG *pPortReg = &g_aPortOpRegs[iReg];
-            rc = pPortReg->pfnWrite(pAhci, &pAhci->ahciPort[iPort], iReg, *(uint32_t *)pv);
+            rc = pPortReg->pfnWrite(pAhci, &pAhci->ahciPort[iPort], iReg, u32Value);
         }
         else
@@ -2219 +2223 @@
 {
     PAHCI pAhci = PDMINS_2_DATA(pDevIns, PAHCI);
-    int   rc = VINF_SUCCESS;
-
-    /* Break up 64 bits reads into two dword reads. */
-    if (cb == 8)
-    {
-        rc = ahciMMIORead(pDevIns, pvUser, GCPhysAddr, pv, 4);
-        if (RT_FAILURE(rc))
-            return rc;
-
-        return ahciMMIORead(pDevIns, pvUser, GCPhysAddr + 4, (uint8_t *)pv + 4, 4);
-    }
-
-    Log2(("#%d ahciMMIORead: pvUser=%p:{%.*Rhxs} cb=%d GCPhysAddr=%RGp rc=%Rrc\n",
-          pDevIns->iInstance, pv, cb, pv, cb, GCPhysAddr, rc));
-
-    uint32_t uOffset = (GCPhysAddr - pAhci->MMIOBase);
-    rc = ahciRegisterRead(pAhci, uOffset, pv, cb);
+    Log2(("#%d ahciMMIORead: pvUser=%p:{%.*Rhxs} cb=%d GCPhysAddr=%RGp\n",
+          pDevIns->iInstance, pv, cb, pv, cb, GCPhysAddr));
+
+    int rc = ahciRegisterRead(pAhci, GCPhysAddr - pAhci->MMIOBase, pv, cb);
 
     Log2(("#%d ahciMMIORead: return pvUser=%p:{%.*Rhxs} cb=%d GCPhysAddr=%RGp rc=%Rrc\n",
@@ -2257 +2248 @@
 {
     PAHCI pAhci = PDMINS_2_DATA(pDevIns, PAHCI);
-    int   rc = VINF_SUCCESS;
+    Assert(cb == 4 || cb == 8);
+    Assert(!(GCPhysAddr & (cb - 1)));
 
     /* Break up 64 bits writes into two dword writes. */
+    /** @todo Eliminate this code once the IOM/EM starts taking care of these
+         *    situations. */
     if (cb == 8)
     {
@@ -2269 +2263 @@
          * which can cause errors in the guest.
          */
+        int rc = VINF_SUCCESS;
         if (!pAhci->f8ByteMMIO4BytesWrittenSuccessfully)
         {
@@ -2289 +2284 @@
     }
 
-    Log2(("#%d ahciMMIOWrite: pvUser=%p:{%.*Rhxs} cb=%d GCPhysAddr=%RGp\n",
-          pDevIns->iInstance, pv, cb, pv, cb, GCPhysAddr));
-
-    /* Validate access. */
-    if (cb != sizeof(uint32_t))
-    {
-        Log2(("%s: Bad write size!!! GCPhysAddr=%RGp cb=%d\n", __FUNCTION__, GCPhysAddr, cb));
-        return VINF_SUCCESS;
-    }
-    if (GCPhysAddr & 0x3)
-    {
-        Log2(("%s: Unaligned write!!! GCPhysAddr=%RGp cb=%d\n", __FUNCTION__, GCPhysAddr, cb));
-        return VINF_SUCCESS;
-    }
-
-    /*
-     * If the access offset is smaller than 100h the guest accesses the global registers.
-     * Otherwise it accesses the registers of a port.
-     */
-    uint32_t uOffset = (GCPhysAddr - pAhci->MMIOBase);
-    rc = ahciRegisterWrite(pAhci, uOffset, pv, cb);
-
-    return rc;
+    /* Do the access. */
+    Log2(("#%d ahciMMIOWrite: pvUser=%p:{%.*Rhxs} cb=%d GCPhysAddr=%RGp\n", pDevIns->iInstance, pv, cb, pv, cb, GCPhysAddr));
+    return ahciRegisterWrite(pAhci, GCPhysAddr - pAhci->MMIOBase, *(uint32_t const *)pv);
 }
 
@@ -2355 +2330 @@
         else
         {
+            /** @todo range check? */
             Assert(iReg == 1);
-            rc = ahciRegisterWrite(pAhci, pAhci->regIdx, &u32, cb);
+            rc = ahciRegisterWrite(pAhci, pAhci->regIdx, u32);
             if (rc == VINF_IOM_R3_MMIO_WRITE)
                 rc = VINF_IOM_R3_IOPORT_WRITE;
@@ -2398 +2374 @@
         {
             Assert(iReg == 1);
+            /** @todo range check? */
             rc = ahciRegisterRead(pAhci, pAhci->regIdx, pu32, cb);
             if (rc == VINF_IOM_R3_MMIO_READ)
@@ -2425 +2402 @@
 
     /* We use the assigned size here, because we currently only support page aligned MMIO ranges. */
+    /** @todo change this to IOMMMIO_FLAGS_WRITE_ONLY_DWORD once EM/IOM starts
+     * handling 2nd DWORD failures on split accesses correctly. */ 
     rc = PDMDevHlpMMIORegister(pDevIns, GCPhysAddress, cb, NULL /*pvUser*/,
-                               IOMMMIO_FLAGS_READ_PASSTHRU | IOMMMIO_FLAGS_WRITE_PASSTHRU,
+                               IOMMMIO_FLAGS_READ_DWORD | IOMMMIO_FLAGS_WRITE_ONLY_DWORD_QWORD,
                                ahciMMIOWrite, ahciMMIORead, "AHCI");
     if (RT_FAILURE(rc))
@@ -5828 +5807 @@
  * @param   rcReq        IPRT Status code of the completed request.
  */
-static DECLCALLBACK(int) ahciTransferCompleteNotify(PPDMIBLOCKASYNCPORT pInterface, void *pvUser, int rcReq)
+static DECLCALLBACK(int) ahciR3TransferCompleteNotify(PPDMIBLOCKASYNCPORT pInterface, void *pvUser, int rcReq)
 {
     PAHCIPort pAhciPort = PDMIBLOCKASYNCPORT_2_PAHCIPORT(pInterface);
@@ -7188 +7167 @@
  * @param   pInterface      Pointer to the interface structure containing the called function pointer.
  */
-static DECLCALLBACK(void) ahciMountNotify(PPDMIMOUNTNOTIFY pInterface)
+static DECLCALLBACK(void) ahciR3MountNotify(PPDMIMOUNTNOTIFY pInterface)
 {
     PAHCIPort pAhciPort = PDMIMOUNTNOTIFY_2_PAHCIPORT(pInterface);
@@ -7217 +7196 @@
  * @param   pInterface      Pointer to the interface structure containing the called function pointer.
  */
-static DECLCALLBACK(void) ahciUnmountNotify(PPDMIMOUNTNOTIFY pInterface)
+static DECLCALLBACK(void) ahciR3UnmountNotify(PPDMIMOUNTNOTIFY pInterface)
 {
     PAHCIPort pAhciPort = PDMIMOUNTNOTIFY_2_PAHCIPORT(pInterface);
@@ -8108 +8087 @@
          */
         pAhciPort->IBase.pfnQueryInterface              = ahciR3PortQueryInterface;
-        pAhciPort->IPortAsync.pfnTransferCompleteNotify = ahciTransferCompleteNotify;
+        pAhciPort->IPortAsync.pfnTransferCompleteNotify = ahciR3TransferCompleteNotify;
         pAhciPort->IPort.pfnQueryDeviceLocation         = ahciR3PortQueryDeviceLocation;
-        pAhciPort->IMountNotify.pfnMountNotify          = ahciMountNotify;
-        pAhciPort->IMountNotify.pfnUnmountNotify        = ahciUnmountNotify;
+        pAhciPort->IMountNotify.pfnMountNotify          = ahciR3MountNotify;
+        pAhciPort->IMountNotify.pfnUnmountNotify        = ahciR3UnmountNotify;
         pAhciPort->fAsyncIOThreadIdle                   = true;
 
@@ -8187 +8166 @@
         return PDMDEV_SET_ERROR(pDevIns, rc, N_("AHCI cannot attach to status driver"));
     }
-    rc = PDMDevHlpSSMRegisterEx(pDevIns, AHCI_SAVED_STATE_VERSION, sizeof(*pThis)+cbTotalBufferSize, NULL,
+    rc = PDMDevHlpSSMRegisterEx(pDevIns, AHCI_SAVED_STATE_VERSION, sizeof(*pThis) + cbTotalBufferSize, NULL,
                                 NULL,           ahciR3LiveExec, NULL,
                                 ahciR3SavePrep, ahciR3SaveExec, NULL,