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

Timestamp:

Dec 15, 2021 3:51:28 PM (3 years ago)

Author:

vboxsync

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

148914

Message:

Improve transitional behavior, and save/load exec code. Some Rx buffer handling code optimization for speed, and make it easier to understand and maintain. Add missing function comments and improve others. Try to make debug logging even clearer and more succinct. And any other miscellaneous small improvements I could find. See BugRef(8651) Comment #171

Location:

trunk/src/VBox/Devices/VirtIO

Files:

• VirtioCore.cpp (modified) (65 diffs)
• VirtioCore.h (modified) (46 diffs)

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

@@ -42 +42 @@
 *   Defined Constants And Macros                                                                                                 *
 *********************************************************************************************************************************/
+
 #define INSTANCE(a_pVirtio)                 ((a_pVirtio)->szInstance)
 #define VIRTQNAME(a_pVirtio, a_uVirtq)      ((a_pVirtio)->aVirtqueues[(a_uVirtq)].szName)
 
-
 #define IS_VIRTQ_EMPTY(pDevIns, pVirtio, pVirtq) \
             (virtioCoreVirtqAvailCnt(pDevIns, pVirtio, pVirtq) == 0)
 
-
 #define IS_DRIVER_OK(a_pVirtio)             ((a_pVirtio)->fDeviceStatus & VIRTIO_STATUS_DRIVER_OK)
 #define WAS_DRIVER_OK(a_pVirtio)            ((a_pVirtio)->fPrevDeviceStatus & VIRTIO_STATUS_DRIVER_OK)
+
+/**
+ * These defines are used to track  guest virtio-net driver writing driver features accepted flags
+ * in two 32-bit operations (in arbitrary order), and one bit dedicated to ensured 'features complete'
+ * is handled once.
+ */
+#define DRIVER_FEATURES_0_WRITTEN                        1   /**< fDriverFeatures[0]  written by guest virtio-net */
+#define DRIVER_FEATURES_1_WRITTEN                        2   /**< fDriverFeatures[1]  written by guest virtio-net */
+#define DRIVER_FEATURES_0_AND_1_WRITTEN                  3   /**< Both 32-bit parts of fDriverFeatures[] written  */
+#define DRIVER_FEATURES_COMPLETE_HANDLED                 4   /**< Features negotiation complete handler called    */
 
 /**
@@ -68 +77 @@
 
 
-/** Marks the start of the virtio saved state (just for sanity). */
-#define VIRTIO_SAVEDSTATE_MARKER                        UINT64_C(0x1133557799bbddff)
-/** The current saved state version for the virtio core. */
-#define VIRTIO_SAVEDSTATE_VERSION                       UINT32_C(1)
-
-
 /*********************************************************************************************************************************
 *   Structures and Typedefs                                                                                                      *
 *********************************************************************************************************************************/
-
 
 /** @name virtq related flags
@@ -90 +92 @@
 
 /**
- * virtq related structs
- * (struct names follow VirtIO 1.0 spec, typedef use VBox style)
+ * virtq-related structs
+ * (struct names follow VirtIO 1.0 spec, field names use VBox styled naming, w/respective spec'd name in comments)
  */
 typedef struct virtq_desc
@@ -125 +127 @@
 } VIRTQ_USED_T, *PVIRTQ_USED_T;
 
-
 const char *virtioCoreGetStateChangeText(VIRTIOVMSTATECHANGED enmState)
 {
@@ -141 +142 @@
 
 static void virtioCoreNotifyGuestDriver(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t uVirtq);
-static int  virtioKick(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint8_t uCause, uint16_t uVec);
+static int  virtioNudgeGuest(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint8_t uCause, uint16_t uVec);
+
+DECLINLINE(uint16_t) virtioCoreR3CountPendingBufs(uint16_t uRingIdx, uint16_t uShadowIdx, uint16_t uQueueSize)
+{
+    if (uShadowIdx == uRingIdx)
+        return 0;
+    else
+    if (uShadowIdx > uRingIdx)
+        return uShadowIdx - uRingIdx;
+    return uQueueSize - (uRingIdx - uShadowIdx);
+}
 
 /** @name Internal queue operations
@@ -156 +167 @@
     uint16_t const cVirtqItems = RT_MAX(pVirtq->uQueueSize, 1); /* Make sure to avoid div-by-zero. */
 
-        virtioCoreGCPhysRead(pVirtio, pDevIns,
-                          pVirtq->GCPhysVirtqDesc + sizeof(VIRTQ_DESC_T) * (idxDesc % cVirtqItems),
-                          pDesc, sizeof(VIRTQ_DESC_T));
+    virtioCoreGCPhysRead(pVirtio, pDevIns,
+                         pVirtq->GCPhysVirtqDesc + sizeof(VIRTQ_DESC_T) * (idxDesc % cVirtqItems),
+                         pDesc, sizeof(VIRTQ_DESC_T));
 }
 #endif
@@ -207 +218 @@
                          pVirtq->GCPhysVirtqAvail + RT_UOFFSETOF(VIRTQ_AVAIL_T, fFlags),
                          &fFlags, sizeof(fFlags));
-
     return fFlags;
 }
@@ -249 +259 @@
 }
 
-
 #ifdef IN_RING3
 DECLINLINE(uint16_t) virtioReadUsedRingIdx(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, PVIRTQUEUE pVirtq)
@@ -289 +298 @@
 
     if (uIdxActual < uIdxShadow)
-        uIdxDelta = (uIdxActual + VIRTQ_SIZE) - uIdxShadow;
+        uIdxDelta = (uIdxActual + pVirtq->uQueueSize) - uIdxShadow;
     else
         uIdxDelta = uIdxActual - uIdxShadow;
-
-    LogFunc(("%s, %u %s\n",
-        pVirtq->szName, uIdxDelta, uIdxDelta == 1 ? "entry" : "entries"));
 
     return uIdxDelta;
@@ -320 +326 @@
     if (!pVirtio->fLegacyDriver && !pVirtq->uEnable)
     {
-        LogRelFunc(("virtq: %d (%s) not enabled\n", uVirtq, VIRTQNAME(pVirtio, uVirtq)));
+        LogRelFunc(("virtq: %s not enabled\n", VIRTQNAME(pVirtio, uVirtq)));
         return 0;
     }
-
     return virtioCoreVirtqAvailCnt(pDevIns, pVirtio, pVirtq);
 }
@@ -440 +445 @@
            else
                cbPrint = RTStrPrintf(pszOut, cbRemain, "%02x %s", c, (col + 1) % 8 ? "" : "  ");
-            ADJCURSOR(cbPrint);
+           ADJCURSOR(cbPrint);
         }
         for (uint16_t idx = row * 16; idx < row * 16 + 16; idx++)
@@ -456 +461 @@
 #undef ADJCURSOR
 }
-#endif /* LOG_ENABLED */
-
-/** API function: See header file */
-int virtioCoreIsLegacyMode(PVIRTIOCORE pVirtio)
-{
-    Log12Func(("%s", pVirtio->fLegacyDriver ? "Legacy Guest Driver handling mode\n" : ""));
-    return pVirtio->fLegacyDriver;
-}
+
 
 /** API function: See header file */
@@ -470 +468 @@
                                 int fHasIndex, uint32_t idx)
 {
-    if (!LogIs6Enabled())
-        return;
-
-    char szIdx[16];
-    if (fHasIndex)
-        RTStrPrintf(szIdx, sizeof(szIdx), "[%d]", idx);
-    else
-        szIdx[0] = '\0';
-
-    if (cb == 1 || cb == 2 || cb == 4 || cb == 8)
-    {
-        char szDepiction[64];
-        size_t cchDepiction;
-        if (uOffset != 0 || cb != uMemberSize) /* display bounds if partial member access */
-            cchDepiction = RTStrPrintf(szDepiction, sizeof(szDepiction), "%s%s[%d:%d]",
-                                       pszMember, szIdx, uOffset, uOffset + cb - 1);
+    if (LogIs6Enabled())
+    {
+        char szIdx[16];
+        if (fHasIndex)
+            RTStrPrintf(szIdx, sizeof(szIdx), "[%d]", idx);
         else
-            cchDepiction = RTStrPrintf(szDepiction, sizeof(szDepiction), "%s%s", pszMember, szIdx);
-
-        /* padding */
-        if (cchDepiction < 30)
-            szDepiction[cchDepiction++] = ' ';
-        while (cchDepiction < 30)
-            szDepiction[cchDepiction++] = '.';
-        szDepiction[cchDepiction] = '\0';
-
-        RTUINT64U uValue;
-        uValue.u = 0;
-        memcpy(uValue.au8, pv, cb);
-        Log6(("%-23s: Guest %s %s %#0*RX64\n",
-                  pszFunc, fWrite ? "wrote" : "read ", szDepiction, 2 + cb * 2, uValue.u));
-    }
-    else /* odd number or oversized access, ... log inline hex-dump style */
-    {
-        Log6(("%-23s: Guest %s %s%s[%d:%d]: %.*Rhxs\n",
-                  pszFunc, fWrite ? "wrote" : "read ", pszMember,
-                  szIdx, uOffset, uOffset + cb, cb, pv));
+            szIdx[0] = '\0';
+
+        if (cb == 1 || cb == 2 || cb == 4 || cb == 8)
+        {
+            char szDepiction[64];
+            size_t cchDepiction;
+            if (uOffset != 0 || cb != uMemberSize) /* display bounds if partial member access */
+                cchDepiction = RTStrPrintf(szDepiction, sizeof(szDepiction), "%s%s[%d:%d]",
+                                           pszMember, szIdx, uOffset, uOffset + cb - 1);
+            else
+                cchDepiction = RTStrPrintf(szDepiction, sizeof(szDepiction), "%s%s", pszMember, szIdx);
+
+            /* padding */
+            if (cchDepiction < 30)
+                szDepiction[cchDepiction++] = ' ';
+            while (cchDepiction < 30)
+                szDepiction[cchDepiction++] = '.';
+            szDepiction[cchDepiction] = '\0';
+
+            RTUINT64U uValue;
+            uValue.u = 0;
+            memcpy(uValue.au8, pv, cb);
+            Log6(("%-23s: Guest %s %s %#0*RX64\n",
+                      pszFunc, fWrite ? "wrote" : "read ", szDepiction, 2 + cb * 2, uValue.u));
+        }
+        else /* odd number or oversized access, ... log inline hex-dump style */
+        {
+            Log6(("%-23s: Guest %s %s%s[%d:%d]: %.*Rhxs\n",
+                      pszFunc, fWrite ? "wrote" : "read ", pszMember,
+                      szIdx, uOffset, uOffset + cb, cb, pv));
+        }
     }
     RT_NOREF2(fWrite, pszFunc);
@@ -512 +510 @@
 
 /**
- * Makes the MMIO-mapped Virtio fDeviceStatus registers non-cryptic (buffers to
- * keep the output clean during multi-threaded activity)
+ * Log MMIO-mapped Virtio fDeviceStatus register bitmask, naming the bits
  */
 DECLINLINE(void) virtioCoreFormatDeviceStatus(uint8_t bStatus, char *pszBuf, size_t uSize)
 {
-
-#define ADJCURSOR(len) cp += len; uSize -= len; sep = (char *)" | ";
-
+#   define ADJCURSOR(len) { cp += len; uSize -= len; sep = (char *)" | "; }
     memset(pszBuf, 0, uSize);
-    size_t len;
-    char *cp = pszBuf;
-    char *sep = (char *)"";
-
-    if (bStatus == 0) {
+    char *cp = pszBuf, *sep = (char *)"";
+    int len;
+    if (bStatus == 0)
         RTStrPrintf(cp, uSize, "RESET");
-        return;
-    }
-    if (bStatus & VIRTIO_STATUS_ACKNOWLEDGE)
-    {
-        len = RTStrPrintf(cp, uSize, "ACKNOWLEDGE");
-        ADJCURSOR(len);
-   }
-    if (bStatus & VIRTIO_STATUS_DRIVER)
-    {
-        len = RTStrPrintf(cp, uSize, "%sDRIVER", sep);
-        ADJCURSOR(len);
-    }
-    if (bStatus & VIRTIO_STATUS_FEATURES_OK)
-    {
-        len = RTStrPrintf(cp, uSize, "%sFEATURES_OK", sep);
-        ADJCURSOR(len);
-    }
-    if (bStatus & VIRTIO_STATUS_DRIVER_OK)
-    {
-        len = RTStrPrintf(cp, uSize, "%sDRIVER_OK", sep);
-        ADJCURSOR(len);
-    }
-    if (bStatus & VIRTIO_STATUS_FAILED)
-    {
-        len = RTStrPrintf(cp, uSize, "%sFAILED", sep);
-        ADJCURSOR(len);
-    }
-    if (bStatus & VIRTIO_STATUS_DEVICE_NEEDS_RESET)
-        RTStrPrintf(cp, uSize, "%sNEEDS_RESET", sep);
-
-#undef ADJCURSOR
+    else
+    {
+        if (bStatus & VIRTIO_STATUS_ACKNOWLEDGE)
+        {
+            len = RTStrPrintf(cp, uSize, "ACKNOWLEDGE");
+            ADJCURSOR(len);
+        }
+        if (bStatus & VIRTIO_STATUS_DRIVER)
+        {
+            len = RTStrPrintf(cp, uSize, "%sDRIVER", sep);
+            ADJCURSOR(len);
+        }
+        if (bStatus & VIRTIO_STATUS_FEATURES_OK)
+        {
+            len = RTStrPrintf(cp, uSize, "%sFEATURES_OK", sep);
+            ADJCURSOR(len);
+        }
+        if (bStatus & VIRTIO_STATUS_DRIVER_OK)
+        {
+            len = RTStrPrintf(cp, uSize, "%sDRIVER_OK", sep);
+            ADJCURSOR(len);
+        }
+        if (bStatus & VIRTIO_STATUS_FAILED)
+        {
+            len = RTStrPrintf(cp, uSize, "%sFAILED", sep);
+            ADJCURSOR(len);
+        }
+        if (bStatus & VIRTIO_STATUS_DEVICE_NEEDS_RESET)
+            RTStrPrintf(cp, uSize, "%sNEEDS_RESET", sep);
+    }
+#   undef ADJCURSOR
+}
+
+#endif /* LOG_ENABLED */
+
+/** API function: See header file */
+int virtioCoreIsLegacyMode(PVIRTIOCORE pVirtio)
+{
+    return pVirtio->fLegacyDriver;
 }
 
@@ -570 +571 @@
     pVirtq->uUsedIdxShadow  = 0;
     pVirtq->fUsedRingEvent = false;
+    pVirtq->fAttached = true;
     RTStrCopy(pVirtq->szName, sizeof(pVirtq->szName), pcszName);
     return VINF_SUCCESS;
+}
+
+int virtioCoreR3VirtqDetach(PVIRTIOCORE pVirtio, uint16_t uVirtqNbr)
+{
+    PVIRTQUEUE pVirtq = &pVirtio->aVirtqueues[uVirtqNbr];
+    pVirtq->uVirtq          = 0;
+    pVirtq->uAvailIdxShadow = 0;
+    pVirtq->uUsedIdxShadow  = 0;
+    pVirtq->fUsedRingEvent  = false;
+    pVirtq->fAttached       = false;
+    memset(pVirtq->szName, 0, sizeof(pVirtq->szName));
+    return VINF_SUCCESS;
+}
+
+bool virtioCoreR3VirtqIsAttached(PVIRTIOCORE pVirtio, uint16_t uVirtqNbr)
+{
+    return pVirtio->aVirtqueues[uVirtqNbr].fAttached;
+}
+
+bool virtioCoreR3VirtqIsEnabled(PVIRTIOCORE pVirtio, uint16_t uVirtqNbr)
+{
+    PVIRTQUEUE pVirtq = &pVirtio->aVirtqueues[uVirtqNbr];
+    return (bool)pVirtq->uEnable && pVirtq->GCPhysVirtqDesc;
 }
 
@@ -582 +607 @@
 
     /** @todo add ability to dump physical contents described by any descriptor (using existing VirtIO core API function) */
-//    bool fDump      = pszArgs && (*pszArgs == 'd' || *pszArgs == 'D'); /* "dump" (avail phys descriptor)"
+//  bool fDump      = pszArgs && (*pszArgs == 'd' || *pszArgs == 'D'); /* "dump" (avail phys descriptor)"
 
     uint16_t uAvailIdx       = virtioReadAvailRingIdx(pDevIns, pVirtio, pVirtq);
@@ -648 +673 @@
         pHlp->pfnPrintf(pHlp,     "      No desc chains available\n");
     pHlp->pfnPrintf(pHlp, "\n");
-
 }
 
