On Fri, 22 Mar 2024 at 03:56, Jinjie Ruan <ruanjin...@huawei.com> wrote: > > > > On 2024/3/21 23:46, Peter Maydell wrote: > > Something somewhere needs to implement "if SCTLR_ELx.NMI is 0 then > > we don't take EXCP_VINMI etc but instead (maybe) EXCP_VIRQ etc". > > At the moment nothing does that: > > * arm_cpu_update_vinmi() doesn't look at the NMI bit before > > deciding whether to set CPU_INTERRUPT_VINMI > > * in arm_excp_unmasked() if NMI is 0 then allIntMask takes its > > default value of false and so arm_excp_unmasked() returns true, > > so VINMI is not masked > > * arm_cpu_exec_interrupt() doesn't look at the NMI bit before > > deciding whether to check the CPU_INTERRUPT_VINMI bit in > > interrupt_request > > > > So even if SCTLR_ELx.NMI is 0 we'll still try to take a VINMI > > if it's set up in the HCR_EL2 bits. > > > > However we do this the required behaviour is that if NMI is 0 > > then it is as if the interrupt doesn't have superpriority and > > it falls back to being handled as an ordinary IRQ, VIRQ, VFIQ etc. > > I think the best place to do this is probably here in > > arm_cpu_exec_interrupt() -- if SCTLR_ELx.NMI isn't set then > > treat the VFNMI bit like VFIQ, the VINMI bit like VIRQ, and > > the NMI bit like IRQ. > > A PE that implements FEAT_NMI and FEAT_GICv3 also implements > FEAT_GICv3_NMI. A PE that does not implement FEAT_NMI, does not > implement FEAT_GICv3_NMI. > > As the GIC side has checked the FEAT_GICv3_NMI is implemented or Not. So > if cpu_isar_feature(aa64_nmi, env_archcpu(env)) is false, the > FEAT_GICv3_NMI will also not implemented,the CPU_INTERRUPT_NMI/VINMI can > not come from GIC, so we only need to check cpu_isar_feature(aa64_nmi, > env_archcpu(env)) and SCTLR_ELx.NMI is set in hcr_write() and > hcrx_write() for CPU side.
If the CPU implements FEAT_NMI then the guest is allowed to write to HCRX_EL2.VINMI even if SCTLR_ELx.NMI is 0. (It might for example choose to set up HCRX_EL2.VINMI first and then set NMI = 1 afterwards.) So you can't fix this by checking for NMI in hcr_write/hcrx_write. If you wanted you could not set the VINMI etc bits in arm_cpu_update_vinmi() and friends if SCTLR_ELx.NMI is 0, as long as you ensure that the update functions all get called again when the NMI bit is written and on exception handling change. -- PMM
