Re: [PATCH v2 4/6] sparc64: add SPARC64 VII CPU, MMU and SMP support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] sparc64: missing VII membar synchronization for __flush_tlb_mm and xcall
--

commit 3658cc67da72bc2e25beda7e9124cb710a511258
Author: Magnus Lindholm <linmag7@gmail.com>
sparc64: add SPARC64 VII CPU, MMU and SMP support

This commit adds support for SPARC64 VII and VII+ processors in the Fujitsu
M3000. It introduces separate CPU, MMU, and SMP paths, managing CPU topology
and restrainable ECC errors. It also provides separate demap routines to satisfy
hardware synchronization requirements that are missing in the Cheetah paths.

> diff --git a/arch/sparc/mm/ultra.S b/arch/sparc/mm/ultra.S
> index 70e658d107e02..c4a9647d6eb04 100644
> --- a/arch/sparc/mm/ultra.S
> +++ b/arch/sparc/mm/ultra.S
[ ... ]
> @@ -981,6 +981,242 @@ xcall_kgdb_capture:
>  
>  #endif /* CONFIG_SMP */
>  
[ ... ]
> +	.globl		sparc64_vii_patch_cachetlbops
> +sparc64_vii_patch_cachetlbops:
> +	save		%sp, -128, %sp
> +
> +	sethi		%hi(__flush_tlb_mm), %o0
> +	or		%o0, %lo(__flush_tlb_mm), %o0
> +	sethi		%hi(__cheetah_flush_tlb_mm), %o1
> +	or		%o1, %lo(__cheetah_flush_tlb_mm), %o1

[Severity: High]
Does this incorrectly reuse the Cheetah variant for VII hardware?

The commit message explicitly notes that SPARC64 VII requires hardware
synchronization (membar #Sync) for context writes and instruction demaps,
and that the existing Cheetah assembly does not establish this. However,
sparc64_vii_patch_cachetlbops patches __flush_tlb_mm using
__cheetah_flush_tlb_mm, which lacks the required membar #Sync after writing
to ASI_DMMU.

> +	call		tlb_patch_one
> +	 mov		19, %o2
> +
> +	sethi		%hi(__flush_tlb_page), %o0
[ ... ]
> +#ifdef CONFIG_SMP
> +	sethi		%hi(xcall_flush_tlb_page), %o0
> +	or		%o0, %lo(xcall_flush_tlb_page), %o0
> +	sethi		%hi(__vii_xcall_flush_tlb_page_patch), %o1
> +	or		%o1, %lo(__vii_xcall_flush_tlb_page_patch), %o1
> +	call		tlb_patch_one
> +	 mov		3, %o2
> +
> +	/* VII uses 32-entry fTLBs: do not use the Spitfire 64-slot sweep. */
> +	sethi		%hi(xcall_flush_tlb_kernel_range), %o0
> +	or		%o0, %lo(xcall_flush_tlb_kernel_range), %o0
> +	sethi		%hi(__cheetah_xcall_flush_tlb_kernel_range), %o1
> +	or		%o1, %lo(__cheetah_xcall_flush_tlb_kernel_range), %o1
> +	call		tlb_patch_one
> +	 mov		44, %o2
> +#endif

[Severity: High]
Is xcall_flush_tlb_mm missing from this patch block?

By omitting xcall_flush_tlb_mm entirely, it seems remote CPUs will fall back
to the unpatched Spitfire handler. This would violate the JPS2 F.10.1
synchronization requirements and potentially leave the VII hardware exposed to
stale TLB or pipeline states.

> +
> +#ifdef DCACHE_ALIASING_POSSIBLE
> +	sethi		%hi(__flush_dcache_page), %o0
[ ... ]

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