Changeset 109188 in vbox for trunk/src/VBox/Devices/Network/DevVirtioNet.cpp

Timestamp:

May 7, 2025 7:42:49 AM (12 days ago)

Author:

vboxsync

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

168752

Message:

VirtioNet: Temporary code from 7.0 branch for debugging descriptor loop issue ​bugref:10572

File:

• trunk/src/VBox/Devices/Network/DevVirtioNet.cpp (modified) (17 diffs)

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

@@ -673 +673 @@
     PVIRTIONETWORKER pWorker = &pThis->aWorkers[uVirtqNbr];
 
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (ASMAtomicReadBool(&pVirtio->fRecovering))
+        LogRel(("[%s] Received notification from the guest on queue %u\n", pThis->szInst, uVirtqNbr));
+#endif /* VIRTIO_REL_INFO_DUMP */
 #if defined (IN_RING3) && defined (LOG_ENABLED)
     RTLogFlush(NULL);
@@ -792 +796 @@
  * @callback_method_impl{FNDBGFHANDLERDEV, virtio-net debugger info callback.}
  */
+#ifdef VIRTIO_REL_INFO_DUMP
+DECLCALLBACK(void) virtioNetR3Info(PPDMDEVINS pDevIns, PCDBGFINFOHLP pHlp, const char *pszArgs)
+#else  /* !VIRTIO_REL_INFO_DUMP */
 static DECLCALLBACK(void) virtioNetR3Info(PPDMDEVINS pDevIns, PCDBGFINFOHLP pHlp, const char *pszArgs)
+#endif /* !VIRTIO_REL_INFO_DUMP */
 {
     PVIRTIONET     pThis   = PDMDEVINS_2_DATA(pDevIns,  PVIRTIONET);
@@ -804 +812 @@
     bool fPointers = pszArgs && (*pszArgs == 'p' || *pszArgs == 'P'); /* "pointers" */
     bool fVirtqs   = pszArgs && (*pszArgs == 'q' || *pszArgs == 'Q'); /* "queues    */
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (pszArgs && (*pszArgs == 't' || *pszArgs == 'T'))
+    {
+        ASMAtomicWriteBool(&pThis->Virtio.fTestRecovery, true);
+        pHlp->pfnPrintf(pHlp, "Triggering recovery for %s\n", pThis->szInst);
+        return;
+    }
+#endif /* VIRTIO_REL_INFO_DUMP */
 
     /* Show basic information. */
@@ -1038 +1054 @@
         return fWrite ? VINF_SUCCESS : VINF_IOM_MMIO_UNUSED_00;
     }
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (ASMAtomicReadBool(&pThis->Virtio.fRecovering))
+        LogRel(("[%s] %s %u bytes @ dev cfg + %u: %.*Rhxs\n", pThis->szInst, fWrite ? "wrote" : "read", cb, uOffsetOfAccess, cb, pv));
+#endif /* VIRTIO_REL_INFO_DUMP */
     return VINF_SUCCESS;
 }
@@ -1537 +1557 @@
 #ifdef IN_RING3
 
+#ifdef VIRTIO_REL_INFO_DUMP
+/**
+ * Attempt to recover from the state when a descriptor loop is discovered in adapter queue.
+ * The idea is to force device reset via setting the DEVICE_NEEDS_RESET bit in the device status field.
+ * To prevent further accesses to the queue in invalid state we need to bring down the link temporarily.
+ * Unfortunately it does not seem to be handled by Linux virtio at all, so the faulty queue will remain
+ * disabled, preventing recovery.
+ *
+ * @param pDevIns               PDM instance
+ *
+ */
+DECLHIDDEN(int) virtioNetAttemptToRecover(PPDMDEVINS pDevIns)
+{
+    PVIRTIONET     pThis   = PDMDEVINS_2_DATA(pDevIns,  PVIRTIONET);
+    PVIRTIONETCC   pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PVIRTIONETCC);
+
+    if (IS_LINK_UP(pThis))
+    {
+        SET_LINK_DOWN(pThis);
+
+        /* Re-establish link in 10 seconds. */
+        int rc = PDMDevHlpTimerSetMillies(pDevIns, pThisCC->hLinkUpTimer, 10000);
+        AssertRC(rc);
+
+        LogRel(("[%s] Virtio disabled until device reset\n", pThis->szInst));
+        pThis->fVirtioReady = false;
+    }
+    virtioCoreResetAll(&pThis->Virtio);
+
+    if (ASMAtomicXchgBool(&pThis->Virtio.fRecovering, true))
+        LogRel(("[%s] Attempting recovery while in recovery!\n", pThis->szInst));
+
+    return VINF_SUCCESS;
+}
+#endif /* VIRTIO_REL_INFO_DUMP */
+
 /**
  * Perform 16-bit 1's compliment checksum on provided packet in accordance with VirtIO specification,
@@ -1651 +1707 @@
         rc = VINF_SUCCESS;
     }
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (ASMAtomicReadBool(&pThis->Virtio.fRecovering))
+        LogRel(("[%s] %sRX buffers available on %s\n", pThis->szInst, rc == VERR_INVALID_STATE ? "Enabling notify as no " : "", pRxVirtq->szName));
+#endif /* VIRTIO_REL_INFO_DUMP */
     virtioCoreVirtqEnableNotify(&pThis->Virtio, pRxVirtq->uIdx, rc == VERR_INVALID_STATE /* fEnable */);
     return rc;
