[RFC PATCH v8 00/23] target/arm: Implement FEAT_NMI and FEAT_GICv3_NMI

[RFC PATCH v8 00/23] target/arm: Implement FEAT_NMI and FEAT_GICv3_NMI

[Jinjie Ruan via]

This patch set implements FEAT_NMI and FEAT_GICv3_NMI for armv8. These
introduce support for a new category of interrupts in the architecture
which we can use to provide NMI like functionality.

There are two modes for using this FEAT_NMI. When PSTATE.ALLINT or
PSTATE.SP & SCTLR_ELx.SCTLR_SPINTMASK is set, any entry to ELx causes all
interrupts including those with superpriority to be masked on entry to ELn
until the mask is explicitly removed by software or hardware. PSTATE.ALLINT
can be managed by software using the new register control ALLINT.ALLINT.
Independent controls are provided for this feature at each EL, usage at EL1
should not disrupt EL2 or EL3.

I have tested it with the following linux patches which try to support
FEAT_NMI in linux kernel:

	https://lore.kernel.org/linux-arm-kernel/Y4sH5qX5bK9xfEBp@lpieralisi/T/#mb4ba4a2c045bf72c10c2202c1dd1b82d3240dc88

In the test, SGI, PPI and SPI interrupts can all be set to have super priority
to be converted to a hardware NMI interrupt. The SGI is tested with kernel
IPI as NMI framework, softlockup, hardlockup and kgdb test cases, and the PPI
interrupt is tested with "perf top" command with hardware NMI enabled, and
the SPI interrupt is tested with a custom test module, in which NMI interrupts
can be received and sent normally.

And the Virtual NMI(VNMI) SGI, PPI and SPI interrupt has also been tested
by accessing the GICD_INMIR, GICR_INMIR0, GICR_ISPENDR0 and GICD_ISPENDR
registers with devmem command.

         +-------------------------------------------------+
         |               Distributor                       |
         +-------------------------------------------------+
             SPI |  NMI                         |  NMI
                \/                            \/
            +--------+                     +-------+
            | Redist |                     | Redist|
            +--------+                     +-------+
            SGI  | NMI                     PPI | NMI
                \/                            \/
          +-------------+             +---------------+
          |CPU Interface|   ...       | CPU Interface |
          +-------------+             +---------------+
               | NMI                         | NMI
              \/                            \/
            +-----+                       +-----+
            |  PE |                       |  PE |
            +-----+                       +-----+

Changes in v8:
- Fix the rcu stall after sending a VNMI in qemu VM.
- Fix an unexpected interrupt bug when sending VNMI by running qemu VM.
- Test the VNMI interrupt.
- Update the commit message.
- Add Reviewed-by.

Changes in v7:
- env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
- Reorder the irqbetter() code for clarity.
- Eliminate the has_superprio local variable for gicv3_get_priority().
- false -> cs->hpplpi.superprio in gicv3_redist_update_noirqset().
- 0x0 -> false in arm_gicv3_common_reset_hold().
- Clear superprio in several places for hppi, hpplpi and hppvlpi.
- Add Reviewed-by.

Changes in v6:
- Fix DISAS_TOO_MANY to DISAS_UPDATE_EXIT for ALLINT MSR (immediate).
- Verify that HCR_EL2.VF is set before checking VFNMI.
- env->cp15.hcr_el2 -> arm_hcr_el2_eff().
- env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
- Not combine VFNMI with CPU_INTERRUPT_VNMI.
- Implement icv_nmiar1_read().
- Put the "extract superprio info" code into gicv3_get_priority().
- Update the comment in irqbetter().
- Reset the cs->hppi.superprio to 0x0.
- Set hppi.superprio to false for LPI.
- Add Reviewed-by.

Changes in v5:
- Remove the comment for ALLINT in cpu.h.
- Merge allint_check() to msr_i_allint to clear the ALLINT MSR (immediate)
  implementation.
- Rename msr_i_allint() to msr_set_allint_el1() to make it clearer.
- Drop the & 1 in trans_MSR_i_ALLINT().
- Add Reviewed-by.

Changes in v4:
- Handle VNMI within the CPU and the GIC.
- Keep PSTATE.ALLINT in env->pstate but not env->allint.
- Fix the ALLINT MSR (immediate) decodetree implementation.
- Accept NMI unconditionally for arm_cpu_has_work() but add comment.
- Improve nmi mask in arm_excp_unmasked().
- Make the GICR_INMIR0 and GICD_INMIR implementation more clearer.
- Improve ICC_NMIAR1_EL1 implementation
- Extract gicv3_get_priority() to avoid priority code repetition.
- Add Reviewed-by.

Changes in v3:
- Remove the FIQ NMI.
- Adjust the patches Sequence.
- Reomve the patch "Set pstate.ALLINT in arm_cpu_reset_hold".
- Check whether support FEAT_NMI and FEAT_GICv3 for FEAT_GICv3_NMI.
- With CPU_INTERRUPT_NMI, both CPSR_I and ISR_IS must be set.
- Not include NMI logic when FEAT_NMI or SCTLR_ELx.NMI not enabled.
- Refator nmi mask in arm_excp_unmasked().
- Add VNMI definitions, add HCRX_VINMI and HCRX_VFNMI support in HCRX_EL2.
- Add Reviewed-by and Acked-by.
- Update the commit message.

Changes in v2:
- Break up the patches so that each one does only one thing.
- Remove the command line option and just implement it in "max" cpu.

Jinjie Ruan (23):
  target/arm: Handle HCR_EL2 accesses for bits introduced with FEAT_NMI
  target/arm: Add PSTATE.ALLINT
  target/arm: Add support for FEAT_NMI, Non-maskable Interrupt
  target/arm: Implement ALLINT MSR (immediate)
  target/arm: Support MSR access to ALLINT
  target/arm: Add support for Non-maskable Interrupt
  target/arm: Add support for NMI in arm_phys_excp_target_el()
  target/arm: Handle IS/FS in ISR_EL1 for NMI
  target/arm: Handle PSTATE.ALLINT on taking an exception
  hw/arm/virt: Wire NMI and VNMI irq lines from GIC to CPU
  hw/intc/arm_gicv3: Add external IRQ lines for NMI
  target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64()
  hw/intc/arm_gicv3: Add irq superpriority information
  hw/intc/arm_gicv3_redist: Implement GICR_INMIR0
  hw/intc/arm_gicv3: Implement GICD_INMIR
  hw/intc: Enable FEAT_GICv3_NMI Feature
  hw/intc/arm_gicv3: Add NMI handling CPU interface registers
  hw/intc/arm_gicv3: Handle icv_nmiar1_read() for icc_nmiar1_read()
  hw/intc/arm_gicv3: Implement NMI interrupt prioirty
  hw/intc/arm_gicv3: Report the NMI interrupt in gicv3_cpuif_update()
  hw/intc/arm_gicv3: Report the VNMI interrupt
  target/arm: Add FEAT_NMI to max
  hw/arm/virt: Add FEAT_GICv3_NMI feature support in virt GIC

 docs/system/arm/emulation.rst      |   1 +
 hw/arm/virt.c                      |  25 +++++-
 hw/intc/arm_gicv3.c                |  69 ++++++++++++++--
 hw/intc/arm_gicv3_common.c         |  10 +++
 hw/intc/arm_gicv3_cpuif.c          | 127 ++++++++++++++++++++++++++---
 hw/intc/arm_gicv3_dist.c           |  36 ++++++++
 hw/intc/arm_gicv3_redist.c         |  22 +++++
 hw/intc/gicv3_internal.h           |   8 ++
 hw/intc/trace-events               |   2 +
 include/hw/intc/arm_gic_common.h   |   2 +
 include/hw/intc/arm_gicv3_common.h |   7 ++
 target/arm/cpu-features.h          |   5 ++
 target/arm/cpu-qom.h               |   4 +-
 target/arm/cpu.c                   |  85 +++++++++++++++++--
 target/arm/cpu.h                   |   7 ++
 target/arm/helper.c                |  67 +++++++++++++++
 target/arm/internals.h             |  12 +++
 target/arm/tcg/a64.decode          |   1 +
 target/arm/tcg/cpu64.c             |   1 +
 target/arm/tcg/helper-a64.c        |  12 +++
 target/arm/tcg/helper-a64.h        |   1 +
 target/arm/tcg/translate-a64.c     |  19 +++++
 22 files changed, 496 insertions(+), 27 deletions(-)

-- 
2.34.1

[RFC PATCH v8 01/23] target/arm: Handle HCR_EL2 accesses for bits introduced with FEAT_NMI

[Jinjie Ruan via]

FEAT_NMI defines another three new bits in HCRX_EL2: TALLINT, HCRX_VINMI and
HCRX_VFNMI. When the feature is enabled, allow these bits to be written in
HCRX_EL2.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v4:
- Update the comment for FEAT_NMI in hcrx_write().
- Update the commit message, s/thress/three/g.
v3:
- Add Reviewed-by.
- Add HCRX_VINMI and HCRX_VFNMI support in HCRX_EL2.
- Upate the commit messsage.
---
 target/arm/cpu-features.h | 5 +++++
 target/arm/helper.c       | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index e5758d9fbc..b300d0446d 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -681,6 +681,11 @@ static inline bool isar_feature_aa64_sme(const ARMISARegisters *id)
     return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, SME) != 0;
 }
 
+static inline bool isar_feature_aa64_nmi(const ARMISARegisters *id)
+{
+    return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, NMI) != 0;
+}
+
 static inline bool isar_feature_aa64_tgran4_lpa2(const ARMISARegisters *id)
 {
     return FIELD_SEX64(id->id_aa64mmfr0, ID_AA64MMFR0, TGRAN4) >= 1;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 3f3a5b55d4..b19a0178ce 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6190,6 +6190,11 @@ static void hcrx_write(CPUARMState *env, const ARMCPRegInfo *ri,
         valid_mask |= HCRX_MSCEN | HCRX_MCE2;
     }
 
+    /* FEAT_NMI adds TALLINT, VINMI and VFNMI */
+    if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) {
+        valid_mask |= HCRX_TALLINT | HCRX_VINMI | HCRX_VFNMI;
+    }
+
     /* Clear RES0 bits.  */
     env->cp15.hcrx_el2 = value & valid_mask;
 }
-- 
2.34.1

[RFC PATCH v8 02/23] target/arm: Add PSTATE.ALLINT

[Jinjie Ruan via]

When PSTATE.ALLINT is set, an IRQ or FIQ interrupt that is targeted to
ELx, with or without superpriority is masked.

As Richard suggested, place ALLINT bit in PSTATE in env->pstate.

With the change to pstate_read/write, exception entry
and return are automatically handled.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v5:
- Remove the ALLINT comment, as it is covered by "all other bits".
- Add Reviewed-by.
v4:
- Keep PSTATE.ALLINT in env->pstate but not env->allint.
- Update the commit message.
v3:
- Remove ALLINT dump in aarch64_cpu_dump_state().
- Update the commit message.
---
 target/arm/cpu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index bc0c84873f..de740d223f 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1430,6 +1430,7 @@ void pmu_init(ARMCPU *cpu);
 #define PSTATE_D (1U << 9)
 #define PSTATE_BTYPE (3U << 10)
 #define PSTATE_SSBS (1U << 12)
+#define PSTATE_ALLINT (1U << 13)
 #define PSTATE_IL (1U << 20)
 #define PSTATE_SS (1U << 21)
 #define PSTATE_PAN (1U << 22)
-- 
2.34.1

[RFC PATCH v8 03/23] target/arm: Add support for FEAT_NMI, Non-maskable Interrupt

[Jinjie Ruan via]

Add support for FEAT_NMI. NMI (FEAT_NMI) is an mandatory feature in
ARMv8.8-A and ARM v9.3-A.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v3:
- Add Reviewed-by.
- Adjust to before the MSR patches.
---
 target/arm/internals.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index dd3da211a3..516e0584bf 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1229,6 +1229,9 @@ static inline uint32_t aarch64_pstate_valid_mask(const ARMISARegisters *id)
     if (isar_feature_aa64_mte(id)) {
         valid |= PSTATE_TCO;
     }
+    if (isar_feature_aa64_nmi(id)) {
+        valid |= PSTATE_ALLINT;
+    }
 
     return valid;
 }
-- 
2.34.1

[RFC PATCH v8 04/23] target/arm: Implement ALLINT MSR (immediate)

[Jinjie Ruan via]

Add ALLINT MSR (immediate) to decodetree, in which the CRm is 0b000x. The
EL0 check is necessary to ALLINT, and the EL1 check is necessary when
imm == 1. So implement it inline for EL2/3, or EL1 with imm==0. Avoid the
unconditional write to pc and use raise_exception_ra to unwind.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v7:
- Add Reviewed-by.
v6:
- Fix DISAS_TOO_MANY to DISAS_UPDATE_EXIT and add the comment.
v5:
- Drop the & 1 in trans_MSR_i_ALLINT().
- Simplify and merge msr_i_allint() and allint_check().
- Rename msr_i_allint() to msr_set_allint_el1().
v4:
- Fix the ALLINT MSR (immediate) decodetree implementation.
- Remove arm_is_el2_enabled() check in allint_check().
- Update env->allint to env->pstate.
- Only call allint_check() when imm == 1.
- Simplify the allint_check() to not pass "op" and extract.
- Implement it inline for EL2/3, or EL1 with imm==0.
- Pass (a->imm & 1) * PSTATE_ALLINT (i64) to simplfy the ALLINT set/clear.
v3:
- Remove EL0 check in allint_check().
- Add TALLINT check for EL1 in allint_check().
- Remove unnecessarily arm_rebuild_hflags() in msr_i_allint helper.
---
 target/arm/tcg/a64.decode      |  1 +
 target/arm/tcg/helper-a64.c    | 12 ++++++++++++
 target/arm/tcg/helper-a64.h    |  1 +
 target/arm/tcg/translate-a64.c | 19 +++++++++++++++++++
 4 files changed, 33 insertions(+)

diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 8a20dce3c8..0e7656fd15 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -207,6 +207,7 @@ MSR_i_DIT       1101 0101 0000 0 011 0100 .... 010 11111 @msr_i
 MSR_i_TCO       1101 0101 0000 0 011 0100 .... 100 11111 @msr_i
 MSR_i_DAIFSET   1101 0101 0000 0 011 0100 .... 110 11111 @msr_i
 MSR_i_DAIFCLEAR 1101 0101 0000 0 011 0100 .... 111 11111 @msr_i
+MSR_i_ALLINT    1101 0101 0000 0 001 0100 000 imm:1 000 11111
 MSR_i_SVCR      1101 0101 0000 0 011 0100 0 mask:2 imm:1 011 11111
 
 # MRS, MSR (register), SYS, SYSL. These are all essentially the
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index ebaa7f00df..7818537890 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -66,6 +66,18 @@ void HELPER(msr_i_spsel)(CPUARMState *env, uint32_t imm)
     update_spsel(env, imm);
 }
 
+void HELPER(msr_set_allint_el1)(CPUARMState *env)
+{
+    /* ALLINT update to PSTATE. */
+    if (arm_hcrx_el2_eff(env) & HCRX_TALLINT) {
+        raise_exception_ra(env, EXCP_UDEF,
+                           syn_aa64_sysregtrap(0, 1, 0, 4, 1, 0x1f, 0),
+                           exception_target_el(env), GETPC());
+    }
+
+    env->pstate |= PSTATE_ALLINT;
+}
+
 static void daif_check(CPUARMState *env, uint32_t op,
                        uint32_t imm, uintptr_t ra)
 {
diff --git a/target/arm/tcg/helper-a64.h b/target/arm/tcg/helper-a64.h
index 575a5dab7d..0518165399 100644
--- a/target/arm/tcg/helper-a64.h
+++ b/target/arm/tcg/helper-a64.h
@@ -22,6 +22,7 @@ DEF_HELPER_FLAGS_1(rbit64, TCG_CALL_NO_RWG_SE, i64, i64)
 DEF_HELPER_2(msr_i_spsel, void, env, i32)
 DEF_HELPER_2(msr_i_daifset, void, env, i32)
 DEF_HELPER_2(msr_i_daifclear, void, env, i32)
+DEF_HELPER_1(msr_set_allint_el1, void, env)
 DEF_HELPER_3(vfp_cmph_a64, i64, f16, f16, ptr)
 DEF_HELPER_3(vfp_cmpeh_a64, i64, f16, f16, ptr)
 DEF_HELPER_3(vfp_cmps_a64, i64, f32, f32, ptr)
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 340265beb0..21758b290d 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -2036,6 +2036,25 @@ static bool trans_MSR_i_DAIFCLEAR(DisasContext *s, arg_i *a)
     return true;
 }
 
