Re: [boot-wrapper PATCH 03/12] Remove cache maintenance

[Andre Przywara <andre.przywara@arm.com>]

On Thu, 29 Jul 2021 16:20:41 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> For models, we assume that out-of-reset caches ar invalid and no cache

Is that "caches are invalid out-of-reset" an architecture guarantee or
just a model property? Since the bootwrapper is officially targeting
the model, that doesn't really matter, but some people (ab)use it for
other platforms, so it might be worth noting.

> maintenance is required.
> 
> We added cache maintenance to the boot-wrapper in commit:
> 
>   28ec269a22c8dc14 ("Add code to clean and invalidate caches")
> 
> ... because the boot-wrapper would teansiently use cacheable mappings,
> and could allocate into caches. As we were using Set/Way operations, we
> were on somewhat shaky ground (e.g. due to system-level caches, or
> dirty line migration). Further, we never took FEAT_CCIDX into account,
> and so would not necessarily invalidate all potential levels of cache
> 
> However, since commit:
> 
>   0bb7b2545582accf ("Remove MMU identity map setup")
> 
> ... we no longer enable the MMU within the boot-wrapper, and so no
> longer have any reason to perform cache maintenance.
> 
> This patch removes the redundant and incomplete cache maintenance.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  Makefile.am   |  2 +-
>  boot_common.c |  3 ---
>  cache.c       | 58 ----------------------------------------------------------

Love that ^^^^, also it removes the *flush*_cache() name ;-)

Indeed I couldn't find anything setting SCTLR.M, so:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

>  3 files changed, 1 insertion(+), 62 deletions(-)
>  delete mode 100644 cache.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index ef6b793..8334049 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -123,7 +123,7 @@ CFLAGS		+= -Wall -fomit-frame-pointer
>  CFLAGS		+= -ffunction-sections -fdata-sections
>  LDFLAGS		+= --gc-sections
>  
> -OFILES		+= boot_common.o bakery_lock.o platform.o $(GIC) cache.o lib.o
> +OFILES		+= boot_common.o bakery_lock.o platform.o $(GIC) lib.o
>  OFILES		+= $(addprefix $(ARCH_SRC),boot.o stack.o $(BOOTMETHOD) utils.o)
>  
>  # Don't lookup all prerequisites in $(top_srcdir), only the source files. When
> diff --git a/boot_common.c b/boot_common.c
> index e7b8e1d..d48b7e1 100644
> --- a/boot_common.c
> +++ b/boot_common.c
> @@ -13,7 +13,6 @@ extern unsigned long entrypoint;
>  extern unsigned long dtb;
>  
>  void init_platform(void);
> -void flush_caches(void);
>  
>  void __noreturn jump_kernel(unsigned long address,
>  			    unsigned long a0,
> @@ -62,8 +61,6 @@ void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry)
>  void __noreturn first_spin(unsigned int cpu, unsigned long *mbox,
>  			   unsigned long invalid)
>  {
> -	flush_caches();
> -
>  	if (cpu == 0) {
>  		init_platform();
>  
> diff --git a/cache.c b/cache.c
> deleted file mode 100644
> index 9d71248..0000000
> --- a/cache.c
> +++ /dev/null
> @@ -1,58 +0,0 @@
> -/*
> - * cache.c - simple cache clean+invalidate code
> - *
> - * Copyright (C) 2015 ARM Limited. All rights reserved.
> - *
> - * Use of this source code is governed by a BSD-style license that can be
> - * found in the LICENSE.txt file.
> - */
> -
> -#include <cpu.h>
> -
> -void flush_caches(void)
> -{
> -	unsigned int level;
> -	uint32_t clidr = read_clidr();
> -	unsigned int max_level = (clidr >> 24) & 0x7;
> -
> -	uint32_t ccsidr;
> -
> -	if (max_level == 0)
> -		return;
> -
> -	for (level = 0; level < max_level; level++) {
> -		uint32_t cache_type = (clidr >> (level * 3)) & 0x7;
> -		unsigned int line_size, num_ways, num_sets, way_shift;
> -		unsigned int way, set;
> -
> -		if (cache_type == 1)
> -			/* No dcache at this level */
> -			continue;
> -
> -		write_csselr(level << 1);
> -		isb();
> -		ccsidr = read_ccsidr();
> -
> -		line_size = (ccsidr & 0x7) + 4; /* log2 line size */
> -		num_ways = ((ccsidr >> 3) & 0x3ff) + 1;
> -		num_sets = ((ccsidr >> 13) & 0x7fff) + 1;
> -
> -		way_shift = clz(num_ways - 1);
> -		for (way = 0; way < num_ways; way++) {
> -			for (set = 0; set < num_sets; set++) {
> -				uint32_t command = level << 1;
> -				command |= way << way_shift;
> -				command |= set << line_size;
> -
> -				dccisw(command);
> -				dsb(sy);
> -			}
> -		}
> -
> -		dsb(sy);
> -	}
> -	dsb(sy);
> -	iciallu();
> -	dsb(sy);
> -	isb();
> -}


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel