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

[Chen, Yu C]

Hi Reinette,

On 8/20/2026 7:08 AM, Reinette Chatre wrote:
> Hi Chenyu,
>
> On 7/25/26 2:23 AM, 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 considers firmware (ERDT table and its sub-tables) support for
> > an event, but 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 read its monitoring data via MMIO exist, otherwise callers would
>
> Are these helpers limited to monitoring ERDT features? First paragraph mentions
> "support for an event" and above mentions "read its monitoring data" that
> imply these helpers are limited to monitoring features but I do not think this is
> the case?

They are not only for monitoring features, but for monitor/allocation 
features.
Let me adjust the description.

> > take the ERDT path and find no implementation behind it. Features are
> > added to erdt_support() as their MMIO implementations are introduced.
>
> nit: "are added" -> "should be added"

OK.

> > Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
> > Tested-by: Hongyu Ning <hongyu.ning@xxxxxxxxxxxxxxx>
> > ---
> > v5->v6:
> >      New patch. Split out of the v5 "x86/resctrl: Introduce helpers to
> >      read L3 occupancy via MMIO" patch, which carried two logical
> >      changes. (Reinette Chatre)
> >      Rename erdt_support_features() to erdt_support(). (Reinette Chatre)
> >      Reword "expects x86 feature flag as parameters" to "expects an x86
> >      feature flag as parameter". (Reinette Chatre)
> >      Explain in the changelog why erdt_support() returns false here: a
> >      feature is only reported as ERDT backed once the MMIO accessors that
> >      read its monitoring data exist. (Reinette Chatre)
> >      Drop the unused asm/cpufeatures.h include. (Reinette Chatre)
> > ---
> >   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 |  2 ++
> >   4 files changed, 31 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> > index e60c2aea7ebd..5491853113dd 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);
> >   int erdt_get_scale(void);
> >   
>
> Can this declaration be dropped? Below there is a duplicate declaration
> added to arch/x86/kernel/cpu/resctrl/internal.h that should be sufficient?

This declaration is needed by [PATCH 9/9] in 
resctrl_arch_round_mon_val(), which
uses erdt_cpu_has(). Maybe we can drop erdt_cpu_has() from internal.h, 
although
I thought exposing both erdt_cpu_has() and erdt_support() might be 
useful too.

thanks,
Chenyu