Re: [PATCH] hw/pci-host/raven.c: Mark raven_io_ops as implementing unaligned accesses

[Peter Maydell]

On Tue, 23 Jan 2024 at 11:03, Cédric Le Goater <clegoate@redhat.com> wrote:
>
> On 1/12/24 14:46, Peter Maydell wrote:
> > The raven_io_ops MemoryRegionOps is the only one in the source tree
> > which sets .valid.unaligned to indicate that it should support
> > unaligned accesses and which does not also set .impl.unaligned to
> > indicate that its read and write functions can do the unaligned
> > handling themselves.  This is a problem, because at the moment the
> > core memory system does not implement the support for handling
> > unaligned accesses by doing a series of aligned accesses and
> > combining them (system/memory.c:access_with_adjusted_size() has a
> > TODO comment noting this).
> >
> > Fortunately raven_io_read() and raven_io_write() will correctly deal
> > with the case of being passed an unaligned address, so we can fix the
> > missing unaligned access support by setting .impl.unaligned in the
> > MemoryRegionOps struct.
> >
> > Fixes: 9a1839164c9c8f06 ("raven: Implement non-contiguous I/O region")
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > Spotted by code inspection: I was looking for devices whose behaviour
> > might be changed by a patch I'm reviewing that adds that missing
> > support for unaligned accesses in the core memory system. But even
> > if we do implement it there, it's more efficient for the raven MR
> > to correctly mark it as handling unaligned accesses itself.
> >
> > Tested with 'make check' and 'make check-avocado' only.
>
> It doesn't affect the prep machine boot with OpenBIOS and a
> "Debian GNU/Linux 3.0 6015" image.
>
> Tested-by: Cédric Le Goater <clg@redhat.com>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks for the review -- is this patch going to go via a
ppc queue, or should I throw it in with my upcoming
target-arm pullreq?

-- PMM