@@ -661 +685 @@
     return cRefs;
 }
-
 
 /** API Function: See header file */
@@ -687 +710 @@
 void virtioCoreNotifyConfigChanged(PVIRTIOCORE pVirtio)
 {
-    virtioKick(pVirtio->pDevInsR3, pVirtio, VIRTIO_ISR_DEVICE_CONFIG, pVirtio->uMsixConfig);
-}
+    virtioNudgeGuest(pVirtio->pDevInsR3, pVirtio, VIRTIO_ISR_DEVICE_CONFIG, pVirtio->uMsixConfig);
+}
+
 
 /** API Function: See header file */
 void virtioCoreVirtqEnableNotify(PVIRTIOCORE pVirtio, uint16_t uVirtq, bool fEnable)
 {
-
     Assert(uVirtq < RT_ELEMENTS(pVirtio->aVirtqueues));
     PVIRTQUEUE pVirtq = &pVirtio->aVirtqueues[uVirtq];
@@ -719 +742 @@
         if (!pVirtio->fLegacyDriver)
             pVirtio->fGenUpdatePending = true;
-        virtioKick(pVirtio->pDevInsR3, pVirtio, VIRTIO_ISR_DEVICE_CONFIG, pVirtio->uMsixConfig);
+        virtioNudgeGuest(pVirtio->pDevInsR3, pVirtio, VIRTIO_ISR_DEVICE_CONFIG, pVirtio->uMsixConfig);
     }
 }
@@ -748 +771 @@
     return VINF_SUCCESS;
 }
-
 
 /** API Function: See header file */
@@ -797 +819 @@
     {
         PVIRTIOSGSEG pSeg;
-
         /*
          * Malicious guests may go beyond paSegsIn or paSegsOut boundaries by linking
@@ -804 +825 @@
          * the following aborts I/O if breach and employs a simple log throttling algorithm to notify.
          */
-        if (cSegsIn + cSegsOut >= VIRTQ_SIZE)
+        if (cSegsIn + cSegsOut >= pVirtq->uQueueSize)
         {
             static volatile uint32_t s_cMessages  = 0;
@@ -823 +844 @@
         if (desc.fFlags & VIRTQ_DESC_F_WRITE)
         {
-            Log6Func(("%s IN  idx=%u seg=%u addr=%RGp cb=%u\n", pVirtq->szName, uDescIdx, cSegsIn, desc.GCPhysBuf, desc.cb));
+            Log6Func(("%s IN  idx=%-4u seg=%-3u addr=%RGp cb=%u\n", pVirtq->szName, uDescIdx, cSegsIn, desc.GCPhysBuf, desc.cb));
             cbIn += desc.cb;
             pSeg = &paSegsIn[cSegsIn++];
@@ -829 +850 @@
         else
         {
-            Log6Func(("%s OUT desc_idx=%u seg=%u addr=%RGp cb=%u\n", pVirtq->szName, uDescIdx, cSegsOut, desc.GCPhysBuf, desc.cb));
+            Log6Func(("%s OUT desc_idx=%-4u seg=%-3u addr=%RGp cb=%u\n", pVirtq->szName, uDescIdx, cSegsOut, desc.GCPhysBuf, desc.cb));
             cbOut += desc.cb;
             pSeg = &paSegsOut[cSegsOut++];
@@ -840 +861 @@
 #endif
         }
-
         pSeg->GCPhys = desc.GCPhysBuf;
         pSeg->cbSeg = desc.cb;
-
         uDescIdx = desc.uDescIdxNext;
     } while (desc.fFlags & VIRTQ_DESC_F_NEXT);
@@ -915 +934 @@
     AssertMsgReturn(IS_DRIVER_OK(pVirtio), ("Guest driver not in ready state.\n"), VERR_INVALID_STATE);
 
-    Log6Func(("    Copying device data to %s (%s guest), desc chain idx %d\n",
-              VIRTQNAME(pVirtio, uVirtq), pVirtio->fLegacyDriver ? "legacy" : "modern", virtioReadUsedRingIdx(pDevIns, pVirtio, pVirtq)));
-
-    /* Copy s/g buf (virtual memory) to guest phys mem (IN direction). */
+    Log6Func(("    Copying device data to %s, [desc:%u → used ring:%u]\n",
+              VIRTQNAME(pVirtio, uVirtq), pVirtqBuf->uHeadIdx, pVirtq->uUsedIdxShadow));
+
+    /* Copy s/g buf (virtual memory) to guest phys mem (VirtIO "IN" direction). */
 
     size_t cbCopy = 0, cbTotal = 0, cbRemain = 0;
@@ -944 +963 @@
     }
 
-    /* If this write-ahead crosses threshold where the driver wants to get an event, flag it */
+    /* Flag if write-ahead crosses threshold where guest driver indicated it wants event notification */
     if (pVirtio->uDriverFeatures & VIRTIO_F_EVENT_IDX)
         if (pVirtq->uUsedIdxShadow == virtioReadAvailUsedEvent(pDevIns, pVirtio, pVirtq))
@@ -951 +970 @@
     /*
      * Place used buffer's descriptor in used ring but don't update used ring's slot index.
-     * That will be done with a subsequent client call to virtioCoreVirtqUsedRingSync() */
+     * That will be done with a subsequent client call to virtioCoreVirtqUsedRingSync()
+     */
     virtioWriteUsedElem(pDevIns, pVirtio, pVirtq, pVirtq->uUsedIdxShadow++, pVirtqBuf->uHeadIdx, (uint32_t)cbTotal);
 
-    if (pSgVirtReturn)
-        Log6Func(("     ... %d segs, %zu bytes, copied to %u byte buf. residual: %zu bytes\n",
-                  pSgVirtReturn->cSegs, cbTotal - cbRemain,  pVirtqBuf->cbPhysReturn, pVirtqBuf->cbPhysReturn - cbTotal));
-
-    Log6Func(("    %s used_idx=%u\n", VIRTQNAME(pVirtio, uVirtq), virtioReadUsedRingIdx(pDevIns, pVirtio, pVirtq)));
+#ifdef LOG_ENABLED
+    if (LogIs6Enabled() && pSgVirtReturn)
+    {
+
+        LogFunc(("     ... %d segs, %zu bytes, copied to %u byte buf@offset=%u. Residual: %zu bytes\n",
+             pSgVirtReturn->cSegs,  cbTotal - cbRemain,  pVirtqBuf->cbPhysReturn,
+              ((virtioCoreGCPhysChainCalcBufSize(pVirtqBuf->pSgPhysReturn) -
+                virtioCoreGCPhysChainCalcLengthLeft(pVirtqBuf->pSgPhysReturn)) - (cbTotal - cbRemain)),
+                virtioCoreGCPhysChainCalcLengthLeft(pVirtqBuf->pSgPhysReturn) ));
+
+        uint16_t uPending = virtioCoreR3CountPendingBufs(
+                                virtioReadUsedRingIdx(pDevIns, pVirtio, pVirtq),
+                                pVirtq->uUsedIdxShadow, pVirtq->uQueueSize);
+
+        LogFunc(("    %u used buf%s not synced in %s\n", uPending, uPending == 1 ? "" : "s ",
+                    VIRTQNAME(pVirtio, uVirtq)));
+    }
+#endif
+    return VINF_SUCCESS;
+}
+
+/** API function: See Header file  */
+int virtioCoreR3VirtqUsedBufPut(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t uVirtq,
+                                size_t cb, void const *pv, PVIRTQBUF pVirtqBuf, uint32_t cbEnqueue, bool fFence)
+{
+    Assert(uVirtq < RT_ELEMENTS(pVirtio->aVirtqueues));
+    Assert(pv);
+
+    PVIRTQUEUE pVirtq = &pVirtio->aVirtqueues[uVirtq];
+    PVIRTIOSGBUF pSgPhysReturn = pVirtqBuf->pSgPhysReturn;
+
+    Assert(pVirtqBuf->u32Magic == VIRTQBUF_MAGIC);
+    Assert(pVirtqBuf->cRefs > 0);
+
+    AssertMsgReturn(IS_DRIVER_OK(pVirtio), ("Guest driver not in ready state.\n"), VERR_INVALID_STATE);
+
+    Log6Func(("    Copying device data to %s, [desc chain head idx:%u]\n",
+              VIRTQNAME(pVirtio, uVirtq), pVirtqBuf->uHeadIdx));
+
+    /*
+     * Convert virtual memory simple buffer to guest physical memory (VirtIO descriptor chain)
+     */
+    uint8_t *pvBuf = (uint8_t *)pv;
+    size_t cbRemain = cb, cbCopy = 0;
+    while (cbRemain)
+    {
+        cbCopy = RT_MIN(pSgPhysReturn->cbSegLeft, cbRemain);
+        Assert(cbCopy > 0);
+        virtioCoreGCPhysWrite(pVirtio, pDevIns, (RTGCPHYS)pSgPhysReturn->GCPhysCur, pvBuf, cbCopy);
+        virtioCoreGCPhysChainAdvance(pSgPhysReturn, cbCopy);
+        pvBuf += cbCopy;
+        cbRemain -= cbCopy;
+    }
+    LogFunc(("     ...%zu bytes, copied to %u byte buf@offset=%u. Residual: %zu bytes\n",
+              cb ,  pVirtqBuf->cbPhysReturn,
+              ((virtioCoreGCPhysChainCalcBufSize(pVirtqBuf->pSgPhysReturn) -
+                 virtioCoreGCPhysChainCalcLengthLeft(pVirtqBuf->pSgPhysReturn)) - cb),
+                 virtioCoreGCPhysChainCalcLengthLeft(pVirtqBuf->pSgPhysReturn)));
+
+    if (cbEnqueue)
+    {
+        if (fFence)
+        {
+            RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); /* needed? */
+            Assert(!(cbCopy >> 32));
+        }
+        /* Flag if write-ahead crosses threshold where guest driver indicated it wants event notification */
+        if (pVirtio->uDriverFeatures & VIRTIO_F_EVENT_IDX)
+            if (pVirtq->uUsedIdxShadow == virtioReadAvailUsedEvent(pDevIns, pVirtio, pVirtq))
+                pVirtq->fUsedRingEvent = true;
+        /*
+         * Place used buffer's descriptor in used ring but don't update used ring's slot index.
+         * That will be done with a subsequent client call to virtioCoreVirtqUsedRingSync()
+         */
+        Log6Func(("    Enqueue desc chain head idx %u to %s used ring @ %u\n", pVirtqBuf->uHeadIdx,
+                VIRTQNAME(pVirtio, uVirtq), pVirtq->uUsedIdxShadow));
+
+        virtioWriteUsedElem(pDevIns, pVirtio, pVirtq, pVirtq->uUsedIdxShadow++, pVirtqBuf->uHeadIdx, cbEnqueue);
+
+#ifdef LOG_ENABLED
+        if (LogIs6Enabled())
+        {
+            uint16_t uPending = virtioCoreR3CountPendingBufs(
+                                    virtioReadUsedRingIdx(pDevIns, pVirtio, pVirtq),
+                                    pVirtq->uUsedIdxShadow, pVirtq->uQueueSize);
+
+            LogFunc(("    %u used buf%s not synced in %s\n",
+                    uPending, uPending == 1 ? "" : "s ", VIRTQNAME(pVirtio, uVirtq)));
+        }
+#endif
+    } /* fEnqueue */
 
     return VINF_SUCCESS;
