Changeset 107924 in vbox for trunk/src/VBox/Devices

Timestamp:

Jan 23, 2025 4:42:56 PM (2 weeks ago)

Author:

vboxsync

Message:

DevXHCI: Removed some Parfait warnings.

File:

• trunk/src/VBox/Devices/USB/DevXHCI.cpp (modified) (35 diffs)

trunk/src/VBox/Devices/USB/DevXHCI.cpp

@@ -315 +315 @@
 
 /** Convert a zero-based index to a 1-based ID. */
-#define IDX_TO_ID(a)    (a + 1)
+#define IDX_TO_ID(a)    (uint8_t)(a + 1)
 /** Convert a 1-based ID to a zero-based index. */
-#define ID_TO_IDX(a)    (a - 1)
+#define ID_TO_IDX(a)    (uint8_t)(a - 1)
 
 /** PCI device related constants. */
@@ -434 +434 @@
 /** @name xHCI Extended capability types.
  * @{ */
-#define XHCI_XCP_USB_LEGACY     1   /**< USB legacy support. */
-#define XHCI_XCP_PROTOCOL       2   /**< Protocols supported by ports. */
-#define XHCI_XCP_EXT_PM         3   /**< Extended power management (non-PCI). */
-#define XHCI_XCP_IOVIRT         4   /**< Hardware xHCI virtualization support. */
-#define XHCI_XCP_MSI            5   /**< Message interrupts (non-PCI). */
-#define XHCI_XCP_LOCAL_MEM      6   /**< Local memory (for debug support). */
-#define XHCI_XCP_USB_DEBUG      10  /**< USB debug capability. */
-#define XHCI_XCP_EXT_MSI        17  /**< MSI-X (non-PCI). */
+#define XHCI_XCP_USB_LEGACY     1u  /**< USB legacy support. */
+#define XHCI_XCP_PROTOCOL       2u  /**< Protocols supported by ports. */
+#define XHCI_XCP_EXT_PM         3u  /**< Extended power management (non-PCI). */
+#define XHCI_XCP_IOVIRT         4u  /**< Hardware xHCI virtualization support. */
+#define XHCI_XCP_MSI            5u  /**< Message interrupts (non-PCI). */
+#define XHCI_XCP_LOCAL_MEM      6u  /**< Local memory (for debug support). */
+#define XHCI_XCP_USB_DEBUG      10u /**< USB debug capability. */
+#define XHCI_XCP_EXT_MSI        17u /**< MSI-X (non-PCI). */
 /** @} */
 
@@ -614 +614 @@
 /** @name Event Ring Deqeue Pointer Register (ERDP) bits
  * @{ */
-#define XHCI_ERDP_DESI_MASK     0x00000007  /**< RW   - Dequeue ERST Segment Index */
-#define XHCI_ERDP_EHB           RT_BIT(3)   /**< RW1C - Event Handler Busy */
+#define XHCI_ERDP_DESI_MASK     UINT64_C(7)     /**< RW   - Dequeue ERST Segment Index */
+#define XHCI_ERDP_EHB           RT_BIT_64(3)    /**< RW1C - Event Handler Busy */
 #define XHCI_ERDP_ADDR_MASK     UINT64_C(0xFFFFFFFFFFFFFFF0)    /**< RW - ERDP address mask */
 /** @} */
@@ -1891 +1891 @@
 
 /** Query the number of configured USB2 ports. */
-#define XHCI_NDP_USB2(a_pThisCC)        ((unsigned)(a_pThisCC)->RootHub2.cPortsImpl)
+#define XHCI_NDP_USB2(a_pThisCC)        ((a_pThisCC)->RootHub2.cPortsImpl)
 
 /** Query the number of configured USB3 ports. */
-#define XHCI_NDP_USB3(a_pThisCC)        ((unsigned)(a_pThisCC)->RootHub3.cPortsImpl)
+#define XHCI_NDP_USB3(a_pThisCC)        ((a_pThisCC)->RootHub3.cPortsImpl)
 
 /** Query the total number of configured ports. */
@@ -1910 +1910 @@
 
 /** Build a Protocol extended capability. */
