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

["Liao, Chang" <liaochang1@huawei.com>]

在 2026/7/9 20:13, Vladimir Murzin 写道:
> From: Ada Couprie Diaz <ada.coupriediaz@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@arm.com>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@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);

It is generally not a good idea to define static DEFINE_PER_CPU variables directly in
a header file, as this creates a separate copy of the variable in every translation unit
that includes it. Unless this header is strictly limited to a single source file. Please
correct me if this was designed for some special purpose.

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

This verification logic is almost same with the counterpart in arm64_lift_exc_context().
Would it make sense to factor this out into a shared helper function to avoid duplication?

> +
> +	arm64_update_exc_hwstate(state, actual.pmr != state.pmr);
> +
> +	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);
> +}
> +
> +#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