@@ -976 +1082 @@
             ("Guest driver not in ready state.\n"), VERR_INVALID_STATE);
 
-    Log6Func(("    %s ++used_idx=%u\n", pVirtq->szName, pVirtq->uUsedIdxShadow));
+    Log6Func(("    Sync %s used ring (%u → idx)\n",
+                        pVirtq->szName, pVirtq->uUsedIdxShadow));
 
     virtioWriteUsedRingIdx(pDevIns, pVirtio, pVirtq, pVirtq->uUsedIdxShadow);
@@ -998 +1105 @@
     PVIRTIOCORECC pVirtioCC = PDMINS_2_DATA_CC(pDevIns, PVIRTIOCORECC);
 
-    /* See VirtIO 1.0, section 4.1.5.2 It implies that uVirtq and uNotifyIdx should match.
-     * Disregarding this notification may cause throughput to stop, however there's no way to know
-     * which was queue was intended for wake-up if the two parameters disagree. */
-
+    /* VirtIO 1.0, section 4.1.5.2 implies uVirtq and uNotifyIdx should match. Disregarding any of
+     * these notifications (if those indicies disagree) may break device/driver synchronization,
+     * causing eternal throughput starvation, yet there's no specified way to disambiguate
+     * which queue to wake-up in any awkward situation where the two parameters differ.
+     */
     AssertMsg(uNotifyIdx == uVirtq,
               ("Guest kicked virtq %d's notify addr w/non-corresponding virtq idx %d\n",
@@ -1010 +1118 @@
     PVIRTQUEUE pVirtq = &pVirtio->aVirtqueues[uVirtq];
 
-    Log6Func(("%s (desc chains: %u)\n", pVirtq->szName,
+    Log6Func(("%s: (desc chains: %u)\n", pVirtq->szName ? pVirtq->szName : "?UNAMED QUEUE?",
         virtioCoreVirtqAvailCnt(pDevIns, pVirtio, pVirtq)));
 
@@ -1048 +1156 @@
                    pVirtq->szName, (uint16_t)virtioReadAvailUsedEvent(pDevIns, pVirtio, pVirtq)));
 #endif
-            virtioKick(pDevIns, pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, pVirtq->uMsixVector);
+            virtioNudgeGuest(pDevIns, pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, pVirtq->uMsixVector);
             pVirtq->fUsedRingEvent = false;
             return;
@@ -1062 +1170 @@
         if (!(virtioReadAvailRingFlags(pDevIns, pVirtio, pVirtq) & VIRTQ_AVAIL_F_NO_INTERRUPT))
         {
-            virtioKick(pDevIns, pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, pVirtq->uMsixVector);
+            virtioNudgeGuest(pDevIns, pVirtio, VIRTIO_ISR_VIRTQ_INTERRUPT, pVirtq->uMsixVector);
             return;
         }
@@ -1077 +1185 @@
  * @param   uVec        MSI-X vector, if enabled
  */
-static int virtioKick(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint8_t uCause, uint16_t uMsixVector)
+static int virtioNudgeGuest(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint8_t uCause, uint16_t uMsixVector)
 {
     if (uCause == VIRTIO_ISR_VIRTQ_INTERRUPT)
-        Log6Func(("reason: buffer added to 'used' ring.\n"));
+        Log6Func(("Reason for interrupt - buffer added to 'used' ring.\n"));
     else
     if (uCause == VIRTIO_ISR_DEVICE_CONFIG)
-       Log6Func(("reason: device config change\n"));
+       Log6Func(("Reason for interrupt - device config change\n"));
 
     if (!pVirtio->fMsiSupport)
@@ -1132 +1240 @@
 {
     LogFunc(("Resetting device VirtIO state\n"));
-    pVirtio->fLegacyDriver          = 1;   /* Assume this. Cleared if VIRTIO_F_VERSION_1 feature ack'd */
+    pVirtio->fLegacyDriver          = pVirtio->fOfferLegacy;   /* Cleared if VIRTIO_F_VERSION_1 feature ack'd */
     pVirtio->uDeviceFeaturesSelect  = 0;
     pVirtio->uDriverFeaturesSelect  = 0;
@@ -1168 +1276 @@
 }
 #endif /* IN_RING3 */
+
+/*
+ * Determines whether guest virtio driver is modern or legacy and does callback
+ * informing device-specific code that feature negotiation is complete.
+ * Should be called only once (coordinated via the 'toggle' flag)
+ */
+#ifdef IN_RING3
+DECLINLINE(void) virtioR3DoFeaturesCompleteOnceOnly(PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC)
+{
+        if (pVirtio->uDriverFeatures & VIRTIO_F_VERSION_1)
+        {
+            LogFunc(("VIRTIO_F_VERSION_1 feature ack'd by guest\n"));
+            pVirtio->fLegacyDriver = 0;
+        }
+        else
+        {
+            if (pVirtio->fOfferLegacy)
+            {
+                pVirtio->fLegacyDriver = 1;
+                LogFunc(("VIRTIO_F_VERSION_1 feature was NOT set by guest\n"));
+            }
+            else
+                AssertMsgFailed(("Guest didn't accept VIRTIO_F_VERSION_1, but fLegacyOffered flag not set.\n"));
+        }
+        pVirtioCC->pfnFeatureNegotiationComplete(pVirtio, pVirtio->uDriverFeatures, pVirtio->fLegacyDriver);
+        pVirtio->fDriverFeaturesWritten |= DRIVER_FEATURES_COMPLETE_HANDLED;
+}
+#endif
 
 /**
@@ -1228 +1364 @@
                 case 0:
                     memcpy(&pVirtio->uDriverFeatures, pv, cb);
+                    pVirtio->fDriverFeaturesWritten |= DRIVER_FEATURES_0_WRITTEN;
+            LogFunc(("Set DRIVER_FEATURES_0_WRITTEN. pVirtio->fDriverFeaturesWritten=%d\n", pVirtio->fDriverFeaturesWritten));
+                    if (     (pVirtio->fDriverFeaturesWritten & DRIVER_FEATURES_0_AND_1_WRITTEN) == DRIVER_FEATURES_0_AND_1_WRITTEN
+                        && !(pVirtio->fDriverFeaturesWritten & DRIVER_FEATURES_COMPLETE_HANDLED))
+#ifdef IN_RING0
+                        return VINF_IOM_R3_MMIO_WRITE;
+#endif
+#ifdef IN_RING3
+                        virtioR3DoFeaturesCompleteOnceOnly(pVirtio, pVirtioCC);
+#endif
                     VIRTIO_DEV_CONFIG_LOG_ACCESS(uDriverFeatures, VIRTIO_PCI_COMMON_CFG_T, uOffsetOfAccess);
                     break;
                 case 1:
                     memcpy((char *)&pVirtio->uDriverFeatures + sizeof(uint32_t), pv, cb);
-                    if (pVirtio->uDriverFeatures & VIRTIO_F_VERSION_1)
-                    {
+                    pVirtio->fDriverFeaturesWritten |= DRIVER_FEATURES_1_WRITTEN;
+            LogFunc(("Set DRIVER_FEATURES_1_WRITTEN. pVirtio->fDriverFeaturesWritten=%d\n", pVirtio->fDriverFeaturesWritten));
+                    if (     (pVirtio->fDriverFeaturesWritten & DRIVER_FEATURES_0_AND_1_WRITTEN) == DRIVER_FEATURES_0_AND_1_WRITTEN
+                        && !(pVirtio->fDriverFeaturesWritten & DRIVER_FEATURES_COMPLETE_HANDLED))
 #ifdef IN_RING0
                         return VINF_IOM_R3_MMIO_WRITE;
 #endif
 #ifdef IN_RING3
-                        pVirtio->fLegacyDriver = 0;
-                        pVirtioCC->pfnGuestVersionHandler(pVirtio, 1 /* fModern */);
-#endif
-                    }
+                        virtioR3DoFeaturesCompleteOnceOnly(pVirtio, pVirtioCC);
+#endif
                     VIRTIO_DEV_CONFIG_LOG_ACCESS(uDriverFeatures, VIRTIO_PCI_COMMON_CFG_T, uOffsetOfAccess + sizeof(uint32_t));
                     break;
@@ -1389 +1535 @@
  * This I/O handler exists only to handle access from legacy drivers.
  */
-
 static DECLCALLBACK(VBOXSTRICTRC) virtioLegacyIOPortIn(PPDMDEVINS pDevIns, void *pvUser, RTIOPORT offPort, uint32_t *pu32, unsigned cb)
 {
-
     PVIRTIOCORE   pVirtio = PDMINS_2_DATA(pDevIns, PVIRTIOCORE);
     STAM_PROFILE_ADV_START(&pVirtio->CTX_SUFF(StatRead), a);
 
     RT_NOREF(pvUser);
+    Log(("%-23s: Port read at offset=%RTiop, cb=%#x%s",
+        __FUNCTION__, offPort, cb,
+        VIRTIO_DEV_CONFIG_MATCH_MEMBER(fIsrStatus, VIRTIO_LEGACY_PCI_COMMON_CFG_T, offPort) ? "" : "\n"));
 
     void *pv = pu32; /* To use existing macros */
@@ -1412 +1559 @@
     if (VIRTIO_DEV_CONFIG_MATCH_MEMBER(uDriverFeatures, VIRTIO_LEGACY_PCI_COMMON_CFG_T, offPort))
     {
-        uint32_t val = pVirtio->uDriverFeatures & 0xffffffff;
+        uint32_t val = pVirtio->uDriverFeatures &  UINT32_C(0xffffffff);
         memcpy(pu32, &val, cb);
         VIRTIO_DEV_CONFIG_LOG_ACCESS(uDriverFeatures, VIRTIO_LEGACY_PCI_COMMON_CFG_T, offPort);
@@ -1420 +1567 @@
     {
         *(uint8_t *)pu32 = pVirtio->fDeviceStatus;
-
         if (LogIs7Enabled())
         {
@@ -1435 +1581 @@
         pVirtio->uISR = 0;
         virtioLowerInterrupt( pDevIns,  0);
-        Log((" ISR read and cleared\n"));
+        Log((" (ISR read and cleared)\n"));
     }
     else
@@ -1482 +1628 @@
         return rc;
     }
-
     STAM_PROFILE_ADV_STOP(&pVirtio->CTX_SUFF(StatRead), a);
     return VINF_SUCCESS;
 }
-
 
 /**
@@ -1504 +1648 @@
     int fWrite = 1;         /* To use existing macros */
 
-//    LogFunc(("Write to port offset=%RTiop, cb=%#x, u32=%#x\n",  offPort, cb, u32));
+    Log(("%-23s: Port written at offset=%RTiop, cb=%#x, u32=%#x\n",  __FUNCTION__, offPort, cb, u32));
 
     if (VIRTIO_DEV_CONFIG_MATCH_MEMBER(   uVirtqSelect,        VIRTIO_LEGACY_PCI_COMMON_CFG_T, offPort))
@@ -1535 +1679 @@
                 pVirtio->uDriverFeatures &= VIRTIO_DEV_INDEPENDENT_LEGACY_FEATURES_OFFERED;
         }
+        if (!((pVirtio->fDriverFeaturesWritten ^= 1) & 1))
+        {
+#ifdef IN_RING0
+            Log6(("%-23s: RING0 => RING3 (demote)\n", __FUNCTION__));
+            return VINF_IOM_R3_MMIO_WRITE;
+#endif
+#ifdef IN_RING3
+            PVIRTIOCORECC pVirtioCC = PDMINS_2_DATA_CC(pDevIns, PVIRTIOCORECC);
+            virtioR3DoFeaturesCompleteOnceOnly(pVirtio, pVirtioCC);
+#endif
+        }
         VIRTIO_DEV_CONFIG_LOG_ACCESS(uDriverFeatures,          VIRTIO_LEGACY_PCI_COMMON_CFG_T, offPort);
     }
@@ -1556 +1711 @@
             Log(("%-23s: Guest wrote fDeviceStatus ................ (%s)\n", __FUNCTION__, szOut));
         }
