RE: [PATCH v8 03/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag

["Moger, Babu" <Babu.Moger@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 6640 bytes --]

[AMD Official Use Only - General]

Hi Fenghua,

> -----Original Message-----
> From: Yu, Fenghua <fenghua.yu@intel.com>
> Sent: Wednesday, November 23, 2022 12:17 PM
> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net; Chatre, Reinette
> <reinette.chatre@intel.com>; tglx@linutronix.de; mingo@redhat.com;
> bp@alien8.de
> Cc: dave.hansen@linux.intel.com; x86@kernel.org; hpa@zytor.com;
> paulmck@kernel.org; akpm@linux-foundation.org; quic_neeraju@quicinc.com;
> rdunlap@infradead.org; damien.lemoal@opensource.wdc.com;
> songmuchun@bytedance.com; peterz@infradead.org; jpoimboe@kernel.org;
> pbonzini@redhat.com; Bae, Chang Seok <chang.seok.bae@intel.com>;
> pawan.kumar.gupta@linux.intel.com; jmattson@google.com;
> daniel.sneddon@linux.intel.com; Das1, Sandipan <Sandipan.Das@amd.com>;
> Luck, Tony <tony.luck@intel.com>; james.morse@arm.com; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; bagasdotme@gmail.com;
> Eranian, Stephane <eranian@google.com>
> Subject: RE: [PATCH v8 03/13] x86/cpufeatures: Add Bandwidth Monitoring
> Event Configuration feature flag
> 
> Hi, Babu,
> 
> > Newer AMD processors support the new feature Bandwidth Monitoring
> > Event Configuration (BMEC).
> >
> > The feature support is identified via CPUID Fn8000_0020_EBX_x0 (ECX=0).
> > Bits    Field Name    Description
> > 3       EVT_CFG       Bandwidth Monitoring Event Configuration (BMEC)
> >
> > Currently, the bandwidth monitoring events mbm_total_bytes and
> > mbm_local_bytes are set to count all the total and local reads/writes
> > respectively. With the introduction of slow memory, the two counters
> > are not enough to count all the different types of memory events. With
> > the feature BMEC, the users have the option to configure
> > mbm_total_bytes and mbm_local_bytes to count the specific type of events.
> >
> > Each BMEC event has a configuration MSR, QOS_EVT_CFG (0xc000_0400h +
> > EventID) which contains one field for each bandwidth type that can be
> > used to configure the bandwidth event to track any combination of
> > supported bandwidth types. The event will count requests from every
> > bandwidth type bit that is set in the corresponding configuration register.
> >
> > Following are the types of events supported:
> >
> > ====    ========================================================
> > Bits    Description
> > ====    ========================================================
> > 6       Dirty Victims from the QOS domain to all types of memory
> > 5       Reads to slow memory in the non-local NUMA domain
> > 4       Reads to slow memory in the local NUMA domain
> > 3       Non-temporal writes to non-local NUMA domain
> > 2       Non-temporal writes to local NUMA domain
> > 1       Reads to memory in the non-local NUMA domain
> > 0       Reads to memory in the local NUMA domain
> > ====    ========================================================
> >
> > By default, the mbm_total_bytes configuration is set to 0x7F to count
> > all the event types and the mbm_local_bytes configuration is set to
> > 0x15 to count all the local memory events.
> >
> > Feature description is available in the specification, "AMD64
> > Technology Platform Quality of Service Extensions, Revision: 1.03
> > Publication
> >
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> > amd.com%2Fen%2Fsupport%2Ftech-docs%2Famd64-technology-
> &amp;data=05%7C0
> >
> 1%7Cbabu.moger%40amd.com%7C50e1807651fd4513648908dacd7efac0%7C3
> dd8961f
> >
> e4884e608e11a82d994e183d%7C0%7C0%7C638048242504277761%7CUnknow
> n%7CTWFp
> >
> bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn
> >
> 0%3D%7C3000%7C%7C%7C&amp;sdata=5lpXbZkZ78mJ1d9PnLf7WmRT5vPogfs
> 5HaZLz76
> > x04I%3D&amp;reserved=0
> > platform-quality-service-extensions
> > Link:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> >
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D206537&amp;data=05%7C01%7Cbab
> u.m
> >
> oger%40amd.com%7C50e1807651fd4513648908dacd7efac0%7C3dd8961fe488
> 4e608e
> >
> 11a82d994e183d%7C0%7C0%7C638048242504277761%7CUnknown%7CTWFpb
> GZsb3d8ey
> >
> JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C300
> >
> 0%7C%7C%7C&amp;sdata=2CjPpzCT4JeA9VPNZIW7zxyL22xpEm2FoXQlhAz5OK
> o%3D&am
> > p;reserved=0
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  arch/x86/include/asm/cpufeatures.h |    1 +
> >  arch/x86/kernel/cpu/cpuid-deps.c   |    1 +
> >  arch/x86/kernel/cpu/scattered.c    |    1 +
> >  3 files changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/cpufeatures.h
> > b/arch/x86/include/asm/cpufeatures.h
> > index d68b4c9c181d..6732ca0117be 100644
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -306,6 +306,7 @@
> >  #define X86_FEATURE_RSB_VMEXIT_LITE	(11*32+17) /* "" Fill RSB on
> VM
> > exit when EIBRS is enabled */
> >  #define X86_FEATURE_CALL_DEPTH		(11*32+18) /* "" Call depth
> > tracking for RSB stuffing */
> >  #define X86_FEATURE_SMBA		(11*32+19) /* Slow Memory
> > Bandwidth Allocation */
> > +#define X86_FEATURE_BMEC		(11*32+20) /* AMD
> Bandwidth
> > Monitoring Event Configuration (BMEC) */
> >
> >  /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
> >  #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* AVX VNNI
> > instructions */
> > diff --git a/arch/x86/kernel/cpu/cpuid-deps.c
> > b/arch/x86/kernel/cpu/cpuid- deps.c index c881bcafba7d..4555f9596ccf
> > 100644
> > --- a/arch/x86/kernel/cpu/cpuid-deps.c
> > +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> > @@ -68,6 +68,7 @@ static const struct cpuid_dep cpuid_deps[] = {
> >  	{ X86_FEATURE_CQM_OCCUP_LLC,
> > 	X86_FEATURE_CQM_LLC   },
> >  	{ X86_FEATURE_CQM_MBM_TOTAL,
> > 	X86_FEATURE_CQM_LLC   },
> >  	{ X86_FEATURE_CQM_MBM_LOCAL,
> > 	X86_FEATURE_CQM_LLC   },
> > +	{ X86_FEATURE_BMEC,			X86_FEATURE_CQM_LLC   },
> 
> Shouldn't X86_FEATURE_BMEC really depend on
> X86_FEATURE_CQM_MBM_LOCAL and _TOTAL?
> 
> CQM_MBM_LOCAL and/or _TOTAL can be disabled but CQM_LLC can still be
> enabled. In this case, BMEC shouldn't be enabled, right? But with this patch,
> BMEC will be enabled but it won't work well as CQM_MBM_TOTAL/_LOCAL
> are not enabled.

Yes. You are right.
> 
> You may remove the above line and add these two lines:
> 
> +	{ X86_FEATURE_BMEC,
> 	X86_FEATURE_CQM_MBM_TOTAL   },
> +	{ X86_FEATURE_BMEC,
> 	X86_FEATURE_CQM_MBM_LOCAL   },
> 

Sure. Will add these lines.
Thanks
Babu

[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 21324 bytes --]