2022-12-22 23:39:14

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 00/13] x86/resctrl: Support for AMD QoS new features

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.

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
---
v10:
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/[email protected]/

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 | 141 +++++++-
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 | 311 ++++++++++++++++--
arch/x86/kernel/cpu/scattered.c | 2 +
include/linux/resctrl.h | 11 +
12 files changed, 557 insertions(+), 41 deletions(-)

--
2.34.1


2022-12-22 23:39:38

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 02/13] x86/cpufeatures: Add Slow Memory Bandwidth Allocation feature flag

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 <[email protected]>
Signed-off-by: Babu Moger <[email protected]>
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

2022-12-22 23:39:57

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 12/13] x86/resctrl: Add interface to write mbm_local_bytes_config

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 <[email protected]>
Signed-off-by: Babu Moger <[email protected]>
---
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 29 +++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index acaf19f5d65b..59debc997162 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1638,6 +1638,32 @@ 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;
+
+ cpus_read_lock();
+ 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);
+ cpus_read_unlock();
+
+ return ret ?: nbytes;
+}
+
/* rdtgroup information files for one cache resource. */
static struct rftype res_common_files[] = {
{
@@ -1745,9 +1771,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

2022-12-22 23:46:40

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 05/13] x86/resctrl: Include new features in command line options

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 <[email protected]>
Signed-off-by: Babu Moger <[email protected]>
---
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 5a0eff3231c6..1935b0d9bd9a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5199,7 +5199,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

2022-12-22 23:51:51

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 09/13] x86/resctrl: Add interface to read mbm_total_bytes_config

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.

Signed-off-by: Babu Moger <[email protected]>
---
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..b2fb09d54f16 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

2022-12-22 23:53:56

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 04/13] x86/cpufeatures: Add Bandwidth Monitoring Event Configuration feature flag

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 <[email protected]>
Signed-off-by: Babu Moger <[email protected]>
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

2022-12-22 23:55:51

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 07/13] x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()

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.

Signed-off-by: Babu Moger <[email protected]>
---
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

2022-12-22 23:56:02

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 06/13] x86/resctrl: Detect and configure Slow Memory Bandwidth Allocation

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 <[email protected]>
Signed-off-by: Babu Moger <[email protected]>
---
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

2022-12-22 23:56:23

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 03/13] x86/resctrl: Add a new resource type RDT_RESOURCE_SMBA

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 <[email protected]>
Signed-off-by: Babu Moger <[email protected]>
---
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

2022-12-23 00:04:19

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config

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 <[email protected]>
---
arch/x86/kernel/cpu/resctrl/monitor.c | 17 ++++
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 126 ++++++++++++++++++++++++-
include/linux/resctrl.h | 11 +++
3 files changed, 153 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..acaf19f5d65b 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1515,6 +1515,129 @@ 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;
+
+ cpus_read_lock();
+ 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);
+ cpus_read_unlock();
+
+ return ret ?: nbytes;
+}
+
/* rdtgroup information files for one cache resource. */
static struct rftype res_common_files[] = {
{
@@ -1615,9 +1738,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

2022-12-23 00:11:08

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v10 10/13] x86/resctrl: Add interface to read mbm_local_bytes_config

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 <[email protected]>
Signed-off-by: Babu Moger <[email protected]>
---
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

2023-01-05 00:41:10

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v10 09/13] x86/resctrl: Add interface to read mbm_total_bytes_config

Hi Babu,

On 12/22/2022 3:31 PM, Babu Moger wrote:
> 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.
>
> Signed-off-by: Babu Moger <[email protected]>

...

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

Some stray white space above.

> +
> +/* Max event bits supported */
> +#define MAX_EVT_CONFIG_BITS GENMASK(6, 0)
>
> struct rdt_fs_context {
> struct kernfs_fs_context kfc;

Please remove stray white space. With that:

Reviewed-by: Reinette Chatre <[email protected]>

Reinette


2023-01-05 00:56:27

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v10 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config

Hi Babu,

On 12/22/2022 3:31 PM, Babu Moger wrote:

...

> +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;
> +
> + cpus_read_lock();

Could you please elaborate why this lock is needed here as
well as in the following patch?

Reinette

2023-01-05 01:08:34

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v10 07/13] x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()

Hi Babu,

On 12/22/2022 3:31 PM, Babu Moger wrote:
> 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.
>
> Signed-off-by: Babu Moger <[email protected]>

Thank you.

Reviewed-by: Reinette Chatre <[email protected]>

Reinette

2023-01-05 15:55:36

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH v10 07/13] x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()


On 1/4/23 18:28, Reinette Chatre wrote:
> Hi Babu,
>
> On 12/22/2022 3:31 PM, Babu Moger wrote:
>> 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.
>>
>> Signed-off-by: Babu Moger <[email protected]>
> Thank you.
>
> Reviewed-by: Reinette Chatre <[email protected]>

Thank You

Babu

>
> Reinette

--
Thanks
Babu Moger

2023-01-05 16:58:10

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH v10 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config

Hi Reinette,

On 1/4/23 18:29, Reinette Chatre wrote:
> Hi Babu,
>
> On 12/22/2022 3:31 PM, Babu Moger wrote:
>
> ...
>
>> +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;
>> +
>> + cpus_read_lock();
> Could you please elaborate why this lock is needed here as
> well as in the following patch?

Holding the cpus_read_lock() make sure that this cpu is online while doing
this operation. This code eventually sends an IPI to write the MSR on one
of the CPUs using the cpumasks. My understanding is to make sure cpumask
is stable while handling this write.  Same thing is done in
rdtgroup_schemata_write.

Thanks

Babu


>
> Reinette
>
--
Thanks
Babu Moger

