Re: [PATCH v10 07/24] x86/resctrl: Add support to enable/disable AMD ABMC feature

From: Reinette Chatre
Date: Thu Dec 19 2024 - 16:48:59 EST


Hi Babu,

On 12/12/24 12:15 PM, Babu Moger wrote:

> static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r)
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 687d9d8d82a4..d54c2701c09c 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c

These functions are clearly monitoring related. Is there a reason why they are
in rdtgroup.c and not in monitor.c?

> @@ -2402,6 +2402,42 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable)
> return 0;
> }
>
> +static void resctrl_abmc_set_one_amd(void *arg)
> +{
> + bool *enable = arg;
> +
> + if (*enable)
> + msr_set_bit(MSR_IA32_L3_QOS_EXT_CFG, ABMC_ENABLE_BIT);
> + else
> + msr_clear_bit(MSR_IA32_L3_QOS_EXT_CFG, ABMC_ENABLE_BIT);
> +}
> +
> +/*
> + * Update L3_QOS_EXT_CFG MSR on all the CPUs associated with the monitor
> + * domain.
> + */
> +static void _resctrl_abmc_enable(struct rdt_resource *r, bool enable)
> +{
> + struct rdt_mon_domain *d;
> +
> + list_for_each_entry(d, &r->mon_domains, hdr.list)
> + on_each_cpu_mask(&d->hdr.cpu_mask,
> + resctrl_abmc_set_one_amd, &enable, 1);
> +}
> +
> +int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable)
> +{
> + struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
> +
> + if (r->mon.mbm_cntr_assignable &&
> + hw_res->mbm_cntr_assign_enabled != enable) {
> + _resctrl_abmc_enable(r, enable);
> + hw_res->mbm_cntr_assign_enabled = enable;
> + }
> +
> + return 0;
> +}
> +
> /*
> * We don't allow rdtgroup directories to be created anywhere
> * except the root directory. Thus when looking for the rdtgroup
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 511cfce8fc21..f11d6fdfd977 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -355,4 +355,7 @@ void resctrl_arch_reset_rmid_all(struct rdt_resource *r, struct rdt_mon_domain *
> extern unsigned int resctrl_rmid_realloc_threshold;
> extern unsigned int resctrl_rmid_realloc_limit;
>
> +int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable);
> +bool resctrl_arch_mbm_cntr_assign_enabled(struct rdt_resource *r);
> +
> #endif /* _RESCTRL_H */

During the software controller work Boris stated [1] that these APIs should
only appear in the main header file at the time they are used. This series
makes a few changes to include/linux/resctrl.h that, considering this
feedback, should rather be in arch/x86/kernel/cpu/resctrl/internal.h
until MPAM starts using them.

Reinette

[1] https://lore.kernel.org/all/20241209222047.GKZ1dtPxIu5_Hxs1fp@fat_crate.local/