-
         if (fDriverStateImproved  || fDriverInitiatedReset)
         {
@@ -1693 +1847 @@
 
         /*
-         * Additionally, anytime any part of the device-specific configuration (which our client maintains)
-         * is READ it needs to be checked to see if it changed since the last time any part was read, in
-         * order to maintain the config generation (see VirtIO 1.0 spec, section 4.1.4.3.1)
+         * Anytime any part of the dev-specific dev config (which this virtio core implementation sees
+         * as a blob, and virtio dev-specific code separates into fields) is READ, it must be compared
+         * for deltas from previous read to maintain a config gen. seq. counter (VirtIO 1.0, section 4.1.4.3.1)
          */
         bool fDevSpecificFieldChanged = RT_BOOL(memcmp(pVirtioCC->pbDevSpecificCfg + uOffset,
@@ -1706 +1860 @@
         {
             ++pVirtio->uConfigGeneration;
-            Log6Func(("Bumped cfg. generation to %d because %s%s\n",
-                      pVirtio->uConfigGeneration,
+            Log6Func(("Bumped cfg. generation to %d because %s%s\n", pVirtio->uConfigGeneration,
                       fDevSpecificFieldChanged ? "<dev cfg changed> " : "",
                       pVirtio->fGenUpdatePending ? "<update was pending>" : ""));
@@ -1769 +1922 @@
 #else
         STAM_PROFILE_ADV_STOP(&pVirtio->CTX_SUFF(StatWrite), a);
+        Log6(("%-23s: RING0 => RING3 (demote)\n", __FUNCTION__));
         return VINF_IOM_R3_MMIO_WRITE;
 #endif
@@ -1819 +1973 @@
     if (uAddress == pVirtio->uPciCfgDataOff)
     {
-        /*
-         * VirtIO 1.0 spec section 4.1.4.7 describes a required alternative access capability
-         * whereby the guest driver can specify a bar, offset, and length via the PCI configuration space
-         * (the virtio_pci_cfg_cap capability), and access data items.
-         * This is used by BIOS to gain early boot access to the the storage device.
-         */
+     /* See comments in PCI Cfg capability initialization (in capabilities setup section of this code) */
         struct virtio_pci_cap *pPciCap = &pVirtioCC->pPciCfgCap->pciCap;
         uint32_t uLength = pPciCap->uLength;
@@ -1863 +2012 @@
     if (uAddress == pVirtio->uPciCfgDataOff)
     {
-        /* VirtIO 1.0 spec section 4.1.4.7 describes a required alternative access capability
-         * whereby the guest driver can specify a bar, offset, and length via the PCI configuration space
-         * (the virtio_pci_cfg_cap capability), and access data items.
-         * This is used by BIOS to gain early boot access to the the storage device.*/
-
+        /* See comments in PCI Cfg capability initialization (in capabilities setup section of this code) */
         struct virtio_pci_cap *pPciCap = &pVirtioCC->pPciCfgCap->pciCap;
         uint32_t uLength = pPciCap->uLength;
@@ -1889 +2034 @@
 
 /*********************************************************************************************************************************
-*   Saved state.                                                                                                                 *
+*   Saved state (SSM)                                                                                                            *
 *********************************************************************************************************************************/
+
+
+/**
+ * Loads a saved device state (called from device-specific code on SSM final pass)
+ *
+ * @param   pVirtio     Pointer to the shared virtio state.
+ * @param   pHlp        The ring-3 device helpers.
+ * @param   pSSM        The saved state handle.
+ * @returns VBox status code.
+ */
+int virtioCoreR3LegacyDeviceLoadExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp,
+                               PSSMHANDLE pSSM, uint32_t uVersion, uint32_t uVirtioLegacy_3_1_Beta)
+{
+    int rc;
+    uint32_t uDriverFeaturesLegacy32bit;
+
+    rc = pHlp->pfnSSMGetU32(  pSSM, &uDriverFeaturesLegacy32bit);
+    AssertRCReturn(rc, rc);
+    pVirtio->uDriverFeatures = (uint64_t)uDriverFeaturesLegacy32bit;
+
+    rc = pHlp->pfnSSMGetU16(  pSSM, &pVirtio->uVirtqSelect);
+    AssertRCReturn(rc, rc);
+
+    rc = pHlp->pfnSSMGetU8(   pSSM, &pVirtio->fDeviceStatus);
+    AssertRCReturn(rc, rc);
+
+    char szOut[80] = { 0 };
+    virtioCoreFormatDeviceStatus(pVirtio->fDeviceStatus, szOut, sizeof(szOut));
+    Log(("Loaded legacy device status = (%s)\n", szOut));
+
+    rc = pHlp->pfnSSMGetU8(   pSSM, &pVirtio->uISR);
+    AssertRCReturn(rc, rc);
+
+    uint32_t cQueues = 3; /* Thes constant default value copied from earliest v0.9 code */
+    if (uVersion > uVirtioLegacy_3_1_Beta)
+    {
+        rc = pHlp->pfnSSMGetU32(pSSM, &cQueues);
+        AssertRCReturn(rc, rc);
+    }
+
+    AssertLogRelMsgReturn(cQueues <= VIRTQ_MAX_COUNT, ("%#x\n", cQueues), VERR_SSM_LOAD_CONFIG_MISMATCH);
+    AssertLogRelMsgReturn(pVirtio->uVirtqSelect < cQueues || (cQueues == 0 && pVirtio->uVirtqSelect),
+                          ("uVirtqSelect=%u cQueues=%u\n", pVirtio->uVirtqSelect, cQueues),
+                          VERR_SSM_LOAD_CONFIG_MISMATCH);
+
+    Log(("\nRestoring %d  legacy-only virtio-net device queues from saved state:\n", cQueues));
+    for (unsigned uVirtq = 0; uVirtq < cQueues; uVirtq++)
+    {
+        PVIRTQUEUE pVirtq = &pVirtio->aVirtqueues[uVirtq];
+
+        if (uVirtq == cQueues - 1)
+            RTStrPrintf(pVirtq->szName, sizeof(pVirtq->szName), "legacy-ctrlq");
+        else if (uVirtq % 2)
+            RTStrPrintf(pVirtq->szName, sizeof(pVirtq->szName), "legacy-xmitq<%d>", uVirtq / 2);
+        else
+            RTStrPrintf(pVirtq->szName, sizeof(pVirtq->szName), "legacy-recvq<%d>", uVirtq / 2);
+
+        rc = pHlp->pfnSSMGetU16(pSSM, &pVirtq->uQueueSize);
+        AssertRCReturn(rc, rc);
+
+        uint32_t uVirtqPfn;
+        rc = pHlp->pfnSSMGetU32(pSSM, &uVirtqPfn);
+        AssertRCReturn(rc, rc);
+
+        rc = pHlp->pfnSSMGetU16(pSSM, &pVirtq->uAvailIdxShadow);
+        AssertRCReturn(rc, rc);
+
+        rc = pHlp->pfnSSMGetU16(pSSM, &pVirtq->uUsedIdxShadow);
+        AssertRCReturn(rc, rc);
+
+        if (uVirtqPfn)
+        {
+            pVirtq->GCPhysVirtqDesc  = (uint64_t)uVirtqPfn * VIRTIO_PAGE_SIZE;
+            pVirtq->GCPhysVirtqAvail = pVirtq->GCPhysVirtqDesc + sizeof(VIRTQ_DESC_T) * pVirtq->uQueueSize;
+            pVirtq->GCPhysVirtqUsed  =
+                RT_ALIGN(pVirtq->GCPhysVirtqAvail + RT_UOFFSETOF_DYN(VIRTQ_AVAIL_T, auRing[pVirtq->uQueueSize]), VIRTIO_PAGE_SIZE);
+            pVirtq->uEnable = 1;
+        }
+        else
+        {
+            LogFunc(("WARNING: QUEUE \"%s\" PAGE NUMBER ZERO IN SAVED STATE\n", pVirtq->szName));
+            pVirtq->uEnable = 0;
+        }
+        pVirtq->uNotifyOffset = 0;  /* unused in legacy mode */
+        pVirtq->uMsixVector   = 0;  /* unused in legacy mode */
+    }
+    pVirtio->fGenUpdatePending = 0; /* unused in legacy mode */
+    pVirtio->uConfigGeneration = 0; /* unused in legacy mode */
+    pVirtio->uPciCfgDataOff    = 0; /* unused in legacy mode (port I/O used instead)   */
+
+    return VINF_SUCCESS;
+}
+
+/**
+ * Loads a saved device state (called from device-specific code on SSM final pass)
+ *
+ * Note: This loads state saved by a Modern (VirtIO 1.0+) device, of which this transitional device is one,
+ *       and thus supports both legacy and modern guest virtio drivers.
+ *
+ * @param   pVirtio     Pointer to the shared virtio state.
+ * @param   pHlp        The ring-3 device helpers.
+ * @param   pSSM        The saved state handle.
+ * @returns VBox status code.
+ */
+int virtioCoreR3ModernDeviceLoadExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM, uint32_t uVersion, uint32_t uTestVersion, uint32_t cQueues)
+{
+    RT_NOREF2(cQueues, uVersion);
+    LogFunc(("\n"));
+    /*
+     * Check the marker and (embedded) version number.
+     */
+    uint64_t uMarker = 0;
+    int rc;
+
+    rc = pHlp->pfnSSMGetU64(pSSM, &uMarker);
+    AssertRCReturn(rc, rc);
+    if (uMarker != VIRTIO_SAVEDSTATE_MARKER)
+        return pHlp->pfnSSMSetLoadError(pSSM, VERR_SSM_DATA_UNIT_FORMAT_CHANGED, RT_SRC_POS,
+                                        N_("Expected marker value %#RX64 found %#RX64 instead"),
+                                        VIRTIO_SAVEDSTATE_MARKER, uMarker);
+    uint32_t uVersionSaved = 0;
+    rc = pHlp->pfnSSMGetU32(pSSM, &uVersionSaved);
+    AssertRCReturn(rc, rc);
+    if (uVersionSaved != uTestVersion)
+        return pHlp->pfnSSMSetLoadError(pSSM, VERR_SSM_DATA_UNIT_FORMAT_CHANGED, RT_SRC_POS,
+                                        N_("Unsupported virtio version: %u"), uVersionSaved);
+    /*
+     * Load the state.
+     */
+    rc = pHlp->pfnSSMGetU32(  pSSM, &pVirtio->fLegacyDriver);
+    AssertRCReturn(rc, rc);
+    rc = pHlp->pfnSSMGetBool( pSSM, &pVirtio->fGenUpdatePending);
+    AssertRCReturn(rc, rc);
+    rc = pHlp->pfnSSMGetU8(   pSSM, &pVirtio->fDeviceStatus);
+    AssertRCReturn(rc, rc);
+    rc = pHlp->pfnSSMGetU8(   pSSM, &pVirtio->uConfigGeneration);
+    AssertRCReturn(rc, rc);
+    rc = pHlp->pfnSSMGetU8(   pSSM, &pVirtio->uPciCfgDataOff);
+    AssertRCReturn(rc, rc);
+    rc = pHlp->pfnSSMGetU8(   pSSM, &pVirtio->uISR);
+    AssertRCReturn(rc, rc);
+    rc = pHlp->pfnSSMGetU16(  pSSM, &pVirtio->uVirtqSelect);
+    AssertRCReturn(rc, rc);
+    rc = pHlp->pfnSSMGetU32(  pSSM, &pVirtio->uDeviceFeaturesSelect);
+    AssertRCReturn(rc, rc);
+    rc = pHlp->pfnSSMGetU32(  pSSM, &pVirtio->uDriverFeaturesSelect);
+    AssertRCReturn(rc, rc);
+    rc = pHlp->pfnSSMGetU64(  pSSM, &pVirtio->uDriverFeatures);
+    AssertRCReturn(rc, rc);
+
+    /** @todo Adapt this loop use cQueues argument instead of static queue count (safely with SSM versioning) */
+    for (uint32_t i = 0; i < VIRTQ_MAX_COUNT; i++)
+    {
+        PVIRTQUEUE pVirtq = &pVirtio->aVirtqueues[i];
+        rc = pHlp->pfnSSMGetGCPhys64( pSSM, &pVirtq->GCPhysVirtqDesc);
+        AssertRCReturn(rc, rc);
+        rc = pHlp->pfnSSMGetGCPhys64( pSSM, &pVirtq->GCPhysVirtqAvail);
+        AssertRCReturn(rc, rc);
+        rc = pHlp->pfnSSMGetGCPhys64( pSSM, &pVirtq->GCPhysVirtqUsed);
+        AssertRCReturn(rc, rc);
+        rc = pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uNotifyOffset);
+        AssertRCReturn(rc, rc);
+        rc = pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uMsixVector);
+        AssertRCReturn(rc, rc);
+        rc = pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uEnable);
+        AssertRCReturn(rc, rc);
+        rc = pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uQueueSize);
+        AssertRCReturn(rc, rc);
+        rc = pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uAvailIdxShadow);
+        AssertRCReturn(rc, rc);
+        rc = pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uUsedIdxShadow);
+        AssertRCReturn(rc, rc);
+        rc = pHlp->pfnSSMGetMem( pSSM, pVirtq->szName,  sizeof(pVirtq->szName));
+        AssertRCReturn(rc, rc);
+    }
+    return VINF_SUCCESS;
+}
 
 /**
@@ -1900 +2222 @@
  * @returns VBox status code.
  */
-int virtioCoreR3SaveExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM)
-{
+int virtioCoreR3SaveExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM, uint32_t uVersion, uint32_t cQueues)
+{
+    RT_NOREF(cQueues);
+    /** @todo figure out a way to save cQueues (with SSM versioning) */
+
     LogFunc(("\n"));
     pHlp->pfnSSMPutU64(pSSM, VIRTIO_SAVEDSTATE_MARKER);
-    pHlp->pfnSSMPutU32(pSSM, VIRTIO_SAVEDSTATE_VERSION);
-
+    pHlp->pfnSSMPutU32(pSSM, uVersion);
+
+    pHlp->pfnSSMPutU32( pSSM, pVirtio->fLegacyDriver);
     pHlp->pfnSSMPutBool(pSSM, pVirtio->fGenUpdatePending);
     pHlp->pfnSSMPutU8(  pSSM, pVirtio->fDeviceStatus);
@@ -1932 +2258 @@
         AssertRCReturn(rc, rc);
     }
