Re: [RFC PATCH 36/36] irqchip/gic-v3: Implement FEAT_GICv3_NMI support

[Jinjie Ruan <ruanjinjie@huawei.com>]

On 7/9/2026 8:13 PM, Vladimir Murzin wrote:
> From: Lorenzo Pieralisi <lpieralisi@kernel.org>
> 
> The FEAT_GICv3_NMI GIC feature coupled with the CPU FEAT_NMI enables
> handling NMI interrupts in HW on aarch64, by adding a superpriority
> interrupt to the existing GIC priority scheme.
> 
> Implement GIC driver support for the FEAT_GICv3_NMI feature.
> 
> Rename gic_supports_nmi() helper function to gic_supports_pseudo_nmis()
> to make the pseudo NMIs code path clearer and more explicit.
> 
> Check, through the ARM64 capabilitity infrastructure, if support
> for FEAT_NMI was detected on the core and the system has not overridden
> the detection and forced pseudo-NMIs enablement.
> 
> If FEAT_NMI is detected, it was not overridden (check embedded in the
> system_uses_nmi() call) and the GIC supports the FEAT_GICv3_NMI
> feature, initialize NMIs related HW GIC registers and route irq to NMI
> handling logic.
> 
> Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  drivers/irqchip/irq-gic-v3.c       | 122 +++++++++++++++++++++++++----
>  include/linux/irqchip/arm-gic-v3.h |   4 +
>  2 files changed, 111 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 94c6a3f2b009..625be4fc06b4 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -64,6 +64,7 @@ struct gic_chip_data {
>  	u32			nr_redist_regions;
>  	u64			flags;
>  	bool			has_rss;
> +	bool			has_nmi;
>  	unsigned int		ppi_nr;
>  	struct partition_affinity *parts;
>  	unsigned int		nr_parts;
> @@ -87,7 +88,7 @@ static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
>  #define GIC_LINE_NR	min(GICD_TYPER_SPIS(gic_data.rdists.gicd_typer), 1020U)
>  #define GIC_ESPI_NR	GICD_TYPER_ESPIS(gic_data.rdists.gicd_typer)
>  
> -static bool nmi_support_forbidden;
> +static bool pnmi_support_forbidden;
>  
>  /*
>   * There are 16 SGIs, though we only actually use 8 in Linux. The other 8 SGIs
> @@ -185,7 +186,7 @@ static void __init gic_prio_init(void)
>  				pr_warn("Broken GIC integration, security disabled\n");
>  		} else {
>  			pr_warn("Broken GIC integration, pNMI forbidden\n");
> -			nmi_support_forbidden = true;
> +			pnmi_support_forbidden = true;
>  		}
>  	}
>  
> @@ -252,6 +253,37 @@ enum gic_intid_range {
>  	__INVALID_RANGE__
>  };
>  
> +#ifdef CONFIG_ARM64
> +#include <asm/cpufeature.h>
> +
> +static inline bool gic_supports_v3_3_nmi(void)
> +{
> +	return gic_data.has_nmi && system_uses_nmi();
> +}
> +
> +static inline u64 gic_read_nmiar(void)
> +{
> +	u64 irqstat;
> +
> +	irqstat = read_sysreg_s(SYS_ICC_NMIAR1_EL1);
> +
> +	dsb(sy);
> +
> +	return irqstat;
> +}
> +
> +#else
> +static inline bool gic_supports_v3_3_nmi(void)
> +{
> +	return false;
> +}
> +
> +static inline u64 gic_read_nmiar(void)
> +{
> +	BUG();
> +}

I think the BUG() here is unnecessary because arm32 will not detect the
feat_nmi feature; a warning or an empty statement would be sufficient.

> +#endif
> +
>  static enum gic_intid_range __get_intid_range(irq_hw_number_t hwirq)
>  {
>  	switch (hwirq) {
> @@ -438,6 +470,8 @@ static u32 convert_offset_index(struct irq_data *d, u32 offset, u32 *index)
>  			return GICD_ICFGRnE;
>  		case GICD_IROUTER:
>  			return GICD_IROUTERnE;
> +		case GICD_INMIR:
> +			return GICD_INMIRnE;
>  		default:
>  			break;
>  		}
> @@ -467,6 +501,42 @@ static int gic_peek_irq(struct irq_data *d, u32 offset)
>  	return !!(readl_relaxed(base + offset + (index / 32) * 4) & mask);
>  }
>  
> +static DEFINE_RAW_SPINLOCK(irq_controller_lock);
> +
> +static void gic_irq_configure_nmi(struct irq_data *d, bool enable)
> +{
> +	void __iomem *base, *addr;
> +	u32 offset, index, mask, val;
> +
> +	offset = convert_offset_index(d, GICD_INMIR, &index);
> +	mask = 1 << (index % 32);
> +
> +	if (gic_irq_in_rdist(d))
> +		base = gic_data_rdist_sgi_base();
> +	else
> +		base = gic_dist_base_alias(d);
> +
> +	addr = base + offset + (index / 32) * 4;
> +
> +	raw_spin_lock(&irq_controller_lock);
> +
> +	val = readl_relaxed(addr);
> +	val = enable ? (val | mask) : (val & ~mask);
> +	writel_relaxed(val, addr);
> +
> +	raw_spin_unlock(&irq_controller_lock);
> +}
> +
> +static void gic_irq_enable_nmi(struct irq_data *d)
> +{
> +	gic_irq_configure_nmi(d, true);
> +}
> +
> +static void gic_irq_disable_nmi(struct irq_data *d)
> +{
> +	gic_irq_configure_nmi(d, false);
> +}
> +
>  static void gic_poke_irq(struct irq_data *d, u32 offset)
>  {
>  	void __iomem *base;
> @@ -512,7 +582,7 @@ static void gic_unmask_irq(struct irq_data *d)
>  	gic_poke_irq(d, GICD_ISENABLER);
>  }
>  
> -static inline bool gic_supports_nmi(void)
> +static inline bool gic_supports_pseudo_nmis(void)
>  {
>  	return IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) &&
>  	       static_branch_likely(&supports_pseudo_nmis);
> @@ -598,7 +668,7 @@ static int gic_irq_nmi_setup(struct irq_data *d)
>  {
>  	struct irq_desc *desc = irq_to_desc(d->irq);
>  
> -	if (!gic_supports_nmi())
> +	if (!gic_supports_pseudo_nmis() && !gic_supports_v3_3_nmi())
>  		return -EINVAL;
>  
>  	if (gic_peek_irq(d, GICD_ISENABLER)) {
> @@ -617,7 +687,10 @@ static int gic_irq_nmi_setup(struct irq_data *d)
>  	if (!gic_irq_in_rdist(d))
>  		desc->handle_irq = handle_fasteoi_nmi;
>  
> -	gic_irq_set_prio(d, dist_prio_nmi);
> +	if (gic_supports_v3_3_nmi())
> +		gic_irq_enable_nmi(d);
> +	else
> +		gic_irq_set_prio(d, dist_prio_nmi);
>  
>  	return 0;
>  }
> @@ -626,7 +699,7 @@ static void gic_irq_nmi_teardown(struct irq_data *d)
>  {
>  	struct irq_desc *desc = irq_to_desc(d->irq);
>  
> -	if (WARN_ON(!gic_supports_nmi()))
> +	if (WARN_ON(!gic_supports_pseudo_nmis() && !gic_supports_v3_3_nmi()))
>  		return;
>  
>  	if (gic_peek_irq(d, GICD_ISENABLER)) {
> @@ -645,7 +718,10 @@ static void gic_irq_nmi_teardown(struct irq_data *d)
>  	if (!gic_irq_in_rdist(d))
>  		desc->handle_irq = handle_fasteoi_irq;
>  
> -	gic_irq_set_prio(d, dist_prio_irq);
> +	if (gic_supports_v3_3_nmi())
> +		gic_irq_disable_nmi(d);
> +	else
> +		gic_irq_set_prio(d, dist_prio_irq);
>  }
>  
>  static bool gic_arm64_erratum_2941627_needed(struct irq_data *d)
> @@ -804,7 +880,7 @@ static inline void gic_complete_ack(u32 irqnr)
>  
>  static bool gic_rpr_is_nmi_prio(void)
>  {
> -	if (!gic_supports_nmi())
> +	if (!gic_supports_pseudo_nmis())
>  		return false;
>  
>  	return unlikely(gic_read_rpr() == GICV3_PRIO_NMI);
> @@ -836,7 +912,8 @@ static void __gic_handle_nmi(u32 irqnr, struct pt_regs *regs)
>  	gic_complete_ack(irqnr);
>  
>  	if (generic_handle_domain_nmi(gic_data.domain, irqnr)) {
> -		WARN_ONCE(true, "Unexpected pseudo-NMI (irqnr %u)\n", irqnr);
> +		WARN_ONCE(true, "Unexpected %sNMI (irqnr %u)\n",
> +			  gic_supports_pseudo_nmis() ? "pseudo-" : "", irqnr);
>  		gic_deactivate_unhandled(irqnr);
>  	}
>  }
> @@ -911,7 +988,11 @@ static void __gic_handle_irq_from_irqsoff(struct pt_regs *regs)
>  
>  static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
>  {
> -	if (unlikely(gic_supports_nmi() && !interrupts_enabled(regs)))
> +	if (gic_supports_v3_3_nmi() && in_nmi()) {
> +		u32 irqnr = gic_read_nmiar();
> +
> +		__gic_handle_nmi(irqnr, regs);
> +	} else if (unlikely(gic_supports_pseudo_nmis() && !interrupts_enabled(regs)))
>  		__gic_handle_irq_from_irqsoff(regs);

Can we rename __gic_handle_irq_from_irqsoff to gic_handle_pseudo_nmi,
and then encapsulate the logic for obtaining the pseudo-NMI interrupt
number into a function similar to gic_read_nmiar()， such as
gic_read_pseudo_nmi_irqnr()? This would make the code look cleaner.


For example, similar to the following code:

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 2835e8f342be..1184f9446f8c 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -260,28 +260,11 @@ bool gic_supports_v3_3_nmi(void)
 {
        return gic_data.has_nmi && system_uses_nmi();
 }
-
-static inline u64 gic_read_nmiar(void)
-{
-       u64 irqstat;
-
-       irqstat = read_sysreg_s(SYS_ICC_NMIAR1_EL1);
-
-       dsb(sy);
-
-       return irqstat;
-}
-
 #else
 static inline bool gic_supports_v3_3_nmi(void)
 {
        return false;
 }
-
-static inline u64 gic_read_nmiar(void)
-{
-       BUG();
-}
 #endif

 static enum gic_intid_range __get_intid_range(irq_hw_number_t hwirq)
@@ -961,7 +944,7 @@ static void __gic_handle_irq_from_irqson(struct
pt_regs *regs)
  *
  * The entry code has performed NMI entry.
  */
-static void __gic_handle_irq_from_irqsoff(struct pt_regs *regs)
+static u32 gic_read_pseudo_nmi_irqnr(struct pt_regs *regs)
 {
        u64 pmr;
        u32 irqnr;
@@ -985,19 +968,42 @@ static void __gic_handle_irq_from_irqsoff(struct
pt_regs *regs)
        irqnr = gic_read_iar();
        gic_write_pmr(pmr);

+       return irqnr;
+}
+
+#ifdef CONFIG_ARM64
+static inline u64 gic_read_nmiar(void)
+{
+       u64 irqstat;
+
+       irqstat = read_sysreg_s(SYS_ICC_NMIAR1_EL1);
+
+       dsb(sy);
+
+       return irqstat;
+}
+#else
+static inline u64 gic_read_nmiar(void) { }
+#endif
+
+static void gic_handle_nmi(struct pt_regs *regs)
+{
+       u32 irqnr;
+
+       if (gic_supports_v3_3_nmi())
+               irqnr = gic_read_nmiar();
+       else if (unlikely(gic_supports_pseudo_nmis()))
+               irqnr = gic_read_pseudo_nmi_irqnr(regs);
+
        __gic_handle_nmi(irqnr, regs);
 }

 static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
 {
-       if (gic_supports_v3_3_nmi() && in_nmi()) {
-               u32 irqnr = gic_read_nmiar();
+       if (in_nmi() || !interrupts_enabled(regs))
+               return gic_handle_nmi(regs);

-               __gic_handle_nmi(irqnr, regs);
-       } else if (unlikely(gic_supports_pseudo_nmis() &&
!interrupts_enabled(regs)))
-               __gic_handle_irq_from_irqsoff(regs);
-       else
-               __gic_handle_irq_from_irqson(regs);
+       __gic_handle_irq_from_irqson(regs);
 }

otherwise, LGTM
Reviewed-by: Jinjie Ruan <ruanjinjie@huawei.com>


>  	else
>  		__gic_handle_irq_from_irqson(regs);
> @@ -1162,7 +1243,7 @@ static void gic_cpu_sys_reg_init(void)
>  	/* Set priority mask register */
>  	if (!gic_prio_masking_enabled()) {
>  		write_gicreg(DEFAULT_PMR_VALUE, ICC_PMR_EL1);
> -	} else if (gic_supports_nmi()) {
> +	} else if (gic_supports_pseudo_nmis()) {
>  		/*
>  		 * Check that all CPUs use the same priority space.
>  		 *
> @@ -1955,15 +2036,25 @@ static const struct gic_quirk gic_quirks[] = {
>  	}
>  };
>  
> -static void gic_enable_nmi_support(void)
> +static void gic_enable_pseudo_nmis(void)
>  {
> -	if (!gic_prio_masking_enabled() || nmi_support_forbidden)
> -		return;
> -
>  	pr_info("Pseudo-NMIs enabled using %s ICC_PMR_EL1 synchronisation\n",
>  		gic_has_relaxed_pmr_sync() ? "relaxed" : "forced");
>  
>  	static_branch_enable(&supports_pseudo_nmis);
> +}
> +
> +static void gic_enable_nmi_support(void)
> +{
> +	if ((!gic_prio_masking_enabled() || pnmi_support_forbidden) && !gic_supports_v3_3_nmi())
> +		return;
> +
> +	/*
> +	 * Initialize pseudo-NMIs only if GIC driver cannot take advantage
> +	 * of core (FEAT_NMI) and GIC (FEAT_GICv3_NMI) in HW
> +	 */
> +	if (!gic_supports_v3_3_nmi())
> +		gic_enable_pseudo_nmis();
>  
>  	if (static_branch_likely(&supports_deactivate_key))
>  		gic_eoimode1_chip.flags |= IRQCHIP_SUPPORTS_NMI;
> @@ -2032,6 +2123,7 @@ static int __init gic_init_bases(phys_addr_t dist_phys_base,
>  	irq_domain_update_bus_token(gic_data.domain, DOMAIN_BUS_WIRED);
>  
>  	gic_data.has_rss = !!(typer & GICD_TYPER_RSS);
> +	gic_data.has_nmi = !!(typer & GICD_TYPER_NMI);
>  
>  	if (typer & GICD_TYPER_MBIS) {
>  		err = mbi_init(handle, gic_data.domain);
> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
> index ea5fd2374ebe..ebccd76cadda 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -30,6 +30,7 @@
>  #define GICD_ICFGR			0x0C00
>  #define GICD_IGRPMODR			0x0D00
>  #define GICD_NSACR			0x0E00
> +#define GICD_INMIR			0x0F80
>  #define GICD_IGROUPRnE			0x1000
>  #define GICD_ISENABLERnE		0x1200
>  #define GICD_ICENABLERnE		0x1400
> @@ -39,6 +40,7 @@
>  #define GICD_ICACTIVERnE		0x1C00
>  #define GICD_IPRIORITYRnE		0x2000
>  #define GICD_ICFGRnE			0x3000
> +#define GICD_INMIRnE			0x3B00
>  #define GICD_IROUTER			0x6000
>  #define GICD_IROUTERnE			0x8000
>  #define GICD_IDREGS			0xFFD0
> @@ -83,6 +85,7 @@
>  #define GICD_TYPER_LPIS			(1U << 17)
>  #define GICD_TYPER_MBIS			(1U << 16)
>  #define GICD_TYPER_ESPI			(1U << 8)
> +#define GICD_TYPER_NMI			(1U << 9)
>  
>  #define GICD_TYPER_ID_BITS(typer)	((((typer) >> 19) & 0x1f) + 1)
>  #define GICD_TYPER_NUM_LPIS(typer)	((((typer) >> 11) & 0x1f) + 1)
> @@ -238,6 +241,7 @@
>  #define GICR_ICFGR0			GICD_ICFGR
>  #define GICR_IGRPMODR0			GICD_IGRPMODR
>  #define GICR_NSACR			GICD_NSACR
> +#define GICR_INMIR0			GICD_INMIR
>  
>  #define GICR_TYPER_PLPIS		(1U << 0)
>  #define GICR_TYPER_VLPIS		(1U << 1)