Changeset 83913 in vbox for trunk/src/VBox/Devices/VirtIO

Timestamp:

Apr 22, 2020 4:52:12 AM (5 years ago)

Author:

vboxsync

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

137452

Message:

Network/DevVirtioNet_1_0.cpp: Various fixes and improvements, most notably: Implemented SSM (save, restore, pause, resume, stop) all tested, implemented STAM similarly to DevVirtioNet.cpp, verified, added support for VIRTIO_F_EVENT_IDX feature, improved internal handling of Rx/Tx queue pair mgt (will facility multiqueue (MQ) feature better), fixed various @todo's added by bird

Location:

trunk/src/VBox/Devices/VirtIO

Files:

• Virtio_1_0.cpp (modified) (14 diffs)
• Virtio_1_0.h (modified) (6 diffs)

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

@@ -174 +174 @@
 DECLINLINE(bool) virtqIsEmpty(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue)
 {
-    uint16_t uAvailGuest = virtioReadAvailRingIdx(pDevIns, pVirtio, idxQueue);
-    bool fEmpty = uAvailGuest == pVirtio->virtqState[idxQueue].uAvailIdx;
-
-    Log6Func(("%s: uAvailGuest=%u uAvailIdx=%u. (%s)\n",
-             VIRTQNAME(pVirtio, idxQueue), uAvailGuest, pVirtio->virtqState[idxQueue].uAvailIdx,
+    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;
@@ -266 +266 @@
 }
 
-#if 0 /* unused - *May* be used when VIRTIO_F_EVENT_IDX optional feature is implemented VirtIO 1.0, 2.4.9.2*/
 DECLINLINE(void) virtioWriteUsedAvailEvent(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue, uint32_t uAvailEventIdx)
 {
@@ -275 +274 @@
                           &uAvailEventIdx, sizeof(uAvailEventIdx));
 }
-#endif
 
 
@@ -390 +388 @@
 }
 
+size_t virtioCoreSgBufCalcTotalLength(PVIRTIOSGBUF pGcSgBuf)
+{
+    size_t   cb = 0;
+    unsigned i  = pGcSgBuf->cSegs;
+     while (i-- > 0)
+         cb += pGcSgBuf->paSegs[i].cbSeg;
+     return cb;
+ }
+
 #ifdef LOG_ENABLED
