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)
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, 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".
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
Link: https://www.amd.com/en/support/tech-docs/amd64-technology-platform-quality-service-extensions
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/kernel/cpu/cpuid-deps.c | 2 ++
arch/x86/kernel/cpu/scattered.c | 1 +
3 files changed, 4 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 00045123f418..db5287c06b65 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -308,6 +308,7 @@
#define X86_FEATURE_CALL_DEPTH (11*32+19) /* "" Call depth tracking for RSB stuffing */
#define X86_FEATURE_MSR_TSX_CTRL (11*32+20) /* "" MSR IA32_TSX_CTRL (Intel) implemented */
#define X86_FEATURE_SMBA (11*32+21) /* Slow Memory Bandwidth Allocation */
+#define X86_FEATURE_BMEC (11*32+22) /* Bandwidth Monitoring Event Configuration */
/* 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 d95221117129..f6748c8bd647 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -68,6 +68,8 @@ 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_MBM_TOTAL },
+ { X86_FEATURE_BMEC, X86_FEATURE_CQM_MBM_LOCAL },
{ X86_FEATURE_AVX512_BF16, X86_FEATURE_AVX512VL },
{ X86_FEATURE_AVX512_FP16, X86_FEATURE_AVX512BW },
{ X86_FEATURE_ENQCMD, X86_FEATURE_XSAVES },
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index d925753084fb..0dad49a09b7a 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -46,6 +46,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_PROC_FEEDBACK, CPUID_EDX, 11, 0x80000007, 0 },
{ X86_FEATURE_MBA, CPUID_EBX, 6, 0x80000008, 0 },
{ X86_FEATURE_SMBA, CPUID_EBX, 2, 0x80000020, 0 },
+ { X86_FEATURE_BMEC, CPUID_EBX, 3, 0x80000020, 0 },
{ X86_FEATURE_PERFMON_V2, CPUID_EAX, 0, 0x80000022, 0 },
{ X86_FEATURE_AMD_LBR_V2, CPUID_EAX, 1, 0x80000022, 0 },
{ 0, 0, 0, 0, 0 }
--
2.34.1
On Mon, Jan 09, 2023 at 10:43:56AM -0600, Babu Moger wrote: > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h > index 00045123f418..db5287c06b65 100644 > --- a/arch/x86/include/asm/cpufeatures.h > +++ b/arch/x86/include/asm/cpufeatures.h > @@ -308,6 +308,7 @@ > #define X86_FEATURE_CALL_DEPTH (11*32+19) /* "" Call depth tracking for RSB stuffing */ > #define X86_FEATURE_MSR_TSX_CTRL (11*32+20) /* "" MSR IA32_TSX_CTRL (Intel) implemented */ > #define X86_FEATURE_SMBA (11*32+21) /* Slow Memory Bandwidth Allocation */ > +#define X86_FEATURE_BMEC (11*32+22) /* Bandwidth Monitoring Event Configuration */ If those flags are meant only for internal kernel code use - it looks like it - and userspace doesn't care, pls put "" in front of both in the comment like the others above them do. This way they won't be visible in /proc/cpuinfo. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Hi Boris, On 1/9/23 12:58, Borislav Petkov wrote: > On Mon, Jan 09, 2023 at 10:43:56AM -0600, Babu Moger wrote: >> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h >> index 00045123f418..db5287c06b65 100644 >> --- a/arch/x86/include/asm/cpufeatures.h >> +++ b/arch/x86/include/asm/cpufeatures.h >> @@ -308,6 +308,7 @@ >> #define X86_FEATURE_CALL_DEPTH (11*32+19) /* "" Call depth tracking for RSB stuffing */ >> #define X86_FEATURE_MSR_TSX_CTRL (11*32+20) /* "" MSR IA32_TSX_CTRL (Intel) implemented */ >> #define X86_FEATURE_SMBA (11*32+21) /* Slow Memory Bandwidth Allocation */ >> +#define X86_FEATURE_BMEC (11*32+22) /* Bandwidth Monitoring Event Configuration */ > If those flags are meant only for internal kernel code use - it looks like it - > and userspace doesn't care, pls put "" in front of both in the comment like the > others above them do. All the QoS(or RDT) features are visible so far. If we make them visible, users can easily figure out if this specific feature is supported or not. There is some benefit if we make it visible. What do you think? Thanks Babu
On Mon, Jan 09, 2023 at 01:49:09PM -0600, Moger, Babu wrote: > All the QoS(or RDT) features are visible so far. If we make them visible, > users can easily figure out if this specific feature is supported or not. What would be the actual, real-life use case where the presence of those flags in /proc/cpuinfo is really needed? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
>> All the QoS(or RDT) features are visible so far. If we make them visible, >> users can easily figure out if this specific feature is supported or not. > > What would be the actual, real-life use case where the presence of those flags > in /proc/cpuinfo is really needed? It feels like the old "rule" was "make it visible in /proc/cpuid" unless there was some good reason NOT to do it. But that has resulted in the "flags" line getting ridiculously long and hard for humans to read (141 fields with 926 bytes on my Skylake, more on more modern CPUs). For RDT I don't see a lot of value in knowing that a feature is present ... all of them have parameters on how many things they can control/monitor ... so you have to either go parse the CPUID leaves, or just mount /sys/fs/resctrl and look in the "info" directory to get the extra information you need to do anything with RDT. I don't know if we'd break anything if we dropped: cat_l3 cdp_l3 mba cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local from /proc/cpuinfo. Perhaps the "rule" should be written in Documentation/{somewhere}? -Tony
On 1/9/23 15:25, Luck, Tony wrote: >>> All the QoS(or RDT) features are visible so far. If we make them visible, >>> users can easily figure out if this specific feature is supported or not. >> What would be the actual, real-life use case where the presence of those flags >> in /proc/cpuinfo is really needed? > It feels like the old "rule" was "make it visible in /proc/cpuid" unless there was some > good reason NOT to do it. But that has resulted in the "flags" line getting ridiculously > long and hard for humans to read (141 fields with 926 bytes on my Skylake, more on > more modern CPUs). > > For RDT I don't see a lot of value in knowing that a feature is present ... all of them > have parameters on how many things they can control/monitor ... so you have to > either go parse the CPUID leaves, or just mount /sys/fs/resctrl and look in the "info" > directory to get the extra information you need to do anything with RDT. > > I don't know if we'd break anything if we dropped: > > cat_l3 cdp_l3 mba cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local > > from /proc/cpuinfo. > > Perhaps the "rule" should be written in Documentation/{somewhere}? Actually, these feature bits are referred in Documentation/x86/resctrl.rst This feature is enabled by the CONFIG_X86_CPU_RESCTRL and the x86 /proc/cpuinfo flag bits: =============================================== ================================ RDT (Resource Director Technology) Allocation "rdt_a" CAT (Cache Allocation Technology) "cat_l3", "cat_l2" CDP (Code and Data Prioritization) "cdp_l3", "cdp_l2" CQM (Cache QoS Monitoring) "cqm_llc", "cqm_occup_llc" MBM (Memory Bandwidth Monitoring) "cqm_mbm_total", "cqm_mbm_local" MBA (Memory Bandwidth Allocation) "mba" SMBA (Slow Memory Bandwidth Allocation) "smba" BMEC (Bandwidth Monitoring Event Configuration) "bmec" =============================================== ================================ So, if we remove them, we need to update here also. Thanks Babu
On Mon, Jan 09, 2023 at 03:44:30PM -0600, Moger, Babu wrote: > So, if we remove them, we need to update here also. We can "reroute" that documentation to /sys/fs/resctrl's info... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
[AMD Official Use Only - General] > -----Original Message----- > From: Borislav Petkov <bp@alien8.de> > Sent: Monday, January 9, 2023 3:52 PM > To: Moger, Babu <Babu.Moger@amd.com> > Cc: Luck, Tony <tony.luck@intel.com>; corbet@lwn.net; Chatre, Reinette > <reinette.chatre@intel.com>; tglx@linutronix.de; mingo@redhat.com; Yu, > Fenghua <fenghua.yu@intel.com>; 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>; > james.morse@arm.com; linux-doc@vger.kernel.org; linux- > kernel@vger.kernel.org; bagasdotme@gmail.com; Eranian, Stephane > <eranian@google.com>; christophe.leroy@csgroup.eu; jarkko@kernel.org; > Hunter, Adrian <adrian.hunter@intel.com>; quic_jiles@quicinc.com; > peternewman@google.com > Subject: Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring > Event Configuration feature flag > > On Mon, Jan 09, 2023 at 03:44:30PM -0600, Moger, Babu wrote: > > So, if we remove them, we need to update here also. > > We can "reroute" that documentation to /sys/fs/resctrl's info... Yes. We could. But at this point we don't have all the features listed in /sys/fs/resctrl/info directory. We need to add all the resctrl feature bits in info directory. How about we take this as separate task and I can send separate series to address it? Thanks Babu
On Mon, Jan 09, 2023 at 11:10:40PM +0000, Moger, Babu wrote: > Yes. We could. > > But at this point we don't have all the features listed in /sys/fs/resctrl/info > directory. We need to add all the resctrl feature bits in info directory. How > about we take this as separate task and I can send separate series to address > it? See my reply to Reinette from just now and lemme know if something's not clear yet. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
On 1/24/2023 5:34 AM, Borislav Petkov wrote: > On Mon, Jan 09, 2023 at 11:10:40PM +0000, Moger, Babu wrote: >> Yes. We could. >> >> But at this point we don't have all the features listed in /sys/fs/resctrl/info >> directory. We need to add all the resctrl feature bits in info directory. How >> about we take this as separate task and I can send separate series to address >> it? > See my reply to Reinette from just now and lemme know if something's not clear > yet. Understood. I am planning to add resctrl feature list inside /sys/fs/resctrl/info/ in my next series. Thanks Babu
On Tue, Jan 24, 2023 at 08:11:21AM -0600, Moger, Babu wrote: > Understood. I am planning to add resctrl feature list inside > /sys/fs/resctrl/info/ in my next series. Maybe I wasn't as clear as I hoped for: so you have a couple of flags in /proc/cpuinfo which are actively being used by tools. Why would you want to move the flags somewhere else and do the extra work for no apparent reason? > We need to add all the resctrl feature bits in info directory. What for? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
On 1/24/23 09:10, Borislav Petkov wrote: > On Tue, Jan 24, 2023 at 08:11:21AM -0600, Moger, Babu wrote: >> Understood. I am planning to add resctrl feature list inside >> /sys/fs/resctrl/info/ in my next series. > Maybe I wasn't as clear as I hoped for: > > so you have a couple of flags in /proc/cpuinfo which are actively being used by > tools. Those flags will be there. Not planning to remove them. > > Why would you want to move the flags somewhere else and do the extra work for no > apparent reason? With this series(v12) we have added two new cpuid features(SMBA and BMEC). But these features are not visible in /proc/cpuinfo. Planning to add them in /sys/fs/resctrl/info. So, users can see them here. > >> We need to add all the resctrl feature bits in info directory. > What for? Same reason as above. Thanks Babu
Hi Babu, On 1/24/2023 8:06 AM, Moger, Babu wrote: > > On 1/24/23 09:10, Borislav Petkov wrote: >> On Tue, Jan 24, 2023 at 08:11:21AM -0600, Moger, Babu wrote: >>> Understood. I am planning to add resctrl feature list inside >>> /sys/fs/resctrl/info/ in my next series. >> Maybe I wasn't as clear as I hoped for: >> >> so you have a couple of flags in /proc/cpuinfo which are actively being used by >> tools. > Those flags will be there. Not planning to remove them. >> >> Why would you want to move the flags somewhere else and do the extra work for no >> apparent reason? > > With this series(v12) we have added two new cpuid features(SMBA and BMEC). > > But these features are not visible in /proc/cpuinfo. Planning to add them > in /sys/fs/resctrl/info. > > So, users can see them here. Could you please elaborate what you are planning to do? Existence and support for SMBA and BMEC is already visible to user space in your current series: * On a system that supports SMBA with the needed kernel support users will find the /sys/fs/resctrl/info/SMBA directory with enumerated properties as well as SMBA within the schemata file. * On a system that supports BMEC with the needed kernel support users will find the relevant files listed within /sys/fs/resctrl/info/L3_MON/mon_features. Reinette
On 1/24/23 10:59, Reinette Chatre wrote: > Hi Babu, > > On 1/24/2023 8:06 AM, Moger, Babu wrote: >> On 1/24/23 09:10, Borislav Petkov wrote: >>> On Tue, Jan 24, 2023 at 08:11:21AM -0600, Moger, Babu wrote: >>>> Understood. I am planning to add resctrl feature list inside >>>> /sys/fs/resctrl/info/ in my next series. >>> Maybe I wasn't as clear as I hoped for: >>> >>> so you have a couple of flags in /proc/cpuinfo which are actively being used by >>> tools. >> Those flags will be there. Not planning to remove them. >>> Why would you want to move the flags somewhere else and do the extra work for no >>> apparent reason? >> With this series(v12) we have added two new cpuid features(SMBA and BMEC). >> >> But these features are not visible in /proc/cpuinfo. Planning to add them >> in /sys/fs/resctrl/info. >> >> So, users can see them here. > Could you please elaborate what you are planning to do? Yes. It is sort of available. But, I wanted to add them explicit using the already available function rdt_cpu_has(). Something like. #cat /sys/fs/resctrl/info/features cmt, mbmtotal, mbmlocal, l3cat, mba, smba, bmec Some of these features can be disabled using boot parameter options. So, this will show only the features which enabled. Thanks Babu > > Existence and support for SMBA and BMEC is already visible to user space > in your current series: > * On a system that supports SMBA with the needed kernel support users will > find the /sys/fs/resctrl/info/SMBA directory with enumerated properties > as well as SMBA within the schemata file. > * On a system that supports BMEC with the needed kernel support users will > find the relevant files listed within /sys/fs/resctrl/info/L3_MON/mon_features. > > Reinette -- Thanks Babu Moger
Hi Babu, On 1/24/2023 9:30 AM, Moger, Babu wrote: > > On 1/24/23 10:59, Reinette Chatre wrote: >> On 1/24/2023 8:06 AM, Moger, Babu wrote: >>> On 1/24/23 09:10, Borislav Petkov wrote: >>>> On Tue, Jan 24, 2023 at 08:11:21AM -0600, Moger, Babu wrote: >>>>> Understood. I am planning to add resctrl feature list inside >>>>> /sys/fs/resctrl/info/ in my next series. >>>> Maybe I wasn't as clear as I hoped for: >>>> >>>> so you have a couple of flags in /proc/cpuinfo which are actively being used by >>>> tools. >>> Those flags will be there. Not planning to remove them. >>>> Why would you want to move the flags somewhere else and do the extra work for no >>>> apparent reason? >>> With this series(v12) we have added two new cpuid features(SMBA and BMEC). >>> >>> But these features are not visible in /proc/cpuinfo. Planning to add them >>> in /sys/fs/resctrl/info. >>> >>> So, users can see them here. >> Could you please elaborate what you are planning to do? > > Yes. It is sort of available. But, I wanted to add them explicit using the > already available function rdt_cpu_has(). > > Something like. > > #cat /sys/fs/resctrl/info/features > > cmt, mbmtotal, mbmlocal, l3cat, mba, smba, bmec > > > Some of these features can be disabled using boot parameter options. So, > this will show only the features which enabled. > From what I understand the only feature that needs additional help is CDP and it appears in /proc/cpuinfo. For all other features /sys/fs/resctrl/info already provides information when they are enabled, no? Reinette
On 1/24/23 11:47, Reinette Chatre wrote: > Hi Babu, > > On 1/24/2023 9:30 AM, Moger, Babu wrote: >> On 1/24/23 10:59, Reinette Chatre wrote: >>> On 1/24/2023 8:06 AM, Moger, Babu wrote: >>>> On 1/24/23 09:10, Borislav Petkov wrote: >>>>> On Tue, Jan 24, 2023 at 08:11:21AM -0600, Moger, Babu wrote: >>>>>> Understood. I am planning to add resctrl feature list inside >>>>>> /sys/fs/resctrl/info/ in my next series. >>>>> Maybe I wasn't as clear as I hoped for: >>>>> >>>>> so you have a couple of flags in /proc/cpuinfo which are actively being used by >>>>> tools. >>>> Those flags will be there. Not planning to remove them. >>>>> Why would you want to move the flags somewhere else and do the extra work for no >>>>> apparent reason? >>>> With this series(v12) we have added two new cpuid features(SMBA and BMEC). >>>> >>>> But these features are not visible in /proc/cpuinfo. Planning to add them >>>> in /sys/fs/resctrl/info. >>>> >>>> So, users can see them here. >>> Could you please elaborate what you are planning to do? >> Yes. It is sort of available. But, I wanted to add them explicit using the >> already available function rdt_cpu_has(). >> >> Something like. >> >> #cat /sys/fs/resctrl/info/features >> >> cmt, mbmtotal, mbmlocal, l3cat, mba, smba, bmec >> >> >> Some of these features can be disabled using boot parameter options. So, >> this will show only the features which enabled. >> > From what I understand the only feature that needs additional help is CDP > and it appears in /proc/cpuinfo. For all other features > /sys/fs/resctrl/info already provides information when they are enabled, no? Yes. It is available. But, the feature BMEC is not explicitly available. I was thinking making all of them explicit. But we can live with that for now. We can think about this in the future. Thanks for the comments. Thanks Babu > Reinette > -- Thanks Babu Moger
Hi Babu, On 1/24/2023 11:03 AM, Moger, Babu wrote: > > Yes. It is available. But, the feature BMEC is not explicitly available. I think your addition [1] to the resctrl documentation explains well how user space can determine which parts of BMEC are available: If the system supports Bandwidth Monitoring Event Configuration (BMEC), then the bandwidth events will be configurable. The output will be:: # cat /sys/fs/resctrl/info/L3_MON/mon_features llc_occupancy mbm_total_bytes mbm_total_bytes_config mbm_local_bytes mbm_local_bytes_config Reinette [1] https://lore.kernel.org/lkml/20230113152039.770054-14-babu.moger@amd.com/
Hi Reinette, On 1/24/23 13:23, Reinette Chatre wrote: > Hi Babu, > > On 1/24/2023 11:03 AM, Moger, Babu wrote: >> Yes. It is available. But, the feature BMEC is not explicitly available. > I think your addition [1] to the resctrl documentation explains > well how user space can determine which parts of BMEC are available: > > If the system supports Bandwidth Monitoring Event > Configuration (BMEC), then the bandwidth events will > be configurable. The output will be:: > > # cat /sys/fs/resctrl/info/L3_MON/mon_features > llc_occupancy > mbm_total_bytes > mbm_total_bytes_config > mbm_local_bytes > mbm_local_bytes_config > Yes. Sure. That works. Thank you. Babu
On Tue, Jan 24, 2023 at 01:03:32PM -0600, Moger, Babu wrote: > Yes. It is available. But, the feature BMEC is not explicitly available. > I was thinking making all of them explicit. But we can live with that for > now. We can think about this in the future. Yes, you can always think about a solution when the requirement presents itself. What I find the wrong approach is thinking that someone *might* need it and then devising some solution. Wait for the actual use case first and then think of a proper solution. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
On Mon, Jan 09, 2023 at 09:25:32PM +0000, Luck, Tony wrote: > It feels like the old "rule" was "make it visible in /proc/cpuid" unless there was some > good reason NOT to do it. But that has resulted in the "flags" line getting ridiculously > long and hard for humans to read (141 fields with 926 bytes on my Skylake, > more on more modern CPUs). Yap, imagine every possible CPUID bit were in there... > I don't know if we'd break anything if we dropped: > > cat_l3 cdp_l3 mba cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local > > from /proc/cpuinfo. I wouldn't mind if we remove them from cpuinfo, frankly. > Perhaps the "rule" should be written in Documentation/{somewhere}? This started documenting it: Documentation/x86/cpuinfo.rst -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Hi Boris and Tony, On 1/9/2023 1:39 PM, Borislav Petkov wrote: > On Mon, Jan 09, 2023 at 09:25:32PM +0000, Luck, Tony wrote: >> It feels like the old "rule" was "make it visible in /proc/cpuid" unless there was some >> good reason NOT to do it. But that has resulted in the "flags" line getting ridiculously >> long and hard for humans to read (141 fields with 926 bytes on my Skylake, >> more on more modern CPUs). > > Yap, imagine every possible CPUID bit were in there... > >> I don't know if we'd break anything if we dropped: >> >> cat_l3 cdp_l3 mba cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local >> >> from /proc/cpuinfo. > > I wouldn't mind if we remove them from cpuinfo, frankly. I am afraid that I am not aware of all the resctrl user space apps and tools being used. I did take a quick look at intel-cmt-cat that has an active user base and from what I can tell it uses /proc/cpuinfo to learn some capabilities: Example of looking for "cqm" (although "cqm" is not in Tony's list): https://github.com/intel/intel-cmt-cat/blob/master/lib/os_cap.c#L420 Example of looking for "cdp_l3": https://github.com/intel/intel-cmt-cat/blob/master/lib/os_cap.c#L520 Example of looking for "cdp_l2": https://github.com/intel/intel-cmt-cat/blob/master/lib/os_cap.c#L564 > >> Perhaps the "rule" should be written in Documentation/{somewhere}? > > This started documenting it: > > Documentation/x86/cpuinfo.rst > We could make a rule that no more resctrl related features are added to cpuinfo but I am hesitant to remove the ones that are already there. Reinette
On Mon, Jan 09, 2023 at 03:43:11PM -0800, Reinette Chatre wrote: > We could make a rule that no more resctrl related features are added to > cpuinfo but I am hesitant to remove the ones that are already there. Yes, that makes sense. And note that we try for /proc/cpuinfo to contain flags where the kernel has received (substantial) enablement work to support a feature. Shadow stack would be one good example. If resctrl needs to use a feature and it cannot use that feature without kernel enablement, then yes, by all means, it should use /proc/cpuinfo and not the corresponding CPUID bit. Because the presence of the flag in /proc/cpuinfo says "yes, you can use the feature and the kernel you're running on has the required support." Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
> This started documenting it: > > Documentation/x86/cpuinfo.rst That does a good job to explain HOW everything works. But doesn't really cover whether a field should be made visible. The section on "" just says to use it: "if it does not make sense for the feature to be exposed to userspace" But that allows for the flimsiest of reasons to used to justify making a flag visible. -Tony
On Mon, Jan 09, 2023 at 09:50:20PM +0000, Luck, Tony wrote: > But that allows for the flimsiest of reasons to used to justify making a > flag visible. How's that for starters? c: The naming override can be "", which means it will not appear in /proc/cpuinfo. ---------------------------------------------------------------------------------- The feature shall be omitted from /proc/cpuinfo if there is no valid use case for userspace to query this flag and cannot rely on other means for detecting feature support. For example, toolchains do use CPUID directly instead of relying on the kernel providing that info. If unsure, that flag can always be omitted initially and, once a valid use case presents itself, be shown later. Not the other way around. Another example is X86_FEATURE_ALWAYS, defined in cpufeatures.h. That flag is an internal kernel feature used in the alternative runtime patching functionality. So, its name is overridden with "". Its flag will not appear in /proc/cpuinfo because it absolutely does not make any sense to appear there. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
© 2016 - 2025 Red Hat, Inc.