On 2/23/24 00:32, Jinjie Ruan via wrote:
This only implements the external delivery method via the GICv3.

Signed-off-by: Jinjie Ruan <[email protected]>
---
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     | 54 ++++++++++++++++++++++++++++++++++++--------
  target/arm/cpu.h     |  4 ++++
  target/arm/helper.c  |  2 ++
  4 files changed, 54 insertions(+), 10 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 5fa86bc8d5..d40ada9c75 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -126,11 +126,20 @@ static bool arm_cpu_has_work(CPUState *cs)
  {
      ARMCPU *cpu = ARM_CPU(cs);
- return (cpu->power_state != PSCI_OFF)
-        && cs->interrupt_request &
-        (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
-         | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ | CPU_INTERRUPT_VSERR
-         | CPU_INTERRUPT_EXITTB);
+    if (cpu_isar_feature(aa64_nmi, cpu)) {
+        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);
+    } else {
+        return (cpu->power_state != PSCI_OFF)
+            && cs->interrupt_request &
+            (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD
+             | CPU_INTERRUPT_VFIQ | CPU_INTERRUPT_VIRQ | CPU_INTERRUPT_VSERR
+             | CPU_INTERRUPT_EXITTB);
+    }

This can be factored better, to avoid repeating everything.

However, I am reconsidering my previous advice to ignore NMI if FEAT_NMI is not 
present.

Consider R_MHWBP, where IRQ with Superpriority, with SCTLR_ELx.NMI == 0, 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. But document this choice here with a comment.


@@ -678,13 +688,26 @@ static inline bool arm_excp_unmasked(CPUState *cs, 
unsigned int excp_idx,
          return false;
      }
+ if (cpu_isar_feature(aa64_nmi, env_archcpu(env))) {
+        nmi_unmasked = (cur_el == target_el) &&
+                       (((env->cp15.sctlr_el[target_el] & SCTLR_NMI) &&
+                        (env->allint & PSTATE_ALLINT)) ||
+                        ((env->cp15.sctlr_el[target_el] & SCTLR_SPINTMASK) &&
+                        (env->pstate & PSTATE_SP)));

In the manual, this is "allintmask".  It is easier to follow the logic if you 
use this...

+        nmi_unmasked = !nmi_unmasked;

... and not the inverse.

      case EXCP_FIQ:
-        pstate_unmasked = !(env->daif & PSTATE_F);
+        pstate_unmasked = (!(env->daif & PSTATE_F)) & nmi_unmasked;

Clearer with "&&".

+    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;
+            }
+        }
+    }

Handling for vNMI?

@@ -957,6 +992,7 @@ static void arm_cpu_set_irq(void *opaque, int irq, int 
level)
          break;
      case ARM_CPU_IRQ:
      case ARM_CPU_FIQ:
+    case ARM_CPU_NMI:
          if (level) {
              cpu_interrupt(cs, mask[irq]);
          } else {

Likewise.


r~

Reply via email to