All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features
@ 2023-01-09 16:43 Babu Moger
  2023-01-09 16:43 ` [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask() Babu Moger
                   ` (12 more replies)
  0 siblings, 13 replies; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:43 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

New AMD processors can now support following QoS features.

1. Slow Memory Bandwidth Allocation (SMBA)
   With this feature, the QOS enforcement policies can be applied
   to the external slow memory connected to the host. QOS enforcement
   is accomplished by assigning a Class Of Service (COS) to a processor
   and specifying allocations or limits for that COS for each resource
   to be allocated.

   Currently, CXL.memory is the only supported "slow" memory device. With
   the support of SMBA feature the hardware enables bandwidth allocation
   on the slow memory devices.

2. Bandwidth Monitoring Event Configuration (BMEC)
   The bandwidth monitoring events mbm_total_event and mbm_local_event 
   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 are memory events. With the feature
   BMEC, the users have the option to configure mbm_total_event and
   mbm_local_event to count the specific type of events.

   Following are the bitmaps 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

This series adds support for these features. Also added a minor cleanup(PATCH 1).

Feature description is available in the specification, "AMD64 Technology Platform Quality of Service Extensions, Revision: 1.03 Publication # 56375
Revision: 1.03 Issue Date: February 2022".

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
---
v11:
 Summary of changes:
 Removed cpus_read_lock() from the patch 11 and 12 based on our discussion.
 https://lore.kernel.org/lkml/3dc31a6d-5485-746d-3c49-df7dcd1827e3@intel.com/
 Picked up all the Reviewed-by by Reinette.
 Minor text changes on patch 13.
 
v10:
 https://lore.kernel.org/lkml/20221222233127.910538-1-babu.moger@amd.com/
 Summary of changes:
 1. Moved the patch 12 to 1 ("x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask()")
 2. No need to write MSR_IA32_EVT_CFG_BASE on all the CPUs. Replaced on_each_cpu_mask with smp_call_function_any.
 3. Updated the subject line of the patches to replace sysfs with resctrl.
 4. Added Reviewed-by for the patches which are reviewed by Reinette.
 5. Added few more comments suggested by Reinette.
  
v9:
 https://lore.kernel.org/lkml/166990882621.17806.16780480657453071426.stgit@bmoger-ubuntu/
 Summary of changes:
 1. Rebased on top of lastest tip/master as of 11/30.
 2. Most of the changes are result of the comments from Fenghua, Reinette and Peter Newman.
 3. Fixed the cpuid dependancy.
 4. Added the __init attribute to rdt_get_mon_l3_config and mbm_config_rftype_init.
 5. Added new function resctrl_arch_reset_rmid_all to clear all rmid statues.
 6. Changed mon_event_config_index_get based on Reinette's comments.
 7. Changed mbm_config_rftype_init to take care of few extra error handling.
 8. Few other minor changes and text changes.

v8:
 https://lore.kernel.org/lkml/166759188265.3281208.11769277079826754455.stgit@bmoger-ubuntu/
 Changes:
 1. Removed init attribute for rdt_cpu_has to make it available for all the files.
 2. Updated the change log for mon_features to correct the names of config files.
 3. Changed configuration file name from mbm_total_config to mbm_total_bytes_config.
    This is more consistant with other changes.
 4. Added lock protection while reading/writing the config file.
 5. Other few minor text changes. I have been missing few comments in last couple of
    revisions. Hope I have addressed all of them this time.

v7:
 https://lore.kernel.org/lkml/166604543832.5345.9696970469830919982.stgit@bmoger-ubuntu/
 Changes:
 Not much of a change. Missed one comment from Reinette from v5. Corrected it now.
 Few format corrections from Sanjaya.

v6:
 https://lore.kernel.org/lkml/166543345606.23830.3120625408601531368.stgit@bmoger-ubuntu/
 Summary of changes:
 1. Rebased on top of lastest tip tree. Fixed few minor conflicts.
 2. Fixed format issue with scattered.c.
 3. Removed config_name from the structure mon_evt. It is not required.
 4. The read/write format for mbm_total_config and mbm_local_config will be same
    as schemata format "id0=val0;id1=val1;...". This is comment from Fenghua.
 5. Added more comments MSR_IA32_EVT_CFG_BASE writng.
 5. Few text changes in resctrl.rst 
 
v5:
  https://lore.kernel.org/lkml/166431016617.373387.1968875281081252467.stgit@bmoger-ubuntu/
  Summary of changes.
  1. Split the series into two. The first two patches are bug fixes. So, sent them separate.
  2. The config files mbm_total_config and mbm_local_config are now under
     /sys/fs/resctrl/info/L3_MON/. Removed these config files from mon groups.
  3. Ran "checkpatch --strict --codespell" on all the patches. Looks good with few known exceptions.
  4. Few minor text changes in resctrl.rst file. 

v4:
  https://lore.kernel.org/lkml/166257348081.1043018.11227924488792315932.stgit@bmoger-ubuntu/
  Got numerios of comments from Reinette Chatre. Addressed most of them. 
  Summary of changes.
  1. Removed mon_configurable under /sys/fs/resctrl/info/L3_MON/.  
  2. Updated mon_features texts if the BMEC is supported.
  3. Added more explanation about the slow memory support.
  4. Replaced smp_call_function_many with on_each_cpu_mask call.
  5. Removed arch_has_empty_bitmaps
  6. Few other text changes.
  7. Removed Reviewed-by if the patch is modified.
  8. Rebased the patches to latest tip.

v3:
  https://lore.kernel.org/lkml/166117559756.6695.16047463526634290701.stgit@bmoger-ubuntu/
  a. Rebased the patches to latest tip. Resolved some conflicts.
     https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
  b. Taken care of feedback from Bagas Sanjaya.
  c. Added Reviewed by from Mingo.
  Note: I am still looking for comments from Reinette or Fenghua.

v2:
  https://lore.kernel.org/lkml/165938717220.724959.10931629283087443782.stgit@bmoger-ubuntu/
  a. Rebased the patches to latest stable tree (v5.18.15). Resolved some conflicts.
  b. Added the patch to fix CBM issue on AMD. This was originally discussed
     https://lore.kernel.org/lkml/20220517001234.3137157-1-eranian@google.com/

v1:
  https://lore.kernel.org/lkml/165757543252.416408.13547339307237713464.stgit@bmoger-ubuntu/


