Changeset 84351 in vbox

Timestamp:

May 19, 2020 5:26:12 AM (5 years ago)

Author:

vboxsync

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

138040

Message:

Network/DevVirtioNet_1_0.cpp: Fixed R0->R3 degradation in I/O path by implementing R0 MMIO, also allows worker thread synchronization via notification from guest to run in R0. Fixed bug where wasn't properly detecting when guest suppressed interrupts, and various minor fixes. See bugref:8651, Comment #70

Location:

trunk/src/VBox/Devices

Files:

• Network/DevVirtioNet_1_0.cpp (modified) (31 diffs)
• Storage/DevVirtioSCSI.cpp (modified) (10 diffs)
• VirtIO/Virtio_1_0.cpp (modified) (34 diffs)
• VirtIO/Virtio_1_0.h (modified) (8 diffs)

trunk/src/VBox/Devices/Network/DevVirtioNet_1_0.cpp

@@ -31 +31 @@
 #define LOG_GROUP LOG_GROUP_DEV_VIRTIO
 #define VIRTIONET_WITH_GSO
+
 #include <iprt/types.h>
-
+#include <iprt/errcore.h>
+#include <iprt/assert.h>
+#include <iprt/string.h>
+
+#include <VBox/sup.h>
 #include <VBox/vmm/pdmdev.h>
 #include <VBox/vmm/stam.h>
@@ -39 +44 @@
 #include <VBox/msi.h>
 #include <VBox/version.h>
-//#include <VBox/asm.h>
 #include <VBox/log.h>
-#include <iprt/errcore.h>
-#include <iprt/assert.h>
-#include <iprt/string.h>
-#include <VBox/sup.h>
+
 #ifdef IN_RING3
-#include <VBox/VBoxPktDmp.h>
-#endif
-#ifdef IN_RING3
+# include <VBox/VBoxPktDmp.h>
 # include <iprt/alloc.h>
 # include <iprt/memcache.h>
@@ -61 +60 @@
 
 /* After debugging single instance case, restore instance name logging */
-#define INSTANCE(pState) (char *)(pState->szInstanceName ? "" : "") // Avoid requiring RT_NOREF in some funcs
+#define INSTANCE(pState) (char *)(pState->szInstanceName ? pState->szInstanceName : "") // Avoid requiring RT_NOREF in some funcs
 
 #define VIRTIONET_SAVED_STATE_VERSION          UINT32_C(1)
@@ -99 +98 @@
 
 #define LUN0    0
+
+#ifdef USING_CRITICAL_SECTION
+#  define ENTER_CRITICAL_SECTION \
+        do { \
+            int rc = virtioNetR3CsEnter(pDevIns, pThis, VERR_SEM_BUSY); \
+            AssertRCReturnVoid(rc); \
+            RT_NOREF(rc);
+        } while(0)
+#  define LEAVE_CRITICAL_SECTION \
+        do { \
+            virtioNetR3CsLeave(pDevIns, pThis); \
+        } while(0)
+#else
+#   define ENTER_CRITICAL_SECTION do { } while(0)
+#   define LEAVE_CRITICAL_SECTION do { } while(0)
+#endif
 
 /*
@@ -294 +309 @@
 {
     SUPSEMEVENT                     hEvtProcess;                /**< handle of associated sleep/wake-up semaphore      */
+    bool volatile                   fSleeping;                  /**< Flags whether worker thread is sleeping or not    */
+    bool volatile                   fNotified;                  /**< Flags whether worker thread notified              */
 } VIRTIONETWORKER;
 /** Pointer to a VirtIO SCSI worker. */
@@ -304 +321 @@
 {
     R3PTRTYPE(PPDMTHREAD)           pThread;                    /**< pointer to worker thread's handle                 */
-    bool volatile                   fSleeping;                  /**< Flags whether worker thread is sleeping or not    */
-    bool volatile                   fNotified;                  /**< Flags whether worker thread notified              */
     uint16_t                        idxQueue;                   /**< Index of associated queue                         */
 } VIRTIONETWORKERR3;
@@ -530 +545 @@
 typedef CTX_SUFF(PVIRTIONET) PVIRTIONETCC;
 
+/**
+ * Wakeup the RX thread.
+ */
+static void virtioNetWakeupRxBufWaiter(PPDMDEVINS pDevIns)
+{
+    PVIRTIONET pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIONET);
+
+    AssertReturnVoid(pThis->hEventRxDescAvail != NIL_SUPSEMEVENT);
+
+    STAM_COUNTER_INC(&pThis->StatRxOverflowWakeup);
+
+    Log10Func(("%s Waking downstream driver's Rx buf waiter thread\n", INSTANCE(pThis)));
+    int rc = PDMDevHlpSUPSemEventSignal(pDevIns, pThis->hEventRxDescAvail);
+    AssertRC(rc);
+}
+
+/**
+ * @callback_method_impl{VIRTIOCORER0,pfnQueueNotified}
+ */
+static DECLCALLBACK(void) virtioNetQueueNotified(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue)
+{
+    RT_NOREF(pVirtio);
+    PVIRTIONET pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIONET);
+
+    uint16_t idxWorker;
+    if (idxQueue == CTRLQIDX)
+        idxWorker = pThis->cWorkers - 1;
+    else
+        idxWorker = idxQueue / 2;
+
+    PVIRTIONETWORKER pWorker = &pThis->aWorkers[idxWorker];
+    AssertReturnVoid(idxQueue < pThis->cVirtQueues);
+
+#if defined (IN_RING3) && defined (LOG_ENABLED)
+     RTLogFlush(NULL);
+#endif
+
+    Log10Func(("%s %s has available buffers\n", INSTANCE(pThis), VIRTQNAME(idxQueue)));
+
+    if (IS_RX_QUEUE(idxQueue))
+    {
+        Log10Func(("%s Receive buffers have been added, waking Rx thread.\n",
+            INSTANCE(pThis)));
+        virtioNetWakeupRxBufWaiter(pDevIns);
+    }
+    else
+    {
+        /* Wake queue's worker thread up if sleeping (e.g. a Tx queue, or the control queue */
+        if (!ASMAtomicXchgBool(&pWorker->fNotified, true))
+        {
+            if (ASMAtomicReadBool(&pWorker->fSleeping))
+            {
+                Log10Func(("%s waking %s worker.\n", INSTANCE(pThis), VIRTQNAME(idxQueue)));
+                int rc = PDMDevHlpSUPSemEventSignal(pDevIns, pWorker->hEvtProcess);
+                AssertRC(rc);
+            }
+        }
+    }
+}
+
 #ifdef IN_RING3 /* spans most of the file, at the moment. */
 
@@ -542 +617 @@
 }
 
-/**
- * Wakeup the RX thread.
- */
-static void virtioNetR3WakeupRxBufWaiter(PPDMDEVINS pDevIns)
-{
-    PVIRTIONET pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIONET);
-
-    AssertReturnVoid(pThis->hEventRxDescAvail != NIL_SUPSEMEVENT);
-
-    STAM_COUNTER_INC(&pThis->StatRxOverflowWakeup);
-
-    Log10Func(("%s Waking downstream driver's Rx buf waiter thread\n", INSTANCE(pThis)));
-    int rc = PDMDevHlpSUPSemEventSignal(pDevIns, pThis->hEventRxDescAvail);
-    AssertRC(rc);
-}
 
 DECLINLINE(void) virtioNetR3SetVirtqNames(PVIRTIONET pThis)
