Changeset 82004 in vbox for trunk/src/VBox

Timestamp:

Nov 19, 2019 4:55:37 PM (5 years ago)

Author:

vboxsync

Message:

Storage/DevVirtioSCSI.cpp: Finish last of Knut's to do items except R0/RC handling. Still working on save/load execå.

Location:

trunk/src/VBox/Devices

Files:

• Storage/DevVirtioSCSI.cpp (modified) (1 diff)
• VirtIO/Virtio_1_0.cpp (modified) (11 diffs)
• VirtIO/Virtio_1_0.h (modified) (2 diffs)

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

@@ -952 +952 @@
                          cbReqSgBuf, pReq->pDescChain->cbPhysReturn),
                        VERR_BUFFER_OVERFLOW);
-
 
         virtioCoreR3QueuePut(pDevIns, &pThis->Virtio, pReq->qIdx, pReqSegBuf, pReq->pDescChain, true /* fFence TBD */);

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

@@ -270 +270 @@
 {
     AssertPtr(pGcSgBuf);
-    Assert(   (cSegs > 0 && VALID_PTR(paSegs))
-           || (!cSegs && !paSegs));
+    Assert(   (cSegs > 0 && VALID_PTR(paSegs)) || (!cSegs && !paSegs));
     Assert(cSegs < (~(unsigned)0 >> 1));
 
@@ -293 +292 @@
     size_t cbData;
     RTGCPHYS pGcBuf;
+
     /* Check that the S/G buffer has memory left. */
     if (RT_LIKELY(pGcSgBuf->idxSeg < pGcSgBuf->cSegs && pGcSgBuf->cbSegLeft))
@@ -302 +302 @@
     }
 
-    AssertMsg(  pGcSgBuf->cbSegLeft <= 128 * _1M
-            && (RTGCPHYS)pGcSgBuf->pGcSegCur >= (RTGCPHYS)pGcSgBuf->paSegs[pGcSgBuf->idxSeg].pGcSeg
-            && (RTGCPHYS)pGcSgBuf->pGcSegCur + pGcSgBuf->cbSegLeft <=
+    AssertMsg(    pGcSgBuf->cbSegLeft <= 128 * _1M
+              && (RTGCPHYS)pGcSgBuf->pGcSegCur >= (RTGCPHYS)pGcSgBuf->paSegs[pGcSgBuf->idxSeg].pGcSeg
+              && (RTGCPHYS)pGcSgBuf->pGcSegCur + pGcSgBuf->cbSegLeft <=
                    (RTGCPHYS)pGcSgBuf->paSegs[pGcSgBuf->idxSeg].pGcSeg + pGcSgBuf->paSegs[pGcSgBuf->idxSeg].cbSeg,
-              ("pGcSgBuf->idxSeg=%d pGcSgBuf->cSegs=%d pGcSgBuf->pGcSegCur=%p pGcSgBuf->cbSegLeft=%zd "
-               "pGcSgBuf->paSegs[%d].pGcSeg=%p pGcSgBuf->paSegs[%d].cbSeg=%zd\n",
-               pGcSgBuf->idxSeg, pGcSgBuf->cSegs, pGcSgBuf->pGcSegCur, pGcSgBuf->cbSegLeft,
-               pGcSgBuf->idxSeg, pGcSgBuf->paSegs[pGcSgBuf->idxSeg].pGcSeg, pGcSgBuf->idxSeg,
-               pGcSgBuf->paSegs[pGcSgBuf->idxSeg].cbSeg));
+                 ("pGcSgBuf->idxSeg=%d pGcSgBuf->cSegs=%d pGcSgBuf->pGcSegCur=%p pGcSgBuf->cbSegLeft=%zd "
+                  "pGcSgBuf->paSegs[%d].pGcSeg=%p pGcSgBuf->paSegs[%d].cbSeg=%zd\n",
+                  pGcSgBuf->idxSeg, pGcSgBuf->cSegs, pGcSgBuf->pGcSegCur, pGcSgBuf->cbSegLeft,
+                  pGcSgBuf->idxSeg, pGcSgBuf->paSegs[pGcSgBuf->idxSeg].pGcSeg, pGcSgBuf->idxSeg,
+                  pGcSgBuf->paSegs[pGcSgBuf->idxSeg].cbSeg));
 
     cbData = RT_MIN(*pcbData, pGcSgBuf->cbSegLeft);
