Add the NMIAR CPU interface registers which deal with acknowledging NMI.
When introduce NMI interrupt, there are some updates to the semantics for the
register ICC_IAR1_EL1 and ICC_HPPIR1_EL1. For ICC_IAR1_EL1 register, it
should return 1022 if the intid has super priority. And for ICC_NMIAR1_EL1
register, it should return 1023 if the intid do not have super priority.
Howerever, these are not necessary for ICC_HPPIR1_EL1 register.
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
hw/intc/arm_gicv3_cpuif.c | 46 ++++++++++++++++++++++++++++++++++++---
hw/intc/gicv3_internal.h | 1 +
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index e1a60d8c15..f5bf8df32b 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -1097,7 +1097,8 @@ static uint64_t icc_hppir0_value(GICv3CPUState *cs, CPUARMState *env)
return cs->hppi.irq;
}
-static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env)
+static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env,
+ bool is_nmi, bool is_hppi)
{
/* Return the highest priority pending interrupt register value
* for group 1.
@@ -1108,6 +1109,16 @@ static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env)
return INTID_SPURIOUS;
}
+ if (!is_hppi) {
+ if (is_nmi && (!cs->hppi.superprio)) {
+ return INTID_SPURIOUS;
+ }
+
+ if ((!is_nmi) && cs->hppi.superprio) {
+ return INTID_NMI;
+ }
+ }
+
/* Check whether we can return the interrupt or if we should return
* a special identifier, as per the CheckGroup1ForSpecialIdentifiers
* pseudocode. (We can simplify a little because for us ICC_SRE_EL1.RM
@@ -1168,7 +1179,30 @@ static uint64_t icc_iar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
if (!icc_hppi_can_preempt(cs)) {
intid = INTID_SPURIOUS;
} else {
- intid = icc_hppir1_value(cs, env);
+ intid = icc_hppir1_value(cs, env, false, false);
+ }
+
+ if (!gicv3_intid_is_special(intid)) {
+ icc_activate_irq(cs, intid);
+ }
+
+ trace_gicv3_icc_iar1_read(gicv3_redist_affid(cs), intid);
+ return intid;
+}
+
+static uint64_t icc_nmiar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+ GICv3CPUState *cs = icc_cs_from_env(env);
+ uint64_t intid;
+
+ if (icv_access(env, HCR_IMO)) {
+ return icv_iar_read(env, ri);
+ }
+
+ if (!icc_hppi_can_preempt(cs)) {
+ intid = INTID_SPURIOUS;
+ } else {
+ intid = icc_hppir1_value(cs, env, true, false);
}
if (!gicv3_intid_is_special(intid)) {
@@ -1555,7 +1589,7 @@ static uint64_t icc_hppir1_read(CPUARMState *env, const ARMCPRegInfo *ri)
return icv_hppir_read(env, ri);
}
- value = icc_hppir1_value(cs, env);
+ value = icc_hppir1_value(cs, env, false, true);
trace_gicv3_icc_hppir1_read(gicv3_redist_affid(cs), value);
return value;
}
@@ -2344,6 +2378,12 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = {
.access = PL1_R, .accessfn = gicv3_irq_access,
.readfn = icc_iar1_read,
},
+ { .name = "ICC_NMIAR1_EL1", .state = ARM_CP_STATE_BOTH,
+ .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 9, .opc2 = 5,
+ .type = ARM_CP_IO | ARM_CP_NO_RAW,
+ .access = PL1_R, .accessfn = gicv3_irq_access,
+ .readfn = icc_nmiar1_read,
+ },
{ .name = "ICC_EOIR1_EL1", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 1,
.type = ARM_CP_IO | ARM_CP_NO_RAW,
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 8d793243f4..93e56b3726 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -511,6 +511,7 @@ FIELD(VTE, RDBASE, 42, RDBASE_PROCNUM_LENGTH)
/* Special interrupt IDs */
#define INTID_SECURE 1020
#define INTID_NONSECURE 1021
+#define INTID_NMI 1022
#define INTID_SPURIOUS 1023
/* Functions internal to the emulated GICv3 */
--
2.34.1On 2/23/24 00:32, Jinjie Ruan via wrote:
> Add the NMIAR CPU interface registers which deal with acknowledging NMI.
>
> When introduce NMI interrupt, there are some updates to the semantics for the
> register ICC_IAR1_EL1 and ICC_HPPIR1_EL1. For ICC_IAR1_EL1 register, it
> should return 1022 if the intid has super priority. And for ICC_NMIAR1_EL1
> register, it should return 1023 if the intid do not have super priority.
> Howerever, these are not necessary for ICC_HPPIR1_EL1 register.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> hw/intc/arm_gicv3_cpuif.c | 46 ++++++++++++++++++++++++++++++++++++---
> hw/intc/gicv3_internal.h | 1 +
> 2 files changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> index e1a60d8c15..f5bf8df32b 100644
> --- a/hw/intc/arm_gicv3_cpuif.c
> +++ b/hw/intc/arm_gicv3_cpuif.c
> @@ -1097,7 +1097,8 @@ static uint64_t icc_hppir0_value(GICv3CPUState *cs, CPUARMState *env)
> return cs->hppi.irq;
> }
>
> -static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env)
> +static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env,
> + bool is_nmi, bool is_hppi)
> {
> /* Return the highest priority pending interrupt register value
> * for group 1.
> @@ -1108,6 +1109,16 @@ static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env)
> return INTID_SPURIOUS;
> }
>
> + if (!is_hppi) {
> + if (is_nmi && (!cs->hppi.superprio)) {
> + return INTID_SPURIOUS;
> + }
> +
> + if ((!is_nmi) && cs->hppi.superprio) {
> + return INTID_NMI;
> + }
> + }
> +
> /* Check whether we can return the interrupt or if we should return
> * a special identifier, as per the CheckGroup1ForSpecialIdentifiers
> * pseudocode. (We can simplify a little because for us ICC_SRE_EL1.RM
> @@ -1168,7 +1179,30 @@ static uint64_t icc_iar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
> if (!icc_hppi_can_preempt(cs)) {
> intid = INTID_SPURIOUS;
> } else {
> - intid = icc_hppir1_value(cs, env);
> + intid = icc_hppir1_value(cs, env, false, false);
> + }
> +
> + if (!gicv3_intid_is_special(intid)) {
> + icc_activate_irq(cs, intid);
> + }
> +
> + trace_gicv3_icc_iar1_read(gicv3_redist_affid(cs), intid);
> + return intid;
> +}
This is incorrect. For icc_iar1_read, you need something like
if (!is_hppi
&& cs->hppi.superprio
&& env->cp15.sctlr_el[current_el] & SCTLR_NMI) {
return INTID_NMI;
}
I think that if SCTLR_NMI is not set, the whole system ignores Superpriority entirely, so
returning SPURIOUS here would be incorrect. This would make sense, letting an OS that is
not configured for FEAT_NMI to run on ARMv8.8 hardware without modification.
> +
> +static uint64_t icc_nmiar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> + GICv3CPUState *cs = icc_cs_from_env(env);
> + uint64_t intid;
> +
> + if (icv_access(env, HCR_IMO)) {
> + return icv_iar_read(env, ri);
> + }
> +
> + if (!icc_hppi_can_preempt(cs)) {
> + intid = INTID_SPURIOUS;
> + } else {
> + intid = icc_hppir1_value(cs, env, true, false);
Here... believe that the result *should* only consider superpriority. I guess SPURIOUS is
the correct result when there is no pending interrupt with superpriority? It's really
unclear to me from the register description.
Peter?
> @@ -2344,6 +2378,12 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = {
> .access = PL1_R, .accessfn = gicv3_irq_access,
> .readfn = icc_iar1_read,
> },
> + { .name = "ICC_NMIAR1_EL1", .state = ARM_CP_STATE_BOTH,
> + .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 9, .opc2 = 5,
> + .type = ARM_CP_IO | ARM_CP_NO_RAW,
> + .access = PL1_R, .accessfn = gicv3_irq_access,
> + .readfn = icc_nmiar1_read,
> + },
This register is UNDEFINED if FEAT_GICv3_NMI is not implemented.
You need to register this separately.
r~
On Fri, 23 Feb 2024 at 20:53, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/23/24 00:32, Jinjie Ruan via wrote:
> > Add the NMIAR CPU interface registers which deal with acknowledging NMI.
> >
> > When introduce NMI interrupt, there are some updates to the semantics for the
> > register ICC_IAR1_EL1 and ICC_HPPIR1_EL1. For ICC_IAR1_EL1 register, it
> > should return 1022 if the intid has super priority. And for ICC_NMIAR1_EL1
> > register, it should return 1023 if the intid do not have super priority.
> > Howerever, these are not necessary for ICC_HPPIR1_EL1 register.
> >
> > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> > +static uint64_t icc_nmiar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
> > +{
> > + GICv3CPUState *cs = icc_cs_from_env(env);
> > + uint64_t intid;
> > +
> > + if (icv_access(env, HCR_IMO)) {
> > + return icv_iar_read(env, ri);
> > + }
> > +
> > + if (!icc_hppi_can_preempt(cs)) {
> > + intid = INTID_SPURIOUS;
> > + } else {
> > + intid = icc_hppir1_value(cs, env, true, false);
>
> Here... believe that the result *should* only consider superpriority. I guess SPURIOUS is
> the correct result when there is no pending interrupt with superpriority? It's really
> unclear to me from the register description.
Should be 1023: the ICC_NMIAR1_EL1[] pseudocode in the GIC
architecture spec (13.1.8) does this:
if !IsNMI(intID) then
return ZeroExtend(INTID_SPURIOUS);
(Note that the logic is "find the highest priority
pending interrupt, and then see if it is an NMI or not",
not "find the highest priority pending NMI".)
-- PMM
On 2024/2/24 4:52, Richard Henderson wrote:
> On 2/23/24 00:32, Jinjie Ruan via wrote:
>> Add the NMIAR CPU interface registers which deal with acknowledging NMI.
>>
>> When introduce NMI interrupt, there are some updates to the semantics
>> for the
>> register ICC_IAR1_EL1 and ICC_HPPIR1_EL1. For ICC_IAR1_EL1 register, it
>> should return 1022 if the intid has super priority. And for
>> ICC_NMIAR1_EL1
>> register, it should return 1023 if the intid do not have super priority.
>> Howerever, these are not necessary for ICC_HPPIR1_EL1 register.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>> hw/intc/arm_gicv3_cpuif.c | 46 ++++++++++++++++++++++++++++++++++++---
>> hw/intc/gicv3_internal.h | 1 +
>> 2 files changed, 44 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
>> index e1a60d8c15..f5bf8df32b 100644
>> --- a/hw/intc/arm_gicv3_cpuif.c
>> +++ b/hw/intc/arm_gicv3_cpuif.c
>> @@ -1097,7 +1097,8 @@ static uint64_t icc_hppir0_value(GICv3CPUState
>> *cs, CPUARMState *env)
>> return cs->hppi.irq;
>> }
>> -static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env)
>> +static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env,
>> + bool is_nmi, bool is_hppi)
>> {
>> /* Return the highest priority pending interrupt register value
>> * for group 1.
>> @@ -1108,6 +1109,16 @@ static uint64_t icc_hppir1_value(GICv3CPUState
>> *cs, CPUARMState *env)
>> return INTID_SPURIOUS;
>> }
>> + if (!is_hppi) {
>> + if (is_nmi && (!cs->hppi.superprio)) {
>> + return INTID_SPURIOUS;
>> + }
>> +
>> + if ((!is_nmi) && cs->hppi.superprio) {
>> + return INTID_NMI;
>> + }
>> + }
>> +
>> /* Check whether we can return the interrupt or if we should return
>> * a special identifier, as per the
>> CheckGroup1ForSpecialIdentifiers
>> * pseudocode. (We can simplify a little because for us
>> ICC_SRE_EL1.RM
>> @@ -1168,7 +1179,30 @@ static uint64_t icc_iar1_read(CPUARMState *env,
>> const ARMCPRegInfo *ri)
>> if (!icc_hppi_can_preempt(cs)) {
>> intid = INTID_SPURIOUS;
>> } else {
>> - intid = icc_hppir1_value(cs, env);
>> + intid = icc_hppir1_value(cs, env, false, false);
>> + }
>> +
>> + if (!gicv3_intid_is_special(intid)) {
>> + icc_activate_irq(cs, intid);
>> + }
>> +
>> + trace_gicv3_icc_iar1_read(gicv3_redist_affid(cs), intid);
>> + return intid;
>> +}
>
> This is incorrect. For icc_iar1_read, you need something like
>
> if (!is_hppi
> && cs->hppi.superprio
> && env->cp15.sctlr_el[current_el] & SCTLR_NMI) {
> return INTID_NMI;
> }
>
> I think that if SCTLR_NMI is not set, the whole system ignores
> Superpriority entirely, so returning SPURIOUS here would be incorrect.
> This would make sense, letting an OS that is not configured for FEAT_NMI
> to run on ARMv8.8 hardware without modification.
You are right. SCTLR_ELx.NMI decide whether IRQ and FIQ interrupts to
have Superpriority as an additional attribute.
>
>
>> +
>> +static uint64_t icc_nmiar1_read(CPUARMState *env, const ARMCPRegInfo
>> *ri)
>> +{
>> + GICv3CPUState *cs = icc_cs_from_env(env);
>> + uint64_t intid;
>> +
>> + if (icv_access(env, HCR_IMO)) {
>> + return icv_iar_read(env, ri);
>> + }
>> +
>> + if (!icc_hppi_can_preempt(cs)) {
>> + intid = INTID_SPURIOUS;
>> + } else {
>> + intid = icc_hppir1_value(cs, env, true, false);
>
> Here... believe that the result *should* only consider superpriority. I
> guess SPURIOUS is the correct result when there is no pending interrupt
> with superpriority? It's really unclear to me from the register
> description.
>
> Peter?
>
>> @@ -2344,6 +2378,12 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[]
>> = {
>> .access = PL1_R, .accessfn = gicv3_irq_access,
>> .readfn = icc_iar1_read,
>> },
>> + { .name = "ICC_NMIAR1_EL1", .state = ARM_CP_STATE_BOTH,
>> + .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 9, .opc2 = 5,
>> + .type = ARM_CP_IO | ARM_CP_NO_RAW,
>> + .access = PL1_R, .accessfn = gicv3_irq_access,
>> + .readfn = icc_nmiar1_read,
>> + },
>
> This register is UNDEFINED if FEAT_GICv3_NMI is not implemented.
> You need to register this separately.
>
>
> r~
© 2016 - 2026 Red Hat, Inc.