[PATCH v2 0/7] x86/resctrl : Support L3 Smart Data Cache Injection Allocation Enforcement (SDCIAE)

[PATCH v2 0/7] x86/resctrl : Support L3 Smart Data Cache Injection Allocation Enforcement (SDCIAE)

[Babu Moger]

This series adds the support for L3 Smart Data Cache Injection Allocation
Enforcement (SDCIAE) to resctrl infrastructure. 

Upcoming AMD hardware implements Smart Data Cache Injection (SDCI).
Smart Data Cache Injection (SDCI) is a mechanism that enables direct
insertion of data from I/O devices into the L3 cache. By directly caching
data from I/O devices rather than first storing the I/O data in DRAM, SDCI
reduces demands on DRAM bandwidth and reduces latency to the processor
consuming the I/O data.

The SDCIAE (SDCI Allocation Enforcement) PQE feature allows system software
to control the portion of the L3 cache used for SDCI devices.

When enabled, SDCIAE forces all SDCI lines to be placed into the L3 cache
partitions identified by the highest-supported L3_MASK_n register, where n
is the maximum supported CLOSID.

The feature details are documented in the APM listed below [1].
[1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
Publication # 24593 Revision 3.41 section 19.4.7 L3 Smart Data Cache
Injection Allocation Enforcement (SDCIAE)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537

The feature requires linux support of TPH (TLP Processing Hints).
The support is available in linux kernel after the commit
48d0fd2b903e3 ("PCI/TPH: Add TPH documentation")

The patches are based on top of commit
33193f21305e39 (origin/master, origin/HEAD) Merge branch into tip/master: 'x86/tdx'

# Linux Implementation

Feature adds following interface files when the resctrl "io_alloc" feature is
supported:

/sys/fs/resctrl/info/L3/io_alloc: Reports the feature status. Feature can be
				  enabled/disabled by writing to the interface.

/sys/fs/resctrl/info/L3/io_alloc_cbm: Capacity Bit Masks (CBMs) available to SDCI
				      supported IO devices. CBM can be configured
				      by writing to the interface in the following
				      format::
# Examples

a. Check if io_alloc feature is available
	#mount -t resctrl resctrl /sys/fs/resctrl/

	# cat /sys/fs/resctrl/info/L3/io_alloc
	0

b. Enable the io_alloc feature. 

	# echo 1 > /sys/fs/resctrl/info/L3/io_alloc 

c. Check the CBM values for the io_alloc feature.

	# cat /sys/fs/resctrl/info/L3/io_alloc_cbm 
	L3:0=ffff;1=ffff

d. Change the CBM value for the domain 1:
	# echo L3:1=FF > /sys/fs/resctrl/info/L3/io_alloc_cbm

	# cat /sys/fs/resctrl/info/L3/io_alloc_cbm 
	L3:0=ffff;1=00ff

d. Disable io_alloc feature and exit.

	# echo 0 > /sys/fs/resctrl/info/L3/io_alloc 
	#umount /sys/fs/resctrl/

---
v2: Added dependancy on X86_FEATURE_CAT_L3
    Removed the "" in CPU feature definition.

    Changed sdciae_capable to io_alloc_capable to make it as generic feature.
    Moved io_alloc_capable field in struct resctrl_cache.

    Changed the name of few arch functions similar to ABMC series.
    resctrl_arch_get_io_alloc_enabled()
    resctrl_arch_io_alloc_enable()

    Renamed the feature to "io_alloc".
    Added generic texts for the feature in commit log and resctrl.rst doc.
    Added resctrl_io_alloc_init_cat() to initialize io_alloc to default values
    when enabled.
    Fixed io_alloc interface to show only on L3 resource.
    Added the locks while processing io_alloc CBMs.

v1: https://lore.kernel.org/lkml/cover.1723824984.git.babu.moger@amd.com/


Babu Moger (7):
  x86/cpufeatures: Add support for L3 Smart Data Cache Injection
    Allocation Enforcement
  x86/resctrl: Add SDCIAE feature in the command line options
  x86/resctrl: Detect Smart Data Cache Injection Allocation Enforcement
  x86/resctrl: Implement "io_alloc" enable/disable handlers
  x86/resctrl: Add interface to enable/disable io_alloc feature
  x86/resctrl: Introduce interface to display io_alloc CBMs
  x86/resctrl: Introduce interface to modify io_alloc Capacity Bit Masks

 .../admin-guide/kernel-parameters.txt         |   2 +-
 Documentation/arch/x86/resctrl.rst            |  35 ++
 arch/x86/include/asm/cpufeatures.h            |   1 +
 arch/x86/include/asm/msr-index.h              |   1 +
 arch/x86/kernel/cpu/cpuid-deps.c              |   1 +
 arch/x86/kernel/cpu/resctrl/core.c            |  13 +
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c     |   4 +-
 arch/x86/kernel/cpu/resctrl/internal.h        |  12 +
 arch/x86/kernel/cpu/resctrl/rdtgroup.c        | 311 ++++++++++++++++++
 arch/x86/kernel/cpu/scattered.c               |   1 +
 include/linux/resctrl.h                       |  13 +
 11 files changed, 391 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH v2 1/7] x86/cpufeatures: Add support for L3 Smart Data Cache Injection Allocation Enforcement

[Babu Moger]

Smart Data Cache Injection (SDCI) is a mechanism that enables direct
insertion of data from I/O devices into the L3 cache. By directly caching
data from I/O devices rather than first storing the I/O data in DRAM,
SDCI reduces demands on DRAM bandwidth and reduces latency to the processor
consuming the I/O data.

The SDCIAE (SDCI Allocation Enforcement) PQE feature allows system software
to control the portion of the L3 cache used for SDCI.

When enabled, SDCIAE forces all SDCI lines to be placed into the L3 cache
partitions identified by the highest-supported L3_MASK_n register, where n
is the maximum supported CLOSID.

Add CPUID feature bit that can be used to configure SDCIAE.

