All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luck, Tony" <tony.luck@intel.com>
To: Reinette Chatre <reinette.chatre@intel.com>
Cc: Fenghua Yu <fenghuay@nvidia.com>,
	Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>,
	Peter Newman <peternewman@google.com>,
	James Morse <james.morse@arm.com>,
	Babu Moger <babu.moger@amd.com>,
	"Drew Fustini" <dfustini@baylibre.com>,
	Dave Martin <Dave.Martin@arm.com>, Chen Yu <yu.c.chen@intel.com>,
	<x86@kernel.org>, <linux-kernel@vger.kernel.org>,
	<patches@lists.linux.dev>
Subject: Re: [RFC PATCH 1/4] x86/resctrl: Check if monitoring features are enabled
Date: Fri, 21 Aug 2026 15:12:11 -0700	[thread overview]
Message-ID: <aojNO9aBBOUcTwX5@agluck-desk3> (raw)
In-Reply-To: <c3cd54b2-8752-47b9-8864-8444254e8be8@intel.com>

On Fri, Aug 21, 2026 at 02:00:41PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 8/21/26 12:44 PM, Luck, Tony wrote:
> > On Fri, Aug 21, 2026 at 11:18:51AM -0700, Reinette Chatre wrote:
> >> Hi Tony,
> >>
> >> Thank you for doing this.
> >>
> >> On 8/19/26 9:13 AM, Tony Luck wrote:
> >>> Both Intel and AMD manuals say that software must first check
> >>> CPUID(0x7,0x0).EBX[12] to see if any monitoring features are enabled
> >>> before checking for specific features enabled in subleaves.
> >>>
> >>> Add the check for X86_FEATURE_CQM.
> >>>
> >>> Fixes: cbc82b172638 ("x86: Add support for Intel Cache QoS Monitoring (CQM) detection")
> >>> Signed-off-by: Tony Luck <tony.luck@intel.com>
> >>> ---
> >>>  arch/x86/kernel/cpu/resctrl/core.c | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> >>> index 55214d6fdc49..2677b8a6c15b 100644
> >>> --- a/arch/x86/kernel/cpu/resctrl/core.c
> >>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> >>> @@ -968,6 +968,9 @@ static __init bool get_rdt_mon_resources(void)
> >>>  	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> >>>  	bool ret = false;
> >>>  
> >>> +	if (!cpu_feature_enabled(X86_FEATURE_CQM))
> >>> +		return false;
> >>
> >> Is this missing a check of X86_FEATURE_CQM_LLC also?
> >>
> >> As I understand resctrl learns from X86_FEATURE_CQM whether the system supports
> >> resource monitoring in general. Contrary to what the feature name suggests, there is
> >> another step needed to determine which resource(s) support monitoring via
> >> CPUID(0xF,0x0).EDX where bit 1 indicates LLC monitoring that needs to be set before
> >> the LLC resource-specific monitoring properties can/should be determined.
> > 
> > Yes. Missing that check. I agree that the feature define names are bad.
> > 
> > To avoid continued confusion I should rename the #defines to match the
> > bit names in the Intel SDM (but leaving the /proc/cpuinfo visible
> > strings at "cqm" and "cqm_llc" as those are user ABI now).
> 
> Naming is as always complicated and here I do not see why the name should be
> picked from Intel SDM instead of AMD's spec. Renaming may be secondary goal. It may even
> add to confusion to have feature name mismatch what is exposed to user space? A change
> like this could perhaps be punted to when/if monitoring of a new resource needs to be
> supported?

Two reasons to pick the Intel SDM name:
1) Intel was here first
   commit cbc82b172638 ("x86: Add support for Intel Cache QoS Monitoring (CQM) detection")
2) Symmetry with the feature bit for allocation:
   #define X86_FEATURE_RDT_A            ( 9*32+15) /* "rdt_a" Resource Director Technology Allocation */

Confusion between the string in /proc/cpuinfo and the X86_FEATURE define
is already rampant. Just a few examples:

#define X86_FEATURE_XMM                 ( 0*32+25) /* "sse" */
#define X86_FEATURE_XSTORE              ( 5*32+ 2) /* "rng" RNG present (xstore) */
#define X86_FEATURE_SVML                (15*32+ 2) /* "svm_lock" SVM locking MSR */
#define X86_FEATURE_TSCRATEMSR          (15*32+ 4) /* "tsc_scale" TSC scaling support */

