Re: [PATCH 01/15] x86/resctrl: Remove hard-coded memory bandwidth limit

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Babu,

On 11/30/2023 4:57 PM, Babu Moger wrote:
> The QOS Memory Bandwidth Enforcement Limit is reported by
> CPUID_Fn80000020_EAX_x01.
> Bits Description
> 31:0 BW_LEN: Size of the QOS Memory Bandwidth Enforcement Limit.
> 
> Remove the hardcoded bandwidth limit value and detect using CPUID command.
> 
> The CPUID details are documentation in the PPR listed below [1].
> [1] Processor Programming Reference (PPR) Vol 1.1 for AMD Family 19h Model
> 11h B1 - 55901 Rev 0.25.
> 
> Fixes: 4d05bf71f157 ("x86/resctrl: Introduce AMD QOS feature")

What makes this change worthy of the "Fixes:" tag. What is the impact
of this? Does this mean that there is AMD hardware out there that is
not being used correctly? In this case it should be highlighted and
the stable folks included?

This also does not seem like it belongs in this series and should
be sent separately as a fix.

> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
> ---
>  arch/x86/kernel/cpu/resctrl/core.c     | 2 +-
>  arch/x86/kernel/cpu/resctrl/internal.h | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 19e0681f0435..3fbae10b662d 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -243,7 +243,7 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
>  
>  	cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full);
>  	hw_res->num_closid = edx.split.cos_max + 1;
> -	r->default_ctrl = MAX_MBA_BW_AMD;
> +	r->default_ctrl = 1 << eax.full;

This does not seem appropriate. You are using eax because it
it convenient but if you take a look at its definition it does not
match the AMD CPUID instruction output at all.

>  
>  	/* AMD does not use delay */
>  	r->membw.delay_linear = false;
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index a4f1aa15f0a2..d2979748fae4 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -18,7 +18,6 @@
>  #define MBM_OVERFLOW_INTERVAL		1000
>  #define MAX_MBA_BW			100u
>  #define MBA_IS_LINEAR			0x4
> -#define MAX_MBA_BW_AMD			0x800
>  #define MBM_CNTR_WIDTH_OFFSET_AMD	20
>  
>  #define RMID_VAL_ERROR			BIT_ULL(63)

Reinette