[v2] support unaligned access to xHCI Capability

[RFC PATCH v2 3/9] hw: npcm7xx_fiu and mx_pic change .impl.unaligned = true

Posted by CJ Chen 2 months, 3 weeks ago

By setting .impl.unaligned = true, we allow QEMU to pass along
unaligned requests directly as-is, rather than splitting them into
multiple aligned sub-requests that might cause repeated device
callbacks or unintended side effects.

Signed-off-by: CJ Chen <cjchen@igel.co.jp>
Tested-by: CJ Chen <cjchen@igel.co.jp>
Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
---
 hw/ssi/npcm7xx_fiu.c | 3 +++
 hw/xtensa/mx_pic.c   | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/hw/ssi/npcm7xx_fiu.c b/hw/ssi/npcm7xx_fiu.c
index 056ce13394..10ee4deb31 100644
--- a/hw/ssi/npcm7xx_fiu.c
+++ b/hw/ssi/npcm7xx_fiu.c
@@ -255,6 +255,9 @@ static const MemoryRegionOps npcm7xx_fiu_flash_ops = {
         .max_access_size = 8,
         .unaligned = true,
     },
+    .impl = {
+        .unaligned = true,
+    },
 };
 
 /* Control register read handler. */
diff --git a/hw/xtensa/mx_pic.c b/hw/xtensa/mx_pic.c
index 8211c993eb..6bf524a918 100644
--- a/hw/xtensa/mx_pic.c
+++ b/hw/xtensa/mx_pic.c
@@ -270,6 +270,9 @@ static const MemoryRegionOps xtensa_mx_pic_ops = {
     .valid = {
         .unaligned = true,
     },
+    .impl = {
+        .unaligned = true,
+    },
 };
 
 MemoryRegion *xtensa_mx_pic_register_cpu(XtensaMxPic *mx,
-- 
2.25.1

Re: [RFC PATCH v2 3/9] hw: npcm7xx_fiu and mx_pic change .impl.unaligned = true

Posted by Philippe Mathieu-Daudé 2 months, 3 weeks ago

Hi,

On 22/8/25 11:24, CJ Chen wrote:
> By setting .impl.unaligned = true, we allow QEMU to pass along
> unaligned requests directly as-is, rather than splitting them into
> multiple aligned sub-requests that might cause repeated device
> callbacks or unintended side effects.
> 
> Signed-off-by: CJ Chen <cjchen@igel.co.jp>
> Tested-by: CJ Chen <cjchen@igel.co.jp>
> Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> ---
>   hw/ssi/npcm7xx_fiu.c | 3 +++
>   hw/xtensa/mx_pic.c   | 3 +++
>   2 files changed, 6 insertions(+)


> diff --git a/hw/xtensa/mx_pic.c b/hw/xtensa/mx_pic.c
> index 8211c993eb..6bf524a918 100644
> --- a/hw/xtensa/mx_pic.c
> +++ b/hw/xtensa/mx_pic.c
> @@ -270,6 +270,9 @@ static const MemoryRegionOps xtensa_mx_pic_ops = {
>       .valid = {
>           .unaligned = true,
>       },
> +    .impl = {
> +        .unaligned = true,
> +    },
>   };

Surely a distinct patch.

Re: [RFC PATCH v2 3/9] hw: npcm7xx_fiu and mx_pic change .impl.unaligned = true

Posted by Peter Xu 2 months, 1 week ago

On Mon, Aug 25, 2025 at 01:00:21PM +0200, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 22/8/25 11:24, CJ Chen wrote:
> > By setting .impl.unaligned = true, we allow QEMU to pass along
> > unaligned requests directly as-is, rather than splitting them into
> > multiple aligned sub-requests that might cause repeated device
> > callbacks or unintended side effects.
> > 
> > Signed-off-by: CJ Chen <cjchen@igel.co.jp>
> > Tested-by: CJ Chen <cjchen@igel.co.jp>
> > Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> > Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> > ---
> >   hw/ssi/npcm7xx_fiu.c | 3 +++
> >   hw/xtensa/mx_pic.c   | 3 +++
> >   2 files changed, 6 insertions(+)
> 
> 
> > diff --git a/hw/xtensa/mx_pic.c b/hw/xtensa/mx_pic.c
> > index 8211c993eb..6bf524a918 100644
> > --- a/hw/xtensa/mx_pic.c
> > +++ b/hw/xtensa/mx_pic.c
> > @@ -270,6 +270,9 @@ static const MemoryRegionOps xtensa_mx_pic_ops = {
> >       .valid = {
> >           .unaligned = true,
> >       },
> > +    .impl = {
> > +        .unaligned = true,
> > +    },
> >   };
> 
> Surely a distinct patch.

Besides that, I also don't understand how it used to work even before this
change..

E.g., both of the xtensa mx_pic read/write ignores size, that doesn't look
right already when there's totally no limitation of impl.*_access_size.

Meanwhile, taking xtensa_mx_pic_ext_reg_read() as example, it'll return a
u32 by offset smaller than MX_MAX_IRQ (where MIROUT==0):

    if (offset < MIROUT + MX_MAX_IRQ) {
        return mx->mirout[offset - MIROUT];
    }

But it returns different u32 for different offsets.. so reading 0x0 returns
the 1st u32, then 0x1 returns the 2nd (rather than reading 0x4 returns
that).

Even if there might be a driver that works with it, it still doesn't look
like the traditional way of doing MMIOs.. irrelevant of setting unaligned
or not in its .impl.

-- 
Peter Xu