Re: [PATCH v12 04/25] x86/resctrl: Apply Intel MBM quirk from rdt_get_l3_mon_config()

[Babu Moger <babu.moger@amd.com>]

Hi Tony,

I cannot test the AET telemetry functionality, but I have tested the 
series on my AMD systems. It appears to work as expected, with no 
observed side effects.

A few comments below.

On 9/16/26 18:12, Tony Luck wrote:
> The Intel quirk to adjust Memory Bandwidth Monitoring (MBM) values on
> certain CPUs is applied early, before discovering if MBM is supported.
> 
> Move the call into rdt_get_l3_mon_config(), but keep the decision on
> whether it is needed in __check_quirks_intel() with all the other
> model specific tests.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
> v12:
> 	Change type of cf_index to unsigned in intel_rdt_mbm_apply_quirk()
> ---
>   arch/x86/kernel/cpu/resctrl/internal.h |  4 +--
>   arch/x86/kernel/cpu/resctrl/core.c     |  2 +-
>   arch/x86/kernel/cpu/resctrl/monitor.c  | 35 +++++++++++++++-----------
>   3 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index e3cfa0c10e92..e46eb9a4c725 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -151,6 +151,8 @@ static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r
>   
>   extern struct rdt_hw_resource rdt_resources_all[];
>   
> +extern bool __initdata intel_rdt_mbm_need_quirk;
> +
>   void arch_mon_domain_online(struct rdt_resource *r, struct rdt_l3_mon_domain *d);
>   
>   /* CPUID.(EAX=10H, ECX=ResID=1).EAX */
> @@ -228,8 +230,6 @@ int rdt_get_l3_mon_config(struct rdt_resource *r);
>   
>   bool rdt_cpu_has(int flag);
>   
> -void __init intel_rdt_mbm_apply_quirk(void);
> -
>   void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
>   void resctrl_arch_mbm_cntr_assign_set_one(struct rdt_resource *r);
>   
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 34ca3c7cc046..82b27a0157ae 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -1011,7 +1011,7 @@ static __init void __check_quirks_intel(void)
>   			set_rdt_options("!l3cat");
>   		fallthrough;
>   	case INTEL_BROADWELL_X:
> -		intel_rdt_mbm_apply_quirk();
> +		intel_rdt_mbm_need_quirk = true;
>   		break;
>   	}
>   }
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index 89b83cc7e800..ab9cb34145c5 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -37,6 +37,8 @@ bool rdt_mon_capable;
>    */
>   unsigned int __ro_after_init rdt_l3_mon_scale;
>   
> +bool __initdata intel_rdt_mbm_need_quirk;
> +
>   #define CF(cf)	((unsigned long)(1048576 * (cf) + 0.5))
>   
>   static int snc_nodes_per_l3_cache = 1;
> @@ -51,7 +53,7 @@ static int snc_nodes_per_l3_cache = 1;
>    * 1. The threshold 0 is changed to rmid count - 1 so don't do correction
>    *    for the case.
>    * 2. MBM total and local correction table indexed by core counter which is
> - *    equal to (x86_cache_max_rmid + 1) / 8 - 1 and is from 0 up to 27.
> + *    equal to r->mon.num_rmid / 8 - 1 and is from 0 up to 27.
>    * 3. The correction factor is normalized to 2^20 (1048576) so it's faster
>    *    to calculate corrected value by shifting:
>    *    corrected_value = (original_value * correction_factor) >> 20
> @@ -421,6 +423,20 @@ static __init int snc_get_config(void)
>   	return ret;
>   }
>   
> +static void __init intel_rdt_mbm_apply_quirk(u32 num_rmid)
> +{
> +	unsigned int cf_index;
> +
> +	cf_index = num_rmid / 8 - 1;
> +	if (cf_index >= ARRAY_SIZE(mbm_cf_table)) {
> +		pr_info("No MBM correction factor available\n");
> +		return;
> +	}
> +
> +	mbm_cf_rmidthreshold = mbm_cf_table[cf_index].rmidthreshold;
> +	mbm_cf = mbm_cf_table[cf_index].cf;
> +}
> +
>   int __init rdt_get_l3_mon_config(struct rdt_resource *r)
>   {
>   	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
> @@ -500,25 +516,14 @@ int __init rdt_get_l3_mon_config(struct rdt_resource *r)
>   		hw_res->mbm_cntr_assign_enabled = true;
>   	}
>   
> +	if (intel_rdt_mbm_need_quirk)
> +		intel_rdt_mbm_apply_quirk(r->mon.num_rmid);

Not a bug right now.  Is it cleaner to send num_rmid instead of SNC 
adjusted r->mon.num_rmid ?

Thanks
Babu