Babu Moger (13):
  x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask()
  x86/cpufeatures: Add Slow Memory Bandwidth Allocation feature flag
  x86/resctrl: Add a new resource type RDT_RESOURCE_SMBA
  x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature
    flag
  x86/resctrl: Include new features in command line options
  x86/resctrl: Detect and configure Slow Memory Bandwidth Allocation
  x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()
  x86/resctrl: Support monitor configuration
  x86/resctrl: Add interface to read mbm_total_bytes_config
  x86/resctrl: Add interface to read mbm_local_bytes_config
  x86/resctrl: Add interface to write mbm_total_bytes_config
  x86/resctrl: Add interface to write mbm_local_bytes_config
  Documentation/x86: Update resctrl.rst for new features

 .../admin-guide/kernel-parameters.txt         |   2 +-
 Documentation/x86/resctrl.rst                 | 142 +++++++-
 arch/x86/include/asm/cpufeatures.h            |   2 +
 arch/x86/include/asm/msr-index.h              |   2 +
 arch/x86/kernel/cpu/cpuid-deps.c              |   2 +
 arch/x86/kernel/cpu/resctrl/core.c            |  54 ++-
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c     |  13 +-
 arch/x86/kernel/cpu/resctrl/internal.h        |  28 ++
 arch/x86/kernel/cpu/resctrl/monitor.c         |  30 +-
 arch/x86/kernel/cpu/resctrl/rdtgroup.c        | 307 ++++++++++++++++--
 arch/x86/kernel/cpu/scattered.c               |   2 +
 include/linux/resctrl.h                       |  11 +
 12 files changed, 554 insertions(+), 41 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask()
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
@ 2023-01-09 16:43 ` Babu Moger
  2023-01-09 23:26   ` Ashok Raj
  2023-01-09 16:43 ` [PATCH v11 02/13] x86/cpufeatures: Add Slow Memory Bandwidth Allocation feature flag Babu Moger
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:43 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

on_each_cpu_mask() runs the function on each CPU specified by cpumask,
which may include the local processor.

Replace smp_call_function_many() with on_each_cpu_mask() to simplify
the code.

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 11 +++------
 arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 29 +++++++----------------
 2 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 1df0e3262bca..7eece3d2d0c3 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -310,7 +310,6 @@ int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
 	enum resctrl_conf_type t;
 	cpumask_var_t cpu_mask;
 	struct rdt_domain *d;
-	int cpu;
 	u32 idx;
 
 	if (!zalloc_cpumask_var(&cpu_mask, GFP_KERNEL))
@@ -341,13 +340,9 @@ int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
 
 	if (cpumask_empty(cpu_mask))
 		goto done;
-	cpu = get_cpu();
-	/* Update resource control msr on this CPU if it's in cpu_mask. */
-	if (cpumask_test_cpu(cpu, cpu_mask))
-		rdt_ctrl_update(&msr_param);
-	/* Update resource control msr on other CPUs. */
-	smp_call_function_many(cpu_mask, rdt_ctrl_update, &msr_param, 1);
-	put_cpu();
+
+	/* Update resource control msr on all the CPUs. */
+	on_each_cpu_mask(cpu_mask, rdt_ctrl_update, &msr_param, 1);
 
 done:
 	free_cpumask_var(cpu_mask);
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index e5a48f05e787..e4e6cdc1ee62 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -325,12 +325,7 @@ static void update_cpu_closid_rmid(void *info)
 static void
 update_closid_rmid(const struct cpumask *cpu_mask, struct rdtgroup *r)
 {
-	int cpu = get_cpu();
-
-	if (cpumask_test_cpu(cpu, cpu_mask))
-		update_cpu_closid_rmid(r);
-	smp_call_function_many(cpu_mask, update_cpu_closid_rmid, r, 1);
-	put_cpu();
+	on_each_cpu_mask(cpu_mask, update_cpu_closid_rmid, r, 1);
 }
 
 static int cpus_mon_write(struct rdtgroup *rdtgrp, cpumask_var_t newmask,
@@ -1864,13 +1859,9 @@ static int set_cache_qos_cfg(int level, bool enable)
 			/* Pick one CPU from each domain instance to update MSR */
 			cpumask_set_cpu(cpumask_any(&d->cpu_mask), cpu_mask);
 	}
-	cpu = get_cpu();
-	/* Update QOS_CFG MSR on this cpu if it's in cpu_mask. */
-	if (cpumask_test_cpu(cpu, cpu_mask))
-		update(&enable);
-	/* Update QOS_CFG MSR on all other cpus in cpu_mask. */
-	smp_call_function_many(cpu_mask, update, &enable, 1);
-	put_cpu();
+
+	/* Update QOS_CFG MSR on all the CPUs in cpu_mask */
+	on_each_cpu_mask(cpu_mask, update, &enable, 1);
 
 	free_cpumask_var(cpu_mask);
 
@@ -2347,7 +2338,7 @@ static int reset_all_ctrls(struct rdt_resource *r)
 	struct msr_param msr_param;
 	cpumask_var_t cpu_mask;
 	struct rdt_domain *d;
-	int i, cpu;
+	int i;
 
 	if (!zalloc_cpumask_var(&cpu_mask, GFP_KERNEL))
 		return -ENOMEM;
@@ -2368,13 +2359,9 @@ static int reset_all_ctrls(struct rdt_resource *r)
 		for (i = 0; i < hw_res->num_closid; i++)
 			hw_dom->ctrl_val[i] = r->default_ctrl;
 	}
-	cpu = get_cpu();
-	/* Update CBM on this cpu if it's in cpu_mask. */
-	if (cpumask_test_cpu(cpu, cpu_mask))
-		rdt_ctrl_update(&msr_param);
-	/* Update CBM on all other cpus in cpu_mask. */
-	smp_call_function_many(cpu_mask, rdt_ctrl_update, &msr_param, 1);
-	put_cpu();
+
+	/* Update CBM on all the CPUs in cpu_mask */
+	on_each_cpu_mask(cpu_mask, rdt_ctrl_update, &msr_param, 1);
 
 	free_cpumask_var(cpu_mask);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 02/13] x86/cpufeatures: Add Slow Memory Bandwidth Allocation feature flag
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
  2023-01-09 16:43 ` [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask() Babu Moger
@ 2023-01-09 16:43 ` Babu Moger
  2023-01-09 16:43 ` [PATCH v11 03/13] x86/resctrl: Add a new resource type RDT_RESOURCE_SMBA Babu Moger
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:43 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

Add the new AMD feature X86_FEATURE_SMBA. With this feature, the QOS
enforcement policies can be applied to external slow memory connected
to the host. QOS enforcement is accomplished by assigning a Class Of
Service (COS) to a processor and specifying allocations or limits for
that COS for each resource to be allocated.

This feature is identified by the CPUID Function 8000_0020_EBX_x0.

CPUID Fn8000_0020_EBX_x0 AMD Bandwidth Enforcement Feature Identifiers
(ECX=0)

Bits    Field Name      Description
2       L3SBE           L3 external slow memory bandwidth enforcement

CXL.memory is the only supported "slow" memory device. With the support
of SMBA feature, the hardware enables bandwidth allocation on the slow
memory devices. If there are multiple slow memory devices in the system,
then the throttling logic groups all the slow sources together and
applies the limit on them as a whole.

The presence of the SMBA feature(with CXL.memory) is independent of
whether slow memory device is actually present in the system. If there
is no slow memory in the system, then setting a SMBA limit will have no
impact on the performance of the system.

Presence of CXL memory can be identified by numactl command.

$numactl -H
available: 2 nodes (0-1)
node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16
node 0 size: 63678 MB node 0 free: 59542 MB
node 1 cpus:
node 1 size: 16122 MB
node 1 free: 15627 MB
node distances:
node   0   1
   0:  10  50
   1:  50  10

CPU list for CXL memory will be empty. The cpu-cxl node distance is
greater than cpu-to-cpu distances. Node 1 has the CXL memory in this
case. CXL memory can also be identified using ACPI SRAT table and
memory maps.

Feature description is available in the specification, "AMD64
Technology Platform Quality of Service Extensions, Revision: 1.03
Publication # 56375 Revision: 1.03 Issue Date: February 2022".

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/scattered.c    | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 61012476d66e..00045123f418 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -307,6 +307,7 @@
 #define X86_FEATURE_SGX_EDECCSSA	(11*32+18) /* "" SGX EDECCSSA user leaf function */
 #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 */
 
 /* 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/scattered.c b/arch/x86/kernel/cpu/scattered.c
index f53944fb8f7f..d925753084fb 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -45,6 +45,7 @@ static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_CPB,		CPUID_EDX,  9, 0x80000007, 0 },
 	{ 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_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


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 03/13] x86/resctrl: Add a new resource type RDT_RESOURCE_SMBA
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
  2023-01-09 16:43 ` [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask() Babu Moger
  2023-01-09 16:43 ` [PATCH v11 02/13] x86/cpufeatures: Add Slow Memory Bandwidth Allocation feature flag Babu Moger
@ 2023-01-09 16:43 ` Babu Moger
  2023-01-09 16:43 ` [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag Babu Moger
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:43 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

Add a new resource type RDT_RESOURCE_SMBA to handle the QoS enforcement
policies on the external slow memory.

Mostly initialization of the essentials. Setting fflags to RFTYPE_RES_MB
configures the SMBA resource to have the same resctrl files as the
existing MBA resource. The SMBA resource has identical properties to
the existing MBA resource. These properties will be enumerated in an
upcoming change and exposed via resctrl because of this flag.

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/core.c     | 12 ++++++++++++
 arch/x86/kernel/cpu/resctrl/internal.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index c98e52ff5f20..f6af3ac1ef20 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -100,6 +100,18 @@ struct rdt_hw_resource rdt_resources_all[] = {
 			.fflags			= RFTYPE_RES_MB,
 		},
 	},
+	[RDT_RESOURCE_SMBA] =
+	{
+		.r_resctrl = {
+			.rid			= RDT_RESOURCE_SMBA,
+			.name			= "SMBA",
+			.cache_level		= 3,
+			.domains		= domain_init(RDT_RESOURCE_SMBA),
+			.parse_ctrlval		= parse_bw,
+			.format_str		= "%d=%*u",
+			.fflags			= RFTYPE_RES_MB,
+		},
+	},
 };
 
 /*
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 5ebd28e6aa0c..fdbbf66312ec 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -409,6 +409,7 @@ enum resctrl_res_level {
 	RDT_RESOURCE_L3,
 	RDT_RESOURCE_L2,
 	RDT_RESOURCE_MBA,
+	RDT_RESOURCE_SMBA,
 
 	/* Must be the last */
 	RDT_NUM_RESOURCES,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
                   ` (2 preceding siblings ...)
  2023-01-09 16:43 ` [PATCH v11 03/13] x86/resctrl: Add a new resource type RDT_RESOURCE_SMBA Babu Moger
@ 2023-01-09 16:43 ` Babu Moger
  2023-01-09 18:58   ` Borislav Petkov
  2023-01-09 16:43 ` [PATCH v11 05/13] x86/resctrl: Include new features in command line options Babu Moger
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:43 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

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


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 05/13] x86/resctrl: Include new features in command line options
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
                   ` (3 preceding siblings ...)
  2023-01-09 16:43 ` [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag Babu Moger
@ 2023-01-09 16:43 ` Babu Moger
  2023-01-09 16:43 ` [PATCH v11 06/13] x86/resctrl: Detect and configure Slow Memory Bandwidth Allocation Babu Moger
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:43 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

Add the command line options to enable or disable the new resctrl features.
smba : Slow Memory Bandwidth Allocation
bmec : Bandwidth Monitor Event Configuration.

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 2 +-
 arch/x86/kernel/cpu/resctrl/core.c              | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 6cfa6e3996cf..0ee891133d76 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5221,7 +5221,7 @@
 	rdt=		[HW,X86,RDT]
 			Turn on/off individual RDT features. List is:
 			cmt, mbmtotal, mbmlocal, l3cat, l3cdp, l2cat, l2cdp,
-			mba.
+			mba, smba, bmec.
 			E.g. to turn on cmt and turn off mba use:
 				rdt=cmt,!mba
 
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index f6af3ac1ef20..10a8c9d96f32 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -659,6 +659,8 @@ enum {
 	RDT_FLAG_L2_CAT,
 	RDT_FLAG_L2_CDP,
 	RDT_FLAG_MBA,
+	RDT_FLAG_SMBA,
+	RDT_FLAG_BMEC,
 };
 
 #define RDT_OPT(idx, n, f)	\
@@ -682,6 +684,8 @@ static struct rdt_options rdt_options[]  __initdata = {
 	RDT_OPT(RDT_FLAG_L2_CAT,    "l2cat",	X86_FEATURE_CAT_L2),
 	RDT_OPT(RDT_FLAG_L2_CDP,    "l2cdp",	X86_FEATURE_CDP_L2),
 	RDT_OPT(RDT_FLAG_MBA,	    "mba",	X86_FEATURE_MBA),
+	RDT_OPT(RDT_FLAG_SMBA,	    "smba",	X86_FEATURE_SMBA),
+	RDT_OPT(RDT_FLAG_BMEC,	    "bmec",	X86_FEATURE_BMEC),
 };
 #define NUM_RDT_OPTIONS ARRAY_SIZE(rdt_options)
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 06/13] x86/resctrl: Detect and configure Slow Memory Bandwidth Allocation
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
                   ` (4 preceding siblings ...)
  2023-01-09 16:43 ` [PATCH v11 05/13] x86/resctrl: Include new features in command line options Babu Moger
@ 2023-01-09 16:43 ` Babu Moger
  2023-01-09 16:43 ` [PATCH v11 07/13] x86/resctrl: Add __init attribute to rdt_get_mon_l3_config() Babu Moger
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:43 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

The QoS slow memory configuration details are available via
CPUID_Fn80000020_EDX_x02. Detect the available details and
initialize the rest to defaults.

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/msr-index.h          |  1 +
 arch/x86/kernel/cpu/resctrl/core.c        | 36 +++++++++++++++++++++--
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  2 +-
 arch/x86/kernel/cpu/resctrl/rdtgroup.c    |  8 +++--
 4 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 37ff47552bcb..e0a40027aa62 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -1061,6 +1061,7 @@
 
 /* - AMD: */
 #define MSR_IA32_MBA_BW_BASE		0xc0000200
+#define MSR_IA32_SMBA_BW_BASE		0xc0000280
 
 /* MSR_IA32_VMX_MISC bits */
 #define MSR_IA32_VMX_MISC_INTEL_PT                 (1ULL << 14)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 10a8c9d96f32..b4fc851f6489 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r)
 	if (!r)
 		return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc;
 
+	/*
+	 * The software controller support is only applicable to MBA resource.
+	 * Make sure to check for resource type.
+	 */
+	if (r->rid != RDT_RESOURCE_MBA)
+		return false;
+
 	return r->membw.mba_sc;
 }
 
@@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r)
 	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
 	union cpuid_0x10_3_eax eax;
 	union cpuid_0x10_x_edx edx;
-	u32 ebx, ecx;
+	u32 ebx, ecx, subleaf;
 
-	cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full);
+	/*
+	 * Query CPUID_Fn80000020_EDX_x01 for MBA and
+	 * CPUID_Fn80000020_EDX_x02 for SMBA
+	 */
+	subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 :  1;
+
+	cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full);
 	hw_res->num_closid = edx.split.cos_max + 1;
 	r->default_ctrl = MAX_MBA_BW_AMD;
 
@@ -750,6 +763,19 @@ static __init bool get_mem_config(void)
 	return false;
 }
 
+static __init bool get_slow_mem_config(void)
+{
+	struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA];
+
+	if (!rdt_cpu_has(X86_FEATURE_SMBA))
+		return false;
+
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+		return __rdt_get_mem_config_amd(&hw_res->r_resctrl);
+
+	return false;
+}
+
 static __init bool get_rdt_alloc_resources(void)
 {
 	struct rdt_resource *r;
@@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void)
 	if (get_mem_config())
 		ret = true;
 
+	if (get_slow_mem_config())
+		ret = true;
+
 	return ret;
 }
 
@@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void)
 		} else if (r->rid == RDT_RESOURCE_MBA) {
 			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
 			hw_res->msr_update = mba_wrmsr_amd;
+		} else if (r->rid == RDT_RESOURCE_SMBA) {
+			hw_res->msr_base = MSR_IA32_SMBA_BW_BASE;
+			hw_res->msr_update = mba_wrmsr_amd;
 		}
 	}
 }
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 7eece3d2d0c3..eb07d4435391 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -209,7 +209,7 @@ static int parse_line(char *line, struct resctrl_schema *s,
 	unsigned long dom_id;
 
 	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP &&
-	    r->rid == RDT_RESOURCE_MBA) {
+	    (r->rid == RDT_RESOURCE_MBA || r->rid == RDT_RESOURCE_SMBA)) {
 		rdt_last_cmd_puts("Cannot pseudo-lock MBA resource\n");
 		return -EINVAL;
 	}
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index e4e6cdc1ee62..aa469d708991 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1208,7 +1208,7 @@ static bool rdtgroup_mode_test_exclusive(struct rdtgroup *rdtgrp)
 
 	list_for_each_entry(s, &resctrl_schema_all, list) {
 		r = s->res;
-		if (r->rid == RDT_RESOURCE_MBA)
+		if (r->rid == RDT_RESOURCE_MBA || r->rid == RDT_RESOURCE_SMBA)
 			continue;
 		has_cache = true;
 		list_for_each_entry(d, &r->domains, list) {
@@ -1397,7 +1397,8 @@ static int rdtgroup_size_show(struct kernfs_open_file *of,
 					ctrl = resctrl_arch_get_config(r, d,
 								       closid,
 								       type);
-				if (r->rid == RDT_RESOURCE_MBA)
+				if (r->rid == RDT_RESOURCE_MBA ||
+				    r->rid == RDT_RESOURCE_SMBA)
 					size = ctrl;
 				else
 					size = rdtgroup_cbm_to_size(r, d, ctrl);
@@ -2832,7 +2833,8 @@ static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp)
 
 	list_for_each_entry(s, &resctrl_schema_all, list) {
 		r = s->res;
-		if (r->rid == RDT_RESOURCE_MBA) {
+		if (r->rid == RDT_RESOURCE_MBA ||
+		    r->rid == RDT_RESOURCE_SMBA) {
 			rdtgroup_init_mba(r, rdtgrp->closid);
 			if (is_mba_sc(r))
 				continue;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 07/13] x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
                   ` (5 preceding siblings ...)
  2023-01-09 16:43 ` [PATCH v11 06/13] x86/resctrl: Detect and configure Slow Memory Bandwidth Allocation Babu Moger
@ 2023-01-09 16:43 ` Babu Moger
  2023-01-09 16:44 ` [PATCH v11 08/13] x86/resctrl: Support monitor configuration Babu Moger
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:43 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

In an upcoming change rdt_get_mon_l3_config() needs to call
rdt_cpu_has() to query the monitor related features. It cannot be
called right now because rdt_cpu_has() has the __init attribute but
rdt_get_mon_l3_config() doesn't.

Add the __init attribute to rdt_get_mon_l3_config() that is only called
by get_rdt_mon_resources() that already has the __init attribute. Also
make rdt_cpu_has() available to by rdt_get_mon_l3_config() via
the internal header file.

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/core.c     | 2 +-
 arch/x86/kernel/cpu/resctrl/internal.h | 1 +
 arch/x86/kernel/cpu/resctrl/monitor.c  | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index b4fc851f6489..030d3b409768 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -728,7 +728,7 @@ static int __init set_rdt_options(char *str)
 }
 __setup("rdt", set_rdt_options);
 
-static bool __init rdt_cpu_has(int flag)
+bool __init rdt_cpu_has(int flag)
 {
 	bool ret = boot_cpu_has(flag);
 	struct rdt_options *o;
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index fdbbf66312ec..f16b8bc5448c 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -512,6 +512,7 @@ void closid_free(int closid);
 int alloc_rmid(void);
 void free_rmid(u32 rmid);
 int rdt_get_mon_l3_config(struct rdt_resource *r);
+bool __init rdt_cpu_has(int flag);
 void mon_event_count(void *info);
 int rdtgroup_mondata_show(struct seq_file *m, void *arg);
 void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index efe0c30d3a12..e33e8d8bd796 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -746,7 +746,7 @@ static void l3_mon_evt_init(struct rdt_resource *r)
 		list_add_tail(&mbm_local_event.list, &r->evt_list);
 }
 
-int rdt_get_mon_l3_config(struct rdt_resource *r)
+int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 {
 	unsigned int mbm_offset = boot_cpu_data.x86_cache_mbm_width_offset;
 	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 08/13] x86/resctrl: Support monitor configuration
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
                   ` (6 preceding siblings ...)
  2023-01-09 16:43 ` [PATCH v11 07/13] x86/resctrl: Add __init attribute to rdt_get_mon_l3_config() Babu Moger
@ 2023-01-09 16:44 ` Babu Moger
  2023-01-09 16:44 ` [PATCH v11 09/13] x86/resctrl: Add interface to read mbm_total_bytes_config Babu Moger
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:44 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

Add a new field in struct mon_evt to support Bandwidth Monitoring Event
Configuration(BMEC) and also update the "mon_features" display.

The resctrl file "mon_features" will display the supported events
and files that can be used to configure those events if monitor
configuration is supported.

Before the change.
        $cat /sys/fs/resctrl/info/L3_MON/mon_features
        llc_occupancy
        mbm_total_bytes
        mbm_local_bytes

After the change when BMEC is supported.
        $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

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/internal.h | 2 ++
 arch/x86/kernel/cpu/resctrl/monitor.c  | 7 +++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 5 ++++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index f16b8bc5448c..0605b04f1b7a 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -52,11 +52,13 @@ DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
  * struct mon_evt - Entry in the event list of a resource
  * @evtid:		event id
  * @name:		name of the event
+ * @configurable:	true if the event is configurable
  * @list:		entry in &rdt_resource->evt_list
  */
 struct mon_evt {
 	enum resctrl_event_id	evtid;
 	char			*name;
+	bool			configurable;
 	struct list_head	list;
 };
 
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index e33e8d8bd796..b39e0eca1879 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -783,6 +783,13 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 	if (ret)
 		return ret;
 
+	if (rdt_cpu_has(X86_FEATURE_BMEC)) {
+		if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL))
+			mbm_total_event.configurable = true;
+		if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))
+			mbm_local_event.configurable = true;
+	}
+
 	l3_mon_evt_init(r);
 
 	r->mon_capable = true;
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index aa469d708991..f34c70c7a791 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -996,8 +996,11 @@ static int rdt_mon_features_show(struct kernfs_open_file *of,
 	struct rdt_resource *r = of->kn->parent->priv;
 	struct mon_evt *mevt;
 
-	list_for_each_entry(mevt, &r->evt_list, list)
+	list_for_each_entry(mevt, &r->evt_list, list) {
 		seq_printf(seq, "%s\n", mevt->name);
+		if (mevt->configurable)
+			seq_printf(seq, "%s_config\n", mevt->name);
+	}
 
 	return 0;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 09/13] x86/resctrl: Add interface to read mbm_total_bytes_config
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
                   ` (7 preceding siblings ...)
  2023-01-09 16:44 ` [PATCH v11 08/13] x86/resctrl: Support monitor configuration Babu Moger
@ 2023-01-09 16:44 ` Babu Moger
  2023-01-09 16:44 ` [PATCH v11 10/13] x86/resctrl: Add interface to read mbm_local_bytes_config Babu Moger
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:44 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

The event configuration can be viewed by the user by reading the
configuration file /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config.
The event configuration settings are domain specific and will affect all
the CPUs in the domain.

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_config is set to 0x7f to count all the
event types.

For example:
    $cat /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config
    0=0x7f;1=0x7f;2=0x7f;3=0x7f

    In this case, the event mbm_total_bytes is configured with 0x7f on
    domains 0 to 3.

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/include/asm/msr-index.h       |   1 +
 arch/x86/kernel/cpu/resctrl/internal.h |  24 ++++++
 arch/x86/kernel/cpu/resctrl/monitor.c  |   4 +-
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 102 +++++++++++++++++++++++++
 4 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index e0a40027aa62..2e5f57c93605 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -1062,6 +1062,7 @@
 /* - AMD: */
 #define MSR_IA32_MBA_BW_BASE		0xc0000200
 #define MSR_IA32_SMBA_BW_BASE		0xc0000280
+#define MSR_IA32_EVT_CFG_BASE		0xc0000400
 
 /* MSR_IA32_VMX_MISC bits */
 #define MSR_IA32_VMX_MISC_INTEL_PT                 (1ULL << 14)
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 0605b04f1b7a..8edecc5763d8 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -30,6 +30,29 @@
  */
 #define MBM_CNTR_WIDTH_OFFSET_MAX (62 - MBM_CNTR_WIDTH_BASE)
 
+/* Reads to Local DRAM Memory */
+#define READS_TO_LOCAL_MEM		BIT(0)
+
+/* Reads to Remote DRAM Memory */
+#define READS_TO_REMOTE_MEM		BIT(1)
+
+/* Non-Temporal Writes to Local Memory */
+#define NON_TEMP_WRITE_TO_LOCAL_MEM	BIT(2)
+
+/* Non-Temporal Writes to Remote Memory */
+#define NON_TEMP_WRITE_TO_REMOTE_MEM	BIT(3)
+
+/* Reads to Local Memory the system identifies as "Slow Memory" */
+#define READS_TO_LOCAL_S_MEM		BIT(4)
+
+/* Reads to Remote Memory the system identifies as "Slow Memory" */
+#define READS_TO_REMOTE_S_MEM		BIT(5)
+
+/* Dirty Victims to All Types of Memory */
+#define DIRTY_VICTIMS_TO_ALL_MEM	BIT(6)
+
+/* Max event bits supported */
+#define MAX_EVT_CONFIG_BITS		GENMASK(6, 0)
 
 struct rdt_fs_context {
 	struct kernfs_fs_context	kfc;
@@ -531,5 +554,6 @@ bool has_busy_rmid(struct rdt_resource *r, struct rdt_domain *d);
 void __check_limbo(struct rdt_domain *d, bool force_free);
 void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
 void __init thread_throttle_mode_init(void);
+void __init mbm_config_rftype_init(const char *config);
 
 #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index b39e0eca1879..2afddebc8636 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -784,8 +784,10 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 		return ret;
 
 	if (rdt_cpu_has(X86_FEATURE_BMEC)) {
-		if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL))
+		if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {
 			mbm_total_event.configurable = true;
+			mbm_config_rftype_init("mbm_total_bytes_config");
+		}
 		if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))
 			mbm_local_event.configurable = true;
 	}
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index f34c70c7a791..d47c675a856f 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1418,6 +1418,93 @@ static int rdtgroup_size_show(struct kernfs_open_file *of,
 	return ret;
 }
 
+struct mon_config_info {
+	u32 evtid;
+	u32 mon_config;
+};
+
+#define INVALID_CONFIG_INDEX   UINT_MAX
+
+/**
+ * mon_event_config_index_get - get the hardware index for the
+ *                              configurable event
+ * @evtid: event id.
+ *
+ * Return: 0 for evtid == QOS_L3_MBM_TOTAL_EVENT_ID
+ *         1 for evtid == QOS_L3_MBM_LOCAL_EVENT_ID
+ *         INVALID_CONFIG_INDEX for invalid evtid
+ */
+static inline unsigned int mon_event_config_index_get(u32 evtid)
+{
+	switch (evtid) {
+	case QOS_L3_MBM_TOTAL_EVENT_ID:
+		return 0;
+	case QOS_L3_MBM_LOCAL_EVENT_ID:
+		return 1;
+	default:
+		/* Should never reach here */
+		return INVALID_CONFIG_INDEX;
+	}
+}
+
+static void mon_event_config_read(void *info)
+{
+	struct mon_config_info *mon_info = info;
+	unsigned int index;
+	u32 h;
+
+	index = mon_event_config_index_get(mon_info->evtid);
+	if (index == INVALID_CONFIG_INDEX) {
+		pr_warn_once("Invalid event id %d\n", mon_info->evtid);
+		return;
+	}
+	rdmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, h);
+
+	/* Report only the valid event configuration bits */
+	mon_info->mon_config &= MAX_EVT_CONFIG_BITS;
+}
+
+static void mondata_config_read(struct rdt_domain *d, struct mon_config_info *mon_info)
+{
+	smp_call_function_any(&d->cpu_mask, mon_event_config_read, mon_info, 1);
+}
+
+static int mbm_config_show(struct seq_file *s, struct rdt_resource *r, u32 evtid)
+{
+	struct mon_config_info mon_info = {0};
+	struct rdt_domain *dom;
+	bool sep = false;
+
+	mutex_lock(&rdtgroup_mutex);
+
+	list_for_each_entry(dom, &r->domains, list) {
+		if (sep)
+			seq_puts(s, ";");
+
+		memset(&mon_info, 0, sizeof(struct mon_config_info));
+		mon_info.evtid = evtid;
+		mondata_config_read(dom, &mon_info);
+
+		seq_printf(s, "%d=0x%02x", dom->id, mon_info.mon_config);
+		sep = true;
+	}
+	seq_puts(s, "\n");
+
+	mutex_unlock(&rdtgroup_mutex);
+
+	return 0;
+}
+
+static int mbm_total_bytes_config_show(struct kernfs_open_file *of,
+				       struct seq_file *seq, void *v)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+
+	mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID);
+
+	return 0;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
@@ -1516,6 +1603,12 @@ static struct rftype res_common_files[] = {
 		.seq_show	= max_threshold_occ_show,
 		.fflags		= RF_MON_INFO | RFTYPE_RES_CACHE,
 	},
+	{
+		.name		= "mbm_total_bytes_config",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= mbm_total_bytes_config_show,
+	},
 	{
 		.name		= "cpus",
 		.mode		= 0644,
@@ -1622,6 +1715,15 @@ void __init thread_throttle_mode_init(void)
 	rft->fflags = RF_CTRL_INFO | RFTYPE_RES_MB;
 }
 
+void __init mbm_config_rftype_init(const char *config)
+{
+	struct rftype *rft;
+
+	rft = rdtgroup_get_rftype_by_name(config);
+	if (rft)
+		rft->fflags = RF_MON_INFO | RFTYPE_RES_CACHE;
+}
+
 /**
  * rdtgroup_kn_mode_restrict - Restrict user access to named resctrl file
  * @r: The resource group with which the file is associated.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 10/13] x86/resctrl: Add interface to read mbm_local_bytes_config
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
                   ` (8 preceding siblings ...)
  2023-01-09 16:44 ` [PATCH v11 09/13] x86/resctrl: Add interface to read mbm_total_bytes_config Babu Moger
@ 2023-01-09 16:44 ` Babu Moger
  2023-01-09 16:44 ` [PATCH v11 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config Babu Moger
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:44 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

The event configuration can be viewed by the user by reading the
configuration file /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config.
The event configuration settings are domain specific and will affect
all the CPUs in the domain.

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_local_bytes_config is set to 0x15 to count all the
local event types.

For example:
    $cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
    0=0x15;1=0x15;2=0x15;3=0x15

    In this case, the event mbm_local_bytes is configured with 0x15 on
    domains 0 to 3.

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/monitor.c  |  4 +++-
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 ++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index 2afddebc8636..7c8a3a745041 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -788,8 +788,10 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 			mbm_total_event.configurable = true;
 			mbm_config_rftype_init("mbm_total_bytes_config");
 		}
-		if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))
+		if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL)) {
 			mbm_local_event.configurable = true;
+			mbm_config_rftype_init("mbm_local_bytes_config");
+		}
 	}
 
 	l3_mon_evt_init(r);
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index d47c675a856f..43e2a866e8e0 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1505,6 +1505,16 @@ static int mbm_total_bytes_config_show(struct kernfs_open_file *of,
 	return 0;
 }
 
+static int mbm_local_bytes_config_show(struct kernfs_open_file *of,
+				       struct seq_file *seq, void *v)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+
+	mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID);
+
+	return 0;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
@@ -1609,6 +1619,12 @@ static struct rftype res_common_files[] = {
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= mbm_total_bytes_config_show,
 	},
+	{
+		.name		= "mbm_local_bytes_config",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= mbm_local_bytes_config_show,
+	},
 	{
 		.name		= "cpus",
 		.mode		= 0644,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
                   ` (9 preceding siblings ...)
  2023-01-09 16:44 ` [PATCH v11 10/13] x86/resctrl: Add interface to read mbm_local_bytes_config Babu Moger
@ 2023-01-09 16:44 ` Babu Moger
  2023-01-11 22:04   ` Reinette Chatre
  2023-01-09 16:44 ` [PATCH v11 12/13] x86/resctrl: Add interface to write mbm_local_bytes_config Babu Moger
  2023-01-09 16:44 ` [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features Babu Moger
  12 siblings, 1 reply; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:44 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

The event configuration for mbm_total_bytes can be changed by the user by
writing to the file /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config.

The event configuration settings are domain specific and affect all the
CPUs in the domain.

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
====  ===========================================================

For example:
To change the mbm_total_bytes to count only reads on domain 0, the bits
0, 1, 4 and 5 needs to be set, which is 110011b (in hex 0x33). Run the
command.
        $echo  0=0x33 > /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config

To change the mbm_total_bytes to count all the slow memory reads on
domain 1, the bits 4 and 5 needs to be set which is 110000b (in hex 0x30).
Run the command.
        $echo  1=0x30 > /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/monitor.c  |  17 ++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 124 ++++++++++++++++++++++++-
 include/linux/resctrl.h                |  11 +++
 3 files changed, 151 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index 7c8a3a745041..c50c43656bdb 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -176,6 +176,23 @@ void resctrl_arch_reset_rmid(struct rdt_resource *r, struct rdt_domain *d,
 		memset(am, 0, sizeof(*am));
 }
 
+/*
+ * Assumes that hardware counters are also reset and thus that there is
+ * no need to record initial non-zero counts.
+ */
+void resctrl_arch_reset_rmid_all(struct rdt_resource *r, struct rdt_domain *d)
+{
+	struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d);
+
+	if (is_mbm_total_enabled())
+		memset(hw_dom->arch_mbm_total, 0,
+		       sizeof(*hw_dom->arch_mbm_total) * r->num_rmid);
+
+	if (is_mbm_local_enabled())
+		memset(hw_dom->arch_mbm_local, 0,
+		       sizeof(*hw_dom->arch_mbm_local) * r->num_rmid);
+}
+
 static u64 mbm_overflow_count(u64 prev_msr, u64 cur_msr, unsigned int width)
 {
 	u64 shift = 64 - width, chunks;
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 43e2a866e8e0..21fdb4a48a11 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1515,6 +1515,127 @@ static int mbm_local_bytes_config_show(struct kernfs_open_file *of,
 	return 0;
 }
 
+static void mon_event_config_write(void *info)
+{
+	struct mon_config_info *mon_info = info;
+	unsigned int index;
+
+	index = mon_event_config_index_get(mon_info->evtid);
+	if (index == INVALID_CONFIG_INDEX) {
+		pr_warn_once("Invalid event id %d\n", mon_info->evtid);
+		return;
+	}
+	wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
+}
+
+static int mbm_config_write_domain(struct rdt_resource *r,
+				   struct rdt_domain *d, u32 evtid, u32 val)
+{
+	struct mon_config_info mon_info = {0};
+	int ret = 0;
+
+	/* mon_config cannot be more than the supported set of events */
+	if (val > MAX_EVT_CONFIG_BITS) {
+		rdt_last_cmd_puts("Invalid event configuration\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Read the current config value first. If both are the same then
+	 * no need to write it again.
+	 */
+	mon_info.evtid = evtid;
+	mondata_config_read(d, &mon_info);
+	if (mon_info.mon_config == val)
+		goto out;
+
+	mon_info.mon_config = val;
+
+	/*
+	 * Update MSR_IA32_EVT_CFG_BASE MSR on one of the CPUs in the
+	 * domain. The MSRs offset from MSR MSR_IA32_EVT_CFG_BASE
+	 * are scoped at the domain level. Writing any of these MSRs
+	 * on one CPU is observed by all the CPUs in the domain.
+	 */
+	smp_call_function_any(&d->cpu_mask, mon_event_config_write,
+			      &mon_info, 1);
+
+	/*
+	 * When an Event Configuration is changed, the bandwidth counters
+	 * for all RMIDs and Events will be cleared by the hardware. The
+	 * hardware also sets MSR_IA32_QM_CTR.Unavailable (bit 62) for
+	 * every RMID on the next read to any event for every RMID.
+	 * Subsequent reads will have MSR_IA32_QM_CTR.Unavailable (bit 62)
+	 * cleared while it is tracked by the hardware. Clear the
+	 * mbm_local and mbm_total counts for all the RMIDs.
+	 */
+	resctrl_arch_reset_rmid_all(r, d);
+
+out:
+	return ret;
+}
+
+static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
+{
+	char *dom_str = NULL, *id_str;
+	unsigned long dom_id, val;
+	struct rdt_domain *d;
+	int ret = 0;
+
+next:
+	if (!tok || tok[0] == '\0')
+		return 0;
+
+	/* Start processing the strings for each domain */
+	dom_str = strim(strsep(&tok, ";"));
+	id_str = strsep(&dom_str, "=");
+
+	if (!id_str || kstrtoul(id_str, 10, &dom_id)) {
+		rdt_last_cmd_puts("Missing '=' or non-numeric domain id\n");
+		return -EINVAL;
+	}
+
+	if (!dom_str || kstrtoul(dom_str, 16, &val)) {
+		rdt_last_cmd_puts("Non-numeric event configuration value\n");
+		return -EINVAL;
+	}
+
+	list_for_each_entry(d, &r->domains, list) {
+		if (d->id == dom_id) {
+			ret = mbm_config_write_domain(r, d, evtid, val);
+			if (ret)
+				return -EINVAL;
+			goto next;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t mbm_total_bytes_config_write(struct kernfs_open_file *of,
+					    char *buf, size_t nbytes,
+					    loff_t off)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+	int ret;
+
+	/* Valid input requires a trailing newline */
+	if (nbytes == 0 || buf[nbytes - 1] != '\n')
+		return -EINVAL;
+
+	mutex_lock(&rdtgroup_mutex);
+
+	rdt_last_cmd_clear();
+
+	buf[nbytes - 1] = '\0';
+
+	ret = mon_config_write(r, buf, QOS_L3_MBM_TOTAL_EVENT_ID);
+
+	mutex_unlock(&rdtgroup_mutex);
+
+	return ret ?: nbytes;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
@@ -1615,9 +1736,10 @@ static struct rftype res_common_files[] = {
 	},
 	{
 		.name		= "mbm_total_bytes_config",
-		.mode		= 0444,
+		.mode		= 0644,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= mbm_total_bytes_config_show,
+		.write		= mbm_total_bytes_config_write,
 	},
 	{
 		.name		= "mbm_local_bytes_config",
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 0cee154abc9f..8334eeacfec5 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -250,6 +250,17 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain *d,
 void resctrl_arch_reset_rmid(struct rdt_resource *r, struct rdt_domain *d,
 			     u32 rmid, enum resctrl_event_id eventid);
 
+/**
+ * resctrl_arch_reset_rmid_all() - Reset all private state associated with
+ *				   all rmids and eventids.
+ * @r:		The resctrl resource.
+ * @d:		The domain for which all architectural counter state will
+ *		be cleared.
+ *
+ * This can be called from any CPU.
+ */
+void resctrl_arch_reset_rmid_all(struct rdt_resource *r, struct rdt_domain *d);
+
 extern unsigned int resctrl_rmid_realloc_threshold;
 extern unsigned int resctrl_rmid_realloc_limit;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 12/13] x86/resctrl: Add interface to write mbm_local_bytes_config
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
                   ` (10 preceding siblings ...)
  2023-01-09 16:44 ` [PATCH v11 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config Babu Moger
@ 2023-01-09 16:44 ` Babu Moger
  2023-01-09 16:44 ` [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features Babu Moger
  12 siblings, 0 replies; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:44 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

The event configuration for mbm_local_bytes can be changed by the
user by writing to the configuration file
/sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config.

The event configuration settings are domain specific and will affect all
the CPUs in the domain.

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
====  ===========================================================

For example:
To change the mbm_local_bytes_config to count all the non-temporal writes
on domain 0, the bits 2 and 3 needs to be set which is 1100b (in hex 0xc).
Run the command.
    $echo  0=0xc > /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config

To change the mbm_local_bytes to count only reads to local NUMA domain 1,
the bit 0 needs to be set which 1b (in hex 0x1). Run the command.
    $echo  1=0x1 > /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 27 +++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 21fdb4a48a11..9fb8e32f4d00 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1636,6 +1636,30 @@ static ssize_t mbm_total_bytes_config_write(struct kernfs_open_file *of,
 	return ret ?: nbytes;
 }
 
+static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of,
+					    char *buf, size_t nbytes,
+					    loff_t off)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+	int ret;
+
+	/* Valid input requires a trailing newline */
+	if (nbytes == 0 || buf[nbytes - 1] != '\n')
+		return -EINVAL;
+
+	mutex_lock(&rdtgroup_mutex);
+
+	rdt_last_cmd_clear();
+
+	buf[nbytes - 1] = '\0';
+
+	ret = mon_config_write(r, buf, QOS_L3_MBM_LOCAL_EVENT_ID);
+
+	mutex_unlock(&rdtgroup_mutex);
+
+	return ret ?: nbytes;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
@@ -1743,9 +1767,10 @@ static struct rftype res_common_files[] = {
 	},
 	{
 		.name		= "mbm_local_bytes_config",
-		.mode		= 0444,
+		.mode		= 0644,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= mbm_local_bytes_config_show,
+		.write		= mbm_local_bytes_config_write,
 	},
 	{
 		.name		= "cpus",
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features
  2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
                   ` (11 preceding siblings ...)
  2023-01-09 16:44 ` [PATCH v11 12/13] x86/resctrl: Add interface to write mbm_local_bytes_config Babu Moger
@ 2023-01-09 16:44 ` Babu Moger
  2023-01-11 22:06   ` Reinette Chatre
  12 siblings, 1 reply; 51+ messages in thread
From: Babu Moger @ 2023-01-09 16:44 UTC (permalink / raw)
  To: corbet, reinette.chatre, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	babu.moger, chang.seok.bae, pawan.kumar.gupta, jmattson,
	daniel.sneddon, sandipan.das, tony.luck, james.morse, linux-doc,
	linux-kernel, bagasdotme, eranian, christophe.leroy, jarkko,
	adrian.hunter, quic_jiles, peternewman

Update the documentation for the new features:
1. Slow Memory Bandwidth allocation (SMBA).
   With this feature, the QOS  enforcement policies can be applied
   to the external slow memory connected to the host. QOS enforcement
   is accomplished by assigning a Class Of Service (COS) to a processor
   and specifying allocations or limits for that COS for each resource
   to be allocated.

2. 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.

Also add configuration instructions with examples.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 Documentation/x86/resctrl.rst | 142 +++++++++++++++++++++++++++++++++-
 1 file changed, 140 insertions(+), 2 deletions(-)

diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
index 71a531061e4e..2860856f4463 100644
--- a/Documentation/x86/resctrl.rst
+++ b/Documentation/x86/resctrl.rst
@@ -17,14 +17,16 @@ AMD refers to this feature as AMD Platform Quality of Service(AMD QoS).
 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"
+===============================================	================================
 
 To use the feature mount the file system::
 
@@ -161,6 +163,83 @@ with the following files:
 "mon_features":
 		Lists the monitoring events if
 		monitoring is enabled for the resource.
+		Example::
+
+			# cat /sys/fs/resctrl/info/L3_MON/mon_features
+			llc_occupancy
+			mbm_total_bytes
+			mbm_local_bytes
+
+		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
+
+"mbm_total_bytes_config", "mbm_local_bytes_config":
+	Read/write files containing the configuration for the mbm_total_bytes
+	and mbm_local_bytes events, respectively, when the Bandwidth
+	Monitoring Event Configuration (BMEC) feature is supported.
+	The event configuration settings are domain specific and affect
+	all the CPUs in the domain. When either event configuration is
+	changed, the bandwidth counters for all RMIDs of both events
+	(mbm_total_bytes as well as mbm_local_bytes) are cleared for that
+	domain. The next read for every RMID will report "Unavailable"
+	and subsequent reads will report the valid value.
+
+	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.
+
+	Examples:
+
+	* To view the current configuration::
+	  ::
+
+	    # cat /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config
+	    0=0x7f;1=0x7f;2=0x7f;3=0x7f
+
+	    # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
+	    0=0x15;1=0x15;3=0x15;4=0x15
+
+	* To change the mbm_total_bytes to count only reads on domain 0,
+	  the bits 0, 1, 4 and 5 needs to be set, which is 110011b in binary
+	  (in hexadecimal 0x33):
+	  ::
+
+	    # echo  "0=0x33" > /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config
+
+	    # cat /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config
+	    0=0x33;1=0x7f;2=0x7f;3=0x7f
+
+	* To change the mbm_local_bytes to count all the slow memory reads on
+	  domain 0 and 1, the bits 4 and 5 needs to be set, which is 110000b
+	  in binary (in hexadecimal 0x30):
+	  ::
+
+	    # echo  "0=0x30;1=0x30" > /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
+
+	    # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config
+	    0=0x30;1=0x30;3=0x15;4=0x15
 
 "max_threshold_occupancy":
 		Read/write file provides the largest value (in
@@ -464,6 +543,25 @@ Memory bandwidth domain is L3 cache.
 
 	MB:<cache_id0>=bw_MBps0;<cache_id1>=bw_MBps1;...
 
+Slow Memory Bandwidth Allocation (SMBA)
+---------------------------------------
+AMD hardware supports Slow Memory Bandwidth Allocation (SMBA).
+CXL.memory is the only supported "slow" memory device. With the
+support of SMBA, the hardware enables bandwidth allocation on
+the slow memory devices. If there are multiple such devices in
+the system, the throttling logic groups all the slow sources
+together and applies the limit on them as a whole.
+
+The presence of SMBA (with CXL.memory) is independent of slow memory
+devices presence. If there are no such devices on the system, then
+configuring SMBA will have no impact on the performance of the system.
+
+The bandwidth domain for slow memory is L3 cache. Its schemata file
+is formatted as:
+::
+
+	SMBA:<cache_id0>=bandwidth0;<cache_id1>=bandwidth1;...
+
 Reading/writing the schemata file
 ---------------------------------
 Reading the schemata file will show the state of all resources
@@ -479,6 +577,46 @@ which you wish to change.  E.g.
   L3DATA:0=fffff;1=fffff;2=3c0;3=fffff
   L3CODE:0=fffff;1=fffff;2=fffff;3=fffff
 
+Reading/writing the schemata file (on AMD systems)
+--------------------------------------------------
+Reading the schemata file will show the current bandwidth limit on all
+domains. The allocated resources are in multiples of one eighth GB/s.
+When writing to the file, you need to specify what cache id you wish to
+configure the bandwidth limit.
+
+For example, to allocate 2GB/s limit on the first cache id:
+
+::
+
+  # cat schemata
+    MB:0=2048;1=2048;2=2048;3=2048
+    L3:0=ffff;1=ffff;2=ffff;3=ffff
+
+  # echo "MB:1=16" > schemata
+  # cat schemata
+    MB:0=2048;1=  16;2=2048;3=2048
+    L3:0=ffff;1=ffff;2=ffff;3=ffff
+
+Reading/writing the schemata file (on AMD systems) with SMBA feature
+--------------------------------------------------------------------
+Reading and writing the schemata file is the same as without SMBA in
+above section.
+
+For example, to allocate 8GB/s limit on the first cache id:
+
+::
+
+  # cat schemata
+    SMBA:0=2048;1=2048;2=2048;3=2048
+      MB:0=2048;1=2048;2=2048;3=2048
+      L3:0=ffff;1=ffff;2=ffff;3=ffff
+
+  # echo "SMBA:1=64" > schemata
+  # cat schemata
+    SMBA:0=2048;1=  64;2=2048;3=2048
+      MB:0=2048;1=2048;2=2048;3=2048
+      L3:0=ffff;1=ffff;2=ffff;3=ffff
+
 Cache Pseudo-Locking
 ====================
 CAT enables a user to specify the amount of cache space that an
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 16:43 ` [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag Babu Moger
@ 2023-01-09 18:58   ` Borislav Petkov
  2023-01-09 19:49     ` Moger, Babu
  0 siblings, 1 reply; 51+ messages in thread
From: Borislav Petkov @ 2023-01-09 18:58 UTC (permalink / raw)
  To: Babu Moger
  Cc: corbet, reinette.chatre, tglx, mingo, fenghua.yu, dave.hansen,
	x86, hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, chang.seok.bae,
	pawan.kumar.gupta, jmattson, daniel.sneddon, sandipan.das,
	tony.luck, james.morse, linux-doc, linux-kernel, bagasdotme,
	eranian, christophe.leroy, jarkko, adrian.hunter, quic_jiles,
	peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 18:58   ` Borislav Petkov
@ 2023-01-09 19:49     ` Moger, Babu
  2023-01-09 21:07       ` Borislav Petkov
  0 siblings, 1 reply; 51+ messages in thread
From: Moger, Babu @ 2023-01-09 19:49 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: corbet, reinette.chatre, tglx, mingo, fenghua.yu, dave.hansen,
	x86, hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, chang.seok.bae,
	pawan.kumar.gupta, jmattson, daniel.sneddon, sandipan.das,
	tony.luck, james.morse, linux-doc, linux-kernel, bagasdotme,
	eranian, christophe.leroy, jarkko, adrian.hunter, quic_jiles,
	peternewman

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


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 19:49     ` Moger, Babu
@ 2023-01-09 21:07       ` Borislav Petkov
  2023-01-09 21:25         ` Luck, Tony
  0 siblings, 1 reply; 51+ messages in thread
From: Borislav Petkov @ 2023-01-09 21:07 UTC (permalink / raw)
  To: Moger, Babu
  Cc: corbet, reinette.chatre, tglx, mingo, fenghua.yu, dave.hansen,
	x86, hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, chang.seok.bae,
	pawan.kumar.gupta, jmattson, daniel.sneddon, sandipan.das,
	tony.luck, james.morse, linux-doc, linux-kernel, bagasdotme,
	eranian, christophe.leroy, jarkko, adrian.hunter, quic_jiles,
	peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 21:07       ` Borislav Petkov
@ 2023-01-09 21:25         ` Luck, Tony
  2023-01-09 21:39           ` Borislav Petkov
  2023-01-09 21:44           ` Moger, Babu
  0 siblings, 2 replies; 51+ messages in thread
From: Luck, Tony @ 2023-01-09 21:25 UTC (permalink / raw)
  To: Borislav Petkov, Moger, Babu
  Cc: corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua, dave.hansen,
	x86, hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, Bae, Chang Seok,
	pawan.kumar.gupta, jmattson, daniel.sneddon, sandipan.das,
	james.morse, linux-doc, linux-kernel, bagasdotme, Eranian,
	Stephane, christophe.leroy, jarkko, Hunter, Adrian, quic_jiles,
	peternewman

>> 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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 21:25         ` Luck, Tony
@ 2023-01-09 21:39           ` Borislav Petkov
  2023-01-09 21:50             ` Luck, Tony
  2023-01-09 23:43             ` Reinette Chatre
  2023-01-09 21:44           ` Moger, Babu
  1 sibling, 2 replies; 51+ messages in thread
From: Borislav Petkov @ 2023-01-09 21:39 UTC (permalink / raw)
  To: Luck, Tony
  Cc: Moger, Babu, corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon,
	sandipan.das, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 21:25         ` Luck, Tony
  2023-01-09 21:39           ` Borislav Petkov
@ 2023-01-09 21:44           ` Moger, Babu
  2023-01-09 21:51             ` Borislav Petkov
  1 sibling, 1 reply; 51+ messages in thread
From: Moger, Babu @ 2023-01-09 21:44 UTC (permalink / raw)
  To: Luck, Tony, Borislav Petkov
  Cc: corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua, dave.hansen,
	x86, hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, Bae, Chang Seok,
	pawan.kumar.gupta, jmattson, daniel.sneddon, sandipan.das,
	james.morse, linux-doc, linux-kernel, bagasdotme, Eranian,
	Stephane, christophe.leroy, jarkko, Hunter, Adrian, quic_jiles,
	peternewman


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



^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 21:39           ` Borislav Petkov
@ 2023-01-09 21:50             ` Luck, Tony
  2023-01-24 11:28               ` Borislav Petkov
  2023-01-09 23:43             ` Reinette Chatre
  1 sibling, 1 reply; 51+ messages in thread
From: Luck, Tony @ 2023-01-09 21:50 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Moger, Babu, corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon,
	sandipan.das, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman

> 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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 21:44           ` Moger, Babu
@ 2023-01-09 21:51             ` Borislav Petkov
  2023-01-09 23:10               ` Moger, Babu
  0 siblings, 1 reply; 51+ messages in thread
From: Borislav Petkov @ 2023-01-09 21:51 UTC (permalink / raw)
  To: Moger, Babu
  Cc: Luck, Tony, corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon,
	sandipan.das, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 21:51             ` Borislav Petkov
@ 2023-01-09 23:10               ` Moger, Babu
  2023-01-24 11:34                 ` Borislav Petkov
  0 siblings, 1 reply; 51+ messages in thread
From: Moger, Babu @ 2023-01-09 23:10 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Luck, Tony, corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon, Das1,
	Sandipan, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman

[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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask()
  2023-01-09 16:43 ` [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask() Babu Moger
@ 2023-01-09 23:26   ` Ashok Raj
  2023-01-10  2:23     ` Moger, Babu
  0 siblings, 1 reply; 51+ messages in thread
From: Ashok Raj @ 2023-01-09 23:26 UTC (permalink / raw)
  To: Babu Moger
  Cc: corbet, reinette.chatre, tglx, mingo, bp, fenghua.yu,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	chang.seok.bae, pawan.kumar.gupta, jmattson, daniel.sneddon,
	sandipan.das, tony.luck, james.morse, linux-doc, linux-kernel,
	bagasdotme, eranian, christophe.leroy, jarkko, adrian.hunter,
	quic_jiles, peternewman, Ashok Raj

On Mon, Jan 09, 2023 at 10:43:53AM -0600, Babu Moger wrote:
> on_each_cpu_mask() runs the function on each CPU specified by cpumask,
> which may include the local processor.
> 
> Replace smp_call_function_many() with on_each_cpu_mask() to simplify
> the code.
> 
> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 11 +++------
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 29 +++++++----------------
>  2 files changed, 11 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index 1df0e3262bca..7eece3d2d0c3 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -310,7 +310,6 @@ int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
>  	enum resctrl_conf_type t;
>  	cpumask_var_t cpu_mask;
>  	struct rdt_domain *d;
> -	int cpu;
>  	u32 idx;
>  
>  	if (!zalloc_cpumask_var(&cpu_mask, GFP_KERNEL))
> @@ -341,13 +340,9 @@ int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
>  
>  	if (cpumask_empty(cpu_mask))
>  		goto done;
> -	cpu = get_cpu();
> -	/* Update resource control msr on this CPU if it's in cpu_mask. */
> -	if (cpumask_test_cpu(cpu, cpu_mask))
> -		rdt_ctrl_update(&msr_param);
> -	/* Update resource control msr on other CPUs. */
> -	smp_call_function_many(cpu_mask, rdt_ctrl_update, &msr_param, 1);
> -	put_cpu();
> +
> +	/* Update resource control msr on all the CPUs. */
> +	on_each_cpu_mask(cpu_mask, rdt_ctrl_update, &msr_param, 1);

Do you require these updates to done immediately via an IPI? or can they be
done bit lazy via schedule_on_each_cpu()? 

Cheers,
Ashok

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 21:39           ` Borislav Petkov
  2023-01-09 21:50             ` Luck, Tony
@ 2023-01-09 23:43             ` Reinette Chatre
  2023-01-24 11:32               ` Borislav Petkov
  1 sibling, 1 reply; 51+ messages in thread
From: Reinette Chatre @ 2023-01-09 23:43 UTC (permalink / raw)
  To: Borislav Petkov, Luck, Tony
  Cc: Moger, Babu, corbet, tglx, mingo, Yu, Fenghua, dave.hansen, x86,
	hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, Bae, Chang Seok,
	pawan.kumar.gupta, jmattson, daniel.sneddon, sandipan.das,
	james.morse, linux-doc, linux-kernel, bagasdotme, Eranian,
	Stephane, christophe.leroy, jarkko, Hunter, Adrian, quic_jiles,
	peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask()
  2023-01-09 23:26   ` Ashok Raj
@ 2023-01-10  2:23     ` Moger, Babu
  2023-01-10 20:58       ` Luck, Tony
  0 siblings, 1 reply; 51+ messages in thread
From: Moger, Babu @ 2023-01-10  2:23 UTC (permalink / raw)
  To: Ashok Raj
  Cc: corbet, reinette.chatre, tglx, mingo, bp, fenghua.yu,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	chang.seok.bae, pawan.kumar.gupta, jmattson, daniel.sneddon,
	Das1, Sandipan, tony.luck, james.morse, linux-doc, linux-kernel,
	bagasdotme, eranian, christophe.leroy, jarkko, adrian.hunter,
	quic_jiles, peternewman, Ashok Raj

[AMD Official Use Only - General]

Hi Ashok,

> -----Original Message-----
> From: Ashok Raj <ashok_raj@linux.intel.com>
> Sent: Monday, January 9, 2023 5:27 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: corbet@lwn.net; reinette.chatre@intel.com; tglx@linutronix.de;
> mingo@redhat.com; bp@alien8.de; 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; 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>;
> tony.luck@intel.com; james.morse@arm.com; linux-doc@vger.kernel.org;
> linux-kernel@vger.kernel.org; bagasdotme@gmail.com; eranian@google.com;
> christophe.leroy@csgroup.eu; jarkko@kernel.org; adrian.hunter@intel.com;
> quic_jiles@quicinc.com; peternewman@google.com; Ashok Raj
> <ashok.raj@intel.com>
> Subject: Re: [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many()
> with on_each_cpu_mask()
> 
> On Mon, Jan 09, 2023 at 10:43:53AM -0600, Babu Moger wrote:
> > on_each_cpu_mask() runs the function on each CPU specified by cpumask,
> > which may include the local processor.
> >
> > Replace smp_call_function_many() with on_each_cpu_mask() to simplify
> > the code.
> >
> > Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 11 +++------
> >  arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 29 +++++++----------------
> >  2 files changed, 11 insertions(+), 29 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> > b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> > index 1df0e3262bca..7eece3d2d0c3 100644
> > --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> > +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> > @@ -310,7 +310,6 @@ int resctrl_arch_update_domains(struct rdt_resource
> *r, u32 closid)
> >  	enum resctrl_conf_type t;
> >  	cpumask_var_t cpu_mask;
> >  	struct rdt_domain *d;
> > -	int cpu;
> >  	u32 idx;
> >
> >  	if (!zalloc_cpumask_var(&cpu_mask, GFP_KERNEL)) @@ -341,13
> +340,9 @@
> > int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
> >
> >  	if (cpumask_empty(cpu_mask))
> >  		goto done;
> > -	cpu = get_cpu();
> > -	/* Update resource control msr on this CPU if it's in cpu_mask. */
> > -	if (cpumask_test_cpu(cpu, cpu_mask))
> > -		rdt_ctrl_update(&msr_param);
> > -	/* Update resource control msr on other CPUs. */
> > -	smp_call_function_many(cpu_mask, rdt_ctrl_update, &msr_param, 1);
> > -	put_cpu();
> > +
> > +	/* Update resource control msr on all the CPUs. */
> > +	on_each_cpu_mask(cpu_mask, rdt_ctrl_update, &msr_param, 1);
> 
> Do you require these updates to done immediately via an IPI? or can they be
> done bit lazy via schedule_on_each_cpu()?

I have not experimented with lazy schedule.  At least I know the call update_cpu_closid_rmid should be completed immediately. Otherwise, the result might be inconsistent as the tasks(or CPUs)  could be running on two different closed/rmids before it is updated on all CPUs in the domain.
Thanks
Babu

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask()
  2023-01-10  2:23     ` Moger, Babu
@ 2023-01-10 20:58       ` Luck, Tony
  2023-01-11 15:42         ` Ashok Raj
  0 siblings, 1 reply; 51+ messages in thread
From: Luck, Tony @ 2023-01-10 20:58 UTC (permalink / raw)
  To: Moger, Babu, Ashok Raj
  Cc: corbet, Chatre, Reinette, tglx, mingo, bp, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon, Das1,
	Sandipan, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman, Raj, Ashok

> > > + /* Update resource control msr on all the CPUs. */
> > > + on_each_cpu_mask(cpu_mask, rdt_ctrl_update, &msr_param, 1);
> >
> > Do you require these updates to done immediately via an IPI? or can they be
> > done bit lazy via schedule_on_each_cpu()?
>
> I have not experimented with lazy schedule.  At least I know the call
> update_cpu_closid_rmid should be completed immediately. Otherwise, the
> result might be inconsistent as the tasks(or CPUs)  could be running on
> two different closed/rmids before it is updated on all CPUs in the domain.

I think this does need to happen somewhat urgently. Imagine trying to give
some extra resources to a CPU bound real-time process. That process will
keep running with the old resource allocation.

-Tony

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask()
  2023-01-10 20:58       ` Luck, Tony
@ 2023-01-11 15:42         ` Ashok Raj
  2023-01-11 19:05           ` Luck, Tony
  0 siblings, 1 reply; 51+ messages in thread
From: Ashok Raj @ 2023-01-11 15:42 UTC (permalink / raw)
  To: Luck, Tony
  Cc: Moger, Babu, Ashok Raj, corbet, Chatre, Reinette, tglx, mingo,
	bp, Yu, Fenghua, dave.hansen, x86, hpa, paulmck, akpm,
	quic_neeraju, rdunlap, damien.lemoal, songmuchun, peterz,
	jpoimboe, pbonzini, Bae, Chang Seok, pawan.kumar.gupta, jmattson,
	daniel.sneddon, Das1, Sandipan, james.morse, linux-doc,
	linux-kernel, bagasdotme, Eranian, Stephane, christophe.leroy,
	jarkko, Hunter, Adrian, quic_jiles, peternewman, Ashok Raj

On Tue, Jan 10, 2023 at 12:58:47PM -0800, Tony Luck wrote:
> > > > + /* Update resource control msr on all the CPUs. */
> > > > + on_each_cpu_mask(cpu_mask, rdt_ctrl_update, &msr_param, 1);
> > >
> > > Do you require these updates to done immediately via an IPI? or can they be
> > > done bit lazy via schedule_on_each_cpu()?
> >
> > I have not experimented with lazy schedule.  At least I know the call
> > update_cpu_closid_rmid should be completed immediately. Otherwise, the
> > result might be inconsistent as the tasks(or CPUs)  could be running on
> > two different closed/rmids before it is updated on all CPUs in the domain.
> 
> I think this does need to happen somewhat urgently. Imagine trying to give
> some extra resources to a CPU bound real-time process. That process will
> keep running with the old resource allocation.

If the resctl was setup before spawning other threads then the thread
starts with the right values from the start, probably inheriting from the
parent?

I wasn't sure if the few ms difference is going to make much material
difference for that process. IPI's does shake things up and introduces
other overheads not related to this process.

Instead of victimizing just this process, we hurt everything else.

Does it make sense to do an experiment and see if there is any other
functional failures?

Cheers,
Ashok

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask()
  2023-01-11 15:42         ` Ashok Raj
@ 2023-01-11 19:05           ` Luck, Tony
  2023-01-11 23:21             ` Ashok Raj
  0 siblings, 1 reply; 51+ messages in thread
From: Luck, Tony @ 2023-01-11 19:05 UTC (permalink / raw)
  To: Raj, Ashok
  Cc: Moger, Babu, Ashok Raj, corbet, Chatre, Reinette, tglx, mingo,
	bp, Yu, Fenghua, dave.hansen, x86, hpa, paulmck, akpm,
	quic_neeraju, rdunlap, damien.lemoal, songmuchun, peterz,
	jpoimboe, pbonzini, Bae, Chang Seok, pawan.kumar.gupta, jmattson,
	daniel.sneddon, Das1, Sandipan, james.morse, linux-doc,
	linux-kernel, bagasdotme, Eranian, Stephane, christophe.leroy,
	jarkko, Hunter, Adrian, quic_jiles, peternewman

> I wasn't sure if the few ms difference is going to make much material
> difference for that process. IPI's does shake things up and introduces
> other overheads not related to this process.

Is it just a few milli-seconds? What is the scheduler priority of the kernel
thread you wake to perform this action? How does that compare to the
priority of a RT thread?  I may be wrong here, but I think an RT thread can
block a kernel thread from running indefinitely.

-Tony

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config
  2023-01-09 16:44 ` [PATCH v11 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config Babu Moger
@ 2023-01-11 22:04   ` Reinette Chatre
  0 siblings, 0 replies; 51+ messages in thread
From: Reinette Chatre @ 2023-01-11 22:04 UTC (permalink / raw)
  To: Babu Moger, corbet, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	chang.seok.bae, pawan.kumar.gupta, jmattson, daniel.sneddon,
	sandipan.das, tony.luck, james.morse, linux-doc, linux-kernel,
	bagasdotme, eranian, christophe.leroy, jarkko, adrian.hunter,
	quic_jiles, peternewman

Hi Babu,

On 1/9/2023 8:44 AM, Babu Moger wrote:
> The event configuration for mbm_total_bytes can be changed by the user by
> writing to the file /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config.
> 
> The event configuration settings are domain specific and affect all the
> CPUs in the domain.
> 
> 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
> ====  ===========================================================
> 
> For example:
> To change the mbm_total_bytes to count only reads on domain 0, the bits
> 0, 1, 4 and 5 needs to be set, which is 110011b (in hex 0x33). Run the
> command.
>         $echo  0=0x33 > /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config
> 
> To change the mbm_total_bytes to count all the slow memory reads on
> domain 1, the bits 4 and 5 needs to be set which is 110000b (in hex 0x30).
> Run the command.
>         $echo  1=0x30 > /sys/fs/resctrl/info/L3_MON/mbm_total_bytes_config
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Thank you

Reinette

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features
  2023-01-09 16:44 ` [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features Babu Moger
@ 2023-01-11 22:06   ` Reinette Chatre
  2023-01-11 22:39     ` Moger, Babu
  0 siblings, 1 reply; 51+ messages in thread
From: Reinette Chatre @ 2023-01-11 22:06 UTC (permalink / raw)
  To: Babu Moger, corbet, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	chang.seok.bae, pawan.kumar.gupta, jmattson, daniel.sneddon,
	sandipan.das, tony.luck, james.morse, linux-doc, linux-kernel,
	bagasdotme, eranian, christophe.leroy, jarkko, adrian.hunter,
	quic_jiles, peternewman

Hi Babu,

On 1/9/2023 8:44 AM, Babu Moger wrote:
> Update the documentation for the new features:
> 1. Slow Memory Bandwidth allocation (SMBA).
>    With this feature, the QOS  enforcement policies can be applied
>    to the external slow memory connected to the host. QOS enforcement
>    is accomplished by assigning a Class Of Service (COS) to a processor
>    and specifying allocations or limits for that COS for each resource
>    to be allocated.
> 
> 2. 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.
> 
> Also add configuration instructions with examples.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  Documentation/x86/resctrl.rst | 142 +++++++++++++++++++++++++++++++++-
>  1 file changed, 140 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
> index 71a531061e4e..2860856f4463 100644
> --- a/Documentation/x86/resctrl.rst
> +++ b/Documentation/x86/resctrl.rst
> @@ -17,14 +17,16 @@ AMD refers to this feature as AMD Platform Quality of Service(AMD QoS).
>  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"
> +===============================================	================================
>  

I expect that you will follow Boris's guidance here and not make these flags visible in
/proc/cpuinfo. That would imply that this addition will have no entries in the second
column. Perhaps this could be made easier to parse by using empty quotes ("") in the second
column to match syntax used in the existing flags as well as the cpufeatures.h change?

If/when making this change, could you please also add a note that documents this new
guidance for other resctrl developers? Something like below but I am looking forward to
improvements:
"Historically new features were made visible by default in /proc/cpuinfo. This resulted
in the flags field becoming hard to parse by humans. Adding a new flag to /proc/cpuinfo
should be avoided if user space can obtain information about the feature from resctrl's
info directory."

The rest of the document looks good to me.

Thank you

Reinette

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features
  2023-01-11 22:06   ` Reinette Chatre
@ 2023-01-11 22:39     ` Moger, Babu
  2023-01-11 22:56       ` Reinette Chatre
  0 siblings, 1 reply; 51+ messages in thread
From: Moger, Babu @ 2023-01-11 22:39 UTC (permalink / raw)
  To: Reinette Chatre, corbet, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	chang.seok.bae, pawan.kumar.gupta, jmattson, daniel.sneddon,
	Das1, Sandipan, tony.luck, james.morse, linux-doc, linux-kernel,
	bagasdotme, eranian, christophe.leroy, jarkko, adrian.hunter,
	quic_jiles, peternewman

[AMD Official Use Only - General]

Hi Reinette,

> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@intel.com>
> Sent: Wednesday, January 11, 2023 4:07 PM
> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net;
> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de
> Cc: 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;
> 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>; tony.luck@intel.com; james.morse@arm.com;
> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> bagasdotme@gmail.com; eranian@google.com; christophe.leroy@csgroup.eu;
> jarkko@kernel.org; adrian.hunter@intel.com; quic_jiles@quicinc.com;
> peternewman@google.com
> Subject: Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new
> features
> 
> Hi Babu,
> 
> On 1/9/2023 8:44 AM, Babu Moger wrote:
> > Update the documentation for the new features:
> > 1. Slow Memory Bandwidth allocation (SMBA).
> >    With this feature, the QOS  enforcement policies can be applied
> >    to the external slow memory connected to the host. QOS enforcement
> >    is accomplished by assigning a Class Of Service (COS) to a processor
> >    and specifying allocations or limits for that COS for each resource
> >    to be allocated.
> >
> > 2. 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.
> >
> > Also add configuration instructions with examples.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> >  Documentation/x86/resctrl.rst | 142
> > +++++++++++++++++++++++++++++++++-
> >  1 file changed, 140 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/x86/resctrl.rst
> > b/Documentation/x86/resctrl.rst index 71a531061e4e..2860856f4463
> > 100644
> > --- a/Documentation/x86/resctrl.rst
> > +++ b/Documentation/x86/resctrl.rst
> > @@ -17,14 +17,16 @@ AMD refers to this feature as AMD Platform Quality
> of Service(AMD QoS).
> >  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"
> > +===============================================
> 	================================
> >
> 
> I expect that you will follow Boris's guidance here and not make these flags
> visible in /proc/cpuinfo. That would imply that this addition will have no entries
> in the second column. Perhaps this could be made easier to parse by using
> empty quotes ("") in the second column to match syntax used in the existing
> flags as well as the cpufeatures.h change?

Hmm.. I thought we dropped that idea for now. Did I miss understand that?
Thanks
Babu 

> 
> If/when making this change, could you please also add a note that documents
> this new guidance for other resctrl developers? Something like below but I am
> looking forward to
> improvements:
> "Historically new features were made visible by default in /proc/cpuinfo. This
> resulted in the flags field becoming hard to parse by humans. Adding a new
> flag to /proc/cpuinfo should be avoided if user space can obtain information
> about the feature from resctrl's info directory."
> 
> The rest of the document looks good to me.
> 
> Thank you
> 
> Reinette

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features
  2023-01-11 22:39     ` Moger, Babu
@ 2023-01-11 22:56       ` Reinette Chatre
  2023-01-12  0:39         ` Moger, Babu
  2023-01-12  0:47         ` Moger, Babu
  0 siblings, 2 replies; 51+ messages in thread
From: Reinette Chatre @ 2023-01-11 22:56 UTC (permalink / raw)
  To: Moger, Babu, corbet, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	chang.seok.bae, pawan.kumar.gupta, jmattson, daniel.sneddon,
	Das1, Sandipan, tony.luck, james.morse, linux-doc, linux-kernel,
	bagasdotme, eranian, christophe.leroy, jarkko, adrian.hunter,
	quic_jiles, peternewman

Hi Babu,

On 1/11/2023 2:39 PM, Moger, Babu wrote:
> [AMD Official Use Only - General]
> 
> Hi Reinette,
> 
>> -----Original Message-----
>> From: Reinette Chatre <reinette.chatre@intel.com>
>> Sent: Wednesday, January 11, 2023 4:07 PM
>> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net;
>> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de
>> Cc: 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;
>> 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>; tony.luck@intel.com; james.morse@arm.com;
>> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
>> bagasdotme@gmail.com; eranian@google.com; christophe.leroy@csgroup.eu;
>> jarkko@kernel.org; adrian.hunter@intel.com; quic_jiles@quicinc.com;
>> peternewman@google.com
>> Subject: Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new
>> features
>>
>> Hi Babu,
>>
>> On 1/9/2023 8:44 AM, Babu Moger wrote:
>>> Update the documentation for the new features:
>>> 1. Slow Memory Bandwidth allocation (SMBA).
>>>    With this feature, the QOS  enforcement policies can be applied
>>>    to the external slow memory connected to the host. QOS enforcement
>>>    is accomplished by assigning a Class Of Service (COS) to a processor
>>>    and specifying allocations or limits for that COS for each resource
>>>    to be allocated.
>>>
>>> 2. 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.
>>>
>>> Also add configuration instructions with examples.
>>>
>>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>>> ---
>>>  Documentation/x86/resctrl.rst | 142
>>> +++++++++++++++++++++++++++++++++-
>>>  1 file changed, 140 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/x86/resctrl.rst
>>> b/Documentation/x86/resctrl.rst index 71a531061e4e..2860856f4463
>>> 100644
>>> --- a/Documentation/x86/resctrl.rst
>>> +++ b/Documentation/x86/resctrl.rst
>>> @@ -17,14 +17,16 @@ AMD refers to this feature as AMD Platform Quality
>> of Service(AMD QoS).
>>>  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"
>>> +===============================================
>> 	================================
>>>
>>
>> I expect that you will follow Boris's guidance here and not make these flags
>> visible in /proc/cpuinfo. That would imply that this addition will have no entries
>> in the second column. Perhaps this could be made easier to parse by using
>> empty quotes ("") in the second column to match syntax used in the existing
>> flags as well as the cpufeatures.h change?
> 
> Hmm.. I thought we dropped that idea for now. Did I miss understand that?

I referred to the guidance in https://lore.kernel.org/lkml/Y7xjxUj+KnOEJssZ@zn.tnic/
Since the SMBA and BMEC features have never appeared in /proc/cpuinfo there cannot
be a user space that expects these flags in /proc/cpuinfo and thus no risk of
breaking user space. User space can get information about SMBA and BMEC
from the info directory. 

Later that thread discussed removal of existing resctrl feature flags from
/proc/cpuinfo - that is what I think we shouldn't do since there are 
user space consumers of those flags. I thus agree that the task described in
https://lore.kernel.org/lkml/MW3PR12MB455384130AF0BDE3AF88BCF095FE9@MW3PR12MB4553.namprd12.prod.outlook.com/
can be dropped.

I do not think this is a big change ... just add the empty quotes to the
two cpufeatures.h patches and a new snippet to the resctrl documentation.

Reinette

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask()
  2023-01-11 19:05           ` Luck, Tony
@ 2023-01-11 23:21             ` Ashok Raj
  0 siblings, 0 replies; 51+ messages in thread
From: Ashok Raj @ 2023-01-11 23:21 UTC (permalink / raw)
  To: Luck, Tony
  Cc: Moger, Babu, Ashok Raj, corbet, Chatre, Reinette, tglx, mingo,
	bp, Yu, Fenghua, dave.hansen, x86, hpa, paulmck, akpm,
	quic_neeraju, rdunlap, damien.lemoal, songmuchun, peterz,
	jpoimboe, pbonzini, Bae, Chang Seok, pawan.kumar.gupta, jmattson,
	daniel.sneddon, Das1, Sandipan, james.morse, linux-doc,
	linux-kernel, bagasdotme, Eranian, Stephane, christophe.leroy,
	jarkko, Hunter, Adrian, quic_jiles, peternewman, Ashok Raj

On Wed, Jan 11, 2023 at 11:05:48AM -0800, Tony Luck wrote:
> > I wasn't sure if the few ms difference is going to make much material
> > difference for that process. IPI's does shake things up and introduces
> > other overheads not related to this process.
> 
> Is it just a few milli-seconds? What is the scheduler priority of the kernel
> thread you wake to perform this action? How does that compare to the
> priority of a RT thread?  I may be wrong here, but I think an RT thread can
> block a kernel thread from running indefinitely.

That's a good point. I don't have an idea about how the kernel thread
priority is compared to user space RT threads.

I assumed that since these threads have some work to be done from kernel,
user space shouldn't get higher priority than kernel tasks.

Uncharted territory for me ... Just a hunch :)

Maybe @peterz can screw my head in the right direction :-)

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features
  2023-01-11 22:56       ` Reinette Chatre
@ 2023-01-12  0:39         ` Moger, Babu
  2023-01-12  0:47         ` Moger, Babu
  1 sibling, 0 replies; 51+ messages in thread
From: Moger, Babu @ 2023-01-12  0:39 UTC (permalink / raw)
  To: Reinette Chatre, Moger, Babu, corbet, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	chang.seok.bae, pawan.kumar.gupta, jmattson, daniel.sneddon,
	Das1, Sandipan, tony.luck, james.morse, linux-doc, linux-kernel,
	bagasdotme, eranian, christophe.leroy, jarkko, adrian.hunter,
	quic_jiles, peternewman

Hi Reinette,

On 1/11/2023 4:56 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 1/11/2023 2:39 PM, Moger, Babu wrote:
>> [AMD Official Use Only - General]
>>
>> Hi Reinette,
>>
>>> -----Original Message-----
>>> From: Reinette Chatre <reinette.chatre@intel.com>
>>> Sent: Wednesday, January 11, 2023 4:07 PM
>>> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net;
>>> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de
>>> Cc: 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;
>>> 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>; tony.luck@intel.com; james.morse@arm.com;
>>> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
>>> bagasdotme@gmail.com; eranian@google.com; christophe.leroy@csgroup.eu;
>>> jarkko@kernel.org; adrian.hunter@intel.com; quic_jiles@quicinc.com;
>>> peternewman@google.com
>>> Subject: Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new
>>> features
>>>
>>> Hi Babu,
>>>
>>> On 1/9/2023 8:44 AM, Babu Moger wrote:
>>>> Update the documentation for the new features:
>>>> 1. Slow Memory Bandwidth allocation (SMBA).
>>>>     With this feature, the QOS  enforcement policies can be applied
>>>>     to the external slow memory connected to the host. QOS enforcement
>>>>     is accomplished by assigning a Class Of Service (COS) to a processor
>>>>     and specifying allocations or limits for that COS for each resource
>>>>     to be allocated.
>>>>
>>>> 2. 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.
>>>>
>>>> Also add configuration instructions with examples.
>>>>
>>>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>>>> ---
>>>>   Documentation/x86/resctrl.rst | 142
>>>> +++++++++++++++++++++++++++++++++-
>>>>   1 file changed, 140 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/x86/resctrl.rst
>>>> b/Documentation/x86/resctrl.rst index 71a531061e4e..2860856f4463
>>>> 100644
>>>> --- a/Documentation/x86/resctrl.rst
>>>> +++ b/Documentation/x86/resctrl.rst
>>>> @@ -17,14 +17,16 @@ AMD refers to this feature as AMD Platform Quality
>>> of Service(AMD QoS).
>>>>   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"
>>>> +===============================================
>>> 	================================
>>> I expect that you will follow Boris's guidance here and not make these flags
>>> visible in /proc/cpuinfo. That would imply that this addition will have no entries
>>> in the second column. Perhaps this could be made easier to parse by using
>>> empty quotes ("") in the second column to match syntax used in the existing
>>> flags as well as the cpufeatures.h change?
>> Hmm.. I thought we dropped that idea for now. Did I miss understand that?
> I referred to the guidance in https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2FY7xjxUj%2BKnOEJssZ%40zn.tnic%2F&data=05%7C01%7CBabu.Moger%40amd.com%7C900eb41c0e6049dd342208daf4270d2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638090745842366944%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2F5GVOhnxq1%2B3nJwGtlApvLfC%2FeX3X9RDaUZa9R92NiY%3D&reserved=0
> Since the SMBA and BMEC features have never appeared in /proc/cpuinfo there cannot
> be a user space that expects these flags in /proc/cpuinfo and thus no risk of
> breaking user space. User space can get information about SMBA and BMEC
> from the info directory.
ok. Got it.
>
> Later that thread discussed removal of existing resctrl feature flags from
> /proc/cpuinfo - that is what I think we shouldn't do since there are
> user space consumers of those flags. I thus agree that the task described in
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2FMW3PR12MB455384130AF0BDE3AF88BCF095FE9%40MW3PR12MB4553.namprd12.prod.outlook.com%2F&data=05%7C01%7CBabu.Moger%40amd.com%7C900eb41c0e6049dd342208daf4270d2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638090745842366944%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=kE7d0cFYyJq1n4ZKKeeF%2FC%2FFDDJy0Sc%2Fd5MZ%2Bc56WQw%3D&reserved=0
> can be dropped.
Sure.
> I do not think this is a big change ... just add the empty quotes to the
> two cpufeatures.h patches and a new snippet to the resctrl documentation.


How about this? I want to get this right.. Hopefully next version can be 
final.

diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
index 2860856f4463..7df5889237f4 100644
--- a/Documentation/x86/resctrl.rst
+++ b/Documentation/x86/resctrl.rst
@@ -24,10 +24,15 @@ 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"
+SMBA (Slow Memory Bandwidth Allocation)         ""
+BMEC (Bandwidth Monitoring Event Configuration) ""
  =============================================== 
================================

+Historically, new features were made visible by default in 
/proc/cpuinfo. This
+resulted in the feature flags becoming hard to parse by the humans. 
Adding a new
+flag to /proc/cpuinfo should be avoided if user space can obtain 
information
+about the feature from resctrl's info directory.
+
  To use the feature mount the file system::

   # mount -t resctrl resctrl [-o cdp[,cdpl2][,mba_MBps]] /sys/fs/resctrl


thanks

Babu


^ permalink raw reply related	[flat|nested] 51+ messages in thread

* RE: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features
  2023-01-11 22:56       ` Reinette Chatre
  2023-01-12  0:39         ` Moger, Babu
@ 2023-01-12  0:47         ` Moger, Babu
  2023-01-12 17:30           ` Reinette Chatre
  1 sibling, 1 reply; 51+ messages in thread
From: Moger, Babu @ 2023-01-12  0:47 UTC (permalink / raw)
  To: Reinette Chatre, corbet, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	chang.seok.bae, pawan.kumar.gupta, jmattson, daniel.sneddon,
	Das1, Sandipan, tony.luck, james.morse, linux-doc, linux-kernel,
	bagasdotme, eranian, christophe.leroy, jarkko, adrian.hunter,
	quic_jiles, peternewman

[AMD Official Use Only - General]

Hi Reinette,

> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@intel.com>
> Sent: Wednesday, January 11, 2023 4:56 PM
> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net;
> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de
> Cc: 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;
> 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>; tony.luck@intel.com; james.morse@arm.com;
> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> bagasdotme@gmail.com; eranian@google.com; christophe.leroy@csgroup.eu;
> jarkko@kernel.org; adrian.hunter@intel.com; quic_jiles@quicinc.com;
> peternewman@google.com
> Subject: Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new
> features
> 
> Hi Babu,
> 
> On 1/11/2023 2:39 PM, Moger, Babu wrote:
> > [AMD Official Use Only - General]
> >
> > Hi Reinette,
> >
> >> -----Original Message-----
> >> From: Reinette Chatre <reinette.chatre@intel.com>
> >> Sent: Wednesday, January 11, 2023 4:07 PM
> >> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net;
> >> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de
> >> Cc: 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; 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>; tony.luck@intel.com; james.morse@arm.com;
> >> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> bagasdotme@gmail.com; eranian@google.com;
> >> christophe.leroy@csgroup.eu; jarkko@kernel.org;
> >> adrian.hunter@intel.com; quic_jiles@quicinc.com;
> >> peternewman@google.com
> >> Subject: Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst
> >> for new features
> >>
> >> Hi Babu,
> >>
> >> On 1/9/2023 8:44 AM, Babu Moger wrote:
> >>> Update the documentation for the new features:
> >>> 1. Slow Memory Bandwidth allocation (SMBA).
> >>>    With this feature, the QOS  enforcement policies can be applied
> >>>    to the external slow memory connected to the host. QOS enforcement
> >>>    is accomplished by assigning a Class Of Service (COS) to a processor
> >>>    and specifying allocations or limits for that COS for each resource
> >>>    to be allocated.
> >>>
> >>> 2. 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.
> >>>
> >>> Also add configuration instructions with examples.
> >>>
> >>> Signed-off-by: Babu Moger <babu.moger@amd.com>
> >>> ---
> >>>  Documentation/x86/resctrl.rst | 142
> >>> +++++++++++++++++++++++++++++++++-
> >>>  1 file changed, 140 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/x86/resctrl.rst
> >>> b/Documentation/x86/resctrl.rst index 71a531061e4e..2860856f4463
> >>> 100644
> >>> --- a/Documentation/x86/resctrl.rst
> >>> +++ b/Documentation/x86/resctrl.rst
> >>> @@ -17,14 +17,16 @@ AMD refers to this feature as AMD Platform
> >>> Quality
> >> of Service(AMD QoS).
> >>>  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"
> >>> +===============================================
> >> 	================================
> >>>
> >>
> >> I expect that you will follow Boris's guidance here and not make
> >> these flags visible in /proc/cpuinfo. That would imply that this
> >> addition will have no entries in the second column. Perhaps this
> >> could be made easier to parse by using empty quotes ("") in the
> >> second column to match syntax used in the existing flags as well as the
> cpufeatures.h change?
> >
> > Hmm.. I thought we dropped that idea for now. Did I miss understand that?
> 
> I referred to the guidance in
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern
> el.org%2Flkml%2FY7xjxUj%2BKnOEJssZ%40zn.tnic%2F&data=05%7C01%7CBabu
> .Moger%40amd.com%7C900eb41c0e6049dd342208daf4270d2b%7C3dd8961fe
> 4884e608e11a82d994e183d%7C0%7C0%7C638090745842366944%7CUnknown
> %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw
> iLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2F5GVOhnxq1%2B3nJwGtlAp
> vLfC%2FeX3X9RDaUZa9R92NiY%3D&reserved=0
> Since the SMBA and BMEC features have never appeared in /proc/cpuinfo
> there cannot be a user space that expects these flags in /proc/cpuinfo and thus
> no risk of breaking user space. User space can get information about SMBA and
> BMEC from the info directory.
> 
> Later that thread discussed removal of existing resctrl feature flags from
> /proc/cpuinfo - that is what I think we shouldn't do since there are user space
> consumers of those flags. I thus agree that the task described in
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern
> el.org%2Flkml%2FMW3PR12MB455384130AF0BDE3AF88BCF095FE9%40MW3P
> R12MB4553.namprd12.prod.outlook.com%2F&data=05%7C01%7CBabu.Moger
> %40amd.com%7C900eb41c0e6049dd342208daf4270d2b%7C3dd8961fe4884e6
> 08e11a82d994e183d%7C0%7C0%7C638090745842366944%7CUnknown%7CTW
> FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC
> I6Mn0%3D%7C3000%7C%7C%7C&sdata=kE7d0cFYyJq1n4ZKKeeF%2FC%2FFDDJ
> y0Sc%2Fd5MZ%2Bc56WQw%3D&reserved=0
> can be dropped.
> 
> I do not think this is a big change ... just add the empty quotes to the two
> cpufeatures.h patches and a new snippet to the resctrl documentation.

Previous one got garbled. Here is the correct one.

diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
index 2860856f4463..7df5889237f4 100644
--- a/Documentation/x86/resctrl.rst
+++ b/Documentation/x86/resctrl.rst
@@ -24,10 +24,15 @@ 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"
+SMBA (Slow Memory Bandwidth Allocation)         ""
+BMEC (Bandwidth Monitoring Event Configuration) ""
 ===============================================        ================================

+Historically, new features were made visible by default in /proc/cpuinfo. This
+resulted in the feature flags becoming hard to parse by the humans. Adding a new
+flag to /proc/cpuinfo should be avoided if user space can obtain information
+about the feature from resctrl's info directory.
+
 To use the feature mount the file system::

  # mount -t resctrl resctrl [-o cdp[,cdpl2][,mba_MBps]] /sys/fs/resctrl

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features
  2023-01-12  0:47         ` Moger, Babu
@ 2023-01-12 17:30           ` Reinette Chatre
  2023-01-12 19:06             ` Moger, Babu
  0 siblings, 1 reply; 51+ messages in thread
From: Reinette Chatre @ 2023-01-12 17:30 UTC (permalink / raw)
  To: Moger, Babu, corbet, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	chang.seok.bae, pawan.kumar.gupta, jmattson, daniel.sneddon,
	Das1, Sandipan, tony.luck, james.morse, linux-doc, linux-kernel,
	bagasdotme, eranian, christophe.leroy, jarkko, adrian.hunter,
	quic_jiles, peternewman

Hi Babu,

On 1/11/2023 4:47 PM, Moger, Babu wrote:
 
> diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
> index 2860856f4463..7df5889237f4 100644
> --- a/Documentation/x86/resctrl.rst
> +++ b/Documentation/x86/resctrl.rst
> @@ -24,10 +24,15 @@ 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"
> +SMBA (Slow Memory Bandwidth Allocation)         ""
> +BMEC (Bandwidth Monitoring Event Configuration) ""
>  ===============================================        ================================
> 
> +Historically, new features were made visible by default in /proc/cpuinfo. This
> +resulted in the feature flags becoming hard to parse by the humans. Adding a new
> +flag to /proc/cpuinfo should be avoided if user space can obtain information
> +about the feature from resctrl's info directory.
> +

Could you please replace "parse by the humans" with "parse by humans"?

The rest looks good to me.

Could you please do a sanity check by building the documentation to ensure
that the usage of the empty quotes looks as expected and is not parsed out by a
tool when, for example, creating the html docs?

>  To use the feature mount the file system::
> 
>   # mount -t resctrl resctrl [-o cdp[,cdpl2][,mba_MBps]] /sys/fs/resctrl

Thank you very much

Reinette

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features
  2023-01-12 17:30           ` Reinette Chatre
@ 2023-01-12 19:06             ` Moger, Babu
  0 siblings, 0 replies; 51+ messages in thread
From: Moger, Babu @ 2023-01-12 19:06 UTC (permalink / raw)
  To: Reinette Chatre, corbet, tglx, mingo, bp
  Cc: fenghua.yu, dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju,
	rdunlap, damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini,
	chang.seok.bae, pawan.kumar.gupta, jmattson, daniel.sneddon,
	Das1, Sandipan, tony.luck, james.morse, linux-doc, linux-kernel,
	bagasdotme, eranian, christophe.leroy, jarkko, adrian.hunter,
	quic_jiles, peternewman


On 1/12/23 11:30, Reinette Chatre wrote:
> Hi Babu,
>
> On 1/11/2023 4:47 PM, Moger, Babu wrote:
>  
>> diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
>> index 2860856f4463..7df5889237f4 100644
>> --- a/Documentation/x86/resctrl.rst
>> +++ b/Documentation/x86/resctrl.rst
>> @@ -24,10 +24,15 @@ 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"
>> +SMBA (Slow Memory Bandwidth Allocation)         ""
>> +BMEC (Bandwidth Monitoring Event Configuration) ""
>>  ===============================================        ================================
>>
>> +Historically, new features were made visible by default in /proc/cpuinfo. This
>> +resulted in the feature flags becoming hard to parse by the humans. Adding a new
>> +flag to /proc/cpuinfo should be avoided if user space can obtain information
>> +about the feature from resctrl's info directory.
>> +
> Could you please replace "parse by the humans" with "parse by humans"?
Sure.
>
> The rest looks good to me.
>
> Could you please do a sanity check by building the documentation to ensure
> that the usage of the empty quotes looks as expected and is not parsed out by a
> tool when, for example, creating the html docs?

Yes. Will do.

Thanks

Babu

>
>>  To use the feature mount the file system::
>>
>>   # mount -t resctrl resctrl [-o cdp[,cdpl2][,mba_MBps]] /sys/fs/resctrl
> Thank you very much
>
> Reinette

-- 
Thanks
Babu Moger


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 21:50             ` Luck, Tony
@ 2023-01-24 11:28               ` Borislav Petkov
  0 siblings, 0 replies; 51+ messages in thread
From: Borislav Petkov @ 2023-01-24 11:28 UTC (permalink / raw)
  To: Luck, Tony
  Cc: Moger, Babu, corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon,
	sandipan.das, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 23:43             ` Reinette Chatre
@ 2023-01-24 11:32               ` Borislav Petkov
  0 siblings, 0 replies; 51+ messages in thread
From: Borislav Petkov @ 2023-01-24 11:32 UTC (permalink / raw)
  To: Reinette Chatre
  Cc: Luck, Tony, Moger, Babu, corbet, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon,
	sandipan.das, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-09 23:10               ` Moger, Babu
@ 2023-01-24 11:34                 ` Borislav Petkov
  2023-01-24 14:11                   ` Moger, Babu
  0 siblings, 1 reply; 51+ messages in thread
From: Borislav Petkov @ 2023-01-24 11:34 UTC (permalink / raw)
  To: Moger, Babu
  Cc: Luck, Tony, corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon, Das1,
	Sandipan, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-24 11:34                 ` Borislav Petkov
@ 2023-01-24 14:11                   ` Moger, Babu
  2023-01-24 15:10                     ` Borislav Petkov
  0 siblings, 1 reply; 51+ messages in thread
From: Moger, Babu @ 2023-01-24 14:11 UTC (permalink / raw)
  To: Borislav Petkov, Moger, Babu
  Cc: Luck, Tony, corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon, Das1,
	Sandipan, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman


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


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-24 14:11                   ` Moger, Babu
@ 2023-01-24 15:10                     ` Borislav Petkov
  2023-01-24 16:06                       ` Moger, Babu
  0 siblings, 1 reply; 51+ messages in thread
From: Borislav Petkov @ 2023-01-24 15:10 UTC (permalink / raw)
  To: babu.moger
  Cc: Luck, Tony, corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon, Das1,
	Sandipan, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-24 15:10                     ` Borislav Petkov
@ 2023-01-24 16:06                       ` Moger, Babu
  2023-01-24 16:59                         ` Reinette Chatre
  0 siblings, 1 reply; 51+ messages in thread
From: Moger, Babu @ 2023-01-24 16:06 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Luck, Tony, corbet, Chatre, Reinette, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon, Das1,
	Sandipan, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman


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



^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-24 16:06                       ` Moger, Babu
@ 2023-01-24 16:59                         ` Reinette Chatre
  2023-01-24 17:30                           ` Moger, Babu
  0 siblings, 1 reply; 51+ messages in thread
From: Reinette Chatre @ 2023-01-24 16:59 UTC (permalink / raw)
  To: babu.moger, Borislav Petkov
  Cc: Luck, Tony, corbet, tglx, mingo, Yu, Fenghua, dave.hansen, x86,
	hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, Bae, Chang Seok,
	pawan.kumar.gupta, jmattson, daniel.sneddon, Das1, Sandipan,
	james.morse, linux-doc, linux-kernel, bagasdotme, Eranian,
	Stephane, christophe.leroy, jarkko, Hunter, Adrian, quic_jiles,
	peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-24 16:59                         ` Reinette Chatre
@ 2023-01-24 17:30                           ` Moger, Babu
  2023-01-24 17:47                             ` Reinette Chatre
  0 siblings, 1 reply; 51+ messages in thread
From: Moger, Babu @ 2023-01-24 17:30 UTC (permalink / raw)
  To: Reinette Chatre, Borislav Petkov
  Cc: Luck, Tony, corbet, tglx, mingo, Yu, Fenghua, dave.hansen, x86,
	hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, Bae, Chang Seok,
	pawan.kumar.gupta, jmattson, daniel.sneddon, Das1, Sandipan,
	james.morse, linux-doc, linux-kernel, bagasdotme, Eranian,
	Stephane, christophe.leroy, jarkko, Hunter, Adrian, quic_jiles,
	peternewman


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


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-24 17:30                           ` Moger, Babu
@ 2023-01-24 17:47                             ` Reinette Chatre
  2023-01-24 19:03                               ` Moger, Babu
  0 siblings, 1 reply; 51+ messages in thread
From: Reinette Chatre @ 2023-01-24 17:47 UTC (permalink / raw)
  To: babu.moger, Borislav Petkov
  Cc: Luck, Tony, corbet, tglx, mingo, Yu, Fenghua, dave.hansen, x86,
	hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, Bae, Chang Seok,
	pawan.kumar.gupta, jmattson, daniel.sneddon, Das1, Sandipan,
	james.morse, linux-doc, linux-kernel, bagasdotme, Eranian,
	Stephane, christophe.leroy, jarkko, Hunter, Adrian, quic_jiles,
	peternewman

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


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-24 17:47                             ` Reinette Chatre
@ 2023-01-24 19:03                               ` Moger, Babu
  2023-01-24 19:11                                 ` Borislav Petkov
  2023-01-24 19:23                                 ` Reinette Chatre
  0 siblings, 2 replies; 51+ messages in thread
From: Moger, Babu @ 2023-01-24 19:03 UTC (permalink / raw)
  To: Reinette Chatre, Borislav Petkov
  Cc: Luck, Tony, corbet, tglx, mingo, Yu, Fenghua, dave.hansen, x86,
	hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, Bae, Chang Seok,
	pawan.kumar.gupta, jmattson, daniel.sneddon, Das1, Sandipan,
	james.morse, linux-doc, linux-kernel, bagasdotme, Eranian,
	Stephane, christophe.leroy, jarkko, Hunter, Adrian, quic_jiles,
	peternewman


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


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-24 19:03                               ` Moger, Babu
@ 2023-01-24 19:11                                 ` Borislav Petkov
  2023-01-24 19:23                                 ` Reinette Chatre
  1 sibling, 0 replies; 51+ messages in thread
From: Borislav Petkov @ 2023-01-24 19:11 UTC (permalink / raw)
  To: Moger, Babu
  Cc: Reinette Chatre, Luck, Tony, corbet, tglx, mingo, Yu, Fenghua,
	dave.hansen, x86, hpa, paulmck, akpm, quic_neeraju, rdunlap,
	damien.lemoal, songmuchun, peterz, jpoimboe, pbonzini, Bae,
	Chang Seok, pawan.kumar.gupta, jmattson, daniel.sneddon, Das1,
	Sandipan, james.morse, linux-doc, linux-kernel, bagasdotme,
	Eranian, Stephane, christophe.leroy, jarkko, Hunter, Adrian,
	quic_jiles, peternewman

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-24 19:03                               ` Moger, Babu
  2023-01-24 19:11                                 ` Borislav Petkov
@ 2023-01-24 19:23                                 ` Reinette Chatre
  2023-01-24 20:12                                   ` Moger, Babu
  1 sibling, 1 reply; 51+ messages in thread
From: Reinette Chatre @ 2023-01-24 19:23 UTC (permalink / raw)
  To: babu.moger, Borislav Petkov
  Cc: Luck, Tony, corbet, tglx, mingo, Yu, Fenghua, dave.hansen, x86,
	hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, Bae, Chang Seok,
	pawan.kumar.gupta, jmattson, daniel.sneddon, Das1, Sandipan,
	james.morse, linux-doc, linux-kernel, bagasdotme, Eranian,
	Stephane, christophe.leroy, jarkko, Hunter, Adrian, quic_jiles,
	peternewman

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/


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag
  2023-01-24 19:23                                 ` Reinette Chatre
@ 2023-01-24 20:12                                   ` Moger, Babu
  0 siblings, 0 replies; 51+ messages in thread
From: Moger, Babu @ 2023-01-24 20:12 UTC (permalink / raw)
  To: Reinette Chatre, Borislav Petkov
  Cc: Luck, Tony, corbet, tglx, mingo, Yu, Fenghua, dave.hansen, x86,
	hpa, paulmck, akpm, quic_neeraju, rdunlap, damien.lemoal,
	songmuchun, peterz, jpoimboe, pbonzini, Bae, Chang Seok,
	pawan.kumar.gupta, jmattson, daniel.sneddon, Das1, Sandipan,
	james.morse, linux-doc, linux-kernel, bagasdotme, Eranian,
	Stephane, christophe.leroy, jarkko, Hunter, Adrian, quic_jiles,
	peternewman

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


^ permalink raw reply	[flat|nested] 51+ messages in thread

end of thread, other threads:[~2023-01-24 20:12 UTC | newest]

Thread overview: 51+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-09 16:43 [PATCH v11 00/13] x86/resctrl: Support for AMD QoS new features Babu Moger
2023-01-09 16:43 ` [PATCH v11 01/13] x86/resctrl: Replace smp_call_function_many() with on_each_cpu_mask() Babu Moger
2023-01-09 23:26   ` Ashok Raj
2023-01-10  2:23     ` Moger, Babu
2023-01-10 20:58       ` Luck, Tony
2023-01-11 15:42         ` Ashok Raj
2023-01-11 19:05           ` Luck, Tony
2023-01-11 23:21             ` Ashok Raj
2023-01-09 16:43 ` [PATCH v11 02/13] x86/cpufeatures: Add Slow Memory Bandwidth Allocation feature flag Babu Moger
2023-01-09 16:43 ` [PATCH v11 03/13] x86/resctrl: Add a new resource type RDT_RESOURCE_SMBA Babu Moger
2023-01-09 16:43 ` [PATCH v11 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag Babu Moger
2023-01-09 18:58   ` Borislav Petkov
2023-01-09 19:49     ` Moger, Babu
2023-01-09 21:07       ` Borislav Petkov
2023-01-09 21:25         ` Luck, Tony
2023-01-09 21:39           ` Borislav Petkov
2023-01-09 21:50             ` Luck, Tony
2023-01-24 11:28               ` Borislav Petkov
2023-01-09 23:43             ` Reinette Chatre
2023-01-24 11:32               ` Borislav Petkov
2023-01-09 21:44           ` Moger, Babu
2023-01-09 21:51             ` Borislav Petkov
2023-01-09 23:10               ` Moger, Babu
2023-01-24 11:34                 ` Borislav Petkov
2023-01-24 14:11                   ` Moger, Babu
2023-01-24 15:10                     ` Borislav Petkov
2023-01-24 16:06                       ` Moger, Babu
2023-01-24 16:59                         ` Reinette Chatre
2023-01-24 17:30                           ` Moger, Babu
2023-01-24 17:47                             ` Reinette Chatre
2023-01-24 19:03                               ` Moger, Babu
2023-01-24 19:11                                 ` Borislav Petkov
2023-01-24 19:23                                 ` Reinette Chatre
2023-01-24 20:12                                   ` Moger, Babu
2023-01-09 16:43 ` [PATCH v11 05/13] x86/resctrl: Include new features in command line options Babu Moger
2023-01-09 16:43 ` [PATCH v11 06/13] x86/resctrl: Detect and configure Slow Memory Bandwidth Allocation Babu Moger
2023-01-09 16:43 ` [PATCH v11 07/13] x86/resctrl: Add __init attribute to rdt_get_mon_l3_config() Babu Moger
2023-01-09 16:44 ` [PATCH v11 08/13] x86/resctrl: Support monitor configuration Babu Moger
2023-01-09 16:44 ` [PATCH v11 09/13] x86/resctrl: Add interface to read mbm_total_bytes_config Babu Moger
2023-01-09 16:44 ` [PATCH v11 10/13] x86/resctrl: Add interface to read mbm_local_bytes_config Babu Moger
2023-01-09 16:44 ` [PATCH v11 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config Babu Moger
2023-01-11 22:04   ` Reinette Chatre
2023-01-09 16:44 ` [PATCH v11 12/13] x86/resctrl: Add interface to write mbm_local_bytes_config Babu Moger
2023-01-09 16:44 ` [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features Babu Moger
2023-01-11 22:06   ` Reinette Chatre
2023-01-11 22:39     ` Moger, Babu
2023-01-11 22:56       ` Reinette Chatre
2023-01-12  0:39         ` Moger, Babu
2023-01-12  0:47         ` Moger, Babu
2023-01-12 17:30           ` Reinette Chatre
2023-01-12 19:06             ` Moger, Babu

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.