Changeset 90993 in vbox

Timestamp:

Aug 30, 2021 10:39:02 AM (3 years ago)

Author:

vboxsync

Message:

UsbMouse: Reworked URB processing to make it simpler and more uniform. Now lines up with UsbKbd.

File:

• trunk/src/VBox/Devices/Input/UsbMouse.cpp (modified) (19 diffs)

trunk/src/VBox/Devices/Input/UsbMouse.cpp

@@ -300 +300 @@
     uint16_t    y;
 } USBHIDMT_REPORT_POINTER;
+
+typedef union USBHIDALL_REPORT
+{
+    USBHIDM_REPORT          m;
+    USBHIDT_REPORT          t;
+    USBHIDMT_REPORT         mt;
+    USBHIDMT_REPORT_POINTER mp;
+} USBHIDALL_REPORT, *PUSBHIDALL_REPORT;
+
 #pragma pack()
 
@@ -1135 +1144 @@
 
 /**
- * Completes the URB with a OK state.
- */
-static int usbHidCompleteOk(PUSBHID pThis, PVUSBURB pUrb, size_t cbData)
-{
-    LogRelFlow(("usbHidCompleteOk/#%u: pUrb=%p:%s cbData=%#zx\n",
-                pThis->pUsbIns->iInstance, pUrb, pUrb->pszDesc, cbData));
+ * Completes the URB after device successfully processed it. Optionally copies data
+ * into the URB. May still generate an error if the URB is not big enough.
+ */
+static int usbHidCompleteOk(PUSBHID pThis, PVUSBURB pUrb, const void *pSrc, size_t cbSrc)
+{
+    Log(("usbHidCompleteOk/#%u: pUrb=%p:%s (cbData=%#x) cbSrc=%#zx\n", pThis->pUsbIns->iInstance, pUrb, pUrb->pszDesc, pUrb->cbData, cbSrc));
 
     pUrb->enmStatus = VUSBSTATUS_OK;
-    pUrb->cbData    = (uint32_t)cbData;
+    size_t  cbCopy  = 0;
+    size_t  cbSetup = 0;
+
+    if (pSrc)   /* Can be NULL if not copying anything. */
+    {
+        Assert(cbSrc);
+        uint8_t *pDst = pUrb->abData;
+
+        /* Returned data is written after the setup message in control URBs. */
+        if (pUrb->enmType == VUSBXFERTYPE_MSG)
+            cbSetup = sizeof(VUSBSETUP);
+
+        Assert(pUrb->cbData >= cbSetup);    /* Only triggers if URB is corrupted. */
+
+        if (pUrb->cbData > cbSetup)
+        {
+            /* There is at least one byte of room in the URB. */
+            cbCopy = RT_MIN(pUrb->cbData - cbSetup, cbSrc);
+            memcpy(pDst + cbSetup, pSrc, cbCopy);
+            pUrb->cbData = (uint32_t)(cbCopy + cbSetup);
+            Log(("Copied %zu bytes to pUrb->abData[%zu], source had %zu bytes\n", cbCopy, cbSetup, cbSrc));
+        }
+
+        /* Need to check length differences. If cbSrc is less than what
+         * the URB has space for, it'll be resolved as a short packet. But
+         * if cbSrc is bigger, there is a real problem and the host needs
+         * to see an overrun/babble error.
+         */
+        if (RT_UNLIKELY(cbSrc > cbCopy))
+            pUrb->enmStatus = VUSBSTATUS_DATA_OVERRUN;
+    }
+    else
+        Assert(cbSrc == 0); /* Make up your mind, caller! */
 
     usbHidLinkDone(pThis, pUrb);
@@ -1193 +1234 @@
 
     if (pUrb)
-        return usbHidCompleteOk(pThis, pUrb, 0);
+        return usbHidCompleteOk(pThis, pUrb, NULL, 0);
     return VINF_SUCCESS;
 }
@@ -1341 +1382 @@
 
     /* Report current state. */
-    USBHIDMT_REPORT *p = (USBHIDMT_REPORT *)&pUrb->abData[0];
+    USBHIDMT_REPORT r;
+    USBHIDMT_REPORT *p = &r;
     RT_ZERO(*p);
 
@@ -1399 +1441 @@
 
     LogRel3(("usbHid: reporting touch contact:\n%.*Rhxd\n", sizeof(USBHIDMT_REPORT), p));
-    return usbHidCompleteOk(pThis, pUrb, sizeof(USBHIDMT_REPORT));
+    return usbHidCompleteOk(pThis, pUrb, p, sizeof(USBHIDMT_REPORT));
 }
 