The feature details are documented in APM listed below [1].
[1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
Publication # 24593 Revision 3.41 section 19.4.7 L3 Smart Data Cache
Injection Allocation Enforcement (SDCIAE)

Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: Added dependancy on X86_FEATURE_CAT_L3
    Removed the "" in CPU feature definition.
    Minor text changes.
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kernel/cpu/cpuid-deps.c   | 1 +
 arch/x86/kernel/cpu/scattered.c    | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 935d44cc4db8..57aa3c26fe4b 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -481,6 +481,7 @@
 #define X86_FEATURE_AMD_FAST_CPPC	(21*32 + 5) /* Fast CPPC */
 #define X86_FEATURE_AMD_HETEROGENEOUS_CORES (21*32 + 6) /* Heterogeneous Core Topology */
 #define X86_FEATURE_AMD_WORKLOAD_CLASS	(21*32 + 7) /* Workload Classification */
+#define X86_FEATURE_SDCIAE		(21*32 + 8) /* L3 Smart Data Cache Injection Allocation Enforcement */
 
 /*
  * BUG word(s)
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 8bd84114c2d9..8185521ce854 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -70,6 +70,7 @@ static const struct cpuid_dep cpuid_deps[] = {
 	{ 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_SDCIAE,			X86_FEATURE_CAT_L3    },
 	{ 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 16f3ca30626a..d18a7ce16388 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -49,6 +49,7 @@ static const struct cpuid_bit cpuid_bits[] = {
 	{ 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_SDCIAE,			CPUID_EBX,  6, 0x80000020, 0 },
 	{ X86_FEATURE_AMD_WORKLOAD_CLASS,	CPUID_EAX, 22, 0x80000021, 0 },
 	{ X86_FEATURE_PERFMON_V2,		CPUID_EAX,  0, 0x80000022, 0 },
 	{ X86_FEATURE_AMD_LBR_V2,		CPUID_EAX,  1, 0x80000022, 0 },
-- 
2.34.1

[PATCH v2 2/7] x86/resctrl: Add SDCIAE feature in the command line options

[Babu Moger]

Add the command line options to enable or disable the new resctrl feature
L3 Smart Data Cache Injection Allocation Enforcement (SDCIAE).

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: No changes.
---
 Documentation/admin-guide/kernel-parameters.txt | 2 +-
 arch/x86/kernel/cpu/resctrl/core.c              | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 7d427d0a4a1a..8e8efccc1d0a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5854,7 +5854,7 @@
 	rdt=		[HW,X86,RDT]
 			Turn on/off individual RDT features. List is:
 			cmt, mbmtotal, mbmlocal, l3cat, l3cdp, l2cat, l2cdp,
-			mba, smba, bmec.
+			mba, smba, bmec, sdciae.
 			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 3d1735ed8d1f..c2450cd52511 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -811,6 +811,7 @@ enum {
 	RDT_FLAG_MBA,
 	RDT_FLAG_SMBA,
 	RDT_FLAG_BMEC,
+	RDT_FLAG_SDCIAE,
 };
 
 #define RDT_OPT(idx, n, f)	\
@@ -836,6 +837,7 @@ static struct rdt_options rdt_options[]  __initdata = {
 	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),
+	RDT_OPT(RDT_FLAG_SDCIAE,    "sdciae",	X86_FEATURE_SDCIAE),
 };
 #define NUM_RDT_OPTIONS ARRAY_SIZE(rdt_options)
 
-- 
2.34.1

[PATCH v2 3/7] x86/resctrl: Detect Smart Data Cache Injection Allocation Enforcement

[Babu Moger]

Introduce io_alloc_capable in struct resctrl_cache to detect SDCIAE
(L3 Smart Data Cache Injection Allocation Enforcement) feature.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: Changed sdciae_capable to io_alloc_capable to make it generic feature.
    Also moved the io_alloc_capable in struct resctrl_cache.
---
 arch/x86/kernel/cpu/resctrl/core.c | 7 +++++++
 include/linux/resctrl.h            | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index c2450cd52511..39e110033d96 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -306,6 +306,11 @@ static void rdt_get_cdp_config(int level)
 	rdt_resources_all[level].r_resctrl.cdp_capable = true;
 }
 
+static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
+{
+	r->cache.io_alloc_capable = true;
+}
+
 static void rdt_get_cdp_l3_config(void)
 {
 	rdt_get_cdp_config(RDT_RESOURCE_L3);
@@ -931,6 +936,8 @@ static __init bool get_rdt_alloc_resources(void)
 		rdt_get_cache_alloc_cfg(1, r);
 		if (rdt_cpu_has(X86_FEATURE_CDP_L3))
 			rdt_get_cdp_l3_config();
+		if (rdt_cpu_has(X86_FEATURE_SDCIAE))
+			rdt_get_sdciae_alloc_cfg(r);
 		ret = true;
 	}
 	if (rdt_cpu_has(X86_FEATURE_CAT_L2)) {
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index d94abba1c716..5837acff7442 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -129,6 +129,8 @@ struct rdt_mon_domain {
  * @arch_has_sparse_bitmasks:	True if a bitmask like f00f is valid.
  * @arch_has_per_cpu_cfg:	True if QOS_CFG register for this cache
  *				level has CPU scope.
+ * @io_alloc_capable:	Smart Data Cache Injection Allocation Enforcement
+ *			capable (SDCIAE).
  */
 struct resctrl_cache {
 	unsigned int	cbm_len;
@@ -136,6 +138,7 @@ struct resctrl_cache {
 	unsigned int	shareable_bits;
 	bool		arch_has_sparse_bitmasks;
 	bool		arch_has_per_cpu_cfg;
+	bool		io_alloc_capable;
 };
 
 /**
@@ -202,6 +205,7 @@ enum resctrl_scope {
  * @evt_list:		List of monitoring events
  * @fflags:		flags to choose base and info files
  * @cdp_capable:	Is the CDP feature available on this resource
+ * @sdciae_capable:	Is SDCIAE feature available on this resource
  */
 struct rdt_resource {
 	int			rid;
-- 
2.34.1

[PATCH v2 4/7] x86/resctrl: Implement "io_alloc" enable/disable handlers

[Babu Moger]

Introduce architecture-specific2yy handlers to manage the detection and
enabling/disabling of this feature.

SDCIAE feature can be enabled by setting bit 1 in MSR L3_QOS_EXT_CFG.
When the state of SDCIAE is modified, the updated value must be applied
across all logical processors within the QOS Domain. By default, the
io_alloc feature is turned off.

The SDCIAE feature details are available in APM listed below [1].
[1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
Publication # 24593 Revision 3.41 section 19.4.7 L3 Smart Data Cache
Injection Allocation Enforcement (SDCIAE)

Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: Renamed the functions to simplify the code.
    Renamed sdciae_capable to io_alloc_capable.

    Changed the name of few arch functions similar to ABMC series.
    resctrl_arch_get_io_alloc_enabled()
    resctrl_arch_io_alloc_enable()
---
 arch/x86/include/asm/msr-index.h       |  1 +
 arch/x86/kernel/cpu/resctrl/internal.h | 10 ++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 34 ++++++++++++++++++++++++++
 include/linux/resctrl.h                |  9 +++++++
 4 files changed, 54 insertions(+)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 3f3e2bc99162..360c52a62da9 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -1196,6 +1196,7 @@
 /* - AMD: */
 #define MSR_IA32_MBA_BW_BASE		0xc0000200
 #define MSR_IA32_SMBA_BW_BASE		0xc0000280
+#define MSR_IA32_L3_QOS_EXT_CFG		0xc00003ff
 #define MSR_IA32_EVT_CFG_BASE		0xc0000400
 
 /* AMD-V MSRs */
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 20c898f09b7e..dff3354c2282 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -56,6 +56,9 @@
 /* Max event bits supported */
 #define MAX_EVT_CONFIG_BITS		GENMASK(6, 0)
 
+/* Setting bit 1 in L3_QOS_EXT_CFG enables the SDCIAE feature. */
+#define SDCIAE_ENABLE_BIT		1
+
 /**
  * cpumask_any_housekeeping() - Choose any CPU in @mask, preferring those that
  *			        aren't marked nohz_full
@@ -479,6 +482,7 @@ struct rdt_parse_data {
  * @mbm_cfg_mask:	Bandwidth sources that can be tracked when Bandwidth
  *			Monitoring Event Configuration (BMEC) is supported.
  * @cdp_enabled:	CDP state of this resource
+ * @sdciae_enabled:	SDCIAE feature is enabled
  *
  * Members of this structure are either private to the architecture
  * e.g. mbm_width, or accessed via helpers that provide abstraction. e.g.
@@ -493,6 +497,7 @@ struct rdt_hw_resource {
 	unsigned int		mbm_width;
 	unsigned int		mbm_cfg_mask;
 	bool			cdp_enabled;
+	bool			sdciae_enabled;
 };
 
 static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r)
@@ -539,6 +544,11 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable);
 
 void arch_mon_domain_online(struct rdt_resource *r, struct rdt_mon_domain *d);
 
+static inline bool resctrl_arch_get_io_alloc_enabled(enum resctrl_res_level l)
+{
+	return rdt_resources_all[l].sdciae_enabled;
+}
+
 /*
  * To return the common struct rdt_resource, which is contained in struct
  * rdt_hw_resource, walk the resctrl member of struct rdt_hw_resource.
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 6419e04d8a7b..398f241b65d5 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1798,6 +1798,40 @@ static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of,
 	return ret ?: nbytes;
 }
 
+static void resctrl_sdciae_set_one_amd(void *arg)
+{
+	bool *enable = arg;
+
+	if (*enable)
+		msr_set_bit(MSR_IA32_L3_QOS_EXT_CFG, SDCIAE_ENABLE_BIT);
+	else
+		msr_clear_bit(MSR_IA32_L3_QOS_EXT_CFG, SDCIAE_ENABLE_BIT);
+}
+
+static int _resctrl_io_alloc_enable(struct rdt_resource *r, bool enable)
+{
+	struct rdt_ctrl_domain *d;
+
+	/* Update L3_QOS_EXT_CFG MSR on all the CPUs in all domains*/
+	list_for_each_entry(d, &r->ctrl_domains, hdr.list)
+		on_each_cpu_mask(&d->hdr.cpu_mask, resctrl_sdciae_set_one_amd, &enable, 1);
+
+	return 0;
+}
+
+int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
+{
+	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
+
+	if (hw_res->r_resctrl.cache.io_alloc_capable &&
+	    hw_res->sdciae_enabled != enable) {
+		_resctrl_io_alloc_enable(r, enable);
+		hw_res->sdciae_enabled = enable;
+	}
+
+	return 0;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 5837acff7442..8c66aeac4768 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -344,6 +344,15 @@ void resctrl_arch_reset_rmid(struct rdt_resource *r, struct rdt_mon_domain *d,
  */
 void resctrl_arch_reset_rmid_all(struct rdt_resource *r, struct rdt_mon_domain *d);
 
+/**
+ * resctrl_arch_io_alloc_enable() - Enable/disable io_alloc feature.
+ * @r:		The resctrl resource.
+ * @enable:	Enable (1) or disable (0) the feature
+ *
+ * This can be called from any CPU.
+ */
+int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable);
+
 extern unsigned int resctrl_rmid_realloc_threshold;
 extern unsigned int resctrl_rmid_realloc_limit;
 
-- 
2.34.1

[PATCH v2 5/7] x86/resctrl: Add interface to enable/disable io_alloc feature

[Babu Moger]

The io_alloc feature in resctrl enables system software to configure
the portion of the L3 cache allocated for I/O traffic.

Smart Data Cache Injection (SDCI) is a mechanism that allows direct
insertion of data from I/O devices into the L3 cache. By caching I/O
data directly in the L3 cache, instead of writing it to DRAM first,
SDCI reduces DRAM bandwidth usage and lowers latency for the processor
consuming the I/O data.

When enabled, SDCIAE forces all SDCI lines to be placed into the L3 cache
partitions identified by the highest-supported L3_MASK_n register as
reported by CPUID Fn0000_0010_EDX_x1.MAX_COS. For example, if MAX_COS=15,
SDCI lines will be allocated into the L3 cache partitions determined by
the bitmask in the L3_MASK_15 register.

Introduce interface to enable/disable "io_alloc" feature on user input.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: Renamed the feature to "io_alloc".
    Added generic texts for the feature in commit log and resctrl.rst doc.
    Added resctrl_io_alloc_init_cat() to initialize io_alloc to default
    values when enabled.
    Fixed io_alloc show functinality to display only on L3 resource.
---
 Documentation/arch/x86/resctrl.rst     |  27 ++++++
 arch/x86/kernel/cpu/resctrl/core.c     |   2 +
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 118 +++++++++++++++++++++++++
 3 files changed, 147 insertions(+)

diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index 6768fc1fad16..52679175ee14 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -135,6 +135,33 @@ related to allocation:
 			"1":
 			      Non-contiguous 1s value in CBM is supported.
 
+"io_alloc":
+		The "io_alloc" feature in resctrl enables system software to
+		configure the portion of the L3 cache allocated for I/O traffic.
+
+		Smart Data Cache Injection (SDCI) is a mechanism that allows
+		direct insertion of data from I/O devices into the L3 cache.
+		By caching I/O data directly in the L3 cache, instead of writing
+		it to DRAM first, SDCI reduces DRAM bandwidth usage and lowers
+		latency for the processor consuming the I/O data.
+
+		When enabled the feature forces all SDCI lines to be placed
+		into the L3 cache partitions identified by the highest-supported
+		CLOSID (num_closids-1). This CLOSID will not be available to the
+		resctrl group.
+
+			"0":
+			    I/O device L3 cache control is not enabled.
+			"1":
+			    I/O device L3 cache control is enabled, allowing users
+			    to manage the portions of the L3 cache allocated for
+			    the I/O device.
+
+		Feature can be enabled/disabled by writing to the interface.
+		Example::
+
+			# echo 1 > /sys/fs/resctrl/info/L3/io_alloc
+
 Memory bandwidth(MB) subdirectory contains the following files
 with respect to allocation:
 
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 39e110033d96..066a7997eaf1 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -309,6 +309,8 @@ static void rdt_get_cdp_config(int level)
 static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
 {
 	r->cache.io_alloc_capable = true;
+	resctrl_file_fflags_init("io_alloc",
+				 RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE);
 }
 
 static void rdt_get_cdp_l3_config(void)
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 398f241b65d5..e30731ce9335 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -62,6 +62,7 @@ static char last_cmd_status_buf[512];
 
 static int rdtgroup_setup_root(struct rdt_fs_context *ctx);
 static void rdtgroup_destroy_root(void);
+static int rdtgroup_init_cat(struct resctrl_schema *s, u32 closid);
 
 struct dentry *debugfs_resctrl;
 
@@ -180,6 +181,25 @@ void closid_free(int closid)
 	__set_bit(closid, &closid_free_map);
 }
 
+/*
+ * io_alloc (SDCIAE) feature uses max CLOSID to route the SDCI traffic.
+ * Get the max CLOSID number
+ */
+static u32 resctrl_io_alloc_closid_get(struct rdt_resource *r)
+{
+	return resctrl_arch_get_num_closid(r) - 1;
+}
+
+static int resctrl_io_alloc_closid_alloc(struct rdt_resource *r)
+{
+	u32 io_alloc_closid = resctrl_io_alloc_closid_get(r);
+
+	if (__test_and_clear_bit(io_alloc_closid, &closid_free_map))
+		return io_alloc_closid;
+	else
+		return -ENOSPC;
+}
+
 /**
  * closid_allocated - test if provided closid is in use
  * @closid: closid to be tested
@@ -1832,6 +1852,97 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
 	return 0;
 }
 
+static int resctrl_io_alloc_show(struct kernfs_open_file *of,
+				 struct seq_file *seq, void *v)
+{
+	struct resctrl_schema *s = of->kn->parent->priv;
+	struct rdt_resource *r = s->res;
+
+	seq_printf(seq, "%x\n", resctrl_arch_get_io_alloc_enabled(r->rid));
+	return 0;
+}
+
+/*
+ * Initialize the io_alloc feature default when enabled
+ */
+static int resctrl_io_alloc_init_cat(struct rdt_resource *r, u32 closid)
+{
+	struct resctrl_schema *s;
+	int ret = 0;
+
+	rdt_staged_configs_clear();
+
+	list_for_each_entry(s, &resctrl_schema_all, list) {
+		r = s->res;
+		if (r->rid == RDT_RESOURCE_L3) {
+			ret = rdtgroup_init_cat(s, closid);
+			if (ret < 0)
+				goto out_init_cat;
+
+			ret = resctrl_arch_update_domains(r, closid);
+			if (ret < 0)
+				goto out_init_cat;
+		}
+	}
+
+out_init_cat:
+	if (ret)
+		rdt_last_cmd_puts("Failed to initialize io_alloc allocations\n");
+
+	rdt_staged_configs_clear();
+	return ret;
+}
+
+static ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
+				      size_t nbytes, loff_t off)
+{
+	struct resctrl_schema *s = of->kn->parent->priv;
+	struct rdt_resource *r = s->res;
+	u32 io_alloc_closid;
+	bool enable;
+	int ret;
+
+	if (!r->cache.io_alloc_capable)
+		return -EINVAL;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret)
+		return ret;
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+
+	rdt_last_cmd_clear();
+
+	io_alloc_closid = resctrl_io_alloc_closid_get(r);
+
+	if (resctrl_arch_get_io_alloc_enabled(r->rid) != enable) {
+		if (enable) {
+			ret = resctrl_io_alloc_closid_alloc(r);
+			if (ret < 0) {
+				rdt_last_cmd_puts("io_alloc CLOSID is not available\n");
+				goto out_io_alloc;
+			}
+			ret = resctrl_io_alloc_init_cat(r, io_alloc_closid);
+			if (ret) {
+				closid_free(io_alloc_closid);
+				goto out_io_alloc;
+			}
+
+		} else {
+			closid_free(io_alloc_closid);
+		}
+
+		ret = resctrl_arch_io_alloc_enable(r, enable);
+	}
+
+out_io_alloc:
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+
+	return ret ?: nbytes;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
@@ -1984,6 +2095,13 @@ static struct rftype res_common_files[] = {
 		.seq_show	= rdtgroup_schemata_show,
 		.fflags		= RFTYPE_CTRL_BASE,
 	},
+	{
+		.name		= "io_alloc",
+		.mode		= 0644,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= resctrl_io_alloc_show,
+		.write		= resctrl_io_alloc_write,
+	},
 	{
 		.name		= "mba_MBps_event",
 		.mode		= 0644,
-- 
2.34.1

[PATCH v2 6/7] x86/resctrl: Introduce interface to display io_alloc CBMs

[Babu Moger]

The io_alloc feature in resctrl enables system software to configure
the portion of the L3 cache allocated for I/O traffic.

Add the interface to display CBMs (Capacity Bit Mask) of io_alloc
feature.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: Fixed to display only on L3 resources.
    Added the locks while processing.
    Rename the displat to io_alloc_cbm (from sdciae_cmd).
---
 arch/x86/kernel/cpu/resctrl/core.c        |  2 ++
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  2 +-
 arch/x86/kernel/cpu/resctrl/internal.h    |  1 +
 arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 33 +++++++++++++++++++++++
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 066a7997eaf1..f1ac7af7b366 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -311,6 +311,8 @@ static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
 	r->cache.io_alloc_capable = true;
 	resctrl_file_fflags_init("io_alloc",
 				 RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE);
+	resctrl_file_fflags_init("io_alloc_cbm",
+				 RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE);
 }
 
 static void rdt_get_cdp_l3_config(void)
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 536351159cc2..d272dea43924 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -444,7 +444,7 @@ u32 resctrl_arch_get_config(struct rdt_resource *r, struct rdt_ctrl_domain *d,
 	return hw_dom->ctrl_val[idx];
 }
 
-static void show_doms(struct seq_file *s, struct resctrl_schema *schema, int closid)
+void show_doms(struct seq_file *s, struct resctrl_schema *schema, int closid)
 {
 	struct rdt_resource *r = schema->res;
 	struct rdt_ctrl_domain *dom;
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index dff3354c2282..1550cb468b8e 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -668,4 +668,5 @@ void resctrl_file_fflags_init(const char *config, unsigned long fflags);
 void rdt_staged_configs_clear(void);
 bool closid_allocated(unsigned int closid);
 int resctrl_find_cleanest_closid(void);
+void show_doms(struct seq_file *s, struct resctrl_schema *schema, int closid);
 #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index e30731ce9335..4d6b83d18790 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1943,6 +1943,33 @@ static ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
 	return ret ?: nbytes;
 }
 
+static int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of,
+				     struct seq_file *seq, void *v)
+{
+	struct resctrl_schema *s = of->kn->parent->priv;
+	struct rdt_resource *r = s->res;
+	u32 io_alloc_closid;
+	int ret = 0;
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+
+	if (!resctrl_arch_get_io_alloc_enabled(r->rid)) {
+		rdt_last_cmd_puts("io_alloc feature is not enabled\n");
+		ret = -EINVAL;
+		goto cbm_show_out;
+	}
+
+	io_alloc_closid = resctrl_io_alloc_closid_get(r);
+
+	show_doms(seq, s, io_alloc_closid);
+
+cbm_show_out:
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+	return ret;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
@@ -2102,6 +2129,12 @@ static struct rftype res_common_files[] = {
 		.seq_show	= resctrl_io_alloc_show,
 		.write		= resctrl_io_alloc_write,
 	},
+	{
+		.name		= "io_alloc_cbm",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= resctrl_io_alloc_cbm_show,
+	},
 	{
 		.name		= "mba_MBps_event",
 		.mode		= 0644,
-- 
2.34.1

[PATCH v2 7/7] x86/resctrl: Introduce interface to modify io_alloc Capacity Bit Masks

[Babu Moger]

"io_alloc" feature is a mechanism that enables direct insertion of data
from I/O devices into the L3 cache. By directly caching data from I/O
devices rather than first storing the I/O data in DRAM, SDCI reduces
demands on DRAM bandwidth and reduces latency to the processor consuming
the I/O data.

The SDCIAE (SDCI Allocation Enforcement) PQE feature allows system
software to limit the portion of the L3 cache used for SDCI.

Provide the interface to modify io_alloc CBMs (Capacity Bit Masks).

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v2: Added more generic text in documentation.
---
 Documentation/arch/x86/resctrl.rst        |   8 ++
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c |   2 +-
 arch/x86/kernel/cpu/resctrl/internal.h    |   1 +
 arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 128 +++++++++++++++++++++-
 4 files changed, 137 insertions(+), 2 deletions(-)

diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index 52679175ee14..da74356adcc2 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -162,6 +162,14 @@ related to allocation:
 
 			# echo 1 > /sys/fs/resctrl/info/L3/io_alloc
 
+"io_alloc_cbm":
+		Capacity Bit Masks (CBMs) available to supported IO devices which
+		can directly insert cache lines in L3 which can help to reduce the
+		latency. CBM can be configured by writing to the interface in the
+		following format::
+
+			L3:<cache_id0>=<cbm>;<cache_id1>=<cbm>;...
+
 Memory bandwidth(MB) subdirectory contains the following files
 with respect to allocation:
 
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index d272dea43924..4dfee0436c1c 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -102,7 +102,7 @@ int parse_bw(struct rdt_parse_data *data, struct resctrl_schema *s,
  * requires at least two bits set.
  * AMD allows non-contiguous bitmasks.
  */
-static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
+bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
 {
 	unsigned long first_bit, zero_bit, val;
 	unsigned int cbm_len = r->cache.cbm_len;
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 1550cb468b8e..5f7236437cb5 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -669,4 +669,5 @@ void rdt_staged_configs_clear(void);
 bool closid_allocated(unsigned int closid);
 int resctrl_find_cleanest_closid(void);
 void show_doms(struct seq_file *s, struct resctrl_schema *schema, int closid);
+bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r);
 #endif /* _ASM_X86_RESCTRL_INTERNAL_H */
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 4d6b83d18790..c2b4221ea469 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1970,6 +1970,131 @@ static int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of,
 	return ret;
 }
 
+/*
+ * Read the CBM and check the validity. Make sure CBM is not shared
+ * with any other exclusive resctrl groups.
+ */
+static int resctrl_io_alloc_parse_cbm(char *buf, struct resctrl_schema *s,
+				      struct rdt_ctrl_domain *d)
+{
+	struct resctrl_staged_config *cfg;
+	struct rdt_resource *r = s->res;
+	u32 io_alloc_closid;
+	u32 cbm_val;
+
+	cfg = &d->staged_config[s->conf_type];
+	if (cfg->have_new_ctrl) {
+		rdt_last_cmd_printf("Duplicate domain %d\n", d->hdr.id);
+		return -EINVAL;
+	}
+
+	if (!cbm_validate(buf, &cbm_val, r))
+		return -EINVAL;
+
+	/*
+	 * The CBM may not overlap with other exclusive group.
+	 */
+	io_alloc_closid = resctrl_io_alloc_closid_get(r);
+	if (rdtgroup_cbm_overlaps(s, d, cbm_val, io_alloc_closid, true)) {
+		rdt_last_cmd_puts("Overlaps with exclusive group\n");
+		return -EINVAL;
+	}
+
+	cfg->new_ctrl = cbm_val;
+	cfg->have_new_ctrl = true;
+
+	return 0;
+}
+
+static int resctrl_io_alloc_parse_line(char *line,  struct rdt_resource *r,
+				       struct resctrl_schema *s)
+{
+	struct rdt_ctrl_domain *d;
+	char *dom = NULL, *id;
+	unsigned long dom_id;
+
+next:
+	if (!line || line[0] == '\0')
+		return 0;
+
+	dom = strsep(&line, ";");
+	id = strsep(&dom, "=");
+	if (!dom || kstrtoul(id, 10, &dom_id)) {
+		rdt_last_cmd_puts("Missing '=' or non-numeric domain\n");
+		return -EINVAL;
+	}
+
+	dom = strim(dom);
+	list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
+		if (d->hdr.id == dom_id) {
+			if (resctrl_io_alloc_parse_cbm(dom, s, d))
+				return -EINVAL;
+			goto next;
+		}
+	}
+	return -EINVAL;
+}
+
+static ssize_t resctrl_io_alloc_cbm_write(struct kernfs_open_file *of,
+					  char *buf, size_t nbytes, loff_t off)
+{
+	struct resctrl_schema *s = of->kn->parent->priv;
+	struct rdt_resource *r = s->res;
+	u32 io_alloc_closid;
+	char *resname;
+	int ret = 0;
+
+	/* Valid input requires a trailing newline */
+	if (nbytes == 0 || buf[nbytes - 1] != '\n')
+		return -EINVAL;
+
+	buf[nbytes - 1] = '\0';
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+
+	rdt_last_cmd_clear();
+	rdt_staged_configs_clear();
+
+	if (!resctrl_arch_get_io_alloc_enabled(r->rid)) {
+		rdt_last_cmd_puts("io_alloc feature is not enabled\n");
+		ret = -EINVAL;
+		goto cbm_write_out;
+	}
+
+	resname = strim(strsep(&buf, ":"));
+	if (!buf) {
+		rdt_last_cmd_puts("Missing ':'\n");
+		ret = -EINVAL;
+		goto cbm_write_out;
+	}
+
+	if (strcmp(resname, "L3")) {
+		rdt_last_cmd_printf("Unsupported resource name '%s'\n", resname);
+		ret = -EINVAL;
+		goto cbm_write_out;
+	}
+
+	if (buf[0] == '\0') {
+		rdt_last_cmd_printf("Missing '%s' value\n", resname);
+		ret = -EINVAL;
+		goto cbm_write_out;
+	}
+
+	ret = resctrl_io_alloc_parse_line(buf, r, s);
+	if (ret)
+		goto cbm_write_out;
+
+	io_alloc_closid = resctrl_io_alloc_closid_get(r);
+	ret = resctrl_arch_update_domains(r, io_alloc_closid);
+
+cbm_write_out:
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+
+	return ret ?: nbytes;
+}
+
 /* rdtgroup information files for one cache resource. */
 static struct rftype res_common_files[] = {
 	{
@@ -2131,9 +2256,10 @@ static struct rftype res_common_files[] = {
 	},
 	{
 		.name		= "io_alloc_cbm",
-		.mode		= 0444,
+		.mode		= 0644,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= resctrl_io_alloc_cbm_show,
+		.write		= resctrl_io_alloc_cbm_write,
 	},
 	{
 		.name		= "mba_MBps_event",
-- 
2.34.1

RE: [PATCH v2 0/7] x86/resctrl : Support L3 Smart Data Cache Injection Allocation Enforcement (SDCIAE)

[Luck, Tony]

I don't have an AMD system, so I added a couple of hacks to the code to pretend I did.
My hacks might have missed something, so the below test may not fail for you.

My test:

# echo 1 > info/L3/io_alloc
# cat info/L3/bit_usage

This gave me a console splat starting with:

[  163.801078] invalid mode for closid 14

That happened because in rdt_bit_usage_show()

                for (i = 0; i < closids_supported(); i++) {
                        if (!closid_allocated(i))
                                continue;
                        ctrl_val = resctrl_arch_get_config(r, dom, i,
                                                           s->conf_type);
                        mode = rdtgroup_mode_by_closid(i);

CLOSID 14 is my highest. It's supported, and allocated.

rdtgroup_mode_by_closid() searches rdt_all_groups to find
one using CLOSID 14, but there isn't one. So it returns
RDT_NUM_MODES

Maybe RDT_NUM_MODES isn't invalid in the switch()
Looks like it should be same action as RDT_MODE_SHAREABLE?

-Tony

RE: [PATCH v2 5/7] x86/resctrl: Add interface to enable/disable io_alloc feature

[Luck, Tony]

> static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
>  {
>       r->cache.io_alloc_capable = true;
> +     resctrl_file_fflags_init("io_alloc",
> +                              RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE);
>  }

I think those fflags will make this file appear in all info cache directories
(L2 and L3).

Presumably you only want this file (and "io_alloc_cbm" added by next
patch) in the L3 directory.

-Tony

Re: [PATCH v2 5/7] x86/resctrl: Add interface to enable/disable io_alloc feature

[Reinette Chatre]

Hi Tony,

On 12/18/24 3:34 PM, Luck, Tony wrote:
>> static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
>>  {
>>       r->cache.io_alloc_capable = true;
>> +     resctrl_file_fflags_init("io_alloc",
>> +                              RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE);
>>  }
> 
> I think those fflags will make this file appear in all info cache directories
> (L2 and L3).
> 
> Presumably you only want this file (and "io_alloc_cbm" added by next
> patch) in the L3 directory.

Could you please elaborate why this file should only be in L3 directory? I do not see
the problem with having it in L2. "io_alloc" communicates to user space if this I/O alloc
feature is supported and does so with its content as opposed to its existence. For L2
it will indicate that the feature is not supported.

Reinette

Re: [PATCH v2 3/7] x86/resctrl: Detect Smart Data Cache Injection Allocation Enforcement

[Reinette Chatre]

Hi Babu,

On 12/18/24 1:37 PM, Babu Moger wrote:
> Introduce io_alloc_capable in struct resctrl_cache to detect SDCIAE
> (L3 Smart Data Cache Injection Allocation Enforcement) feature.

Please distinguish clearly between the resctrl feature ("io_alloc_capable")
and the architecture specific feature (SDCIAE) that backs it. This is similar
to what you have done for ABMC and makes the work much easier to understand.
When the resctrl and arch feature is used interchangeably it becomes confusing.

> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> v2: Changed sdciae_capable to io_alloc_capable to make it generic feature.
>     Also moved the io_alloc_capable in struct resctrl_cache.
> ---
>  arch/x86/kernel/cpu/resctrl/core.c | 7 +++++++
>  include/linux/resctrl.h            | 4 ++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index c2450cd52511..39e110033d96 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -306,6 +306,11 @@ static void rdt_get_cdp_config(int level)
>  	rdt_resources_all[level].r_resctrl.cdp_capable = true;
>  }
>  
> +static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
> +{
> +	r->cache.io_alloc_capable = true;
> +}

rdt_get_sdciae_alloc_cfg() looks like one that should have "set" in its name, not "get".
This also does not seem architecture specific so "sdciae" should not be in the name.
rdt_set_io_alloc_capable()?

> +
>  static void rdt_get_cdp_l3_config(void)
>  {
>  	rdt_get_cdp_config(RDT_RESOURCE_L3);
> @@ -931,6 +936,8 @@ static __init bool get_rdt_alloc_resources(void)
>  		rdt_get_cache_alloc_cfg(1, r);
>  		if (rdt_cpu_has(X86_FEATURE_CDP_L3))
>  			rdt_get_cdp_l3_config();
> +		if (rdt_cpu_has(X86_FEATURE_SDCIAE))
> +			rdt_get_sdciae_alloc_cfg(r);
>  		ret = true;
>  	}
>  	if (rdt_cpu_has(X86_FEATURE_CAT_L2)) {
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index d94abba1c716..5837acff7442 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -129,6 +129,8 @@ struct rdt_mon_domain {
>   * @arch_has_sparse_bitmasks:	True if a bitmask like f00f is valid.
>   * @arch_has_per_cpu_cfg:	True if QOS_CFG register for this cache
>   *				level has CPU scope.
> + * @io_alloc_capable:	Smart Data Cache Injection Allocation Enforcement
> + *			capable (SDCIAE).

Please remove arch specific text here. For example,
"True if portion of the L3 cache can be allocated for I/O traffic."

>   */
>  struct resctrl_cache {
>  	unsigned int	cbm_len;
> @@ -136,6 +138,7 @@ struct resctrl_cache {
>  	unsigned int	shareable_bits;
>  	bool		arch_has_sparse_bitmasks;
>  	bool		arch_has_per_cpu_cfg;
> +	bool		io_alloc_capable;
>  };
>  
>  /**
> @@ -202,6 +205,7 @@ enum resctrl_scope {
>   * @evt_list:		List of monitoring events
>   * @fflags:		flags to choose base and info files
>   * @cdp_capable:	Is the CDP feature available on this resource
> + * @sdciae_capable:	Is SDCIAE feature available on this resource
>   */
>  struct rdt_resource {
>  	int			rid;

Leftover from previous version?

Reinette

Re: [PATCH v2 4/7] x86/resctrl: Implement "io_alloc" enable/disable handlers

[Reinette Chatre]

Hi Babu,

On 12/18/24 1:38 PM, Babu Moger wrote:
> Introduce architecture-specific2yy handlers to manage the detection and

"architecture-specific2yy"?

> enabling/disabling of this feature.

Please add more to the context. It just jumps in with a "this feature" without
any introduction.

> 
> SDCIAE feature can be enabled by setting bit 1 in MSR L3_QOS_EXT_CFG.
> When the state of SDCIAE is modified, the updated value must be applied
> across all logical processors within the QOS Domain. By default, the
> io_alloc feature is turned off.
> 
> The SDCIAE feature details are available in APM listed below [1].
> [1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
> Publication # 24593 Revision 3.41 section 19.4.7 L3 Smart Data Cache
> Injection Allocation Enforcement (SDCIAE)
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> v2: Renamed the functions to simplify the code.
>     Renamed sdciae_capable to io_alloc_capable.
> 
>     Changed the name of few arch functions similar to ABMC series.
>     resctrl_arch_get_io_alloc_enabled()
>     resctrl_arch_io_alloc_enable()
> ---
>  arch/x86/include/asm/msr-index.h       |  1 +
>  arch/x86/kernel/cpu/resctrl/internal.h | 10 ++++++++
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 34 ++++++++++++++++++++++++++
>  include/linux/resctrl.h                |  9 +++++++
>  4 files changed, 54 insertions(+)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 3f3e2bc99162..360c52a62da9 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -1196,6 +1196,7 @@
>  /* - AMD: */
>  #define MSR_IA32_MBA_BW_BASE		0xc0000200
>  #define MSR_IA32_SMBA_BW_BASE		0xc0000280
> +#define MSR_IA32_L3_QOS_EXT_CFG		0xc00003ff
>  #define MSR_IA32_EVT_CFG_BASE		0xc0000400
>  
>  /* AMD-V MSRs */
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 20c898f09b7e..dff3354c2282 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -56,6 +56,9 @@
>  /* Max event bits supported */
>  #define MAX_EVT_CONFIG_BITS		GENMASK(6, 0)
>  
> +/* Setting bit 1 in L3_QOS_EXT_CFG enables the SDCIAE feature. */
> +#define SDCIAE_ENABLE_BIT		1
> +
>  /**
>   * cpumask_any_housekeeping() - Choose any CPU in @mask, preferring those that
>   *			        aren't marked nohz_full
> @@ -479,6 +482,7 @@ struct rdt_parse_data {
>   * @mbm_cfg_mask:	Bandwidth sources that can be tracked when Bandwidth
>   *			Monitoring Event Configuration (BMEC) is supported.
>   * @cdp_enabled:	CDP state of this resource
> + * @sdciae_enabled:	SDCIAE feature is enabled
>   *
>   * Members of this structure are either private to the architecture
>   * e.g. mbm_width, or accessed via helpers that provide abstraction. e.g.
> @@ -493,6 +497,7 @@ struct rdt_hw_resource {
>  	unsigned int		mbm_width;
>  	unsigned int		mbm_cfg_mask;
>  	bool			cdp_enabled;
> +	bool			sdciae_enabled;
>  };
>  
>  static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r)
> @@ -539,6 +544,11 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable);
>  
>  void arch_mon_domain_online(struct rdt_resource *r, struct rdt_mon_domain *d);
>  
> +static inline bool resctrl_arch_get_io_alloc_enabled(enum resctrl_res_level l)

The custom is to pass a pointer to the resource when interacting with it. Why is it
needed to pass the ID here?

> +{
> +	return rdt_resources_all[l].sdciae_enabled;
> +}
> +
>  /*
>   * To return the common struct rdt_resource, which is contained in struct
>   * rdt_hw_resource, walk the resctrl member of struct rdt_hw_resource.
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 6419e04d8a7b..398f241b65d5 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -1798,6 +1798,40 @@ static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of,
>  	return ret ?: nbytes;
>  }
>  
> +static void resctrl_sdciae_set_one_amd(void *arg)
> +{
> +	bool *enable = arg;
> +
> +	if (*enable)
> +		msr_set_bit(MSR_IA32_L3_QOS_EXT_CFG, SDCIAE_ENABLE_BIT);
> +	else
> +		msr_clear_bit(MSR_IA32_L3_QOS_EXT_CFG, SDCIAE_ENABLE_BIT);
> +}
> +
> +static int _resctrl_io_alloc_enable(struct rdt_resource *r, bool enable)
> +{
> +	struct rdt_ctrl_domain *d;
> +
> +	/* Update L3_QOS_EXT_CFG MSR on all the CPUs in all domains*/
> +	list_for_each_entry(d, &r->ctrl_domains, hdr.list)
> +		on_each_cpu_mask(&d->hdr.cpu_mask, resctrl_sdciae_set_one_amd, &enable, 1);
> +
> +	return 0;

Same comment as in V1 about this arch specific handler always returning 0 and can thus
just return void. Also the name should not reflect that it is resctrl code. One
option could be _resctrl_arch_io_alloc_enable().

> +}
> +
> +int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
> +{
> +	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
> +
> +	if (hw_res->r_resctrl.cache.io_alloc_capable &&
> +	    hw_res->sdciae_enabled != enable) {
> +		_resctrl_io_alloc_enable(r, enable);
> +		hw_res->sdciae_enabled = enable;
> +	}
> +
> +	return 0;
> +}
> +
>  /* rdtgroup information files for one cache resource. */
>  static struct rftype res_common_files[] = {
>  	{
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 5837acff7442..8c66aeac4768 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -344,6 +344,15 @@ void resctrl_arch_reset_rmid(struct rdt_resource *r, struct rdt_mon_domain *d,
>   */
>  void resctrl_arch_reset_rmid_all(struct rdt_resource *r, struct rdt_mon_domain *d);
>  
> +/**
> + * resctrl_arch_io_alloc_enable() - Enable/disable io_alloc feature.
> + * @r:		The resctrl resource.
> + * @enable:	Enable (1) or disable (0) the feature
> + *
> + * This can be called from any CPU.
> + */
> +int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable);
> +
>  extern unsigned int resctrl_rmid_realloc_threshold;
>  extern unsigned int resctrl_rmid_realloc_limit;
>  

Reinette

Re: [PATCH v2 5/7] x86/resctrl: Add interface to enable/disable io_alloc feature

[Reinette Chatre]

Hi Babu,

On 12/18/24 1:38 PM, Babu Moger wrote:
> The io_alloc feature in resctrl enables system software to configure
> the portion of the L3 cache allocated for I/O traffic.
> 

Above is about resctrl feature.

> Smart Data Cache Injection (SDCI) is a mechanism that allows direct
> insertion of data from I/O devices into the L3 cache. By caching I/O
> data directly in the L3 cache, instead of writing it to DRAM first,
> SDCI reduces DRAM bandwidth usage and lowers latency for the processor
> consuming the I/O data.
> 
> When enabled, SDCIAE forces all SDCI lines to be placed into the L3 cache
> partitions identified by the highest-supported L3_MASK_n register as
> reported by CPUID Fn0000_0010_EDX_x1.MAX_COS. For example, if MAX_COS=15,
> SDCI lines will be allocated into the L3 cache partitions determined by
> the bitmask in the L3_MASK_15 register.

Above is about AMD feature.

> 
> Introduce interface to enable/disable "io_alloc" feature on user input.

Back to resctrl feature.

Please do not jump from resctrl to AMD feature in a way that makes it seem that
they are interchangeable. To help with this you could use similar style as in
ABMC where the text flows like:

	<resctrl feature description>.

	On AMD <resctrl feature> is backed by <AMD feature> that <AMD feature details>.

> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> v2: Renamed the feature to "io_alloc".
>     Added generic texts for the feature in commit log and resctrl.rst doc.
>     Added resctrl_io_alloc_init_cat() to initialize io_alloc to default
>     values when enabled.
>     Fixed io_alloc show functinality to display only on L3 resource.
> ---
>  Documentation/arch/x86/resctrl.rst     |  27 ++++++
>  arch/x86/kernel/cpu/resctrl/core.c     |   2 +
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 118 +++++++++++++++++++++++++
>  3 files changed, 147 insertions(+)
> 
> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
> index 6768fc1fad16..52679175ee14 100644
> --- a/Documentation/arch/x86/resctrl.rst
> +++ b/Documentation/arch/x86/resctrl.rst
> @@ -135,6 +135,33 @@ related to allocation:
>  			"1":
>  			      Non-contiguous 1s value in CBM is supported.
>  
> +"io_alloc":
> +		The "io_alloc" feature in resctrl enables system software to
> +		configure the portion of the L3 cache allocated for I/O traffic.
> +
> +		Smart Data Cache Injection (SDCI) is a mechanism that allows
> +		direct insertion of data from I/O devices into the L3 cache.
> +		By caching I/O data directly in the L3 cache, instead of writing
> +		it to DRAM first, SDCI reduces DRAM bandwidth usage and lowers
> +		latency for the processor consuming the I/O data.
> +
> +		When enabled the feature forces all SDCI lines to be placed
> +		into the L3 cache partitions identified by the highest-supported
> +		CLOSID (num_closids-1). This CLOSID will not be available to the
> +		resctrl group.

Same comment as V1. The above two paragraphs cannot be guaranteed to be
specific to the "io_alloc" feature ... it is only specific to SDCIAE. 

> +
> +			"0":
> +			    I/O device L3 cache control is not enabled.
> +			"1":
> +			    I/O device L3 cache control is enabled, allowing users
> +			    to manage the portions of the L3 cache allocated for
> +			    the I/O device.
> +
> +		Feature can be enabled/disabled by writing to the interface.
> +		Example::
> +
> +			# echo 1 > /sys/fs/resctrl/info/L3/io_alloc

Similar to comment of V1 there is no information about what user can
expect when enabling this. For example, if this fails then one cause may
be that a resource group already owns that CLOSID and that removing that resource
group would make it possible to enable this feature. Even so, user space does not
know about CLOSIDs, only resource groups, making it difficult to correct without
more help. 

> +
>  Memory bandwidth(MB) subdirectory contains the following files
>  with respect to allocation:
>  
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 39e110033d96..066a7997eaf1 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -309,6 +309,8 @@ static void rdt_get_cdp_config(int level)
>  static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
>  {
>  	r->cache.io_alloc_capable = true;
> +	resctrl_file_fflags_init("io_alloc",
> +				 RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE);
>  }
>  
>  static void rdt_get_cdp_l3_config(void)
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 398f241b65d5..e30731ce9335 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -62,6 +62,7 @@ static char last_cmd_status_buf[512];
>  
>  static int rdtgroup_setup_root(struct rdt_fs_context *ctx);
>  static void rdtgroup_destroy_root(void);
> +static int rdtgroup_init_cat(struct resctrl_schema *s, u32 closid);
>  
>  struct dentry *debugfs_resctrl;
>  
> @@ -180,6 +181,25 @@ void closid_free(int closid)
>  	__set_bit(closid, &closid_free_map);
>  }
>  
> +/*
> + * io_alloc (SDCIAE) feature uses max CLOSID to route the SDCI traffic.

Please do not use io_alloc and SDCIAE interchangeably.

> + * Get the max CLOSID number
> + */
> +static u32 resctrl_io_alloc_closid_get(struct rdt_resource *r)
> +{
> +	return resctrl_arch_get_num_closid(r) - 1;
> +}
> +
> +static int resctrl_io_alloc_closid_alloc(struct rdt_resource *r)
> +{
> +	u32 io_alloc_closid = resctrl_io_alloc_closid_get(r);
> +
> +	if (__test_and_clear_bit(io_alloc_closid, &closid_free_map))
> +		return io_alloc_closid;
> +	else
> +		return -ENOSPC;
> +}

This does not look right. The way resctrl manages CLOSID is to use the
*minimum* of all CLOSID supported across all resources. It may thus be possible
for the L3 resource to support more CLOSID than other resources causing
the closid_free_map to be sized to a value smaller than the L3 max CLOSID.
The bit being tested/cleared here may thus exceed what is in the bitmap.

Also, during V1 we discussed how CDP was not handled and I am not able to
see where/if it is handled in this version.

> +
>  /**
>   * closid_allocated - test if provided closid is in use
>   * @closid: closid to be tested
> @@ -1832,6 +1852,97 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
>  	return 0;
>  }
>  
> +static int resctrl_io_alloc_show(struct kernfs_open_file *of,
> +				 struct seq_file *seq, void *v)
> +{
> +	struct resctrl_schema *s = of->kn->parent->priv;
> +	struct rdt_resource *r = s->res;
> +
> +	seq_printf(seq, "%x\n", resctrl_arch_get_io_alloc_enabled(r->rid));
> +	return 0;
> +}
> +
> +/*
> + * Initialize the io_alloc feature default when enabled

It is not clear what this comment describes.

> + */
> +static int resctrl_io_alloc_init_cat(struct rdt_resource *r, u32 closid)
> +{
> +	struct resctrl_schema *s;
> +	int ret = 0;
> +
> +	rdt_staged_configs_clear();
> +
> +	list_for_each_entry(s, &resctrl_schema_all, list) {
> +		r = s->res;
> +		if (r->rid == RDT_RESOURCE_L3) {

It looks like the function ignores the resource provided to it via function
parameter and instead uses internal hardcode of which resource to act on?

> +			ret = rdtgroup_init_cat(s, closid);
> +			if (ret < 0)
> +				goto out_init_cat;
> +
> +			ret = resctrl_arch_update_domains(r, closid);
> +			if (ret < 0)
> +				goto out_init_cat;
> +		}
> +	}
> +
> +out_init_cat:
> +	if (ret)
> +		rdt_last_cmd_puts("Failed to initialize io_alloc allocations\n");
> +
> +	rdt_staged_configs_clear();
> +	return ret;
> +}
> +
> +static ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
> +				      size_t nbytes, loff_t off)
> +{
> +	struct resctrl_schema *s = of->kn->parent->priv;
> +	struct rdt_resource *r = s->res;
> +	u32 io_alloc_closid;
> +	bool enable;
> +	int ret;
> +
> +	if (!r->cache.io_alloc_capable)
> +		return -EINVAL;
> +
> +	ret = kstrtobool(buf, &enable);
> +	if (ret)
> +		return ret;
> +
> +	cpus_read_lock();
> +	mutex_lock(&rdtgroup_mutex);
> +
> +	rdt_last_cmd_clear();
> +
> +	io_alloc_closid = resctrl_io_alloc_closid_get(r);
> +
> +	if (resctrl_arch_get_io_alloc_enabled(r->rid) != enable) {
> +		if (enable) {
> +			ret = resctrl_io_alloc_closid_alloc(r);
> +			if (ret < 0) {
> +				rdt_last_cmd_puts("io_alloc CLOSID is not available\n");

Can this be more useful to the user? The user does not know what the CLOSID is nor
what can be remedied to fix this. What if the message instead contains the name of
the resource group to which the CLOSID is assigned so that user knows which resource
group could be removed to be able to enable io_alloc?

> +				goto out_io_alloc;
> +			}
> +			ret = resctrl_io_alloc_init_cat(r, io_alloc_closid);
> +			if (ret) {
> +				closid_free(io_alloc_closid);

Could you please make a resctrl_io_alloc_closid_free() that is symmetrical to
resctrl_io_alloc_closid_alloc()?

> +				goto out_io_alloc;
> +			}
> +
> +		} else {
> +			closid_free(io_alloc_closid);
> +		}
> +
> +		ret = resctrl_arch_io_alloc_enable(r, enable);
> +	}
> +
> +out_io_alloc:
> +	mutex_unlock(&rdtgroup_mutex);
> +	cpus_read_unlock();
> +
> +	return ret ?: nbytes;
> +}
> +
>  /* rdtgroup information files for one cache resource. */
>  static struct rftype res_common_files[] = {
>  	{
> @@ -1984,6 +2095,13 @@ static struct rftype res_common_files[] = {
>  		.seq_show	= rdtgroup_schemata_show,
>  		.fflags		= RFTYPE_CTRL_BASE,
>  	},
> +	{
> +		.name		= "io_alloc",
> +		.mode		= 0644,
> +		.kf_ops		= &rdtgroup_kf_single_ops,
> +		.seq_show	= resctrl_io_alloc_show,
> +		.write		= resctrl_io_alloc_write,
> +	},
>  	{
>  		.name		= "mba_MBps_event",
>  		.mode		= 0644,

Reinette

RE: [PATCH v2 5/7] x86/resctrl: Add interface to enable/disable io_alloc feature

[Luck, Tony]

> >> static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
> >>  {
> >>       r->cache.io_alloc_capable = true;
> >> +     resctrl_file_fflags_init("io_alloc",
> >> +                              RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE);
> >>  }
> >
> > I think those fflags will make this file appear in all info cache directories
> > (L2 and L3).
> >
> > Presumably you only want this file (and "io_alloc_cbm" added by next
> > patch) in the L3 directory.
>
> Could you please elaborate why this file should only be in L3 directory? I do not see
> the problem with having it in L2. "io_alloc" communicates to user space if this I/O alloc
> feature is supported and does so with its content as opposed to its existence. For L2
> it will indicate that the feature is not supported.

Good point. I withdraw my comment.

-Tony

Re: [PATCH v2 3/7] x86/resctrl: Detect Smart Data Cache Injection Allocation Enforcement

[Moger, Babu]

Hi Reinette,

Sorry.. Was on a vacation.  Addressing the comments now.

On 12/23/24 15:13, Reinette Chatre wrote:
> Hi Babu,
> 
> On 12/18/24 1:37 PM, Babu Moger wrote:
>> Introduce io_alloc_capable in struct resctrl_cache to detect SDCIAE
>> (L3 Smart Data Cache Injection Allocation Enforcement) feature.
> 
> Please distinguish clearly between the resctrl feature ("io_alloc_capable")
> and the architecture specific feature (SDCIAE) that backs it. This is similar
> to what you have done for ABMC and makes the work much easier to understand.
> When the resctrl and arch feature is used interchangeably it becomes confusing.

Sure. Will rewrite the commit text.

> 
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> v2: Changed sdciae_capable to io_alloc_capable to make it generic feature.
>>     Also moved the io_alloc_capable in struct resctrl_cache.
>> ---
>>  arch/x86/kernel/cpu/resctrl/core.c | 7 +++++++
>>  include/linux/resctrl.h            | 4 ++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>> index c2450cd52511..39e110033d96 100644
>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>> @@ -306,6 +306,11 @@ static void rdt_get_cdp_config(int level)
>>  	rdt_resources_all[level].r_resctrl.cdp_capable = true;
>>  }
>>  
>> +static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
>> +{
>> +	r->cache.io_alloc_capable = true;
>> +}
> 
> rdt_get_sdciae_alloc_cfg() looks like one that should have "set" in its name, not "get".
> This also does not seem architecture specific so "sdciae" should not be in the name.
> rdt_set_io_alloc_capable()?

Sure.

> 
>> +
>>  static void rdt_get_cdp_l3_config(void)
>>  {
>>  	rdt_get_cdp_config(RDT_RESOURCE_L3);
>> @@ -931,6 +936,8 @@ static __init bool get_rdt_alloc_resources(void)
>>  		rdt_get_cache_alloc_cfg(1, r);
>>  		if (rdt_cpu_has(X86_FEATURE_CDP_L3))
>>  			rdt_get_cdp_l3_config();
>> +		if (rdt_cpu_has(X86_FEATURE_SDCIAE))
>> +			rdt_get_sdciae_alloc_cfg(r);
>>  		ret = true;
>>  	}
>>  	if (rdt_cpu_has(X86_FEATURE_CAT_L2)) {
>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index d94abba1c716..5837acff7442 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -129,6 +129,8 @@ struct rdt_mon_domain {
>>   * @arch_has_sparse_bitmasks:	True if a bitmask like f00f is valid.
>>   * @arch_has_per_cpu_cfg:	True if QOS_CFG register for this cache
>>   *				level has CPU scope.
>> + * @io_alloc_capable:	Smart Data Cache Injection Allocation Enforcement
>> + *			capable (SDCIAE).
> 
> Please remove arch specific text here. For example,
> "True if portion of the L3 cache can be allocated for I/O traffic."

Sure.
> 
>>   */
>>  struct resctrl_cache {
>>  	unsigned int	cbm_len;
>> @@ -136,6 +138,7 @@ struct resctrl_cache {
>>  	unsigned int	shareable_bits;
>>  	bool		arch_has_sparse_bitmasks;
>>  	bool		arch_has_per_cpu_cfg;
>> +	bool		io_alloc_capable;
>>  };
>>  
>>  /**
>> @@ -202,6 +205,7 @@ enum resctrl_scope {
>>   * @evt_list:		List of monitoring events
>>   * @fflags:		flags to choose base and info files
>>   * @cdp_capable:	Is the CDP feature available on this resource
>> + * @sdciae_capable:	Is SDCIAE feature available on this resource
>>   */
>>  struct rdt_resource {
>>  	int			rid;
> 
> Leftover from previous version?

Yes. Will remove it.

Thanks
Babu Moger

Re: [PATCH v2 4/7] x86/resctrl: Implement "io_alloc" enable/disable handlers

[Moger, Babu]

Hi Reinette,

On 12/23/24 15:14, Reinette Chatre wrote:
> Hi Babu,
> 
> On 12/18/24 1:38 PM, Babu Moger wrote:
>> Introduce architecture-specific2yy handlers to manage the detection and
> 
> "architecture-specific2yy"?

My bad.

> 
>> enabling/disabling of this feature.
> 
> Please add more to the context. It just jumps in with a "this feature" without
> any introduction.

Sure. Will rewrite the commit text.

> 
>>
>> SDCIAE feature can be enabled by setting bit 1 in MSR L3_QOS_EXT_CFG.
>> When the state of SDCIAE is modified, the updated value must be applied
>> across all logical processors within the QOS Domain. By default, the
>> io_alloc feature is turned off.
>>
>> The SDCIAE feature details are available in APM listed below [1].
>> [1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
>> Publication # 24593 Revision 3.41 section 19.4.7 L3 Smart Data Cache
>> Injection Allocation Enforcement (SDCIAE)
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> v2: Renamed the functions to simplify the code.
>>     Renamed sdciae_capable to io_alloc_capable.
>>
>>     Changed the name of few arch functions similar to ABMC series.
>>     resctrl_arch_get_io_alloc_enabled()
>>     resctrl_arch_io_alloc_enable()
>> ---
>>  arch/x86/include/asm/msr-index.h       |  1 +
>>  arch/x86/kernel/cpu/resctrl/internal.h | 10 ++++++++
>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 34 ++++++++++++++++++++++++++
>>  include/linux/resctrl.h                |  9 +++++++
>>  4 files changed, 54 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
>> index 3f3e2bc99162..360c52a62da9 100644
>> --- a/arch/x86/include/asm/msr-index.h
>> +++ b/arch/x86/include/asm/msr-index.h
>> @@ -1196,6 +1196,7 @@
>>  /* - AMD: */
>>  #define MSR_IA32_MBA_BW_BASE		0xc0000200
>>  #define MSR_IA32_SMBA_BW_BASE		0xc0000280
>> +#define MSR_IA32_L3_QOS_EXT_CFG		0xc00003ff
>>  #define MSR_IA32_EVT_CFG_BASE		0xc0000400
>>  
>>  /* AMD-V MSRs */
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index 20c898f09b7e..dff3354c2282 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -56,6 +56,9 @@
>>  /* Max event bits supported */
>>  #define MAX_EVT_CONFIG_BITS		GENMASK(6, 0)
>>  
>> +/* Setting bit 1 in L3_QOS_EXT_CFG enables the SDCIAE feature. */
>> +#define SDCIAE_ENABLE_BIT		1
>> +
>>  /**
>>   * cpumask_any_housekeeping() - Choose any CPU in @mask, preferring those that
>>   *			        aren't marked nohz_full
>> @@ -479,6 +482,7 @@ struct rdt_parse_data {
>>   * @mbm_cfg_mask:	Bandwidth sources that can be tracked when Bandwidth
>>   *			Monitoring Event Configuration (BMEC) is supported.
>>   * @cdp_enabled:	CDP state of this resource
>> + * @sdciae_enabled:	SDCIAE feature is enabled
>>   *
>>   * Members of this structure are either private to the architecture
>>   * e.g. mbm_width, or accessed via helpers that provide abstraction. e.g.
>> @@ -493,6 +497,7 @@ struct rdt_hw_resource {
>>  	unsigned int		mbm_width;
>>  	unsigned int		mbm_cfg_mask;
>>  	bool			cdp_enabled;
>> +	bool			sdciae_enabled;
>>  };
>>  
>>  static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r)
>> @@ -539,6 +544,11 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable);
>>  
>>  void arch_mon_domain_online(struct rdt_resource *r, struct rdt_mon_domain *d);
>>  
>> +static inline bool resctrl_arch_get_io_alloc_enabled(enum resctrl_res_level l)
> 
> The custom is to pass a pointer to the resource when interacting with it. Why is it
> needed to pass the ID here?

Will change it.

> 
>> +{
>> +	return rdt_resources_all[l].sdciae_enabled;
>> +}
>> +
>>  /*
>>   * To return the common struct rdt_resource, which is contained in struct
>>   * rdt_hw_resource, walk the resctrl member of struct rdt_hw_resource.
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 6419e04d8a7b..398f241b65d5 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -1798,6 +1798,40 @@ static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of,
>>  	return ret ?: nbytes;
>>  }
>>  
>> +static void resctrl_sdciae_set_one_amd(void *arg)
>> +{
>> +	bool *enable = arg;
>> +
>> +	if (*enable)
>> +		msr_set_bit(MSR_IA32_L3_QOS_EXT_CFG, SDCIAE_ENABLE_BIT);
>> +	else
>> +		msr_clear_bit(MSR_IA32_L3_QOS_EXT_CFG, SDCIAE_ENABLE_BIT);
>> +}
>> +
>> +static int _resctrl_io_alloc_enable(struct rdt_resource *r, bool enable)
>> +{
>> +	struct rdt_ctrl_domain *d;
>> +
>> +	/* Update L3_QOS_EXT_CFG MSR on all the CPUs in all domains*/
>> +	list_for_each_entry(d, &r->ctrl_domains, hdr.list)
>> +		on_each_cpu_mask(&d->hdr.cpu_mask, resctrl_sdciae_set_one_amd, &enable, 1);
>> +
>> +	return 0;
> 
> Same comment as in V1 about this arch specific handler always returning 0 and can thus
> just return void. Also the name should not reflect that it is resctrl code. One
> option could be _resctrl_arch_io_alloc_enable().

Sure.

> 
>> +}
>> +
>> +int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
>> +{
>> +	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>> +
>> +	if (hw_res->r_resctrl.cache.io_alloc_capable &&
>> +	    hw_res->sdciae_enabled != enable) {
>> +		_resctrl_io_alloc_enable(r, enable);
>> +		hw_res->sdciae_enabled = enable;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  /* rdtgroup information files for one cache resource. */
>>  static struct rftype res_common_files[] = {
>>  	{
>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index 5837acff7442..8c66aeac4768 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -344,6 +344,15 @@ void resctrl_arch_reset_rmid(struct rdt_resource *r, struct rdt_mon_domain *d,
>>   */
>>  void resctrl_arch_reset_rmid_all(struct rdt_resource *r, struct rdt_mon_domain *d);
>>  
>> +/**
>> + * resctrl_arch_io_alloc_enable() - Enable/disable io_alloc feature.
>> + * @r:		The resctrl resource.
>> + * @enable:	Enable (1) or disable (0) the feature
>> + *
>> + * This can be called from any CPU.
>> + */
>> +int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable);
>> +
>>  extern unsigned int resctrl_rmid_realloc_threshold;
>>  extern unsigned int resctrl_rmid_realloc_limit;
>>  
> 
> Reinette
> 

-- 
Thanks
Babu Moger

Re: [PATCH v2 5/7] x86/resctrl: Add interface to enable/disable io_alloc feature

[Moger, Babu]

Hi Reinette,

On 12/23/24 15:37, Reinette Chatre wrote:
> Hi Babu,
> 
> On 12/18/24 1:38 PM, Babu Moger wrote:
>> The io_alloc feature in resctrl enables system software to configure
>> the portion of the L3 cache allocated for I/O traffic.
>>
> 
> Above is about resctrl feature.
> 
>> Smart Data Cache Injection (SDCI) is a mechanism that allows direct
>> insertion of data from I/O devices into the L3 cache. By caching I/O
>> data directly in the L3 cache, instead of writing it to DRAM first,
>> SDCI reduces DRAM bandwidth usage and lowers latency for the processor
>> consuming the I/O data.
>>
>> When enabled, SDCIAE forces all SDCI lines to be placed into the L3 cache
>> partitions identified by the highest-supported L3_MASK_n register as
>> reported by CPUID Fn0000_0010_EDX_x1.MAX_COS. For example, if MAX_COS=15,
>> SDCI lines will be allocated into the L3 cache partitions determined by
>> the bitmask in the L3_MASK_15 register.
> 
> Above is about AMD feature.
> 
>>
>> Introduce interface to enable/disable "io_alloc" feature on user input.
> 
> Back to resctrl feature.
> 
> Please do not jump from resctrl to AMD feature in a way that makes it seem that
> they are interchangeable. To help with this you could use similar style as in
> ABMC where the text flows like:
> 
> 	<resctrl feature description>.
> 
> 	On AMD <resctrl feature> is backed by <AMD feature> that <AMD feature details>.

Yes. Need to rewrite the commit log.

> 
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> v2: Renamed the feature to "io_alloc".
>>     Added generic texts for the feature in commit log and resctrl.rst doc.
>>     Added resctrl_io_alloc_init_cat() to initialize io_alloc to default
>>     values when enabled.
>>     Fixed io_alloc show functinality to display only on L3 resource.
>> ---
>>  Documentation/arch/x86/resctrl.rst     |  27 ++++++
>>  arch/x86/kernel/cpu/resctrl/core.c     |   2 +
>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 118 +++++++++++++++++++++++++
>>  3 files changed, 147 insertions(+)
>>
>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
>> index 6768fc1fad16..52679175ee14 100644
>> --- a/Documentation/arch/x86/resctrl.rst
>> +++ b/Documentation/arch/x86/resctrl.rst
>> @@ -135,6 +135,33 @@ related to allocation:
>>  			"1":
>>  			      Non-contiguous 1s value in CBM is supported.
>>  
>> +"io_alloc":
>> +		The "io_alloc" feature in resctrl enables system software to
>> +		configure the portion of the L3 cache allocated for I/O traffic.
>> +
>> +		Smart Data Cache Injection (SDCI) is a mechanism that allows
>> +		direct insertion of data from I/O devices into the L3 cache.
>> +		By caching I/O data directly in the L3 cache, instead of writing
>> +		it to DRAM first, SDCI reduces DRAM bandwidth usage and lowers
>> +		latency for the processor consuming the I/O data.
>> +
>> +		When enabled the feature forces all SDCI lines to be placed
>> +		into the L3 cache partitions identified by the highest-supported
>> +		CLOSID (num_closids-1). This CLOSID will not be available to the
>> +		resctrl group.
> 
> Same comment as V1. The above two paragraphs cannot be guaranteed to be
> specific to the "io_alloc" feature ... it is only specific to SDCIAE. 

Yes. Need to rewrite.

> 
>> +
>> +			"0":
>> +			    I/O device L3 cache control is not enabled.
>> +			"1":
>> +			    I/O device L3 cache control is enabled, allowing users
>> +			    to manage the portions of the L3 cache allocated for
>> +			    the I/O device.
>> +
>> +		Feature can be enabled/disabled by writing to the interface.
>> +		Example::
>> +
>> +			# echo 1 > /sys/fs/resctrl/info/L3/io_alloc
> 
> Similar to comment of V1 there is no information about what user can
> expect when enabling this. For example, if this fails then one cause may
> be that a resource group already owns that CLOSID and that removing that resource
> group would make it possible to enable this feature. Even so, user space does not
> know about CLOSIDs, only resource groups, making it difficult to correct without
> more help. 

Yes. Additional documentation required.

> 
>> +
>>  Memory bandwidth(MB) subdirectory contains the following files
>>  with respect to allocation:
>>  
>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>> index 39e110033d96..066a7997eaf1 100644
>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>> @@ -309,6 +309,8 @@ static void rdt_get_cdp_config(int level)
>>  static void rdt_get_sdciae_alloc_cfg(struct rdt_resource *r)
>>  {
>>  	r->cache.io_alloc_capable = true;
>> +	resctrl_file_fflags_init("io_alloc",
>> +				 RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE);
>>  }
>>  
>>  static void rdt_get_cdp_l3_config(void)
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 398f241b65d5..e30731ce9335 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -62,6 +62,7 @@ static char last_cmd_status_buf[512];
>>  
>>  static int rdtgroup_setup_root(struct rdt_fs_context *ctx);
>>  static void rdtgroup_destroy_root(void);
>> +static int rdtgroup_init_cat(struct resctrl_schema *s, u32 closid);
>>  
>>  struct dentry *debugfs_resctrl;
>>  
>> @@ -180,6 +181,25 @@ void closid_free(int closid)
>>  	__set_bit(closid, &closid_free_map);
>>  }
>>  
>> +/*
>> + * io_alloc (SDCIAE) feature uses max CLOSID to route the SDCI traffic.
> 
> Please do not use io_alloc and SDCIAE interchangeably.

ok

> 
>> + * Get the max CLOSID number
>> + */
>> +static u32 resctrl_io_alloc_closid_get(struct rdt_resource *r)
>> +{
>> +	return resctrl_arch_get_num_closid(r) - 1;
>> +}
>> +
>> +static int resctrl_io_alloc_closid_alloc(struct rdt_resource *r)
>> +{
>> +	u32 io_alloc_closid = resctrl_io_alloc_closid_get(r);
>> +
>> +	if (__test_and_clear_bit(io_alloc_closid, &closid_free_map))
>> +		return io_alloc_closid;
>> +	else
>> +		return -ENOSPC;
>> +}
> 
> This does not look right. The way resctrl manages CLOSID is to use the
> *minimum* of all CLOSID supported across all resources. It may thus be possible
> for the L3 resource to support more CLOSID than other resources causing
> the closid_free_map to be sized to a value smaller than the L3 max CLOSID.
> The bit being tested/cleared here may thus exceed what is in the bitmap.

That is correct, though chances of that happening is rare.

Hardware needs to program L3 max CLOSID to support this feature.

So, our option is to add a check here to verify that. If the check fails
we can report error and exit.

> 
> Also, during V1 we discussed how CDP was not handled and I am not able to
> see where/if it is handled in this version.

https://lore.kernel.org/lkml/ecdffce0-796b-4ebe-8999-73f2be1e703b@amd.com/

This is another case where we need to allow SDCIAE even when CLOS 15 is
already taken by CDP. Will add the check and documentation about it.



> 
>> +
>>  /**
>>   * closid_allocated - test if provided closid is in use
>>   * @closid: closid to be tested
>> @@ -1832,6 +1852,97 @@ int resctrl_arch_io_alloc_enable(struct rdt_resource *r, bool enable)
>>  	return 0;
>>  }
>>  
>> +static int resctrl_io_alloc_show(struct kernfs_open_file *of,
>> +				 struct seq_file *seq, void *v)
>> +{
>> +	struct resctrl_schema *s = of->kn->parent->priv;
>> +	struct rdt_resource *r = s->res;
>> +
>> +	seq_printf(seq, "%x\n", resctrl_arch_get_io_alloc_enabled(r->rid));
>> +	return 0;
>> +}
>> +
>> +/*
>> + * Initialize the io_alloc feature default when enabled
> 
> It is not clear what this comment describes.

Yes. Need more details here.

> 
>> + */
>> +static int resctrl_io_alloc_init_cat(struct rdt_resource *r, u32 closid)
>> +{
>> +	struct resctrl_schema *s;
>> +	int ret = 0;
>> +
>> +	rdt_staged_configs_clear();
>> +
>> +	list_for_each_entry(s, &resctrl_schema_all, list) {
>> +		r = s->res;
>> +		if (r->rid == RDT_RESOURCE_L3) {
> 
> It looks like the function ignores the resource provided to it via function
> parameter and instead uses internal hardcode of which resource to act on?

Yes. This check is not required. We can get the schemata directly.

> 
>> +			ret = rdtgroup_init_cat(s, closid);
>> +			if (ret < 0)
>> +				goto out_init_cat;
>> +
>> +			ret = resctrl_arch_update_domains(r, closid);
>> +			if (ret < 0)
>> +				goto out_init_cat;
>> +		}
>> +	}
>> +
>> +out_init_cat:
>> +	if (ret)
>> +		rdt_last_cmd_puts("Failed to initialize io_alloc allocations\n");
>> +
>> +	rdt_staged_configs_clear();
>> +	return ret;
>> +}
>> +
>> +static ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
>> +				      size_t nbytes, loff_t off)
>> +{
>> +	struct resctrl_schema *s = of->kn->parent->priv;
>> +	struct rdt_resource *r = s->res;
>> +	u32 io_alloc_closid;
>> +	bool enable;
>> +	int ret;
>> +
>> +	if (!r->cache.io_alloc_capable)
>> +		return -EINVAL;
>> +
>> +	ret = kstrtobool(buf, &enable);
>> +	if (ret)
>> +		return ret;
>> +
>> +	cpus_read_lock();
>> +	mutex_lock(&rdtgroup_mutex);
>> +
>> +	rdt_last_cmd_clear();
>> +
>> +	io_alloc_closid = resctrl_io_alloc_closid_get(r);
>> +
>> +	if (resctrl_arch_get_io_alloc_enabled(r->rid) != enable) {
>> +		if (enable) {
>> +			ret = resctrl_io_alloc_closid_alloc(r);
>> +			if (ret < 0) {
>> +				rdt_last_cmd_puts("io_alloc CLOSID is not available\n");
> 
> Can this be more useful to the user? The user does not know what the CLOSID is nor
> what can be remedied to fix this. What if the message instead contains the name of
> the resource group to which the CLOSID is assigned so that user knows which resource
> group could be removed to be able to enable io_alloc?

Yes. We can do that.

> 
>> +				goto out_io_alloc;
>> +			}
>> +			ret = resctrl_io_alloc_init_cat(r, io_alloc_closid);
>> +			if (ret) {
>> +				closid_free(io_alloc_closid);
> 
> Could you please make a resctrl_io_alloc_closid_free() that is symmetrical to
> resctrl_io_alloc_closid_alloc()?

Sure.

> 
>> +				goto out_io_alloc;
>> +			}
>> +
>> +		} else {
>> +			closid_free(io_alloc_closid);
>> +		}
>> +
>> +		ret = resctrl_arch_io_alloc_enable(r, enable);
>> +	}
>> +
>> +out_io_alloc:
>> +	mutex_unlock(&rdtgroup_mutex);
>> +	cpus_read_unlock();
>> +
>> +	return ret ?: nbytes;
>> +}
>> +
>>  /* rdtgroup information files for one cache resource. */
>>  static struct rftype res_common_files[] = {
>>  	{
>> @@ -1984,6 +2095,13 @@ static struct rftype res_common_files[] = {
>>  		.seq_show	= rdtgroup_schemata_show,
>>  		.fflags		= RFTYPE_CTRL_BASE,
>>  	},
>> +	{
>> +		.name		= "io_alloc",
>> +		.mode		= 0644,
>> +		.kf_ops		= &rdtgroup_kf_single_ops,
>> +		.seq_show	= resctrl_io_alloc_show,
>> +		.write		= resctrl_io_alloc_write,
>> +	},
>>  	{
>>  		.name		= "mba_MBps_event",
>>  		.mode		= 0644,
> 
> Reinette
> 

-- 
Thanks
Babu Moger

Re: RE: [PATCH v2 0/7] x86/resctrl : Support L3 Smart Data Cache Injection Allocation Enforcement (SDCIAE)

[Moger, Babu]

Hi Tony,

Thanks for testing..

On 12/18/24 17:27, Luck, Tony wrote:
> I don't have an AMD system, so I added a couple of hacks to the code to pretend I did.
> My hacks might have missed something, so the below test may not fail for you.
> 
> My test:
> 
> # echo 1 > info/L3/io_alloc
> # cat info/L3/bit_usage
> 
> This gave me a console splat starting with:
> 
> [  163.801078] invalid mode for closid 14

I recreated the issue.

1124.714954] ------------[ cut here ]------------
[ 1124.714955] invalid mode for closid 15
[ 1124.714956] WARNING: CPU: 71 PID: 3553 at
arch/x86/kernel/cpu/resctrl/rdtgroup.c:1082 rdt_bit_usage_show+0x238/0x2d0


> 
> That happened because in rdt_bit_usage_show()
> 
>                 for (i = 0; i < closids_supported(); i++) {
>                         if (!closid_allocated(i))
>                                 continue;
>                         ctrl_val = resctrl_arch_get_config(r, dom, i,
>                                                            s->conf_type);
>                         mode = rdtgroup_mode_by_closid(i);
> 
> CLOSID 14 is my highest. It's supported, and allocated.
> 
> rdtgroup_mode_by_closid() searches rdt_all_groups to find
> one using CLOSID 14, but there isn't one. So it returns
> RDT_NUM_MODES
> 
> Maybe RDT_NUM_MODES isn't invalid in the switch()
> Looks like it should be same action as RDT_MODE_SHAREABLE?

Yes. That is correct. Will take care of this in next revision.
-- 
Thanks
Babu Moger