Changeset 61691 in vbox for trunk

Timestamp:

Jun 14, 2016 9:44:53 AM (9 years ago)

Author:

vboxsync

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

108049

Message:

bugref:8319: Devices/E1000: Interrupt throttling re-worked

File:

• trunk/src/VBox/Devices/Network/DevE1000.cpp (modified) (12 diffs)

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

@@ -62 +62 @@
  */
 #define E1K_LSC_ON_SLU
-/** @def E1K_ITR_ENABLED
- * E1K_ITR_ENABLED reduces the number of interrupts generated by E1000 if a
- * guest driver requested it by writing non-zero value to the Interrupt
- * Throttling Register (see section 13.4.18 in "8254x Family of Gigabit
- * Ethernet Controllers Software Developer’s Manual").
- */
-//#define E1K_ITR_ENABLED
 /** @def E1K_TX_DELAY
  * E1K_TX_DELAY aims to improve guest-host transfer rate for TCP streams by
@@ -181 +174 @@
 
 #if 0
+# define LOG_ENABLED
 # define E1kLogRel(a) LogRel(a)
+# undef Log6
+# define Log6(a) LogRel(a)
 #else
 # define E1kLogRel(a) do { } while (0)
@@ -1085 +1081 @@
     /** EMT: Compute Ethernet CRC for RX packets. */
     bool        fEthernetCRC;
-
-    bool        Alignment2[3];
+    /** All: throttle interrupts. */
+    bool        fItrEnabled;
+    /** All: throttle RX interrupts. */
+    bool        fItrRxEnabled;
+
+    bool        Alignment2;
     /** Link up delay (in milliseconds). */
     uint32_t    cMsLinkUpDelay;
@@ -1865 +1865 @@
 
 /**
+ * Raise an interrupt later.
+ *
+ * @param   pThis       The device state structure.
+ */
+inline void e1kPostponeInterrupt(PE1KSTATE pThis, uint64_t uNanoseconds)
+{
+    if (!TMTimerIsActive(pThis->CTX_SUFF(pIntTimer)))
+        TMTimerSetNano(pThis->CTX_SUFF(pIntTimer), uNanoseconds);
+}
+
+/**
  * Raise interrupt if not masked.
  *
@@ -1879 +1890 @@
     if (ICR & IMS)
     {
-#if 0
-        if (pThis->fDelayInts)
-        {
-            E1K_INC_ISTAT_CNT(pThis->uStatIntDly);
-            pThis->iStatIntLostOne = 1;
-            E1kLog2(("%s e1kRaiseInterrupt: Delayed. ICR=%08x\n",
-                    pThis->szPrf, ICR));
-#define E1K_LOST_IRQ_THRSLD 20
-//#define E1K_LOST_IRQ_THRSLD 200000000
-            if (pThis->iStatIntLost >= E1K_LOST_IRQ_THRSLD)
-            {
-                E1kLog2(("%s WARNING! Disabling delayed interrupt logic: delayed=%d, delivered=%d\n",
-                        pThis->szPrf, pThis->uStatIntDly, pThis->uStatIntLate));
-                pThis->fIntMaskUsed = false;
-                pThis->uStatDisDly++;
-            }
-        }
-        else
-#endif
         if (pThis->fIntRaised)
         {
@@ -1906 +1898 @@
         else
         {
-#ifdef E1K_ITR_ENABLED
-            uint64_t tstamp = TMTimerGet(pThis->CTX_SUFF(pIntTimer));
-            /* interrupts/sec = 1 / (256 * 10E-9 * ITR) */
-            E1kLog2(("%s e1kRaiseInterrupt: tstamp - pThis->u64AckedAt = %d, ITR * 256 = %d\n",
-                        pThis->szPrf, (uint32_t)(tstamp - pThis->u64AckedAt), ITR * 256));
-            //if (!!ITR && pThis->fIntMaskUsed && tstamp - pThis->u64AckedAt < ITR * 256)
-            if (!!ITR && tstamp - pThis->u64AckedAt < ITR * 256 && !(ICR & ICR_RXT0))
+            uint64_t tsNow = TMTimerGet(pThis->CTX_SUFF(pIntTimer));
+            if (!!ITR && tsNow - pThis->u64AckedAt < ITR * 256
+                     && pThis->fItrEnabled && (pThis->fItrRxEnabled || !(ICR & ICR_RXT0)))
             {
                 E1K_INC_ISTAT_CNT(pThis->uStatIntEarly);
                 E1kLog2(("%s e1kRaiseInterrupt: Too early to raise again: %d ns < %d ns.\n",
-                        pThis->szPrf, (uint32_t)(tstamp - pThis->u64AckedAt), ITR * 256));
+                        pThis->szPrf, (uint32_t)(tsNow - pThis->u64AckedAt), ITR * 256));
+                e1kPostponeInterrupt(pThis, ITR * 256);
             }
             else