@@ -2009 +2069 @@
             cbHdrEnqueued = cbPktHdr;
             int rc = virtioCoreR3VirtqAvailBufGet(pDevIns, &pThis->Virtio, pRxVirtq->uIdx, pVirtqBuf, true);
+#ifdef VIRTIO_REL_INFO_DUMP
+            if (rc == VERR_INVALID_STATE)
+                return virtioNetAttemptToRecover(pDevIns);
+            if (rc == VERR_NOT_AVAILABLE)
+                return VINF_SUCCESS;
+#endif /* VIRTIO_REL_INFO_DUMP */
             AssertMsgReturn(rc == VINF_SUCCESS || rc == VERR_NOT_AVAILABLE, ("%Rrc\n", rc), rc);
             AssertMsgReturn(rc == VINF_SUCCESS && pVirtqBuf->cbPhysReturn,
@@ -2053 +2119 @@
     PVIRTQBUF pVirtqBuf = &VirtqBuf;
     int rc = virtioCoreR3VirtqAvailBufGet(pDevIns, &pThis->Virtio, pRxVirtq->uIdx, pVirtqBuf, true);
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (rc == VERR_INVALID_STATE)
+        return virtioNetAttemptToRecover(pDevIns);
+    if (rc == VERR_NOT_AVAILABLE)
+        return VINF_SUCCESS;
+#endif /* VIRTIO_REL_INFO_DUMP */
     AssertMsgReturn(rc == VINF_SUCCESS || rc == VERR_NOT_AVAILABLE, ("%Rrc\n", rc), rc);
     AssertMsgReturn(rc == VINF_SUCCESS && pVirtqBuf->cbPhysReturn,
@@ -2080 +2152 @@
     }
     STAM_PROFILE_STOP(&pThis->StatReceiveStore, a);
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (ASMAtomicXchgBool(&pThis->Virtio.fRecovering, false))
+        LogRel(("[%s] Recovery complete on successfully receiving a packet\n", pThis->szInst));
+#endif /* VIRTIO_REL_INFO_DUMP */
     return VINF_SUCCESS;
 }
@@ -2721 +2797 @@
     if (!cPkts)
     {
+#ifdef VIRTIO_REL_INFO_DUMP
+        if (ASMAtomicReadBool(&pThis->Virtio.fRecovering))
+            LogRel(("[%s] No packets to send found on %s\n", pThis->szInst, pTxVirtq->szName));
+#endif /* VIRTIO_REL_INFO_DUMP */
         LogFunc(("[%s] No packets to send found on %s\n", pThis->szInst, pTxVirtq->szName));
 
@@ -2729 +2809 @@
         return VERR_MISSING;
     }
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (ASMAtomicReadBool(&pThis->Virtio.fRecovering))
+        LogRel(("[%s] About to transmit %d pending packet%c\n", pThis->szInst, cPkts, cPkts == 1 ? ' ' : 's'));
+#endif /* VIRTIO_REL_INFO_DUMP */
     LogFunc(("[%s] About to transmit %d pending packet%c\n", pThis->szInst, cPkts, cPkts == 1 ? ' ' : 's'));
 
