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

[Philippe Mathieu-Daudé]

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>

>          if (!arm_feature(&s->cpu->env, ARM_FEATURE_M_MAIN)) {
>              return MEMTX_OK;
>          }
>