[RFC PATCH 14/36] arm64: interrupts: introduce generic interrupt masking helpers

[Vladimir Murzin]

On 7/15/26 10:30, Liao, Chang wrote:
> 在 2026/7/9 20:13, Vladimir Murzin 写道:
>> From: Ada Couprie Diaz <ada.coupriediaz at arm.com>
>>
>> As for the entry code, we want to replace `local_daif_...` helpers
>> so that they can properly handle both DAIF and PMR, as well controlling
>> their use more strongly.
>>
>> Introduce new `local_all_irqs_...` helpers to replace them, which should
>> only be called in save/restore pairs.
>>
>> Save the requested interrupt state as well, so we can check for
>> inconsistent interrupt masking in between save and restore.
>>
>> There are two exceptions where it does not make sense to force
>> save/restore pairs for modifying the interrupt masks:
>>  - when initializing a CPU or
>>  - preparing to turn it off.
>>
>> As we otherwise want to force save/restore pairs, those cases are
>> handled with specific helpers, making clear that they should not be
>> used outside of those cases, enforced with `CONFIG_DEBUG_IRQFLAGS`
>> enabled.
>>
>> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz at arm.com>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin at arm.com>
>> ---
>>  arch/arm64/include/asm/interrupts/masking.h | 101 ++++++++++++++++++++
>>  1 file changed, 101 insertions(+)
>>  create mode 100644 arch/arm64/include/asm/interrupts/masking.h
>>
>> diff --git a/arch/arm64/include/asm/interrupts/masking.h b/arch/arm64/include/asm/interrupts/masking.h
>> new file mode 100644
>> index 000000000000..66ee03f7ab68
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/interrupts/masking.h
>> @@ -0,0 +1,101 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (C) 2025 Arm Ltd.
>> + */
>> +#ifndef __ASM_INTERRUPTS_MASKING_H
>> +#define __ASM_INTERRUPTS_MASKING_H
>> +
>> +#include <asm/arch_gicv3.h>
>> +#include <asm/bug.h>
>> +#include <asm/cpufeature.h>
>> +#include <asm/interrupts/common_flags.h>
>> +#include <asm/ptrace.h>
>> +
>> +typedef struct arm64_exc_hwstates {
>> +	arm64_exc_hwstate_t saved;
>> +	arm64_exc_hwstate_t expected;
>> +} arm64_exc_hwstates_t;
>> +
>> +#ifdef CONFIG_DEBUG_IRQFLAGS
>> +/* Make sure the CPU init/tear down masking functions are only used once. */
>> +static DEFINE_PER_CPU(bool, irqs_masks_cpu_init_done);
>> +static DEFINE_PER_CPU(bool, irqs_masks_cpu_final_done);
>> +#endif
>> +
>> +static inline
>> +arm64_exc_hwstates_t local_all_irqs_save_mask(arm64_exc_context_t new)
>> +{
>> +	arm64_exc_hwstate_t state = arm64_exc_hwstate_of_context(new);
>> +	arm64_exc_hwstate_t actual = {.flags = arch_local_save_flags()};
>> +
>> +	if (IS_ENABLED(CONFIG_DEBUG_IRQFLAGS)) {
>> +		bool pnmi = system_uses_irq_prio_masking();
>> +
>> +		WARN_ON_ONCE(new < CRITICAL_CONTEXT &&
>> +			     actual.daif == DAIF_MASK);
>> +
>> +		WARN_ON_ONCE(new < ERROR_CONTEXT &&
>> +			     actual.daif == DAIF_ERRCTX);
>> +
>> +		WARN_ON_ONCE(new < NONMI_CONTEXT &&
>> +			     pnmi && actual.daif == DAIF_PROCCTX_NOIRQ);
>> +
>> +		WARN_ON_ONCE(new < NOIRQ_CONTEXT &&
>> +			     ((pnmi && actual.pmr == GIC_PRIO_IRQOFF) ||
>> +			      (!pnmi && actual.daif == DAIF_PROCCTX_NOIRQ)));
>> +	}
>> +
>> +	arm64_update_exc_hwstate(state, actual.pmr != state.pmr);
> Is it necessary to compare pmr field unconditionally here, does this function's
> usage imply a strict dependency on pseudo NMI？If this function moves forward to
> support FEAT_NMI, checking PMR seems unneccesary.
> 

Yes, it is not necessary, but it is cheap. :) However, I think I need
to revisit this approach. More on that below...

>> +
>> +	if (!arch_irqs_disabled_flags(actual.flags))
>> +		trace_hardirqs_off();
>> +
>> +	return (arm64_exc_hwstates_t){.saved = actual, .expected = state};
>> +}
>> +
>> +static inline void local_all_irqs_restore(arm64_exc_hwstates_t states)
>> +{
>> +	arm64_debug_exc_hwstate(states.expected);
>> +
>> +	if (!arch_irqs_disabled_flags(states.saved.flags))
>> +		trace_hardirqs_on();
>> +
>> +	arm64_update_exc_hwstate(states.saved, true);
> I'm curious why it forces update_pmr to true for this specific path?
> 

We force an update because we cannot fully trust that the HW state
matches the expected state. The expected state exists for debug
purposes only.

In the local_all_irqs_save_mask() case above, we have just read the HW
state, so we can use that information to avoid some unnecessary HW
updates.

I think it would be better to provide two versions of
arm64_update_exc_hwstate(): a strong version and a relaxed version,
with the strong version being the default. That would let us limit the
relaxed version to places where we know it is safe to skip redundant
HW updates.


>> +}
>> +
>> +#ifdef CONFIG_DEBUG_IRQFLAGS
>> +static inline
>> +void local_all_irqs_cpu_init_mask(arm64_exc_context_t context)
>> +{
>> +	WARN_ON(__this_cpu_read(irqs_masks_cpu_init_done));
>> +	if (context == PROCESS_CONTEXT)
>> +		trace_hardirqs_on();
>> +	arm64_update_exc_context(context, true);
>> +	__this_cpu_write(irqs_masks_cpu_init_done, true);
>> +	__this_cpu_write(irqs_masks_cpu_final_done, false);
>> +}
>> +
>> +static inline void local_all_irqs_final_mask(void)
>> +{
>> +	WARN_ON(__this_cpu_read(irqs_masks_cpu_final_done));
>> +	arm64_update_exc_context(CRITICAL_CONTEXT, true);
>> +	trace_hardirqs_off();
>> +	__this_cpu_write(irqs_masks_cpu_final_done, true);
>> +	__this_cpu_write(irqs_masks_cpu_init_done, false);
>> +}
>> +#else /* CONFIG_DEBUG_IRQFLAGS */
>> +static inline
>> +void local_all_irqs_cpu_init_mask(arm64_exc_context_t context)
>> +{
>> +	if (context == PROCESS_CONTEXT)
>> +		trace_hardirqs_on();
>> +	arm64_update_exc_context(context, true);
>> +}
>> +
>> +static inline void local_all_irqs_final_mask(void)
>> +{
>> +	arm64_update_exc_context(CRITICAL_CONTEXT, true);
>> +	trace_hardirqs_off();
>> +}
>> +#endif /* CONFIG_DEBUG_IRQFLAGS */
>> +#endif /* __ASM_INTERRUPTS_MASKING_H */
> 
> -- BR Liao, Chang
> 

Cheers
Vladimir