mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andreas Larsson <andreas@gaisler.com>
To: Imre Kaloz <kaloz@kernel.org>, "David S. Miller" <davem@davemloft.net>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>,
	"Mike Rapoport (IBM)" <rppt@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org,
	nsafran1217 <54966414+nsafran1217@users.noreply.github.com>,
	stable@vger.kernel.org, Stian Halseth <stian@itx.no>
Subject: Re: [PATCH v2 2/2] sparc64: flush vmalloc ranges from the D-cache on map and unmap
Date: Wed, 21 Oct 2026 16:18:04 +0200	[thread overview]
Message-ID: <1649afcb-ae04-406c-ac41-92f62d7343ed@gaisler.com> (raw)
In-Reply-To: <20260928132102.1707-3-kaloz@kernel.org>

Hi Imre,

On 2026-09-28 15:21, Imre Kaloz wrote:
> The L1 D-cache of Spitfire and Cheetah is indexed with VA bit 13, but
> flush_cache_vmap() and flush_cache_vunmap() are no-ops. Lines loaded
> through a vmalloc address survive the unmap, and once the page is
> written through the other colour, e.g. zeroed through the linear
> mapping, a later mapping of the old colour reads stale data. Anonymous
> user pages arrive the same way, as tlb_batch_add() does not flush them.
> On UltraSPARC III this corrupts BPF programs and module data.
> 
> Flush each page on every CPU on map and unmap, or the whole D-cache in
> one cross call once the range exceeds its size.
> 
> flush_cache_vmap() also runs on the vmemmap, from sparse_init() before
> init_IRQ() sets up the cross-call mondo blocks; with the mondo block PA
> still zero, flush_dcache_all()'s cross-call lands on physical address 0
> instead. The vmemmap has no aliases to flush, so skip addresses outside
> vmalloc and module space.

I would not say a cross call lands on address 0, which kind of sounds
like execution would end up there, rather that it would do writes
starting at physical address 0. Clear language about problems that
"would" happen would be nice so that it cannot be read like it happens
with this patch.

I'm afraid this solution could be a bit fragile though, in case some
vmalloc() later on would happen between vmalloc_init() and init_IRQ(),
we would be in trouble here. Better might be to in xcall_deliver() build
the CPU-list first locally, returning if coming up empty, and only after
that start accessing the trap block. This could be a preparatory patch
that is not marked for stable. 

This patch under review could still do its job as this manner and have
the is_vmalloc_or_module_addr() filter be a performance enhancer in the
context of a less fragile xcall_deliver(), but needed for a stable
pickup, clearly mentioned in the commit message.


> Link: https://github.com/sparclinux/issues/issues/29#issuecomment-5041121375
> Closes: https://github.com/sparclinux/issues/issues/29
> Suggested-by: nsafran1217 <54966414+nsafran1217@users.noreply.github.com>

This is a strange email address to use for a Suggested-by

> Cc: stable@vger.kernel.org

Given that this patch depends on the previous patch in the series, that
comes with a Fixes: tag, which this one don't, a comment that this
depends on that previous fix could perhaps be good here


