Changeset 19113 in vbox for trunk/src/VBox/Devices/Network

Timestamp:

Apr 22, 2009 3:04:04 PM (16 years ago)

Author:

vboxsync

Message:

PCNet: handle rare cases were the register content changed when we left the critical section during RX

File:

• trunk/src/VBox/Devices/Network/DevPCNet.cpp (modified) (10 diffs)

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

@@ -1825 +1825 @@
  * Write data into guest receive buffers.
  */
-static void pcnetReceiveNoSync(PCNetState *pThis, const uint8_t *buf, size_t size)
+static void pcnetReceiveNoSync(PCNetState *pThis, const uint8_t *buf, size_t cbToRecv)
 {
     PPDMDEVINS pDevIns = PCNETSTATE_2_DEVINS(pThis);
     int is_padr = 0, is_bcast = 0, is_ladr = 0;
     unsigned iRxDesc;
-    int pkt_size;
-
-    if (RT_UNLIKELY(CSR_DRX(pThis) || CSR_STOP(pThis) || CSR_SPND(pThis) || !size))
+    int cbPacket;
+
+    if (RT_UNLIKELY(CSR_DRX(pThis) || CSR_STOP(pThis) || CSR_SPND(pThis) || !cbToRecv))
         return;
 
@@ -1841 +1841 @@
         return;
 
-    Log(("#%d pcnetReceiveNoSync: size=%d\n", PCNET_INST_NR, size));
+    Log(("#%d pcnetReceiveNoSync: size=%d\n", PCNET_INST_NR, cbToRecv));
 
     /*
@@ -1847 +1847 @@
      */
     if (   CSR_PROM(pThis)
-        || (is_padr  = padr_match(pThis, buf, size))
-        || (is_bcast = padr_bcast(pThis, buf, size))
-        || (is_ladr  = ladr_match(pThis, buf, size)))
+        || (is_padr  = padr_match(pThis, buf, cbToRecv))
+        || (is_bcast = padr_bcast(pThis, buf, cbToRecv))
+        || (is_ladr  = ladr_match(pThis, buf, cbToRecv)))
     {
         if (HOST_IS_OWNER(CSR_CRST(pThis)))
@@ -1878 +1878 @@
             RTGCPHYS32 crda = CSR_CRDA(pThis);
             RTGCPHYS32 next_crda;
-            RMD      rmd, next_rmd;
-
-            memcpy(src, buf, size);
+            RMD        rmd, next_rmd;
+
+            memcpy(src, buf, cbToRecv);
             if (!CSR_ASTRP_RCV(pThis))
             {
@@ -1886 +1886 @@
                 uint8_t *p = src;
 
-                while (size < 60)
-                    src[size++] = 0;
-                while (p != &src[size])
+                while (cbToRecv < 60)
+                    src[cbToRecv++] = 0;
+                while (p != &src[cbToRecv])
                     CRC(fcs, *p++);
-                ((uint32_t *)&src[size])[0] = htonl(fcs);
+                ((uint32_t *)&src[cbToRecv])[0] = htonl(fcs);
                 /* FCS at end of packet */
             }
-            size += 4;
-            pkt_size = (int)size;                           Assert((size_t)pkt_size == size);
+            cbToRecv += 4;
+            cbPacket = (int)cbToRecv;                           Assert((size_t)cbPacket == cbToRecv);
 
 #ifdef PCNET_DEBUG_MATCH
@@ -1904 +1904 @@
                 rmd.rmd1.stp = 1;
 
-            size_t count = RT_MIN(4096 - (size_t)rmd.rmd1.bcnt, size);
+            size_t cbBuf = RT_MIN(4096 - (size_t)rmd.rmd1.bcnt, cbToRecv);
             RTGCPHYS32 rbadr = PHYSADDR(pThis, rmd.rmd0.rbadr);
+
+            /* save the old value to check if it was changed as long as we didn't
+             * hold the critical section */
+            iRxDesc = CSR_RCVRC(pThis);
 
             /* We have to leave the critical section here or we risk deadlocking
@@ -1917 +1921 @@
              */
             PDMCritSectLeave(&pThis->CritSect);
-            PDMDevHlpPhysWrite(pDevIns, rbadr, src, count);
+            PDMDevHlpPhysWrite(pDevIns, rbadr, src, cbBuf);
             int rc = PDMCritSectEnter(&pThis->CritSect, VERR_SEM_BUSY);
             AssertReleaseRC(rc);
 
-            src  += count;
-            size -= count;
-
-            /* adapt the current RX descriptor counter */
-            iRxDesc = CSR_RCVRC(pThis);
-            if (iRxDesc-- < 2)
-                iRxDesc = CSR_RCVRL(pThis);
-            CSR_RCVRC(pThis) = iRxDesc;
-
-            while (size > 0)
+            /* RX disabled in the meantime? If so, abort RX. */
+            if (RT_UNLIKELY(CSR_DRX(pThis) || CSR_STOP(pThis) || CSR_SPND(pThis)))
+                return;
+
+            /* Was the register modified in the meantime? If so, don't touch the
+             * register but still update the RX descriptor. */
+            if (RT_LIKELY(iRxDesc == CSR_RCVRC(pThis)))
+            {
+                if (iRxDesc-- < 2)
+                    iRxDesc = CSR_RCVRL(pThis);
+                CSR_RCVRC(pThis) = iRxDesc;
+            }
+            else
+                iRxDesc = CSR_RCVRC(pThis);
+
+            src      += cbBuf;
+            cbToRecv -= cbBuf;
+
+            while (cbToRecv > 0)
             {
                 /* Read the entire next descriptor as we're likely to need it. */
@@ -1948 +1961 @@
                 rmd  = next_rmd;
 
-                count = RT_MIN(4096 - (size_t)rmd.rmd1.bcnt, size);
+                cbBuf = RT_MIN(4096 - (size_t)rmd.rmd1.bcnt, cbToRecv);
                 RTGCPHYS32 rbadr = PHYSADDR(pThis, rmd.rmd0.rbadr);
 
@@ -1955 +1968 @@
                  * handler associated with it. See above for additional comments. */
                 PDMCritSectLeave(&pThis->CritSect);
-                PDMDevHlpPhysWrite(pDevIns, rbadr, src, count);
-                int rc = PDMCritSectEnter(&pThis->CritSect, VERR_SEM_BUSY);
+                PDMDevHlpPhysWrite(pDevIns, rbadr, src, cbBuf);
+                rc = PDMCritSectEnter(&pThis->CritSect, VERR_SEM_BUSY);
                 AssertReleaseRC(rc);
 
-                src  += count;
-                size -= count;
-
-                /* adapt the current RX descriptor counter */
-                iRxDesc = CSR_RCVRC(pThis);
-                if (iRxDesc-- < 1)
-                    iRxDesc = CSR_RCVRL(pThis);
-                CSR_RCVRC(pThis) = iRxDesc;
+                /* RX disabled in the meantime? If so, abort RX. */
+                if (RT_UNLIKELY(CSR_DRX(pThis) || CSR_STOP(pThis) || CSR_SPND(pThis)))
+                    return;
+
+                /* Was the register modified in the meantime? If so, don't touch the
+                 * register but still update the RX descriptor. */
+                if (RT_LIKELY(iRxDesc == CSR_RCVRC(pThis)))
+                {
+                    if (iRxDesc-- < 2)
+                        iRxDesc = CSR_RCVRL(pThis);
+                    CSR_RCVRC(pThis) = iRxDesc;
+                }
+                else
+                    iRxDesc = CSR_RCVRC(pThis);
+
+                src      += cbBuf;
+                cbToRecv -= cbBuf;
             }
 
-            if (RT_LIKELY(size == 0))
+            if (RT_LIKELY(cbToRecv == 0))
             {
                 rmd.rmd1.enp  = 1;
@@ -1975 +1997 @@
                 rmd.rmd1.lafm = !CSR_PROM(pThis) && is_ladr;
                 rmd.rmd1.bam  = !CSR_PROM(pThis) && is_bcast;
-                rmd.rmd2.mcnt = pkt_size;
-
-                STAM_REL_COUNTER_ADD(&pThis->StatReceiveBytes, pkt_size);
+                rmd.rmd2.mcnt = cbPacket;
+
+                STAM_REL_COUNTER_ADD(&pThis->StatReceiveBytes, cbPacket);
             }
             else
             {
-                Log(("#%d: Overflow by %ubytes\n", PCNET_INST_NR, size));
+                Log(("#%d: Overflow by %ubytes\n", PCNET_INST_NR, cbToRecv));
                 rmd.rmd1.oflo = 1;
                 rmd.rmd1.buff = 1;