Philippe - From what I've seen, access size has nothing to do with alignment.
What the code in access_with_adjusted_size() will do is make sure that "size" is >= min_access_size and <= max_access_size. So reading 2-bytes from address 2 turns into reading 4-bytes from address 2: xhci_cap_read() is called with reg 2, size 4. But, due to the fact our change to support reg 2 only returns back 2-bytes, and how the loops work in access_with_adjusted_size(), we only call xhci_cap_read() once. It seems like we should also change impl.min_access_size for xhci_cap_ops to be 2. But, after that, to support people doing strange things like reading traditionally 4-byte values as 2 2-byte values, we probably need to change xhci_cap_read() to handle every memory range in steps of 2-bytes. But I'll defer to Gerd on this... Cameron Esfahani di...@apple.com "Americans are very skilled at creating a custom meaning from something that's mass-produced." Ann Powers > On Mar 31, 2020, at 12:52 AM, Philippe Mathieu-Daudé <phi...@redhat.com> > wrote: > > 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 <di...@apple.com> >> --- >> 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;