-
-    return VINF_SUCCESS;
-}
-
-/**
- * Called from the FNSSMDEVLOADEXEC function of the device.
- *
- * @param   pVirtio     Pointer to the shared virtio state.
- * @param   pHlp        The ring-3 device helpers.
- * @param   pSSM        The saved state handle.
- * @returns VBox status code.
- */
-int virtioCoreR3LoadExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM)
-{
-    LogFunc(("\n"));
-    /*
-     * Check the marker and (embedded) version number.
-     */
-    uint64_t uMarker = 0;
-    int rc = pHlp->pfnSSMGetU64(pSSM, &uMarker);
-    AssertRCReturn(rc, rc);
-    if (uMarker != VIRTIO_SAVEDSTATE_MARKER)
-        return pHlp->pfnSSMSetLoadError(pSSM, VERR_SSM_DATA_UNIT_FORMAT_CHANGED, RT_SRC_POS,
-                                        N_("Expected marker value %#RX64 found %#RX64 instead"),
-                                        VIRTIO_SAVEDSTATE_MARKER, uMarker);
-    uint32_t uVersion = 0;
-    rc = pHlp->pfnSSMGetU32(pSSM, &uVersion);
-    AssertRCReturn(rc, rc);
-    if (uVersion != VIRTIO_SAVEDSTATE_VERSION)
-        return pHlp->pfnSSMSetLoadError(pSSM, VERR_SSM_DATA_UNIT_FORMAT_CHANGED, RT_SRC_POS,
-                                        N_("Unsupported virtio version: %u"), uVersion);
-    /*
-     * Load the state.
-     */
-    pHlp->pfnSSMGetBool( pSSM, &pVirtio->fGenUpdatePending);
-    pHlp->pfnSSMGetU8(   pSSM, &pVirtio->fDeviceStatus);
-    pHlp->pfnSSMGetU8(   pSSM, &pVirtio->uConfigGeneration);
-    pHlp->pfnSSMGetU8(   pSSM, &pVirtio->uPciCfgDataOff);
-    pHlp->pfnSSMGetU8(   pSSM, &pVirtio->uISR);
-    pHlp->pfnSSMGetU16(  pSSM, &pVirtio->uVirtqSelect);
-    pHlp->pfnSSMGetU32(  pSSM, &pVirtio->uDeviceFeaturesSelect);
-    pHlp->pfnSSMGetU32(  pSSM, &pVirtio->uDriverFeaturesSelect);
-    pHlp->pfnSSMGetU64(  pSSM, &pVirtio->uDriverFeatures);
-
-    for (uint32_t i = 0; i < VIRTQ_MAX_COUNT; i++)
-    {
-        PVIRTQUEUE pVirtq = &pVirtio->aVirtqueues[i];
-
-        pHlp->pfnSSMGetGCPhys64( pSSM, &pVirtq->GCPhysVirtqDesc);
-        pHlp->pfnSSMGetGCPhys64( pSSM, &pVirtq->GCPhysVirtqAvail);
-        pHlp->pfnSSMGetGCPhys64( pSSM, &pVirtq->GCPhysVirtqUsed);
-        pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uNotifyOffset);
-        pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uMsixVector);
-        pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uEnable);
-        pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uQueueSize);
-        pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uAvailIdxShadow);
-        pHlp->pfnSSMGetU16(      pSSM, &pVirtq->uUsedIdxShadow);
-        rc = pHlp->pfnSSMGetMem( pSSM, pVirtq->szName,  sizeof(pVirtq->szName));
-        AssertRCReturn(rc, rc);
-    }
-
     return VINF_SUCCESS;
 }
