Re: [PATCH v4 02/10] sparc64: honor queued spinlock layout in secondary startup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] sparc64: invalid [%g2 + %lo(prom_entry_lock) + 3] operand in trampoline_64.S
--

commit 3122e42ebc7be814860c71cd7ad49ac4a1876063
Author: Magnus Lindholm <linmag7@gmail.com>

sparc64: honor queued spinlock layout in secondary startup

This commit updates secondary CPU startup in arch/sparc/kernel/trampoline_64.S
to acquire prom_entry_lock as a 32-bit word with CASA and release byte offset 3
with STB to match the big-endian queued spinlock layout.

> diff --git a/arch/sparc/kernel/trampoline_64.S b/arch/sparc/kernel/trampoline_64.S
> index 51bf1eb92a36e..4473347301229 100644
> --- a/arch/sparc/kernel/trampoline_64.S
> +++ b/arch/sparc/kernel/trampoline_64.S

[ ... ]

> @@ -212,7 +222,13 @@ startup_continue:
>  	 nop
>  
>  	sethi		%hi(prom_entry_lock), %g2
> -	stb		%g0, [%g2 + %lo(prom_entry_lock)]
> +	/*
> +	 * Retain explicit assembly release ordering; C unlock relies on TSO.
> +	 * The predicted branch protects Spitfire erratum 51.
> +	 */
> +	ba,pt		%xcc, 2f
> +	 membar		#StoreStore | #LoadStore
> +2:	stb		%g0, [%g2 + %lo(prom_entry_lock) + 3]
>  
>  	ba,pt		%xcc, after_lock_tlb
>  	 nop

[ ... ]

> @@ -389,7 +415,13 @@ after_lock_tlb:
>  	 add		%sp, (2047 + 128), %o0
>  
>  3:	sethi		%hi(prom_entry_lock), %g2
> -	stb		%g0, [%g2 + %lo(prom_entry_lock)]
> +	/*
> +	 * Retain explicit assembly release ordering; C unlock relies on TSO.
> +	 * The predicted branch protects Spitfire erratum 51.
> +	 */
> +	ba,pt		%xcc, 2f
> +	 membar		#StoreStore | #LoadStore
> +2:	stb		%g0, [%g2 + %lo(prom_entry_lock) + 3]

[Severity: Low]
Does this assemble on SPARC64 SMP builds (CONFIG_SPARC64=y && CONFIG_SMP=y)?

In both startup_continue and after_lock_tlb in
arch/sparc/kernel/trampoline_64.S, the unlock sequences use:

    stb %g0, [%g2 + %lo(prom_entry_lock) + 3]

In SPARC assembly syntax (both GNU as and LLVM's SparcAsmParser), %lo(...) is
parsed as the single immediate operand in [rs1 + simm13] and does not accept a
trailing "+ 3" after the closing parenthesis, which causes the assembler to
reject the instruction with "Error: Illegal operands".

Should the byte offset be placed inside the relocation modifier as
[%g2 + %lo(prom_entry_lock + 3)], or should %g2 first be formed with
"or %g2, %lo(prom_entry_lock), %g2" followed by "stb %g0, [%g2 + 3]"?

>  
>  	ldx		[%l0], %g6
>  	ldx		[%g6 + TI_TASK], %g4

титься

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018134738.1016106-1-linmag7@gmail.com?part=2