+static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a)
+{
+    if (!dc_isar_feature(aa64_nmi, s) || s->current_el == 0) {
+        return false;
+    }
+
+    if (a->imm == 0) {
+        clear_pstate_bits(PSTATE_ALLINT);
+    } else if (s->current_el > 1) {
+        set_pstate_bits(PSTATE_ALLINT);
+    } else {
+        gen_helper_msr_set_allint_el1(tcg_env);
+    }
+
+    /* Exit the cpu loop to re-evaluate pending IRQs. */
+    s->base.is_jmp = DISAS_UPDATE_EXIT;
+    return true;
+}
+
 static bool trans_MSR_i_SVCR(DisasContext *s, arg_MSR_i_SVCR *a)
 {
     if (!dc_isar_feature(aa64_sme, s) || a->mask == 0) {
-- 
2.34.1

[RFC PATCH v8 05/23] target/arm: Support MSR access to ALLINT

[Jinjie Ruan via]

Support ALLINT msr access as follow:
	mrs <xt>, ALLINT	// read allint
	msr ALLINT, <xt>	// write allint with imm

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v5:
- Add Reviewed-by.
v4:
- Remove arm_is_el2_enabled() check in allint_check().
- Change to env->pstate instead of env->allint.
v3:
- Remove EL0 check in aa64_allint_access() which alreay checks in .access
  PL1_RW.
- Use arm_hcrx_el2_eff() in aa64_allint_access() instead of env->cp15.hcrx_el2.
- Make ALLINT msr access function controlled by aa64_nmi.
---
 target/arm/helper.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index b19a0178ce..aa0151c775 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4752,6 +4752,36 @@ static void aa64_daif_write(CPUARMState *env, const ARMCPRegInfo *ri,
     env->daif = value & PSTATE_DAIF;
 }
 
+static void aa64_allint_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                              uint64_t value)
+{
+    env->pstate = (env->pstate & ~PSTATE_ALLINT) | (value & PSTATE_ALLINT);
+}
+
+static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return env->pstate & PSTATE_ALLINT;
+}
+
+static CPAccessResult aa64_allint_access(CPUARMState *env,
+                                         const ARMCPRegInfo *ri, bool isread)
+{
+    if (arm_current_el(env) == 1 && (arm_hcrx_el2_eff(env) & HCRX_TALLINT)) {
+        return CP_ACCESS_TRAP_EL2;
+    }
+    return CP_ACCESS_OK;
+}
+
+static const ARMCPRegInfo nmi_reginfo[] = {
+    { .name = "ALLINT", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .opc2 = 0, .crn = 4, .crm = 3,
+      .type = ARM_CP_NO_RAW,
+      .access = PL1_RW, .accessfn = aa64_allint_access,
+      .fieldoffset = offsetof(CPUARMState, pstate),
+      .writefn = aa64_allint_write, .readfn = aa64_allint_read,
+      .resetfn = arm_cp_reset_ignore },
+};
+
 static uint64_t aa64_pan_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     return env->pstate & PSTATE_PAN;
@@ -9889,6 +9919,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
     if (cpu_isar_feature(aa64_nv2, cpu)) {
         define_arm_cp_regs(cpu, nv2_reginfo);
     }
+
+    if (cpu_isar_feature(aa64_nmi, cpu)) {
+        define_arm_cp_regs(cpu, nmi_reginfo);
+    }
 #endif
 
     if (cpu_isar_feature(any_predinv, cpu)) {
-- 
2.34.1

[RFC PATCH v8 06/23] target/arm: Add support for Non-maskable Interrupt

[Jinjie Ruan via]

This only implements the external delivery method via the GICv3.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v8:
- Fix the rcu stall after sending a VNMI in qemu VM.
v7:
- Add Reviewed-by.
v6:
- env->cp15.hcr_el2 -> arm_hcr_el2_eff().
- env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
- Not include VF && VFNMI in CPU_INTERRUPT_VNMI.
v4:
- Accept NMI unconditionally for arm_cpu_has_work() but add comment.
- Change from & to && for EXCP_IRQ or EXCP_FIQ.
- Refator nmi mask in arm_excp_unmasked().
- Also handle VNMI in arm_cpu_exec_interrupt() and arm_cpu_set_irq().
- Rename virtual to Virtual.
v3:
- Not include CPU_INTERRUPT_NMI when FEAT_NMI not enabled
- Add ARM_CPU_VNMI.
- Refator nmi mask in arm_excp_unmasked().
- Test SCTLR_ELx.NMI for ALLINT mask for NMI.
---
 target/arm/cpu-qom.h   |  4 +-
 target/arm/cpu.c       | 85 +++++++++++++++++++++++++++++++++++++++---
 target/arm/cpu.h       |  4 ++
 target/arm/helper.c    |  2 +
 target/arm/internals.h |  9 +++++
 5 files changed, 97 insertions(+), 7 deletions(-)

diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 8e032691db..e0c9e18036 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -36,11 +36,13 @@ 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 six 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
+#define ARM_CPU_VNMI 5
 
 /* 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 ab8d007a86..f8d931a917 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -122,6 +122,13 @@ void arm_restore_state_to_opc(CPUState *cs,
 }
 #endif /* CONFIG_TCG */
 
+/*
+ * With SCTLR_ELx.NMI == 0, IRQ with Superpriority is masked identically with
+ * IRQ without Superpriority. Moreover, if the GIC is configured so that
+ * FEAT_GICv3_NMI is only set if FEAT_NMI is set, then we won't ever see
+ * CPU_INTERRUPT_*NMI anyway. So we might as well accept NMI here
+ * unconditionally.
+ */
 static bool arm_cpu_has_work(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -129,6 +136,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_NMI | CPU_INTERRUPT_VNMI
          | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ | CPU_INTERRUPT_VSERR
          | CPU_INTERRUPT_EXITTB);
 }
@@ -668,6 +676,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 allIntMask = false;
 
     /*
      * Don't take exceptions if they target a lower EL.
@@ -678,13 +687,31 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
         return false;
     }
 
+    if (cpu_isar_feature(aa64_nmi, env_archcpu(env)) &&
+        env->cp15.sctlr_el[target_el] & SCTLR_NMI && cur_el == target_el) {
+        allIntMask = env->pstate & PSTATE_ALLINT ||
+                     ((env->cp15.sctlr_el[target_el] & SCTLR_SPINTMASK) &&
+                      (env->pstate & PSTATE_SP));
+    }
+
     switch (excp_idx) {
+    case EXCP_NMI:
+        pstate_unmasked = !allIntMask;
+        break;
+
+    case EXCP_VNMI:
+        if ((!(hcr_el2 & HCR_IMO) && !(hcr_el2 & HCR_FMO)) ||
+             (hcr_el2 & HCR_TGE)) {
+            /* VNMIs(VIRQs or VFIQs) are only taken when hypervized.  */
+            return false;
+        }
+        return !allIntMask;
     case EXCP_FIQ:
-        pstate_unmasked = !(env->daif & PSTATE_F);
+        pstate_unmasked = (!(env->daif & PSTATE_F)) && (!allIntMask);
         break;
 
     case EXCP_IRQ:
-        pstate_unmasked = !(env->daif & PSTATE_I);
+        pstate_unmasked = (!(env->daif & PSTATE_I)) && (!allIntMask);
         break;
 
     case EXCP_VFIQ:
@@ -692,13 +719,13 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
             /* VFIQs are only taken when hypervized.  */
             return false;
         }
-        return !(env->daif & PSTATE_F);
+        return !(env->daif & PSTATE_F) && (!allIntMask);
     case EXCP_VIRQ:
         if (!(hcr_el2 & HCR_IMO) || (hcr_el2 & HCR_TGE)) {
             /* VIRQs are only taken when hypervized.  */
             return false;
         }
-        return !(env->daif & PSTATE_I);
+        return !(env->daif & PSTATE_I) && (!allIntMask);
     case EXCP_VSERR:
         if (!(hcr_el2 & HCR_AMO) || (hcr_el2 & HCR_TGE)) {
             /* VIRQs are only taken when hypervized.  */
@@ -804,6 +831,24 @@ 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_VNMI) {
+            excp_idx = EXCP_VNMI;
+            target_el = 1;
+            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);
@@ -900,6 +945,28 @@ void arm_cpu_update_vfiq(ARMCPU *cpu)
     }
 }
 
+void arm_cpu_update_vnmi(ARMCPU *cpu)
+{
+    /*
+     * Update the interrupt level for VNMI, which is the logical OR of
+     * the HCRX_EL2.VINMI bit and the input line level from the GIC.
+     */
+    CPUARMState *env = &cpu->env;
+    CPUState *cs = CPU(cpu);
+
+    bool new_state = ((arm_hcr_el2_eff(env) & HCR_VI) &&
+                      (arm_hcrx_el2_eff(env) & HCRX_VINMI)) ||
+        (env->irq_line_state & CPU_INTERRUPT_VNMI);
+
+    if (new_state != ((cs->interrupt_request & CPU_INTERRUPT_VNMI) != 0)) {
+        if (new_state) {
+            cpu_interrupt(cs, CPU_INTERRUPT_VNMI);
+        } else {
+            cpu_reset_interrupt(cs, CPU_INTERRUPT_VNMI);
+        }
+    }
+}
+
 void arm_cpu_update_vserr(ARMCPU *cpu)
 {
     /*
@@ -929,7 +996,9 @@ 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,
+        [ARM_CPU_VNMI] = CPU_INTERRUPT_VNMI
     };
 
     if (!arm_feature(env, ARM_FEATURE_EL2) &&
@@ -955,8 +1024,12 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
     case ARM_CPU_VFIQ:
         arm_cpu_update_vfiq(cpu);
         break;
+    case ARM_CPU_VNMI:
+        arm_cpu_update_vnmi(cpu);
+        break;
     case ARM_CPU_IRQ:
     case ARM_CPU_FIQ:
+    case ARM_CPU_NMI:
         if (level) {
             cpu_interrupt(cs, mask[irq]);
         } else {
@@ -1355,7 +1428,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, 6);
     }
 
     qdev_init_gpio_out(DEVICE(cpu), cpu->gt_timer_outputs,
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index de740d223f..629221e1a9 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -61,6 +61,8 @@
 #define EXCP_DIVBYZERO      23   /* v7M DIVBYZERO UsageFault */
 #define EXCP_VSERR          24
 #define EXCP_GPC            25   /* v9 Granule Protection Check Fault */
+#define EXCP_NMI            26
+#define EXCP_VNMI           27
 /* NB: add new EXCP_ defines to the array in arm_log_exception() too */
 
 #define ARMV7M_EXCP_RESET   1
@@ -80,6 +82,8 @@
 #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
+#define CPU_INTERRUPT_VNMI  CPU_INTERRUPT_TGT_EXT_0
 
 /* 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 aa0151c775..875a7fa8da 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10793,6 +10793,8 @@ void arm_log_exception(CPUState *cs)
             [EXCP_DIVBYZERO] = "v7M DIVBYZERO UsageFault",
             [EXCP_VSERR] = "Virtual SERR",
             [EXCP_GPC] = "Granule Protection Check",
+            [EXCP_NMI] = "NMI",
+            [EXCP_VNMI] = "Virtual NMI"
         };
 
         if (idx >= 0 && idx < ARRAY_SIZE(excnames)) {
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 516e0584bf..cb217a9ce7 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1109,6 +1109,15 @@ void arm_cpu_update_virq(ARMCPU *cpu);
  */
 void arm_cpu_update_vfiq(ARMCPU *cpu);
 
+/**
+ * arm_cpu_update_vnmi: Update CPU_INTERRUPT_VNMI bit in cs->interrupt_request
+ *
+ * Update the CPU_INTERRUPT_VNMI bit in cs->interrupt_request, following
+ * a change to either the input VNMI line from the GIC or the HCRX_EL2.VINMI.
+ * Must be called with the BQL held.
+ */
+void arm_cpu_update_vnmi(ARMCPU *cpu);
+
 /**
  * arm_cpu_update_vserr: Update CPU_INTERRUPT_VSERR bit
  *
-- 
2.34.1

[RFC PATCH v8 07/23] target/arm: Add support for NMI in arm_phys_excp_target_el()

[Jinjie Ruan via]

According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt
with superpriority is always IRQ, never FIQ, so handle NMI same as IRQ in
arm_phys_excp_target_el().

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v4:
- Add Reviewed-by.
v3:
- Remove nmi_is_irq flag in CPUARMState.
- Handle NMI same as IRQ in arm_phys_excp_target_el().
---
 target/arm/helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 875a7fa8da..5c637f3413 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10735,6 +10735,7 @@ uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
     hcr_el2 = arm_hcr_el2_eff(env);
     switch (excp_idx) {
     case EXCP_IRQ:
+    case EXCP_NMI:
         scr = ((env->cp15.scr_el3 & SCR_IRQ) == SCR_IRQ);
         hcr = hcr_el2 & HCR_IMO;
         break;
-- 
2.34.1

[RFC PATCH v8 08/23] target/arm: Handle IS/FS in ISR_EL1 for NMI

[Jinjie Ruan via]

Add IS and FS bit in ISR_EL1 and handle the read. With CPU_INTERRUPT_NMI or
CPU_INTERRUPT_VNMI, both CPSR_I and ISR_IS must be set. With
CPU_INTERRUPT_VFIQ and HCRX_EL2.VFNMI set, both CPSR_F and ISR_FS must be set.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v7:
- env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
- Add Reviewed-by.
v6:
- Verify that HCR_EL2.VF is set before checking VFNMI.
v4；
- Also handle VNMI.
v3:
- CPU_INTERRUPT_NMI do not set FIQ, so remove it.
- With CPU_INTERRUPT_NMI, both CPSR_I and ISR_IS must be set.
---
 target/arm/cpu.h    |  2 ++
 target/arm/helper.c | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 629221e1a9..fba0a364db 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1396,6 +1396,8 @@ void pmu_init(ARMCPU *cpu);
 #define CPSR_N (1U << 31)
 #define CPSR_NZCV (CPSR_N | CPSR_Z | CPSR_C | CPSR_V)
 #define CPSR_AIF (CPSR_A | CPSR_I | CPSR_F)
+#define ISR_FS (1U << 9)
+#define ISR_IS (1U << 10)
 
 #define CPSR_IT (CPSR_IT_0_1 | CPSR_IT_2_7)
 #define CACHED_CPSR_BITS (CPSR_T | CPSR_AIF | CPSR_GE | CPSR_IT | CPSR_Q \
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5c637f3413..4bc63bf7ca 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2021,15 +2021,29 @@ static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri)
         if (cs->interrupt_request & CPU_INTERRUPT_VIRQ) {
             ret |= CPSR_I;
         }
+        if (cs->interrupt_request & CPU_INTERRUPT_VNMI) {
+            ret |= ISR_IS;
+            ret |= CPSR_I;
+        }
     } else {
         if (cs->interrupt_request & CPU_INTERRUPT_HARD) {
             ret |= CPSR_I;
         }
+
+        if (cs->interrupt_request & CPU_INTERRUPT_NMI) {
+            ret |= ISR_IS;
+            ret |= CPSR_I;
+        }
     }
 
     if (hcr_el2 & HCR_FMO) {
         if (cs->interrupt_request & CPU_INTERRUPT_VFIQ) {
             ret |= CPSR_F;
+
+            if ((arm_hcr_el2_eff(env) & HCR_VF) &&
+                (arm_hcrx_el2_eff(env) & HCRX_VFNMI)) {
+                ret |= ISR_FS;
+            }
         }
     } else {
         if (cs->interrupt_request & CPU_INTERRUPT_FIQ) {
-- 
2.34.1

[RFC PATCH v8 09/23] target/arm: Handle PSTATE.ALLINT on taking an exception

[Jinjie Ruan via]

Set or clear PSTATE.ALLINT on taking an exception to ELx according to the
SCTLR_ELx.SPINTMASK bit.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v3:
- Add Reviewed-by.
---
 target/arm/helper.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 4bc63bf7ca..81f4a8f194 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11705,6 +11705,15 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
         }
     }
 
+    if (cpu_isar_feature(aa64_nmi, cpu) &&
+        (env->cp15.sctlr_el[new_el] & SCTLR_NMI)) {
+        if (!(env->cp15.sctlr_el[new_el] & SCTLR_SPINTMASK)) {
+            new_mode |= PSTATE_ALLINT;
+        } else {
+            new_mode &= ~PSTATE_ALLINT;
+        }
+    }
+
     pstate_write(env, PSTATE_DAIF | new_mode);
     env->aarch64 = true;
     aarch64_restore_sp(env, new_el);
-- 
2.34.1

[RFC PATCH v8 10/23] hw/arm/virt: Wire NMI and VNMI irq lines from GIC to CPU

[Jinjie Ruan via]

Wire the new NMI and VNMI interrupt line from the GIC to each CPU.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v4:
- Add Reviewed-by.
v3:
- Also add VNMI wire.
---
 hw/arm/virt.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e5cd935232..dc7959e164 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -821,7 +821,8 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
 
     /* Wire the outputs from each CPU's generic timer and the GICv3
      * maintenance interrupt signal to the appropriate GIC PPI inputs,
-     * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
+     * and the GIC's IRQ/FIQ/VIRQ/VFIQ/NMI/VNMI interrupt outputs to the
+     * CPU's inputs.
      */
     for (i = 0; i < smp_cpus; i++) {
         DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
@@ -865,6 +866,10 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
                            qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
         sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
                            qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+        sysbus_connect_irq(gicbusdev, i + 4 * smp_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_NMI));
+        sysbus_connect_irq(gicbusdev, i + 5 * smp_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VNMI));
     }
 
     fdt_add_gic_node(vms);
