[PATCH] hw/usb/core: fix inconsistent ep and pid (UBS_TOKEN_SETUP)

[PATCH] hw/usb/core: fix inconsistent ep and pid (UBS_TOKEN_SETUP)

[Qiang Liu]

Sometimes, `ep` is not enforced to 0 when `pid` is `USB_TOKEN_SETUP`. I
trigger this through dwc2
(https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg07179.html).
A similar bug was found
[here](https://gitlab.com/qemu-project/qemu/-/issues/119) and
[here](https://gitlab.com/qemu-project/qemu/-/issues/303).

Fixes: 25d5de7d81a5 ("usb: link packets to endpoints not devices")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/119
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/303
Cc: Paul Zimmerman <pauldzim@gmail.com>
Signed-off-by: Qiang Liu <cyruscyliu@gmail.com>
---
 hw/usb/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/core.c b/hw/usb/core.c
index 975f76250a..eb50a7bff8 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -738,7 +738,7 @@ struct USBEndpoint *usb_ep_get(USBDevice *dev, int pid, int ep)
     struct USBEndpoint *eps;
 
     assert(dev != NULL);
-    if (ep == 0) {
+    if (ep == 0 || pid == USB_TOKEN_SETUP) {
         return &dev->ep_ctl;
     }
     assert(pid == USB_TOKEN_IN || pid == USB_TOKEN_OUT);
-- 
2.25.1

Re: [PATCH] hw/usb/core: fix inconsistent ep and pid (UBS_TOKEN_SETUP)

[Qiang Liu]

[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]

Hi all,

I'm sure this patch will prevent the assertion failure due to the
inconsistent ep and pid (UBS_TOKEN_SETUP) (
https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg07179.html).

For UHCI (https://gitlab.com/qemu-project/qemu/-/issues/119) and OHCI (
https://gitlab.com/qemu-project/qemu/-/issues/303), this patch may be
right.

For EHCI, I found another way to trigger this assertion even with my patch
because ehci_get_pid() returns 0 if qtd->token.QTD_TOKEN_PID is not
valid[0]. In this case, the patch cannot capture it because pid is zero[2].
This case is specific to EHCI as far as I know. It seems we want to drop
the operation if ehci_get_pid() returns 0.

```static int ehci_get_pid(EHCIqtd *qtd)
{
    switch (get_field(qtd->token, QTD_TOKEN_PID)) {
    case 0:
        return USB_TOKEN_OUT;
    case 1:
        return USB_TOKEN_IN;
    case 2:
        return USB_TOKEN_SETUP;
    default:
        fprintf(stderr, "bad token\n"); //
---------------------------------------------> [0]
        return 0;
    }
}

static int ehci_execute(EHCIPacket *p, const char *action)
{
    p->pid = ehci_get_pid(&p->qtd); //
--------------------------------------------> [1]
    p->queue->last_pid = p->pid;
    endp = get_field(p->queue->qh.epchar, QH_EPCHAR_EP);
    ep = usb_ep_get(p->queue->dev, p->pid/*=0*/, endp); //
-----------------------> [2]
```

A qtest sequence is like
```
writel 0x1011b000 0x10124000
writel 0x10124004 0x358cbd80
writel 0x10124018 0x9e4bba36
writel 0x10124014 0x10139000
writel 0xfebd5020 0x1c4a5135
writel 0x10139008 0x3d5c4b84
clock_step 0xb17b0
writel 0xfebd5064 0x5f919911
clock_step 0xa9229
writel 0xfebd5064 0x5431e207
writel 0xfebd5038 0x1b2034b5
writel 0x1b2034a0 0x10100000
writel 0x10100000 0x10109000
writel 0x10109000 0x1011b000
clock_step 0xa9229
```

Best,
Qiang

[-- Attachment #2: Type: text/html, Size: 2512 bytes --]