@@ -2859 +2943 @@
             virtioCoreVirtqEnableNotify(&pThis->Virtio, pTxVirtq->uIdx, true /* fEnable */);
     }
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (rc == VERR_INVALID_STATE)
+    {
+        /*
+         * Our best bet for recovering from a bad queue state is to do a device reset, but
+         * it is not enough because NetKVM driver does not complete NBLs on reset. We need to lower
+         * the link in order to prevent adding new descriptors to the TX queue first. Then we
+         * need to simulate processing of all outstanding descriptor chains by simply copying
+         * all head indices from avail to used ring. Note that we have only 'peeked' into the avail
+         * ring, so the avail index should not have moved yet. This is not the case for other queues.
+         */
+        virtioNetAttemptToRecover(pDevIns);
+        virtioCorePutAllAvailBufsToUsedRing(pDevIns, &pThis->Virtio, pTxVirtq->uIdx);
+    }
+    else if (ASMAtomicXchgBool(&pThis->Virtio.fRecovering, false))
+        LogRel(("[%s] Recovery complete on successfully transmitting a packet\n", pThis->szInst));
+#endif /* VIRTIO_REL_INFO_DUMP */
     virtioNetR3SetWriteLed(pThisCC, false);
 
@@ -2891 +2992 @@
     PVIRTIONETCC pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PVIRTIONETCC);
 
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (RT_UNLIKELY(!pThis->fVirtioReady))
+    {
+        if (ASMAtomicXchgBool(&pThis->Virtio.fRecovering, false))
+        {
+            LogRel(("[%s] Recovery failed, VM reset is needed!\n", pThis->szInst));
+            return;
+        }
+    }
+#endif /* VIRTIO_REL_INFO_DUMP */
+
     SET_LINK_UP(pThis);
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (ASMAtomicReadBool(&pThis->Virtio.fRecovering))
+        LogRel(("[%s] Link is up\n", pThis->szInst));
+#endif /* VIRTIO_REL_INFO_DUMP */
     virtioNetWakeupRxBufWaiter(pDevIns);
 
@@ -3140 +3256 @@
             PVIRTQBUF pVirtqBuf = &VirtqBuf;
             int rc = virtioCoreR3VirtqAvailBufGet(pDevIns, &pThis->Virtio, pVirtq->uIdx, pVirtqBuf, true);
+#ifdef VIRTIO_REL_INFO_DUMP
+            if (rc == VERR_INVALID_STATE)
+            {
+                virtioNetAttemptToRecover(pDevIns);
+                continue;
+            }
+#endif /* VIRTIO_REL_INFO_DUMP */
             if (rc == VERR_NOT_AVAILABLE)
             {
@@ -3205 +3328 @@
     else
     {
+#ifdef VIRTIO_REL_INFO_DUMP
+        if (ASMAtomicReadBool(&pThis->Virtio.fRecovering))
+            LogRel(("[%s] Resetting...\n", pThis->szInst));
+#endif /* VIRTIO_REL_INFO_DUMP */
         Log(("\n%-23s: %s VirtIO is resetting ***\n", __FUNCTION__, pThis->szInst));
 
@@ -3233 +3360 @@
         }
     }
+
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (pThisCC->pDrv)
+        pThisCC->pDrv->pfnNotifyLinkChanged(pThisCC->pDrv, pThis->virtioNetConfig.uStatus ? PDMNETWORKLINKSTATE_UP : PDMNETWORKLINKSTATE_DOWN);
+#endif /* VIRTIO_REL_INFO_DUMP */
 }
 
@@ -3243 +3375 @@
 
     LogFunc(("[Feature Negotiation Complete] Guest Driver version is: %s\n", fLegacy ? "legacy" : "modern"));
+#ifdef VIRTIO_REL_INFO_DUMP
+    if (ASMAtomicReadBool(&pThis->Virtio.fRecovering))
+        LogRel(("[%s] Feature Negotiation Complete for %s driver\n", pThis->szInst, fLegacy ? "legacy" : "modern"));
+#endif /* VIRTIO_REL_INFO_DUMP */
     virtioNetConfigurePktHdr(pThis, fLegacy);
     virtioNetR3SetVirtqNames(pThis, fLegacy);