+
+void virtioPrintFeatures(VIRTIOCORE *pVirtio)
+{
+#ifdef LOG_ENABLED
+    static struct
+    {
+        uint64_t fFeatureBit;
+        const char *pcszDesc;
+    } const s_aFeatures[] =
+    {
+        { VIRTIO_F_RING_INDIRECT_DESC,      "   RING_INDIRECT_DESC   Driver can use descriptors with VIRTQ_DESC_F_INDIRECT flag set\n" },
+        { VIRTIO_F_RING_EVENT_IDX,          "   RING_EVENT_IDX       Enables use_event and avail_event fields described in 2.4.7, 2.4.8\n" },
+        { VIRTIO_F_VERSION_1,               "   VERSION              Used to detect legacy drivers.\n" },
+    };
+
+#define MAXLINE 80
+    /* Display as a single buf to prevent interceding log messages */
+    uint16_t cbBuf = RT_ELEMENTS(s_aFeatures) * 132;
+    char *pszBuf = (char *)RTMemAllocZ(cbBuf);
+    Assert(pszBuf);
+    char *cp = pszBuf;
+    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;
+        cp += RTStrPrintf(cp, cbBuf - (cp - pszBuf), "        %s       %s   %s",
+                          isOffered ? "+" : "-", isNegotiated ? "x" : " ", s_aFeatures[i].pcszDesc);
+    }
+    Log3(("VirtIO Features Configuration\n\n"
+          "    Offered  Accepted  Feature              Description\n"
+          "    -------  --------  -------              -----------\n"
+          "%s\n", pszBuf));
+    RTMemFree(pszBuf);
+
+#else  /* !LOG_ENABLED */
+    RT_NOREF3(pThis, fFeatures, pcszText);
+#endif /* !LOG_ENABLED */
+}
+
 
 /**
@@ -910 +956 @@
     uint16_t uHeadIdx = virtioReadAvailDescIdx(pDevIns, pVirtio, idxQueue, pVirtq->uAvailIdx);
 
+    if (pVirtio->uDriverFeatures & VIRTIO_F_EVENT_IDX)
+        virtioWriteUsedAvailEvent(pDevIns,pVirtio, idxQueue, pVirtq->uAvailIdx + 1);
+
     if (fRemove)
         pVirtq->uAvailIdx++;
@@ -961 +1010 @@
     Assert(pDescChain->cRefs > 0);
 
-    AssertMsgReturn(IS_DRIVER_OK(pVirtio) /*&& pVirtio->uQueueEnable[idxQueue]*/,
-                    ("Guest driver not in ready state.\n"), VERR_INVALID_STATE);
+    AssertMsgReturn(IS_DRIVER_OK(pVirtio), ("Guest driver not in ready state.\n"), VERR_INVALID_STATE);
 
     Log6Func(("Copying client data to %s, desc chain (head desc_idx %d)\n",
@@ -973 +1021 @@
     if (pSgVirtReturn)
     {
+        size_t cbTarget = virtioCoreSgBufCalcTotalLength(pSgPhysReturn);
         cbRemain = cbTotal = RTSgBufCalcTotalLength(pSgVirtReturn);
+        AssertMsgReturn(cbTarget >= cbRemain, ("Not space to write data to phys memory"), VERR_BUFFER_OVERFLOW);
         virtioCoreSgBufReset(pSgPhysReturn); /* Reset ptr because req data may have already been written */
         while (cbRemain)
         {
-            PVIRTIOSGSEG paSeg = &pSgPhysReturn->paSegs[pSgPhysReturn->idxSeg];
-            /** @todo r=bird: Shouldn't this be: RT_MIN(pSgVirtReturn->cbSegLeft, pSgPhysReturn->cbSegLeft); */
-            cbCopy = RT_MIN(pSgVirtReturn->cbSegLeft, paSeg->cbSeg - (size_t)(pSgPhysReturn->gcPhysCur - paSeg->gcPhys));
-            Assert(cbCopy > 0); /** @todo r=bird: There is no check that there is sufficient space in the output
-                                 * buffer (pSgPhysReturn), so we might loop here forever if the caller is careless,
-                                 * right?  I'm pretty sure virtioScsiR3SendEvent could do more checks. */
+            cbCopy = RT_MIN(pSgVirtReturn->cbSegLeft,  pSgPhysReturn->cbSegLeft);
+            Assert(cbCopy > 0);
             PDMDevHlpPhysWrite(pDevIns, (RTGCPHYS)pSgPhysReturn->gcPhysCur, pSgVirtReturn->pvSegCur, cbCopy);
             RTSgBufAdvance(pSgVirtReturn, cbCopy);
@@ -1030 +1076 @@
  * @param   pVirtio     Pointer to the shared virtio state.
  * @param   idxQueue    Queue number
+ * @param   fForce      Force guest notification even if VIRTQ_USED_F_NO_NOTIFY is set
  *
  * @returns VBox status code.
@@ -1035 +1082 @@
  * @retval  VERR_INVALID_STATE VirtIO not in ready state
  */
-int virtioCoreQueueSync(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue)
+int virtioCoreQueueSync(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue, bool fForce)
 {
     Assert(idxQueue < RT_ELEMENTS(pVirtio->virtqState));
@@ -1047 +1094 @@
 
     virtioWriteUsedRingIdx(pDevIns, pVirtio, idxQueue, pVirtq->uUsedIdx);
-    virtioNotifyGuestDriver(pDevIns, pVirtio, idxQueue, false);
+    virtioNotifyGuestDriver(pDevIns, pVirtio, idxQueue, fForce);
 
     return VINF_SUCCESS;
@@ -1101 +1148 @@
         if (pVirtq->fEventThresholdReached)
         {
+#ifdef IN_RING3
+        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);
             pVirtq->fEventThresholdReached = false;
             return;
         }
-        Log6Func(("...skipping interrupt: VIRTIO_F_EVENT_IDX set but threshold not reached\n"));
+#ifdef IN_RING3
+        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
     }
     else
@@ -1112 +1172 @@
         if (fForce || !(virtioReadUsedRingFlags(pDevIns, pVirtio, idxQueue) & VIRTQ_AVAIL_F_NO_INTERRUPT))
         {
+            if (fForce)
+                Log6Func(("... kicking guest, queue %s, because force flag set\n"));
             virtioKick(pDevIns, pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, pVirtio->uQueueMsixVector[idxQueue], fForce);
             return;
         }
-/** TEMPORARY DEBUGGING HACK, EVALUATE AND REMOVE */
-virtioKick(pDevIns, pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, pVirtio->uQueueMsixVector[idxQueue], fForce);
-
-        Log6Func(("...skipping interrupt. Guest flagged VIRTQ_AVAIL_F_NO_INTERRUPT for queue\n"));
+        Log6Func(("...skipping interrupt, queue %s, Guest flagged VIRTQ_AVAIL_F_NO_INTERRUPT for queue\n"));
     }
 }
@@ -1134 +1193 @@
 {
     if (fForce)
-        Log6Func(("reason: resumed after suspend\n"));
+        Log6Func(("reason: forced\n"));
     else
     if (uCause == VIRTIO_ISR_VIRTQ_INTERRUPT)
@@ -1170 +1229 @@
 {
     Assert(idxQueue < RT_ELEMENTS(pVirtio->virtqState));
-    PVIRTQSTATE pVirtQ = &pVirtio->virtqState[idxQueue];
-    pVirtQ->uAvailIdx = 0;
-    pVirtQ->uUsedIdx  = 0;
+    PVIRTQSTATE pVirtq = &pVirtio->virtqState[idxQueue];
+    pVirtq->uAvailIdx = 0;
+    pVirtq->uUsedIdx  = 0;
+    pVirtq->fEventThresholdReached = false;
     pVirtio->uQueueEnable[idxQueue] = false;
     pVirtio->uQueueSize[idxQueue] = VIRTQ_MAX_SIZE;
     pVirtio->uQueueNotifyOff[idxQueue] = idxQueue;
     pVirtio->uQueueMsixVector[idxQueue] = idxQueue + 2;
-
     if (!pVirtio->fMsiSupport) /* VirtIO 1.0, 4.1.4.3 and 4.1.5.1.2 */
         pVirtio->uQueueMsixVector[idxQueue] = VIRTIO_MSI_NO_VECTOR;

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

@@ -269 +269 @@
     uint8_t                     uPrevDeviceStatus;                  /**< (MMIO) Prev Device Status           GUEST */
     uint8_t                     uConfigGeneration;                  /**< (MMIO) Device config sequencer       HOST */
-
     VIRTQSTATE                  virtqState[VIRTQ_MAX_CNT];          /**< Local impl-specific queue context         */
 
@@ -281 +280 @@
     /** @} */
 
-    bool                        fGenUpdatePending;                  /**< If set, update cfg gen after driver reads */
+    bool                        fGenUpdatePending;                 /**< If set, update cfg gen after driver reads */
     uint8_t                     uPciCfgDataOff;
-    uint8_t                     uISR;                               /**< Interrupt Status Register.                */
+    uint8_t                     uISR;                              /**< Interrupt Status Register.                */
     uint8_t                     fMsiSupport;
 
@@ -419 +418 @@
 
 int  virtioCoreR3QueuePendingCount(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue);
-int  virtioCoreQueueSync(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue);
+int  virtioCoreQueueSync(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue, bool fForce);
 bool virtioCoreQueueIsEmpty(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue);
 void virtioCoreQueueEnable(PVIRTIOCORE pVirtio, uint16_t idxQueue, bool fEnabled);
@@ -425 +424 @@
 void virtioCoreNotifyConfigChanged(PVIRTIOCORE pVirtio);
 void virtioCoreResetAll(PVIRTIOCORE pVirtio);
+void virtioPrintFeatures(VIRTIOCORE *pVirtio);
 
 /**
@@ -474 +474 @@
     return pVirtio->uDriverFeatures;
 }
+
 
 /**
@@ -503 +504 @@
 size_t   virtioCoreSgBufCalcTotalLength(PCVIRTIOSGBUF pGcSgBuf);
 void     virtioCoreSgBufReset(PVIRTIOSGBUF pGcSgBuf);
+size_t   virtioCoreSgBufCalcTotalLength(PVIRTIOSGBUF pGcSgBuf);
 int      virtioCoreR3SaveExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM);
 int      virtioCoreR3LoadExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM);