Re: [PATCH v5 01/27] arm64/cpuinfo: Remove refrences to reserved cache type

From: Will Deacon <will@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Joey Gouly <joey.gouly@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v5 01/27] arm64/cpuinfo: Remove refrences to reserved cache type
Date: Tue, 28 Jun 2022 15:26:06 +0100	[thread overview]
Message-ID: <20220628142605.GC24116@willie-the-truck> (raw)
In-Reply-To: <20220622174416.1406282-2-broonie@kernel.org>

On Wed, Jun 22, 2022 at 06:43:50PM +0100, Mark Brown wrote:
> In 155433cb365ee466 ("arm64: cache: Remove support for ASID-tagged VIVT
> I-caches") we removed all the support fir AIVIVT cache types and renamed

s/fir/for/

> all references to the field to say "unknown" since support for AIVIVT
> caches was removed from the architecture. Some confusion has resulted since
> the corresponding change to the architecture left the value named as
> AIVIVT but documented it as reserved in v8, refactor the code so we don't
> define the constant instead. This will help with automatic generation of
> this register field since it means we care less about the correspondence
> with the ARM.
> 
> No functional change, the value displayed to userspace is unchanged.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/cache.h |  1 -
>  arch/arm64/kernel/cpuinfo.c    | 27 +++++++++++++++++----------
>  2 files changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
> index 7c2181c72116..0cbe75b9e4e5 100644
> --- a/arch/arm64/include/asm/cache.h
> +++ b/arch/arm64/include/asm/cache.h
> @@ -25,7 +25,6 @@
>  #define CTR_L1IP(ctr)		(((ctr) >> CTR_L1IP_SHIFT) & CTR_L1IP_MASK)
>  
>  #define ICACHE_POLICY_VPIPT	0
> -#define ICACHE_POLICY_RESERVED	1
>  #define ICACHE_POLICY_VIPT	2
>  #define ICACHE_POLICY_PIPT	3
>  
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index 8eff0a34ffd4..7ecf9ffb590b 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -33,12 +33,19 @@
>  DEFINE_PER_CPU(struct cpuinfo_arm64, cpu_data);
>  static struct cpuinfo_arm64 boot_cpu_data;
>  
> -static const char *icache_policy_str[] = {
> -	[ICACHE_POLICY_VPIPT]		= "VPIPT",
> -	[ICACHE_POLICY_RESERVED]	= "RESERVED/UNKNOWN",
> -	[ICACHE_POLICY_VIPT]		= "VIPT",
> -	[ICACHE_POLICY_PIPT]		= "PIPT",
> -};
> +static inline const char *icache_policy_str(int l1ip)
> +{
> +	switch (l1ip) {
> +        case ICACHE_POLICY_VPIPT:
> +                return "VPIPT";
> +	case ICACHE_POLICY_VIPT:
> +                return "VIPT";
> +	case ICACHE_POLICY_PIPT:
> +                return "PIPT";
> +        default:
> +                return "RESERVED/UNKNOWN";
> +        }

Looks like the indentation has gone wonky here.

>  unsigned long __icache_flags;
>  
> @@ -342,19 +349,19 @@ static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
>  	u32 l1ip = CTR_L1IP(info->reg_ctr);
>  
>  	switch (l1ip) {
> -	case ICACHE_POLICY_PIPT:
> -		break;
>  	case ICACHE_POLICY_VPIPT:
>  		set_bit(ICACHEF_VPIPT, &__icache_flags);
>  		break;
> -	case ICACHE_POLICY_RESERVED:
>  	case ICACHE_POLICY_VIPT:
>  		/* Assume aliasing */
>  		set_bit(ICACHEF_ALIASING, &__icache_flags);
>  		break;

Shouldn't we still assume aliasing if we see an I-cache that we don't know
about?

Will

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