Re: [PATCH v8 2/2] x86/cacheinfo: Delete global num_cache_leaves

[Ricardo Neri]

On Wed, Dec 04, 2024 at 03:32:06PM +0100, Borislav Petkov wrote:
> On Wed, Nov 27, 2024 at 04:22:47PM -0800, Ricardo Neri wrote:
> >  arch/x86/kernel/cpu/cacheinfo.c | 41 +++++++++++++++------------------
> >  1 file changed, 18 insertions(+), 23 deletions(-)
> 
> Does that work too?
> 
> ---
> diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
> index 95e38ab98a72..e6fa03ed9172 100644
> --- a/arch/x86/kernel/cpu/cacheinfo.c
> +++ b/arch/x86/kernel/cpu/cacheinfo.c
> @@ -715,22 +715,23 @@ void cacheinfo_hygon_init_llc_id(struct cpuinfo_x86 *c)
>  
>  void init_amd_cacheinfo(struct cpuinfo_x86 *c)
>  {
> -
> -	unsigned int cpu = c->cpu_index;
> +	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
>  
>  	if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
> -		get_cpu_cacheinfo(cpu)->num_leaves = find_num_cache_leaves(c);
> +		ci->num_leaves = find_num_cache_leaves(c);
>  	} else if (c->extended_cpuid_level >= 0x80000006) {
>  		if (cpuid_edx(0x80000006) & 0xf000)
> -			get_cpu_cacheinfo(cpu)->num_leaves = 4;
> +			ci->num_leaves = 4;
>  		else
> -			get_cpu_cacheinfo(cpu)->num_leaves = 3;
> +			ci->num_leaves = 3;
>  	}
>  }
>  
>  void init_hygon_cacheinfo(struct cpuinfo_x86 *c)
>  {
> -	get_cpu_cacheinfo(c->cpu_index)->num_leaves = find_num_cache_leaves(c);
> +	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
> +
> +	ci->num_leaves = find_num_cache_leaves(c);
>  }
>  
>  void init_intel_cacheinfo(struct cpuinfo_x86 *c)
> @@ -740,20 +741,21 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
>  	unsigned int new_l1d = 0, new_l1i = 0; /* Cache sizes from cpuid(4) */
>  	unsigned int new_l2 = 0, new_l3 = 0, i; /* Cache sizes from cpuid(4) */
>  	unsigned int l2_id = 0, l3_id = 0, num_threads_sharing, index_msb;
> +	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(c->cpu_index);
>  
>  	if (c->cpuid_level > 3) {
>  		/*
>  		 * There should be at least one leaf. A non-zero value means
>  		 * that the number of leaves has been initialized.
>  		 */
> -		if (!get_cpu_cacheinfo(c->cpu_index)->num_leaves)
> -			get_cpu_cacheinfo(c->cpu_index)->num_leaves = find_num_cache_leaves(c);
> +		if (!ci->num_leaves)
> +			ci->num_leaves = find_num_cache_leaves(c);
>  
>  		/*
>  		 * Whenever possible use cpuid(4), deterministic cache
>  		 * parameters cpuid leaf to find the cache details
>  		 */
> -		for (i = 0; i < get_cpu_cacheinfo(c->cpu_index)->num_leaves; i++) {
> +		for (i = 0; i < ci->num_leaves; i++) {
>  			struct _cpuid4_info_regs this_leaf = {};
>  			int retval;
>  
> @@ -789,14 +791,14 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
>  	 * Don't use cpuid2 if cpuid4 is supported. For P4, we use cpuid2 for
>  	 * trace cache
>  	 */
> -	if ((!get_cpu_cacheinfo(c->cpu_index)->num_leaves || c->x86 == 15) && c->cpuid_level > 1) {
> +	if ((!ci->num_leaves || c->x86 == 15) && c->cpuid_level > 1) {
>  		/* supports eax=2  call */
>  		int j, n;
>  		unsigned int regs[4];
>  		unsigned char *dp = (unsigned char *)regs;
>  		int only_trace = 0;
>  
> -		if (get_cpu_cacheinfo(c->cpu_index)->num_leaves && c->x86 == 15)
> +		if (ci->num_leaves && c->x86 == 15)
>  			only_trace = 1;
>  
>  		/* Number of times to iterate */
> @@ -990,8 +992,10 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
>  
>  int init_cache_level(unsigned int cpu)
>  {
> +	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
> +
>  	/* There should be at least one leaf. */
> -	if (!get_cpu_cacheinfo(cpu)->num_leaves)
> +	if (!ci->num_leaves)
>  		return -ENOENT;
>  
>  	return 0;

Yes, this looks OK to me and is an improvement.

I can post a v9 with these changes.

I think I can keep the Reviewed-by tags as they were given to Patch 1.

Thanks and BR,
Ricardo