@@ -1033 +1093 @@
      * to tell it that's going to happen...
      */
-    virtioNetR3WakeupRxBufWaiter(pDevIns);
+    virtioNetWakeupRxBufWaiter(pDevIns);
 
     PDMDevHlpSetAsyncNotification(pDevIns, virtioNetR3DeviceQuiesced);
@@ -1068 +1128 @@
 
     virtioNetR3QuiesceDevice(pDevIns, enmType);
-    virtioNetR3WakeupRxBufWaiter(pDevIns);
+    virtioNetWakeupRxBufWaiter(pDevIns);
 }
 
@@ -2158 +2218 @@
     }
 
-    int cPkts = virtioCoreR3QueuePendingCount(pVirtio->pDevIns, pVirtio, idxTxQueue);
+    int cPkts = virtioCoreR3QueuePendingCount(pVirtio->pDevInsR3, pVirtio, idxTxQueue);
     if (!cPkts)
     {
@@ -2175 +2235 @@
     int rc;
     PVIRTIO_DESC_CHAIN_T pDescChain = NULL;
-    while ((rc = virtioCoreR3QueuePeek(pVirtio->pDevIns, pVirtio, idxTxQueue, &pDescChain)) == VINF_SUCCESS)
+    while ((rc = virtioCoreR3QueuePeek(pVirtio->pDevInsR3, pVirtio, idxTxQueue, &pDescChain)) == VINF_SUCCESS)
     {
         Log10Func(("%s fetched descriptor chain from %s\n", INSTANCE(pThis), VIRTQNAME(idxTxQueue)));
@@ -2205 +2265 @@
             uint64_t uOffset;
 
-            /** @todo Optimize away the extra copying! (lazy bird) */
             PPDMSCATTERGATHER pSgBufToPdmLeafDevice;
             rc = pThisCC->pDrv->pfnAllocBuf(pThisCC->pDrv, uSize, pGso, &pSgBufToPdmLeafDevice);
@@ -2265 +2324 @@
 
             /* No data to return to guest, but call is needed put elem (e.g. desc chain) on used ring */
-            virtioCoreR3QueuePut(pVirtio->pDevIns, pVirtio, idxTxQueue, NULL, pDescChain, false);
+            virtioCoreR3QueuePut(pVirtio->pDevInsR3, pVirtio, idxTxQueue, NULL, pDescChain, false);
 
             /* Update used ring idx and notify guest that we've transmitted the data it sent */
-            virtioCoreQueueSync(pVirtio->pDevIns, pVirtio, idxTxQueue, false);
+            virtioCoreQueueSync(pVirtio->pDevInsR3, pVirtio, idxTxQueue, false);
         }
 
@@ -2297 +2356 @@
           selection algorithm feasible or even necessary */
     virtioNetR3TransmitPendingPackets(pDevIns, pThis, pThisCC, TXQIDX(0), false /*fOnWorkerThread*/);
-}
-
-/**
- * @callback_method_impl{VIRTIOCORER3,pfnQueueNotified}
- */
-static DECLCALLBACK(void) virtioNetR3QueueNotified(PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC, uint16_t idxQueue)
-{
-    PVIRTIONET         pThis     = RT_FROM_MEMBER(pVirtio, VIRTIONET, Virtio);
-    PVIRTIONETCC       pThisCC   = RT_FROM_MEMBER(pVirtioCC, VIRTIONETCC, Virtio);
-    PPDMDEVINS         pDevIns   = pThisCC->pDevIns;
-
-    uint16_t idxWorker;
-    if (idxQueue == CTRLQIDX)
-        idxWorker = pThis->cWorkers - 1;
-    else
-        idxWorker = idxQueue / 2;
-
-    PVIRTIONETWORKER   pWorker   = &pThis->aWorkers[idxWorker];
-    PVIRTIONETWORKERR3 pWorkerR3 = &pThisCC->aWorkers[idxWorker];
-    AssertReturnVoid(idxQueue < pThis->cVirtQueues);
-
-#ifdef LOG_ENABLED
-    RTLogFlush(NULL);
-#endif
-
-    Log10Func(("%s %s has available buffers\n", INSTANCE(pThis), VIRTQNAME(idxQueue)));
-
-    if (IS_RX_QUEUE(idxQueue))
-    {
-        Log10Func(("%s Receive buffers have been added, waking Rx thread.\n",
-            INSTANCE(pThis)));
-        virtioNetR3WakeupRxBufWaiter(pDevIns);
-    }
-    else
-    {
-        /* Wake queue's worker thread up if sleeping (e.g. a Tx queue, or the control queue */
-        if (!ASMAtomicXchgBool(&pWorkerR3->fNotified, true))
-        {
-            if (ASMAtomicReadBool(&pWorkerR3->fSleeping))
-            {
-                Log10Func(("%s waking %s worker.\n", INSTANCE(pThis), VIRTQNAME(idxQueue)));
-                int rc = PDMDevHlpSUPSemEventSignal(pDevIns, pWorker->hEvtProcess);
-                AssertRC(rc);
-            }
-        }
-    }
 }
 
@@ -2367 +2380 @@
         if (virtioCoreQueueIsEmpty(pDevIns, &pThis->Virtio, idxQueue))
         {
+            /* Atomic interlocks avoid missing alarm while going to sleep & notifier waking the awoken */
             virtioCoreQueueSetNotify(&pThis->Virtio, idxQueue, true);
-            /* Atomic interlocks avoid missing alarm while going to sleep & notifier waking the awoken */
-            ASMAtomicWriteBool(&pWorkerR3->fSleeping, true);
-            bool fNotificationSent = ASMAtomicXchgBool(&pWorkerR3->fNotified, false);
+            ASMAtomicWriteBool(&pWorker->fSleeping, true);
+            bool fNotificationSent = ASMAtomicXchgBool(&pWorker->fNotified, false);
             if (!fNotificationSent)
             {
-                virtioCoreQueueSetNotify(&pThis->Virtio, idxQueue, true);
                 Log10Func(("%s %s worker sleeping...\n", INSTANCE(pThis), VIRTQNAME(idxQueue)));
-                Assert(ASMAtomicReadBool(&pWorkerR3->fSleeping));
+                Assert(ASMAtomicReadBool(&pWorker->fSleeping));
                 int rc = PDMDevHlpSUPSemEventWaitNoResume(pDevIns, pWorker->hEvtProcess, RT_INDEFINITE_WAIT);
                 STAM_COUNTER_INC(&pThis->StatTransmitByThread);
@@ -2387 +2399 @@
                 }
                 Log10Func(("%s %s worker woken\n", INSTANCE(pThis), VIRTQNAME(idxQueue)));
-                ASMAtomicWriteBool(&pWorkerR3->fNotified, false);
+                ASMAtomicWriteBool(&pWorker->fNotified, false);
             }
-            ASMAtomicWriteBool(&pWorkerR3->fSleeping, false);
+            ASMAtomicWriteBool(&pWorker->fSleeping, false);
             virtioCoreQueueSetNotify(&pThis->Virtio, idxQueue, false);
         }
@@ -2414 +2426 @@
                  Log10Func(("%s Notified of data to transmit\n", INSTANCE(pThis)));
                  virtioNetR3TransmitPendingPackets(pDevIns, pThis, pThisCC,
-                                                   idxQueue, true /* fOnWorkerThread */);
+                                                   idxQueue, false /* fOnWorkerThread */);
              }
 
              /* Rx queues aren't handled by our worker threads. Instead, the PDM network
               * leaf driver invokes PDMINETWORKDOWN.pfnWaitReceiveAvail() callback,
-              * which waits until notified directly by virtioNetR3QueueNotified()
+              * which waits until notified directly by virtioNetQueueNotified()
               * that guest IN buffers have been added to receive virt queue.
               */
@@ -2427 +2439 @@
 }
 
+#ifdef USING_CRITICAL_SECTION
 DECLINLINE(int) virtioNetR3CsEnter(PPDMDEVINS pDevIns, PVIRTIONET pThis, int rcBusy)
 {
@@ -2440 +2453 @@
     LogFunc(("%s CS unimplemented. What does the critical section protect in orig driver??", INSTANCE(pThis)));
 }
-
+#endif /* USING_CRITICAL_SECTION */
 
 /**
@@ -2451 +2464 @@
     RT_NOREF(pTimer, pvUser);
 
-    int rc = virtioNetR3CsEnter(pDevIns, pThis, VERR_SEM_BUSY);
-    AssertRCReturnVoid(rc);
+    ENTER_CRITICAL_SECTION;
 
     SET_LINK_UP(pThis);
 
-    virtioNetR3WakeupRxBufWaiter(pDevIns);
-
-    virtioNetR3CsLeave(pDevIns, pThis);
+    virtioNetWakeupRxBufWaiter(pDevIns);
+
+    LEAVE_CRITICAL_SECTION;
 
     LogFunc(("%s Link is up\n", INSTANCE(pThis)));
@@ -2646 +2658 @@
             (void) virtioCoreR3QueueAttach(&pThis->Virtio, idxQueue, VIRTQNAME(idxQueue));
             pThis->afQueueAttached[idxQueue] = true;
-            virtioCoreQueueIsEmpty(pThisCC->pDevIns, &pThis->Virtio, idxQueue);
-            virtioCoreQueueSetNotify(&pThis->Virtio, idxQueue, true);
+            if (virtioCoreQueueIsEmpty(pThisCC->pDevIns, &pThis->Virtio, idxQueue))
+                virtioCoreQueueSetNotify(&pThis->Virtio, idxQueue, true);
         }
     }
@@ -2695 +2707 @@
     AssertLogRelReturnVoid(iLUN == 0);
 
-    int rc = virtioNetR3CsEnter(pDevIns, pThis, VERR_SEM_BUSY);
-    AssertMsgRCReturnVoid(rc, ("Failed to enter critical section"));
+    ENTER_CRITICAL_SECTION;
+
+    RT_NOREF(pThis);
 
     /*
@@ -2704 +2717 @@
     pThisCC->pDrv     = NULL;
 
-    virtioNetR3CsLeave(pDevIns, pThis);
+    LEAVE_CRITICAL_SECTION;
 }
 
@@ -2723 +2736 @@
     AssertLogRelReturn(iLUN == 0, VERR_PDM_NO_SUCH_LUN);
 
-    int rc = virtioNetR3CsEnter(pDevIns, pThis, VERR_SEM_BUSY);
-    AssertMsgRCReturn(rc, ("Failed to enter critical section"), rc);
-
-    rc = PDMDevHlpDriverAttach(pDevIns, 0, &pDevIns->IBase, &pThisCC->pDrvBase, "Network Port");
+    ENTER_CRITICAL_SECTION;
+
+    RT_NOREF(pThis);
+
+    int rc = PDMDevHlpDriverAttach(pDevIns, 0, &pDevIns->IBase, &pThisCC->pDrvBase, "Network Port");
     if (RT_SUCCESS(rc))
     {
@@ -2737 +2751 @@
                     Log(("%s No attached driver!\n", INSTANCE(pThis)));
 
-    virtioNetR3CsLeave(pDevIns, pThis);
+    LEAVE_CRITICAL_SECTION;
     return rc;
 
@@ -2811 +2825 @@
      */
     Log7Func(("PDM device instance: %d\n", iInstance));
-
 
     RTStrPrintf(pThis->szInstanceName, sizeof(pThis->szInstanceName), "VIRTIONET", iInstance);
@@ -2864 +2877 @@
     memcpy(pThis->virtioNetConfig.uMacAddress.au8, pThis->macConfigured.au8, sizeof(pThis->virtioNetConfig.uMacAddress)); /* TBD */
 
+
+    LogFunc(("RC=%RTbool R0=%RTbool\n", pDevIns->fRCEnabled, pDevIns->fR0Enabled));
+
     /*
      * Do core virtio initialization.
@@ -2877 +2893 @@
 
     /* Initialize the generic Virtio core: */
+    pThisCC->Virtio.pfnQueueNotified        = virtioNetQueueNotified;
     pThisCC->Virtio.pfnStatusChanged        = virtioNetR3StatusChanged;
-    pThisCC->Virtio.pfnQueueNotified        = virtioNetR3QueueNotified;
     pThisCC->Virtio.pfnDevCapRead           = virtioNetR3DevCapRead;
     pThisCC->Virtio.pfnDevCapWrite          = virtioNetR3DevCapWrite;
@@ -3014 +3030 @@
     PVIRTIONET   pThis   = PDMDEVINS_2_DATA(pDevIns, PVIRTIONET);
     PVIRTIONETCC pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PVIRTIONETCC);
-
-LogRelFunc(("\n"));
-    return virtioCoreRZInit(pDevIns, &pThis->Virtio, &pThisCC->Virtio);
+    pThisCC->Virtio.pfnQueueNotified = virtioNetQueueNotified;
+    return virtioCoreRZInit(pDevIns, &pThis->Virtio);
 }
 
 #endif /* !IN_RING3 */
 
-
-
 /**
  * The device registration structure.
@@ -3028 +3041 @@
 const PDMDEVREG g_DeviceVirtioNet_1_0 =
 {
-    /* .uVersion = */             PDM_DEVREG_VERSION,
+    /* .uVersion = */               PDM_DEVREG_VERSION,
     /* .uReserved0 = */             0,
     /* .szName = */                 "virtio-net-1-dot-0",
-    /* .fFlags = */                 PDM_DEVREG_FLAGS_DEFAULT_BITS | PDM_DEVREG_FLAGS_NEW_STYLE | PDM_DEVREG_FLAGS_RZ
-                                    | PDM_DEVREG_FLAGS_FIRST_SUSPEND_NOTIFICATION
-                                    | PDM_DEVREG_FLAGS_FIRST_POWEROFF_NOTIFICATION,
+    /* .fFlags = */                 PDM_DEVREG_FLAGS_DEFAULT_BITS | PDM_DEVREG_FLAGS_NEW_STYLE | PDM_DEVREG_FLAGS_RZ,
     /* .fClass = */                 PDM_DEVREG_CLASS_NETWORK,
     /* .cMaxInstances = */          ~0U,

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