-- 
2.34.1

[RFC PATCH v8 11/23] hw/intc/arm_gicv3: Add external IRQ lines for NMI

[Jinjie Ruan via]

Augment the GICv3's QOM device interface by adding one
new set of sysbus IRQ line, to signal NMI to each CPU.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v4:
- Add Reviewed-by.
v3:
- Add support for VNMI.
---
 hw/intc/arm_gicv3_common.c         | 6 ++++++
 include/hw/intc/arm_gic_common.h   | 2 ++
 include/hw/intc/arm_gicv3_common.h | 2 ++
 3 files changed, 10 insertions(+)

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index cb55c72681..c52f060026 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -299,6 +299,12 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
     for (i = 0; i < s->num_cpu; i++) {
         sysbus_init_irq(sbd, &s->cpu[i].parent_vfiq);
     }
+    for (i = 0; i < s->num_cpu; i++) {
+        sysbus_init_irq(sbd, &s->cpu[i].parent_nmi);
+    }
+    for (i = 0; i < s->num_cpu; i++) {
+        sysbus_init_irq(sbd, &s->cpu[i].parent_vnmi);
+    }
 
     memory_region_init_io(&s->iomem_dist, OBJECT(s), ops, s,
                           "gicv3_dist", 0x10000);
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index 7080375008..97fea4102d 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -71,6 +71,8 @@ struct GICState {
     qemu_irq parent_fiq[GIC_NCPU];
     qemu_irq parent_virq[GIC_NCPU];
     qemu_irq parent_vfiq[GIC_NCPU];
+    qemu_irq parent_nmi[GIC_NCPU];
+    qemu_irq parent_vnmi[GIC_NCPU];
     qemu_irq maintenance_irq[GIC_NCPU];
 
     /* GICD_CTLR; for a GIC with the security extensions the NS banked version
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index 4e2fb518e7..7324c7d983 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -155,6 +155,8 @@ struct GICv3CPUState {
     qemu_irq parent_fiq;
     qemu_irq parent_virq;
     qemu_irq parent_vfiq;
+    qemu_irq parent_nmi;
+    qemu_irq parent_vnmi;
 
     /* Redistributor */
     uint32_t level;                  /* Current IRQ level */
-- 
2.34.1

[RFC PATCH v8 12/23] target/arm: Handle NMI in arm_cpu_do_interrupt_aarch64()

[Jinjie Ruan via]

According to Arm GIC section 4.6.3 Interrupt superpriority, the interrupt
with superpriority is always IRQ, never FIQ, so the NMI exception trap entry
behave like IRQ. And VNMI(vIRQ with Superpriority) can be raised from the
GIC or come from the hcrx_el2.HCRX_VINMI bit.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v7:
- Add Reviewed-by.
v6:
- Not combine VFNMI with CPU_INTERRUPT_VNMI.
v4:
- Also handle VNMI in arm_cpu_do_interrupt_aarch64().
v3:
- Remove the FIQ NMI handle.
---
 target/arm/helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 81f4a8f194..e279b26e9d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11625,6 +11625,8 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
         break;
     case EXCP_IRQ:
     case EXCP_VIRQ:
+    case EXCP_NMI:
+    case EXCP_VNMI:
         addr += 0x80;
         break;
     case EXCP_FIQ:
-- 
2.34.1

[RFC PATCH v8 13/23] hw/intc/arm_gicv3: Add irq superpriority information

[Jinjie Ruan via]

A SPI, PPI or SGI interrupt can have a superpriority property. So
maintain superpriority information in PendingIrq and GICR/GICD.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Acked-by: Richard Henderson <richard.henderson@linaro.org>
---
v3:
- Place this ahead of implement GICR_INMIR.
- Add Acked-by.
---
 include/hw/intc/arm_gicv3_common.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index 7324c7d983..df4380141d 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -146,6 +146,7 @@ typedef struct {
     int irq;
     uint8_t prio;
     int grp;
+    bool superprio;
 } PendingIrq;
 
 struct GICv3CPUState {
@@ -172,6 +173,7 @@ struct GICv3CPUState {
     uint32_t gicr_ienabler0;
     uint32_t gicr_ipendr0;
     uint32_t gicr_iactiver0;
+    uint32_t gicr_isuperprio;
     uint32_t edge_trigger; /* ICFGR0 and ICFGR1 even bits */
     uint32_t gicr_igrpmodr0;
     uint32_t gicr_nsacr;
@@ -274,6 +276,7 @@ struct GICv3State {
     GIC_DECLARE_BITMAP(active);       /* GICD_ISACTIVER */
     GIC_DECLARE_BITMAP(level);        /* Current level */
     GIC_DECLARE_BITMAP(edge_trigger); /* GICD_ICFGR even bits */
+    GIC_DECLARE_BITMAP(superprio);    /* GICD_INMIR */
     uint8_t gicd_ipriority[GICV3_MAXIRQ];
     uint64_t gicd_irouter[GICV3_MAXIRQ];
     /* Cached information: pointer to the cpu i/f for the CPUs specified
@@ -313,6 +316,7 @@ GICV3_BITMAP_ACCESSORS(pending)
 GICV3_BITMAP_ACCESSORS(active)
 GICV3_BITMAP_ACCESSORS(level)
 GICV3_BITMAP_ACCESSORS(edge_trigger)
+GICV3_BITMAP_ACCESSORS(superprio)
 
 #define TYPE_ARM_GICV3_COMMON "arm-gicv3-common"
 typedef struct ARMGICv3CommonClass ARMGICv3CommonClass;
-- 
2.34.1

[RFC PATCH v8 14/23] hw/intc/arm_gicv3_redist: Implement GICR_INMIR0

[Jinjie Ruan via]

Add GICR_INMIR0 register and support access GICR_INMIR0.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v6:
- Add Reviewed-by.
v4:
- Make the GICR_INMIR0 implementation more clearer.
---
 hw/intc/arm_gicv3_redist.c | 19 +++++++++++++++++++
 hw/intc/gicv3_internal.h   |  1 +
 2 files changed, 20 insertions(+)

diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index 8153525849..7a16a058b1 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -35,6 +35,15 @@ static int gicr_ns_access(GICv3CPUState *cs, int irq)
     return extract32(cs->gicr_nsacr, irq * 2, 2);
 }
 
+static void gicr_write_bitmap_reg(GICv3CPUState *cs, MemTxAttrs attrs,
+                                  uint32_t *reg, uint32_t val)
+{
+    /* Helper routine to implement writing to a "set" register */
+    val &= mask_group(cs, attrs);
+    *reg = val;
+    gicv3_redist_update(cs);
+}
+
 static void gicr_write_set_bitmap_reg(GICv3CPUState *cs, MemTxAttrs attrs,
                                       uint32_t *reg, uint32_t val)
 {
@@ -406,6 +415,10 @@ static MemTxResult gicr_readl(GICv3CPUState *cs, hwaddr offset,
         *data = value;
         return MEMTX_OK;
     }
+    case GICR_INMIR0:
+        *data = cs->gic->nmi_support ?
+                gicr_read_bitmap_reg(cs, attrs, cs->gicr_isuperprio) : 0;
+        return MEMTX_OK;
     case GICR_ICFGR0:
     case GICR_ICFGR1:
     {
@@ -555,6 +568,12 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr offset,
         gicv3_redist_update(cs);
         return MEMTX_OK;
     }
+    case GICR_INMIR0:
+        if (cs->gic->nmi_support) {
+            gicr_write_bitmap_reg(cs, attrs, &cs->gicr_isuperprio, value);
+        }
+        return MEMTX_OK;
+
     case GICR_ICFGR0:
         /* Register is all RAZ/WI or RAO/WI bits */
         return MEMTX_OK;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 29d5cdc1b6..f35b7d2f03 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -109,6 +109,7 @@
 #define GICR_ICFGR1           (GICR_SGI_OFFSET + 0x0C04)
 #define GICR_IGRPMODR0        (GICR_SGI_OFFSET + 0x0D00)
 #define GICR_NSACR            (GICR_SGI_OFFSET + 0x0E00)
+#define GICR_INMIR0           (GICR_SGI_OFFSET + 0x0F80)
 
 /* VLPI redistributor registers, offsets from VLPI_base */
 #define GICR_VPROPBASER       (GICR_VLPI_OFFSET + 0x70)
-- 
2.34.1

[RFC PATCH v8 15/23] hw/intc/arm_gicv3: Implement GICD_INMIR

[Jinjie Ruan via]

Add GICD_INMIR, GICD_INMIRnE register and support access GICD_INMIR0.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v4:
- Make the GICD_INMIR implementation more clearer.
- Udpate the commit message.
v3:
- Add Reviewed-by.
---
 hw/intc/arm_gicv3_dist.c | 34 ++++++++++++++++++++++++++++++++++
 hw/intc/gicv3_internal.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index 35e850685c..9739404e35 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -89,6 +89,29 @@ static int gicd_ns_access(GICv3State *s, int irq)
     return extract32(s->gicd_nsacr[irq / 16], (irq % 16) * 2, 2);
 }
 
+static void gicd_write_bitmap_reg(GICv3State *s, MemTxAttrs attrs,
+                                  uint32_t *bmp, maskfn *maskfn,
+                                  int offset, uint32_t val)
+{
+    /*
+     * Helper routine to implement writing to a "set" register
+     * (GICD_INMIR, etc).
+     * Semantics implemented here:
+     * RAZ/WI for SGIs, PPIs, unimplemented IRQs
+     * Bits corresponding to Group 0 or Secure Group 1 interrupts RAZ/WI.
+     * offset should be the offset in bytes of the register from the start
+     * of its group.
+     */
+    int irq = offset * 8;
+
+    if (irq < GIC_INTERNAL || irq >= s->num_irq) {
+        return;
+    }
+    val &= mask_group_and_nsacr(s, attrs, maskfn, irq);
+    *gic_bmp_ptr32(bmp, irq) = val;
+    gicv3_update(s, irq, 32);
+}
+
 static void gicd_write_set_bitmap_reg(GICv3State *s, MemTxAttrs attrs,
                                       uint32_t *bmp,
                                       maskfn *maskfn,
@@ -543,6 +566,11 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
         /* RAZ/WI since affinity routing is always enabled */
         *data = 0;
         return true;
+    case GICD_INMIR ... GICD_INMIR + 0x7f:
+        *data = (!s->nmi_support) ? 0 :
+                gicd_read_bitmap_reg(s, attrs, s->superprio, NULL,
+                                     offset - GICD_INMIR);
+        return true;
     case GICD_IROUTER ... GICD_IROUTER + 0x1fdf:
     {
         uint64_t r;
@@ -752,6 +780,12 @@ static bool gicd_writel(GICv3State *s, hwaddr offset,
     case GICD_SPENDSGIR ... GICD_SPENDSGIR + 0xf:
         /* RAZ/WI since affinity routing is always enabled */
         return true;
+    case GICD_INMIR ... GICD_INMIR + 0x7f:
+        if (s->nmi_support) {
+            gicd_write_bitmap_reg(s, attrs, s->superprio, NULL,
+                                  offset - GICD_INMIR, value);
+        }
+        return true;
     case GICD_IROUTER ... GICD_IROUTER + 0x1fdf:
     {
         uint64_t r;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index f35b7d2f03..a1fc34597e 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -52,6 +52,8 @@
 #define GICD_SGIR            0x0F00
 #define GICD_CPENDSGIR       0x0F10
 #define GICD_SPENDSGIR       0x0F20
+#define GICD_INMIR           0x0F80
+#define GICD_INMIRnE         0x3B00
 #define GICD_IROUTER         0x6000
 #define GICD_IDREGS          0xFFD0
 
-- 
2.34.1

[RFC PATCH v8 16/23] hw/intc: Enable FEAT_GICv3_NMI Feature

[Jinjie Ruan via]

Added properties to enable FEAT_GICv3_NMI feature, setup distributor
and redistributor registers to indicate NMI support.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v4:
- Add Reviewed-by.
---
 hw/intc/arm_gicv3_common.c         | 1 +
 hw/intc/arm_gicv3_dist.c           | 2 ++
 hw/intc/gicv3_internal.h           | 1 +
 include/hw/intc/arm_gicv3_common.h | 1 +
 4 files changed, 5 insertions(+)

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index c52f060026..2d2cea6858 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -569,6 +569,7 @@ static Property arm_gicv3_common_properties[] = {
     DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
     DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
     DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
+    DEFINE_PROP_BOOL("has-nmi", GICv3State, nmi_support, 0),
     DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
     /*
      * Compatibility property: force 8 bits of physical priority, even
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index 9739404e35..c4e28d209a 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -412,6 +412,7 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
          *                      by GICD_TYPER.IDbits)
          * MBIS == 0 (message-based SPIs not supported)
          * SecurityExtn == 1 if security extns supported
+         * NMI = 1 if Non-maskable interrupt property is supported
          * CPUNumber == 0 since for us ARE is always 1
          * ITLinesNumber == (((max SPI IntID + 1) / 32) - 1)
          */
@@ -425,6 +426,7 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
         bool dvis = s->revision >= 4;
 
         *data = (1 << 25) | (1 << 24) | (dvis << 18) | (sec_extn << 10) |
+            (s->nmi_support << GICD_TYPER_NMI_SHIFT) |
             (s->lpi_enable << GICD_TYPER_LPIS_SHIFT) |
             (0xf << 19) | itlinesnumber;
         return true;
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index a1fc34597e..8d793243f4 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -70,6 +70,7 @@
 #define GICD_CTLR_E1NWF             (1U << 7)
 #define GICD_CTLR_RWP               (1U << 31)
 
+#define GICD_TYPER_NMI_SHIFT           9
 #define GICD_TYPER_LPIS_SHIFT          17
 
 /* 16 bits EventId */
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index df4380141d..16c5fa7256 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -251,6 +251,7 @@ struct GICv3State {
     uint32_t num_irq;
     uint32_t revision;
     bool lpi_enable;
+    bool nmi_support;
     bool security_extn;
     bool force_8bit_prio;
     bool irq_reset_nonsecure;
-- 
2.34.1

[RFC PATCH v8 17/23] hw/intc/arm_gicv3: Add NMI handling CPU interface registers

[Jinjie Ruan via]

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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v7:
- Add Reviewed-by.
v4:
- Define ICC_NMIAR1_EL1 only if FEAT_GICv3_NMI is implemented.
- Check sctrl_elx.SCTLR_NMI to return 1022 for icc_iar1_read().
- Add gicv3_icc_nmiar1_read() trace event.
- Do not check icc_hppi_can_preempt() for icc_nmiar1_read().
- Add icv_nmiar1_read() and call it when EL2Enabled() and HCR_EL2.IMO == '1'
---
 hw/intc/arm_gicv3_cpuif.c | 59 +++++++++++++++++++++++++++++++++++++--
 hw/intc/gicv3_internal.h  |  1 +
 hw/intc/trace-events      |  1 +
 3 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index e1a60d8c15..df82a413c6 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -795,6 +795,13 @@ static uint64_t icv_iar_read(CPUARMState *env, const ARMCPRegInfo *ri)
     return intid;
 }
 
+static uint64_t icv_nmiar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    /* todo */
+    uint64_t intid = INTID_SPURIOUS;
+    return intid;
+}
+
 static uint32_t icc_fullprio_mask(GICv3CPUState *cs)
 {
     /*
@@ -1097,7 +1104,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 +1116,19 @@ static uint64_t icc_hppir1_value(GICv3CPUState *cs, CPUARMState *env)
         return INTID_SPURIOUS;
     }
 
+    if (!is_hppi) {
+        int el = arm_current_el(env);
+
+        if (is_nmi && (!cs->hppi.superprio)) {
+            return INTID_SPURIOUS;
+        }
+
+        if ((!is_nmi) && cs->hppi.superprio
+            && env->cp15.sctlr_el[el] & SCTLR_NMI) {
+            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 +1189,7 @@ 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)) {
@@ -1179,6 +1200,25 @@ static uint64_t icc_iar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
     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_nmiar1_read(env, ri);
+    }
+
+    intid = icc_hppir1_value(cs, env, true, false);
+
+    if (!gicv3_intid_is_special(intid)) {
+        icc_activate_irq(cs, intid);
+    }
+
+    trace_gicv3_icc_nmiar1_read(gicv3_redist_affid(cs), intid);
+    return intid;
+}
+
 static void icc_drop_prio(GICv3CPUState *cs, int grp)
 {
     /* Drop the priority of the currently active interrupt in
@@ -1555,7 +1595,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;
 }
@@ -2482,6 +2522,15 @@ static const ARMCPRegInfo gicv3_cpuif_icc_apxr23_reginfo[] = {
     },
 };
 
+static const ARMCPRegInfo gicv3_cpuif_gicv3_nmi_reginfo[] = {
+    { .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,
+    },
+};
+
 static uint64_t ich_ap_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     GICv3CPUState *cs = icc_cs_from_env(env);
@@ -2838,6 +2887,10 @@ void gicv3_init_cpuif(GICv3State *s)
          */
         define_arm_cp_regs(cpu, gicv3_cpuif_reginfo);
 
+        if (s->nmi_support) {
+            define_arm_cp_regs(cpu, gicv3_cpuif_gicv3_nmi_reginfo);
+        }
+
         /*
          * The CPU implementation specifies the number of supported
          * bits of physical priority. For backwards compatibility
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 */
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 1ef29d0256..94030550d5 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -116,6 +116,7 @@ gicv3_cpuif_set_irqs(uint32_t cpuid, int fiqlevel, int irqlevel) "GICv3 CPU i/f
 gicv3_icc_generate_sgi(uint32_t cpuid, int irq, int irm, uint32_t aff, uint32_t targetlist) "GICv3 CPU i/f 0x%x generating SGI %d IRM %d target affinity 0x%xxx targetlist 0x%x"
 gicv3_icc_iar0_read(uint32_t cpu, uint64_t val) "GICv3 ICC_IAR0 read cpu 0x%x value 0x%" PRIx64
 gicv3_icc_iar1_read(uint32_t cpu, uint64_t val) "GICv3 ICC_IAR1 read cpu 0x%x value 0x%" PRIx64
+gicv3_icc_nmiar1_read(uint32_t cpu, uint64_t val) "GICv3 ICC_NMIAR1 read cpu 0x%x value 0x%" PRIx64
 gicv3_icc_eoir_write(int grp, uint32_t cpu, uint64_t val) "GICv3 ICC_EOIR%d write cpu 0x%x value 0x%" PRIx64
 gicv3_icc_hppir0_read(uint32_t cpu, uint64_t val) "GICv3 ICC_HPPIR0 read cpu 0x%x value 0x%" PRIx64
 gicv3_icc_hppir1_read(uint32_t cpu, uint64_t val) "GICv3 ICC_HPPIR1 read cpu 0x%x value 0x%" PRIx64
-- 
2.34.1

[RFC PATCH v8 18/23] hw/intc/arm_gicv3: Handle icv_nmiar1_read() for icc_nmiar1_read()

[Jinjie Ruan via]

Implement icv_nmiar1_read() for icc_nmiar1_read(), so add definition for
ICH_LR_EL2.NMI and ICH_AP1R_EL2.NMI bit.

If FEAT_GICv3_NMI is supported, ich_ap_write() should consider ICH_AP1R_EL2.NMI
bit. In icv_activate_irq() and icv_eoir_write(), the ICH_AP1R_EL2.NMI bit
should be set or clear according to the Superpriority info.

By the way, add gicv3_icv_nmiar1_read trace event.

If the hpp irq is a NMI, the icv iar read should return 1022 and trap for
NMI again

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v8:
- Fix an unexpected interrupt bug when sending VNMI by running qemu VM.
v7:
- Add Reviewed-by.
v6:
- Implement icv_nmiar1_read().
---
 hw/intc/arm_gicv3_cpuif.c | 52 ++++++++++++++++++++++++++++++++++-----
 hw/intc/gicv3_internal.h  |  3 +++
 hw/intc/trace-events      |  1 +
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index df82a413c6..d67d62359b 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -728,7 +728,7 @@ static uint64_t icv_hppir_read(CPUARMState *env, const ARMCPRegInfo *ri)
     return value;
 }
 
-static void icv_activate_irq(GICv3CPUState *cs, int idx, int grp)
+static void icv_activate_irq(GICv3CPUState *cs, int idx, int grp, bool nmi)
 {
     /* Activate the interrupt in the specified list register
      * by moving it from Pending to Active state, and update the
@@ -742,7 +742,12 @@ static void icv_activate_irq(GICv3CPUState *cs, int idx, int grp)
 
     cs->ich_lr_el2[idx] &= ~ICH_LR_EL2_STATE_PENDING_BIT;
     cs->ich_lr_el2[idx] |= ICH_LR_EL2_STATE_ACTIVE_BIT;
-    cs->ich_apr[grp][regno] |= (1 << regbit);
+
+    if (cs->gic->nmi_support) {
+        cs->ich_apr[grp][regno] |= (1 << regbit) | (nmi ? ICH_AP1R_EL2_NMI : 0);
+    } else {
+        cs->ich_apr[grp][regno] |= (1 << regbit);
+    }
 }
 
 static void icv_activate_vlpi(GICv3CPUState *cs)
@@ -775,8 +780,10 @@ static uint64_t icv_iar_read(CPUARMState *env, const ARMCPRegInfo *ri)
 
         if (thisgrp == grp && icv_hppi_can_preempt(cs, lr)) {
             intid = ich_lr_vintid(lr);
-            if (!gicv3_intid_is_special(intid)) {
-                icv_activate_irq(cs, idx, grp);
+            if (!gicv3_intid_is_special(intid) && !(lr & ICH_LR_EL2_NMI)) {
+                icv_activate_irq(cs, idx, grp, false);
+            } else if (lr & ICH_LR_EL2_NMI) {
+                intid = INTID_NMI;
             } else {
                 /* Interrupt goes from Pending to Invalid */
                 cs->ich_lr_el2[idx] &= ~ICH_LR_EL2_STATE_PENDING_BIT;
@@ -797,8 +804,32 @@ static uint64_t icv_iar_read(CPUARMState *env, const ARMCPRegInfo *ri)
 
 static uint64_t icv_nmiar1_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
-    /* todo */
+    GICv3CPUState *cs = icc_cs_from_env(env);
+    int idx = hppvi_index(cs);
     uint64_t intid = INTID_SPURIOUS;
+
+    if (idx >= 0 && idx != HPPVI_INDEX_VLPI) {
+        uint64_t lr = cs->ich_lr_el2[idx];
+        int thisgrp = (lr & ICH_LR_EL2_GROUP) ? GICV3_G1NS : GICV3_G0;
+
+        if ((thisgrp == GICV3_G1NS) && (lr & ICH_LR_EL2_NMI)) {
+            intid = ich_lr_vintid(lr);
+            if (!gicv3_intid_is_special(intid)) {
+                icv_activate_irq(cs, idx, GICV3_G1NS, true);
+            } else {
+                /* Interrupt goes from Pending to Invalid */
+                cs->ich_lr_el2[idx] &= ~ICH_LR_EL2_STATE_PENDING_BIT;
+                /* We will now return the (bogus) ID from the list register,
+                 * as per the pseudocode.
+                 */
+            }
+        }
+    }
+
+    trace_gicv3_icv_nmiar1_read(gicv3_redist_affid(cs), intid);
+
+    gicv3_cpuif_virt_update(cs);
+
     return intid;
 }
 
@@ -1403,6 +1434,11 @@ static int icv_drop_prio(GICv3CPUState *cs)
             return (apr0count + i * 32) << (icv_min_vbpr(cs) + 1);
         } else {
             *papr1 &= *papr1 - 1;
+
+            if (cs->gic->nmi_support && (*papr1 & ICH_AP1R_EL2_NMI)) {
+                *papr1 &= ~ICH_AP1R_EL2_NMI;
+            }
+
             return (apr1count + i * 32) << (icv_min_vbpr(cs) + 1);
         }
     }
@@ -2552,7 +2588,11 @@ static void ich_ap_write(CPUARMState *env, const ARMCPRegInfo *ri,
 
     trace_gicv3_ich_ap_write(ri->crm & 1, regno, gicv3_redist_affid(cs), value);
 
-    cs->ich_apr[grp][regno] = value & 0xFFFFFFFFU;
+    if (cs->gic->nmi_support) {
+        cs->ich_apr[grp][regno] = value & (0xFFFFFFFFU | ICH_AP1R_EL2_NMI);
+    } else {
+        cs->ich_apr[grp][regno] = value & 0xFFFFFFFFU;
+    }
     gicv3_cpuif_virt_irq_fiq_update(cs);
 }
 
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 93e56b3726..5e2b32861d 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -242,6 +242,7 @@ FIELD(GICR_VPENDBASER, VALID, 63, 1)
 #define ICH_LR_EL2_PRIORITY_SHIFT 48
 #define ICH_LR_EL2_PRIORITY_LENGTH 8
 #define ICH_LR_EL2_PRIORITY_MASK (0xffULL << ICH_LR_EL2_PRIORITY_SHIFT)
+#define ICH_LR_EL2_NMI (1ULL << 59)
 #define ICH_LR_EL2_GROUP (1ULL << 60)
 #define ICH_LR_EL2_HW (1ULL << 61)
 #define ICH_LR_EL2_STATE_SHIFT 62
@@ -273,6 +274,8 @@ FIELD(GICR_VPENDBASER, VALID, 63, 1)
 #define ICH_VTR_EL2_PREBITS_SHIFT 26
 #define ICH_VTR_EL2_PRIBITS_SHIFT 29
 
+#define ICH_AP1R_EL2_NMI (1ULL << 63)
+
 /* ITS Registers */
 
 FIELD(GITS_BASER, SIZE, 0, 8)
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 94030550d5..47340b5bc1 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -152,6 +152,7 @@ gicv3_icv_rpr_read(uint32_t cpu, uint64_t val) "GICv3 ICV_RPR read cpu 0x%x valu
 gicv3_icv_hppir_read(int grp, uint32_t cpu, uint64_t val) "GICv3 ICV_HPPIR%d read cpu 0x%x value 0x%" PRIx64
 gicv3_icv_dir_write(uint32_t cpu, uint64_t val) "GICv3 ICV_DIR write cpu 0x%x value 0x%" PRIx64
 gicv3_icv_iar_read(int grp, uint32_t cpu, uint64_t val) "GICv3 ICV_IAR%d read cpu 0x%x value 0x%" PRIx64
+gicv3_icv_nmiar1_read(uint32_t cpu, uint64_t val) "GICv3 ICV_NMIAR1 read cpu 0x%x value 0x%" PRIx64
 gicv3_icv_eoir_write(int grp, uint32_t cpu, uint64_t val) "GICv3 ICV_EOIR%d write cpu 0x%x value 0x%" PRIx64
 gicv3_cpuif_virt_update(uint32_t cpuid, int idx, int hppvlpi, int grp, int prio) "GICv3 CPU i/f 0x%x virt HPPI update LR index %d HPPVLPI %d grp %d prio %d"
 gicv3_cpuif_virt_set_irqs(uint32_t cpuid, int fiqlevel, int irqlevel) "GICv3 CPU i/f 0x%x virt HPPI update: setting FIQ %d IRQ %d"
-- 
2.34.1

[RFC PATCH v8 19/23] hw/intc/arm_gicv3: Implement NMI interrupt prioirty

[Jinjie Ruan via]

If GICD_CTLR_DS bit is zero and the NMI is non-secure, the NMI prioirty
is higher than 0x80, otherwise it is higher than 0x0. And save NMI
super prioirty information in hppi.superprio to deliver NMI exception.
Since both GICR and GICD can deliver NMI, it is both necessary to check
whether the pending irq is NMI in gicv3_redist_update_noirqset and
gicv3_update_noirqset. And In irqbetter(), only a non-NMI with the same
priority and a smaller interrupt number can be preempted but not NMI.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v8:
- Add Reviewed-by.
v7:
- Reorder the irqbetter() code for clarity.
- Eliminate the has_superprio local variable for gicv3_get_priority().
- false -> cs->hpplpi.superprio in gicv3_redist_update_noirqset().
- 0x0 -> false in arm_gicv3_common_reset_hold().
- Clear superprio in several places for hppi, hpplpi and hppvlpi.
v6:
- Put the "extract superprio info" logic into gicv3_get_priority().
- Update the comment in irqbetter().
- Reset the cs->hppi.superprio to 0x0.
- Set hppi.superprio to false for LPI.
v4:
- Replace is_nmi with has_superprio to not a mix NMI and superpriority.
- Update the comment in irqbetter().
- Extract gicv3_get_priority() to avoid code repeat.
---
v3:
- Add missing brace
---
 hw/intc/arm_gicv3.c        | 69 +++++++++++++++++++++++++++++++++-----
 hw/intc/arm_gicv3_common.c |  3 ++
 hw/intc/arm_gicv3_redist.c |  3 ++
 3 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
index 0b8f79a122..9496a28005 100644
--- a/hw/intc/arm_gicv3.c
+++ b/hw/intc/arm_gicv3.c
@@ -21,7 +21,8 @@
 #include "hw/intc/arm_gicv3.h"
 #include "gicv3_internal.h"
 
-static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio)
+static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio,
+                      bool has_superprio)
 {
     /* Return true if this IRQ at this priority should take
      * precedence over the current recorded highest priority
@@ -30,14 +31,23 @@ static bool irqbetter(GICv3CPUState *cs, int irq, uint8_t prio)
      * is the same as this one (a property which the calling code
      * relies on).
      */
-    if (prio < cs->hppi.prio) {
-        return true;
+    if (prio != cs->hppi.prio) {
+        return prio < cs->hppi.prio;
+    }
+
+    /*
+     * The same priority IRQ with superpriority should signal to the CPU
+     * as it have the priority higher than the labelled 0x80 or 0x00.
+     */
+    if (has_superprio != cs->hppi.superprio) {
+        return has_superprio;
     }
+
     /* If multiple pending interrupts have the same priority then it is an
      * IMPDEF choice which of them to signal to the CPU. We choose to
      * signal the one with the lowest interrupt number.
      */
-    if (prio == cs->hppi.prio && irq <= cs->hppi.irq) {
+    if (irq <= cs->hppi.irq) {
         return true;
     }
     return false;
@@ -129,6 +139,40 @@ static uint32_t gicr_int_pending(GICv3CPUState *cs)
     return pend;
 }
 
+static bool gicv3_get_priority(GICv3CPUState *cs, bool is_redist,
+                               uint8_t *prio, int irq)
+{
+    uint32_t superprio = 0x0;
+
+    if (is_redist) {
+        superprio = extract32(cs->gicr_isuperprio, irq, 1);
+    } else {
+        superprio = *gic_bmp_ptr32(cs->gic->superprio, irq);
+        superprio = superprio & (1 << (irq & 0x1f));
+    }
+
+    if (superprio) {
+        /* DS = 0 & Non-secure NMI */
+        if (!(cs->gic->gicd_ctlr & GICD_CTLR_DS) &&
+            ((is_redist && extract32(cs->gicr_igroupr0, irq, 1)) ||
+             (!is_redist && gicv3_gicd_group_test(cs->gic, irq)))) {
+            *prio = 0x80;
+        } else {
+            *prio = 0x0;
+        }
+
+        return true;
+    }
+
+    if (is_redist) {
+        *prio = cs->gicr_ipriorityr[irq];
+    } else {
+        *prio = cs->gic->gicd_ipriority[irq];
+    }
+
+    return false;
+}
+
 /* Update the interrupt status after state in a redistributor
  * or CPU interface has changed, but don't tell the CPU i/f.
  */
@@ -141,6 +185,7 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
     uint8_t prio;
     int i;
     uint32_t pend;
+    bool has_superprio = false;
 
     /* Find out which redistributor interrupts are eligible to be
      * signaled to the CPU interface.
@@ -152,10 +197,11 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
             if (!(pend & (1 << i))) {
                 continue;
             }
-            prio = cs->gicr_ipriorityr[i];
-            if (irqbetter(cs, i, prio)) {
+            has_superprio = gicv3_get_priority(cs, true, &prio, i);
+            if (irqbetter(cs, i, prio, has_superprio)) {
                 cs->hppi.irq = i;
                 cs->hppi.prio = prio;
+                cs->hppi.superprio = has_superprio;
                 seenbetter = true;
             }
         }
@@ -168,9 +214,11 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
     if ((cs->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) && cs->gic->lpi_enable &&
         (cs->gic->gicd_ctlr & GICD_CTLR_EN_GRP1NS) &&
         (cs->hpplpi.prio != 0xff)) {
-        if (irqbetter(cs, cs->hpplpi.irq, cs->hpplpi.prio)) {
+        if (irqbetter(cs, cs->hpplpi.irq, cs->hpplpi.prio,
+                      cs->hpplpi.superprio)) {
             cs->hppi.irq = cs->hpplpi.irq;
             cs->hppi.prio = cs->hpplpi.prio;
+            cs->hppi.superprio = cs->hpplpi.superprio;
             cs->hppi.grp = cs->hpplpi.grp;
             seenbetter = true;
         }
@@ -213,6 +261,7 @@ static void gicv3_update_noirqset(GICv3State *s, int start, int len)
     int i;
     uint8_t prio;
     uint32_t pend = 0;
+    bool has_superprio = false;
 
     assert(start >= GIC_INTERNAL);
     assert(len > 0);
@@ -240,10 +289,11 @@ static void gicv3_update_noirqset(GICv3State *s, int start, int len)
              */
             continue;
         }
-        prio = s->gicd_ipriority[i];
-        if (irqbetter(cs, i, prio)) {
+        has_superprio = gicv3_get_priority(cs, false, &prio, i);
+        if (irqbetter(cs, i, prio, has_superprio)) {
             cs->hppi.irq = i;
             cs->hppi.prio = prio;
+            cs->hppi.superprio = has_superprio;
             cs->seenbetter = true;
         }
     }
@@ -293,6 +343,7 @@ void gicv3_full_update_noirqset(GICv3State *s)
 
     for (i = 0; i < s->num_cpu; i++) {
         s->cpu[i].hppi.prio = 0xff;
+        s->cpu[i].hppi.superprio = false;
     }
 
     /* Note that we can guarantee that these functions will not
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 2d2cea6858..822d99d8b4 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -498,8 +498,11 @@ static void arm_gicv3_common_reset_hold(Object *obj)
         memset(cs->gicr_ipriorityr, 0, sizeof(cs->gicr_ipriorityr));
 
         cs->hppi.prio = 0xff;
+        cs->hppi.superprio = false;
         cs->hpplpi.prio = 0xff;
+        cs->hpplpi.superprio = false;
         cs->hppvlpi.prio = 0xff;
+        cs->hppvlpi.superprio = false;
 
         /* State in the CPU interface must *not* be reset here, because it
          * is part of the CPU's reset domain, not the GIC device's.
diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index 7a16a058b1..4118c2f297 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -120,6 +120,7 @@ static void update_for_one_lpi(GICv3CPUState *cs, int irq,
         ((prio == hpp->prio) && (irq <= hpp->irq))) {
         hpp->irq = irq;
         hpp->prio = prio;
+        hpp->superprio = false;
         /* LPIs and vLPIs are always non-secure Grp1 interrupts */
         hpp->grp = GICV3_G1NS;
     }
@@ -156,6 +157,7 @@ static void update_for_all_lpis(GICv3CPUState *cs, uint64_t ptbase,
     int i, bit;
 
     hpp->prio = 0xff;
+    hpp->superprio = false;
 
     for (i = GICV3_LPI_INTID_START / 8; i < pendt_size / 8; i++) {
         address_space_read(as, ptbase + i, MEMTXATTRS_UNSPECIFIED, &pend, 1);
@@ -241,6 +243,7 @@ static void gicv3_redist_update_vlpi_only(GICv3CPUState *cs)
 
     if (!FIELD_EX64(cs->gicr_vpendbaser, GICR_VPENDBASER, VALID)) {
         cs->hppvlpi.prio = 0xff;
+        cs->hppvlpi.superprio = false;
         return;
     }
 
-- 
2.34.1

[RFC PATCH v8 20/23] hw/intc/arm_gicv3: Report the NMI interrupt in gicv3_cpuif_update()

[Jinjie Ruan via]

In CPU Interface, if the IRQ has the superpriority property, report
NMI to the corresponding PE.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v6:
- Add Reviewed-by.
v4:
- Swap the ordering of the IFs.
v3:
- Remove handling nmi_is_irq flag.
---
 hw/intc/arm_gicv3_cpuif.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index d67d62359b..29142fcf8d 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -969,6 +969,7 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
     /* Tell the CPU about its highest priority pending interrupt */
     int irqlevel = 0;
     int fiqlevel = 0;
+    int nmilevel = 0;
     ARMCPU *cpu = ARM_CPU(cs->cpu);
     CPUARMState *env = &cpu->env;
 
@@ -1007,6 +1008,8 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
 
         if (isfiq) {
             fiqlevel = 1;
+        } else if (cs->hppi.superprio) {
+            nmilevel = 1;
         } else {
             irqlevel = 1;
         }
@@ -1016,6 +1019,7 @@ void gicv3_cpuif_update(GICv3CPUState *cs)
 
     qemu_set_irq(cs->parent_fiq, fiqlevel);
     qemu_set_irq(cs->parent_irq, irqlevel);
+    qemu_set_irq(cs->parent_nmi, nmilevel);
 }
 
 static uint64_t icc_pmr_read(CPUARMState *env, const ARMCPRegInfo *ri)
-- 
2.34.1

[RFC PATCH v8 21/23] hw/intc/arm_gicv3: Report the VNMI interrupt

[Jinjie Ruan via]

In vCPU Interface, if the vIRQ has the superpriority property, report
vNMI to the corresponding vPE.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v6:
- Add Reviewed-by.
---
 hw/intc/arm_gicv3_cpuif.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 29142fcf8d..0bea6cb020 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -465,6 +465,7 @@ void gicv3_cpuif_virt_irq_fiq_update(GICv3CPUState *cs)
     int idx;
     int irqlevel = 0;
     int fiqlevel = 0;
+    int nmilevel = 0;
 
     idx = hppvi_index(cs);
     trace_gicv3_cpuif_virt_update(gicv3_redist_affid(cs), idx,
@@ -482,9 +483,17 @@ void gicv3_cpuif_virt_irq_fiq_update(GICv3CPUState *cs)
         uint64_t lr = cs->ich_lr_el2[idx];
 
         if (icv_hppi_can_preempt(cs, lr)) {
-            /* Virtual interrupts are simple: G0 are always FIQ, and G1 IRQ */
+            /*
+             * Virtual interrupts are simple: G0 are always FIQ, and G1 are
+             * IRQ or NMI which depends on the ICH_LR<n>_EL2.NMI to have
+             * non-maskable property.
+             */
             if (lr & ICH_LR_EL2_GROUP) {
-                irqlevel = 1;
+                if (cs->gic->nmi_support && (lr & ICH_LR_EL2_NMI)) {
+                    nmilevel = 1;
+                } else {
+                    irqlevel = 1;
+                }
             } else {
                 fiqlevel = 1;
             }
@@ -494,6 +503,7 @@ void gicv3_cpuif_virt_irq_fiq_update(GICv3CPUState *cs)
     trace_gicv3_cpuif_virt_set_irqs(gicv3_redist_affid(cs), fiqlevel, irqlevel);
     qemu_set_irq(cs->parent_vfiq, fiqlevel);
     qemu_set_irq(cs->parent_virq, irqlevel);
+    qemu_set_irq(cs->parent_vnmi, nmilevel);
 }
 
 static void gicv3_cpuif_virt_update(GICv3CPUState *cs)
-- 
2.34.1

[RFC PATCH v8 22/23] target/arm: Add FEAT_NMI to max

[Jinjie Ruan via]

Enable FEAT_NMI on the 'max' CPU.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v3:
- Add Reviewed-by.
- Sorted to last.
---
 docs/system/arm/emulation.rst | 1 +
 target/arm/tcg/cpu64.c        | 1 +
 2 files changed, 2 insertions(+)

diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst
index 2a7bbb82dc..a9ae7ede9f 100644
--- a/docs/system/arm/emulation.rst
+++ b/docs/system/arm/emulation.rst
@@ -64,6 +64,7 @@ the following architecture extensions:
 - FEAT_MTE (Memory Tagging Extension)
 - FEAT_MTE2 (Memory Tagging Extension)
 - FEAT_MTE3 (MTE Asymmetric Fault Handling)
+- FEAT_NMI (Non-maskable Interrupt)
 - FEAT_NV (Nested Virtualization)
 - FEAT_NV2 (Enhanced nested virtualization support)
 - FEAT_PACIMP (Pointer authentication - IMPLEMENTATION DEFINED algorithm)
diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c
index 9f7a9f3d2c..62c4663512 100644
--- a/target/arm/tcg/cpu64.c
+++ b/target/arm/tcg/cpu64.c
@@ -1175,6 +1175,7 @@ void aarch64_max_tcg_initfn(Object *obj)
     t = FIELD_DP64(t, ID_AA64PFR1, RAS_FRAC, 0);  /* FEAT_RASv1p1 + FEAT_DoubleFault */
     t = FIELD_DP64(t, ID_AA64PFR1, SME, 1);       /* FEAT_SME */
     t = FIELD_DP64(t, ID_AA64PFR1, CSV2_FRAC, 0); /* FEAT_CSV2_2 */
+    t = FIELD_DP64(t, ID_AA64PFR1, NMI, 1);       /* FEAT_NMI */
     cpu->isar.id_aa64pfr1 = t;
 
     t = cpu->isar.id_aa64mmfr0;
-- 
2.34.1

[RFC PATCH v8 23/23] hw/arm/virt: Add FEAT_GICv3_NMI feature support in virt GIC

[Jinjie Ruan via]

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

So included support FEAT_GICv3_NMI feature as part of virt platform
GIC initialization if FEAT_NMI and FEAT_GICv3 supported.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
v4:
- Add Reviewed-by.
v3:
- Adjust to be the last after add FEAT_NMI to max.
- Check whether support FEAT_NMI and FEAT_GICv3 for FEAT_GICv3_NMI.
---
 hw/arm/virt.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index dc7959e164..680a8a3eb6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -729,6 +729,19 @@ static void create_v2m(VirtMachineState *vms)
     vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
 }
 
+/*
+ * 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.
+ */
+static bool gicv3_nmi_present(VirtMachineState *vms)
+{
+    ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0));
+
+    return cpu_isar_feature(aa64_nmi, cpu) &&
+           (vms->gic_version != VIRT_GIC_VERSION_2);
+}
+
 static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
 {
     MachineState *ms = MACHINE(vms);
@@ -802,6 +815,11 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
                               vms->virt);
         }
     }