-static uint32_t xhciR3BuildProtocolCaps(uint8_t *pbCap, uint32_t cbMax, int cPorts, int nPortOfs, int ver)
+static uint32_t xhciR3BuildProtocolCaps(uint8_t *pbCap, uint32_t cbMax, unsigned cPorts, unsigned nPortOfs, int ver)
 {
     uint32_t    *pu32Cap = (uint32_t *)pbCap;
@@ -2815 +2815 @@
 {
     XHCI_CTX_XFER_COMPLETE  *pCtx = (XHCI_CTX_XFER_COMPLETE *)pvContext;
-    int                     rc;
     unsigned                uXferLen;
     unsigned                uResidue;
@@ -2874 +2873 @@
         if (pXferTRB->norm.ioc || (pXferTRB->norm.isp && uResidue))
         {
-            rc = xhciR3PostXferEvent(pDevIns, pThis, pXferTRB->norm.int_tgt, uResidue, cc,
-                                     pCtx->uSlotID, pCtx->uEpDCI, GCPhysXfrTRB, pXferTRB->norm.bei, false);
+            xhciR3PostXferEvent(pDevIns, pThis, pXferTRB->norm.int_tgt, uResidue, cc,
+                                pCtx->uSlotID, pCtx->uEpDCI, GCPhysXfrTRB, pXferTRB->norm.bei, false);
         }
         break;
@@ -2881 +2880 @@
         if (pXferTRB->evtd.ioc)
         {
-            rc = xhciR3PostXferEvent(pDevIns, pThis, pXferTRB->evtd.int_tgt, pCtx->uEDTLA, pCtx->uLastCC,
-                                     pCtx->uSlotID, pCtx->uEpDCI, pXferTRB->evtd.evt_data, pXferTRB->evtd.bei, true);
+            xhciR3PostXferEvent(pDevIns, pThis, pXferTRB->evtd.int_tgt, pCtx->uEDTLA, pCtx->uLastCC,
+                                pCtx->uSlotID, pCtx->uEpDCI, pXferTRB->evtd.evt_data, pXferTRB->evtd.bei, true);
         }
         /* Clear the EDTLA. */
@@ -2931 +2930 @@
     bool            fInFlight;
     bool            fContinue = true;
-    int             rc;
     unsigned        cTrbs = 0;
 
@@ -2974 +2972 @@
                     pEpCtx->trep = pEpCtx->trdp;
                 if (xfer.link.ioc)
-                    rc = xhciR3PostXferEvent(pDevIns, pThis, xfer.link.int_tgt, 0, XHCI_TCC_SUCCESS, uSlotID, uEpDCI,
-                                             GCPhysXfrTRB, false, false);
+                    xhciR3PostXferEvent(pDevIns, pThis, xfer.link.int_tgt, 0, XHCI_TCC_SUCCESS, uSlotID, uEpDCI,
+                                        GCPhysXfrTRB, false, false);
                 break;
             case XHCI_TRB_NOOP_XFER:
@@ -2986 +2984 @@
                     pEpCtx->trep += sizeof(XHCI_XFER_TRB);
                 if (xfer.nop.ioc)
-                    rc = xhciR3PostXferEvent(pDevIns, pThis, xfer.nop.int_tgt, 0, XHCI_TCC_SUCCESS, uSlotID, uEpDCI,
-                                             GCPhysXfrTRB, false, false);
+                    xhciR3PostXferEvent(pDevIns, pThis, xfer.nop.int_tgt, 0, XHCI_TCC_SUCCESS, uSlotID, uEpDCI,
+                                        GCPhysXfrTRB, false, false);
                 break;
             default:
@@ -3011 +3009 @@
             /* Get out of the loop. */
             fContinue = false;
-            rc = VERR_NOT_SUPPORTED;    /* No good error code really... */
         }
     } while (fContinue);
@@ -3043 +3040 @@
     XHCI_XFER_TRB   xfer;
     RTGCPHYS        GCPhysXfrTRB;
-    int             rc;
     unsigned        uResidue = 0;
     uint8_t         uSlotID  = pUrb->pHci->uSlotID;
@@ -3120 +3116 @@
             ep_ctx.ep_state = XHCI_EPST_HALTED;
             cc = XHCI_TCC_STALL;
-            rc = xhciR3PostXferEvent(pDevIns, pThis, xfer.gen.int_tgt, uResidue, cc,
-                                     uSlotID, uEpDCI, GCPhysXfrTRB, false, false);
+            xhciR3PostXferEvent(pDevIns, pThis, xfer.gen.int_tgt, uResidue, cc,
+                                uSlotID, uEpDCI, GCPhysXfrTRB, false, false);
             break;
         case VUSBSTATUS_DNR:
