Re: [Qemu-devel] [PATCH] hw/intc/armv7m_nvic: Allow byte accesses to SHPR1

[Peter Maydell]

On Tue, 12 Feb 2019 at 18:25, Philippe Mathieu-Daudé <address@hidden> wrote:
>
> Hi Peter,
>
> On 2/12/19 7:19 PM, Peter Maydell wrote:
> > The code for handling the NVIC SHPR1 register intends to permit
> > byte and halfword accesses (as the architecture requires). However
> > the 'case' line for it only lists the base address of the
> > register, so attempts to access bytes other than the first one
> > end up in the "bad write" default logic. This bug was added
> > accidentally when we split out the SHPR1 logic from SHPR2 and
> > SHPR3 to support v6M.
> >
> > Fixes: 7c9140afd594 ("nvic: Handle ARMv6-M SCS reserved registers")
> > Signed-off-by: Peter Maydell <address@hidden>
> > ---
> > The Zephyr RTOS happens to access SHPR1 byte at a time,
> > which is how I spotted this.
> > ---
> >  hw/intc/armv7m_nvic.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> > index 790a3d95849..2fd40f9dc4c 100644
> > --- a/hw/intc/armv7m_nvic.c
> > +++ b/hw/intc/armv7m_nvic.c
> > @@ -1956,7 +1956,7 @@ static MemTxResult nvic_sysreg_write(void *opaque, 
> > hwaddr addr,
> >          }
> >          nvic_irq_update(s);
> >          return MEMTX_OK;
> > -    case 0xd18: /* System Handler Priority (SHPR1) */
> > +    case 0xd18 ... 0xd1b: /* System Handler Priority (SHPR1) */
>
> Can you fix the nvic_sysreg_read() case too?
>
> With that:
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

Thanks for the catch -- careless of me not to check the read code too.

-- PMM