Re: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16 bit accesses

[Peter Maydell]

On Tue, 2 Aug 2022 at 15:20, Konrad, Frederic <Frederic.Konrad@amd.com> wrote:
>
> Hi Peter,
>
> CC'ing Philippe.
>
> > -----Original Message-----
> > From: Qemu-devel <qemu-devel-
> > bounces+fkonrad=amd.com@nongnu.org> On Behalf Of Peter Maydell
> > Sent: 02 August 2022 14:19
> > To: qemu-devel@nongnu.org
> > Cc: Fabien Chouteau <chouteau@adacore.com>; Frederic Konrad
> > <konrad.frederic@yahoo.fr>
> > Subject: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16 bit
> > accesses
> >
> > In real hardware, the APB and AHB PNP data tables can be accessed
> > with byte and halfword reads as well as word reads.  Our
> > implementation currently only handles word reads.  Add support for
> > the 8 and 16 bit accesses.  Note that we only need to handle aligned
> > accesses -- unaligned accesses should continue to trap, as happens on
> > hardware.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1132
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > It would be nice if we could just set the .valid.min_access_size in
> > the MemoryRegionOps to 1 and have the memory system core synthesize
> > the 1 and 2 byte accesses from a 4 byte read, but currently that
> > doesn't work (see various past mailing list threads).
>
> That looks good to me but I thought this was fixed by 1a5a5570 and 0fbe394a
> because RTEMS do bytes accesses?
>
> Did that break at some point?

I definitely tried letting the .impl vs .valid settings handle this,
but the access_with_adjusted_size() code doesn't do the right thing.
(In particular, the test case ELF in the bug report works with
this patch, and doesn't work without it...)

I'm pretty sure the problem with access_with_adjusted_size() is a
long-standing bug -- I found a couple of mailing list threads about
it. We really ought to fix that properly, but that's definitely not
for-7.1 material.

-- PMM