| Message ID | 20240221130823.677762-7-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm: Implement FEAT_NMI and FEAT_GICv3_NMI | expand |
On 2/21/24 03:08, Jinjie Ruan via wrote: > This only implements the external delivery method via the GICv3. > > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > target/arm/cpu-qom.h | 3 ++- > target/arm/cpu.c | 39 ++++++++++++++++++++++++++++++++++----- > target/arm/cpu.h | 2 ++ > target/arm/helper.c | 1 + > 4 files changed, 39 insertions(+), 6 deletions(-) > > diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h > index 8e032691db..66d555a605 100644 > --- a/target/arm/cpu-qom.h > +++ b/target/arm/cpu-qom.h > @@ -36,11 +36,12 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU, > #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU > #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX) > > -/* Meanings of the ARMCPU object's four inbound GPIO lines */ > +/* Meanings of the ARMCPU object's five inbound GPIO lines */ > #define ARM_CPU_IRQ 0 > #define ARM_CPU_FIQ 1 > #define ARM_CPU_VIRQ 2 > #define ARM_CPU_VFIQ 3 > +#define ARM_CPU_NMI 4 > > /* For M profile, some registers are banked secure vs non-secure; > * these are represented as a 2-element array where the first element > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 5e5978c302..055670343e 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -128,7 +128,7 @@ static bool arm_cpu_has_work(CPUState *cs) > > return (cpu->power_state != PSCI_OFF) > && cs->interrupt_request & > - (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD > + (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI > | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ | CPU_INTERRUPT_VSERR > | CPU_INTERRUPT_EXITTB); > } I think you should not include CPU_INTERRUPT_NMI when it cannot be delivered, e.g. FEAT_NMI not enabled. > @@ -668,6 +668,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx, > CPUARMState *env = cpu_env(cs); > bool pstate_unmasked; > bool unmasked = false; > + bool nmi_unmasked = false; > > /* > * Don't take exceptions if they target a lower EL. > @@ -678,13 +679,29 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx, > return false; > } > > + nmi_unmasked = (!(env->allint & PSTATE_ALLINT)) & > + (!((env->cp15.sctlr_el[target_el] & SCTLR_SPINTMASK) && > + (env->pstate & PSTATE_SP) && cur_el == target_el)); I don't see SCTLR_ELx.NMI being tested anywhere, which is required to enable everything else. > case EXCP_FIQ: > - pstate_unmasked = !(env->daif & PSTATE_F); > + if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) { > + pstate_unmasked = (!(env->daif & PSTATE_F)) & nmi_unmasked; > + } else { > + pstate_unmasked = !(env->daif & PSTATE_F); > + } > break; > > case EXCP_IRQ: > - pstate_unmasked = !(env->daif & PSTATE_I); > + if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) { > + pstate_unmasked = (!(env->daif & PSTATE_I)) & nmi_unmasked; > + } else { > + pstate_unmasked = !(env->daif & PSTATE_I); > + } > break; I don't see what this is doing. While Superpriority is IMPLEMENTATION DEFINED, how are you defining it for QEMU? Is there a definition from real hw which makes sense under emulation? r~
On 2/21/24 03:08, Jinjie Ruan via wrote: > This only implements the external delivery method via the GICv3. > > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > target/arm/cpu-qom.h | 3 ++- > target/arm/cpu.c | 39 ++++++++++++++++++++++++++++++++++----- > target/arm/cpu.h | 2 ++ > target/arm/helper.c | 1 + > 4 files changed, 39 insertions(+), 6 deletions(-) > > diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h > index 8e032691db..66d555a605 100644 > --- a/target/arm/cpu-qom.h > +++ b/target/arm/cpu-qom.h > @@ -36,11 +36,12 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU, > #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU > #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX) > > -/* Meanings of the ARMCPU object's four inbound GPIO lines */ > +/* Meanings of the ARMCPU object's five inbound GPIO lines */ > #define ARM_CPU_IRQ 0 > #define ARM_CPU_FIQ 1 > #define ARM_CPU_VIRQ 2 > #define ARM_CPU_VFIQ 3 > +#define ARM_CPU_NMI 4 You need a 6th GPIO for vNMI. r~
On 2024/2/22 4:06, Richard Henderson wrote: > On 2/21/24 03:08, Jinjie Ruan via wrote: >> This only implements the external delivery method via the GICv3. >> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> >> --- >> target/arm/cpu-qom.h | 3 ++- >> target/arm/cpu.c | 39 ++++++++++++++++++++++++++++++++++----- >> target/arm/cpu.h | 2 ++ >> target/arm/helper.c | 1 + >> 4 files changed, 39 insertions(+), 6 deletions(-) >> >> diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h >> index 8e032691db..66d555a605 100644 >> --- a/target/arm/cpu-qom.h >> +++ b/target/arm/cpu-qom.h >> @@ -36,11 +36,12 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU, >> #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU >> #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX) >> -/* Meanings of the ARMCPU object's four inbound GPIO lines */ >> +/* Meanings of the ARMCPU object's five inbound GPIO lines */ >> #define ARM_CPU_IRQ 0 >> #define ARM_CPU_FIQ 1 >> #define ARM_CPU_VIRQ 2 >> #define ARM_CPU_VFIQ 3 >> +#define ARM_CPU_NMI 4 >> /* For M profile, some registers are banked secure vs non-secure; >> * these are represented as a 2-element array where the first element >> diff --git a/target/arm/cpu.c b/target/arm/cpu.c >> index 5e5978c302..055670343e 100644 >> --- a/target/arm/cpu.c >> +++ b/target/arm/cpu.c >> @@ -128,7 +128,7 @@ static bool arm_cpu_has_work(CPUState *cs) >> return (cpu->power_state != PSCI_OFF) >> && cs->interrupt_request & >> - (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD >> + (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI >> | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ | >> CPU_INTERRUPT_VSERR >> | CPU_INTERRUPT_EXITTB); >> } > > I think you should not include CPU_INTERRUPT_NMI when it cannot be > delivered, e.g. FEAT_NMI not enabled. OK! I'll fix it. > > >> @@ -668,6 +668,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, >> unsigned int excp_idx, >> CPUARMState *env = cpu_env(cs); >> bool pstate_unmasked; >> bool unmasked = false; >> + bool nmi_unmasked = false; >> /* >> * Don't take exceptions if they target a lower EL. >> @@ -678,13 +679,29 @@ static inline bool arm_excp_unmasked(CPUState >> *cs, unsigned int excp_idx, >> return false; >> } >> + nmi_unmasked = (!(env->allint & PSTATE_ALLINT)) & >> + (!((env->cp15.sctlr_el[target_el] & >> SCTLR_SPINTMASK) && >> + (env->pstate & PSTATE_SP) && cur_el == target_el)); > > I don't see SCTLR_ELx.NMI being tested anywhere, which is required to > enable everything else. OK! I'll fix it. > >> case EXCP_FIQ: >> - pstate_unmasked = !(env->daif & PSTATE_F); >> + if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) { >> + pstate_unmasked = (!(env->daif & PSTATE_F)) & nmi_unmasked; >> + } else { >> + pstate_unmasked = !(env->daif & PSTATE_F); >> + } >> break; >> case EXCP_IRQ: >> - pstate_unmasked = !(env->daif & PSTATE_I); >> + if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) { >> + pstate_unmasked = (!(env->daif & PSTATE_I)) & nmi_unmasked; >> + } else { >> + pstate_unmasked = !(env->daif & PSTATE_I); >> + } >> break; > > I don't see what this is doing. While Superpriority is IMPLEMENTATION > DEFINED, how are you defining it for QEMU? Is there a definition from > real hw which makes sense under emulation? These code add allint mask for IRQ or FIQ,since ALLINT also mask them. The superpriority is DEFINED in gicv3 code and it is a NMI exception for PE. > > > r~
On 2024/2/22 5:23, Richard Henderson wrote: > On 2/21/24 03:08, Jinjie Ruan via wrote: >> This only implements the external delivery method via the GICv3. >> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> >> --- >> target/arm/cpu-qom.h | 3 ++- >> target/arm/cpu.c | 39 ++++++++++++++++++++++++++++++++++----- >> target/arm/cpu.h | 2 ++ >> target/arm/helper.c | 1 + >> 4 files changed, 39 insertions(+), 6 deletions(-) >> >> diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h >> index 8e032691db..66d555a605 100644 >> --- a/target/arm/cpu-qom.h >> +++ b/target/arm/cpu-qom.h >> @@ -36,11 +36,12 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU, >> #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU >> #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX) >> -/* Meanings of the ARMCPU object's four inbound GPIO lines */ >> +/* Meanings of the ARMCPU object's five inbound GPIO lines */ >> #define ARM_CPU_IRQ 0 >> #define ARM_CPU_FIQ 1 >> #define ARM_CPU_VIRQ 2 >> #define ARM_CPU_VFIQ 3 >> +#define ARM_CPU_NMI 4 > > You need a 6th GPIO for vNMI. Thank you! I'll fix it. > > > r~
On 2024/2/22 4:06, Richard Henderson wrote: > On 2/21/24 03:08, Jinjie Ruan via wrote: >> This only implements the external delivery method via the GICv3. >> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> >> --- >> target/arm/cpu-qom.h | 3 ++- >> target/arm/cpu.c | 39 ++++++++++++++++++++++++++++++++++----- >> target/arm/cpu.h | 2 ++ >> target/arm/helper.c | 1 + >> 4 files changed, 39 insertions(+), 6 deletions(-) >> >> diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h >> index 8e032691db..66d555a605 100644 >> --- a/target/arm/cpu-qom.h >> +++ b/target/arm/cpu-qom.h >> @@ -36,11 +36,12 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU, >> #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU >> #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX) >> -/* Meanings of the ARMCPU object's four inbound GPIO lines */ >> +/* Meanings of the ARMCPU object's five inbound GPIO lines */ >> #define ARM_CPU_IRQ 0 >> #define ARM_CPU_FIQ 1 >> #define ARM_CPU_VIRQ 2 >> #define ARM_CPU_VFIQ 3 >> +#define ARM_CPU_NMI 4 >> /* For M profile, some registers are banked secure vs non-secure; >> * these are represented as a 2-element array where the first element >> diff --git a/target/arm/cpu.c b/target/arm/cpu.c >> index 5e5978c302..055670343e 100644 >> --- a/target/arm/cpu.c >> +++ b/target/arm/cpu.c >> @@ -128,7 +128,7 @@ static bool arm_cpu_has_work(CPUState *cs) >> return (cpu->power_state != PSCI_OFF) >> && cs->interrupt_request & >> - (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD >> + (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI >> | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ | >> CPU_INTERRUPT_VSERR >> | CPU_INTERRUPT_EXITTB); >> } > > I think you should not include CPU_INTERRUPT_NMI when it cannot be > delivered, e.g. FEAT_NMI not enabled. I'll fix it. > > >> @@ -668,6 +668,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, >> unsigned int excp_idx, >> CPUARMState *env = cpu_env(cs); >> bool pstate_unmasked; >> bool unmasked = false; >> + bool nmi_unmasked = false; >> /* >> * Don't take exceptions if they target a lower EL. >> @@ -678,13 +679,29 @@ static inline bool arm_excp_unmasked(CPUState >> *cs, unsigned int excp_idx, >> return false; >> } >> + nmi_unmasked = (!(env->allint & PSTATE_ALLINT)) & >> + (!((env->cp15.sctlr_el[target_el] & >> SCTLR_SPINTMASK) && >> + (env->pstate & PSTATE_SP) && cur_el == target_el)); > > I don't see SCTLR_ELx.NMI being tested anywhere, which is required to > enable everything else. I'll Add it > >> case EXCP_FIQ: >> - pstate_unmasked = !(env->daif & PSTATE_F); >> + if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) { >> + pstate_unmasked = (!(env->daif & PSTATE_F)) & nmi_unmasked; >> + } else { >> + pstate_unmasked = !(env->daif & PSTATE_F); >> + } >> break; >> case EXCP_IRQ: >> - pstate_unmasked = !(env->daif & PSTATE_I); >> + if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) { >> + pstate_unmasked = (!(env->daif & PSTATE_I)) & nmi_unmasked; >> + } else { >> + pstate_unmasked = !(env->daif & PSTATE_I); >> + } >> break; > > I don't see what this is doing. While Superpriority is IMPLEMENTATION > DEFINED, how are you defining it for QEMU? Is there a definition from > real hw which makes sense under emulation? > > > r~
diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h index 8e032691db..66d555a605 100644 --- a/target/arm/cpu-qom.h +++ b/target/arm/cpu-qom.h @@ -36,11 +36,12 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU, #define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX) -/* Meanings of the ARMCPU object's four inbound GPIO lines */ +/* Meanings of the ARMCPU object's five inbound GPIO lines */ #define ARM_CPU_IRQ 0 #define ARM_CPU_FIQ 1 #define ARM_CPU_VIRQ 2 #define ARM_CPU_VFIQ 3 +#define ARM_CPU_NMI 4 /* For M profile, some registers are banked secure vs non-secure; * these are represented as a 2-element array where the first element diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 5e5978c302..055670343e 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -128,7 +128,7 @@ static bool arm_cpu_has_work(CPUState *cs) return (cpu->power_state != PSCI_OFF) && cs->interrupt_request & - (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD + (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ | CPU_INTERRUPT_VSERR | CPU_INTERRUPT_EXITTB); } @@ -668,6 +668,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx, CPUARMState *env = cpu_env(cs); bool pstate_unmasked; bool unmasked = false; + bool nmi_unmasked = false; /* * Don't take exceptions if they target a lower EL. @@ -678,13 +679,29 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx, return false; } + nmi_unmasked = (!(env->allint & PSTATE_ALLINT)) & + (!((env->cp15.sctlr_el[target_el] & SCTLR_SPINTMASK) && + (env->pstate & PSTATE_SP) && cur_el == target_el)); + switch (excp_idx) { + case EXCP_NMI: + pstate_unmasked = nmi_unmasked; + break; + case EXCP_FIQ: - pstate_unmasked = !(env->daif & PSTATE_F); + if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) { + pstate_unmasked = (!(env->daif & PSTATE_F)) & nmi_unmasked; + } else { + pstate_unmasked = !(env->daif & PSTATE_F); + } break; case EXCP_IRQ: - pstate_unmasked = !(env->daif & PSTATE_I); + if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) { + pstate_unmasked = (!(env->daif & PSTATE_I)) & nmi_unmasked; + } else { + pstate_unmasked = !(env->daif & PSTATE_I); + } break; case EXCP_VFIQ: @@ -804,6 +821,16 @@ static bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request) /* The prioritization of interrupts is IMPLEMENTATION DEFINED. */ + if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) { + if (interrupt_request & CPU_INTERRUPT_NMI) { + excp_idx = EXCP_NMI; + target_el = arm_phys_excp_target_el(cs, excp_idx, cur_el, secure); + if (arm_excp_unmasked(cs, excp_idx, target_el, + cur_el, secure, hcr_el2)) { + goto found; + } + } + } if (interrupt_request & CPU_INTERRUPT_FIQ) { excp_idx = EXCP_FIQ; target_el = arm_phys_excp_target_el(cs, excp_idx, cur_el, secure); @@ -929,7 +956,8 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level) [ARM_CPU_IRQ] = CPU_INTERRUPT_HARD, [ARM_CPU_FIQ] = CPU_INTERRUPT_FIQ, [ARM_CPU_VIRQ] = CPU_INTERRUPT_VIRQ, - [ARM_CPU_VFIQ] = CPU_INTERRUPT_VFIQ + [ARM_CPU_VFIQ] = CPU_INTERRUPT_VFIQ, + [ARM_CPU_NMI] = CPU_INTERRUPT_NMI }; if (!arm_feature(env, ARM_FEATURE_EL2) && @@ -957,6 +985,7 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level) break; case ARM_CPU_IRQ: case ARM_CPU_FIQ: + case ARM_CPU_NMI: if (level) { cpu_interrupt(cs, mask[irq]); } else { @@ -1358,7 +1387,7 @@ static void arm_cpu_initfn(Object *obj) */ qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 4); } else { - qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 4); + qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 5); } qdev_init_gpio_out(DEVICE(cpu), cpu->gt_timer_outputs, diff --git a/target/arm/cpu.h b/target/arm/cpu.h index f9646dbbfb..5257343bcb 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -60,6 +60,7 @@ #define EXCP_DIVBYZERO 23 /* v7M DIVBYZERO UsageFault */ #define EXCP_VSERR 24 #define EXCP_GPC 25 /* v9 Granule Protection Check Fault */ +#define EXCP_NMI 26 /* NB: add new EXCP_ defines to the array in arm_log_exception() too */ #define ARMV7M_EXCP_RESET 1 @@ -79,6 +80,7 @@ #define CPU_INTERRUPT_VIRQ CPU_INTERRUPT_TGT_EXT_2 #define CPU_INTERRUPT_VFIQ CPU_INTERRUPT_TGT_EXT_3 #define CPU_INTERRUPT_VSERR CPU_INTERRUPT_TGT_INT_0 +#define CPU_INTERRUPT_NMI CPU_INTERRUPT_TGT_EXT_4 /* The usual mapping for an AArch64 system register to its AArch32 * counterpart is for the 32 bit world to have access to the lower diff --git a/target/arm/helper.c b/target/arm/helper.c index 211156d640..bd7858e02e 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -10626,6 +10626,7 @@ void arm_log_exception(CPUState *cs) [EXCP_DIVBYZERO] = "v7M DIVBYZERO UsageFault", [EXCP_VSERR] = "Virtual SERR", [EXCP_GPC] = "Granule Protection Check", + [EXCP_NMI] = "NMI" }; if (idx >= 0 && idx < ARRAY_SIZE(excnames)) {
This only implements the external delivery method via the GICv3. Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- target/arm/cpu-qom.h | 3 ++- target/arm/cpu.c | 39 ++++++++++++++++++++++++++++++++++----- target/arm/cpu.h | 2 ++ target/arm/helper.c | 1 + 4 files changed, 39 insertions(+), 6 deletions(-)