Re: [RFC PATCH 09/36] arm64: irqflags: introduce arm64-specific irqflags type

[Jinjie Ruan <ruanjinjie@huawei.com>]

On 7/9/2026 8:13 PM, Vladimir Murzin wrote:
> From: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> 
> With pseudo-NMIs enabled, we have two mechanisms that control
> interrupt masking in parallel :
> - The DAIF flags, masking at the CPU
> - The GIC PMR, masking before the CPU
> 
> However, our irqflags implementation currently assumes that only one
> of the two is used at a time, so both DAIF and PMR masking use the same
> `unsigned long flags` in their own way.
> This is incorrect, as some parts of the kernel will mask interrupts
> with DAIF directly or bypass the local_irq masking via the PMR,
> and makes tracking the state and changes of both in parallel impossible.
> 
> The irqflags API expects `unsigned long`s to be passed around, but
> they should not be manipulated outside of the arch-specific code.
> So, we can encode the information we need however we want as long as
> we return and accept `unsigned long`s.
> 
> Introduce a union type for arm64 irqflags whose first member is
> a struct allowing us to track DAIF and PMR in parallel, and the second
> is the `unsigned long` expected by the irqflags API.
> 
> DAIF is a two byte value, to maintain compatibility with existing defines.
> PMR is a one byte value, which is the maximum amount of priority bits
> allowed by the GICv3 architecture.
> 
> Update the internal irqflags functions to use this new union and convert
> back and forth with the irqflags unsigned long.
> There should be no functional changes.
> 
> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm64/include/asm/irqflags.h | 80 ++++++++++++++++++++-----------
>  1 file changed, 53 insertions(+), 27 deletions(-)

I believe that using the newly introduced arm64_exc_hwstate_t only at
the lowest level results in the least changes and is the most readable.

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

arch/arm64/include/asm/irqflags.h | 42
++++++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/irqflags.h
b/arch/arm64/include/asm/irqflags.h
index a8cb5a5c93b7..4b4521007183 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -9,6 +9,8 @@
 #include <asm/ptrace.h>
 #include <asm/sysreg.h>

+#include <linux/compiler.h>
+
 /*
  * Aarch64 has flags for masking: Debug, Asynchronous (serror),
Interrupts and
  * FIQ exceptions, in the 'daif' register. We mask and unmask them in
'daif'
@@ -20,6 +22,22 @@
  * exceptions should be unmasked.
  */