@@ -2002 +2267 @@
 
 /**
- * This must be called by the client to handle VM state changes
- * after the client takes care of its device-specific tasks for the state change.
- * (i.e. Reset, suspend, power-off, resume)
+ * This must be called by the client to handle VM state changes after the client takes care of its device-specific
+ * tasks for the state change (i.e. reset, suspend, power-off, resume)
  *
  * @param   pDevIns     The device instance.
@@ -2057 +2321 @@
 /** API Function: See header file */
 int virtioCoreR3Init(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC, PVIRTIOPCIPARAMS pPciParams,
-                     const char *pcszInstance, uint64_t fDevSpecificFeatures, void *pvDevSpecificCfg, uint16_t cbDevSpecificCfg)
+                     const char *pcszInstance, uint64_t fDevSpecificFeatures, uint32_t fOfferLegacy,
+                     void *pvDevSpecificCfg, uint16_t cbDevSpecificCfg)
 {
     /*
-     * The pVirtio state must be the first member of the shared device instance
-     * data, otherwise we cannot get our bearings in the PCI configuration callbacks.
+     * Virtio state must be the first member of shared device instance data,
+     * otherwise can't get our bearings in PCI config callbacks.
      */
     AssertLogRelReturn(pVirtio == PDMINS_2_DATA(pDevIns, PVIRTIOCORE), VERR_STATE_CHANGED);
@@ -2073 +2338 @@
     AssertReturn(pVirtioCC->pfnStatusChanged, VERR_INVALID_POINTER);
     AssertReturn(pVirtioCC->pfnVirtqNotified, VERR_INVALID_POINTER);
-    AssertReturn(pVirtioCC->pfnGuestVersionHandler,  VERR_INVALID_POINTER);
+    AssertReturn(pVirtioCC->pfnFeatureNegotiationComplete,  VERR_INVALID_POINTER);
     AssertReturn(VIRTQ_SIZE > 0 && VIRTQ_SIZE <= 32768,  VERR_OUT_OF_RANGE); /* VirtIO specification-defined limit */
 
 #if 0 /* Until pdmR3DvHlp_PCISetIrq() impl is fixed and Assert that limits vec to 0 is removed
-       * The legacy MSI support has not been implemented yet
+       * VBox legacy MSI support has not been implemented yet
        */
 # ifdef VBOX_WITH_MSI_DEVICES
@@ -2084 +2349 @@
 #endif
 
-    /* Tell the device-specific code that guest is in legacy mode (for now) */
-    pVirtioCC->pfnGuestVersionHandler(pVirtio, false /* fModern */);
-
     /*
-     * The host features offered include both device-specific features
-     * and reserved feature bits (device independent)
+     * Host features (presented as a smörgasbord for guest to select from)
+     * include both dev-specific features & reserved dev-independent features (bitmask).
      */
     pVirtio->uDeviceFeatures = VIRTIO_F_VERSION_1
                              | VIRTIO_DEV_INDEPENDENT_FEATURES_OFFERED
                              | fDevSpecificFeatures;
+
+    pVirtio->fLegacyDriver = pVirtio->fOfferLegacy = fOfferLegacy;
 
     RTStrCopy(pVirtio->szInstance, sizeof(pVirtio->szInstance), pcszInstance);
@@ -2138 +2402 @@
     uint32_t cbRegion = 0;
 
-    /* Common capability (VirtIO 1.0 spec, section 4.1.4.3) */
+    /*
+     * Common capability (VirtIO 1.0, section 4.1.4.3)
+     */
     pCfg = (PVIRTIO_PCI_CAP_T)&pPciDev->abConfig[0x40];
     pCfg->uCfgType = VIRTIO_PCI_CAP_COMMON_CFG;
@@ -2152 +2418 @@
 
     /*
-     * 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 uNotifyOffset equal to (VirtqSelect) ordinal
-     * value of the queue (different strategies are possible according to spec).
+     * Notify capability (VirtIO 1.0, section 4.1.4.4).
+     *
+     * The size of the spec-defined subregion described by this VirtIO capability is
+     * based-on the choice of this implementation to make the notification area of each
+     * queue equal to queue's ordinal position (e.g. queue selector value). The VirtIO
+     * specification leaves it up to implementation to define queue notification area layout.
      */
     pCfg = (PVIRTIO_PCI_CAP_T)&pPciDev->abConfig[pCfg->uCapNext];
@@ -2170 +2439 @@
     pVirtioCC->pNotifyCap->uNotifyOffMultiplier = VIRTIO_NOTIFY_OFFSET_MULTIPLIER;
 
-    /* ISR capability (VirtIO 1.0 spec, section 4.1.4.5)
+    /* ISR capability (VirtIO 1.0, section 4.1.4.5)
      *
-     * VirtIO 1.0 spec says 8-bit, unaligned in MMIO space. Example/diagram
-     * of spec shows it as a 32-bit field with upper bits 'reserved'
-     * Will take spec's words more literally than the diagram for now.
+     * VirtIO 1.0 spec says 8-bit, unaligned in MMIO space. The specification example/diagram
+     * illustrates this capability as 32-bit field with upper bits 'reserved'. Those depictions
+     * differ. The spec's wording, not the diagram, is seen to work in practice.
      */
     pCfg = (PVIRTIO_PCI_CAP_T)&pPciDev->abConfig[pCfg->uCapNext];
@@ -2189 +2458 @@
     pVirtioCC->pIsrCap = pCfg;
 
-    /*  PCI Cfg capability (VirtIO 1.0 spec, section 4.1.4.7)
-     *  This capability doesn't get page-MMIO mapped. Instead uBar, uOffset and uLength are intercepted
-     *  by trapping PCI configuration I/O and get modulated by consumers to locate fetch and read/write
-     *  values from any region. NOTE: The linux driver not only doesn't use this feature, it will not
-     *  even list it as present if uLength isn't non-zero and also 4-byte-aligned as the linux driver is
-     *  initializing.
+    /*  PCI Cfg capability (VirtIO 1.0, section 4.1.4.7)
+     *
+     *  This capability facilitates early-boot access to this device (BIOS).
+     *  This region isn't page-MMIO mapped. PCI configuration accesses are intercepted,
+     *  wherein uBar, uOffset and uLength are modulated by consumers to locate and read/write
+     *  values in any part of any region. (NOTE: Linux driver doesn't utilize this feature.
+     *  This capability only appears in lspci output on Linux if uLength is non-zero, 4-byte aligned,
+     *  during initialization of linux virtio driver).
      */
     pVirtio->uPciCfgDataOff = pCfg->uCapNext + RT_OFFSETOF(VIRTIO_PCI_CFG_CAP_T, uPciCfgData);
@@ -2211 +2482 @@
     if (pVirtioCC->pbDevSpecificCfg)
     {
-        /* Device specific config capability (via VirtIO 1.0, section 4.1.4.6).
+        /* Device-specific config capability (VirtIO 1.0, section 4.1.4.6).
+         *
          * Client defines the device-specific config struct and passes size to virtioCoreR3Init()
-         * to inform this. */
+         * to inform this.
+         */
         pCfg = (PVIRTIO_PCI_CAP_T)&pPciDev->abConfig[pCfg->uCapNext];
         pCfg->uCfgType = VIRTIO_PCI_CAP_DEVICE_CFG;
@@ -2263 +2536 @@
         return PDMDEV_SET_ERROR(pDevIns, rc, N_("virtio: out of memory allocating string")); /* can we put params in this error? */
 
-    /* As a transitional device that supports legacy VirtIO drivers, this VirtIO device generic implementation presents
-     * legacy driver interface in I/O space at BAR0. The following maps the common (e.g. device independent)
-     * dev config area as well as device-specific dev config area (whose size is passed to init function of this VirtIO
-     * generic device code) for access via Port I/O, since legacy drivers (e.g. pre VirtIO 1.0) don't use MMIO callbacks.
-     * (See VirtIO 1.1, Section 4.1.4.8).
-     */
-    rc = PDMDevHlpPCIIORegionCreateIo(pDevIns, VIRTIO_REGION_LEGACY_IO, sizeof(VIRTIO_LEGACY_PCI_COMMON_CFG_T) + cbDevSpecificCfg,
-                                      virtioLegacyIOPortOut, virtioLegacyIOPortIn, NULL /*pvUser*/, pVirtioCC->pcszPortIoName,
-                                      NULL /*paExtDescs*/, &pVirtio->hLegacyIoPorts);
-    AssertLogRelRCReturn(rc, PDMDEV_SET_ERROR(pDevIns, rc, N_("virtio: cannot register legacy config in I/O space at BAR0 */")));
+    if (pVirtio->fOfferLegacy)
+    {
+        /* As a transitional device that supports legacy VirtIO drivers, this VirtIO device generic implementation presents
+         * legacy driver interface in I/O space at BAR0. The following maps the common (e.g. device independent)
+         * dev config area as well as device-specific dev config area (whose size is passed to init function of this VirtIO
+         * generic device code) for access via Port I/O, since legacy drivers (e.g. pre VirtIO 1.0) don't use MMIO callbacks.
+         * (See VirtIO 1.1, Section 4.1.4.8).
+         */
+        rc = PDMDevHlpPCIIORegionCreateIo(pDevIns, VIRTIO_REGION_LEGACY_IO, sizeof(VIRTIO_LEGACY_PCI_COMMON_CFG_T) + cbDevSpecificCfg,
+                                          virtioLegacyIOPortOut, virtioLegacyIOPortIn, NULL /*pvUser*/, pVirtioCC->pcszPortIoName,
+                                          NULL /*paExtDescs*/, &pVirtio->hLegacyIoPorts);
+        AssertLogRelRCReturn(rc, PDMDEV_SET_ERROR(pDevIns, rc, N_("virtio: cannot register legacy config in I/O space at BAR0 */")));
+    }
 
     /*  Note: The Linux driver at drivers/virtio/virtio_pci_modern.c tries to map at least a page for the
-     * 'unknown' device-specific capability without querying the capability to figure
-     *  out size, so pad with an extra page
+     * 'unknown' device-specific capability without querying the capability to determine size, so pad w/extra page.
      */
     rc = PDMDevHlpPCIIORegionCreateMmio(pDevIns, VIRTIO_REGION_PCI_CAP, RT_ALIGN_32(cbRegion + VIRTIO_PAGE_SIZE, VIRTIO_PAGE_SIZE),
@@ -2304 +2579 @@
 # endif /* VBOX_WITH_STATISTICS */
 
-    virtioResetDevice(pDevIns, pVirtio); /* Reset VirtIO specific state of device */
-
     return VINF_SUCCESS;
 }

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

@@ -36 +36 @@
 # define VIRTIO_HEX_DUMP(logLevel, pv, cb, base, title) do { } while (0)
 #endif
+
+/** Marks the start of the virtio saved state (just for sanity). */
+#define VIRTIO_SAVEDSTATE_MARKER                        UINT64_C(0x1133557799bbddff)
 
 /** Pointer to the shared VirtIO state. */
@@ -57 +60 @@
 #define VIRTIO_PAGE_SIZE                 4096                    /**< Page size used by VirtIO specification   */
 
-
-/* Note: The VirtIO specification, particularly rev. 0.95, and clarified in rev 1.0 for transitional devices,
-         says the page sized used for Queue Size calculations is usually 4096 bytes, but dependent on the
-         the transport. In an appendix of the 0.95 spec, the 'mmio device', which has not been
-         implemented by VBox legacy device in VirtualBox, says guest must report the page size. For now
-         will set page size to a static 4096 based on the original VBox legacy VirtIO implementation which
-         tied it to PAGE_SIZE which appears to work (or at least good enough for most practical purposes)      */
-
-
-/** The following virtioCoreGCPhysChain*() functions mimic the functionality of the related RT s/g functions,
- *  except they work with the data type GCPhys rather than void *
+/**
+ * @todo Move the following virtioCoreGCPhysChain*() functions mimic the functionality of the related
+ *       into some VirtualBox source tree common location and out of this code.
+ *
+ *       They behave identically to the S/G utilities in the RT library, except they work with that
+ *       GCPhys data type specifically instead of void *, to avoid potentially disastrous mismatch
+ *       between sizeof(void *) and sizeof(GCPhys).
+ *
  */
 typedef struct VIRTIOSGSEG                                      /**< An S/G entry                              */
@@ -91 +91 @@
 
 /**
- * VirtIO buffers are descriptor chains (scatter-gather vectors). Each buffer is described
- * by the index of its head descriptor, which in optionally chains to another descriptor
- * and so on.
- *
- * Each descriptor, [len, GCPhys] pair in the chain represents either an OUT segment (e.g. guest-to-host)
- * or an IN segment (host-to-guest). A VIRTQBUF is created and retured from a call to
- * virtioCoreR3VirtqAvailBufPeek() or virtioCoreR3VirtqAvailBufGet(). That function consolodates
- * the VirtIO descriptor chain into a representation, where pSgPhysSend is a GCPhys s/g buffer containing
- * all of the OUT descriptors and pSgPhysReturn is a GCPhys s/g buffer containing all of IN descriptors
- * to be filled with data on the host to return to theguest.
+ * VirtIO buffers are descriptor chains (e.g. scatter-gather vectors). A VirtIO buffer is referred to by the index
+ * of its head descriptor. Each descriptor optionally chains to another descriptor, and so on.
+ *
+ * For any given descriptor, each length and GCPhys pair in the chain represents either an OUT segment (e.g. guest-to-host)
+ * or an IN segment (host-to-guest).
+ *
+ * A VIRTQBUF is created and retured from a call to to either virtioCoreR3VirtqAvailBufPeek() or virtioCoreR3VirtqAvailBufGet().
+ *
+ * Those functions consolidate the VirtIO descriptor chain into a single representation where:
+ *
+ *     pSgPhysSend    GCPhys s/g buffer containing all of the (VirtIO) OUT descriptors
+ *     pSgPhysReturn  GCPhys s/g buffer containing all of the (VirtIO)  IN descriptors
+ *
+ * The OUT descriptors are data sent from guest to host (dev-specific commands and/or data)
+ * The IN are to be filled with data (converted to physical) on host, to be returned to guest
+ *
  */
 typedef struct VIRTQBUF
@@ -166 +172 @@
 static const VIRTIO_FEATURES_LIST s_aCoreFeatures[] =
 {
+    { VIRTIO_F_VERSION_1,               "   VERSION_1            Guest driver supports VirtIO specification V1.0+ (e.g. \"modern\")\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_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 VIRTIO_DEV_INDEPENDENT_FEATURES_OFFERED ( 0 )            /**< TBD: Add VIRTIO_F_INDIRECT_DESC           */
@@ -202 +207 @@
     kvirtIoVmStateChangedFor32BitHack = 0x7fffffff
 } VIRTIOVMSTATECHANGED;
-
-
 
 /** @def Virtio Device PCI Capabilities type codes */
@@ -305 +308 @@
 typedef struct VIRTQUEUE
 {
-    RTGCPHYS                    GCPhysVirtqDesc;                  /**< (MMIO) PhysAdr per-Q desc structs   GUEST */
-    RTGCPHYS                    GCPhysVirtqAvail;                 /**< (MMIO) PhysAdr per-Q avail structs  GUEST */
-    RTGCPHYS                    GCPhysVirtqUsed;                  /**< (MMIO) PhysAdr per-Q used structs   GUEST */
-    uint16_t                    uMsixVector;                      /**< (MMIO) Per-queue vector for MSI-X   GUEST */
-    uint16_t                    uEnable;                          /**< (MMIO) Per-queue enable             GUEST */
-    uint16_t                    uNotifyOffset;                    /**< (MMIO) per-Q notify offset           HOST */
-    uint16_t                    uQueueSize;                       /**< (MMIO) Per-queue size          HOST/GUEST */
+    RTGCPHYS                    GCPhysVirtqDesc;                  /**< (MMIO) Addr of virtq's desc  ring   GUEST */
+    RTGCPHYS                    GCPhysVirtqAvail;                 /**< (MMIO) Addr of virtq's avail ring   GUEST */
+    RTGCPHYS                    GCPhysVirtqUsed;                  /**< (MMIO) Addr of virtq's used  ring   GUEST */
+    uint16_t                    uMsixVector;                      /**< (MMIO) MSI-X vector                 GUEST */
+    uint16_t                    uEnable;                          /**< (MMIO) Queue enable flag            GUEST */
+    uint16_t                    uNotifyOffset;                    /**< (MMIO) Notification offset for queue HOST */
+    uint16_t                    uQueueSize;                       /**< (MMIO) Size of queue           HOST/GUEST */
     uint16_t                    uAvailIdxShadow;                  /**< Consumer's position in avail ring         */
     uint16_t                    uUsedIdxShadow;                   /**< Consumer's position in used ring          */
@@ -317 +320 @@
     char                        szName[32];                       /**< Dev-specific name of queue                */
     bool                        fUsedRingEvent;                   /**< Flags if used idx to notify guest reached */
-    uint8_t                     padding[3];
+    bool                        fAttached;                        /**< Flags if dev-specific client attached     */
 } VIRTQUEUE, *PVIRTQUEUE;
 
@@ -331 +334 @@
     uint64_t                    uDeviceFeatures;                  /**< (MMIO) Host features offered         HOST */
     uint64_t                    uDriverFeatures;                  /**< (MMIO) Host features accepted       GUEST */
+    uint32_t                    fDriverFeaturesWritten;           /**< (MMIO) Host features complete tracking    */
     uint32_t                    uDeviceFeaturesSelect;            /**< (MMIO) hi/lo select uDeviceFeatures GUEST */
     uint32_t                    uDriverFeaturesSelect;            /**< (MMIO) hi/lo select uDriverFeatures GUEST */
@@ -343 +347 @@
     uint8_t                     fMsiSupport;                      /**< Flag set if using MSI instead of ISR      */
     uint16_t                    uVirtqSelect;                     /**< (MMIO) queue selector               GUEST */
-    uint32_t                    fLegacyDriver;                    /**< Set if guest driver < VirtIO 1.0          */
+    uint32_t                    fLegacyDriver;                    /**< Set if guest drv < VirtIO 1.0 and allowed */
+    uint32_t                    fOfferLegacy;                     /**< Set at init call from dev-specific code   */
 
     /** @name The locations of the capability structures in PCI config space and the BAR.
@@ -354 +359 @@
     /** @} */
 
-
-
     IOMMMIOHANDLE               hMmioPciCap;                      /**< MMIO handle of PCI cap. region (\#2)      */
     IOMIOPORTHANDLE             hLegacyIoPorts;                   /**< Handle of legacy I/O port range.          */
-
 
 #ifdef VBOX_WITH_STATISTICS
@@ -374 +376 @@
     STAMPROFILEADV              StatWriteRC;                       /** I/O port and MMIO R3 Write profiling      */
 #endif
-
-
     /** @} */
+
 } VIRTIOCORE;
 
@@ -389 +390 @@
      * @{  */
     /**
-     * Implementation-specific client callback to report VirtIO version as modern or legacy.
-     * That's the only meaningful distinction in the VirtIO specification. Beyond that
-     * versioning is loosely discernable through feature negotiation. There will be two callbacks,
-     * the first indicates the guest driver is considered legacy VirtIO, as it is critical to
-     * assume that initially. A 2nd callback will occur during feature negotiation
-     * which will indicate the guest is modern, if the guest acknowledges VIRTIO_F_VERSION_1,
-     * feature, or legacy if the feature isn't negotiated. That 2nd callback allows
-     * the device-specific code to configure its behavior in terms of both guest version and features.
+     * Implementation-specific client callback to report VirtIO when feature negotiation is
+     * complete. It should be invoked by the VirtIO core only once.
      *
-     * @param   pVirtio    Pointer to the shared virtio state.
-     * @param   fModern    True if guest driver identified itself as modern (e.g. VirtIO 1.0 featured)
+     * @param   pVirtio           Pointer to the shared virtio state.
+     * @param   fDriverFeatures   Bitmask of features the guest driver has accepted/declined.
+     * @param   fLegacy           true if legacy mode offered and until guest driver identifies itself
+     *                            as modern(e.g. VirtIO 1.0 featured)
      */
-    DECLCALLBACKMEMBER(void, pfnGuestVersionHandler,(PVIRTIOCORE pVirtio, uint32_t fModern));
+    DECLCALLBACKMEMBER(void, pfnFeatureNegotiationComplete, (PVIRTIOCORE pVirtio, uint64_t fDriverFeatures, uint32_t fLegacy));
 
     /**
@@ -436 +433 @@
     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
@@ -469 +465 @@
 {
     /**
-     * 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.
+     * This callback notifies the device-specific portion of this device implementation (if guest-to-host
+     * queue notifications are enabled), that the guest driver has notified the host (this device)
+     * that the VirtIO "avail" ring of a queue has some new s/g buffers added by the guest VirtIO driver.
      *
      * @param   pVirtio    Pointer to the shared virtio state.
@@ -488 +485 @@
 } VIRTIOCORERC;
 
-
 /** @typedef VIRTIOCORECC
  * The instance data for the current context. */
 typedef CTX_SUFF(VIRTIOCORE) VIRTIOCORECC;
 
-
 /** @name API for VirtIO parent device
  * @{ */
@@ -502 +497 @@
  * This should be called from PDMDEVREGR3::pfnConstruct.
  *
- * @param   pDevIns                 The device instance.
+ * @param   pDevIns                 Device instance.
  * @param   pVirtio                 Pointer to the shared virtio state.  This
  *                                  must be the first member in the shared
@@ -519 +514 @@
 int virtioCoreR3Init(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC,
                           PVIRTIOPCIPARAMS pPciParams, const char *pcszInstance,
-                          uint64_t fDevSpecificFeatures, void *pvDevSpecificCfg, uint16_t cbDevSpecificCfg);
-
+                          uint64_t fDevSpecificFeatures, uint32_t fOfferLegacy, void *pvDevSpecificCfg, uint16_t cbDevSpecificCfg);
 /**
  * Initiate orderly reset procedure. This is an exposed API for clients that might need it.
@@ -532 +526 @@
  * 'Attaches' host device-specific implementation's queue state to host VirtIO core
  * virtqueue management infrastructure, informing the virtio core of the name of the
- * queue associated with the queue number. uVirtqNbr is used as the 'handle' for virt queues
- * in this API (and is opaquely the index into the VirtIO core's array of queue state).
- *
- * Virtqueue numbers are VirtIO specification defined (i.e. they are unique within each
- * VirtIO device type).
+ * queue to associate with the queue number.
+
+ * Note: uVirtqNbr (ordinal index) is used as the 'handle' for virtqs in this VirtioCore
+ * implementation's API (as an opaque selector into the VirtIO core's array of queues' states).
+ *
+ * Virtqueue numbers are actually VirtIO-specification defined device-specifically
+ * (i.e. they are unique within each VirtIO device type), but are in some cases scalable
+ * so only the pattern of queue numbers is defined by the spec and implementations may contain
+ * a self-determined plurality of queues.
  *
  * @param   pVirtio     Pointer to the shared virtio state.
@@ -547 +545 @@
 
 /**
- * Enables or disables a virtq
+ * Detaches host device-specific implementation's queue state from the host VirtIO core
+ * virtqueue management infrastructure, informing the VirtIO core that the queue is
+ * not utilized by the device-specific code.
  *
  * @param   pVirtio     Pointer to the shared virtio state.
  * @param   uVirtqNbr   Virtq number
- * @param   fEnable     Flags whether to enable or disable the virtq
- *
- */
-void  virtioCoreVirtqEnable(PVIRTIOCORE pVirtio, uint16_t uVirtqNbr, bool fEnable);
+ * @param   pcszName    Name to give queue
+ *
+ * @returns VBox status code.
+ */
+int  virtioCoreR3VirtqDetach(PVIRTIOCORE pVirtio, uint16_t uVirtqNbr);
+
+/**
+ * Checks to see whether queue is attached to core.
+ *
+ * @param   pVirtio     Pointer to the shared virtio state.
+ * @param   uVirtqNbr   Virtq number
+ *
+ * Returns boolean true or false indicating whether dev-specific reflection
+ * of queue is attached to core.
+ */
+bool  virtioCoreR3VirtqIsAttached(PVIRTIOCORE pVirtio, uint16_t uVirtqNbr);
+
+/**
+ * Checks to see whether queue is enabled.
+ *
+ * @param   pVirtio     Pointer to the shared virtio state.
+ * @param   uVirtqNbr   Virtq number
+ *
+ * Returns boolean true or false indicating core queue enable state.
+ * There is no API function to enable the queue, because the actual enabling is handled
+ * by the guest via MMIO.
+ *
+ * NOTE: Guest VirtIO driver's claim over this state is overridden (which violates VirtIO 1.0 spec
+ * in a carefully controlled manner) in the case where the queue MUST be disabled, due to observed
+ * control queue corruption (e.g. null GCPhys virtq base addr) while restoring legacy-only device's
+ * (DevVirtioNet.cpp) as a way to flag that the queue is unusable-as-saved and must to be removed.
+ * That is all handled in the load/save exec logic. Device reset could potentially, depending on
+ * parameters passed from host VirtIO device to guest VirtIO driver, result in guest re-establishing
+ * queue, except, in that situation, the queue operational state would be valid.
+ */
+bool  virtioCoreR3VirtqIsEnabled(PVIRTIOCORE pVirtio, uint16_t uVirtqNbr);
 
 /**
  * Enable or disable notification for the specified queue.
  *
- * With notification enabled, the guest driver notifies the host device (via MMIO
- * to the queue notification offset describe in VirtIO 1.0, 4.1.4.4 "Notification Structure Layout")
- * whenever the guest driver adds a new entry to the avail ring of the respective queue.
- *
- * Note: In the VirtIO world, the device sets flags in the used ring to communicate to the driver how to
- * handle notifications for the avail ring and the drivers sets flags in the avail ring to communicate
- * to the device how to handle sending interrupts for the used ring.
+ * When queue notifications are enabled, the guest VirtIO driver notifies host VirtIO device
+ * (via MMIO, see VirtIO 1.0, 4.1.4.4 "Notification Structure Layout") whenever guest driver adds
+ * a new s/g buffer to the "avail" ring of the queue.
+ *
+ * Note: VirtIO queue layout includes flags the device controls in "used" ring to inform guest
+ * driver if it should notify host of guest's buffer additions to the "avail" ring, and
+ * conversely, the guest driver sets flags in the "avail" ring to communicate to host device
+ * whether or not to interrupt guest when it adds buffers to used ring.
  *
  * @param   pVirtio     Pointer to the shared virtio state.
@@ -581 +614 @@
 
 /**
- * Displays the VirtIO spec-related features offered and their accepted/declined status
- * by both the VirtIO core and dev-specific device code (which invokes this function).
- * The result is a comprehensive list of available features the VirtIO specification
- * defines, which ones were actually offered by the device, and which ones were accepted
- * by the guest driver, thus providing a legible summary view of the configuration
- * the device is operating with.
- *
+ * Displays a well-formated human-readable translation of otherwise inscrutable bitmasks
+ * that embody features VirtIO specification definitions, indicating: Totality of features
+ * that can be implemented by host and guest, which features were offered by the host, and
+ * which were actually accepted by the guest. It displays it as a summary view of the device's
+ * finalized operational state (host-guest negotiated architecture) in such a way that shows
+ * which options are available for implementing or enabling.
+ *
+ * The non-device-specific VirtIO features list are managed by core API (e.g. implied).
+ * Only dev-specific features must be passed as parameter.
+
  * @param   pVirtio     Pointer to the shared virtio state.
  * @param   pHlp        Pointer to the debug info hlp struct
- * @param   s_aDevSpecificFeatures
- *                      Features specification lists for device-specific implementation
- *                      (i.e: net controller, scsi controller ...)
+ * @param   s_aDevSpecificFeatures  Dev-specific features (virtio-net, virtio-scsi...)
  * @param   cFeatures   Number of features in aDevSpecificFeatures
  */
@@ -599 +633 @@
 
 /*
- * Debuging assist feature displays the state of the VirtIO core code, which includes
+ * Debug-assist utility function to display state of the VirtIO core code, including
  * an overview of the state of all of the queues.
  *
@@ -608 +642 @@
  *
  * This is implemented currently to be invoked by the inheriting device-specific code
- * (see DevVirtioNet for an example, which receives the debugvm callback directly).
- * DevVirtioNet lists the available sub-options if no arguments are provided. In that
+ * (see the the VirtualBox virtio-net (VirtIO network controller device implementation)
+ * for an example of code that receive debugvm callback directly).
+ *
+ * DevVirtioNet lists available sub-options if no arguments are provided. In that
  * example this virtq info related function is invoked hierarchically when virtio-net
  * displays its device-specific queue info.
@@ -629 +665 @@
 
 /**
- * This function is identical to virtioCoreR3VirtqAvailBufGet(), except it doesn't 'consume'
- * the buffer from the avail ring of the virtq. The peek operation becomes identical to a get
- * operation if virtioCoreR3VirtqAvailRingNext() is called to consume the buffer from the avail ring,
- * at which point virtioCoreR3VirtqUsedBufPut() must be called to complete the roundtrip
- * transaction by putting the descriptor on the used ring.
- *
+ * This function is identical to virtioCoreR3VirtqAvailBufGet(), *except* it doesn't consume
+ * peeked buffer from avail ring of the virtq. The function *becomes* identical to the
+ * virtioCoreR3VirtqAvailBufGet() only if virtioCoreR3VirtqAvailRingNext() is invoked to
+ * consume buf from the queue's avail ring, followed by invocation of virtioCoreR3VirtqUsedBufPut(),
+ * to hand host-processed buffer back to guest, which completes guest-initiated virtq buffer circuit.
  *
  * @param   pDevIns     The device instance.
@@ -652 +687 @@
 /**
  * This function fetches the next buffer (descriptor chain) from the VirtIO "avail" ring of
- * indicated queue, and converts the buf's s/g vectors into OUT (e.g. guest-to-host)
+ * indicated queue, separating the buf's s/g vectors into OUT (e.g. guest-to-host)
  * components and and IN (host-to-guest) components.
  *
- * The caller is responsible for GCPhys to host virtual memory conversions. If the
+ * Caller is responsible for GCPhys to host virtual memory conversions. If the
  * virtq buffer being peeked at is "consumed", virtioCoreR3VirtqAvailRingNext() must
- * be called and in that case virtioCoreR3VirtqUsedBufPut() must be called to
- * complete the roundtrip virtq transaction.
+ * be called, and after that virtioCoreR3VirtqUsedBufPut() must be called to
+ * complete the buffer transfer cycle with the guest.
  *
  * @param   pDevIns     The device instance.
@@ -678 +713 @@
 
 /**
- * Fetches a specific descriptor chain using avail ring of indicated queue and converts the descriptor
- * chain into its OUT (to device) and IN (to guest) components.
+ * Fetches a specific descriptor chain using avail ring of indicated queue and converts the
+ * descriptor chain into its OUT (to device) and IN (to guest) components.
  *
  * The caller is responsible for GCPhys to host virtual memory conversions and *must*
@@ -704 +739 @@
 /**
  * Returns data to the guest to complete a transaction initiated by virtioCoreR3VirtqAvailBufGet(),
- * or virtioCoreR3VirtqAvailBufPeek()/virtioCoreR3VirtqBufSync() call pairs to complete each
- * intervening a roundtrip transaction, ultimately putting each descriptor chain pulled from the
- * avail ring of a queue onto the used ring of the queue. wherein I/O transactions are always
- * initiated by the guest and completed by the host. In other words, for the host to send any
- * data to the guest, the guest must provide buffers, for the host to fill, via the avail ring
- * of the virtq.
+ * (or virtioCoreR3VirtqAvailBufPeek()/virtioCoreR3VirtqBufSync() call pair), to complete each
+ * buffer transfer transaction (guest-host buffer cycle), ultimately moving each descriptor chain
+ * from the avail ring of a queue onto the used ring of the queue. Note that VirtIO buffer
+ * transactions are *always* initiated by the guest and completed by the host. In other words,
+ * for the host to send any I/O related data to the guest (and in some cases configuration data),
+ * the guest must provide buffers via the virtq's avail ring, for the host to fill.
  *
  * At some some point virtioCoreR3VirtqUsedRingSync() must be called to return data to the guest,
- * completing all pending virtioCoreR3VirtqAvailBufPut() transactions that have accumulated since
- * the last call to virtioCoreR3VirtqUsedRingSync()
-
- * @note This does a write-ahead to the used ring of the guest's queue. The data
- *       written won't be seen by the guest until the next call to virtioCoreVirtqUsedRingSync()
- *
+ * completing all pending virtioCoreR3VirtqAvailBufPut() operations that have accumulated since
+ * the last call to virtioCoreR3VirtqUsedRingSync().
+
+ * @note This function effectively performs write-ahead to the used ring of the virtq.
+ *       Data written won't be seen by the guest until the next call to virtioCoreVirtqUsedRingSync()
  *
  * @param   pDevIns         The device instance (for reading).
@@ -729 +763 @@
  *                          buffer originally pulled from the queue.
  *
- * @param   fFence          If true, put up copy fence (memory barrier) after
+ * @param   fFence          If true (default), put up copy-fence (memory barrier) after
  *                          copying to guest phys. mem.
  *
@@ -741 +775 @@
  */
 int virtioCoreR3VirtqUsedBufPut(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t uVirtqNbr, PRTSGBUF pSgVirtReturn,
-                                 PVIRTQBUF pVirtqBuf, bool fFence);
+                                 PVIRTQBUF pVirtqBuf, bool fFence = true);
+
+
+/**
+ * Quicker variant of same-named function (directly above) that it overloads,
+ * Instead, this variant accepts as input a pointer to a buffer and count,
+ * instead of S/G buffer thus doesn't have to copy between two S/G buffers and avoids some overhead.
+ *
+ * @param   pDevIns         The device instance (for reading).
+ * @param   pVirtio         Pointer to the shared virtio state.
+ * @param   uVirtqNbr       Virtq number
+ * @param   cb              Number of bytes to add to copy to phys. buf.
+ * @param   pv              Virtual mem buf to copy to phys buf.
+ * @param   cbEnqueue       How many bytes in packet to enqueue (0 = don't enqueue)
+ * @param   fFence          If true (default), put up copy-fence (memory barrier) after
+ *                          copying to guest phys. mem.
+ *
+ * @returns VBox status code.
+ * @retval  VINF_SUCCESS       Success
+ * @retval  VERR_INVALID_STATE VirtIO not in ready state
+ * @retval  VERR_NOT_AVAILABLE Virtq is empty
+ *
+ * @note    This function will not release any reference to pVirtqBuf.  The
+ *          caller must take care of that.
+ */
+int virtioCoreR3VirtqUsedBufPut(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t uVirtq, size_t cb, const void *pv,
+                            PVIRTQBUF pVirtqBuf, uint32_t cbEnqueue, bool fFence = true);
+
+
 /**
  * Advance index of avail ring to next entry in specified virtq (see virtioCoreR3VirtqAvailBufPeek())
@@ -751 +813 @@
 
 /**
- * Checks to see if guest has acknowledged device's VIRTIO_F_VERSION_1 feature.
- * If not, it's presumed to be a VirtIO legacy guest driver. Note that legacy drivers
- * may start using the device prematurely, as opposed to the rigorously sane protocol
- * prescribed by the "modern" VirtIO spec. Early access implies a legacy driver.
- * Therefore legacy mode is the assumption until feature negotiation.
+ * Checks to see if guest has accepted host device's VIRTIO_F_VERSION_1 (i.e. "modern")
+ * behavioral modeling, indicating guest agreed to comply with the modern VirtIO 1.0+ specification.
+ * Otherwise unavoidable presumption is that the host device is dealing with legacy VirtIO
+ * guest drive, thus must be prepared to cope with less mature architecture and behaviors
+ * from  prototype era of VirtIO. (see comments in PDM-invoked device constructor for more information).
  *
  * @param   pVirtio      Pointer to the virtio state.
@@ -761 +823 @@
 int virtioCoreIsLegacyMode(PVIRTIOCORE pVirtio);
 
+/**
+ * This VirtIO transitional device supports "modern" (rev 1.0+) as well as "legacy" (e.g. < 1.0) VirtIO drivers.
+ * Some legacy guest drivers are known to mishandle PCI bus mastering wherein the PCI flavor of GC phys
+ * access functions can't be used. The following wrappers select the memory access method based on whether the
+ * device is operating in legacy mode or not.
+ */
+DECLINLINE(int) virtioCoreGCPhysWrite(PVIRTIOCORE pVirtio, PPDMDEVINS pDevIns, RTGCPHYS GCPhys, void *pvBuf, size_t cbWrite)
+{
+    int rc;
+    if (virtioCoreIsLegacyMode(pVirtio))
+        rc = PDMDevHlpPhysWrite(pDevIns, GCPhys, pvBuf, cbWrite);
+    else
+        rc = PDMDevHlpPCIPhysWrite(pDevIns, GCPhys, pvBuf, cbWrite);
+    return rc;
+}
+
+DECLINLINE(int) virtioCoreGCPhysRead(PVIRTIOCORE pVirtio, PPDMDEVINS pDevIns, RTGCPHYS GCPhys, void *pvBuf, size_t cbRead)
+{
+    int rc;
+    if (virtioCoreIsLegacyMode(pVirtio))
+        rc = PDMDevHlpPhysRead(pDevIns, GCPhys, pvBuf, cbRead);
+    else
+        rc = PDMDevHlpPCIPhysRead(pDevIns, GCPhys, pvBuf, cbRead);
+    return rc;
+}
+
+/*
+ * (See comments for corresponding function in sg.h)
+ */
 DECLINLINE(void) virtioCoreGCPhysChainInit(PVIRTIOSGBUF pGcSgBuf, PVIRTIOSGSEG paSegs, size_t cSegs)
 {
@@ -782 +873 @@
 }
 
+/*
+ * (See comments for corresponding function in sg.h)
+ */
 DECLINLINE(RTGCPHYS) virtioCoreGCPhysChainGet(PVIRTIOSGBUF pGcSgBuf, size_t *pcbData)
 {
@@ -826 +920 @@
 }
 
+/*
+ * (See comments for corresponding function in sg.h)
+ */
 DECLINLINE(void) virtioCoreGCPhysChainReset(PVIRTIOSGBUF pGcSgBuf)
 {
@@ -843 +940 @@
 }
 
+/*
+ * (See comments for corresponding function in sg.h)
+ */
 DECLINLINE(RTGCPHYS) virtioCoreGCPhysChainAdvance(PVIRTIOSGBUF pGcSgBuf, size_t cbAdvance)
 {
@@ -860 +960 @@
 }
 
+/*
+ * (See comments for corresponding function in sg.h)
+ */
 DECLINLINE(RTGCPHYS) virtioCoreGCPhysChainGetNextSeg(PVIRTIOSGBUF pGcSgBuf, size_t *pcbSeg)
 {
@@ -871 +974 @@
 }
 
-DECLINLINE(size_t) virtioCoreGCPhysChainCalcBufSize(PVIRTIOSGBUF pGcSgBuf)
+/**
+ * Calculate the length of a GCPhys s/g buffer by tallying the size of each segment.
+ *
+ * @param   pGcSgBuf        Guest Context (GCPhys) S/G buffer to calculate length of
+ */
+DECLINLINE(size_t) virtioCoreGCPhysChainCalcBufSize(PCVIRTIOSGBUF pGcSgBuf)
 {
     size_t   cb = 0;
     unsigned i  = pGcSgBuf->cSegs;
-     while (i-- > 0)
-         cb += pGcSgBuf->paSegs[i].cbSeg;
-     return cb;
- }
-
+    while (i-- > 0)
+        cb += pGcSgBuf->paSegs[i].cbSeg;
+    return cb;
+}
+
+/*
+ * (See comments for corresponding function in sg.h)
+ */
+DECLINLINE(size_t) virtioCoreGCPhysChainCalcLengthLeft(PVIRTIOSGBUF pGcSgBuf)
+{
+    size_t   cb = pGcSgBuf->cbSegLeft;
+    unsigned i  = pGcSgBuf->cSegs;
+    while (i-- > pGcSgBuf->idxSeg + 1)
+        cb += pGcSgBuf->paSegs[i].cbSeg;
+    return cb;
+}
 #define VIRTQNAME(a_pVirtio, a_uVirtq) ((a_pVirtio)->aVirtqueues[(a_uVirtq)].szName)
 
 /**
- * Add some bytes to a virtq (s/g) buffer, converting them from virtual memory to GCPhys
- *
- * To be performant it is left to the caller to validate the size of the buffer with regard
- * to data being pulled from it to avoid overruns/underruns.
+ * Convert and append bytes from a virtual-memory simple buffer to VirtIO guest's
+ * physical memory described by a buffer pulled form the avail ring of a virtq.
  *
  * @param   pVirtio     Pointer to the shared virtio state.
- * @param   pVirtqBuf   output: virtq buffer
+ * @param   pVirtqBuf   VirtIO buffer to fill
  * @param   pv          input: virtual memory buffer to receive bytes
  * @param   cb          number of bytes to add to the s/g buffer.
@@ -895 +1012 @@
 DECLINLINE(void) virtioCoreR3VirqBufFill(PVIRTIOCORE pVirtio, PVIRTQBUF pVirtqBuf, void *pv, size_t cb)
 {
-    uint8_t *pb = (uint8_t *)pv;
-    size_t cbLim = RT_MIN(pVirtqBuf->cbPhysReturn, cb);
-    while (cbLim)
+    uint8_t *pvBuf = (uint8_t *)pv;
+    size_t cbRemain = cb, cbTotal = 0;
+    PVIRTIOSGBUF pSgPhysReturn = pVirtqBuf->pSgPhysReturn;
+    while (cbRemain)
     {
-        size_t cbSeg = cbLim;
-        RTGCPHYS GCPhys = virtioCoreGCPhysChainGetNextSeg(pVirtqBuf->pSgPhysReturn, &cbSeg);
-        PDMDevHlpPCIPhysWrite(pVirtio->pDevInsR3, GCPhys, pb, cbSeg);
-        pb += cbSeg;
-        cbLim -= cbSeg;
-        pVirtqBuf->cbPhysSend -= cbSeg;
+        uint32_t cbBounded = RT_MIN(pSgPhysReturn->cbSegLeft, cbRemain);
+        Assert(cbBounded > 0);
+        virtioCoreGCPhysWrite(pVirtio, CTX_SUFF(pVirtio->pDevIns), (RTGCPHYS)pSgPhysReturn->GCPhysCur, pvBuf, cbBounded);
+        virtioCoreGCPhysChainAdvance(pSgPhysReturn, cbBounded);
+        pvBuf += cbBounded;
+        cbRemain -= cbBounded;
+        cbTotal += cbBounded;
     }
-    LogFunc(("Added %d/%d bytes to %s buffer, head idx: %u (%d bytes remain)\n",
-             cb - cbLim, cb, VIRTQNAME(pVirtio, pVirtqBuf->uVirtq),
-             pVirtqBuf->uHeadIdx, pVirtqBuf->cbPhysReturn));
-}
-
-/**
- * Extract some bytes out of a virtq (s/g) buffer, converting them from GCPhys to virtual memory
- *
- * To be performant it is left to the caller to validate the size of the buffer with regard
- * to data being pulled from it to avoid overruns/underruns.
+    LogFunc(("Appended %d bytes to guest phys buf [head: %u]. %d bytes unused in buf.)\n",
+             cbTotal, pVirtqBuf->uHeadIdx, virtioCoreGCPhysChainCalcLengthLeft(pSgPhysReturn)));
+}
+
+/**
+ * Extract some bytes from of a virtq s/g buffer, converting them from GCPhys space to
+ * to ordinary virtual memory (i.e. making data directly accessible to host device code)
+ *
+ * As a performance optimization, it is left to the caller to validate buffer size.
  *
  * @param   pVirtio     Pointer to the shared virtio state.
@@ -937 +1055 @@
     LogFunc(("Drained %d/%d bytes from %s buffer, head idx: %u (%d bytes left)\n",
              cb - cbLim, cb, VIRTQNAME(pVirtio, pVirtqBuf->uVirtq),
-             pVirtqBuf->uHeadIdx, pVirtqBuf->cbPhysSend));
+             pVirtqBuf->uHeadIdx, virtioCoreGCPhysChainCalcLengthLeft(pVirtqBuf->pSgPhysReturn)));
 }
 
@@ -1016 +1134 @@
  * VirtIO implementation to identify this device's operational configuration after features
  * have been negotiated with guest VirtIO driver. Feature negotiation entails host indicating
- * to guest which features it supports, then guest accepting among those offered which features
+ * to guest which features it supports, then guest accepting from among the offered, which features
  * it will enable. That becomes the agreement between the host and guest. The bitmask containing
  * virtio core features plus device-specific features is provided as a parameter to virtioCoreR3Init()
@@ -1031 +1149 @@
 
 /**
- * Get the the name of the VM state change associated with the enumeration variable
+ * Get name of the VM state change associated with the enumeration variable
  *
  * @param enmState       VM state (enumeration value)
@@ -1078 +1196 @@
 /**
  * Debug assist for any consumer device code
-&
  * Do a hex dump of memory in guest physical context
  *
@@ -1093 +1210 @@
  */
 