@@ -3129 +3125 @@
             ep_ctx.ep_state = XHCI_EPST_HALTED;
             cc = XHCI_TCC_USB_XACT_ERR;
-            rc = xhciR3PostXferEvent(pDevIns, pThis, xfer.gen.int_tgt, uResidue, cc,
-                                     uSlotID, uEpDCI, GCPhysXfrTRB, false, false);
+            xhciR3PostXferEvent(pDevIns, pThis, xfer.gen.int_tgt, uResidue, cc,
+                                uSlotID, uEpDCI, GCPhysXfrTRB, false, false);
             break;
         case VUSBSTATUS_CRC:    /// @todo Separate status for canceling?!
             ep_ctx.ep_state = XHCI_EPST_HALTED;
             cc = XHCI_TCC_USB_XACT_ERR;
-            rc = xhciR3PostXferEvent(pDevIns, pThis, xfer.gen.int_tgt, uResidue, cc,
-                                     uSlotID, uEpDCI, GCPhysXfrTRB, false, false);
+            xhciR3PostXferEvent(pDevIns, pThis, xfer.gen.int_tgt, uResidue, cc,
+                                uSlotID, uEpDCI, GCPhysXfrTRB, false, false);
 
             /* NB: The TRDP is *not* advanced and TREP is reset. */
@@ -3148 +3144 @@
             ep_ctx.ep_state = XHCI_EPST_HALTED;
             cc = XHCI_TCC_DATA_BUF_ERR;
-            rc = xhciR3PostXferEvent(pDevIns, pThis, xfer.gen.int_tgt, uResidue, cc,
-                                     uSlotID, uEpDCI, GCPhysXfrTRB, false, false);
+            xhciR3PostXferEvent(pDevIns, pThis, xfer.gen.int_tgt, uResidue, cc,
+                                uSlotID, uEpDCI, GCPhysXfrTRB, false, false);
             break;
         default:
@@ -3493 +3489 @@
     uint64_t                uTREP;
     PVUSBURB                pUrb;
-    unsigned                cIsoPackets;
+    uint8_t                 cIsoPackets;
     uint32_t                cbPktMax;
 
@@ -3806 +3802 @@
     bool            dcs;
     bool            fContinue = true;
-    int             rc;
+    int             rc = VINF_SUCCESS;
     unsigned        cTrbs = 0;
 
@@ -4014 +4010 @@
         xhciR3WriteBackEp(pDevIns, pThis, uSlotID, uDBTarget, &ep_ctx);
     }
-    return VINF_SUCCESS;
+    return rc;
 }
 
@@ -5048 +5044 @@
         /* Reset the given endpoint. */
         Log(("Reset Endpoint: slot ID %u, EP ID %u, TSP=%u\n", pCmd->rse.slot_id, pCmd->rse.ep_id, pCmd->rse.tsp));
-        cc = XHCI_TCC_SUCCESS;
         slot = ID_TO_IDX(pCmd->rse.slot_id);
         if ((slot >= RT_ELEMENTS(pThis->aSlotState)) || (pThis->aSlotState[slot] == XHCI_DEVSLOT_EMPTY))
@@ -5061 +5056 @@
         /* Stop the given endpoint. */
         Log(("Stop Endpoint: slot ID %u, EP ID %u, SP=%u\n", pCmd->stp.slot_id, pCmd->stp.ep_id, pCmd->stp.sp));
-        cc = XHCI_TCC_SUCCESS;
         slot = ID_TO_IDX(pCmd->stp.slot_id);
         if ((slot >= RT_ELEMENTS(pThis->aSlotState)) || (pThis->aSlotState[slot] == XHCI_DEVSLOT_EMPTY))
@@ -5074 +5068 @@
         /* Set TR Dequeue Pointer. */
         Log(("Set TRDP: slot ID %u, EP ID %u, TRDP=%RX64\n", pCmd->stdp.slot_id, pCmd->stdp.ep_id, pCmd->stdp.tr_dqp));
-        cc = XHCI_TCC_SUCCESS;
         slot = ID_TO_IDX(pCmd->stdp.slot_id);
         if ((slot >= RT_ELEMENTS(pThis->aSlotState)) || (pThis->aSlotState[slot] == XHCI_DEVSLOT_EMPTY))
