Re: [PATCH RFC Dovetail 7.1] arm64: irq_pipeline: 7.1 specific adjustments

Florian Bezdeka <[email protected]> writes:

> On Thu, 2026-06-11 at 15:40 +0200, Florian Bezdeka wrote:
>> Signed-off-by: Florian Bezdeka <[email protected]>
>> ---
>> 
>> Hi Philippe,
>> 
>> final tests are still running but the local tests are looking good.
>> The EVL testsuite triggered a WARN_ONCE() - see process.c below - which
>> caused the systems to hang.
>> 
>> Turned out the arm64 entry code got some refactorings that needed some
>> adjustments - pipline wise.
>> 
>> I would fold the following into 7.1, so could you please double check?
>> Thanks!
>> 
>>  arch/arm64/kernel/entry-common.c | 35 ++++++++++++++++++++++++++++----
>>  arch/arm64/kernel/process.c      |  6 +++---
>>  include/linux/irq-entry-common.h |  3 +++
>>  3 files changed, 37 insertions(+), 7 deletions(-)
>> 
>> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
>> index b0e141266349..97f6d6904a5d 100644
>> --- a/arch/arm64/kernel/entry-common.c
>> +++ b/arch/arm64/kernel/entry-common.c
>> @@ -166,7 +166,16 @@ static noinstr irqentry_state_t arm64_enter_from_kernel_mode(struct pt_regs *reg
>>  static void noinstr arm64_exit_to_kernel_mode(struct pt_regs *regs,
>>  					      irqentry_state_t state)
>>  {
>> -	local_irq_disable();
>> +	if (irqs_pipelined()) {
>> +		hard_local_irq_disable();
>> +		if (running_inband()) {
>> +			stall_inband_nocheck();
>> +			trace_hardirqs_off();
>> +		}
>> +	} else {
>> +		local_irq_disable();
>> +	}
>> +
>>  	irqentry_exit_to_kernel_mode_preempt(regs, state);
>>  	local_daif_mask();
>>  	mte_check_tfsr_exit();
>> @@ -175,7 +184,14 @@ static void noinstr arm64_exit_to_kernel_mode(struct pt_regs *regs,
>>  
>>  static __always_inline void arm64_syscall_enter_from_user_mode(struct pt_regs *regs)
>>  {
>> -	enter_from_user_mode(regs);
>> +	if (running_inband()) {
>> +		WARN_ON_ONCE(irq_pipeline_debug() && irqs_disabled());
>> +		stall_inband_nocheck();
>> +		enter_from_user_mode(regs);
>> +		trace_hardirqs_on();
>> +		unstall_inband_nocheck();
>> +	}
>
> Should enter_from_user_mode() be called while the inband stage is
> stalled, or after unstalling?

It should be stalled on entry, to reflect the hardware state.

> The exit part is called while being
> stalled, so I kept that in sync, but I'm not 100% sure.
>

That's correct.

>> +
>>  	mte_disable_tco_entry(current);
>>  	sme_enter_from_user_mode();
>>  }
>> @@ -191,6 +207,7 @@ static __always_inline void arm64_enter_from_user_mode(struct pt_regs *regs)
>>  		WARN_ON_ONCE(irq_pipeline_debug() && irqs_disabled());
>>  		stall_inband_nocheck();
>>  		enter_from_user_mode(regs);
>> +		rseq_note_user_irq_entry();
>>  		trace_hardirqs_on();
>>  		unstall_inband_nocheck();
>>  	}
>> @@ -226,8 +243,18 @@ static __always_inline void arm64_syscall_exit_to_user_mode(struct pt_regs *regs
>>   */
>>  static __always_inline void arm64_exit_to_user_mode(struct pt_regs *regs)
>>  {
>> -	local_irq_disable();
>> -	irqentry_exit_to_user_mode_prepare(regs);
>> +	if (irqs_pipelined()) {
>> +		hard_local_irq_disable();
>> +		if (running_inband()) {
>> +			stall_inband_nocheck();
>> +			trace_hardirqs_off();
>> +			irqentry_exit_to_user_mode_prepare(regs);
>> +		}
>> +	} else {
>> +		local_irq_disable();
>> +		irqentry_exit_to_user_mode_prepare(regs);
>> +	}
>> +
>>  	local_daif_mask();
>>  	sme_exit_to_user_mode();
>>  	mte_check_tfsr_exit();
>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
>> index b1eac5fd75c1..0047d0bdedd9 100644
>> --- a/arch/arm64/kernel/process.c
>> +++ b/arch/arm64/kernel/process.c
>> @@ -748,8 +748,8 @@ static inline void debug_switch_state(void)
>>  		unsigned long pmr_expected = GIC_PRIO_IRQOFF;
>>  		unsigned long pmr_actual = read_sysreg_s(SYS_ICC_PMR_EL1);
>>  
>> -		WARN_ONCE(daif_actual != daif_expected ||
>> -			  pmr_actual != pmr_expected,
>> +		WARN_ONCE(running_inband() && (daif_actual != daif_expected ||
>> +			  pmr_actual != pmr_expected),
>>  			  "Unexpected DAIF + PMR: 0x%lx + 0x%lx (expected 0x%lx + 0x%lx)\n",
>>  			  daif_actual, pmr_actual,
>>  			  daif_expected, pmr_expected);
>> @@ -757,7 +757,7 @@ static inline void debug_switch_state(void)
>>  		unsigned long daif_expected = DAIF_PROCCTX_NOIRQ;
>>  		unsigned long daif_actual = read_sysreg(daif);
>>  
>> -		WARN_ONCE(daif_actual != daif_expected,
>> +		WARN_ONCE(running_inband() && daif_actual != daif_expected,
>>  			  "Unexpected DAIF value: 0x%lx (expected 0x%lx)\n",
>>  			  daif_actual, daif_expected);
>
> That was the warning triggered by CI, that stalled the system. Is that
> expected? I'm quite sure that in the oob case the DAIF flags are
> different, but is WARN_ONCE() expected to completely halt the system?
>

WARN_ONCE() triggers a trap in the middle of the context switch
implementation, cobalt/evl are going to switch the caller to the in-band
stage in handle_oob_trap_entry(), which would cause __switch_to() calls
to pile up. So yes, this crash is definitely expected.

>
>>  	}
>> diff --git a/include/linux/irq-entry-common.h b/include/linux/irq-entry-common.h
>> index 58c28b2a8f2b..61a8e129e71f 100644
>> --- a/include/linux/irq-entry-common.h
>> +++ b/include/linux/irq-entry-common.h
>> @@ -600,6 +600,9 @@ irqentry_exit_to_kernel_mode_after_preempt(struct pt_regs *regs, irqentry_state_
>>  {
>>  	bool synchronized;
>>  
>> +	if (running_oob())
>> +		return;
>> +
>
> The enter/exit code moved one layer down, from irqentry_enter() to
> irqentry_enter_from_kernel_mode(), so this code needs the oob check as
> well.

Ok.

>
>>  	instrumentation_begin();
>>  	synchronized = irqentry_syncstage(state);
>>  
>> -- 
>> 2.54.0

-- 
Philippe.