@@ -1419 +1461 @@
     if (pUrb)
     {
-        PUSBHIDTM_REPORT    pReport = (PUSBHIDTM_REPORT)&pUrb->abData[0];
+        USBHIDTM_REPORT     report;
+        PUSBHIDTM_REPORT    pReport = &report;
         size_t              cbCopy;
 
         cbCopy = usbHidFillReport(pReport, &pThis->PtrDelta, pThis->enmMode);
         pThis->fHasPendingChanges = false;
-        return usbHidCompleteOk(pThis, pUrb, cbCopy);
+        return usbHidCompleteOk(pThis, pUrb, pReport, cbCopy);
     }
     else
@@ -1764 +1807 @@
             AssertFailed();
             LogRelFlow(("usbHidHandleIntrDevToHost: Entering STATUS\n"));
-            return usbHidCompleteOk(pThis, pUrb, 0);
+            return usbHidCompleteOk(pThis, pUrb, NULL, 0);
         }
 
@@ -1775 +1818 @@
             LogRelFlow(("usbHidHandleIntrDevToHost: Entering READY\n"));
             pThis->enmState = USBHIDREQSTATE_READY;
-            return usbHidCompleteOk(pThis, pUrb, 0);
+            return usbHidCompleteOk(pThis, pUrb, NULL, 0);
         }
 
@@ -1804 +1847 @@
 #define SET_PROTOCOL 0x0B
 
-static uint8_t const g_abQASampleBlob[256] =
-{
+static uint8_t const g_abQASampleBlob[256 + 1] =
+{
+    REPORTID_TOUCH_QABLOB,  /* Report Id. */
     0xfc, 0x28, 0xfe, 0x84, 0x40, 0xcb, 0x9a, 0x87,
     0x0d, 0xbe, 0x57, 0x3c, 0xb6, 0x70, 0x09, 0x88,
@@ -1867 +1911 @@
             if (pSetup->bRequest == GET_REPORT)
             {
-                uint32_t cbData = 0; /* 0 means that the report is unsupported. */
+                uint8_t     abData[sizeof(USBHIDALL_REPORT)];
+                uint8_t     *pData = (uint8_t *)&abData;
+                uint32_t    cbData = 0; /* 0 means that the report is unsupported. */
 
                 if (u8ReportType == 1 && u8ReportID == REPORTID_TOUCH_POINTER)
                 {
-                    USBHIDMT_REPORT_POINTER *p = (USBHIDMT_REPORT_POINTER *)&pUrb->abData[sizeof(VUSBSETUP)];
+                    USBHIDMT_REPORT_POINTER *p = (USBHIDMT_REPORT_POINTER *)&abData;
                     /* The actual state should be reported here. */
                     p->idReport = REPORTID_TOUCH_POINTER;
@@ -1881 +1927 @@
                 else if (u8ReportType == 1 && u8ReportID == REPORTID_TOUCH_EVENT)
                 {
-                    USBHIDMT_REPORT *p = (USBHIDMT_REPORT *)&pUrb->abData[sizeof(VUSBSETUP)];
+                    USBHIDMT_REPORT *p = (USBHIDMT_REPORT *)&abData;
                     /* The actual state should be reported here. */
                     RT_ZERO(*p);
@@ -1889 +1935 @@
                 else if (u8ReportType == 3 && u8ReportID == REPORTID_TOUCH_MAX_COUNT)
                 {
-                    pUrb->abData[sizeof(VUSBSETUP) + 0] = REPORTID_TOUCH_MAX_COUNT;
-                    pUrb->abData[sizeof(VUSBSETUP) + 1] = MT_CONTACT_MAX_COUNT; /* Contact count maximum. */
-                    pUrb->abData[sizeof(VUSBSETUP) + 2] = 0;  /* Device identifier */
+                    abData[0] = REPORTID_TOUCH_MAX_COUNT;
+                    abData[1] = MT_CONTACT_MAX_COUNT;   /* Contact count maximum. */
+                    abData[2] = 0;                      /* Device identifier */
                     cbData = 3;
                 }
                 else if (u8ReportType == 3 && u8ReportID == REPORTID_TOUCH_QABLOB)
                 {
-                    pUrb->abData[sizeof(VUSBSETUP) + 0] = REPORTID_TOUCH_QABLOB;  /* Report Id. */
-                    memcpy(&pUrb->abData[sizeof(VUSBSETUP) + 1],
-                           g_abQASampleBlob, sizeof(g_abQASampleBlob));
-                    cbData = sizeof(g_abQASampleBlob) + 1;
+                    pData  = (uint8_t *)&g_abQASampleBlob;
+                    cbData = sizeof(g_abQASampleBlob);
                 }
                 else if (u8ReportType == 3 && u8ReportID == REPORTID_TOUCH_DEVCONFIG)
                 {
-                    pUrb->abData[sizeof(VUSBSETUP) + 0] = REPORTID_TOUCH_DEVCONFIG;
-                    pUrb->abData[sizeof(VUSBSETUP) + 1] = 2;  /* Device mode:
-                                                               * "HID touch device supporting contact
-                                                               * identifier and contact count maximum."
-                                                               */
-                    pUrb->abData[sizeof(VUSBSETUP) + 2] = 0;  /* Device identifier */
+                    abData[0] = REPORTID_TOUCH_DEVCONFIG;
+                    abData[1] = 2;  /* Device mode:
+                                     * "HID touch device supporting contact
+                                     * identifier and contact count maximum."
+                                     */
+                    abData[2] = 0;  /* Device identifier */
                     cbData = 3;
                 }
@@ -1914 +1958 @@
                 if (cbData > 0)
                 {
-                    rc = usbHidCompleteOk(pThis, pUrb, sizeof(VUSBSETUP) + cbData);
+                    rc = usbHidCompleteOk(pThis, pUrb, pData, cbData);
                 }
                 else
@@ -1924 +1968 @@
             {
                 /* SET_REPORT */
-                rc = usbHidCompleteOk(pThis, pUrb, pUrb->cbData);
+                rc = usbHidCompleteOk(pThis, pUrb, NULL, 0);
             }
         } break;
@@ -2001 +2045 @@
                                 }
                                 /* Returned data is written after the setup message. */
-                                cbCopy = pUrb->cbData - sizeof(*pSetup);
-                                cbCopy = RT_MIN(cbCopy, cbDesc);
+                                cbCopy = RT_MIN(pSetup->wValue, cbDesc);
                                 LogRelFlow(("usbHidMouse: GET_DESCRIPTOR DT_IF_HID_DESCRIPTOR wValue=%#x wIndex=%#x cbCopy=%#x\n",
                                             pSetup->wValue, pSetup->wIndex,
                                             cbCopy));
-                                memcpy(&pUrb->abData[sizeof(*pSetup)], pDesc, cbCopy);
-                                return usbHidCompleteOk(pThis, pUrb, cbCopy + sizeof(*pSetup));
+                                return usbHidCompleteOk(pThis, pUrb, pDesc, cbCopy);
                             }
 
@@ -2032 +2074 @@
                                 }
                                 /* Returned data is written after the setup message. */
-                                cbCopy = pUrb->cbData - sizeof(*pSetup);
-                                cbCopy = RT_MIN(cbCopy, cbDesc);
+                                cbCopy = RT_MIN(pSetup->wLength, cbDesc);
                                 LogRelFlow(("usbHid: GET_DESCRIPTOR DT_IF_HID_REPORT wValue=%#x wIndex=%#x cbCopy=%#x\n",
                                             pSetup->wValue, pSetup->wIndex,
                                             cbCopy));
-                                memcpy(&pUrb->abData[sizeof(*pSetup)], pDesc, cbCopy);
-                                return usbHidCompleteOk(pThis, pUrb, cbCopy + sizeof(*pSetup));
+                                return usbHidCompleteOk(pThis, pUrb, pDesc, cbCopy);
                             }
 