@@ -5087 +5080 @@
         /* Reset a device. */
         Log(("Reset Device: slot ID %u\n", pCmd->rsd.slot_id));
-        cc = XHCI_TCC_SUCCESS;
         slot = ID_TO_IDX(pCmd->rsd.slot_id);
         if ((slot >= RT_ELEMENTS(pThis->aSlotState)) || (pThis->aSlotState[slot] == XHCI_DEVSLOT_EMPTY))
@@ -5100 +5092 @@
         /* Get port bandwidth. */
         Log(("Get Port Bandwidth: Dev Speed %u, Hub Slot ID %u, Context=%RX64\n", pCmd->gpbw.spd, pCmd->gpbw.slot_id, pCmd->gpbw.pbctx_ptr));
-        cc = XHCI_TCC_SUCCESS;
         if (pCmd->gpbw.slot_id)
             cc = XHCI_TCC_PARM_ERR; /* Potential undefined behavior, see 4.6.15. */
@@ -5559 +5550 @@
  * @returns true if device was connected and the flag was cleared.
  */
-static bool xhciR3RhPortSetIfConnected(PXHCI pThis, int iPort, uint32_t fValue)
+static bool xhciR3RhPortSetIfConnected(PXHCI pThis, unsigned iPort, uint32_t fValue)
 {
     /*
@@ -5948 +5939 @@
         Log(("Unknown USBCMD bits %#x are set!\n", val & ~XHCI_CMD_MASK));
 
-    uint32_t old_cmd = pThis->cmd;
+
+    /* First deal with resets. These must be done in R3 and will initialize
+     * the USBCMD register.
+     */
+    if (val & (XHCI_CMD_HCRST | XHCI_CMD_LCRST))
+    {
 #ifdef IN_RING3
-    pThis->cmd = val;
-#endif
-
-    if (val & XHCI_CMD_HCRST)
-    {
-#ifdef IN_RING3
-        LogRel(("xHCI: Hardware reset\n"));
-        xhciR3DoReset(pThis, pThisCC, XHCI_USB_RESET, true /* reset devices */);
+        /* NB: xhciR3DoReset() overwrites pThis->cmd */
+        if (val & XHCI_CMD_HCRST)
+        {
+            LogRel(("xHCI: Hardware reset\n"));
+            xhciR3DoReset(pThis, pThisCC, XHCI_USB_RESET, true /* reset devices */);
+        }
+        else if (val & XHCI_CMD_LCRST)
+        {
+            LogRel(("xHCI: Software reset\n"));
+            xhciR3DoReset(pThis, pThisCC, XHCI_USB_SUSPEND, false /* N/A */);
+        }
+        else
+            Assert(0);
+
+        return VINF_SUCCESS;
 #else
         return VINF_IOM_R3_MMIO_WRITE;
 #endif
     }
-    else if (val & XHCI_CMD_LCRST)
-    {
-#ifdef IN_RING3
-        LogRel(("xHCI: Software reset\n"));
-        xhciR3DoReset(pThis, pThisCC, XHCI_USB_SUSPEND, false /* N/A */);
-#else
-        return VINF_IOM_R3_MMIO_WRITE;
-#endif
-    }
-    else if (pThis->status & XHCI_STATUS_HCE)
-    {
-        /* If HCE is set, don't restart the controller. Only a reset
-         * will clear the HCE bit.
+
+    /* Not resetting, handle the remaining bits. */
+    if (pThis->status & XHCI_STATUS_HCE)
+    {
+        /* If the HCE (HC Error) status bit is set, don't do anything.
+         * Only a reset will clear the HCE bit.
          */
         Log(("xHCI: HCE bit set, ignoring USBCMD register changes!\n"));
-        pThis->cmd = old_cmd;
         return VINF_SUCCESS;
     }
     else
     {
-        /* See what changed and take action on that. First the R/S bit. */
-        uint32_t old_state = old_cmd & XHCI_CMD_RS;
-        uint32_t new_state = val     & XHCI_CMD_RS;
+        /* See what changed and take action on that. First the R/S bit.
+         * Note that R/S changes must also be done in R3, so we get them
+         * out of the way first.
+         * NB: xhciR3BusStop() modifies USBCMD.
+         */
+        uint32_t old_state = pThis->cmd & XHCI_CMD_RS;
+        uint32_t new_state = val        & XHCI_CMD_RS;
 
         if (old_state != new_state)
@@ -6006 +6005 @@
 
         /* Check EWE (Enable MFINDEX Wraparound Event) changes. */
-        old_state = old_cmd & XHCI_CMD_EWE;
-        new_state = val     & XHCI_CMD_EWE;
+        old_state = pThis->cmd & XHCI_CMD_EWE;
+        new_state = val        & XHCI_CMD_EWE;
 
         if (old_state != new_state)
@@ -6025 +6024 @@
 
         /* INTE transitions need to twiddle interrupts. */
-        old_state = old_cmd & XHCI_CMD_INTE;
-        new_state = val     & XHCI_CMD_INTE;
+        old_state = pThis->cmd & XHCI_CMD_INTE;
+        new_state = val        & XHCI_CMD_INTE;
         if (old_state != new_state)
         {
@@ -6061 +6060 @@
         }
     }
-#ifndef IN_RING3
+
+    /* Finally update the USBCMD register. */
     pThis->cmd = val;
-#endif
+
     return VINF_SUCCESS;
 }
@@ -7889 +7889 @@
  * Worker for xhciR3Construct that registers a LUN (USB root hub).
  */
-static int xhciR3RegisterHub(PPDMDEVINS pDevIns, PXHCIROOTHUBR3 pRh, int iLun, const char *pszDesc)
+static int xhciR3RegisterHub(PPDMDEVINS pDevIns, PXHCIROOTHUBR3 pRh, uint32_t iLun, const char *pszDesc)
 {
     int rc = PDMDevHlpDriverAttach(pDevIns, iLun, &pRh->IBase, &pRh->pIBase, pszDesc);
-    AssertMsgRCReturn(rc, ("Configuration error: Failed to attach root hub driver to LUN #%d! (%Rrc)\n", iLun, rc), rc);
+    AssertMsgRCReturn(rc, ("Configuration error: Failed to attach root hub driver to LUN #%u! (%Rrc)\n", iLun, rc), rc);
 
     pRh->pIRhConn = PDMIBASE_QUERY_INTERFACE(pRh->pIBase, VUSBIROOTHUBCONNECTOR);
@@ -7917 +7917 @@
     PXHCICC         pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PXHCICC);
     PCPDMDEVHLPR3   pHlp    = pDevIns->pHlpR3;
-    uint32_t        cUsb2Ports;
-    uint32_t        cUsb3Ports;
+    uint8_t         cUsb2Ports;
+    uint8_t         cUsb3Ports;
     int             rc;
     LogFlow(("xhciR3Construct:\n"));
@@ -7934 +7934 @@
 
     /* Number of USB2 ports option. */
-    rc = pHlp->pfnCFGMQueryU32Def(pCfg, "USB2Ports", &cUsb2Ports, XHCI_NDP_20_DEFAULT);
+    rc = pHlp->pfnCFGMQueryU8Def(pCfg, "USB2Ports", &cUsb2Ports, XHCI_NDP_20_DEFAULT);
     if (RT_FAILURE(rc))
         return PDMDEV_SET_ERROR(pDevIns, rc,
@@ -7945 +7945 @@
 
     /* Number of USB3 ports option. */
-    rc = pHlp->pfnCFGMQueryU32Def(pCfg, "USB3Ports", &cUsb3Ports, XHCI_NDP_30_DEFAULT);
+    rc = pHlp->pfnCFGMQueryU8Def(pCfg, "USB3Ports", &cUsb3Ports, XHCI_NDP_30_DEFAULT);
     if (RT_FAILURE(rc))
         return PDMDEV_SET_ERROR(pDevIns, rc,
@@ -8039 +8039 @@
 
     /* Set up the USB2 root hub interface. */
-    pThis->cUsb2Ports                              = (uint8_t)cUsb2Ports;
+    pThis->cUsb2Ports                              = cUsb2Ports;
     pThisCC->RootHub2.pXhciR3                      = pThisCC;
     pThisCC->RootHub2.cPortsImpl                   = cUsb2Ports;
@@ -8053 +8053 @@
 
     /* Now the USB3 root hub interface. */
-    pThis->cUsb3Ports                              = (uint8_t)cUsb3Ports;
+    pThis->cUsb3Ports                              = cUsb3Ports;
     pThisCC->RootHub3.pXhciR3                      = pThisCC;
     pThisCC->RootHub3.cPortsImpl                   = cUsb3Ports;