2023-01-05 17:03:14

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH v10 09/13] x86/resctrl: Add interface to read mbm_total_bytes_config

Hi Reinette,

On 1/4/23 18:29, Reinette Chatre wrote:
> Hi Babu,
>
> On 12/22/2022 3:31 PM, Babu Moger wrote:
>> 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.
>>
>> Signed-off-by: Babu Moger <[email protected]>
> ...
>
>> --- 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)
> Some stray white space above.
Sure.
>
>> +
>> +/* Max event bits supported */
>> +#define MAX_EVT_CONFIG_BITS GENMASK(6, 0)
>>
>> struct rdt_fs_context {
>> struct kernfs_fs_context kfc;
> Please remove stray white space. With that:
>
> Reviewed-by: Reinette Chatre <[email protected]>

Thanks

Babu

>
> Reinette
>
>
--
Thanks
Babu Moger

2023-01-05 18:06:46

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v10 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config

Hi Babu,

On 1/5/2023 8:04 AM, Moger, Babu wrote:
> Hi Reinette,
>
> On 1/4/23 18:29, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 12/22/2022 3:31 PM, Babu Moger wrote:
>>
>> ...
>>
>>> +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;
>>> +
>>> + cpus_read_lock();
>> Could you please elaborate why this lock is needed here as
>> well as in the following patch?
>
> Holding the cpus_read_lock() make sure that this cpu is online while doing
> this operation. This code eventually sends an IPI to write the MSR on one
> of the CPUs using the cpumasks. My understanding is to make sure cpumask
> is stable while handling this write.  Same thing is done in

This flow uses smp_call_function_any() to send the IPI and update the MSR.
smp_call_function_any() itself disables preemption to protect against
CPUs going offline while attempting the update.

The domain's cpumask itself cannot change during this flow because rdtgroup_mutex
is held the entire time. This mutex is needed by the resctrl CPU online/offline
callbacks that may update the mask.

> rdtgroup_schemata_write.

Yes, rdtgroup_schemata_write uses this but please take a look at _why_ it
is using it. This was something added later as part of the pseudo-locking
code. Please see the commit message for the details that explain the usage:
80b71c340f17 ("x86/intel_rdt: Ensure a CPU remains online for the region's pseudo-locking sequence")

Could you please provide more detail if you still find that this lock is needed?
If you prefer to refer to existing code flows there are other examples
in resctrl where the domain's CPU mask is used to read/write registers without the
hotplug lock that you can use for reference:
* Even in this patch series itself, reading of the config.
* When creating a new resource group (the mkdir flow) the MSRs are written with an
initial config without hotplug lock.
* When writing to the tasks file the CPU on which task may be running receives IPI
without hotplug lock held the entire time.
* See resctrl flow of monitoring data reads.

Alternatively you may want to take a closer look at where the hotplug lock _is_ held in
resctrl to consider if those usages match this work. Understanding
why the hotplug lock is currently used should be clear with the commits associated
with their introduction because there has been a few bugs surrounding this.

Reinette

2023-01-05 20:14:23

by Moger, Babu

[permalink] [raw]
Subject: Re: [PATCH v10 11/13] x86/resctrl: Add interface to write mbm_total_bytes_config

Hi Reinette,

On 1/5/23 11:49, Reinette Chatre wrote:
> Hi Babu,
>
> On 1/5/2023 8:04 AM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 1/4/23 18:29, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 12/22/2022 3:31 PM, Babu Moger wrote:
>>>
>>> ...
>>>
>>>> +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;
>>>> +
>>>> + cpus_read_lock();
>>> Could you please elaborate why this lock is needed here as
>>> well as in the following patch?
>> Holding the cpus_read_lock() make sure that this cpu is online while doing
>> this operation. This code eventually sends an IPI to write the MSR on one
>> of the CPUs using the cpumasks. My understanding is to make sure cpumask
>> is stable while handling this write.  Same thing is done in
> This flow uses smp_call_function_any() to send the IPI and update the MSR.
> smp_call_function_any() itself disables preemption to protect against
> CPUs going offline while attempting the update.
>
> The domain's cpumask itself cannot change during this flow because rdtgroup_mutex
> is held the entire time. This mutex is needed by the resctrl CPU online/offline
> callbacks that may update the mask.
Ok
>
>> rdtgroup_schemata_write.
> Yes, rdtgroup_schemata_write uses this but please take a look at _why_ it
> is using it. This was something added later as part of the pseudo-locking
> code. Please see the commit message for the details that explain the usage:
> 80b71c340f17 ("x86/intel_rdt: Ensure a CPU remains online for the region's pseudo-locking sequence")
Ok. That make sense.
>
> Could you please provide more detail if you still find that this lock is needed?
> If you prefer to refer to existing code flows there are other examples
> in resctrl where the domain's CPU mask is used to read/write registers without the
> hotplug lock that you can use for reference:
> * Even in this patch series itself, reading of the config.
> * When creating a new resource group (the mkdir flow) the MSRs are written with an
> initial config without hotplug lock.
> * When writing to the tasks file the CPU on which task may be running receives IPI
> without hotplug lock held the entire time.
> * See resctrl flow of monitoring data reads.
>
> Alternatively you may want to take a closer look at where the hotplug lock _is_ held in
> resctrl to consider if those usages match this work. Understanding
> why the hotplug lock is currently used should be clear with the commits associated
> with their introduction because there has been a few bugs surrounding this.

This is overlook from my side looking at the code in rdtgroup_schemata_write.

I was concerned about domains cpu_mask being changed while doing the IPI.
Looks like that is safe with rdtgroup_mutex.

I will remove the cpus_read_lock() from here and following patch. Will run
few tests. Will send the updated patches soon.

Thanks for the explanation.

Thanks

Babu