Re: [PATCH v2 02/16] fs, x86/resctrl: Add architecture routines for kernel mode initialization

[Reinette Chatre]

Hi Babu,

On 3/12/26 1:36 PM, Babu Moger wrote:
> Implement the resctrl kernel mode (kmode) arch initialization.
> 
> - Add resctrl_arch_get_kmode_cfg() to fill the default kernel mode
>   (INHERIT_CTRL_AND_MON). This can be extended later (e.g. for PLZA) to set
>   additional modes.

I do not think this is something that the architecture should set, at least
at this time. Every mode has different requirements and this just lets the arch set
it without any support for what configurations it implies. For example, if
arch sets a different default mode than INHERIT_CTRL_AND_MON then PQR_PLZA_ASSOC
needs to be programmed as the CPUs come online and this does not seem to
accommodate this. This implementation appears to have significant assumptions on
what architecture will end up setting since it is only considering PLZA.

> 
> - Add global resctrl_kcfg and resctrl_kmode_init() to initialize default
>   values.
> 
> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> ---
> v2: New patch to handle PLZA interfaces with /sys/fs/resctrl/info/ directory.
>     https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@xxxxxxxxx/
> ---
>  arch/x86/kernel/cpu/resctrl/core.c |  7 +++++++
>  fs/resctrl/rdtgroup.c              | 10 ++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 7667cf7c4e94..4c3ab2d93909 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -892,6 +892,13 @@ bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt)
>  	}
>  }
>  
> +void resctrl_arch_get_kmode_cfg(struct resctrl_kmode_cfg *kcfg)
> +{
> +	kcfg->kmode = INHERIT_CTRL_AND_MON;
> +	kcfg->kmode_cur = INHERIT_CTRL_AND_MON;
> +	kcfg->k_rdtgrp = NULL;
> +}

I already commented on the arch vs filesystem settings.

When using an arch helper this forces all architectures to support this helper. Is a
helper required? Is it perhaps possible for arch to set a property instead? For example,
how enumeration is handled? 
I think the assumption here is that INHERIT_CTRL_AND_MON is the default and expected to
be supported by all architectures. I do not see why arch should set this as default but
instead this should be from resctrl fs. At the same time it is expected that the
architecture supports this mode so there needs to be a failure if an architecture does
not support this mode?

I'm going to stop here. I think the comments so far may result in major changes already
making further detailed review of patches unnecessary.

Reinette