Changeset 85895 in vbox

Timestamp:

Aug 27, 2020 7:14:53 AM (5 years ago)

Author:

vboxsync

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

140077

Message:

AMD IOMMU: bugref:9654 Logging, assertion and comment about unmapping MMIO region.

File:

• trunk/src/VBox/Devices/Bus/DevIommuAmd.cpp (modified) (18 diffs)

trunk/src/VBox/Devices/Bus/DevIommuAmd.cpp

@@ -456 +456 @@
  * @{ */
 /** Log prefix string. */
-#define IOMMU_LOG_PFX                               "AMD_IOMMU"
+#define IOMMU_LOG_PFX                               "IOMMU-AMD"
 /** The current saved state version. */
 #define IOMMU_SAVED_STATE_VERSION                   1
@@ -2595 +2595 @@
 {
     IOMMU_ASSERT_LOCKED(pDevIns);
+    LogFlowFunc(("\n"));
 
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
     if (   !ASMAtomicXchgBool(&pThis->fCmdThreadSignaled, true)
         &&  ASMAtomicReadBool(&pThis->fCmdThreadSleeping))
+    {
+        LogFlowFunc(("Signaling command thread\n"));
         PDMDevHlpSUPSemEventSignal(pDevIns, pThis->hEvtCmdThread);
+    }
 }
 
@@ -4442 +4446 @@
 
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
+    LogFlowFunc(("uDevId=%#x uIova=%#RX64 cbRead=%u\n", uDevId, uIova, cbRead));
 
     /* Addresses are forwarded without translation when the IOMMU is disabled. */
@@ -4479 +4484 @@
 
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
+    LogFlowFunc(("uDevId=%#x uIova=%#RX64 cbWrite=%u\n", uDevId, uIova, cbWrite));
 
     /* Addresses are forwarded without translation when the IOMMU is disabled. */
@@ -4791 +4797 @@
 
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
+    LogFlowFunc(("uDevId=%#x\n", uDevId));
 
     /* Interrupts are forwarded with remapping when the IOMMU is disabled. */
@@ -4815 +4822 @@
     Assert(!(off & (cb - 1)));
 
+    LogFlowFunc(("off=%RGp cb=%u\n", off, cb));
+
     uint64_t const uValue = cb == 8 ? *(uint64_t const *)pv : *(uint32_t const *)pv;
     return iommuAmdWriteRegister(pDevIns, off, cb, uValue);
@@ -4828 +4837 @@
     Assert(cb == 4 || cb == 8);
     Assert(!(off & (cb - 1)));
+
+    LogFlowFunc(("off=%RGp cb=%u\n", off, cb));
 
     uint64_t uResult;
@@ -5087 +5098 @@
 {
     RT_NOREF(pThread);
-
+    LogFlowFunc(("\n"));
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
     return PDMDevHlpSUPSemEventSignal(pDevIns, pThis->hEvtCmdThread);
@@ -5099 +5110 @@
                                                           unsigned cb, uint32_t *pu32Value)
 {
+    LogFlowFunc(("\n"));
     /** @todo IOMMU: PCI config read stat counter. */
     VBOXSTRICTRC rcStrict = PDMDevHlpPCIConfigRead(pDevIns, pPciDev, uAddress, cb, pu32Value);
@@ -5114 +5126 @@
 {
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
+    LogFlowFunc(("\n"));
 
     /*
@@ -5149 +5162 @@
             if (pThis->IommuBar.n.u1Enable)
             {
-                Assert(pThis->hMmio == NIL_IOMMMIOHANDLE);
+                Assert(pThis->hMmio != NIL_IOMMMIOHANDLE);
+                Assert(PDMDevHlpMmioGetMappingAddress(pDevIns, pThis->hMmio) == NIL_RTGCPHYS);
                 Assert(!pThis->ExtFeat.n.u1PerfCounterSup); /* Base is 16K aligned when performance counters aren't supported. */
                 RTGCPHYS const GCPhysMmioBase = RT_MAKE_U64(pThis->IommuBar.au32[0] & 0xffffc000, pThis->IommuBar.au32[1]);
@@ -5751 +5765 @@
     /** @todo IOMMU: Save state. */
     RT_NOREF2(pDevIns, pSSM);
+    LogFlowFunc(("\n"));
     return VERR_NOT_IMPLEMENTED;
 }
@@ -5762 +5777 @@
     /** @todo IOMMU: Load state. */
     RT_NOREF4(pDevIns, pSSM, uVersion, uPass);
+    LogFlowFunc(("\n"));
     return VERR_NOT_IMPLEMENTED;
 }
@@ -5781 +5797 @@
     PDMPCIDEV_ASSERT_VALID(pDevIns, pPciDev);
 
+    LogFlowFunc(("\n"));
+
     memset(&pThis->aDevTabBaseAddrs[0], 0, sizeof(pThis->aDevTabBaseAddrs));
 
@@ -5844 +5862 @@
     PDMPciDevSetDWord(pPciDev, IOMMU_PCI_OFF_BASE_ADDR_REG_LO, 0);
     PDMPciDevSetDWord(pPciDev, IOMMU_PCI_OFF_BASE_ADDR_REG_HI, 0);
+
+    /*
+     * I ASSUME all MMIO regions mapped by a PDM device are automatically unmapped
+     * on VM reset. If not, we need to enable the following...
+     */
+#if 0
+    /* Unmap the MMIO region on reset if it has been mapped previously. */
+    Assert(pThis->hMmio != NIL_IOMMMIOHANDLE);
+    if (PDMDevHlpMmioGetMappingAddress(pDevIns, pThis->hMmio) != NIL_RTGCPHYS)
+        PDMDevHlpMmioUnmap(pDevIns, pThis->hMmio);
+#endif
 }
 
@@ -5853 +5882 @@
 {
     PDMDEV_CHECK_VERSIONS_RETURN_QUIET(pDevIns);
-    PIOMMU   pThis   = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
+    PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
     LogFlowFunc(("\n"));
 
@@ -5872 +5901 @@
 {
     PDMDEV_CHECK_VERSIONS_RETURN(pDevIns);
-    RT_NOREF2(iInstance, pCfg);
-    LogFlowFunc(("\n"));
+    RT_NOREF(pCfg);
 
     PIOMMU   pThis   = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
     PIOMMUCC pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PIOMMUCC);
     pThisCC->pDevInsR3 = pDevIns;
+
+    LogFlowFunc(("iInstance=%d\n", iInstance));
 
     /*
@@ -6034 +6064 @@
      * Create the command thread and its event semaphore.
      */
+    char szDevIommu[64];
+    RT_ZERO(szDevIommu);
+    RTStrPrintf(szDevIommu, sizeof(szDevIommu), "IOMMU-%u", iInstance);
     rc = PDMDevHlpThreadCreate(pDevIns, &pThisCC->pCmdThread, pThis, iommuAmdR3CmdThread, iommuAmdR3CmdThreadWakeUp,
-                               0 /* cbStack */, RTTHREADTYPE_IO, "AMD-IOMMU");
+                               0 /* cbStack */, RTTHREADTYPE_IO, szDevIommu);
     AssertLogRelRCReturn(rc, rc);