Re: [PATCH v1] usb: Add read support for HCIVERSION register to XHCI

[Philippe Mathieu-Daudé]

On 3/30/20 11:44 PM, Cameron Esfahani via wrote:
> macOS will read HCIVERSION separate from CAPLENGTH.  Add a distinct
> handler for that register.

Apparently a fix for https://bugs.launchpad.net/qemu/+bug/1693050.

> Signed-off-by: Cameron Esfahani <address@hidden>
> ---
>   hw/usb/hcd-xhci.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index b330e36fe6..061f8438de 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -2739,6 +2739,9 @@ static uint64_t xhci_cap_read(void *ptr, hwaddr reg, 
> unsigned size)
>       case 0x00: /* HCIVERSION, CAPLENGTH */
>           ret = 0x01000000 | LEN_CAP;
>           break;
> +    case 0x02: /* HCIVERSION */
> +        ret = 0x0100;
> +        break;

But we have:

static const MemoryRegionOps xhci_cap_ops = {
    .read = xhci_cap_read,
    .write = xhci_cap_write,
    .valid.min_access_size = 1,
    .valid.max_access_size = 4,
    .impl.min_access_size = 4,
    .impl.max_access_size = 4,
    .endianness = DEVICE_LITTLE_ENDIAN,
};

IIUC ".impl.min_access_size = 4" means the case 'reg == 2' can not 
happen. It seems we have a bug in memory.c elsewhere.

How can we reproduce?

If not easy, can you share the backtrace of:

-- >8 --
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index b330e36fe6..d021129f3f 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2735,6 +2735,7 @@ static uint64_t xhci_cap_read(void *ptr, hwaddr 
reg, unsigned size)
     XHCIState *xhci = ptr;
     uint32_t ret;

+    assert(reg != 2);
     switch (reg) {
     case 0x00: /* HCIVERSION, CAPLENGTH */
         ret = 0x01000000 | LEN_CAP;
---

>       case 0x04: /* HCSPARAMS 1 */
>           ret = ((xhci->numports_2+xhci->numports_3)<<24)
>               | (xhci->numintrs<<8) | xhci->numslots;