@@ -881 +881 @@
 static void virtioNotifyGuestDriver(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue, bool fForce)
 {
+
     Assert(idxQueue < RT_ELEMENTS(pVirtio->virtqState));
     PVIRTQSTATE pVirtq = &pVirtio->virtqState[idxQueue];
@@ -1308 +1309 @@
  * @callback_method_impl{FNIOMMMIONEWREAD,
  * Memory mapped I/O Handler for PCI Capabilities read operations.}
+ *
+ * This MMIO handler specifically supports the VIRTIO_PCI_CAP_PCI_CFG capability defined
+ * in the VirtIO 1.0 specification, section 4.1.4.7, and as such is limited to cb == 1, cb == 2, or cb==4 type reads.
+ *
  */
 static DECLCALLBACK(VBOXSTRICTRC) virtioMmioRead(PPDMDEVINS pDevIns, void *pvUser, RTGCPHYS off, void *pv, unsigned cb)
@@ -1313 +1318 @@
     PVIRTIOCORE   pVirtio   = PDMINS_2_DATA(pDevIns, PVIRTIOCORE);
     PVIRTIOCORECC pVirtioCC = PDMINS_2_DATA_CC(pDevIns, PVIRTIOCORECC);
+    AssertReturn(cb == 1 || cb == 2 || cb == 4, VERR_INVALID_PARAMETER);
     Assert(pVirtio == (PVIRTIOCORE)pvUser); RT_NOREF(pvUser);
-
-    /** @todo r=bird: This code does not handle reads spanning more than one
-     * capability structure/area.   How does that match the spec?   For instance
-     * if the guest uses a 64-bit MOV instruction on this MMIO region, you'll
-     * see cb=8 here.  Same if it uses 16 or 32 byte reads.  Intel allows all
-     * this, so question is how it's supposed to be handled.  At a minimum there
-     * must be an explanation of that here.
-     */
 
     uint32_t offIntra;
@@ -1381 +1379 @@
  * @callback_method_impl{FNIOMMMIONEWREAD,
  * Memory mapped I/O Handler for PCI Capabilities write operations.}
+ *
+ * This MMIO handler specifically supports the VIRTIO_PCI_CAP_PCI_CFG capability defined
+ * in the VirtIO 1.0 specification, section 4.1.4.7, and as such is limited to cb == 1, cb == 2, or cb==4 type writes.
  */
 static DECLCALLBACK(VBOXSTRICTRC) virtioMmioWrite(PPDMDEVINS pDevIns, void *pvUser, RTGCPHYS off, void const *pv, unsigned cb)
@@ -1386 +1387 @@
     PVIRTIOCORE   pVirtio   = PDMINS_2_DATA(pDevIns, PVIRTIOCORE);
     PVIRTIOCORECC pVirtioCC = PDMINS_2_DATA_CC(pDevIns, PVIRTIOCORECC);
+
+    AssertReturn(cb == 1 || cb == 2 || cb == 4, VERR_INVALID_PARAMETER);
+
     Assert(pVirtio == (PVIRTIOCORE)pvUser); RT_NOREF(pvUser);
-
-    /** @todo r=bird: This code does not handle writes spanning more than one
-     * capability structure/area.   How does that match the spec?   For instance
-     * if the guest uses a 64-bit MOV instruction on this MMIO region, you'll
-     * see cb=8 here.  Same if it uses 16 or 32 byte reads.  Intel allows all
-     * this, so question is how it's supposed to be handled.  At a minimum there
-     * must be an explanation of that here.
-     */
 
     uint32_t offIntra;
@@ -1636 +1632 @@
 {
 
-    LogFunc(("State changing to %s: ***FUNCTIONALITY TBD***\n",
+    LogFunc(("State changing to %s\n",
         virtioCoreGetStateChangeText(enmState)));
 
@@ -1649 +1645 @@
             break;
         case kvirtIoVmStateChangedResume:
+            virtioNotifyGuestDriver(pVirtio->pDevIns, pVirtio, 0 /* idxQueue */, true /* fForce */);
             break;
         default:
@@ -1656 +1653 @@
     RT_NOREF(pDevIns, pVirtio);
 }
-
-
-/**
- * This sends notification ('kicks') guest driver to check queues for any new
- * elements in the used queue to process.
- *
- * It should be called after resuming in case anything was added to the queues
- * during suspend/quiescing and a notification was missed, to prevent the guest
- * from stalling after suspend.
- */
-void virtioCoreR3PropagateResumeNotification(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio)
-{
-    virtioNotifyGuestDriver(pDevIns, pVirtio, 0 /* idxQueue */, true /* fForce */);
-}
-
 
 /**

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

@@ -450 +450 @@
 }
 
-
 DECLINLINE(size_t) virtioCoreSgBufCalcTotalLength(PCVIRTIOSGBUF pGcSgBuf)
 {
@@ -470 +469 @@
 RTGCPHYS virtioCoreSgBufGetNextSegment(PVIRTIOSGBUF pGcSgBuf, size_t *pcbSeg);
 RTGCPHYS virtioCoreSgBufAdvance(PVIRTIOSGBUF pGcSgBuf, size_t cbAdvance);
-
 void     virtioCoreSgBufInit(PVIRTIOSGBUF pSgBuf, PVIRTIOSGSEG paSegs, size_t cSegs);
 size_t   virtioCoreSgBufCalcTotalLength(PCVIRTIOSGBUF pGcSgBuf);