@@ -2075 +2115 @@
                         LogRelFlow(("usbHid: GET_STATUS (device)\n"));
                         wRet = 0;   /* Not self-powered, no remote wakeup. */
-                        memcpy(&pUrb->abData[sizeof(*pSetup)], &wRet, sizeof(wRet));
-                        return usbHidCompleteOk(pThis, pUrb, sizeof(wRet) + sizeof(*pSetup));
+                        return usbHidCompleteOk(pThis, pUrb, &wRet, sizeof(wRet));
                     }
 
@@ -2083 +2122 @@
                         if (pSetup->wIndex == 0)
                         {
-                            memcpy(&pUrb->abData[sizeof(*pSetup)], &wRet, sizeof(wRet));
-                            return usbHidCompleteOk(pThis, pUrb, sizeof(wRet) + sizeof(*pSetup));
+                            return usbHidCompleteOk(pThis, pUrb, &wRet, sizeof(wRet));
                         }
                         LogRelFlow(("usbHid: GET_STATUS (interface) invalid, wIndex=%#x\n", pSetup->wIndex));
@@ -2095 +2133 @@
                         {
                             wRet = pThis->aEps[pSetup->wIndex].fHalted ? 1 : 0;
-                            memcpy(&pUrb->abData[sizeof(*pSetup)], &wRet, sizeof(wRet));
-                            return usbHidCompleteOk(pThis, pUrb, sizeof(wRet) + sizeof(*pSetup));
+                            return usbHidCompleteOk(pThis, pUrb, &wRet, sizeof(wRet));
                         }
                         LogRelFlow(("usbHid: GET_STATUS (endpoint) invalid, wIndex=%#x\n", pSetup->wIndex));