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

[Gerd Hoffmann]

On Tue, Mar 31, 2020 at 02:57:56AM -0700, Cameron Esfahani wrote:
> 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.

Ouch.  I didn't expect xhci being called like that.  IMO memory.c should
do properly aligned 4-byte reads, then extract the bytes needed, i.e.
2-byte read @ both offset 0 and 2 would result in xhci being called with
a 4-byte read at offset 0, then memory.c would extract the lower or upper
bytes and return them to the guest.

> 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.

Well, the point of having MemoryRegionOps.valid and MemoryRegionOps.impl
in the first place is to allow memory.c handle this on behalf of the
device, so we don't end up reinventing the wheel in each and every
device ...

So, I think xhci is correct and memory.c has a bug somewhere (and it
seems Philippe is on track pinning it down ...)

cheers,
  Gerd