@@ -343 +343 @@
 {
     SUPSEMEVENT                     hEvtProcess;                /**< handle of associated sleep/wake-up semaphore      */
+    bool volatile                   fSleeping;                  /**< Flags whether worker thread is sleeping or not    */
+    bool volatile                   fNotified;                  /**< Flags whether worker thread notified              */
 } VIRTIOSCSIWORKER;
 /** Pointer to a VirtIO SCSI worker. */
@@ -353 +355 @@
 {
     R3PTRTYPE(PPDMTHREAD)           pThread;                    /**< pointer to worker thread's handle                 */
-    bool volatile                   fSleeping;                  /**< Flags whether worker thread is sleeping or not    */
-    bool volatile                   fNotified;                  /**< Flags whether worker thread notified              */
     uint16_t                        auRedoDescs[VIRTQ_MAX_SIZE];/**< List of previously suspended reqs to re-submit    */
     uint16_t                        cRedoDescs;                 /**< Number of redo desc chain head desc idxes in list */
@@ -567 +567 @@
 } VIRTIOSCSIREQ;
 typedef VIRTIOSCSIREQ *PVIRTIOSCSIREQ;
+
+
+/**
+ * @callback_method_impl{VIRTIOCORER0,pfnQueueNotified}
+ */
+static DECLCALLBACK(void) virtioScsiNotified(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t qIdx)
+{
+
+    RT_NOREF(pVirtio);
+    PVIRTIOSCSI pThis   = PDMDEVINS_2_DATA(pDevIns, PVIRTIOSCSI);
+
+    AssertReturnVoid(qIdx < VIRTIOSCSI_QUEUE_CNT);
+    PVIRTIOSCSIWORKER pWorker = &pThis->aWorkers[qIdx];
+
+#if defined (IN_RING3) && defined (LOG_ENABLED)
+   RTLogFlush(NULL);
+#endif
+
+    if (qIdx == CONTROLQ_IDX || IS_REQ_QUEUE(qIdx))
+    {
+        Log6Func(("%s has available data\n", VIRTQNAME(qIdx)));
+        /* Wake queue's worker thread up if sleeping */
+        if (!ASMAtomicXchgBool(&pWorker->fNotified, true))
+        {
+            if (ASMAtomicReadBool(&pWorker->fSleeping))
+            {
+                Log6Func(("waking %s worker.\n", VIRTQNAME(qIdx)));
+                int rc = PDMDevHlpSUPSemEventSignal(pDevIns, pWorker->hEvtProcess);
+                AssertRC(rc);
+            }
+        }
+    }
+    else if (qIdx == EVENTQ_IDX)
+    {
+        Log3Func(("Driver queued buffer(s) to %s\n", VIRTQNAME(qIdx)));
+//        if (ASMAtomicXchgBool(&pThis->fEventsMissed, false))
+//            virtioScsiR3ReportEventsMissed(pDevIns, pThis, 0);
+    }
+    else
+        LogFunc(("Unexpected queue idx (ignoring): %d\n", qIdx));
+}
 
 
@@ -1509 +1550 @@
         {
             /* Atomic interlocks avoid missing alarm while going to sleep & notifier waking the awoken */
-            ASMAtomicWriteBool(&pWorkerR3->fSleeping, true);
-            bool fNotificationSent = ASMAtomicXchgBool(&pWorkerR3->fNotified, false);
+            ASMAtomicWriteBool(&pWorker->fSleeping, true);
+            bool fNotificationSent = ASMAtomicXchgBool(&pWorker->fNotified, false);
             if (!fNotificationSent)
             {
                 Log6Func(("%s worker sleeping...\n", VIRTQNAME(qIdx)));
-                Assert(ASMAtomicReadBool(&pWorkerR3->fSleeping));
+                Assert(ASMAtomicReadBool(&pWorker->fSleeping));
                 int rc = PDMDevHlpSUPSemEventWaitNoResume(pDevIns, pWorker->hEvtProcess, RT_INDEFINITE_WAIT);
                 AssertLogRelMsgReturn(RT_SUCCESS(rc) || rc == VERR_INTERRUPTED, ("%Rrc\n", rc), rc);
@@ -1522 +1563 @@
                     continue;
                 Log6Func(("%s worker woken\n", VIRTQNAME(qIdx)));
-                ASMAtomicWriteBool(&pWorkerR3->fNotified, false);
+                ASMAtomicWriteBool(&pWorker->fNotified, false);
             }
-            ASMAtomicWriteBool(&pWorkerR3->fSleeping, false);
+            ASMAtomicWriteBool(&pWorker->fSleeping, false);
         }
 
@@ -1624 +1665 @@
 
 #endif
-
-/**
- * @callback_method_impl{VIRTIOCORER3,pfnQueueNotified}
- */
-static DECLCALLBACK(void) virtioScsiR3Notified(PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC, uint16_t qIdx)
-{
-    PVIRTIOSCSI         pThis     = RT_FROM_MEMBER(pVirtio, VIRTIOSCSI, Virtio);
-    PVIRTIOSCSICC       pThisCC   = RT_FROM_MEMBER(pVirtioCC, VIRTIOSCSICC, Virtio);
-    PPDMDEVINS          pDevIns   = pThisCC->pDevIns;
-    AssertReturnVoid(qIdx < VIRTIOSCSI_QUEUE_CNT);
-    PVIRTIOSCSIWORKER   pWorker   = &pThis->aWorkers[qIdx];
-    PVIRTIOSCSIWORKERR3 pWorkerR3 = &pThisCC->aWorkers[qIdx];
-
-#ifdef LOG_ENABLED
-    RTLogFlush(NULL);
-#endif
-
-    if (qIdx == CONTROLQ_IDX || IS_REQ_QUEUE(qIdx))
-    {
-        Log6Func(("%s has available data\n", VIRTQNAME(qIdx)));
-        /* Wake queue's worker thread up if sleeping */
-        if (!ASMAtomicXchgBool(&pWorkerR3->fNotified, true))
-        {
-            if (ASMAtomicReadBool(&pWorkerR3->fSleeping))
-            {
-                Log6Func(("waking %s worker.\n", VIRTQNAME(qIdx)));
-                int rc = PDMDevHlpSUPSemEventSignal(pDevIns, pWorker->hEvtProcess);
-                AssertRC(rc);
-            }
-        }
-    }
-    else if (qIdx == EVENTQ_IDX)
-    {
-        Log3Func(("Driver queued buffer(s) to %s\n", VIRTQNAME(qIdx)));
-        if (ASMAtomicXchgBool(&pThis->fEventsMissed, false))
-            virtioScsiR3ReportEventsMissed(pDevIns, pThis, 0);
-    }
-    else
-        LogFunc(("Unexpected queue idx (ignoring): %d\n", qIdx));
-}
 
 /**
@@ -2320 +2321 @@
     for (uint16_t qIdx = 0; qIdx < VIRTIOSCSI_REQ_QUEUE_CNT; qIdx++)
     {
-        if (ASMAtomicReadBool(&pThisCC->aWorkers[qIdx].fSleeping))
+        if (ASMAtomicReadBool(&pThis->aWorkers[qIdx].fSleeping))
         {
             Log6Func(("waking %s worker.\n", VIRTQNAME(qIdx)));
@@ -2436 +2437 @@
      */
     pThisCC->pDevIns = pDevIns;
