On Mon, Aug 8, 2022 at 11:15 AM Konrad, Frederic <frederic.kon...@amd.com> wrote: > > -----Original Message----- > > From: Peter Maydell <peter.mayd...@linaro.org> > > Sent: 02 August 2022 15:34 > > To: Konrad, Frederic <frederic.kon...@amd.com> > > Cc: qemu-devel@nongnu.org; Fabien Chouteau <chout...@adacore.com>; > > Frederic Konrad <konrad.frede...@yahoo.fr>; f4...@amsat.org > > Subject: Re: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16 > > bit accesses > > > > On Tue, 2 Aug 2022 at 15:20, Konrad, Frederic <frederic.kon...@amd.com> > > wrote: > > > > > > Hi Peter, > > > > > > CC'ing Philippe. > > > > > > > -----Original Message----- > > > > From: Qemu-devel <qemu-devel- > > > > bounces+fkonrad=amd....@nongnu.org> On Behalf Of Peter Maydell > > > > Sent: 02 August 2022 14:19 > > > > To: qemu-devel@nongnu.org > > > > Cc: Fabien Chouteau <chout...@adacore.com>; Frederic Konrad > > > > <konrad.frede...@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.mayd...@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. > > Ok got it, thanks. > > Reviewed-by: Frederic Konrad <fkon...@amd.com>
Thanks Frederic. Mark, Peter, since I'm preparing a pull request for MIPS, I'll queue this single SPARC patch. Per https://gitlab.com/qemu-project/qemu/-/issues/1132#note_1048558340 I'll also add: Tested-by: Tomasz Martyniak <gitlab.com/tom4r> Regards, Phil.
