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? Regards, Fred > --- > hw/misc/grlib_ahb_apb_pnp.c | 10 ++++++---- > hw/misc/trace-events | 4 ++-- > 2 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/hw/misc/grlib_ahb_apb_pnp.c b/hw/misc/grlib_ahb_apb_pnp.c > index 43e001c3c7b..5b05f158592 100644 > --- a/hw/misc/grlib_ahb_apb_pnp.c > +++ b/hw/misc/grlib_ahb_apb_pnp.c > @@ -136,7 +136,8 @@ static uint64_t grlib_ahb_pnp_read(void *opaque, > hwaddr offset, unsigned size) > uint32_t val; > > val = ahb_pnp->regs[offset >> 2]; > - trace_grlib_ahb_pnp_read(offset, val); > + val = extract32(val, (4 - (offset & 3) - size) * 8, size * 8); > + trace_grlib_ahb_pnp_read(offset, size, val); > > return val; > } > @@ -152,7 +153,7 @@ static const MemoryRegionOps grlib_ahb_pnp_ops = > { > .write = grlib_ahb_pnp_write, > .endianness = DEVICE_BIG_ENDIAN, > .impl = { > - .min_access_size = 4, > + .min_access_size = 1, > .max_access_size = 4, > }, > }; > @@ -247,7 +248,8 @@ static uint64_t grlib_apb_pnp_read(void *opaque, > hwaddr offset, unsigned size) > uint32_t val; > > val = apb_pnp->regs[offset >> 2]; > - trace_grlib_apb_pnp_read(offset, val); > + val = extract32(val, (4 - (offset & 3) - size) * 8, size * 8); > + trace_grlib_apb_pnp_read(offset, size, val); > > return val; > } > @@ -263,7 +265,7 @@ static const MemoryRegionOps grlib_apb_pnp_ops = > { > .write = grlib_apb_pnp_write, > .endianness = DEVICE_BIG_ENDIAN, > .impl = { > - .min_access_size = 4, > + .min_access_size = 1, > .max_access_size = 4, > }, > }; > diff --git a/hw/misc/trace-events b/hw/misc/trace-events > index 4d51a80de1d..c18bc0605e8 100644 > --- a/hw/misc/trace-events > +++ b/hw/misc/trace-events > @@ -247,8 +247,8 @@ via1_adb_poll(uint8_t data, const char *vadbint, int > status, int index, int size > via1_auxmode(int mode) "setting auxmode to %d" > > # grlib_ahb_apb_pnp.c > -grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read > addr:0x%03"PRIx64" data:0x%08x" > -grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read > addr:0x%03"PRIx64" data:0x%08x" > +grlib_ahb_pnp_read(uint64_t addr, unsigned size, uint32_t value) "AHB PnP > read addr:0x%03"PRIx64" size:%u data:0x%08x" > +grlib_apb_pnp_read(uint64_t addr, unsigned size, uint32_t value) "APB PnP > read addr:0x%03"PRIx64" size:%u data:0x%08x" > > # led.c > led_set_intensity(const char *color, const char *desc, uint8_t > intensity_percent) "LED desc:'%s' color:%s intensity: %u%%" > -- > 2.25.1 >