+
+    if (gicv3_nmi_present(vms)) {
+        qdev_prop_set_bit(vms->gic, "has-nmi", true);
+    }
+
     gicbusdev = SYS_BUS_DEVICE(vms->gic);
     sysbus_realize_and_unref(gicbusdev, &error_fatal);
     sysbus_mmio_map(gicbusdev, 0, vms->memmap[VIRT_GIC_DIST].base);
-- 
2.34.1

Re: [RFC PATCH v8 05/23] target/arm: Support MSR access to ALLINT

[Peter Maydell]

On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> Support ALLINT msr access as follow:
>         mrs <xt>, ALLINT        // read allint
>         msr ALLINT, <xt>        // write allint with imm
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v5:
> - Add Reviewed-by.
> v4:
> - Remove arm_is_el2_enabled() check in allint_check().
> - Change to env->pstate instead of env->allint.
> v3:
> - Remove EL0 check in aa64_allint_access() which alreay checks in .access
>   PL1_RW.
> - Use arm_hcrx_el2_eff() in aa64_allint_access() instead of env->cp15.hcrx_el2.
> - Make ALLINT msr access function controlled by aa64_nmi.
> ---
>  target/arm/helper.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)

If you configure with --target-list=aarch64-softmmu,arm-softmmu
you'll find this fails to build:

>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index b19a0178ce..aa0151c775 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -4752,6 +4752,36 @@ static void aa64_daif_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      env->daif = value & PSTATE_DAIF;
>  }
>
> +static void aa64_allint_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                              uint64_t value)
> +{
> +    env->pstate = (env->pstate & ~PSTATE_ALLINT) | (value & PSTATE_ALLINT);
> +}
> +
> +static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    return env->pstate & PSTATE_ALLINT;
> +}
> +
> +static CPAccessResult aa64_allint_access(CPUARMState *env,
> +                                         const ARMCPRegInfo *ri, bool isread)
> +{
> +    if (arm_current_el(env) == 1 && (arm_hcrx_el2_eff(env) & HCRX_TALLINT)) {
> +        return CP_ACCESS_TRAP_EL2;
> +    }
> +    return CP_ACCESS_OK;
> +}
> +
> +static const ARMCPRegInfo nmi_reginfo[] = {
> +    { .name = "ALLINT", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 0, .opc2 = 0, .crn = 4, .crm = 3,
> +      .type = ARM_CP_NO_RAW,
> +      .access = PL1_RW, .accessfn = aa64_allint_access,
> +      .fieldoffset = offsetof(CPUARMState, pstate),
> +      .writefn = aa64_allint_write, .readfn = aa64_allint_read,
> +      .resetfn = arm_cp_reset_ignore },
> +};

These functions and the array have been put in a bit of the
file that is built whether TARGET_AARCH64 is defined or not...

> +
>  static uint64_t aa64_pan_read(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
>      return env->pstate & PSTATE_PAN;
> @@ -9889,6 +9919,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>      if (cpu_isar_feature(aa64_nv2, cpu)) {
>          define_arm_cp_regs(cpu, nv2_reginfo);
>      }
> +
> +    if (cpu_isar_feature(aa64_nmi, cpu)) {
> +        define_arm_cp_regs(cpu, nmi_reginfo);
> +    }
>  #endif

...but the only reference to them is inside an ifdef TARGET_AARCH64.

Moving the nmi_reginfo[] and the functions so they are next to
some other TARGET_AARCH64-only reginfo array inside one of the
existing ifdef blocks is probably the nicest fix.

>
>      if (cpu_isar_feature(any_predinv, cpu)) {
> --

thanks
-- PMM

Re: [RFC PATCH v8 09/23] target/arm: Handle PSTATE.ALLINT on taking an exception

[Peter Maydell]

On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> Set or clear PSTATE.ALLINT on taking an exception to ELx according to the
> SCTLR_ELx.SPINTMASK bit.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v3:
> - Add Reviewed-by.
> ---
>  target/arm/helper.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 4bc63bf7ca..81f4a8f194 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -11705,6 +11705,15 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>          }
>      }
>
> +    if (cpu_isar_feature(aa64_nmi, cpu) &&
> +        (env->cp15.sctlr_el[new_el] & SCTLR_NMI)) {

This shouldn't be checking the value of SCTLR_NMI here:
the new PSTATE.ALLINT is set to !SPINTMASK even if NMI == 0.
(The SPINTMASK bit description is a bit confusing, but
the correct behaviour is clear in the AArch64.TakeException()
pseudocode.)

> +        if (!(env->cp15.sctlr_el[new_el] & SCTLR_SPINTMASK)) {
> +            new_mode |= PSTATE_ALLINT;
> +        } else {
> +            new_mode &= ~PSTATE_ALLINT;
> +        }
> +    }
> +
>      pstate_write(env, PSTATE_DAIF | new_mode);
>      env->aarch64 = true;
>      aarch64_restore_sp(env, new_el);
> --
> 2.34.1
>

thanks
-- PMM

Re: [RFC PATCH v8 06/23] target/arm: Add support for Non-maskable Interrupt

[Peter Maydell]