-#endif
             {
 
@@ -2885 +2873 @@
     if (RT_SUCCESS(rc))
     {
+        /* Do not return masked bits. */
+        value &= IMS;
         if (value)
         {
@@ -2961 +2951 @@
     E1kLogRel(("E1000: irq enabled, RDH=%x RDT=%x TDH=%x TDT=%x\n", RDH, RDT, TDH, TDT));
     E1kLog(("%s e1kRegWriteIMS: IRQ enabled\n", pThis->szPrf));
-    /* Mask changes, we need to raise pending interrupts. */
-    if ((ICR & IMS) && !pThis->fLocked)
-    {
-        E1kLog2(("%s e1kRegWriteIMS: IRQ pending (%08x), arming late int timer...\n",
-                 pThis->szPrf, ICR));
-        /* Raising an interrupt immediately causes win7 to hang upon NIC reconfiguration, see @bugref{5023}. */
-        TMTimerSet(pThis->CTX_SUFF(pIntTimer), TMTimerFromNano(pThis->CTX_SUFF(pIntTimer), ITR * 256) +
-                   TMTimerGet(pThis->CTX_SUFF(pIntTimer)));
-    }
+    e1kRaiseInterrupt(pThis, VINF_IOM_R3_MMIO_WRITE, 0);
 
     return VINF_SUCCESS;
@@ -7473 +7455 @@
     if (!CFGMR3AreValuesValid(pCfg, "MAC\0" "CableConnected\0" "AdapterType\0"
                                     "LineSpeed\0" "GCEnabled\0" "R0Enabled\0"
+                                    "ItrEnabled\0" "ItrRxEnabled\0"
                                     "EthernetCRC\0" "GSOEnabled\0" "LinkUpDelay\0"))
         return PDMDEV_SET_ERROR(pDevIns, VERR_PDM_DEVINS_UNKNOWN_CFG_VALUES,
@@ -7479 +7462 @@
     /** @todo: LineSpeed unused! */
 
-    pThis->fR0Enabled   = true;
-    pThis->fRCEnabled   = true;
-    pThis->fEthernetCRC = true;
-    pThis->fGSOEnabled  = true;
+    pThis->fR0Enabled    = true;
+    pThis->fRCEnabled    = true;
+    pThis->fEthernetCRC  = true;
+    pThis->fGSOEnabled   = true;
+    pThis->fItrEnabled   = true;
+    pThis->fItrRxEnabled = true;
 
     /* Get config params */
@@ -7518 +7503 @@
                                 N_("Configuration error: Failed to get the value of 'GSOEnabled'"));
 
+    rc = CFGMR3QueryBoolDef(pCfg, "ItrEnabled", &pThis->fItrEnabled, true);
+    if (RT_FAILURE(rc))
+        return PDMDEV_SET_ERROR(pDevIns, rc,
+                                N_("Configuration error: Failed to get the value of 'ItrEnabled'"));
+
+    rc = CFGMR3QueryBoolDef(pCfg, "ItrRxEnabled", &pThis->fItrRxEnabled, true);
+    if (RT_FAILURE(rc))
+        return PDMDEV_SET_ERROR(pDevIns, rc,
+                                N_("Configuration error: Failed to get the value of 'ItrRxEnabled'"));
+
     rc = CFGMR3QueryU32Def(pCfg, "LinkUpDelay", (uint32_t*)&pThis->cMsLinkUpDelay, 5000); /* ms */
     if (RT_FAILURE(rc))
@@ -7528 +7523 @@
         LogRel(("%s WARNING! Link up delay is disabled!\n", pThis->szPrf));
 
-    E1kLog(("%s Chip=%s LinkUpDelay=%ums EthernetCRC=%s GSO=%s R0=%s GC=%s\n", pThis->szPrf,
+    LogRel(("%s Chip=%s LinkUpDelay=%ums EthernetCRC=%s GSO=%s Itr=%s ItrRx=%s R0=%s GC=%s\n", pThis->szPrf,
             g_Chips[pThis->eChip].pcszName, pThis->cMsLinkUpDelay,
             pThis->fEthernetCRC ? "on" : "off",
             pThis->fGSOEnabled ? "enabled" : "disabled",
+            pThis->fItrEnabled ? "enabled" : "disabled",
+            pThis->fItrRxEnabled ? "enabled" : "disabled",
             pThis->fR0Enabled ? "enabled" : "disabled",
             pThis->fRCEnabled ? "enabled" : "disabled"));