| Message ID | 20201119215617.29887-22-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Implement v8.1M and Cortex-M55 | expand |
On 11/19/20 3:56 PM, Peter Maydell wrote: > + } else { > + /* BFHFNMIGN is RAZ/WI from NS if AIRCR.BFHFNMINS is 0 */ > + if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { > + value &= ~R_V7M_CCR_BFHFNMIGN_MASK; > + } This isn't WI, as you're zeroing the bit not preserving it. I think you need to merge the current state of the bit to preserve the value for secure state. r~
On Tue, 1 Dec 2020 at 15:16, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 11/19/20 3:56 PM, Peter Maydell wrote: > > + } else { > > + /* BFHFNMIGN is RAZ/WI from NS if AIRCR.BFHFNMINS is 0 */ > > + if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { > > + value &= ~R_V7M_CCR_BFHFNMIGN_MASK; > > + } > > This isn't WI, as you're zeroing the bit not preserving it. > I think you need to merge the current state of the bit to preserve the value > for secure state. Oops, yes. thanks -- PMM
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index effc4a784ca..deb4bd56c95 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -1100,6 +1100,12 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) */ val = cpu->env.v7m.ccr[attrs.secure]; val |= cpu->env.v7m.ccr[M_REG_NS] & R_V7M_CCR_BFHFNMIGN_MASK; + /* BFHFNMIGN is RAZ/WI from NS if AIRCR.BFHFNMINS is 0 */ + if (!attrs.secure) { + if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { + val &= ~R_V7M_CCR_BFHFNMIGN_MASK; + } + } return val; case 0xd24: /* System Handler Control and State (SHCSR) */ if (!arm_feature(&cpu->env, ARM_FEATURE_V7)) { @@ -1662,6 +1668,11 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, (cpu->env.v7m.ccr[M_REG_NS] & ~R_V7M_CCR_BFHFNMIGN_MASK) | (value & R_V7M_CCR_BFHFNMIGN_MASK); value &= ~R_V7M_CCR_BFHFNMIGN_MASK; + } else { + /* BFHFNMIGN is RAZ/WI from NS if AIRCR.BFHFNMINS is 0 */ + if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { + value &= ~R_V7M_CCR_BFHFNMIGN_MASK; + } } cpu->env.v7m.ccr[attrs.secure] = value;
The CCR is a register most of whose bits are banked between security states but where BFHFNMIGN is not, and we keep it in the non-secure entry of the v7m.ccr[] array. The logic which tries to handle this bit fails to implement the "RAZ/WI from Nonsecure if AIRCR.BFHFNMINS is zero" requirement; correct the omission. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/armv7m_nvic.c | 11 +++++++++++ 1 file changed, 11 insertions(+)