[PATCH 0/2] support unaligned access for some xHCI registers

[PATCH 0/2] support unaligned access for some xHCI registers

[Tomoyuki HIROSE]

According to xHCI spec rev 1.2, unaligned access to xHCI Host
Controller Capability Registers are not prohibited. But current
implementation does not support unaligned access to 'MemoryRegion'.
These patches contain 2 changes:
1. support unaligned access to 'MemoryRegion'
2. allow unaligned access to Host Controller Capability Registers.

Tomoyuki HIROSE (2):
  system/memory.c: support unaligned access
  hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers

 hw/usb/hcd-xhci.c |  4 +++-
 system/memory.c   | 22 ++++++++++++++++------
 2 files changed, 19 insertions(+), 7 deletions(-)

-- 
2.39.2

[PATCH 1/2] system/memory.c: support unaligned access

[Tomoyuki HIROSE]

The previous code ignored 'impl.unaligned' and handled unaligned accesses
as is. But this implementation cannot emulate specific registers of some
devices that allow unaligned access such as xHCI Host Controller Capability
Registers.
This commit checks 'impl.unaligned' and if it is false, QEMU emulates
unaligned access with multiple aligned access.

Signed-off-by: Tomoyuki HIROSE <tomoyuki.hirose@igel.co.jp>
---
 system/memory.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 798b6c0a17..b0caa90fef 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -539,6 +539,9 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
     unsigned i;
     MemTxResult r = MEMTX_OK;
     bool reentrancy_guard_applied = false;
+    hwaddr aligned_addr;
+    unsigned corrected_size = size;
+    signed align_diff = 0;
 
     if (!access_size_min) {
         access_size_min = 1;
@@ -560,18 +563,25 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
         reentrancy_guard_applied = true;
     }
 
-    /* FIXME: support unaligned access? */
     access_size = MAX(MIN(size, access_size_max), access_size_min);
     access_mask = MAKE_64BIT_MASK(0, access_size * 8);
