The controller registers need a 4 byte access, but the 8 bytes at 0x80 are not registers. They are a SRAM buffer holding the EP0 SETUP packet. A read or a write that is not 4 bytes returned the wrong data, so the driver saw a broken SETUP packet.
Introduce a separate SETUP MMIO region that handles 1, 2 and 4 byte reads and writes. Signed-off-by: Jamin Lin <[email protected]> --- include/hw/usb/aspeed-udc.h | 11 ++++++-- hw/usb/aspeed-udc.c | 56 +++++++++++++++++++++++++++++++++---- hw/usb/trace-events | 2 ++ 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/include/hw/usb/aspeed-udc.h b/include/hw/usb/aspeed-udc.h index 7701c1aa34..42916c0bd6 100644 --- a/include/hw/usb/aspeed-udc.h +++ b/include/hw/usb/aspeed-udc.h @@ -24,11 +24,14 @@ OBJECT_DECLARE_SIMPLE_TYPE(AspeedUDCState, ASPEED_UDC) OBJECT_DECLARE_SIMPLE_TYPE(AspeedUDCGadget, ASPEED_UDC_GADGET) /* - * Register map: root/global block at 0x000 - 0x087, then one 0x10 byte bank - * per programmable endpoint from 0x200. + * Register map: root/global block at 0x000 - 0x07f, the SETUP buffer at + * 0x080 - 0x087, then one 0x10 byte bank per programmable endpoint from + * 0x200. */ #define ASPEED_UDC_MEM_SIZE 0x300 -#define ASPEED_UDC_ROOT_NR_REGS (0x88 >> 2) +#define ASPEED_UDC_ROOT_NR_REGS (0x80 >> 2) +#define ASPEED_UDC_SETUP_BASE 0x80 +#define ASPEED_UDC_SETUP_SIZE 0x08 #define ASPEED_UDC_EP_REG_BASE 0x200 #define ASPEED_UDC_EP_NR_REGS (0x10 >> 2) @@ -62,9 +65,11 @@ struct AspeedUDCState { MemoryRegion udc_container; MemoryRegion root_mr; + MemoryRegion setup_mr; MemoryRegion *dram_mr; AddressSpace dram_as; uint32_t regs[ASPEED_UDC_ROOT_NR_REGS]; + uint8_t setup_buf[ASPEED_UDC_SETUP_SIZE]; AspeedUDCEP ep[ASPEED_UDC_NUM_EP]; qemu_irq irq; diff --git a/hw/usb/aspeed-udc.c b/hw/usb/aspeed-udc.c index 4c90b4f4c2..355512822b 100644 --- a/hw/usb/aspeed-udc.c +++ b/hw/usb/aspeed-udc.c @@ -58,9 +58,6 @@ REG32(UDC_EP0_CTRL, 0x30) FIELD(UDC_EP0_CTRL, STALL, 0, 1) REG32(UDC_EP0_DATA_BUFF, 0x34) FIELD(UDC_EP0_DATA_BUFF, BASE_ADDR, 0, 31) -/* EP0 SETUP packet buffer: SETUP0 = bytes 0...3, SETUP1 = bytes 4...7 */ -REG32(UDC_SETUP0, 0x80) -REG32(UDC_SETUP1, 0x84) /* Per programmable-endpoint registers (offset from the EP register base) */ REG32(EP_CONFIG, 0x00) @@ -363,6 +360,45 @@ static const MemoryRegionOps aspeed_udc_ops = { }, }; +static uint64_t aspeed_udc_setup_read(void *opaque, hwaddr offset, + unsigned size) +{ + AspeedUDCState *s = ASPEED_UDC(opaque); + uint64_t value = 0; + int i; + + for (i = 0; i < size; i++) { + value |= (uint64_t)s->setup_buf[offset + i] << (8 * i); + } + + trace_aspeed_udc_setup_read(offset, size, value); + + return value; +} + +static void aspeed_udc_setup_write(void *opaque, hwaddr offset, + uint64_t value, unsigned size) +{ + AspeedUDCState *s = ASPEED_UDC(opaque); + int i; + + trace_aspeed_udc_setup_write(offset, size, value); + + for (i = 0; i < size; i++) { + s->setup_buf[offset + i] = value >> (8 * i); + } +} + +static const MemoryRegionOps aspeed_udc_setup_ops = { + .read = aspeed_udc_setup_read, + .write = aspeed_udc_setup_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .valid = { + .min_access_size = 1, + .max_access_size = 4, + }, +}; + /* * Copy len bytes from guest memory at addr into the IN packet, going through * a bounce buffer one buf-full at a time. Returns false on DMA failure. @@ -708,6 +744,7 @@ static void aspeed_udc_reset_hold(Object *obj, ResetType type) int i; memset(s->regs, 0, sizeof(s->regs)); + memset(s->setup_buf, 0, sizeof(s->setup_buf)); for (i = 0; i < ASPEED_UDC_NUM_EP; i++) { memset(s->ep[i].regs, 0, sizeof(s->ep[i].regs)); s->ep[i].pkt = NULL; @@ -754,6 +791,12 @@ static void aspeed_udc_realize(DeviceState *dev, Error **errp) ASPEED_UDC_ROOT_NR_REGS << 2); memory_region_add_subregion(&s->udc_container, 0, &s->root_mr); + memory_region_init_io(&s->setup_mr, OBJECT(s), &aspeed_udc_setup_ops, + s, TYPE_ASPEED_UDC ".setup", + ASPEED_UDC_SETUP_SIZE); + memory_region_add_subregion(&s->udc_container, + ASPEED_UDC_SETUP_BASE, &s->setup_mr); + /* Each programmable endpoint has its own register bank */ for (i = 0; i < ASPEED_UDC_NUM_EP; i++) { g_autofree char *name = g_strdup_printf(TYPE_ASPEED_UDC ".ep%d", i); @@ -918,8 +961,11 @@ static void aspeed_udc_gadget_handle_control(USBDevice *udev, USBPacket *p, * Reconstruct the 8-byte SETUP packet into the SETUP data buffer where * the guest gadget driver reads it from. */ - s->regs[R_UDC_SETUP0] = type | (req << 8) | ((value & 0xffff) << 16); - s->regs[R_UDC_SETUP1] = (index & 0xffff) | ((length & 0xffff) << 16); + s->setup_buf[0] = type; + s->setup_buf[1] = req; + stw_le_p(&s->setup_buf[2], value); + stw_le_p(&s->setup_buf[4], index); + stw_le_p(&s->setup_buf[6], length); /* A new SETUP clears the EP0 STALL condition */ s->regs[R_UDC_EP0_CTRL] &= ~R_UDC_EP0_CTRL_STALL_MASK; diff --git a/hw/usb/trace-events b/hw/usb/trace-events index 931996e8cd..e84561d2e8 100644 --- a/hw/usb/trace-events +++ b/hw/usb/trace-events @@ -381,6 +381,8 @@ canokey_unrealize(void) # aspeed-udc.c aspeed_udc_read(uint64_t offset, uint32_t value) "offset 0x%" PRIx64 " value 0x%x" aspeed_udc_write(uint64_t offset, uint32_t value) "offset 0x%" PRIx64 " value 0x%x" +aspeed_udc_setup_read(uint64_t offset, unsigned size, uint64_t value) "offset 0x%" PRIx64 " size %u value 0x%" PRIx64 +aspeed_udc_setup_write(uint64_t offset, unsigned size, uint64_t value) "offset 0x%" PRIx64 " size %u value 0x%" PRIx64 aspeed_udc_ep_read(int ep, uint64_t offset, uint32_t value) "ep %d, offset 0x%" PRIx64 " value 0x%x" aspeed_udc_ep_write(int ep, uint64_t offset, uint32_t value) "ep %d, offset 0x%" PRIx64 " value 0x%x" aspeed_udc_pullup(int on, int attached) "on %d, attached %d" -- 2.53.0