On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> This only implements the external delivery method via the GICv3.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v8:
> - Fix the rcu stall after sending a VNMI in qemu VM.
> v7:
> - Add Reviewed-by.
> v6:
> - env->cp15.hcr_el2 -> arm_hcr_el2_eff().
> - env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
> - Not include VF && VFNMI in CPU_INTERRUPT_VNMI.
> v4:
> - Accept NMI unconditionally for arm_cpu_has_work() but add comment.
> - Change from & to && for EXCP_IRQ or EXCP_FIQ.
> - Refator nmi mask in arm_excp_unmasked().
> - Also handle VNMI in arm_cpu_exec_interrupt() and arm_cpu_set_irq().
> - Rename virtual to Virtual.
> v3:
> - Not include CPU_INTERRUPT_NMI when FEAT_NMI not enabled
> - Add ARM_CPU_VNMI.
> - Refator nmi mask in arm_excp_unmasked().
> - Test SCTLR_ELx.NMI for ALLINT mask for NMI.
> ---
>  target/arm/cpu-qom.h   |  4 +-
>  target/arm/cpu.c       | 85 +++++++++++++++++++++++++++++++++++++++---
>  target/arm/cpu.h       |  4 ++
>  target/arm/helper.c    |  2 +
>  target/arm/internals.h |  9 +++++
>  5 files changed, 97 insertions(+), 7 deletions(-)
>
> diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
> index 8e032691db..e0c9e18036 100644
> --- a/target/arm/cpu-qom.h
> +++ b/target/arm/cpu-qom.h
> @@ -36,11 +36,13 @@ 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 six 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
> +#define ARM_CPU_VNMI 5
>
>  /* For M profile, some registers are banked secure vs non-secure;
>   * these are represented as a 2-element array where the first element

> @@ -678,13 +687,31 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
>          return false;
>      }
>
> +    if (cpu_isar_feature(aa64_nmi, env_archcpu(env)) &&
> +        env->cp15.sctlr_el[target_el] & SCTLR_NMI && cur_el == target_el) {
> +        allIntMask = env->pstate & PSTATE_ALLINT ||
> +                     ((env->cp15.sctlr_el[target_el] & SCTLR_SPINTMASK) &&
> +                      (env->pstate & PSTATE_SP));
> +    }
> +
>      switch (excp_idx) {
> +    case EXCP_NMI:
> +        pstate_unmasked = !allIntMask;
> +        break;
> +
> +    case EXCP_VNMI:
> +        if ((!(hcr_el2 & HCR_IMO) && !(hcr_el2 & HCR_FMO)) ||
> +             (hcr_el2 & HCR_TGE)) {
> +            /* VNMIs(VIRQs or VFIQs) are only taken when hypervized.  */
> +            return false;
> +        }

VINMI and VFNMI aren't the same thing: do we definitely want to
merge them into one EXCP_VNMI ? It feels like it would be simpler
to keep them separate. Similarly CPU_INTERRUPT_VNMI, and
arm_cpu_update_vnmi() probably want VINMI and VFNMI versions.

> +        return !allIntMask;
>      case EXCP_FIQ:
> -        pstate_unmasked = !(env->daif & PSTATE_F);
> +        pstate_unmasked = (!(env->daif & PSTATE_F)) && (!allIntMask);
>          break;
>
>      case EXCP_IRQ:
> -        pstate_unmasked = !(env->daif & PSTATE_I);
> +        pstate_unmasked = (!(env->daif & PSTATE_I)) && (!allIntMask);
>          break;
>
>      case EXCP_VFIQ:
> @@ -692,13 +719,13 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
>              /* VFIQs are only taken when hypervized.  */
>              return false;
>          }
> -        return !(env->daif & PSTATE_F);
> +        return !(env->daif & PSTATE_F) && (!allIntMask);
>      case EXCP_VIRQ:
>          if (!(hcr_el2 & HCR_IMO) || (hcr_el2 & HCR_TGE)) {
>              /* VIRQs are only taken when hypervized.  */
>              return false;
>          }
> -        return !(env->daif & PSTATE_I);
> +        return !(env->daif & PSTATE_I) && (!allIntMask);
>      case EXCP_VSERR:
>          if (!(hcr_el2 & HCR_AMO) || (hcr_el2 & HCR_TGE)) {
>              /* VIRQs are only taken when hypervized.  */
> @@ -804,6 +831,24 @@ 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_VNMI) {
> +            excp_idx = EXCP_VNMI;
> +            target_el = 1;
> +            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);
> @@ -900,6 +945,28 @@ void arm_cpu_update_vfiq(ARMCPU *cpu)
>      }
>  }
>
> +void arm_cpu_update_vnmi(ARMCPU *cpu)
> +{
> +    /*
> +     * Update the interrupt level for VNMI, which is the logical OR of
> +     * the HCRX_EL2.VINMI bit and the input line level from the GIC.
> +     */
> +    CPUARMState *env = &cpu->env;
> +    CPUState *cs = CPU(cpu);
> +
> +    bool new_state = ((arm_hcr_el2_eff(env) & HCR_VI) &&
> +                      (arm_hcrx_el2_eff(env) & HCRX_VINMI)) ||
> +        (env->irq_line_state & CPU_INTERRUPT_VNMI);
> +
> +    if (new_state != ((cs->interrupt_request & CPU_INTERRUPT_VNMI) != 0)) {
> +        if (new_state) {
> +            cpu_interrupt(cs, CPU_INTERRUPT_VNMI);
> +        } else {
> +            cpu_reset_interrupt(cs, CPU_INTERRUPT_VNMI);
> +        }
> +    }
> +}

I think Richard noted on a previous version of the series that
the existing arm_cpu_update_virq() and arm_cpu_update_vfiq()
also need changing so they don't set CPU_INTERRUPT_VIRQ
or CPU_INTERRUPT_VFIQ if the HCRX_EL2 bits indicate that
we should be signalling a VINMI or VFNMI instead.
That also means that VIRQ and VFIQ will change values based
on changes in HCRX_EL2, which means that hcrx_write() needs
to have calls to arm_cpu_update_{virq,vfiq,vnmi} the way
that do_hcr_write() already does.

The use of the _eff() versions of the functions here is
correct but it introduces a new case where we need to
reevaluate the status of the VNMI etc interrupt status:
when we change from Secure to NonSecure or when we change
SCR_EL3.EEL2 or SCR_EL3.HXEN. We either need to make sure
we reevaluate when we drop from EL3 to EL2 (which would be
OK since VINMI and VFNMI can't be taken at EL3 and none of
these bits can change except at EL3) or else make the calls
to reevaluate them when we write to SCR_EL3. At least, I don't
think we currently reevaluate these bits on an EL change.

> +
>  void arm_cpu_update_vserr(ARMCPU *cpu)
>  {
>      /*
> @@ -929,7 +996,9 @@ 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,
> +        [ARM_CPU_VNMI] = CPU_INTERRUPT_VNMI
>      };
>
>      if (!arm_feature(env, ARM_FEATURE_EL2) &&
> @@ -955,8 +1024,12 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
>      case ARM_CPU_VFIQ:
>          arm_cpu_update_vfiq(cpu);
>          break;
> +    case ARM_CPU_VNMI:
> +        arm_cpu_update_vnmi(cpu);
> +        break;
>      case ARM_CPU_IRQ:
>      case ARM_CPU_FIQ:
> +    case ARM_CPU_NMI:
>          if (level) {
>              cpu_interrupt(cs, mask[irq]);
>          } else {
> @@ -1355,7 +1428,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, 6);

You should also update the value passed when we init the
GPIOs in the arm_cpu_kvm_set_irq case, and update the comment
that explains why, which currently reads:

        /* VIRQ and VFIQ are unused with KVM but we add them to maintain
         * the same interface as non-KVM CPUs.
         */

so it mentions also the NMI and VNMI inputs.

>      }
>
>      qdev_init_gpio_out(DEVICE(cpu), cpu->gt_timer_outputs,
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index de740d223f..629221e1a9 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -61,6 +61,8 @@
>  #define EXCP_DIVBYZERO      23   /* v7M DIVBYZERO UsageFault */
>  #define EXCP_VSERR          24
>  #define EXCP_GPC            25   /* v9 Granule Protection Check Fault */
> +#define EXCP_NMI            26
> +#define EXCP_VNMI           27
>  /* NB: add new EXCP_ defines to the array in arm_log_exception() too */
>
>  #define ARMV7M_EXCP_RESET   1
> @@ -80,6 +82,8 @@
>  #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
> +#define CPU_INTERRUPT_VNMI  CPU_INTERRUPT_TGT_EXT_0

>  /* 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 aa0151c775..875a7fa8da 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -10793,6 +10793,8 @@ void arm_log_exception(CPUState *cs)
>              [EXCP_DIVBYZERO] = "v7M DIVBYZERO UsageFault",
>              [EXCP_VSERR] = "Virtual SERR",
>              [EXCP_GPC] = "Granule Protection Check",
> +            [EXCP_NMI] = "NMI",
> +            [EXCP_VNMI] = "Virtual NMI"
>          };
>
>          if (idx >= 0 && idx < ARRAY_SIZE(excnames)) {
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 516e0584bf..cb217a9ce7 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1109,6 +1109,15 @@ void arm_cpu_update_virq(ARMCPU *cpu);
>   */
>  void arm_cpu_update_vfiq(ARMCPU *cpu);
>
> +/**
> + * arm_cpu_update_vnmi: Update CPU_INTERRUPT_VNMI bit in cs->interrupt_request
> + *
> + * Update the CPU_INTERRUPT_VNMI bit in cs->interrupt_request, following
> + * a change to either the input VNMI line from the GIC or the HCRX_EL2.VINMI.
> + * Must be called with the BQL held.
> + */
> +void arm_cpu_update_vnmi(ARMCPU *cpu);

thanks
-- PMM

Re: [RFC PATCH v8 05/23] target/arm: Support MSR access to ALLINT

[Peter Maydell]

On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> Support ALLINT msr access as follow:
>         mrs <xt>, ALLINT        // read allint
>         msr ALLINT, <xt>        // write allint with imm
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v5:
> - Add Reviewed-by.
> v4:
> - Remove arm_is_el2_enabled() check in allint_check().
> - Change to env->pstate instead of env->allint.
> v3:
> - Remove EL0 check in aa64_allint_access() which alreay checks in .access
>   PL1_RW.
> - Use arm_hcrx_el2_eff() in aa64_allint_access() instead of env->cp15.hcrx_el2.
> - Make ALLINT msr access function controlled by aa64_nmi.
> ---
>  target/arm/helper.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index b19a0178ce..aa0151c775 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -4752,6 +4752,36 @@ static void aa64_daif_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      env->daif = value & PSTATE_DAIF;
>  }
>
> +static void aa64_allint_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                              uint64_t value)
> +{
> +    env->pstate = (env->pstate & ~PSTATE_ALLINT) | (value & PSTATE_ALLINT);
> +}
> +
> +static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    return env->pstate & PSTATE_ALLINT;
> +}
> +
> +static CPAccessResult aa64_allint_access(CPUARMState *env,
> +                                         const ARMCPRegInfo *ri, bool isread)
> +{
> +    if (arm_current_el(env) == 1 && (arm_hcrx_el2_eff(env) & HCRX_TALLINT)) {
> +        return CP_ACCESS_TRAP_EL2;
> +    }
> +    return CP_ACCESS_OK;

Forgot to note in my earlier email: HCRX_EL2.TALLINT traps
only writes to ALLINT, not reads, so the condition
here needs to also look at 'isread'.

> +}

thanks
-- PMM

Re: [RFC PATCH v8 06/23] target/arm: Add support for Non-maskable Interrupt

[Richard Henderson]