+//    pDevIns->pVirtio = &pThis->Virtio;
 
     LogFunc(("PDM device instance: %d\n", iInstance));
@@ -2481 +2483 @@
 
     /* Initialize the generic Virtio core: */
+    pThisCC->Virtio.pfnQueueNotified        = virtioScsiNotified;
     pThisCC->Virtio.pfnStatusChanged        = virtioScsiR3StatusChanged;
-    pThisCC->Virtio.pfnQueueNotified        = virtioScsiR3Notified;
     pThisCC->Virtio.pfnDevCapRead           = virtioScsiR3DevCapRead;
     pThisCC->Virtio.pfnDevCapWrite          = virtioScsiR3DevCapWrite;
@@ -2642 +2644 @@
 {
     PDMDEV_CHECK_VERSIONS_RETURN(pDevIns);
+
     PVIRTIOSCSI   pThis   = PDMDEVINS_2_DATA(pDevIns, PVIRTIOSCSI);
     PVIRTIOSCSICC pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PVIRTIOSCSICC);
 
-    return virtioCoreRZInit(pDevIns, &pThis->Virtio, &pThisCC->Virtio);
+
+    pThisCC->Virtio.pfnQueueNotified = virtioScsiNotified;
+    return virtioCoreRZInit(pDevIns, &pThis->Virtio);
 }
 

trunk/src/VBox/Devices/VirtIO/Virtio_1_0.cpp

@@ -123 +123 @@
  * @{ */
 
-#if 0 /* unused */
-DECLINLINE(int) virtqIsEventNeeded(uint16_t uEventIdx, uint16_t uDescIdxNew, uint16_t uDescIdxOld)
-{
-    return (uint16_t)(uDescIdxNew - uEventIdx - 1) < (uint16_t)(uDescIdxNew - uDescIdxOld);
-}
-#endif
-
 /**
  * Accessor for virtq descriptor
@@ -160 +153 @@
     return uDescIdx;
 }
-#endif
-
-DECLINLINE(uint16_t) virtioReadAvailRingIdx(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue)
-{
-    uint16_t uIdx = 0;
-    AssertMsg(pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK, ("Called with guest driver not ready\n"));
-    PDMDevHlpPCIPhysRead(pDevIns,
-                      pVirtio->aGCPhysQueueAvail[idxQueue] + RT_UOFFSETOF(VIRTQ_AVAIL_T, uIdx),
-                      &uIdx, sizeof(uIdx));
-    return uIdx;
-}
-
-DECLINLINE(bool) virtqIsEmpty(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue)
-{
-    uint16_t uAvailGuestIdx = virtioReadAvailRingIdx(pDevIns, pVirtio, idxQueue);
-    bool fEmpty = uAvailGuestIdx == pVirtio->virtqState[idxQueue].uAvailIdx;
-
-    Log6Func(("%s idx=%u, shadow idx=%u (%s)\n",
-             VIRTQNAME(pVirtio, idxQueue), uAvailGuestIdx, pVirtio->virtqState[idxQueue].uAvailIdx,
-             fEmpty ? "Queue empty" : "Queue has available descriptors"));
-    return fEmpty;
-}
-
-#if 0 /* unused - Will be used when VIRTIO_F_EVENT_IDX optional feature is implemented, VirtIO 1.0, 2.4.7 */
-DECLINLINE(uint16_t) virtioReadAvailFlags(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue)
-{
-    uint16_t fFlags;
-    AssertMsg(pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK, ("Called with guest driver not ready\n"));
-    PDMDevHlpPCIPhysRead(pDevIns,
-                      pVirtio->aGCPhysQueueAvail[idxQueue] + RT_UOFFSETOF(VIRTQ_AVAIL_T, fFlags),
-                      &fFlags, sizeof(fFlags));
-    return fFlags;
-}
-#endif
-
-#ifdef IN_RING3
+
 DECLINLINE(uint16_t) virtioReadAvailUsedEvent(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue)
 {
@@ -207 +165 @@
 }
 #endif
+
+DECLINLINE(uint16_t) virtioReadAvailRingIdx(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue)
+{
+    uint16_t uIdx = 0;
+    AssertMsg(pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK, ("Called with guest driver not ready\n"));
+    PDMDevHlpPCIPhysRead(pDevIns,
+                      pVirtio->aGCPhysQueueAvail[idxQueue] + RT_UOFFSETOF(VIRTQ_AVAIL_T, uIdx),
+                      &uIdx, sizeof(uIdx));
+    return uIdx;
+}
+
+DECLINLINE(bool) virtqIsEmpty(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue)
+{
+    uint16_t uAvailGuestIdx = virtioReadAvailRingIdx(pDevIns, pVirtio, idxQueue);
+    bool fEmpty = uAvailGuestIdx == pVirtio->virtqState[idxQueue].uAvailIdx;
+
+    Log6Func(("%s idx=%u, shadow idx=%u (%s)\n",
+             VIRTQNAME(pVirtio, idxQueue), uAvailGuestIdx, pVirtio->virtqState[idxQueue].uAvailIdx,
+             fEmpty ? "Queue empty" : "Queue has available descriptors"));
+    return fEmpty;
+}
+
+DECLINLINE(uint16_t) virtioReadAvailRingFlags(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue)
+{
+    uint16_t fFlags;
+    AssertMsg(pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK, ("Called with guest driver not ready\n"));
+    PDMDevHlpPCIPhysRead(pDevIns,
+                      pVirtio->aGCPhysQueueAvail[idxQueue] + RT_UOFFSETOF(VIRTQ_AVAIL_T, fFlags),
+                      &fFlags, sizeof(fFlags));
+    return fFlags;
+}
 
 /** @} */
@@ -422 +411 @@
     for (unsigned i = 0; i < RT_ELEMENTS(s_aFeatures); ++i)
     {
-        bool isOffered    = !!(pVirtio->uDeviceFeatures  & s_aFeatures[i].fFeatureBit);
-        bool isNegotiated = !!(pVirtio->uDriverFeatures  & s_aFeatures[i].fFeatureBit);
+        bool isOffered    = RT_BOOL(pVirtio->uDeviceFeatures & s_aFeatures[i].fFeatureBit);
+        bool isNegotiated = RT_BOOL(pVirtio->uDriverFeatures & s_aFeatures[i].fFeatureBit);
         cp += RTStrPrintf(cp, cbBuf - (cp - pszBuf), "        %s       %s   %s",
                           isOffered ? "+" : "-", isNegotiated ? "x" : " ", s_aFeatures[i].pcszDesc);
@@ -810 +799 @@
 void virtioCoreNotifyConfigChanged(PVIRTIOCORE pVirtio)
 {
-    virtioKick(pVirtio->pDevIns, pVirtio, VIRTIO_ISR_DEVICE_CONFIG, pVirtio->uMsixConfig, false);
+    virtioKick(pVirtio->pDevInsR3, pVirtio, VIRTIO_ISR_DEVICE_CONFIG, pVirtio->uMsixConfig, false);
 }
 
@@ -824 +813 @@
     if (pVirtio->uDeviceStatus & VIRTIO_STATUS_DRIVER_OK)
     {
-        uint16_t fFlags = virtioReadUsedRingFlags(pVirtio->pDevIns, pVirtio, idxQueue);
+        uint16_t fFlags = virtioReadUsedRingFlags(pVirtio->pDevInsR3, pVirtio, idxQueue);
 
         if (fEnabled)
@@ -831 +820 @@
             fFlags |= VIRTQ_USED_F_NO_NOTIFY;
 
-        virtioWriteUsedRingFlags(pVirtio->pDevIns, pVirtio, idxQueue, fFlags);
+        virtioWriteUsedRingFlags(pVirtio->pDevInsR3, pVirtio, idxQueue, fFlags);
     }
 }
@@ -848 +837 @@
     {
         pVirtio->fGenUpdatePending = true;
-        virtioKick(pVirtio->pDevIns, pVirtio, VIRTIO_ISR_DEVICE_CONFIG, pVirtio->uMsixConfig, false /* fForce */);
+        virtioKick(pVirtio->pDevInsR3, pVirtio, VIRTIO_ISR_DEVICE_CONFIG, pVirtio->uMsixConfig, false /* fForce */);
     }
 }
