Re: [PATCH v8 8/9] x86/resctrl: Introduce erdt_cpu_has() and erdt_support()

[Reinette Chatre]

Hi Chenyu,

On 9/17/26 9:51 PM, Chen Yu wrote:
> Introduce erdt_cpu_has() to verify if a specific RDT feature is backed by an
> ERDT table. erdt_cpu_has() is derived from rdt_cpu_has(), which not only

"is derived from" - needs imperative

> considers firmware (ERDT table and its sub-tables) support for a feature, but

"rdt_cpu_has(), which not only considers firmware (ERDT table ..." reads as though
rdt_cpu_has() considers ERDT, but it does not?

> also considers userspace input like "rdt=!cmt". Similar to rdt_cpu_has(),
> erdt_cpu_has() expects an x86 feature flag as parameter.
> 
> Introduce erdt_support() as a helper called by erdt_cpu_has() to check whether
> the ERDT firmware supports a given feature.
> 
> erdt_support() deliberately claims no feature yet and returns false. A feature
> may only be reported as ERDT backed once the low level accessors that access
> its data via MMIO exist, otherwise callers would take the ERDT path and find no
> implementation behind it. Features should be added to erdt_support() as their
> MMIO implementations are introduced.
> 
> Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
> Tested-by: Hongyu Ning <hongyu.ning@xxxxxxxxxxxxxxx>
> ---
>  arch/x86/include/asm/resctrl.h         |  1 +
>  arch/x86/kernel/cpu/resctrl/core.c     | 23 +++++++++++++++++++++++
>  arch/x86/kernel/cpu/resctrl/erdt.c     |  5 +++++
>  arch/x86/kernel/cpu/resctrl/internal.h |  1 +
>  4 files changed, 30 insertions(+)
> 
> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> index 9a32ed418c33..bcfcc8ac1fe6 100644
> --- a/arch/x86/include/asm/resctrl.h
> +++ b/arch/x86/include/asm/resctrl.h
> @@ -49,6 +49,7 @@ DECLARE_STATIC_KEY_FALSE(rdt_enable_key);
>  DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
>  DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
>  
> +bool erdt_cpu_has(int flag);
>  unsigned int erdt_get_scale(void);

Same comment as earlier in response to the erdt_get_scale() prototype
addition.

>  
>  static inline bool resctrl_arch_alloc_capable(void)
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 3514d73a8056..ca7e67f976d5 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -897,6 +897,29 @@ bool rdt_cpu_has(int flag)
>  	return ret;
>  }
>  
> +bool erdt_cpu_has(int flag)
> +{
> +	struct rdt_options *o;
> +	bool ret;
> +
> +	ret = erdt_support(flag);
> +
> +	if (!ret)
> +		return ret;
> +
> +	for (o = rdt_options; o < &rdt_options[NUM_RDT_OPTIONS]; o++) {
> +		if (flag == o->flag) {
> +			if (o->force_off)
> +				ret = false;
> +			if (o->force_on)
> +				ret = true;
> +			break;
> +		}
> +	}
> +
> +	return ret;
> +}
> +

erdt_cpu_has() is practically identical to rdt_cpu_has() just above it. how about
something like:

	static bool apply_rdt_options(int flag, bool ret)
	{
		struct rdt_options *o;

		for (o = rdt_options; o < &rdt_options[NUM_RDT_OPTIONS]; o++) {
			...
	}

	bool rdt_cpu_has(int flag)
	{
		return apply_rdt_options(flag, boot_cpu_has(flag));
	}

	bool erdt_cpu_has(int flag)
	{
		return apply_rdt_options(flag, erdt_support(flag));
	}

>  bool resctrl_arch_is_evt_configurable(enum resctrl_event_id evt)
>  {
>  	if (!rdt_cpu_has(X86_FEATURE_BMEC))
> diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
> index a8a7417c2f82..400973ebf2b8 100644
> --- a/arch/x86/kernel/cpu/resctrl/erdt.c
> +++ b/arch/x86/kernel/cpu/resctrl/erdt.c
> @@ -48,6 +48,11 @@ static unsigned int erdt_max_rmid;
>   */
>  static unsigned int erdt_scale;
>  
> +bool erdt_support(int flag)
> +{
> +	return false;
> +}
> +
>  unsigned int erdt_get_max_rmid(void)
>  {
>  	return erdt_max_rmid;
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 26c3e0c546ad..ce037e017fc3 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -287,6 +287,7 @@ static inline void intel_aet_mon_domain_setup(int cpu, int id, struct rdt_resour
>  static inline bool intel_handle_aet_option(bool force_off, char *tok) { return false; }
>  #endif
>  
> +bool erdt_support(int flag);
>  unsigned int erdt_get_max_rmid(void);
>  int erdt_init(void);
>  void erdt_exit(void);

Reinette