On 3/19/24 07:28, Peter Maydell wrote:
>>       switch (excp_idx) {
>> +    case EXCP_NMI:
>> +        pstate_unmasked = !allIntMask;
>> +        break;
>> +
>> +    case EXCP_VNMI:
>> +        if ((!(hcr_el2 & HCR_IMO) && !(hcr_el2 & HCR_FMO)) ||
>> +             (hcr_el2 & HCR_TGE)) {
>> +            /* VNMIs(VIRQs or VFIQs) are only taken when hypervized.  */
>> +            return false;
>> +        }
> 
> VINMI and VFNMI aren't the same thing: do we definitely want to
> merge them into one EXCP_VNMI ?

We do not, which is why VFNMI is going through EXCP_VFIQ.  A previous version did, and I 
see the comment did not change to match the new implementation.

> The use of the _eff() versions of the functions here is
> correct but it introduces a new case where we need to
> reevaluate the status of the VNMI etc interrupt status:
> when we change from Secure to NonSecure or when we change
> SCR_EL3.EEL2 or SCR_EL3.HXEN. We either need to make sure
> we reevaluate when we drop from EL3 to EL2 (which would be
> OK since VINMI and VFNMI can't be taken at EL3 and none of
> these bits can change except at EL3) or else make the calls
> to reevaluate them when we write to SCR_EL3. At least, I don't
> think we currently reevaluate these bits on an EL change.

We re-evaluate these bits on EL change via gicv3_cpuif_el_change_hook.


r~

Re: [RFC PATCH v8 06/23] target/arm: Add support for Non-maskable Interrupt

[Peter Maydell]

On Tue, 19 Mar 2024 at 18:51, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/19/24 07:28, Peter Maydell wrote:
> >>       switch (excp_idx) {
> >> +    case EXCP_NMI:
> >> +        pstate_unmasked = !allIntMask;
> >> +        break;
> >> +
> >> +    case EXCP_VNMI:
> >> +        if ((!(hcr_el2 & HCR_IMO) && !(hcr_el2 & HCR_FMO)) ||
> >> +             (hcr_el2 & HCR_TGE)) {
> >> +            /* VNMIs(VIRQs or VFIQs) are only taken when hypervized.  */
> >> +            return false;
> >> +        }
> >
> > VINMI and VFNMI aren't the same thing: do we definitely want to
> > merge them into one EXCP_VNMI ?
>
> We do not, which is why VFNMI is going through EXCP_VFIQ.  A previous version did, and I
> see the comment did not change to match the new implementation.

Why do we put VFNMI through VFIQ, though? They're not
the same thing either... I think I would find this less
confusing if we implemented what the spec says and distinguished
all of (IRQ, FIQ, IRQ-with-superpriority and FIQ-with-superpriority).

> > The use of the _eff() versions of the functions here is
> > correct but it introduces a new case where we need to
> > reevaluate the status of the VNMI etc interrupt status:
> > when we change from Secure to NonSecure or when we change
> > SCR_EL3.EEL2 or SCR_EL3.HXEN. We either need to make sure
> > we reevaluate when we drop from EL3 to EL2 (which would be
> > OK since VINMI and VFNMI can't be taken at EL3 and none of
> > these bits can change except at EL3) or else make the calls
> > to reevaluate them when we write to SCR_EL3. At least, I don't
> > think we currently reevaluate these bits on an EL change.
>
> We re-evaluate these bits on EL change via gicv3_cpuif_el_change_hook.

Only if the GIC is connected to the VIRQ and VFIQ interrupt lines,
which it doesn't in theory have to be (though in practice it
usually will be). Plus it feels a bit fragile against
somebody deciding to put in a "this line hasn't changed state
so don't bother calling the handler again" optimization in the
future.

thanks
-- PMM

Re: [RFC PATCH v8 05/23] target/arm: Support MSR access to ALLINT

[Jinjie Ruan via]

On 2024/3/20 0:45, Peter Maydell wrote:
> On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>
>> Support ALLINT msr access as follow:
>>         mrs <xt>, ALLINT        // read allint
>>         msr ALLINT, <xt>        // write allint with imm
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> v5:
>> - Add Reviewed-by.
>> v4:
>> - Remove arm_is_el2_enabled() check in allint_check().
>> - Change to env->pstate instead of env->allint.
>> v3:
>> - Remove EL0 check in aa64_allint_access() which alreay checks in .access
>>   PL1_RW.
>> - Use arm_hcrx_el2_eff() in aa64_allint_access() instead of env->cp15.hcrx_el2.
>> - Make ALLINT msr access function controlled by aa64_nmi.
>> ---
>>  target/arm/helper.c | 34 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
> 
> If you configure with --target-list=aarch64-softmmu,arm-softmmu
> you'll find this fails to build:
> 
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index b19a0178ce..aa0151c775 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -4752,6 +4752,36 @@ static void aa64_daif_write(CPUARMState *env, const ARMCPRegInfo *ri,
>>      env->daif = value & PSTATE_DAIF;
>>  }
>>
>> +static void aa64_allint_write(CPUARMState *env, const ARMCPRegInfo *ri,
>> +                              uint64_t value)
>> +{
>> +    env->pstate = (env->pstate & ~PSTATE_ALLINT) | (value & PSTATE_ALLINT);
>> +}
>> +
>> +static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo *ri)
>> +{
>> +    return env->pstate & PSTATE_ALLINT;
>> +}
>> +
>> +static CPAccessResult aa64_allint_access(CPUARMState *env,
>> +                                         const ARMCPRegInfo *ri, bool isread)
>> +{
>> +    if (arm_current_el(env) == 1 && (arm_hcrx_el2_eff(env) & HCRX_TALLINT)) {
>> +        return CP_ACCESS_TRAP_EL2;
>> +    }
>> +    return CP_ACCESS_OK;
>> +}
>> +
>> +static const ARMCPRegInfo nmi_reginfo[] = {
>> +    { .name = "ALLINT", .state = ARM_CP_STATE_AA64,
>> +      .opc0 = 3, .opc1 = 0, .opc2 = 0, .crn = 4, .crm = 3,
>> +      .type = ARM_CP_NO_RAW,
>> +      .access = PL1_RW, .accessfn = aa64_allint_access,
>> +      .fieldoffset = offsetof(CPUARMState, pstate),
>> +      .writefn = aa64_allint_write, .readfn = aa64_allint_read,
>> +      .resetfn = arm_cp_reset_ignore },
>> +};
> 
> These functions and the array have been put in a bit of the
> file that is built whether TARGET_AARCH64 is defined or not...
> 
>> +
>>  static uint64_t aa64_pan_read(CPUARMState *env, const ARMCPRegInfo *ri)
>>  {
>>      return env->pstate & PSTATE_PAN;
>> @@ -9889,6 +9919,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>>      if (cpu_isar_feature(aa64_nv2, cpu)) {
>>          define_arm_cp_regs(cpu, nv2_reginfo);
>>      }
>> +
>> +    if (cpu_isar_feature(aa64_nmi, cpu)) {
>> +        define_arm_cp_regs(cpu, nmi_reginfo);
>> +    }
>>  #endif
> 
> ...but the only reference to them is inside an ifdef TARGET_AARCH64.
> 
> Moving the nmi_reginfo[] and the functions so they are next to
> some other TARGET_AARCH64-only reginfo array inside one of the
> existing ifdef blocks is probably the nicest fix.

Thank you! I'll fix it.

> 
>>
>>      if (cpu_isar_feature(any_predinv, cpu)) {
>> --
> 
> thanks
> -- PMM

Re: [RFC PATCH v8 05/23] target/arm: Support MSR access to ALLINT

[Jinjie Ruan via]

On 2024/3/20 1:30, Peter Maydell wrote:
> On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>
>> Support ALLINT msr access as follow:
>>         mrs <xt>, ALLINT        // read allint
>>         msr ALLINT, <xt>        // write allint with imm
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> v5:
>> - Add Reviewed-by.
>> v4:
>> - Remove arm_is_el2_enabled() check in allint_check().
>> - Change to env->pstate instead of env->allint.
>> v3:
>> - Remove EL0 check in aa64_allint_access() which alreay checks in .access
>>   PL1_RW.
>> - Use arm_hcrx_el2_eff() in aa64_allint_access() instead of env->cp15.hcrx_el2.
>> - Make ALLINT msr access function controlled by aa64_nmi.
>> ---
>>  target/arm/helper.c | 34 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index b19a0178ce..aa0151c775 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -4752,6 +4752,36 @@ static void aa64_daif_write(CPUARMState *env, const ARMCPRegInfo *ri,
>>      env->daif = value & PSTATE_DAIF;
>>  }
>>
>> +static void aa64_allint_write(CPUARMState *env, const ARMCPRegInfo *ri,
>> +                              uint64_t value)
>> +{
>> +    env->pstate = (env->pstate & ~PSTATE_ALLINT) | (value & PSTATE_ALLINT);
>> +}
>> +
>> +static uint64_t aa64_allint_read(CPUARMState *env, const ARMCPRegInfo *ri)
>> +{
>> +    return env->pstate & PSTATE_ALLINT;
>> +}
>> +
>> +static CPAccessResult aa64_allint_access(CPUARMState *env,
>> +                                         const ARMCPRegInfo *ri, bool isread)
>> +{
>> +    if (arm_current_el(env) == 1 && (arm_hcrx_el2_eff(env) & HCRX_TALLINT)) {
>> +        return CP_ACCESS_TRAP_EL2;
>> +    }
>> +    return CP_ACCESS_OK;
> 
> Forgot to note in my earlier email: HCRX_EL2.TALLINT traps
> only writes to ALLINT, not reads, so the condition
> here needs to also look at 'isread'.

Thank you! I'll fix it.

> 
>> +}
> 
> thanks
> -- PMM

Re: [RFC PATCH v8 06/23] target/arm: Add support for Non-maskable Interrupt

[Jinjie Ruan via]

On 2024/3/20 3:26, Peter Maydell wrote:
> On Tue, 19 Mar 2024 at 18:51, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 3/19/24 07:28, Peter Maydell wrote:
>>>>       switch (excp_idx) {
>>>> +    case EXCP_NMI:
>>>> +        pstate_unmasked = !allIntMask;
>>>> +        break;
>>>> +
>>>> +    case EXCP_VNMI:
>>>> +        if ((!(hcr_el2 & HCR_IMO) && !(hcr_el2 & HCR_FMO)) ||
>>>> +             (hcr_el2 & HCR_TGE)) {
>>>> +            /* VNMIs(VIRQs or VFIQs) are only taken when hypervized.  */
>>>> +            return false;
>>>> +        }
>>>
>>> VINMI and VFNMI aren't the same thing: do we definitely want to
>>> merge them into one EXCP_VNMI ?
>>
>> We do not, which is why VFNMI is going through EXCP_VFIQ.  A previous version did, and I
>> see the comment did not change to match the new implementation.
> 
> Why do we put VFNMI through VFIQ, though? They're not
> the same thing either... I think I would find this less
> confusing if we implemented what the spec says and distinguished
> all of (IRQ, FIQ, IRQ-with-superpriority and FIQ-with-superpriority).

Should there be separate VNMI lines and exceptions for the
IRQ-with-superpriority and FIQ-with-superpriority ?

> 
>>> The use of the _eff() versions of the functions here is
>>> correct but it introduces a new case where we need to
>>> reevaluate the status of the VNMI etc interrupt status:
>>> when we change from Secure to NonSecure or when we change
>>> SCR_EL3.EEL2 or SCR_EL3.HXEN. We either need to make sure
>>> we reevaluate when we drop from EL3 to EL2 (which would be
>>> OK since VINMI and VFNMI can't be taken at EL3 and none of
>>> these bits can change except at EL3) or else make the calls
>>> to reevaluate them when we write to SCR_EL3. At least, I don't
>>> think we currently reevaluate these bits on an EL change.
>>
>> We re-evaluate these bits on EL change via gicv3_cpuif_el_change_hook.
> 
> Only if the GIC is connected to the VIRQ and VFIQ interrupt lines,
> which it doesn't in theory have to be (though in practice it
> usually will be). Plus it feels a bit fragile against
> somebody deciding to put in a "this line hasn't changed state
> so don't bother calling the handler again" optimization in the
> future.
> 
> thanks
> -- PMM

Re: [RFC PATCH v8 06/23] target/arm: Add support for Non-maskable Interrupt

[Jinjie Ruan via]

On 2024/3/20 1:28, Peter Maydell wrote:
> On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>
>> This only implements the external delivery method via the GICv3.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> v8:
>> - Fix the rcu stall after sending a VNMI in qemu VM.
>> v7:
>> - Add Reviewed-by.
>> v6:
>> - env->cp15.hcr_el2 -> arm_hcr_el2_eff().
>> - env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
>> - Not include VF && VFNMI in CPU_INTERRUPT_VNMI.
>> v4:
>> - Accept NMI unconditionally for arm_cpu_has_work() but add comment.
>> - Change from & to && for EXCP_IRQ or EXCP_FIQ.
>> - Refator nmi mask in arm_excp_unmasked().
>> - Also handle VNMI in arm_cpu_exec_interrupt() and arm_cpu_set_irq().
>> - Rename virtual to Virtual.
>> v3:
>> - Not include CPU_INTERRUPT_NMI when FEAT_NMI not enabled
>> - Add ARM_CPU_VNMI.
>> - Refator nmi mask in arm_excp_unmasked().
>> - Test SCTLR_ELx.NMI for ALLINT mask for NMI.
>> ---
>>  target/arm/cpu-qom.h   |  4 +-
>>  target/arm/cpu.c       | 85 +++++++++++++++++++++++++++++++++++++++---
>>  target/arm/cpu.h       |  4 ++
>>  target/arm/helper.c    |  2 +
>>  target/arm/internals.h |  9 +++++
>>  5 files changed, 97 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
>> index 8e032691db..e0c9e18036 100644
>> --- a/target/arm/cpu-qom.h
>> +++ b/target/arm/cpu-qom.h
>> @@ -36,11 +36,13 @@ 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 six 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
>> +#define ARM_CPU_VNMI 5
>>
>>  /* For M profile, some registers are banked secure vs non-secure;
>>   * these are represented as a 2-element array where the first element
> 
>> @@ -678,13 +687,31 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
>>          return false;
>>      }
>>
>> +    if (cpu_isar_feature(aa64_nmi, env_archcpu(env)) &&
>> +        env->cp15.sctlr_el[target_el] & SCTLR_NMI && cur_el == target_el) {
>> +        allIntMask = env->pstate & PSTATE_ALLINT ||
>> +                     ((env->cp15.sctlr_el[target_el] & SCTLR_SPINTMASK) &&
>> +                      (env->pstate & PSTATE_SP));
>> +    }
>> +
>>      switch (excp_idx) {
>> +    case EXCP_NMI:
>> +        pstate_unmasked = !allIntMask;
>> +        break;
>> +
>> +    case EXCP_VNMI:
>> +        if ((!(hcr_el2 & HCR_IMO) && !(hcr_el2 & HCR_FMO)) ||
>> +             (hcr_el2 & HCR_TGE)) {
>> +            /* VNMIs(VIRQs or VFIQs) are only taken when hypervized.  */
>> +            return false;
>> +        }
> 
> VINMI and VFNMI aren't the same thing: do we definitely want to
> merge them into one EXCP_VNMI ? It feels like it would be simpler
> to keep them separate. Similarly CPU_INTERRUPT_VNMI, and
> arm_cpu_update_vnmi() probably want VINMI and VFNMI versions.

It's not like that. The VFNMI cannot be reported from the GIC, there
will be no opportunity to call arm_cpu_update_vfnmi().
> 
>> +        return !allIntMask;
>>      case EXCP_FIQ:
>> -        pstate_unmasked = !(env->daif & PSTATE_F);
>> +        pstate_unmasked = (!(env->daif & PSTATE_F)) && (!allIntMask);
>>          break;
>>
>>      case EXCP_IRQ:
>> -        pstate_unmasked = !(env->daif & PSTATE_I);
>> +        pstate_unmasked = (!(env->daif & PSTATE_I)) && (!allIntMask);
>>          break;
>>
>>      case EXCP_VFIQ:
>> @@ -692,13 +719,13 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
>>              /* VFIQs are only taken when hypervized.  */
>>              return false;
>>          }
>> -        return !(env->daif & PSTATE_F);
>> +        return !(env->daif & PSTATE_F) && (!allIntMask);
>>      case EXCP_VIRQ:
>>          if (!(hcr_el2 & HCR_IMO) || (hcr_el2 & HCR_TGE)) {
>>              /* VIRQs are only taken when hypervized.  */
>>              return false;
>>          }
>> -        return !(env->daif & PSTATE_I);
>> +        return !(env->daif & PSTATE_I) && (!allIntMask);
>>      case EXCP_VSERR:
>>          if (!(hcr_el2 & HCR_AMO) || (hcr_el2 & HCR_TGE)) {
>>              /* VIRQs are only taken when hypervized.  */
>> @@ -804,6 +831,24 @@ 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_VNMI) {
>> +            excp_idx = EXCP_VNMI;
>> +            target_el = 1;
>> +            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);
>> @@ -900,6 +945,28 @@ void arm_cpu_update_vfiq(ARMCPU *cpu)
>>      }
>>  }
>>
>> +void arm_cpu_update_vnmi(ARMCPU *cpu)
>> +{
>> +    /*
>> +     * Update the interrupt level for VNMI, which is the logical OR of
>> +     * the HCRX_EL2.VINMI bit and the input line level from the GIC.
>> +     */
>> +    CPUARMState *env = &cpu->env;
>> +    CPUState *cs = CPU(cpu);
>> +
>> +    bool new_state = ((arm_hcr_el2_eff(env) & HCR_VI) &&
>> +                      (arm_hcrx_el2_eff(env) & HCRX_VINMI)) ||
>> +        (env->irq_line_state & CPU_INTERRUPT_VNMI);
>> +
>> +    if (new_state != ((cs->interrupt_request & CPU_INTERRUPT_VNMI) != 0)) {
>> +        if (new_state) {
>> +            cpu_interrupt(cs, CPU_INTERRUPT_VNMI);
>> +        } else {
>> +            cpu_reset_interrupt(cs, CPU_INTERRUPT_VNMI);
>> +        }
>> +    }
>> +}
> 
> I think Richard noted on a previous version of the series that
> the existing arm_cpu_update_virq() and arm_cpu_update_vfiq()
> also need changing so they don't set CPU_INTERRUPT_VIRQ
> or CPU_INTERRUPT_VFIQ if the HCRX_EL2 bits indicate that
> we should be signalling a VINMI or VFNMI instead.
> That also means that VIRQ and VFIQ will change values based
> on changes in HCRX_EL2, which means that hcrx_write() needs
> to have calls to arm_cpu_update_{virq,vfiq,vnmi} the way
> that do_hcr_write() already does.
> 
> The use of the _eff() versions of the functions here is
> correct but it introduces a new case where we need to
> reevaluate the status of the VNMI etc interrupt status:
> when we change from Secure to NonSecure or when we change
> SCR_EL3.EEL2 or SCR_EL3.HXEN. We either need to make sure
> we reevaluate when we drop from EL3 to EL2 (which would be
> OK since VINMI and VFNMI can't be taken at EL3 and none of
> these bits can change except at EL3) or else make the calls
> to reevaluate them when we write to SCR_EL3. At least, I don't
> think we currently reevaluate these bits on an EL change.
> 
>> +
>>  void arm_cpu_update_vserr(ARMCPU *cpu)
>>  {
>>      /*
>> @@ -929,7 +996,9 @@ 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,
>> +        [ARM_CPU_VNMI] = CPU_INTERRUPT_VNMI
>>      };
>>
>>      if (!arm_feature(env, ARM_FEATURE_EL2) &&
>> @@ -955,8 +1024,12 @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)
>>      case ARM_CPU_VFIQ:
>>          arm_cpu_update_vfiq(cpu);
>>          break;
>> +    case ARM_CPU_VNMI:
>> +        arm_cpu_update_vnmi(cpu);
>> +        break;
>>      case ARM_CPU_IRQ:
>>      case ARM_CPU_FIQ:
>> +    case ARM_CPU_NMI:
>>          if (level) {
>>              cpu_interrupt(cs, mask[irq]);
>>          } else {
>> @@ -1355,7 +1428,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, 6);
> 
> You should also update the value passed when we init the
> GPIOs in the arm_cpu_kvm_set_irq case, and update the comment
> that explains why, which currently reads:
> 
>         /* VIRQ and VFIQ are unused with KVM but we add them to maintain
>          * the same interface as non-KVM CPUs.
>          */
> 
> so it mentions also the NMI and VNMI inputs.
> 
>>      }
>>
>>      qdev_init_gpio_out(DEVICE(cpu), cpu->gt_timer_outputs,
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index de740d223f..629221e1a9 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -61,6 +61,8 @@
>>  #define EXCP_DIVBYZERO      23   /* v7M DIVBYZERO UsageFault */
>>  #define EXCP_VSERR          24
>>  #define EXCP_GPC            25   /* v9 Granule Protection Check Fault */
>> +#define EXCP_NMI            26
>> +#define EXCP_VNMI           27
>>  /* NB: add new EXCP_ defines to the array in arm_log_exception() too */
>>
>>  #define ARMV7M_EXCP_RESET   1
>> @@ -80,6 +82,8 @@
>>  #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
>> +#define CPU_INTERRUPT_VNMI  CPU_INTERRUPT_TGT_EXT_0
> 
>>  /* 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 aa0151c775..875a7fa8da 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -10793,6 +10793,8 @@ void arm_log_exception(CPUState *cs)
>>              [EXCP_DIVBYZERO] = "v7M DIVBYZERO UsageFault",
>>              [EXCP_VSERR] = "Virtual SERR",
>>              [EXCP_GPC] = "Granule Protection Check",
>> +            [EXCP_NMI] = "NMI",
>> +            [EXCP_VNMI] = "Virtual NMI"
>>          };
>>
>>          if (idx >= 0 && idx < ARRAY_SIZE(excnames)) {
>> diff --git a/target/arm/internals.h b/target/arm/internals.h
>> index 516e0584bf..cb217a9ce7 100644
>> --- a/target/arm/internals.h
>> +++ b/target/arm/internals.h
>> @@ -1109,6 +1109,15 @@ void arm_cpu_update_virq(ARMCPU *cpu);
>>   */
>>  void arm_cpu_update_vfiq(ARMCPU *cpu);
>>
>> +/**
>> + * arm_cpu_update_vnmi: Update CPU_INTERRUPT_VNMI bit in cs->interrupt_request
>> + *
>> + * Update the CPU_INTERRUPT_VNMI bit in cs->interrupt_request, following
>> + * a change to either the input VNMI line from the GIC or the HCRX_EL2.VINMI.
>> + * Must be called with the BQL held.
>> + */
>> +void arm_cpu_update_vnmi(ARMCPU *cpu);
> 
> thanks
> -- PMM

Re: [RFC PATCH v8 06/23] target/arm: Add support for Non-maskable Interrupt

[Peter Maydell]

On Thu, 21 Mar 2024 at 09:27, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
>
>
> On 2024/3/20 1:28, Peter Maydell wrote:
> > On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> >>
> >> This only implements the external delivery method via the GICv3.
> >>
> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >> v8:
> >> - Fix the rcu stall after sending a VNMI in qemu VM.
> >> v7:
> >> - Add Reviewed-by.
> >> v6:
> >> - env->cp15.hcr_el2 -> arm_hcr_el2_eff().
> >> - env->cp15.hcrx_el2 -> arm_hcrx_el2_eff().
> >> - Not include VF && VFNMI in CPU_INTERRUPT_VNMI.
> >> v4:
> >> - Accept NMI unconditionally for arm_cpu_has_work() but add comment.
> >> - Change from & to && for EXCP_IRQ or EXCP_FIQ.
> >> - Refator nmi mask in arm_excp_unmasked().
> >> - Also handle VNMI in arm_cpu_exec_interrupt() and arm_cpu_set_irq().
> >> - Rename virtual to Virtual.
> >> v3:
> >> - Not include CPU_INTERRUPT_NMI when FEAT_NMI not enabled
> >> - Add ARM_CPU_VNMI.
> >> - Refator nmi mask in arm_excp_unmasked().
> >> - Test SCTLR_ELx.NMI for ALLINT mask for NMI.
> >> ---
> >>  target/arm/cpu-qom.h   |  4 +-
> >>  target/arm/cpu.c       | 85 +++++++++++++++++++++++++++++++++++++++---
> >>  target/arm/cpu.h       |  4 ++
> >>  target/arm/helper.c    |  2 +
> >>  target/arm/internals.h |  9 +++++
> >>  5 files changed, 97 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
> >> index 8e032691db..e0c9e18036 100644
> >> --- a/target/arm/cpu-qom.h
> >> +++ b/target/arm/cpu-qom.h
> >> @@ -36,11 +36,13 @@ 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 six 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
> >> +#define ARM_CPU_VNMI 5
> >>
> >>  /* For M profile, some registers are banked secure vs non-secure;
> >>   * these are represented as a 2-element array where the first element
> >
> >> @@ -678,13 +687,31 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
> >>          return false;
> >>      }
> >>
> >> +    if (cpu_isar_feature(aa64_nmi, env_archcpu(env)) &&
> >> +        env->cp15.sctlr_el[target_el] & SCTLR_NMI && cur_el == target_el) {
> >> +        allIntMask = env->pstate & PSTATE_ALLINT ||
> >> +                     ((env->cp15.sctlr_el[target_el] & SCTLR_SPINTMASK) &&
> >> +                      (env->pstate & PSTATE_SP));
> >> +    }
> >> +
> >>      switch (excp_idx) {
> >> +    case EXCP_NMI:
> >> +        pstate_unmasked = !allIntMask;
> >> +        break;
> >> +
> >> +    case EXCP_VNMI:
> >> +        if ((!(hcr_el2 & HCR_IMO) && !(hcr_el2 & HCR_FMO)) ||
> >> +             (hcr_el2 & HCR_TGE)) {
> >> +            /* VNMIs(VIRQs or VFIQs) are only taken when hypervized.  */
> >> +            return false;
> >> +        }
> >
> > VINMI and VFNMI aren't the same thing: do we definitely want to
> > merge them into one EXCP_VNMI ? It feels like it would be simpler
> > to keep them separate. Similarly CPU_INTERRUPT_VNMI, and
> > arm_cpu_update_vnmi() probably want VINMI and VFNMI versions.
>
> It's not like that. The VFNMI cannot be reported from the GIC, there
> will be no opportunity to call arm_cpu_update_vfnmi().