+ /*
+  * Internally, we want to independently manipulate and track the different
+  * interrupt masking mechanisms.
+  * Externally, the generic irqflags API expects unsgined longs to
represent
+  * the state of interrupts, which are treated as obscure arch-specific
data.
+  */
+typedef union arm64_exc_hwstate {
+       struct {
+               u16 daif;
+               u8 pmr;
+       };
+       unsigned long flags;
+} arm64_exc_hwstate_t;
+
+static_assert(sizeof(arm64_exc_hwstate_t) == sizeof(unsigned long));
+
 static __always_inline void __daif_local_irq_enable(void)
 {
        barrier();
@@ -79,12 +97,16 @@ static __always_inline void arch_local_irq_disable(void)

 static __always_inline unsigned long __daif_local_save_flags(void)
 {
-       return read_sysreg(daif);
+       arm64_exc_hwstate_t state = { .daif = read_sysreg(daif) };
+
+       return state.flags;
 }

 static __always_inline unsigned long __pmr_local_save_flags(void)
 {
-       return read_sysreg_s(SYS_ICC_PMR_EL1);
+       arm64_exc_hwstate_t state = { .pmr =
read_sysreg_s(SYS_ICC_PMR_EL1) };
+
+       return state.flags;
 }

 /*
@@ -101,12 +123,16 @@ static __always_inline unsigned long
arch_local_save_flags(void)

 static __always_inline bool __daif_irqs_disabled_flags(unsigned long flags)
 {
-       return flags & PSR_I_BIT;
+       arm64_exc_hwstate_t hwstate = { .flags = flags };
+
+       return hwstate.daif & PSR_I_BIT;
 }

 static __always_inline bool __pmr_irqs_disabled_flags(unsigned long flags)
 {
-       return flags != GIC_PRIO_IRQON;
+       arm64_exc_hwstate_t hwstate = { .flags = flags };
+
+       return hwstate.pmr != GIC_PRIO_IRQON;
 }

 static __always_inline bool arch_irqs_disabled_flags(unsigned long flags)
@@ -171,15 +197,19 @@ static __always_inline unsigned long
arch_local_irq_save(void)

 static __always_inline void __daif_local_irq_restore(unsigned long flags)
 {
+       arm64_exc_hwstate_t hwstate = { .flags = flags };
+
        barrier();
-       write_sysreg(flags, daif);
+       write_sysreg(hwstate.daif, daif);
        barrier();
 }

 static __always_inline void __pmr_local_irq_restore(unsigned long flags)
 {
+       arm64_exc_hwstate_t hwstate = { .flags = flags };
+
        barrier();
-       write_sysreg_s(flags, SYS_ICC_PMR_EL1);
+       write_sysreg_s(hwstate.pmr, SYS_ICC_PMR_EL1);
        pmr_sync();
        barrier();
 }


> 
> diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
> index a8cb5a5c93b7..7775904ba6a9 100644
> --- a/arch/arm64/include/asm/irqflags.h
> +++ b/arch/arm64/include/asm/irqflags.h
> @@ -9,6 +9,8 @@
>  #include <asm/ptrace.h>
>  #include <asm/sysreg.h>
>  
> +#include <linux/compiler.h>
> +
>  /*
>   * Aarch64 has flags for masking: Debug, Asynchronous (serror), Interrupts and
>   * FIQ exceptions, in the 'daif' register. We mask and unmask them in 'daif'
> @@ -20,6 +22,22 @@
>   * exceptions should be unmasked.
>   */
>  
> + /*
> +  * Internally, we want to independently manipulate and track the different
> +  * interrupt masking mechanisms.
> +  * Externally, the generic irqflags API expects unsgined longs to represent
> +  * the state of interrupts, which are treated as obscure arch-specific data.
> +  */
> +typedef union arm64_exc_hwstate {
> +	struct {
> +		u16 daif;
> +		u8 pmr;
> +	};
> +	unsigned long flags;
> +} arm64_exc_hwstate_t;
> +
> +static_assert(sizeof(arm64_exc_hwstate_t) == sizeof(unsigned long));
> +
>  static __always_inline void __daif_local_irq_enable(void)
>  {
>  	barrier();
> @@ -77,14 +95,14 @@ static __always_inline void arch_local_irq_disable(void)
>  	}
>  }
>  
> -static __always_inline unsigned long __daif_local_save_flags(void)
> +static __always_inline arm64_exc_hwstate_t __daif_local_save_flags(void)
>  {
> -	return read_sysreg(daif);
> +	return (arm64_exc_hwstate_t){ .daif = read_sysreg(daif) };
>  }
>  
> -static __always_inline unsigned long __pmr_local_save_flags(void)
> +static __always_inline arm64_exc_hwstate_t __pmr_local_save_flags(void)
>  {
> -	return read_sysreg_s(SYS_ICC_PMR_EL1);
> +	return (arm64_exc_hwstate_t){ .pmr = read_sysreg_s(SYS_ICC_PMR_EL1) };
>  }
>  
>  /*
> @@ -93,28 +111,32 @@ static __always_inline unsigned long __pmr_local_save_flags(void)
>  static __always_inline unsigned long arch_local_save_flags(void)
>  {
>  	if (system_uses_irq_prio_masking()) {
> -		return __pmr_local_save_flags();
> +		return __pmr_local_save_flags().flags;
>  	} else {
> -		return __daif_local_save_flags();
> +		return __daif_local_save_flags().flags;
>  	}
>  }
>  
> -static __always_inline bool __daif_irqs_disabled_flags(unsigned long flags)
> +static __always_inline
> +bool __daif_irqs_disabled_flags(arm64_exc_hwstate_t hwstate)
>  {
> -	return flags & PSR_I_BIT;
> +	return hwstate.daif & PSR_I_BIT;
>  }
>  
> -static __always_inline bool __pmr_irqs_disabled_flags(unsigned long flags)
> +static __always_inline
> +bool __pmr_irqs_disabled_flags(arm64_exc_hwstate_t hwstate)
>  {
> -	return flags != GIC_PRIO_IRQON;
> +	return hwstate.pmr != GIC_PRIO_IRQON;
>  }
>  
>  static __always_inline bool arch_irqs_disabled_flags(unsigned long flags)
>  {
> +	arm64_exc_hwstate_t hwstate = { .flags = flags };
> +
>  	if (system_uses_irq_prio_masking()) {
> -		return __pmr_irqs_disabled_flags(flags);
> +		return __pmr_irqs_disabled_flags(hwstate);
>  	} else {
> -		return __daif_irqs_disabled_flags(flags);
> +		return __daif_irqs_disabled_flags(hwstate);
>  	}
>  }
>  
> @@ -137,49 +159,51 @@ static __always_inline bool arch_irqs_disabled(void)
>  	}
>  }
>  
> -static __always_inline unsigned long __daif_local_irq_save(void)
> +static __always_inline arm64_exc_hwstate_t __daif_local_irq_save(void)
>  {
> -	unsigned long flags = __daif_local_save_flags();
> +	arm64_exc_hwstate_t hwstate = __daif_local_save_flags();
>  
>  	__daif_local_irq_disable();
>  
> -	return flags;
> +	return hwstate;
>  }
>  
> -static __always_inline unsigned long __pmr_local_irq_save(void)
> +static __always_inline arm64_exc_hwstate_t __pmr_local_irq_save(void)
>  {
> -	unsigned long flags = __pmr_local_save_flags();
> +	arm64_exc_hwstate_t hwstate = __pmr_local_save_flags();
>  
>  	/*
>  	 * There are too many states with IRQs disabled, just keep the current
>  	 * state if interrupts are already disabled/masked.
>  	 */
> -	if (!__pmr_irqs_disabled_flags(flags))
> +	if (!__pmr_irqs_disabled_flags(hwstate))
>  		__pmr_local_irq_disable();
>  
> -	return flags;
> +	return hwstate;
>  }
>  
>  static __always_inline unsigned long arch_local_irq_save(void)
>  {
>  	if (system_uses_irq_prio_masking()) {
> -		return __pmr_local_irq_save();
> +		return __pmr_local_irq_save().flags;
>  	} else {
> -		return __daif_local_irq_save();
> +		return __daif_local_irq_save().flags;
>  	}
>  }
>  
> -static __always_inline void __daif_local_irq_restore(unsigned long flags)
> +static __always_inline
> +void __daif_local_irq_restore(arm64_exc_hwstate_t hwstate)
>  {
>  	barrier();
> -	write_sysreg(flags, daif);
> +	write_sysreg(hwstate.daif, daif);
>  	barrier();
>  }
>  
> -static __always_inline void __pmr_local_irq_restore(unsigned long flags)
> +static __always_inline
> +void __pmr_local_irq_restore(arm64_exc_hwstate_t hwstate)
>  {
>  	barrier();
> -	write_sysreg_s(flags, SYS_ICC_PMR_EL1);
> +	write_sysreg_s(hwstate.pmr, SYS_ICC_PMR_EL1);
>  	pmr_sync();
>  	barrier();
>  }
> @@ -189,10 +213,12 @@ static __always_inline void __pmr_local_irq_restore(unsigned long flags)
>   */
>  static __always_inline void arch_local_irq_restore(unsigned long flags)
>  {
> +	arm64_exc_hwstate_t hwstate = { .flags = flags };
> +
>  	if (system_uses_irq_prio_masking()) {
> -		__pmr_local_irq_restore(flags);
> +		__pmr_local_irq_restore(hwstate);
>  	} else {
> -		__daif_local_irq_restore(flags);
> +		__daif_local_irq_restore(hwstate);
>  	}
>  }
>