Changeset 98063 in vbox

Timestamp:

Jan 12, 2023 3:01:04 PM (23 months ago)

Author:

vboxsync

Message:

Devices/Virtio,Devices/DevVirtioNet_1_0: Additional fixes for the virtio-net device implementation, don't reset the shadow index counters in the virtq structure when it is being attached because the guest might have used the queues before setting the DRIVER_OK flag (FreeBSD's legacy virtio-net driver included up to version 12.3 and pfSense 2.6.0). Also add a device reset callback to properly reset the device state, avoiding the RX code to work on stale data overwriting random guest memory. Some style fixes, ticketref:21201

Location:

trunk

Files:

• doc/manual/user_ChangeLogImpl.xml (modified) (1 diff)
• src/VBox/Devices/Network/DevVirtioNet_1_0.cpp (modified) (6 diffs)
• src/VBox/Devices/VirtIO/VirtioCore.cpp (modified) (4 diffs)
• src/VBox/Devices/VirtIO/VirtioCore.h (modified) (1 diff)

trunk/doc/manual/user_ChangeLogImpl.xml

@@ -82 +82 @@
 
       <listitem>
-       <para></para>
+       <para>virtio-net: Follow up fixes for FreeBSD 12.3 and pfSense 2.6.0 (bug #21201)</para>
       </listitem>
 

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

@@ -1061 +1061 @@
     PVIRTIONET pThis = PDMDEVINS_2_DATA(pDevIns, PVIRTIONET);
 
-    Log10Func(("[%s] uOffset: %d, cb: %d: %.*Rhxs\n", pThis->szInst, uOffset, cb, RT_MAX(cb, 8) , pv));
+    Log10Func(("[%s] uOffset: %u, cb: %u: %.*Rhxs\n", pThis->szInst, uOffset, cb, cb, pv));
     RT_NOREF(pThis);
     return virtioNetR3DevCfgAccess(PDMDEVINS_2_DATA(pDevIns, PVIRTIONET), uOffset, (void *)pv, cb, true /*fWrite*/);
@@ -3180 +3180 @@
          * Dispatch to the handler for the queue this worker is set up to drive
          */
-         if (pVirtq->fCtlVirtq)
-         {
-             Log10Func(("[%s] %s worker woken. Fetching desc chain\n", pThis->szInst, pVirtq->szName));
+        if (pVirtq->fCtlVirtq)
+        {
+            Log10Func(("[%s] %s worker woken. Fetching desc chain\n", pThis->szInst, pVirtq->szName));
 #ifdef VIRTIO_VBUF_ON_STACK
             VIRTQBUF_T VirtqBuf;
@@ -3188 +3188 @@
             int rc = virtioCoreR3VirtqAvailBufGet(pDevIns, &pThis->Virtio, pVirtq->uIdx, pVirtqBuf, true);
 #else /* !VIRTIO_VBUF_ON_STACK */
-             PVIRTQBUF pVirtqBuf = NULL;
-             int rc = virtioCoreR3VirtqAvailBufGet(pDevIns, &pThis->Virtio, pVirtq->uIdx, &pVirtqBuf, true);
+            PVIRTQBUF pVirtqBuf = NULL;
+            int rc = virtioCoreR3VirtqAvailBufGet(pDevIns, &pThis->Virtio, pVirtq->uIdx, &pVirtqBuf, true);
 #endif /* !VIRTIO_VBUF_ON_STACK */
-             if (rc == VERR_NOT_AVAILABLE)
-             {
+            if (rc == VERR_NOT_AVAILABLE)
+            {
                 Log10Func(("[%s] %s worker woken. Nothing found in queue\n", pThis->szInst, pVirtq->szName));
                 continue;
-             }
-             virtioNetR3Ctrl(pDevIns, pThis, pThisCC, pVirtqBuf);
+            }
+            virtioNetR3Ctrl(pDevIns, pThis, pThisCC, pVirtqBuf);
 #ifndef VIRTIO_VBUF_ON_STACK
-             virtioCoreR3VirtqBufRelease(&pThis->Virtio, pVirtqBuf);
+            virtioCoreR3VirtqBufRelease(&pThis->Virtio, pVirtqBuf);
 #endif /* !VIRTIO_VBUF_ON_STACK */
-         }
-         else /* Must be Tx queue */
-         {
-             Log10Func(("[%s] %s worker woken. Virtq has data to transmit\n",  pThis->szInst, pVirtq->szName));
-             virtioNetR3TransmitPkts(pDevIns, pThis, pThisCC, pVirtq, false /* fOnWorkerThread */);
-         }
-         /* Note: Surprise! Rx queues aren't handled by local worker threads. Instead, the PDM network leaf driver
-          * invokes PDMINETWORKDOWN.pfnWaitReceiveAvail() callback, which waits until woken by virtioNetVirtqNotified()
-          * indicating that guest IN buffers have been added to Rx virt queue.
-          */
+        }
+        else /* Must be Tx queue */
+        {
+            Log10Func(("[%s] %s worker woken. Virtq has data to transmit\n",  pThis->szInst, pVirtq->szName));
+            virtioNetR3TransmitPkts(pDevIns, pThis, pThisCC, pVirtq, false /* fOnWorkerThread */);
+        }
+        /* Note: Surprise! Rx queues aren't handled by local worker threads. Instead, the PDM network leaf driver
+         * invokes PDMINETWORKDOWN.pfnWaitReceiveAvail() callback, which waits until woken by virtioNetVirtqNotified()
+         * indicating that guest IN buffers have been added to Rx virt queue.
+         */
     }
     Log10(("[%s] %s worker thread exiting\n", pThis->szInst, pVirtq->szName));