The GIC can't trigger it, but the hypervisor can by setting
the HCRX_EL2.VFNMI bit. So writes to HCRX_EL2 (and HCR_EL2)
would need to call arm_cpu_update_vfnmi().

-- PMM

Re: [RFC PATCH v8 06/23] target/arm: Add support for Non-maskable Interrupt

[Peter Maydell]

On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> This only implements the external delivery method via the GICv3.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

> @@ -692,13 +719,13 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
>              /* VFIQs are only taken when hypervized.  */
>              return false;
>          }
> -        return !(env->daif & PSTATE_F);
> +        return !(env->daif & PSTATE_F) && (!allIntMask);
>      case EXCP_VIRQ:
>          if (!(hcr_el2 & HCR_IMO) || (hcr_el2 & HCR_TGE)) {
>              /* VIRQs are only taken when hypervized.  */
>              return false;
>          }
> -        return !(env->daif & PSTATE_I);
> +        return !(env->daif & PSTATE_I) && (!allIntMask);
>      case EXCP_VSERR:
>          if (!(hcr_el2 & HCR_AMO) || (hcr_el2 & HCR_TGE)) {
>              /* VIRQs are only taken when hypervized.  */
> @@ -804,6 +831,24 @@ 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_VNMI) {
> +            excp_idx = EXCP_VNMI;
> +            target_el = 1;
> +            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);

This part adds handling for taking IRQNMI and VIRQNMI, but not
VFIQNMI. So because at the moment we merge VFIQNMI and VFIQ
they both come out as interrupt_request having CPU_INTERRUPT_VFIQ
set. But we don't handle that in this function or in
arm_excp_unmasked(), so we treat it exactly the same as VFIQ, which
means that if PSTATE.F is 1 then we will incorrectly fail to take
the VFIQNMI.

I think the code is going to be a lot more straightforward
if we keep all of these things separate. Compare how we handle
VSError, which is also (for QEMU) something where only the
virtual version exists and which is only triggered via the
HCR bits.

thanks
-- PMM

Re: [RFC PATCH v8 13/23] hw/intc/arm_gicv3: Add irq superpriority information

[Peter Maydell]

On Mon, 18 Mar 2024 at 09:38, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
> A SPI, PPI or SGI interrupt can have a superpriority property. So
> maintain superpriority information in PendingIrq and GICR/GICD.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v3:
> - Place this ahead of implement GICR_INMIR.
> - Add Acked-by.
> ---
>  include/hw/intc/arm_gicv3_common.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
> index 7324c7d983..df4380141d 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -146,6 +146,7 @@ typedef struct {
>      int irq;
>      uint8_t prio;
>      int grp;
> +    bool superprio;
>  } PendingIrq;
>
>  struct GICv3CPUState {
> @@ -172,6 +173,7 @@ struct GICv3CPUState {
>      uint32_t gicr_ienabler0;
>      uint32_t gicr_ipendr0;
>      uint32_t gicr_iactiver0;
> +    uint32_t gicr_isuperprio;

This field stores the state that is in the GICR_INMIR0
register, so please name it that way: gicr_inmir0.

>      uint32_t edge_trigger; /* ICFGR0 and ICFGR1 even bits */
>      uint32_t gicr_igrpmodr0;
>      uint32_t gicr_nsacr;
> @@ -274,6 +276,7 @@ struct GICv3State {
>      GIC_DECLARE_BITMAP(active);       /* GICD_ISACTIVER */
>      GIC_DECLARE_BITMAP(level);        /* Current level */
>      GIC_DECLARE_BITMAP(edge_trigger); /* GICD_ICFGR even bits */
> +    GIC_DECLARE_BITMAP(superprio);    /* GICD_INMIR */
>      uint8_t gicd_ipriority[GICV3_MAXIRQ];
>      uint64_t gicd_irouter[GICV3_MAXIRQ];
>      /* Cached information: pointer to the cpu i/f for the CPUs specified
> @@ -313,6 +316,7 @@ GICV3_BITMAP_ACCESSORS(pending)
>  GICV3_BITMAP_ACCESSORS(active)
>  GICV3_BITMAP_ACCESSORS(level)
>  GICV3_BITMAP_ACCESSORS(edge_trigger)
> +GICV3_BITMAP_ACCESSORS(superprio)

This is the state behind the GICD_INMIR<n> registers, and
the GIC spec calls the bits in those registers NMI<x>,
so I would call this bitmap nmi, not superprio.

This commit adds new device state, so it also needs to be migrated.
You'll want to add a new subsection to vmstate_gicv3_cpu which
is present if the GIC implements NMIs, and which has an entry
for the gicr_inmir0 field. Similarly, you want a new subsection
in vmstate_gicv3 which is present if NMIs are implemented and which
has a field for the nmi array.

thanks
-- PMM

Re: [RFC PATCH v8 13/23] hw/intc/arm_gicv3: Add irq superpriority information

[Jinjie Ruan via]

On 2024/3/21 21:17, Peter Maydell wrote:
> On Mon, 18 Mar 2024 at 09:38, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>
>> A SPI, PPI or SGI interrupt can have a superpriority property. So
>> maintain superpriority information in PendingIrq and GICR/GICD.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> Acked-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> v3:
>> - Place this ahead of implement GICR_INMIR.
>> - Add Acked-by.
>> ---
>>  include/hw/intc/arm_gicv3_common.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
>> index 7324c7d983..df4380141d 100644
>> --- a/include/hw/intc/arm_gicv3_common.h
>> +++ b/include/hw/intc/arm_gicv3_common.h
>> @@ -146,6 +146,7 @@ typedef struct {
>>      int irq;
>>      uint8_t prio;
>>      int grp;
>> +    bool superprio;
>>  } PendingIrq;
>>
>>  struct GICv3CPUState {
>> @@ -172,6 +173,7 @@ struct GICv3CPUState {
>>      uint32_t gicr_ienabler0;
>>      uint32_t gicr_ipendr0;
>>      uint32_t gicr_iactiver0;
>> +    uint32_t gicr_isuperprio;
> 
> This field stores the state that is in the GICR_INMIR0
> register, so please name it that way: gicr_inmir0.
> 
>>      uint32_t edge_trigger; /* ICFGR0 and ICFGR1 even bits */
>>      uint32_t gicr_igrpmodr0;
>>      uint32_t gicr_nsacr;
>> @@ -274,6 +276,7 @@ struct GICv3State {
>>      GIC_DECLARE_BITMAP(active);       /* GICD_ISACTIVER */
>>      GIC_DECLARE_BITMAP(level);        /* Current level */
>>      GIC_DECLARE_BITMAP(edge_trigger); /* GICD_ICFGR even bits */
>> +    GIC_DECLARE_BITMAP(superprio);    /* GICD_INMIR */
>>      uint8_t gicd_ipriority[GICV3_MAXIRQ];
>>      uint64_t gicd_irouter[GICV3_MAXIRQ];
>>      /* Cached information: pointer to the cpu i/f for the CPUs specified
>> @@ -313,6 +316,7 @@ GICV3_BITMAP_ACCESSORS(pending)
>>  GICV3_BITMAP_ACCESSORS(active)
>>  GICV3_BITMAP_ACCESSORS(level)
>>  GICV3_BITMAP_ACCESSORS(edge_trigger)
>> +GICV3_BITMAP_ACCESSORS(superprio)
> 
> This is the state behind the GICD_INMIR<n> registers, and
> the GIC spec calls the bits in those registers NMI<x>,
> so I would call this bitmap nmi, not superprio.
> 
> This commit adds new device state, so it also needs to be migrated.
> You'll want to add a new subsection to vmstate_gicv3_cpu which
> is present if the GIC implements NMIs, and which has an entry
> for the gicr_inmir0 field. Similarly, you want a new subsection
> in vmstate_gicv3 which is present if NMIs are implemented and which
> has a field for the nmi array.

OK, I'll add it.

> 
> thanks
> -- PMM

Re: [RFC PATCH v8 09/23] target/arm: Handle PSTATE.ALLINT on taking an exception

[Jinjie Ruan via]

On 2024/3/20 0:47, Peter Maydell wrote:
> On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>
>> Set or clear PSTATE.ALLINT on taking an exception to ELx according to the
>> SCTLR_ELx.SPINTMASK bit.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> v3:
>> - Add Reviewed-by.
>> ---
>>  target/arm/helper.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 4bc63bf7ca..81f4a8f194 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -11705,6 +11705,15 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>>          }
>>      }
>>
>> +    if (cpu_isar_feature(aa64_nmi, cpu) &&
>> +        (env->cp15.sctlr_el[new_el] & SCTLR_NMI)) {
> 
> This shouldn't be checking the value of SCTLR_NMI here:
> the new PSTATE.ALLINT is set to !SPINTMASK even if NMI == 0.
> (The SPINTMASK bit description is a bit confusing, but
> the correct behaviour is clear in the AArch64.TakeException()
> pseudocode.)

It seems unreasonable to remove the SCTLR_NMI check, because if the
hardware supports FEAT_NMI but the kernel do not enable it, the ALLINT
bit in pstate will also set or clear when an exception is caught, which
seems unreasonable.

> 
>> +        if (!(env->cp15.sctlr_el[new_el] & SCTLR_SPINTMASK)) {
>> +            new_mode |= PSTATE_ALLINT;
>> +        } else {
>> +            new_mode &= ~PSTATE_ALLINT;
>> +        }
>> +    }
>> +
>>      pstate_write(env, PSTATE_DAIF | new_mode);
>>      env->aarch64 = true;
>>      aarch64_restore_sp(env, new_el);
>> --
>> 2.34.1
>>
> 
> thanks
> -- PMM

Re: [RFC PATCH v8 09/23] target/arm: Handle PSTATE.ALLINT on taking an exception

[Peter Maydell]

On Thu, 28 Mar 2024 at 08:57, Jinjie Ruan via <qemu-arm@nongnu.org> wrote:
>
>
>
> On 2024/3/20 0:47, Peter Maydell wrote:
> > On Mon, 18 Mar 2024 at 09:37, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> >>
> >> Set or clear PSTATE.ALLINT on taking an exception to ELx according to the
> >> SCTLR_ELx.SPINTMASK bit.
> >>
> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >> v3:
> >> - Add Reviewed-by.
> >> ---
> >>  target/arm/helper.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/target/arm/helper.c b/target/arm/helper.c
> >> index 4bc63bf7ca..81f4a8f194 100644
> >> --- a/target/arm/helper.c
> >> +++ b/target/arm/helper.c
> >> @@ -11705,6 +11705,15 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
> >>          }
> >>      }
> >>
> >> +    if (cpu_isar_feature(aa64_nmi, cpu) &&
> >> +        (env->cp15.sctlr_el[new_el] & SCTLR_NMI)) {
> >
> > This shouldn't be checking the value of SCTLR_NMI here:
> > the new PSTATE.ALLINT is set to !SPINTMASK even if NMI == 0.
> > (The SPINTMASK bit description is a bit confusing, but
> > the correct behaviour is clear in the AArch64.TakeException()
> > pseudocode.)
>
> It seems unreasonable to remove the SCTLR_NMI check, because if the
> hardware supports FEAT_NMI but the kernel do not enable it, the ALLINT
> bit in pstate will also set or clear when an exception is caught, which
> seems unreasonable.

Whether we personally think it is "unreasonable" or not does not
matter here. The architecture says that we must not check SCTLR_NMI,
and therefore we must not check SCTLR_NMI, or we will be implementing
the wrong behaviour.

thanks
-- PMM