-/**
- * Calculate the length of a GCPhys s/g buffer by tallying the size of each segment.
- *
- * @param   pGcSgBuf        Guest Context (GCPhys) S/G buffer to calculate length of
- */
-DECLINLINE(size_t) virtioCoreGCPhysChainCalcBufSize(PCVIRTIOSGBUF pGcSgBuf)
-{
-    size_t   cb = 0;
-    unsigned i  = pGcSgBuf->cSegs;
-    while (i-- > 0)
-        cb += pGcSgBuf->paSegs[i].cbSeg;
-    return cb;
-}
-
-/**
- * This VirtIO transitional device supports "modern" (rev 1.0+) as well as "legacy" (e.g. < 1.0) VirtIO drivers.
- * Some legacy guest drivers are known to mishandle PCI bus mastering wherein the PCI flavor of GC phys
- * access functions can't be used. The following wrappers select the mem access method based on whether the
- * device is operating in legacy mode or not.
- */
-DECLINLINE(int) virtioCoreGCPhysWrite(PVIRTIOCORE pVirtio, PPDMDEVINS pDevIns, RTGCPHYS GCPhys, void *pvBuf, size_t cbWrite)
-{
-    int rc;
-    if (virtioCoreIsLegacyMode(pVirtio))
-        rc = PDMDevHlpPhysWrite(pDevIns, GCPhys, pvBuf, cbWrite);
-    else
-        rc = PDMDevHlpPCIPhysWrite(pDevIns, GCPhys, pvBuf, cbWrite);
-    return rc;
-}
-
-DECLINLINE(int) virtioCoreGCPhysRead(PVIRTIOCORE pVirtio, PPDMDEVINS pDevIns, RTGCPHYS GCPhys, void *pvBuf, size_t cbRead)
-{
-    int rc;
-    if (virtioCoreIsLegacyMode(pVirtio))
-        rc = PDMDevHlpPhysRead(pDevIns, GCPhys, pvBuf, cbRead);
-    else
-        rc = PDMDevHlpPCIPhysRead(pDevIns, GCPhys, pvBuf, cbRead);
-    return rc;
-}
-
 /** Misc VM and PDM boilerplate */
