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

[Vladimir Murzin <vladimir.murzin@arm.com>]

On 7/10/26 09:40, Jinjie Ruan wrote:
> 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();
>  }

Thanks! I'll have a closer look at the impact it has.

Vladimir