@@ -3254 +3254 @@
                 virtioCoreVirtqEnableNotify(&pThis->Virtio, pVirtq->uIdx, true /* fEnable */);
         }
+
+        virtioNetWakeupRxBufWaiter(pThisCC->pDevIns);
     }
     else
@@ -3376 +3378 @@
     PDMIBASE_RETURN_INTERFACE(pszIID, PDMILEDPORTS,      &pThisCC->ILeds);
     return NULL;
+}
+
+/**
+ * @interface_method_impl{PDMDEVREGR3,pfnReset}
+ */
+static DECLCALLBACK(void) virtioNetR3Reset(PPDMDEVINS pDevIns)
+{
+    PVIRTIONET   pThis   = PDMDEVINS_2_DATA(pDevIns, PVIRTIONET);
+    PVIRTIONETCC pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PVIRTIONETCC);
+
+    virtioCoreR3ResetDevice(pDevIns, &pThis->Virtio, &pThisCC->Virtio);
 }
 
@@ -3702 +3715 @@
     /* .pfnMemSetup = */            NULL,
     /* .pfnPowerOn = */             NULL,
-    /* .pfnReset = */               NULL,
+    /* .pfnReset = */               virtioNetR3Reset,
     /* .pfnSuspend = */             virtioNetWakeupRxBufWaiter,
     /* .pfnResume = */              NULL,

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

@@ -590 +590 @@
     PVIRTQUEUE pVirtq = &pVirtio->aVirtqueues[uVirtq];
     pVirtq->uVirtq = uVirtq;
-    pVirtq->uAvailIdxShadow = 0;
-    pVirtq->uUsedIdxShadow  = 0;
     pVirtq->fUsedRingEvent = false;
     pVirtq->fAttached = true;
@@ -1014 +1012 @@
                     ("Guest driver not in ready state.\n"), VERR_INVALID_STATE);
 
-    Log6Func(("    Copying device data to %s, [desc:%u → used ring:%u]\n",
+    Log6Func(("    Copying device data to %s, [desc:%u -> used ring:%u]\n",
               VIRTQNAME(pVirtio, uVirtq), pVirtqBuf->uHeadIdx, pVirtq->uUsedIdxShadow));
 
@@ -1161 +1159 @@
             ("Guest driver not in ready state.\n"), VERR_INVALID_STATE);
 
-    Log6Func(("    Sync %s used ring (%u → idx)\n",
+    Log6Func(("    Sync %s used ring (%u -> idx)\n",
                         pVirtq->szName, pVirtq->uUsedIdxShadow));
 
@@ -1353 +1351 @@
     pVirtioCC->pfnStatusChanged(pVirtio, pVirtioCC, 0 /* fDriverOk */);
     virtioResetDevice(pDevIns, pVirtio);
+}
+
+DECLHIDDEN(void) virtioCoreR3ResetDevice(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC)
+{
+    virtioGuestR3WasReset(pDevIns, pVirtio, pVirtioCC);
 }
 #endif /* IN_RING3 */

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

@@ -549 +549 @@
 
 /**
+ * Resets the device state upon a VM reset for instance.
+ *
+ * @returns nothing.
+ * @param   pVirtio     Pointer to the virtio state.
+ *
+ * @note Calls back into the upper device when the status changes.
+ */
+DECLHIDDEN(void) virtioCoreR3ResetDevice(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC);
+
+/**
  * 'Attaches' host device-specific implementation's queue state to host VirtIO core
  * virtqueue management infrastructure, informing the virtio core of the name of the