Re: [RFC PATCH v2 14/45] arm64: entry: Introduce entry specific exception masking helpers

[Jinjie Ruan <ruanjinjie@huawei.com>]

在 2026/8/3 20:12, Vladimir Murzin 写道:
> On 7/28/26 09:48, Jinjie Ruan wrote:
>>
>> 在 2026/7/28 0:34, Vladimir Murzin 写道:
>>> From: Ada Couprie Diaz <ada.coupriediaz@arm.com>
>>>
>>> The entry code handles interrupt masking differently from the rest of
>>> the kernel. Exception handlers enter and exit with all exceptions
>>> masked, but they must temporarily unmask the appropriate set of
>>> exceptions so that the rest of the handler executes with the expected
>>> exception state.
>>>
>>> For EL0 handlers, this means dropping to masking context appropriate
>>> for the work to be performed. For EL1 handlers, this means restoring
>>> the masking context of the interrupted task. In both cases, all
>>> exceptions must be masked again before returning from the exception
>>> handler.
>>>
>>> The rest of the kernel typically follows the opposite pattern: it
>>> raises the masking context to protect a critical section and later
>>> restores the previous context.
>>>
>>> Given these different usage patterns, introduce a dedicated set of
>>> exception masking helpers for the entry code. Keeping these helpers
>>> separate from the generic interrupt masking APIs makes the intended
>>> usage explicit and helps avoid mixing the two masking models.
>>>
>>> Signed-off-by: Ada Couprie Diaz <ada.coupriediaz@arm.com>
>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>>> ---
>>>  arch/arm64/include/asm/interrupts/entry.h | 113 ++++++++++++++++++++++
>>>  1 file changed, 113 insertions(+)
>>>  create mode 100644 arch/arm64/include/asm/interrupts/entry.h
>>>
>>> diff --git a/arch/arm64/include/asm/interrupts/entry.h b/arch/arm64/include/asm/interrupts/entry.h
>>> new file mode 100644
>>> index 000000000000..d66eb5d633f0
>>> --- /dev/null
>>> +++ b/arch/arm64/include/asm/interrupts/entry.h
>>> @@ -0,0 +1,113 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Copyright (C) 2025 Arm Ltd.
>>> + */
>>> +#ifndef __ASM_INTERRUPTS_ENTRY_H
>>> +#define __ASM_INTERRUPTS_ENTRY_H
>>> +
>>> +#include <asm/arch_gicv3.h>
>>> +#include <asm/bug.h>
>>> +#include <asm/cpufeature.h>
>>> +#include <asm/interrupts/common_flags.h>
>>> +
>>> +
>> [...]
>>> +static __always_inline
>>> +arm64_exc_hwstate_t arm64_drop_exc_context(arm64_exc_hwstate_t prev, arm64_exc_context_t context)
>>> +{
>>> +	arm64_exc_hwstate_t next = arm64_exc_hwstate_of_context(context);
>>> +
>>> +	if (IS_ENABLED(CONFIG_DEBUG_IRQFLAGS)) {
>>> +		bool pnmi = system_uses_irq_prio_masking();
>>> +
>>> +		WARN_ON_ONCE(context > ERROR_CONTEXT &&
>>> +			     prev.daif == DAIF_ERRCTX);
>>> +
>>> +		WARN_ON_ONCE(context > NONMI_CONTEXT &&
>>> +			     prev.daif == DAIF_PROCCTX_NOIRQ);
>>> +
>>> +		WARN_ON_ONCE(context > NOIRQ_CONTEXT &&
>>> +			     pnmi && prev.pmr == GIC_PRIO_IRQOFF);
>>> +
>>> +		WARN_ON_ONCE(context > PROCESS_CONTEXT &&
>>> +			     ((pnmi && prev.daif == DAIF_PROCCTX && prev.pmr == GIC_PRIO_IRQON) ||
>>> +			      (!pnmi && prev.daif == DAIF_PROCCTX)));
>>> +	}
>>> +
>>> +	return __arm64_switch_exc_hwstate_to(prev, next);
>>> +}
>>> +
>>> +static __always_inline
>>> +arm64_exc_hwstate_t arm64_lift_exc_context(arm64_exc_hwstate_t prev, arm64_exc_context_t context)
>>> +{
>>> +	arm64_exc_hwstate_t next = arm64_exc_hwstate_of_context(context);
>>> +
>>> +	if (IS_ENABLED(CONFIG_DEBUG_IRQFLAGS)) {
>>> +		bool pnmi = system_uses_irq_prio_masking();
>>> +
>>> +		WARN_ON_ONCE(context < CRITICAL_CONTEXT &&
>>> +			     prev.daif == DAIF_MASK);
>>> +
>>> +		WARN_ON_ONCE(context < ERROR_CONTEXT &&
>>> +			     prev.daif == DAIF_ERRCTX);
>>> +
>>> +		WARN_ON_ONCE(context < NONMI_CONTEXT &&
>>> +			     pnmi && prev.daif == DAIF_PROCCTX_NOIRQ);
>>> +
>>> +		WARN_ON_ONCE(context < NOIRQ_CONTEXT &&
>>> +			     ((pnmi && prev.pmr == GIC_PRIO_IRQOFF) ||
>>> +			      (!pnmi && prev.daif == DAIF_PROCCTX_NOIRQ)));
>>> +	}
>>> +
>>> +	return __arm64_switch_exc_hwstate_to(prev, next);
> 
> Hi Jinjie,
> 
>> Hi Vladimir,
>>
>> These two functions appear to have similar functionality, with only the
>> debug code being different. Can they be merged?
>>
> 
> Functionality is indeed similar, but the intent is different. It
> gently encourages the user to think about the exception context they
> are currently in, which should make it easier to reason about
> exception transitions. That is not always possible with a merged "take
> me there" interface.

I see the intent. The function names clearly indicate the relaxation of
exception masking on entry and the tightening on exit, which makes
reasoning about the transitions straightforward.

> 
> The debug code then ensures that the requested exception transition is
> in the correct direction.

Functions that distinguish two directions are more useful in debugging.

> 
> Cheers
> Vladimir
> 
>>> +}
>>> +
>>> +
>>> +static __always_inline
>>> +arm64_exc_hwstate_t arm64_unmask_exc_context(arm64_exc_context_t context)
>>> +{
>>> +	arm64_exc_hwstate_t prev = arm64_exc_hwstate_of_context(CRITICAL_CONTEXT);
>>> +
>>> +	return arm64_drop_exc_context(prev, context);
>>> +}
>>> +
>>> +static __always_inline
>>> +arm64_exc_hwstate_t arm64_mask_exc_context(arm64_exc_hwstate_t prev)
>>> +{
>>> +	return arm64_lift_exc_context(prev, CRITICAL_CONTEXT);
>>> +}
>> otherwise, LGTM
>> Reviewed-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>
>>> +
>>> +#endif /* __ASM_INTERRUPTS_ENTRY_H */
>>
> 
>