@@ -915 +904 @@
                     ("Guest driver not in ready state.\n"), VERR_INVALID_STATE);
 
-    if (virtioCoreQueueIsEmpty(pVirtio->pDevIns, pVirtio, idxQueue))
+    if (virtioCoreQueueIsEmpty(pVirtio->pDevInsR3, pVirtio, idxQueue))
         return VERR_NOT_AVAILABLE;
 
@@ -1024 +1013 @@
         size_t cbTarget = virtioCoreSgBufCalcTotalLength(pSgPhysReturn);
         cbRemain = cbTotal = RTSgBufCalcTotalLength(pSgVirtReturn);
-        AssertMsgReturn(cbTarget >= cbRemain, ("Not space to write data to phys memory"), VERR_BUFFER_OVERFLOW);
+        AssertMsgReturn(cbTarget >= cbRemain, ("No space to write data to phys memory"), VERR_BUFFER_OVERFLOW);
         virtioCoreSgBufReset(pSgPhysReturn); /* Reset ptr because req data may have already been written */
         while (cbRemain)
@@ -1085 +1074 @@
 int virtioCoreQueueSync(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue, bool fForce)
 {
-    Assert(idxQueue < RT_ELEMENTS(pVirtio->virtqState));
+    Assert(idxQueue        < RT_ELEMENTS(pVirtio->virtqState));
     PVIRTQSTATE pVirtq = &pVirtio->virtqState[idxQueue];
 
@@ -1100 +1089 @@
 }
 
-#ifdef IN_RING3
-/**
- */
-static void virtioR3QueueNotified(PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC, uint16_t idxQueue, uint16_t uNotifyIdx)
-{
+/**
+ */
+static void virtioQueueNotified(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue, uint16_t uNotifyIdx)
+{
+
+    PVIRTIOCORECC pVirtioCC = PDMDEVINS_2_DATA_CC(pDevIns, PVIRTIOCORECC);
+
     /* See VirtIO 1.0, section 4.1.5.2 It implies that idxQueue and uNotifyIdx should match.
      * Disregarding this notification may cause throughput to stop, however there's no way to know
@@ -1118 +1109 @@
 
     /* Inform client */
-    pVirtioCC->pfnQueueNotified(pVirtio, pVirtioCC, idxQueue);
-}
-#endif /* IN_RING3 */
+    pVirtioCC->pfnQueueNotified(pDevIns, pVirtio, idxQueue);
+}
 
 /**
@@ -1152 +1142 @@
         Log6Func(("...kicking guest %s, VIRTIO_F_EVENT_IDX set and threshold (%d) reached\n",
                    VIRTQNAME(pVirtio, idxQueue), (uint16_t)virtioReadAvailUsedEvent(pDevIns, pVirtio, idxQueue)));
-#else
-        Log6Func(("... kicking guest %s, VIRTIO_F_EVENT_IDX set and threshold reached\n",
-                   VIRTQNAME(pVirtio, idxQueue)));
 #endif
             virtioKick(pDevIns, pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, pVirtio->uQueueMsixVector[idxQueue], fForce);
@@ -1163 +1150 @@
         Log6Func(("...skipping interrupt %s, VIRTIO_F_EVENT_IDX set but threshold (%d) not reached (%d)\n",
                    VIRTQNAME(pVirtio, idxQueue),(uint16_t)virtioReadAvailUsedEvent(pDevIns, pVirtio, idxQueue), pVirtq->uUsedIdx));
-#else
-        Log6Func(("...skipping interrupt %s, VIRTIO_F_EVENT_IDX set but threshold not reached\n",
-                    VIRTQNAME(pVirtio, idxQueue)));
 #endif
     }
@@ -1171 +1155 @@
     {
         /** If guest driver hasn't suppressed interrupts, interrupt  */
-        if (fForce || !(virtioReadUsedRingFlags(pDevIns, pVirtio, idxQueue) & VIRTQ_AVAIL_F_NO_INTERRUPT))
+        if (fForce || !(virtioReadAvailRingFlags(pDevIns, pVirtio, idxQueue) & VIRTQ_AVAIL_F_NO_INTERRUPT))
         {
             if (fForce)
@@ -1242 +1226 @@
         pVirtio->uQueueMsixVector[idxQueue] = VIRTIO_MSI_NO_VECTOR;
 
-    virtioLowerInterrupt(pVirtio->pDevIns, pVirtio->uQueueMsixVector[idxQueue]);
+    virtioLowerInterrupt(pVirtio->pDevInsR3, pVirtio->uQueueMsixVector[idxQueue]);
 }
 
