[PATCH v2 01/11] hw/usb/aspeed-udc: Make the SETUP buffer byte addressable

[Jamin Lin <jamin_lin@aspeedtech.com>]

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 <jamin_lin@aspeedtech.com>
---
 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 80ead23358..aa260dacad 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.43.0