> Tested-by: Stian Halseth <stian@itx.no> # Sun Fire V240, SPARC T4-1
> Signed-off-by: Imre Kaloz <kaloz@kernel.org>
> ---
> v2: skip the flush outside vmalloc/module space, since
>     flush_cache_vmap() also runs on the vmemmap during sparse_init()
>     before cross-calls are set up (Stian Halseth); picked up his
>     Tested-by.
> 
>  arch/sparc/include/asm/cacheflush_64.h |  6 ++--
>  arch/sparc/kernel/smp_64.c             | 18 +++++++++++
>  arch/sparc/mm/init_64.c                | 43 ++++++++++++++++++++++++++
>  arch/sparc/mm/ultra.S                  | 25 +++++++++++++++
>  4 files changed, 90 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/sparc/include/asm/cacheflush_64.h b/arch/sparc/include/asm/cacheflush_64.h
> index 02c969417e7b..54a2d37008e6 100644
> --- a/arch/sparc/include/asm/cacheflush_64.h
> +++ b/arch/sparc/include/asm/cacheflush_64.h
> @@ -42,6 +42,8 @@ void smp_flush_dcache_folio_impl(struct folio *folio, int cpu);
>  #define smp_flush_dcache_folio_impl(folio, cpu) flush_dcache_folio_impl(folio)
>  #endif
>  void flush_dcache_page_all(struct mm_struct *mm, struct page *page);
> +void __flush_dcache_all(unsigned long size, unsigned long line_size);
> +void flush_dcache_all(void);
>  
>  void __flush_dcache_range(unsigned long start, unsigned long end);
>  #define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 1
> @@ -73,9 +75,9 @@ void flush_ptrace_access(struct vm_area_struct *, struct page *,
>  #define flush_dcache_mmap_lock(mapping)		do { } while (0)
>  #define flush_dcache_mmap_unlock(mapping)	do { } while (0)
>  
> -#define flush_cache_vmap(start, end)		do { } while (0)
> +void flush_cache_vmap(unsigned long start, unsigned long end);
>  #define flush_cache_vmap_early(start, end)	do { } while (0)
> -#define flush_cache_vunmap(start, end)		do { } while (0)
> +#define flush_cache_vunmap(start, end)		flush_cache_vmap(start, end)
>  
>  #endif /* !__ASSEMBLER__ */
>  
> diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
> index 18b6145da591..bebfc44241b3 100644
> --- a/arch/sparc/kernel/smp_64.c
> +++ b/arch/sparc/kernel/smp_64.c
> @@ -915,6 +915,7 @@ extern unsigned long xcall_kgdb_capture;
>  
>  #ifdef DCACHE_ALIASING_POSSIBLE
>  extern unsigned long xcall_flush_dcache_page_cheetah;
> +extern unsigned long xcall_flush_dcache_all;
>  #endif
>  extern unsigned long xcall_flush_dcache_page_spitfire;
>  
> @@ -1025,6 +1026,23 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page)
>  	preempt_enable();
>  }
>  
> +#ifdef DCACHE_ALIASING_POSSIBLE
> +void flush_dcache_all(void)
> +{
> +	unsigned long size, line_size;
> +
> +	if (tlb_type == hypervisor)
> +		return;
> +
> +	preempt_disable();
> +	size = local_cpu_data().dcache_size;
> +	line_size = local_cpu_data().dcache_line_size;
> +	smp_cross_call(&xcall_flush_dcache_all, 0, size, line_size);
> +	__flush_dcache_all(size, line_size);
> +	preempt_enable();
> +}
> +#endif
> +
>  #ifdef CONFIG_KGDB
>  void kgdb_roundup_cpus(void)
>  {
> diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
> index 8792e5d92517..3e2096545e52 100644
> --- a/arch/sparc/mm/init_64.c
> +++ b/arch/sparc/mm/init_64.c
> @@ -27,6 +27,7 @@
>  #include <linux/percpu.h>
>  #include <linux/mmzone.h>
>  #include <linux/gfp.h>
> +#include <linux/vmalloc.h>
>  
>  #include <asm/head.h>
>  #include <asm/page.h>
> @@ -234,6 +235,17 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page)
>  		__flush_icache_page(page_to_phys(page));
>  #endif
>  }
> +
> +#ifdef DCACHE_ALIASING_POSSIBLE
> +void flush_dcache_all(void)
> +{
> +	if (tlb_type == hypervisor)
> +		return;
> +
> +	__flush_dcache_all(local_cpu_data().dcache_size,
> +			   local_cpu_data().dcache_line_size);
> +}
> +#endif
>  #endif
>  
>  #define PG_dcache_dirty		PG_arch_1
> @@ -3072,6 +3084,37 @@ arch_initcall(report_memory);
>  #define do_flush_tlb_kernel_range	__flush_tlb_kernel_range
>  #endif
>  
> +/*
> + * The L1 D-cache is indexed with VA bit 13, so lines loaded through a
> + * vmalloc address outlive the mapping.  Stores through a mapping of the
> + * other colour do not update them, and a later load through their colour
> + * returns stale data.  Flush at unmap so vmalloc's lines do not follow
> + * the page back to the allocator, and at map time since anonymous user
> + * pages are freed without a flush (see tlb_batch_add()).  Past the
> + * D-cache size, one whole-cache flush is cheaper than a cross call per
> + * page.
> + */
> +void flush_cache_vmap(unsigned long start, unsigned long end)
> +{
> +#ifdef DCACHE_ALIASING_POSSIBLE
> +	/* The vmemmap has no aliases and is mapped before cross calls work. */
> +	if (tlb_type == hypervisor || !is_vmalloc_or_module_addr((void *)start))
> +		return;
> +
> +	if (end - start > cpu_data(raw_smp_processor_id()).dcache_size) {
> +		flush_dcache_all();
> +		return;
> +	}
> +
> +	for (; start < end; start += PAGE_SIZE) {
> +		struct page *page = vmalloc_to_page((void *)start);
> +
> +		if (page)
> +			flush_dcache_page_all(NULL, page);
> +	}
> +#endif
> +}
> +
>  void flush_tlb_kernel_range(unsigned long start, unsigned long end)
>  {
>  	if (start < HI_OBP_ADDRESS && end > LOW_OBP_ADDRESS) {
> diff --git a/arch/sparc/mm/ultra.S b/arch/sparc/mm/ultra.S
> index 70e658d107e0..ff190670a235 100644
> --- a/arch/sparc/mm/ultra.S
> +++ b/arch/sparc/mm/ultra.S
> @@ -242,6 +242,20 @@ __flush_dcache_page:	/* %o0=kaddr, %o1=flush_icache */
>  	retl
>  	 nop
>  
> +	/* Zeroing every D-cache tag invalidates the whole cache on
> +	 * Spitfire and Cheetah alike.
> +	 */
> +	.align		32
> +	.globl		__flush_dcache_all
> +__flush_dcache_all:	/* %o0=D-cache size, %o1=D-cache line size */
> +1:	subcc		%o0, %o1, %o0
> +	stxa		%g0, [%o0] ASI_DCACHE_TAG
> +	membar		#Sync
> +	bne,pt		%icc, 1b
> +	 nop
> +	retl
> +	 nop
> +
>  #endif /* DCACHE_ALIASING_POSSIBLE */
>  
>  	.previous
> @@ -801,6 +815,17 @@ xcall_flush_dcache_page_cheetah: /* %g1 == physical page address */
>  	 nop
>  	retry
>  	nop
> +
> +	.align		32
> +	.globl		xcall_flush_dcache_all
> +xcall_flush_dcache_all: /* %g1 == D-cache size, %g7 == D-cache line size */
> +1:	subcc		%g1, %g7, %g1
> +	stxa		%g0, [%g1] ASI_DCACHE_TAG
> +	membar		#Sync
> +	bne,pt		%icc, 1b
> +	 nop
> +	retry
> +	nop
>  #endif /* DCACHE_ALIASING_POSSIBLE */
>  
>  	.globl		xcall_flush_dcache_page_spitfire

Thanks,
Andreas


  reply	other threads:[~2026-10-21 14:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-28 13:21 [PATCH v2 0/2] sparc64: D-cache alias flushing fixes Imre Kaloz
2026-09-28 13:21 ` [PATCH v2 1/2] sparc64: flush only the aliased page, not its whole folio Imre Kaloz
2026-10-15 14:46   ` Andreas Larsson
2026-10-15 19:26     ` Imre Kaloz
2026-10-17 10:34       ` John Paul Adrian Glaubitz
2026-10-21 14:14   ` Andreas Larsson
2026-10-21 23:24     ` Imre Kaloz
2026-09-28 13:21 ` [PATCH v2 2/2] sparc64: flush vmalloc ranges from the D-cache on map and unmap Imre Kaloz
2026-10-21 14:18   ` Andreas Larsson [this message]
2026-10-21 23:31     ` Imre Kaloz
2026-10-11  9:33 ` [PATCH v2 0/2] sparc64: D-cache alias flushing fixes John Paul Adrian Glaubitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1649afcb-ae04-406c-ac41-92f62d7343ed@gaisler.com \
    --to=andreas@gaisler.com \
    --cc=54966414+nsafran1217@users.noreply.github.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=kaloz@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rppt@kernel.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=stian@itx.no \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®