Re: [Qemu-devel] [PATCH v2 02/11] hw/intc/armv7m_nvic: Fix ICSR PENDNMISET/CLR handling

[Richard Henderson]

On 02/09/2018 08:58 AM, Peter Maydell wrote:
> The PENDNMISET/CLR bits in the ICSR should be RAZ/WI from
> NonSecure state if the AIRCR.BFHFNMINS bit is zero. We had
> misimplemented this as making the bits RAZ/WI from both
> Secure and NonSecure states. Fix this bug by checking
> attrs.secure so that Secure code can pend and unpend NMIs.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/intc/armv7m_nvic.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH v2 02/11] hw/intc/armv7m_nvic: Fix ICSR PENDNMISET/CLR handling

[Peter Maydell]

The PENDNMISET/CLR bits in the ICSR should be RAZ/WI from
NonSecure state if the AIRCR.BFHFNMINS bit is zero. We had
misimplemented this as making the bits RAZ/WI from both
Secure and NonSecure states. Fix this bug by checking
attrs.secure so that Secure code can pend and unpend NMIs.

Signed-off-by: Peter Maydell 
---
 hw/intc/armv7m_nvic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 63da0fee34..06b9598fbe 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -830,8 +830,8 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, 
MemTxAttrs attrs)
 }
 }
 /* NMIPENDSET */
-if ((cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) &&
-s->vectors[ARMV7M_EXCP_NMI].pending) {
+if ((attrs.secure || (cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK))
+&& s->vectors[ARMV7M_EXCP_NMI].pending) {
 val |= (1 << 31);
 }
 /* ISRPREEMPT: RES0 when halting debug not implemented */
@@ -1193,7 +1193,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 break;
 }
 case 0xd04: /* Interrupt Control State (ICSR) */
-if (cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) {
+if (attrs.secure || cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) {
 if (value & (1 << 31)) {
 armv7m_nvic_set_pending(s, ARMV7M_EXCP_NMI, false);
 } else if (value & (1 << 30) &&
-- 
2.16.1