Re: [PATCH v11 13/23] x86/resctrl: Handle systems when AET is the only resource

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

Hi Tony,

On 8/31/26 10:44 AM, Tony Luck wrote:
> On a system with no L3 monitor resources it should still be possible to
> run with just AET.
> 
> Update get_rdt_mon_resources() to return true based on whether the
> system supports monitoring rather than on the presence of any of the
> L3 monitor features.
> 
> Make rdt_get_tree() handle the case where AET is the only feature, but
> is not enabled for a specific mount.

I think it will be more useful to describe how this scenario is handled than
a mention that it is handled and reviewer needs to look for how in the patch.
Consider something like: "Do not allow mount of resctrl filesystem if AET is
the only feature, but it is not enabled."

Even so, this does not seem accurate to me since it implies that any monitoring
capable system has AET as the only feature, which does not seem the right thing
to infer?

> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
> v11:
> 	Split the AET RMID cap change into separate commit.
> 	s/Fix get_rdt_mon_resources/Update get_rdt_mon_resources/
> 	New subject for this patch
> 
>  arch/x86/kernel/cpu/resctrl/internal.h | 2 +-
>  arch/x86/kernel/cpu/resctrl/core.c     | 7 ++++---
>  arch/x86/kernel/cpu/resctrl/monitor.c  | 4 +---
>  fs/resctrl/rdtgroup.c                  | 8 ++++++--
>  4 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index e46eb9a4c725..d70c8cb1cb81 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -226,7 +226,7 @@ union l3_qos_abmc_cfg {
>  
>  void rdt_ctrl_update(void *arg);
>  
> -int rdt_get_l3_mon_config(struct rdt_resource *r);
> +void rdt_get_l3_mon_config(struct rdt_resource *r);
>  
>  bool rdt_cpu_has(int flag);
>  
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index ef37fbb586d3..0ddec2b67e3d 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -1016,10 +1016,11 @@ static __init bool get_rdt_mon_resources(void)
>  			ret = true;
>  	}
>  
> -	if (!ret)
> -		return false;
> +	if (ret)
> +		rdt_get_l3_mon_config(r);
>  
> -	return !rdt_get_l3_mon_config(r);
> +	/* Even if there are no L3 monitor events, AET might be supported */
> +	return resctrl_arch_system_max_rmid_idx() > 0;
>  }

Please see comments about get_rdt_mon_resources() in patch #2 and patch #12.

>  
>  static __init void __check_quirks_intel(void)
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index bc2df287ccfa..e3485dc55923 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -431,7 +431,7 @@ static void __init intel_rdt_mbm_apply_quirk(u32 num_rmid)
>  	mbm_cf = mbm_cf_table[cf_index].cf;
>  }
>  
> -int __init rdt_get_l3_mon_config(struct rdt_resource *r)
> +void __init rdt_get_l3_mon_config(struct rdt_resource *r)
>  {
>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>  	unsigned int mbm_offset;
> @@ -514,8 +514,6 @@ int __init rdt_get_l3_mon_config(struct rdt_resource *r)
>  		intel_rdt_mbm_apply_quirk(r->mon.num_rmid);
>  
>  	r->mon_capable = true;
> -
> -	return 0;
>  }
>  
>  static void resctrl_abmc_set_one_amd(void *arg)
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index de07ccdffdb2..3ac518ed2368 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -3195,6 +3195,11 @@ static int rdt_get_tree(struct fs_context *fc)
>  		goto out;
>  	}
>  
> +	if (!resctrl_alloc_capable() && !resctrl_mon_capable()) {
> +		ret = -EINVAL;

EINVAL is quite a "catch-all" for mounting errors. I think we could treat
dmesg via "invalfc()" as the mount equivalent of last_cmd_status, this already
started with handling of the mba_MBps mount option.

More specifically, please consider something like:
		
		ret = invalfc(fc, "No allocation or monitoring available/(or enabled?).\n");


> +		goto out;
> +	}
> +
>  	ret = setup_rmid_lru_list();
>  	if (ret)
>  		goto out;
> @@ -3253,8 +3258,7 @@ static int rdt_get_tree(struct fs_context *fc)
>  	if (resctrl_mon_capable())
>  		resctrl_arch_enable_mon();
>  
> -	if (resctrl_alloc_capable() || resctrl_mon_capable())
> -		resctrl_mounted = true;
> +	resctrl_mounted = true;
>  
>  	if (resctrl_is_mbm_enabled()) {
>  		r = resctrl_arch_get_resource(RDT_RESOURCE_L3);

Reinette