[Qemu-devel] [PATCH] xhci: allow 1 and 2 bytes accesses to capability registers

[Alejandro Martinez Ruiz]

Some xHC drivers (most notably on Windows and BSD systems) read
the first capability registers using 1 and 2 bytes accesses, since
this is how they are defined in section 5.3 of the xHCI specs.

Enabling these kind of read accesses allows Windows and FreeBSD
guests to properly recognize the host controller.

As this is an exception to the general 4-byte aligned accesses rule,
we special-case the code path for capability reading and implement
checks to guard against wrong size/alignment combinations.

Signed-off-by: Alejandro Martinez Ruiz <address@hidden>
---
 hw/usb/hcd-xhci.c | 75 ++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 20 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 6c2ff02..6cca161 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2320,13 +2320,30 @@ static void xhci_reset(DeviceState *dev)
     xhci->ev_buffer_get = 0;
 }
 
-static uint32_t xhci_cap_read(XHCIState *xhci, uint32_t reg)
+static uint32_t xhci_cap_read(XHCIState *xhci, uint32_t reg, unsigned size)
 {
-    uint32_t ret;
+    uint32_t ret = 0;
+
+    /*
+     * Section 5.3 of the xHCI specification defines the first capability
+     * registers as being only 1 and 2 bytes in size. In fact, these are
+     * often accessed as 1 or 2 bytes reads.
+     *
+     * Some drivers read the first 4 bytes in one go, while others -most
+     * notably the original NEC Renesas driver for Windows and the *BSDs-
+     * read one register at a time. This is the only known exception to
+     * the 4 byte accesses rule, so we'll special-case the code.
+     */
 
     switch (reg) {
-    case 0x00: /* HCIVERSION, CAPLENGTH */
-        ret = 0x01000000 | LEN_CAP;
+    case 0x00: /* CAPLENGTH [, HCIVERSION] */
+        ret = LEN_CAP;
+        if (size < 4) {
+            break;
+        }
+        /* fall-through if asking for all 4 bytes */
+    case 0x02: /* HCIVERSION */
+        ret |= 0x01000000 >> (4 - size) * CHAR_BIT;
         break;
     case 0x04: /* HCSPARAMS 1 */
         ret = (MAXPORTS<<24) | (MAXINTRS<<8) | MAXSLOTS;
@@ -2685,26 +2702,43 @@ static void xhci_doorbell_write(XHCIState *xhci, 
uint32_t reg, uint32_t val)
 static uint64_t xhci_mem_read(void *ptr, target_phys_addr_t addr,
                               unsigned size)
 {
+    uint64_t ret = 0;
     XHCIState *xhci = ptr;
 
-    /* Only aligned reads are allowed on xHCI */
-    if (addr & 3) {
-        fprintf(stderr, "xhci_mem_read: Mis-aligned read\n");
-        return 0;
-    }
-
+    /* Allow 1, 2 and 4-byte aligned reads on capabilities, and only
+     * 4-byte reads elsewhere.
+     */
     if (addr < LEN_CAP) {
-        return xhci_cap_read(xhci, addr);
-    } else if (addr >= OFF_OPER && addr < (OFF_OPER + LEN_OPER)) {
-        return xhci_oper_read(xhci, addr - OFF_OPER);
-    } else if (addr >= OFF_RUNTIME && addr < (OFF_RUNTIME + LEN_RUNTIME)) {
-        return xhci_runtime_read(xhci, addr - OFF_RUNTIME);
-    } else if (addr >= OFF_DOORBELL && addr < (OFF_DOORBELL + LEN_DOORBELL)) {
-        return xhci_doorbell_read(xhci, addr - OFF_DOORBELL);
+        /* deny accesses to odd addresses, specially since we accept 1-byte 
reads */
+        if (addr & 1) {
+            fprintf(stderr, "xhci_mem_read: invalid %ud-byte capability read 
at address %x\n", size, (unsigned int) addr);
+            goto out;
+        }
+
+        /* We deal with read size down in xhci_cap_read, since access
+         * is variable for some addresses.
+         */
+        ret = xhci_cap_read(xhci, addr, size);
     } else {
-        fprintf(stderr, "xhci_mem_read: Bad offset %x\n", (int)addr);
-        return 0;
+        /* non capability read */
+        if (size < 4) {
+            fprintf(stderr, "xhci_mem_read: mis-aligned %ud-byte read on 
address %x\n", size, (unsigned int) addr);
+            goto out;
+        }
+
+        if (addr >= OFF_OPER && addr < (OFF_OPER + LEN_OPER)) {
+            ret = xhci_oper_read(xhci, addr - OFF_OPER);
+        } else if (addr >= OFF_RUNTIME && addr < (OFF_RUNTIME + LEN_RUNTIME)) {
+            ret = xhci_runtime_read(xhci, addr - OFF_RUNTIME);
+        } else if (addr >= OFF_DOORBELL && addr < (OFF_DOORBELL + 
LEN_DOORBELL)) {
+            ret = xhci_doorbell_read(xhci, addr - OFF_DOORBELL);
+        } else {
+            fprintf(stderr, "xhci_mem_read: tried to read %ud bytes from bad 
offset %x\n", size, (unsigned int) addr);
+        }
     }
+
+out:
+    return ret;
 }
 
 static void xhci_mem_write(void *ptr, target_phys_addr_t addr,
@@ -2732,8 +2766,9 @@ static void xhci_mem_write(void *ptr, target_phys_addr_t 
addr,
 static const MemoryRegionOps xhci_mem_ops = {
     .read = xhci_mem_read,
     .write = xhci_mem_write,
-    .valid.min_access_size = 4,
+    .valid.min_access_size = 1,
     .valid.max_access_size = 4,
+    .valid.unaligned = false,
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-- 
1.7.12.rc2.18.g61b472e