+    if (!mr->ops->impl.unaligned) {
+        aligned_addr = addr & ~(access_size - 1);
+        align_diff = addr - aligned_addr;
+        corrected_size = size < access_size ? access_size :
+                            size + (align_diff > 0 ? access_size : 0);
+        addr = aligned_addr;
+    }
     if (memory_region_big_endian(mr)) {
-        for (i = 0; i < size; i += access_size) {
+        for (i = 0; i < corrected_size; i += access_size) {
             r |= access_fn(mr, addr + i, value, access_size,
-                        (size - access_size - i) * 8, access_mask, attrs);
+                        (size - access_size - i + align_diff) * 8,
+                        access_mask, attrs);
         }
     } else {
-        for (i = 0; i < size; i += access_size) {
-            r |= access_fn(mr, addr + i, value, access_size, i * 8,
-                        access_mask, attrs);
+        for (i = 0; i < corrected_size; i += access_size) {
+            r |= access_fn(mr, addr + i, value, access_size,
+                        ((signed)i - align_diff) * 8, access_mask, attrs);
         }
     }
     if (mr->dev && reentrancy_guard_applied) {
-- 
2.39.2

[PATCH 2/2] hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers

[Tomoyuki HIROSE]

According to xHCI spec rev 1.2, unaligned access to xHCI Host
Controller Capability Registers is not prohibited. In Addition, the
limit of access size is also unspecified. Actually, some real devices
allow unaligned access and 8-byte access to these registers.
This commit makes it possible to unaligned access and 8-byte access
to Host Controller Capability Registers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/143
Signed-off-by: Tomoyuki HIROSE <tomoyuki.hirose@igel.co.jp>
---
 hw/usb/hcd-xhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4b60114207..41abeb9ac5 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3181,9 +3181,11 @@ static const MemoryRegionOps xhci_cap_ops = {
     .read = xhci_cap_read,
     .write = xhci_cap_write,
     .valid.min_access_size = 1,
-    .valid.max_access_size = 4,
+    .valid.max_access_size = 8,
+    .valid.unaligned = true,
     .impl.min_access_size = 4,
     .impl.max_access_size = 4,
+    .impl.unaligned = false,
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-- 
2.39.2

Re: [PATCH 1/2] system/memory.c: support unaligned access

[Cédric Le Goater]

Hello,

On 12/11/23 08:12, Tomoyuki HIROSE wrote:
> The previous code ignored 'impl.unaligned' and handled unaligned accesses
> as is. But this implementation cannot emulate specific registers of some
> devices that allow unaligned access such as xHCI Host Controller Capability
> Registers.
> This commit checks 'impl.unaligned' and if it is false, QEMU emulates
> unaligned access with multiple aligned access.
> 
> Signed-off-by: Tomoyuki HIROSE <tomoyuki.hirose@igel.co.jp>

FWIW, there has been a previous proposal for unaligned accesses support [1].

Thanks,

C.

[1] https://lore.kernel.org/qemu-devel/20170630030058.28943-1-andrew@aj.id.au/


> ---
>   system/memory.c | 22 ++++++++++++++++------
>   1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/system/memory.c b/system/memory.c
> index 798b6c0a17..b0caa90fef 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -539,6 +539,9 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
>       unsigned i;
>       MemTxResult r = MEMTX_OK;
>       bool reentrancy_guard_applied = false;
> +    hwaddr aligned_addr;
> +    unsigned corrected_size = size;
> +    signed align_diff = 0;
>   
>       if (!access_size_min) {
>           access_size_min = 1;
> @@ -560,18 +563,25 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
>           reentrancy_guard_applied = true;
>       }
>   
> -    /* FIXME: support unaligned access? */
>       access_size = MAX(MIN(size, access_size_max), access_size_min);
>       access_mask = MAKE_64BIT_MASK(0, access_size * 8);
> +    if (!mr->ops->impl.unaligned) {
> +        aligned_addr = addr & ~(access_size - 1);
> +        align_diff = addr - aligned_addr;
> +        corrected_size = size < access_size ? access_size :
> +                            size + (align_diff > 0 ? access_size : 0);
> +        addr = aligned_addr;
> +    }
>       if (memory_region_big_endian(mr)) {
> -        for (i = 0; i < size; i += access_size) {
> +        for (i = 0; i < corrected_size; i += access_size) {
>               r |= access_fn(mr, addr + i, value, access_size,
> -                        (size - access_size - i) * 8, access_mask, attrs);
> +                        (size - access_size - i + align_diff) * 8,
> +                        access_mask, attrs);
>           }
>       } else {
> -        for (i = 0; i < size; i += access_size) {
> -            r |= access_fn(mr, addr + i, value, access_size, i * 8,
> -                        access_mask, attrs);
> +        for (i = 0; i < corrected_size; i += access_size) {
> +            r |= access_fn(mr, addr + i, value, access_size,
> +                        ((signed)i - align_diff) * 8, access_mask, attrs);
>           }
>       }
>       if (mr->dev && reentrancy_guard_applied) {

Re: [PATCH 2/2] hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers

[Peter Maydell]

On Mon, 11 Dec 2023 at 07:13, Tomoyuki HIROSE
<tomoyuki.hirose@igel.co.jp> wrote:
>
> According to xHCI spec rev 1.2, unaligned access to xHCI Host
> Controller Capability Registers is not prohibited. In Addition, the
> limit of access size is also unspecified. Actually, some real devices
> allow unaligned access and 8-byte access to these registers.
> This commit makes it possible to unaligned access and 8-byte access
> to Host Controller Capability Registers.

We should definitely look at fixing the unaligned access
stuff, but the linked bug report is not trying to do an
unaligned access -- it wants to do a 2-byte read from offset 2,
which is aligned. The capability registers in the xHCI spec
are also all at offsets and sizes that mean that a natural
read of them is not unaligned.

thanks
-- PMM

Re: [PATCH 2/2] hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers

[Tomoyuki Hirose]

Thanks for comment.

On Mon, Dec 11, 2023 at 10:57 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> We should definitely look at fixing the unaligned access
> stuff, but the linked bug report is not trying to do an
> unaligned access -- it wants to do a 2-byte read from offset 2,
> which is aligned. The capability registers in the xHCI spec
> are also all at offsets and sizes that mean that a natural
> read of them is not unaligned.

Shouldn't I link this bug report?
Or is it not appropriate to allow unaligned access?

thanks,
Tomoyuki HIROSE

Re: [PATCH 2/2] hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers

[Peter Maydell]

On Tue, 12 Dec 2023 at 01:43, Tomoyuki Hirose
<tomoyuki.hirose@igel.co.jp> wrote:
>
> Thanks for comment.
>
> On Mon, Dec 11, 2023 at 10:57 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > We should definitely look at fixing the unaligned access
> > stuff, but the linked bug report is not trying to do an
> > unaligned access -- it wants to do a 2-byte read from offset 2,
> > which is aligned. The capability registers in the xHCI spec
> > are also all at offsets and sizes that mean that a natural
> > read of them is not unaligned.
>
> Shouldn't I link this bug report?
> Or is it not appropriate to allow unaligned access?

The bug report is definitely relevant. But depending
on how tricky the unaligned access handling turns out to
be to get right, we might be able to fix the bug by
permitting aligned-but-not-4-bytes accesses. (I'm
a bit surprised that doesn't work already, in fact:
we use it in other devices.)

thanks
-- PMM

Re: [PATCH 1/2] system/memory.c: support unaligned access

[Tomoyuki Hirose]

On Mon, Dec 11, 2023 at 10:31 PM Cédric Le Goater <clg@redhat.com> wrote:
> FWIW, there has been a previous proposal for unaligned accesses support [1].
>
> Thanks,
>
> C.
>
> [1] https://lore.kernel.org/qemu-devel/20170630030058.28943-1-andrew@aj.id.au/
>

Thanks for your information.
This patch is an RFC, and unfortunately it doesn't seem to have been merged.
This feature is useful for devices that allow unaligned access.

Regards,

Tomoyuki HIROSE

On Mon, Dec 11, 2023 at 10:31 PM Cédric Le Goater <clg@redhat.com> wrote:
>
> Hello,
>
> On 12/11/23 08:12, Tomoyuki HIROSE wrote:
> > The previous code ignored 'impl.unaligned' and handled unaligned accesses
> > as is. But this implementation cannot emulate specific registers of some
> > devices that allow unaligned access such as xHCI Host Controller Capability
> > Registers.
> > This commit checks 'impl.unaligned' and if it is false, QEMU emulates
> > unaligned access with multiple aligned access.
> >
> > Signed-off-by: Tomoyuki HIROSE <tomoyuki.hirose@igel.co.jp>
>
> FWIW, there has been a previous proposal for unaligned accesses support [1].
>
> Thanks,
>
> C.
>
> [1] https://lore.kernel.org/qemu-devel/20170630030058.28943-1-andrew@aj.id.au/
>
>
> > ---
> >   system/memory.c | 22 ++++++++++++++++------
> >   1 file changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/system/memory.c b/system/memory.c
> > index 798b6c0a17..b0caa90fef 100644
> > --- a/system/memory.c
> > +++ b/system/memory.c
> > @@ -539,6 +539,9 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
> >       unsigned i;
> >       MemTxResult r = MEMTX_OK;
> >       bool reentrancy_guard_applied = false;
> > +    hwaddr aligned_addr;
> > +    unsigned corrected_size = size;
> > +    signed align_diff = 0;
> >
> >       if (!access_size_min) {
> >           access_size_min = 1;
> > @@ -560,18 +563,25 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
> >           reentrancy_guard_applied = true;
> >       }
> >
> > -    /* FIXME: support unaligned access? */
> >       access_size = MAX(MIN(size, access_size_max), access_size_min);
> >       access_mask = MAKE_64BIT_MASK(0, access_size * 8);
> > +    if (!mr->ops->impl.unaligned) {
> > +        aligned_addr = addr & ~(access_size - 1);
> > +        align_diff = addr - aligned_addr;
> > +        corrected_size = size < access_size ? access_size :
> > +                            size + (align_diff > 0 ? access_size : 0);
> > +        addr = aligned_addr;
> > +    }
> >       if (memory_region_big_endian(mr)) {
> > -        for (i = 0; i < size; i += access_size) {
> > +        for (i = 0; i < corrected_size; i += access_size) {
> >               r |= access_fn(mr, addr + i, value, access_size,
> > -                        (size - access_size - i) * 8, access_mask, attrs);
> > +                        (size - access_size - i + align_diff) * 8,
> > +                        access_mask, attrs);
> >           }
> >       } else {
> > -        for (i = 0; i < size; i += access_size) {
> > -            r |= access_fn(mr, addr + i, value, access_size, i * 8,
> > -                        access_mask, attrs);
> > +        for (i = 0; i < corrected_size; i += access_size) {
> > +            r |= access_fn(mr, addr + i, value, access_size,
> > +                        ((signed)i - align_diff) * 8, access_mask, attrs);
> >           }
> >       }
> >       if (mr->dev && reentrancy_guard_applied) {
>

Re: [PATCH 2/2] hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers

[Tomoyuki Hirose]

On Tue, Dec 12, 2023 at 7:26 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 12 Dec 2023 at 01:43, Tomoyuki Hirose
> <tomoyuki.hirose@igel.co.jp> wrote:
> >
> > Thanks for comment.
> >
> > On Mon, Dec 11, 2023 at 10:57 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > > We should definitely look at fixing the unaligned access
> > > stuff, but the linked bug report is not trying to do an
> > > unaligned access -- it wants to do a 2-byte read from offset 2,
> > > which is aligned. The capability registers in the xHCI spec
> > > are also all at offsets and sizes that mean that a natural
> > > read of them is not unaligned.
> >
> > Shouldn't I link this bug report?
> > Or is it not appropriate to allow unaligned access?
>
> The bug report is definitely relevant. But depending
> on how tricky the unaligned access handling turns out to
> be to get right, we might be able to fix the bug by
> permitting aligned-but-not-4-bytes accesses. (I'm
> a bit surprised that doesn't work already, in fact:
> we use it in other devices.)
>
> thanks
> -- PMM

Thank you for answering my question.
The unaligned access handling of my patch is not so tricky.
If the access is unaligned, just correct the access size
and address and read the value as before.
Also, it is allowed by the specifications, and byte access
was possible even on real devices.

Regards,
Tomoyuki HIROSE

Re: [PATCH 0/2] support unaligned access for some xHCI registers

[Tomoyuki Hirose]

I would be grateful if you would any comments on my patch.

ping,

Tomoyuki HIROSE

On Mon, Dec 11, 2023 at 4:12 PM Tomoyuki HIROSE
<tomoyuki.hirose@igel.co.jp> wrote:
>
> According to xHCI spec rev 1.2, unaligned access to xHCI Host
> Controller Capability Registers are not prohibited. But current
> implementation does not support unaligned access to 'MemoryRegion'.
> These patches contain 2 changes:
> 1. support unaligned access to 'MemoryRegion'
> 2. allow unaligned access to Host Controller Capability Registers.
>
> Tomoyuki HIROSE (2):
>   system/memory.c: support unaligned access
>   hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers
>
>  hw/usb/hcd-xhci.c |  4 +++-
>  system/memory.c   | 22 ++++++++++++++++------
>  2 files changed, 19 insertions(+), 7 deletions(-)
>
> --
> 2.39.2
>

Re: [PATCH 0/2] support unaligned access for some xHCI registers

[Peter Maydell]

On Tue, 19 Dec 2023 at 04:49, Tomoyuki Hirose
<tomoyuki.hirose@igel.co.jp> wrote:
>
> I would be grateful if you would any comments on my patch.

It's on my todo list, but at this point I'm afraid I'm
not going to be able to get to it before I break for
the holidays, so it will be January before I can look at
it. (It's a bit complicated because I'll need to look
at this patch, at the other suggested patch from the
past that also tried to address this, at various
mailing list discussions we've had about the problem,
and at how the code in general is doing things, so it's
not a trivial change to review.)

thanks
-- PMM

Re: [PATCH 0/2] support unaligned access for some xHCI registers

[Tomoyuki Hirose]

On Tue, Dec 19, 2023 at 8:26 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 19 Dec 2023 at 04:49, Tomoyuki Hirose
> <tomoyuki.hirose@igel.co.jp> wrote:
> >
> > I would be grateful if you would any comments on my patch.
>
> It's on my todo list, but at this point I'm afraid I'm
> not going to be able to get to it before I break for
> the holidays, so it will be January before I can look at
> it. (It's a bit complicated because I'll need to look
> at this patch, at the other suggested patch from the
> past that also tried to address this, at various
> mailing list discussions we've had about the problem,
> and at how the code in general is doing things, so it's
> not a trivial change to review.)

OK, I understand.
Enjoy your holidays!

Regards,
Tomoyuki HIROSE

Re: [PATCH 1/2] system/memory.c: support unaligned access

[Peter Maydell]

On Mon, 11 Dec 2023 at 07:14, Tomoyuki HIROSE
<tomoyuki.hirose@igel.co.jp> wrote:
>
> The previous code ignored 'impl.unaligned' and handled unaligned accesses
> as is. But this implementation cannot emulate specific registers of some
> devices that allow unaligned access such as xHCI Host Controller Capability
> Registers.
> This commit checks 'impl.unaligned' and if it is false, QEMU emulates
> unaligned access with multiple aligned access.
>
> Signed-off-by: Tomoyuki HIROSE <tomoyuki.hirose@igel.co.jp>

Sorry this has taken me so long to get to reviewing.

So, first of all, I think this is definitely a cleaner looking
patch than the other one that Cédric posted a link to: if we
can have access_with_adjusted_size() cope with the unaligned
case that seems nicer than having different functions for the
aligned and the unaligned cases.

My review comments below are basically about fiddly corner case
details.

> ---
>  system/memory.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/system/memory.c b/system/memory.c
> index 798b6c0a17..b0caa90fef 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -539,6 +539,9 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
>      unsigned i;
>      MemTxResult r = MEMTX_OK;
>      bool reentrancy_guard_applied = false;
> +    hwaddr aligned_addr;
> +    unsigned corrected_size = size;
> +    signed align_diff = 0;
>
>      if (!access_size_min) {
>          access_size_min = 1;
> @@ -560,18 +563,25 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
>          reentrancy_guard_applied = true;
>      }
>
> -    /* FIXME: support unaligned access? */
>      access_size = MAX(MIN(size, access_size_max), access_size_min);
>      access_mask = MAKE_64BIT_MASK(0, access_size * 8);
> +    if (!mr->ops->impl.unaligned) {
> +        aligned_addr = addr & ~(access_size - 1);
> +        align_diff = addr - aligned_addr;
> +        corrected_size = size < access_size ? access_size :
> +                            size + (align_diff > 0 ? access_size : 0);

I don't think this calculation of corrected_size is right for
the case when size < access_size. Consider:
 * size = 2, access_size = 4, addr = 3, little-endian:
memory contents from 0 are bytes AA BB CC DD EE FF ...

We calculate corrected_size of 4, and we will then do a
single 4-byte read of 0xDDCCBBAA. But we need to do two
4-byte reads, because the final result we want to return
is 0xEEDD.

I think also that we don't really get the optimal behaviour
here because we select access_size assuming the aligned case,
rather than selecting it specifically for the combination
of input size and align_diff in the unaligned case.
Consider: access_size_min = 2, access_size_max = 8, size = 4,
addr = 2. We'll compute access_size to be 4, and then do
the unaligned access with two 4-byte reads. But we could
better have done it with two 2-byte reads. This matters
especially for the write case, because two 2-byte writes
allows us to avoid the problem of "what do we write for
the parts of the 4-byte writes that we don't have data
from the caller for". (See below for more on that.)

> +        addr = aligned_addr;
> +    }
>      if (memory_region_big_endian(mr)) {
> -        for (i = 0; i < size; i += access_size) {
> +        for (i = 0; i < corrected_size; i += access_size) {
>              r |= access_fn(mr, addr + i, value, access_size,
> -                        (size - access_size - i) * 8, access_mask, attrs);
> +                        (size - access_size - i + align_diff) * 8,
> +                        access_mask, attrs);
>          }
>      } else {
> -        for (i = 0; i < size; i += access_size) {
> -            r |= access_fn(mr, addr + i, value, access_size, i * 8,
> -                        access_mask, attrs);
> +        for (i = 0; i < corrected_size; i += access_size) {
> +            r |= access_fn(mr, addr + i, value, access_size,
> +                        ((signed)i - align_diff) * 8, access_mask, attrs);
>          }

So, with these loops, for unaligned accesses we now load an
extra chunk and adjust the shifts so we get the right parts
of the chunks we read. However I think we also need to be
careful with the access mask for the final access (or the
first access in the big-endian case).

Consider:
 * access_size = 2, size = 4, align_diff = 1, little endian,
   addr = 1 initially (so aligned down to 0), read:
and the memory being bytes AA BB CC DD EE FF ... starting at 0.
We'll load:
 * from addr 0, 0xBBAA, which we shift right by 1 for 0xBB
 * from addr 2, 0xDDCC, shift left 1, for 0xDDCC00
 * from addr 4, 0xFFEE, shift left 3, for 0xFFEE000000
and then we OR those together for
  0xFFEEDDCCBB
so we will have written into *value an extra 0xFF byte that
we should not have done. That last access from addr 4 should
have an access_mask that says we only want part of it.

For writes, things are worse, because we'll do a 2 byte
write that writes whatever garbage might have been in the
high part of *value. If the device permits an access of
smaller size (in this case byte) we can do the end part at
that size (or even do the whole write at that size if it's
simpler). If the device doesn't permit that smaller size
write it's not clear to me what the behaviour should be.
(I was going to suggest maybe we should just rule that as
not permitted until we run into it, but then your patch 2
puts the xhci-usb device into this category, although
harmlessly because it happens to implement writes as ignored.)

thanks
-- PMM

Re: [PATCH 1/2] system/memory.c: support unaligned access

[Tomoyuki Hirose]

Hello,

Thank you for reviewing my patches.
Examples of corner case you explained is very helpful.
I'm currently working on patches. I will submit v2 as soon
as it's completed, so please wait a little longer.

thanks,
Tomoyuki HIROSE

Re: [PATCH 2/2] hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers

[Peter Xu]

On Mon, Dec 11, 2023 at 01:57:42PM +0000, Peter Maydell wrote:
> On Mon, 11 Dec 2023 at 07:13, Tomoyuki HIROSE
> <tomoyuki.hirose@igel.co.jp> wrote:
> >
> > According to xHCI spec rev 1.2, unaligned access to xHCI Host
> > Controller Capability Registers is not prohibited. In Addition, the
> > limit of access size is also unspecified. Actually, some real devices
> > allow unaligned access and 8-byte access to these registers.
> > This commit makes it possible to unaligned access and 8-byte access
> > to Host Controller Capability Registers.
> 
> We should definitely look at fixing the unaligned access
> stuff, but the linked bug report is not trying to do an
> unaligned access -- it wants to do a 2-byte read from offset 2,
> which is aligned.

IIUC the issue is the xHCI xhci_cap_ops defines impl.min_access_size=4.
Then it'll be enlarged to 4 bytes read on offset 0x2.  IOW, I think it'll
also work if xhci_cap_ops can support impl.min_access_size=2, however I
don't know whether that can be more than that register, so that the memory
patch is still a more generic approach.

Thanks,

-- 
Peter Xu