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

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

On 8/29/26 03:34, Jinjie Ruan wrote:
> 
> 在 2026/7/28 17:29, Jinjie Ruan 写道:
>>
>> 在 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_switch_exc_hwstate_to(arm64_exc_hwstate_t prev,
>>> +						  arm64_exc_hwstate_t next)
>> may be const arm64_exc_hwstate_t next
>>
>>> +{
>>> +	bool irqs_disabled = arch_irqs_disabled_flags(next.flags);
>>> +	bool force;
>>> +
>>> +	arm64_debug_exc_hwstate(prev);
>>> +
>>> +	if (prev.flags == next.flags)
>>> +		return next;
>>> +
>>> +	if (!irqs_disabled)
>>> +		trace_hardirqs_on();
>>> +
>>> +	force = system_uses_irq_prio_masking() && prev.pmr != next.pmr;
>>> +
>>> +	__arm64_update_exc_hwstate(next, force);
>>> +
>>> +	if (irqs_disabled)
>>> +		trace_hardirqs_off();
>>> +
>>> +	return next;
>> We do not update next; it should be "void".
> Hi Vladimir,
> 
> What do you think?

I have no strong opinion, TBH. It is done that way to make it
interchangeable with the wrappers around it. For instance, the
arm64_inherit_exc_context() wrapper can be easily replaced with
the bare version:

__arm64_switch_exc_hwstate_to(arm64_exc_hwstate_of_context(CRITICAL_CONTEXT),
			      arm64_inherit_exc_hwstate(regs))

yes, it is less readable (and it is the reason we have wrappers),
but functionally equivalent.

I'm happy to change it if people feel strongly about it.

Cheers
Vladimir