-int      virtioCoreR3SaveExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM);
-int      virtioCoreR3LoadExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM);
+int      virtioCoreR3SaveExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM, uint32_t uVersion, uint32_t cQueues);
+int      virtioCoreR3ModernDeviceLoadExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM, uint32_t uVersion, uint32_t uTestVersion, uint32_t cQueues);
+int      virtioCoreR3LegacyDeviceLoadExec(PVIRTIOCORE pVirtio, PCPDMDEVHLPR3 pHlp, PSSMHANDLE pSSM, uint32_t uVersion, uint32_t uVirtioLegacy_3_1_Beta);
 void     virtioCoreR3VmStateChanged(PVIRTIOCORE pVirtio, VIRTIOVMSTATECHANGED enmState);
 void     virtioCoreR3Term(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, PVIRTIOCORECC pVirtioCC);
@@ -1147 +1225 @@
  * cb, pv and fWrite are implicit parameters and must be defined by the invoker.
  */
-
 #ifdef LOG_ENABLED
 
@@ -1201 +1278 @@
  * the memory described by cb and pv.
  *
- * cb, pv and fWrite are implicit parameters and must be defined by the invoker.
+ * cb, pv and fWrite are implicit parameters and must be defined by invoker.
  */
 #define VIRTIO_DEV_CONFIG_ACCESS(member, tCfgStruct, uOffsetOfAccess, pCfgStruct) \
@@ -1216 +1293 @@
 /**
  * Copies bytes into memory described by cb, pv from the specified member field of the config struct.
- * The operation is a nop and logs error if implied parameter fWrite is true.
+ * The operation is a NOP, logging an error if an implied parameter, fWrite, is boolean true.
  *
  * cb, pv and fWrite are implicit parameters and must be defined by the invoker.
@@ -1237 +1314 @@
  * the memory described by cb and pv.
  *
- * cb, pv and fWrite are implicit parameters and must be defined by the invoker.
+ * cb, pv and fWrite are implicit parameters and must be defined by invoker.
  */
 #define VIRTIO_DEV_CONFIG_ACCESS_INDEXED(member, uIdx, tCfgStruct, uOffsetOfAccess, pCfgStruct) \
@@ -1254 +1331 @@
  * The operation is a nop and logs error if implied parameter fWrite is true.
  *
- * cb, pv and fWrite are implicit parameters and must be defined by the invoker.
+ * cb, pv and fWrite are implicit parameters and must be defined by invoker.
  */
 #define VIRTIO_DEV_CONFIG_ACCESS_INDEXED_READONLY(member, uidx, tCfgStruct, uOffsetOfAccess, pCfgStruct) \