So anyone wanting to find the #define that goes with a feature name in
/proc/cpuinfo would be advised to just grep for the string in
<asm/cpufeatures.h>

> 
> > 
> > X86_FEATURE_CQM -> X86_FEATURE_RDT_M
> > X86_FEATURE_CQM_LLC -> X86_FEATURE_L3_MON
> > 
> >>
> >> Apart from the checks here I see that cpuid_deps[] accurately reflects the relationship
> >> between X86_FEATURE_CQM_LLC and the different LLC monitoring features checked for below.
> >> I do not see cpuid_deps[] capturing the relationship between X86_FEATURE_CQM and
> >> X86_FEATURE_CQM_LLC though. Could adding it complete the handling of relationships between
> >> these leaves?
> > 
> > Yes.
> >>
> >> Similarly I think cpuid_deps[] may be missing X86_FEATURE_ABMC's dependency on 
> >> X86_FEATURE_CQM_LLC. 
> > 
> > Maybe no? X86_FEATURE_ABMC isn't enumerated in CPUID(0xF,*). It comes
> > from the AMD CPUID(0x80000020,0)EBX{5}
> > 
> > Babu: The AMD architecture programmer's manual just says:
> > 
> >   "Support for ABMC is identified by CPUID Fn8000_0020_EBX_x0[ABMC] (bit 5)
> >    being set. If ABMC is supported, the feature’s attributes and capabilities
> >    are enumerated by CPUID Fn8000_0020_x5 as detailed in Appendix E of APM volume 3"
> > 
> > It isn't explicitly stated whether this depends on CPUID Fn0000_000F_x0
> > EDX{1}, which AMD names: "L3CacheMon - L3 monitoring capability"
> 
> Even if the spec does not explicitly state this, the implementation requires this.
> 
> I believe patch 2/4 makes this clear since it demonstrates how resctrl obtains the general
> L3 monitoring properties from CPUID(0xF, 0x1) before moving to the ABMC feature specific
> properties.

Agreed.

> How these general L3 monitoring properties are required by ABMC can also be seen
> in the ABMC counter reading code:  resctrl_arch_cntr_read() calls get_corrected_val()
> that uses the general L3 monitoring properties hw_res->mbm_width and hw_res->mon_scale.
> ABMC feature properties also do not expose its own number of RMID supported, this is
> learned from general L3 monitoring properties.
> 
> One item of note here is just that ABMC (as I understand) does _not_ depend on any of
> the individual L3 monitoring features (X86_FEATURE_CQM_OCCUP_LLC, X86_FEATURE_CQM_MBM_TOTAL,
> and X86_FEATURE_CQM_MBM_LOCAL) since it defines its own transactions that can be counted.
> 
> Reinette

-Tony

  reply	other threads:[~2026-08-21 22:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-19 16:13 [RFC PATCH 0/4] x86/resctrl: Refactor resctrl enumeration Tony Luck
2026-08-19 16:13 ` [RFC PATCH 1/4] x86/resctrl: Check if monitoring features are enabled Tony Luck
2026-08-21 18:18   ` Reinette Chatre
2026-08-21 19:44     ` Luck, Tony
2026-08-21 21:00       ` Reinette Chatre
2026-08-21 22:12         ` Luck, Tony [this message]
2026-08-19 16:13 ` [RFC PATCH 2/4] x86/resctrl: Enumerate monitor features in rdt_get_l3_mon_config() Tony Luck
2026-08-19 16:13 ` [RFC PATCH 3/4] x86/resctrl: Apply Intel MBM quirk from rdt_get_l3_mon_config() Tony Luck
2026-08-19 16:13 ` [RFC PATCH 4/4] x86/resctrl: Delete resctrl_cpu_detect() Tony Luck
2026-08-19 20:08   ` Borislav Petkov
2026-08-19 17:12 ` [RFC PATCH 0/4] x86/resctrl: Refactor resctrl enumeration Luck, Tony

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aojNO9aBBOUcTwX5@agluck-desk3 \
    --to=tony.luck@intel.com \
    --cc=Dave.Martin@arm.com \
    --cc=babu.moger@amd.com \
    --cc=dfustini@baylibre.com \
    --cc=fenghuay@nvidia.com \
    --cc=james.morse@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maciej.wieczor-retman@intel.com \
    --cc=patches@lists.linux.dev \
    --cc=peternewman@google.com \
    --cc=reinette.chatre@intel.com \
    --cc=x86@kernel.org \
    --cc=yu.c.chen@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.