@@ -1476 +1460 @@
         {
             uint8_t const fNewStatus = *(uint8_t *)pv;
-            Log7Func(("Guest wrote uDeviceStatus (%#x, was %#x, change #%x) ................ (",
-                      fNewStatus, pVirtio->uDeviceStatus, fNewStatus ^ pVirtio->uDeviceStatus));
+            Log7Func(("Guest wrote uDeviceStatus ................ (", fNewStatus ^ pVirtio->uDeviceStatus));
             if (LogIs7Enabled())
                 virtioLogDeviceStatus(fNewStatus);
@@ -1579 +1562 @@
  *
  * This MMIO handler specifically supports the VIRTIO_PCI_CAP_PCI_CFG capability defined
- * in the VirtIO 1.0 specification, section 4.1.4.7, and as such is limited to cb == 1, cb == 2, or cb==4 type reads.
+ * in the VirtIO 1.0 specification, section 4.1.4.7, and as such is restricted to reads
+ * of 1, 2 or 4 bytes, only.
  *
  */
@@ -1588 +1572 @@
     AssertReturn(cb == 1 || cb == 2 || cb == 4, VERR_INVALID_PARAMETER);
     Assert(pVirtio == (PVIRTIOCORE)pvUser); RT_NOREF(pvUser);
+
 
     uint32_t offIntra;
@@ -1603 +1588 @@
          * order to maintain the config generation (see VirtIO 1.0 spec, section 4.1.4.3.1)
          */
-        bool fDevSpecificFieldChanged = !!memcmp(pVirtioCC->pbDevSpecificCfg + offIntra,
+        bool fDevSpecificFieldChanged = RT_BOOL(memcmp(pVirtioCC->pbDevSpecificCfg + offIntra,
                                                  pVirtioCC->pbPrevDevSpecificCfg + offIntra,
-                                                 RT_MIN(cb, pVirtioCC->cbDevSpecificCfg - offIntra));
+                                                 RT_MIN(cb, pVirtioCC->cbDevSpecificCfg - offIntra)));
 
         memcpy(pVirtioCC->pbPrevDevSpecificCfg, pVirtioCC->pbDevSpecificCfg, pVirtioCC->cbDevSpecificCfg);
@@ -1647 +1632 @@
  *
  * This MMIO handler specifically supports the VIRTIO_PCI_CAP_PCI_CFG capability defined
- * in the VirtIO 1.0 specification, section 4.1.4.7, and as such is limited
- * to cb == 1, cb == 2, or cb==4 type writes.
+ * in the VirtIO 1.0 specification, section 4.1.4.7, and as such is restricted to writes
+ * of 1, 2 or 4 bytes, only.
  */
 static DECLCALLBACK(VBOXSTRICTRC) virtioMmioWrite(PPDMDEVINS pDevIns, void *pvUser, RTGCPHYS off, void const *pv, unsigned cb)
@@ -1658 +1643 @@
 
     Assert(pVirtio == (PVIRTIOCORE)pvUser); RT_NOREF(pvUser);
-
     uint32_t offIntra;
     if (MATCHES_VIRTIO_CAP_STRUCT(off, cb, offIntra, pVirtio->LocDeviceCap))
@@ -1688 +1672 @@
     if (MATCHES_VIRTIO_CAP_STRUCT(off, cb, offIntra, pVirtio->LocNotifyCap) && cb == sizeof(uint16_t))
     {
-#ifdef IN_RING3
-        virtioR3QueueNotified(pVirtio, pVirtioCC, offIntra / VIRTIO_NOTIFY_OFFSET_MULTIPLIER, *(uint16_t *)pv);
+        virtioQueueNotified(pDevIns, pVirtio, offIntra / VIRTIO_NOTIFY_OFFSET_MULTIPLIER, *(uint16_t *)pv);
         return VINF_SUCCESS;
-#else
-        return VINF_IOM_R3_MMIO_WRITE;
-#endif
     }
 
@@ -1721 +1701 @@
          * (the virtio_pci_cfg_cap capability), and access data items.
          */
-        uint32_t uLength = pVirtioCC->pPciCfgCap->pciCap.uLength;
-        uint32_t uOffset = pVirtioCC->pPciCfgCap->pciCap.uOffset;
-        uint8_t  uBar    = pVirtioCC->pPciCfgCap->pciCap.uBar;
+        struct virtio_pci_cap *pPciCap = &pVirtioCC->pPciCfgCap->pciCap;
+        uint32_t uLength = pPciCap->uLength;
 
         if (   (uLength != 1 && uLength != 2 && uLength != 4)
             || cb != uLength
-            || uBar != VIRTIO_REGION_PCI_CAP)
+            ||  pPciCap->uBar != VIRTIO_REGION_PCI_CAP)
         {
             ASSERT_GUEST_MSG_FAILED(("Guest read virtio_pci_cfg_cap.pci_cfg_data using mismatching config. Ignoring\n"));
@@ -1734 +1713 @@
         }
 
-        VBOXSTRICTRC rcStrict = virtioMmioRead(pDevIns, pVirtio, uOffset, pu32Value, cb);
+        VBOXSTRICTRC rcStrict = virtioMmioRead(pDevIns, pVirtio, pPciCap->uOffset, pu32Value, cb);
         Log2Func(("virtio: Guest read  virtio_pci_cfg_cap.pci_cfg_data, bar=%d, offset=%d, length=%d, result=%d -> %Rrc\n",
-                  uBar, uOffset, uLength, *pu32Value, VBOXSTRICTRC_VAL(rcStrict)));
+                   pPciCap->uBar, pPciCap->uOffset, uLength, *pu32Value, VBOXSTRICTRC_VAL(rcStrict)));
         return rcStrict;
     }
@@ -1759 +1738 @@
          * (the virtio_pci_cfg_cap capability), and access data items. */
 
-        uint32_t uLength = pVirtioCC->pPciCfgCap->pciCap.uLength;
-        uint32_t uOffset = pVirtioCC->pPciCfgCap->pciCap.uOffset;
-        uint8_t  uBar    = pVirtioCC->pPciCfgCap->pciCap.uBar;
+        struct virtio_pci_cap *pPciCap = &pVirtioCC->pPciCfgCap->pciCap;
+        uint32_t uLength = pPciCap->uLength;
 
         if (   (uLength != 1 && uLength != 2 && uLength != 4)
             || cb != uLength
-            || uBar != VIRTIO_REGION_PCI_CAP)
+            ||  pPciCap->uBar != VIRTIO_REGION_PCI_CAP)
         {
             ASSERT_GUEST_MSG_FAILED(("Guest write virtio_pci_cfg_cap.pci_cfg_data using mismatching config. Ignoring\n"));
@@ -1771 +1749 @@
         }
 
-        VBOXSTRICTRC rcStrict = virtioMmioWrite(pDevIns, pVirtio, uOffset, &u32Value, cb);
+        VBOXSTRICTRC rcStrict = virtioMmioWrite(pDevIns, pVirtio, pPciCap->uOffset, &u32Value, cb);
         Log2Func(("Guest wrote  virtio_pci_cfg_cap.pci_cfg_data, bar=%d, offset=%x, length=%x, value=%d -> %Rrc\n",
-                  uBar, uOffset, uLength, u32Value, VBOXSTRICTRC_VAL(rcStrict)));
+                   pPciCap->uBar, pPciCap->uOffset, uLength, u32Value, VBOXSTRICTRC_VAL(rcStrict)));
         return rcStrict;
     }
@@ -1913 +1891 @@
             break;
         case kvirtIoVmStateChangedResume:
-            virtioNotifyGuestDriver(pVirtio->pDevIns, pVirtio, 0 /* idxQueue */, true /* fForce */);
+            virtioNotifyGuestDriver(pVirtio->pDevInsR3, pVirtio, 0 /* idxQueue */, true /* fForce */);
             break;
         default:
@@ -1939 +1917 @@
 
 
-/**
+/**rr
  * Setup PCI device controller and Virtio state
  *
@@ -1969 +1947 @@
     AssertLogRelReturn(pVirtioCC == PDMINS_2_DATA_CC(pDevIns, PVIRTIOCORECC), VERR_STATE_CHANGED);
 
-    pVirtio->pDevIns = pDevIns;
+    pVirtio->pDevInsR3 = pDevIns;
 
     /*
@@ -2057 +2035 @@
 
     /*
-     * Notify capability (VirtIO 1.0 spec, section 4.1.4.4). Note: uLength is based the choice
-     * of this implementation that each queue's uQueueNotifyOff is set equal to (QueueSelect) ordinal
-     * value of the queue
+     * Notify capability (VirtIO 1.0 spec, section 4.1.4.4). Note: uLength is based on the choice
+     * of this implementation to make each queue's uQueueNotifyOff equal to (QueueSelect) ordinal
+     * value of the queue (different strategies are possible according to spec).
      */
     pCfg = (PVIRTIO_PCI_CAP_T)&pPciDev->abConfig[pCfg->uCapNext];
@@ -2165 +2143 @@
      *  out size, so pad with an extra page
      */
+    int cbSize = RTStrPrintf(pVirtioCC->pcszMmioName, sizeof(pVirtioCC->pcszMmioName), "%s MMIO", pcszInstance);
+    if (cbSize <= 0)
+        return PDMDEV_SET_ERROR(pDevIns, rc, N_("virtio: out of memory allocating string")); /* can we put params in this error? */
+
     rc = PDMDevHlpPCIIORegionCreateMmio(pDevIns, VIRTIO_REGION_PCI_CAP, RT_ALIGN_32(cbRegion + PAGE_SIZE, PAGE_SIZE),
                                         PCI_ADDRESS_SPACE_MEM, virtioMmioWrite, virtioMmioRead, pVirtio,
-                                        IOMMMIO_FLAGS_READ_PASSTHRU | IOMMMIO_FLAGS_WRITE_PASSTHRU, "virtio-scsi MMIO",
+                                        IOMMMIO_FLAGS_READ_PASSTHRU | IOMMMIO_FLAGS_WRITE_PASSTHRU,
+                                        pVirtioCC->pcszMmioName,
                                         &pVirtio->hMmioPciCap);
     AssertLogRelRCReturn(rc, PDMDEV_SET_ERROR(pDevIns, rc, N_("virtio: cannot register PCI Capabilities address space")));
-
     /*
      * Statistics.
@@ -2195 +2177 @@
  * @param   pVirtio     Pointer to the shared virtio state.  This must be the first
  *                      member in the shared device instance data!
- * @param   pVirtioCC   Pointer to the current context virtio state.  This must be the
- *                      first member in the currenct context's device instance data!
- */
-int virtioCoreRZInit(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC)
+ */
+int virtioCoreRZInit(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio)
 {
     AssertLogRelReturn(pVirtio == PDMINS_2_DATA(pDevIns, PVIRTIOCORE), VERR_STATE_CHANGED);
-    AssertLogRelReturn(pVirtioCC == PDMINS_2_DATA_CC(pDevIns, PVIRTIOCORECC), VERR_STATE_CHANGED);
-
+
+#ifdef FUTURE_OPTIMIZATION
+    int rc = PDMDevHlpSetDeviceCritSect(pDevIns, PDMDevHlpCritSectGetNop(pDevIns));
+    AssertRCReturn(rc, rc);
+#endif
     int rc = PDMDevHlpMmioSetUpContext(pDevIns, pVirtio->hMmioPciCap, virtioMmioWrite, virtioMmioRead, pVirtio);
     AssertRCReturn(rc, rc);

trunk/src/VBox/Devices/VirtIO/Virtio_1_0.h

@@ -81 +81 @@
 typedef struct VIRTIOSGBUF
 {
-    PVIRTIOSGSEG paSegs;                                       /**< Pointer to the scatter/gather array       */
+    PVIRTIOSGSEG paSegs;                                        /**< Pointer to the scatter/gather array       */
     unsigned  cSegs;                                            /**< Number of segments                        */
     unsigned  idxSeg;                                           /**< Current segment we are in                 */
@@ -251 +251 @@
 {
     char                        szInstance[16];                     /**< Instance name, e.g. "VIRTIOSCSI0"         */
-    PPDMDEVINS                  pDevIns;                            /**< Client device instance                    */
+    PPDMDEVINS                  pDevInsR0;                          /**< Client device instance                    */
+    PPDMDEVINS                  pDevInsR3;                          /**< Client device instance                    */
     RTGCPHYS                    aGCPhysQueueDesc[VIRTQ_MAX_CNT];    /**< (MMIO) PhysAdr per-Q desc structs   GUEST */
     RTGCPHYS                    aGCPhysQueueAvail[VIRTQ_MAX_CNT];   /**< (MMIO) PhysAdr per-Q avail structs  GUEST */
@@ -297 +298 @@
 } VIRTIOCORE;
 
+#define MAX_NAME 64
+
 
 /**
@@ -317 +320 @@
 
     /**
-     * When guest-to-host queue notifications are enabled, the guest driver notifies the host
-     * that the avail queue has buffers, and this callback informs the client.
-     *
-     * @param   pVirtio    Pointer to the shared virtio state.
-     * @param   pVirtioCC  Pointer to the ring-3 virtio state.
-     * @param   idxQueue   Index of the notified queue
-     */
-    DECLCALLBACKMEMBER(void, pfnQueueNotified)(PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC, uint16_t idxQueue);
-
-    /**
      * Implementation-specific client callback to access VirtIO Device-specific capabilities
      * (other VirtIO capabilities and features are handled in VirtIO implementation)
@@ -347 +340 @@
      */
     DECLCALLBACKMEMBER(int,  pfnDevCapWrite)(PPDMDEVINS pDevIns, uint32_t offCap, const void *pvBuf, uint32_t cbWrite);
+
+
+    /**
+     * When guest-to-host queue notifications are enabled, the guest driver notifies the host
+     * that the avail queue has buffers, and this callback informs the client.
+     *
+     * @param   pVirtio    Pointer to the shared virtio state.
+     * @param   pVirtioCC  Pointer to the ring-3 virtio state.
+     * @param   idxQueue   Index of the notified queue
+     */
+    DECLCALLBACKMEMBER(void, pfnQueueNotified)(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue);
+
     /** @} */
+
 
     R3PTRTYPE(PVIRTIO_PCI_CFG_CAP_T)    pPciCfgCap;                 /**< Pointer to struct in the PCI configuration area. */
@@ -359 +365 @@
     R3PTRTYPE(uint8_t *)        pbPrevDevSpecificCfg;               /**< Previous read dev-specific cfg of client  */
     bool                        fGenUpdatePending;                  /**< If set, update cfg gen after driver reads */
+    char                        pcszMmioName[MAX_NAME];             /**< MMIO mapping name                         */
 } VIRTIOCORER3;
 
@@ -367 +374 @@
 typedef struct VIRTIOCORER0
 {
-    uint64_t                    uUnusedAtTheMoment;
+
+    /**
+     * When guest-to-host queue notifications are enabled, the guest driver notifies the host
+     * that the avail queue has buffers, and this callback informs the client.
+     *
+     * @param   pVirtio    Pointer to the shared virtio state.
+     * @param   pVirtioCC  Pointer to the ring-3 virtio state.
+     * @param   idxQueue   Index of the notified queue
+     */
+    DECLCALLBACKMEMBER(void, pfnQueueNotified)(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue);
+
 } VIRTIOCORER0;
 
@@ -511 +528 @@
 int      virtioCoreR3Init(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC, PVIRTIOPCIPARAMS pPciParams,
                       const char *pcszInstance, uint64_t fDevSpecificFeatures, void *pvDevSpecificCfg, uint16_t cbDevSpecificCfg);
-int      virtioCoreRZInit(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC);
+int      virtioCoreRZInit(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio);
 const char *virtioCoreGetStateChangeText(VIRTIOVMSTATECHANGED enmState);