x86,fs/resctrl: Support L3 Smart Data Cache Injection Allocation Enforcement (SDCIAE)

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

---

---
 .../admin-guide/kernel-parameters.txt         |  2 +-
 Documentation/filesystems/resctrl.rst         | 21 ++++++++++---------
 arch/x86/kernel/cpu/resctrl/core.c            |  2 ++
 3 files changed, 14 insertions(+), 11 deletions(-)

Documentation/admin-guide/kernel-parameters.txt +1 -1

index 0d2ea9a60145..0170c0af03cc 100644
@@ -6089,7 +6089,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
 

Documentation/filesystems/resctrl.rst +0 -8

index c7949dd44f2f..f08699030312 100644
@@ -17,16 +17,17 @@ AMD refers to this feature as AMD Platform Quality of Service(AMD QoS).
 This feature is enabled by the CONFIG_X86_CPU_RESCTRL and the x86 /proc/cpuinfo
 flag bits:
 
-===============================================	================================
-RDT (Resource Director Technology) Allocation	"rdt_a"
-CAT (Cache Allocation Technology)		"cat_l3", "cat_l2"
-CDP (Code and Data Prioritization)		"cdp_l3", "cdp_l2"
-CQM (Cache QoS Monitoring)			"cqm_llc", "cqm_occup_llc"
-MBM (Memory Bandwidth Monitoring)		"cqm_mbm_total", "cqm_mbm_local"
-MBA (Memory Bandwidth Allocation)		"mba"
-SMBA (Slow Memory Bandwidth Allocation)         ""

Hi Babu,

Quoted 1 line · Babu Moger

Add the command line options to enable or disable the new resctrl feature

options -> option?

Quoted 43 lines

L3 Smart Data Cache Injection Allocation Enforcement (SDCIAE).

Signed-off-by Babu Moger <[email protected]>

---

---
  .../admin-guide/kernel-parameters.txt         |  2 +-
  Documentation/filesystems/resctrl.rst         | 21 ++++++++++---------
  arch/x86/kernel/cpu/resctrl/core.c            |  2 ++
  3 files changed, 14 insertions(+), 11 deletions(-)

Documentation/admin-guide/kernel-parameters.txt +1 -1

index 0d2ea9a60145..0170c0af03cc 100644
@@ -6089,7 +6089,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
 

Documentation/filesystems/resctrl.rst +1 -10

index c7949dd44f2f..f08699030312 100644
@@ -17,16 +17,17 @@ AMD refers to this feature as AMD Platform Quality of Service(AMD QoS).
  This feature is enabled by the CONFIG_X86_CPU_RESCTRL and the x86 /proc/cpuinfo
  flag bits:
 
-===============================================	================================
-RDT (Resource Director Technology) Allocation	"rdt_a"
-CAT (Cache Allocation Technology)		"cat_l3", "cat_l2"
-CDP (Code and Data Prioritization)		"cdp_l3", "cdp_l2"
-CQM (Cache QoS Monitoring)			"cqm_llc", "cqm_occup_llc"
-MBM (Memory Bandwidth Monitoring)		"cqm_mbm_total", "cqm_mbm_local"
-MBA (Memory Bandwidth Allocation)		"mba"
-SMBA (Slow Memory Bandwidth Allocation)         ""
-BMEC (Bandwidth Monitoring Event Configuration) ""
-===============================================	================================
+=============================================================== ================================

Documentation/filesystems/resctrl.rst AMD refers to this feature as AMD Platform Quality of Service(AMD QoS). · line 27

Quoted 7 lines

Documentation/filesystems/resctrl.rst +7 -0

+RDT (Resource Director Technology) Allocation			"rdt_a"
+CAT (Cache Allocation Technology)				"cat_l3", "cat_l2"
+CDP (Code and Data Prioritization)				"cdp_l3", "cdp_l2"
+CQM (Cache QoS Monitoring)					"cqm_llc", "cqm_occup_llc"
+MBM (Memory Bandwidth Monitoring)				"cqm_mbm_total", "cqm_mbm_local"
+MBA (Memory Bandwidth Allocation)				"mba"
+SMBA (Slow Memory Bandwidth Allocation)         		""

With all these adjustments this looks like an ideal time to fix the missing TABs in the SMBA spacing?

Documentation/filesystems/resctrl.rst AMD refers to this feature as AMD Platform Quality of Service(AMD QoS). · line 30

Quoted 4 lines

Documentation/filesystems/resctrl.rst +3 -0

+BMEC (Bandwidth Monitoring Event Configuration) 		""
+SDCIAE (Smart Data Cache Injection Allocation Enforcement)	""
+=============================================================== ================================

Reinette

"io_alloc" enables direct insertion of data from I/O devices into the cache.

On AMD systems, "io_alloc" feature is backed by L3 Smart Data Cache Injection Allocation Enforcement (SDCIAE). Change SDCIAE state by setting (to enable) or clearing (to disable) bit 1 of MSR L3_QOS_EXT_CFG on all logical processors within the cache domain.

Introduce architecture-specific call to enable and disable the feature.

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

---

Hi Babu,

Quoted 15 lines · Babu Moger

"io_alloc" enables direct insertion of data from I/O devices into the cache.

On AMD systems, "io_alloc" feature is backed by L3 Smart Data Cache Injection Allocation Enforcement (SDCIAE). Change SDCIAE state by setting (to enable) or clearing (to disable) bit 1 of MSR L3_QOS_EXT_CFG on all logical processors within the cache domain.

Introduce architecture-specific call to enable and disable the feature.

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)

Quoted 1 line

Link https://bugzilla.kernel.org/show_bug.cgi?id=206537

fyi ... Documentation/process/maintainer-tip.rst has some examples how the "Link:" can be associated with the changelog. For example, above could be:

	The SDCIAE feature details are available in APM [1] available
	from [2].
	[1] ...

	Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 # [2]

Quoted 2 lines

Signed-off-by Babu Moger <[email protected]>

---

Reviewed-by Reinette Chatre <[email protected]>

"io_alloc" feature in resctrl allows direct insertion of data from I/O devices into the cache.

Introduce the 'io_alloc' resctrl file to indicate the support for the feature.

Signed-off-by Babu Moger <[email protected]>

---

---
 Documentation/filesystems/resctrl.rst | 29 +++++++++++++++++++++++++++
 fs/resctrl/ctrlmondata.c              | 21 +++++++++++++++++++
 fs/resctrl/internal.h                 |  5 +++++
 fs/resctrl/rdtgroup.c                 | 24 +++++++++++++++++++++-
 4 files changed, 78 insertions(+), 1 deletion(-)

Documentation/filesystems/resctrl.rst +0 -0

index f08699030312..fff7e04d1e2a 100644
@@ -136,6 +136,35 @@ related to allocation:
 			"1":

Hi Babu,

Quoted 8 lines · Babu Moger

"io_alloc" feature in resctrl allows direct insertion of data from I/O devices into the cache.

Introduce the 'io_alloc' resctrl file to indicate the support for the feature.

Signed-off-by Babu Moger <[email protected]>

---

...

Quoted 15 lines

---
  Documentation/filesystems/resctrl.rst | 29 +++++++++++++++++++++++++++
  fs/resctrl/ctrlmondata.c              | 21 +++++++++++++++++++
  fs/resctrl/internal.h                 |  5 +++++
  fs/resctrl/rdtgroup.c                 | 24 +++++++++++++++++++++-
  4 files changed, 78 insertions(+), 1 deletion(-)

Documentation/filesystems/resctrl.rst +0 -0

index f08699030312..fff7e04d1e2a 100644
@@ -136,6 +136,35 @@ related to allocation:
  			"1":
  			      Non-contiguous 1s value in CBM is supported.
 

Documentation/filesystems/resctrl.rst related to allocation: · line 146

Quoted 8 lines

Documentation/filesystems/resctrl.rst +8 -0

+"io_alloc":
+		"io_alloc" enables system software to configure the portion of
+		the cache allocated for I/O traffic. File may only exist if the
+		system supports this feature on some of its cache resources.
+
+			"disabled":
+			      Portions of cache used for allocation of I/O traffic
+			      cannot be configured.

To help distinguish difference between "disabled" and "not supported" it may be useful to prepend something like "Resource supports "io_alloc" but the feature is disabled."

Documentation/filesystems/resctrl.rst related to allocation: · line 158

Quoted 13 lines

Documentation/filesystems/resctrl.rst +12 -0

+			"enabled":
+			      Portions of cache used for allocation of I/O traffic
+			      can be configured using "io_alloc_cbm".
+			"not supported":
+			      Support not available for this resource.
+
+		The underlying implementation may reduce resources available to
+		general (CPU) cache allocation. See architecture specific notes
+		below. Depending on usage requirements the feature can be enabled
+		or disabled.
+
+		On AMD systems, the io_alloc feature is supported by the L3 Smart

drop "the"?

Documentation/filesystems/resctrl.rst related to allocation: · line 170

Quoted 13 lines

Documentation/filesystems/resctrl.rst +9 -0

+		Data Cache Injection Allocation Enforcement (SDCIAE). The CLOSID for
+		io_alloc is the highest CLOSID supported by the resource. When
+		io_alloc is enabled, the highest CLOSID is dedicated to io_alloc and
+		no longer available for general (CPU) cache allocation. When CDP is
+		enabled, io_alloc routes I/O traffic using the highest CLOSID allocated
+		for the instruction cache (L3CODE), making this CLOSID no longer
+		available for general (CPU) cache allocation for both the L3CODE and
+		L3DATA resources.
+
  Memory bandwidth(MB) subdirectory contains the following files
  with respect to allocation:
 

Code looks good to me.

"io_alloc" feature in resctrl enables direct insertion of data from I/O devices into the cache.

On AMD systems, when io_alloc is enabled, the highest CLOSID is reserved exclusively for I/O allocation traffic and is no longer available for general CPU cache allocation. Users are encouraged to enable it only when running workloads that can benefit from this functionality.

Since CLOSIDs are managed by resctrl fs, it is least invasive to make the "io_alloc is supported by maximum supported CLOSID" part of the initial resctrl fs support for io_alloc. Take care not to expose this use of CLOSID for io_alloc to user space so that this is not required from other architectures that may support io_alloc differently in the future.

Introduce user interface to enable/disable io_alloc feature.

fs/resctrl/ctrlmondata.c +8 -0

index d495a5d5c9d5..bf982eab7b18 100644
@@ -685,3 +685,140 @@ int resctrl_io_alloc_show(struct kernfs_open_file *of, struct seq_file *seq, voi
 
 	return 0;
 }
+
+/*
+ * resctrl_io_alloc_closid_supported() - io_alloc feature utilizes the
+ * highest CLOSID value to direct I/O traffic. Ensure that io_alloc_closid
+ * is in the supported range.
+ */
+static bool resctrl_io_alloc_closid_supported(u32 io_alloc_closid)
+{

fs/resctrl/ctrlmondata.c +6 -0

+	enum resctrl_conf_type peer_type;
+	struct resctrl_schema *peer_s;
+	int ret;
+
+	rdt_staged_configs_clear();
+

Hi Babu,

Quoted 14 lines · Babu Moger

"io_alloc" feature in resctrl enables direct insertion of data from I/O devices into the cache.

On AMD systems, when io_alloc is enabled, the highest CLOSID is reserved exclusively for I/O allocation traffic and is no longer available for general CPU cache allocation. Users are encouraged to enable it only when running workloads that can benefit from this functionality.

Since CLOSIDs are managed by resctrl fs, it is least invasive to make the "io_alloc is supported by maximum supported CLOSID" part of the initial resctrl fs support for io_alloc. Take care not to expose this use of CLOSID for io_alloc to user space so that this is not required from other architectures that may support io_alloc differently in the future.

Quoted 1 line

Introduce user interface to enable/disable io_alloc feature.

Please include high level overview of what this patch does to enable and disable io_alloc. Doing so will help connect why the changelog contains information about CLOSID management.

Quoted 23 lines

fs/resctrl/ctrlmondata.c +14 -0

index d495a5d5c9d5..bf982eab7b18 100644
@@ -685,3 +685,140 @@ int resctrl_io_alloc_show(struct kernfs_open_file *of, struct seq_file *seq, voi
 
  	return 0;
  }
+
+/*
+ * resctrl_io_alloc_closid_supported() - io_alloc feature utilizes the
+ * highest CLOSID value to direct I/O traffic. Ensure that io_alloc_closid
+ * is in the supported range.
+ */
+static bool resctrl_io_alloc_closid_supported(u32 io_alloc_closid)
+{
+	return io_alloc_closid < closids_supported();
+}
+
+static struct resctrl_schema *resctrl_get_schema(enum resctrl_conf_type type)
+{
+	struct resctrl_schema *schema;

fs/resctrl/ctrlmondata.c int resctrl_io_alloc_show(struct kernfs_open_file *of, struct seq_file *seq, voi · line 705

Quoted 4 lines

fs/resctrl/ctrlmondata.c +4 -0

+
+	list_for_each_entry(schema, &resctrl_schema_all, list) {
+		if (schema->conf_type == type)
+			return schema;

This does not look right. More than one resource can have the same configuration type, no? Think about L2 and L3 having CDP enabled ...
Looks like this is missing a resource type as parameter and a check for the resource ... but is this function even necessary (more below)?

fs/resctrl/ctrlmondata.c int resctrl_io_alloc_show(struct kernfs_open_file *of, struct seq_file *seq, voi · line 717

Quoted 13 lines

fs/resctrl/ctrlmondata.c +12 -0

+	}
+
+	return NULL;
+}
+
+/*
+ * Initialize io_alloc CLOSID cache resource CBM with all usable (shared
+ * and unused) cache portions.
+ */
+static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32 closid)
+{
+	struct rdt_resource *r = s->res;

Needs reverse fir.

Quoted 13 lines

fs/resctrl/ctrlmondata.c +12 -0

+	enum resctrl_conf_type peer_type;
+	struct resctrl_schema *peer_s;
+	int ret;
+
+	rdt_staged_configs_clear();
+
+	ret = rdtgroup_init_cat(s, closid);
+	if (ret < 0)
+		goto out;
+
+	/* Initialize schema for both CDP_DATA and CDP_CODE when CDP is enabled */
+	if (resctrl_arch_get_cdp_enabled(r->rid)) {

fs/resctrl/ctrlmondata.c int resctrl_io_alloc_show(struct kernfs_open_file *of, struct seq_file *seq, voi · line 733

Quoted 4 lines

fs/resctrl/ctrlmondata.c +4 -0

+		peer_type = resctrl_peer_type(s->conf_type);
+		peer_s = resctrl_get_schema(peer_type);
+		if (peer_s) {
+			ret = rdtgroup_init_cat(peer_s, closid);

This is unexpected. In v7 I suggested that when parsing the CBM of one of the CDP resources it is not necessary to do so again for the peer. The CBM can be parsed *once* and the configuration just copied over. See:
https://lore.kernel.org/lkml/[email protected]/

Generally when feedback is provided it is good to check all places in series where it is relevant. oh ... but looking ahead you ignored the feedback in the patch it was given also :(

Hi Reinette,

Quoted 18 lines · Reinette Chatre

Hi Babu,

Quoted 15 lines · Babu Moger

"io_alloc" feature in resctrl enables direct insertion of data from I/O devices into the cache.

On AMD systems, when io_alloc is enabled, the highest CLOSID is reserved exclusively for I/O allocation traffic and is no longer available for general CPU cache allocation. Users are encouraged to enable it only when running workloads that can benefit from this functionality.

Since CLOSIDs are managed by resctrl fs, it is least invasive to make the "io_alloc is supported by maximum supported CLOSID" part of the initial resctrl fs support for io_alloc. Take care not to expose this use of CLOSID
for io_alloc to user space so that this is not required from other architectures that may support io_alloc differently in the future.

Quoted 4 lines

Quoted 1 line

Introduce user interface to enable/disable io_alloc feature.

Please include high level overview of what this patch does to enable and disable io_alloc. Doing so will help connect why the changelog contains information about CLOSID management.

Sure.

Quoted 27 lines

Quoted 27 lines

fs/resctrl/ctrlmondata.c +4 -0

index d495a5d5c9d5..bf982eab7b18 100644
@@ -685,3 +685,140 @@ int resctrl_io_alloc_show(struct kernfs_open_file
 *of, struct seq_file *seq, voi
         return 0;
   }
+
+/*
+ * resctrl_io_alloc_closid_supported() - io_alloc feature utilizes the
+ * highest CLOSID value to direct I/O traffic. Ensure that

io_alloc_closid
+ * is in the supported range.
+ */

fs/resctrl/ctrlmondata.c +6 -0

+static bool resctrl_io_alloc_closid_supported(u32 io_alloc_closid)
+{
+    return io_alloc_closid < closids_supported();
+}
+
+static struct resctrl_schema *resctrl_get_schema(enum

resctrl_conf_type type)

fs/resctrl/ctrlmondata.c +4 -0

+{
+    struct resctrl_schema *schema;
+
+    list_for_each_entry(schema, &resctrl_schema_all, list) {

Quoted 8 lines

Quoted 2 lines

+        if (schema->conf_type == type)
+            return schema;

This does not look right. More than one resource can have the same configuration type, no?
Think about L2 and L3 having CDP enabled ...
Looks like this is missing a resource type as parameter and a check for the resource ...
but is this function even necessary (more below)?

May not be required.  Comments below.

Quoted 15 lines

Quoted 11 lines

+    }

fs/resctrl/ctrlmondata.c +9 -0

+
+    return NULL;
+}
+
+/*
+ * Initialize io_alloc CLOSID cache resource CBM with all usable (shared
+ * and unused) cache portions.
+ */
+static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32

Quoted 3 lines

closid)
+{
+    struct rdt_resource *r = s->res;

Needs reverse fir.

Sure.

Quoted 16 lines

Quoted 16 lines

+    enum resctrl_conf_type peer_type;
+    struct resctrl_schema *peer_s;
+    int ret;

fs/resctrl/ctrlmondata.c +8 -0

+
+    rdt_staged_configs_clear();
+
+    ret = rdtgroup_init_cat(s, closid);
+    if (ret < 0)
+        goto out;
+
+    /* Initialize schema for both CDP_DATA and CDP_CODE when CDP is

enabled */
+    if (resctrl_arch_get_cdp_enabled(r->rid)) { +        peer_type = resctrl_peer_type(s->conf_type); +        peer_s = resctrl_get_schema(peer_type);

Quoted 8 lines

Quoted 2 lines

+        if (peer_s) {
+            ret = rdtgroup_init_cat(peer_s, closid);

This is unexpected. In v7 I suggested that when parsing the CBM of one of the CDP
resources it is not necessary to do so again for the peer. The CBM can be parsed *once* and the configuration just copied over. See:
https://lore.kernel.org/
lkml/[email protected]/

Let met try to understand.

So, rdtgroup_init_cat() sets up the staged _config for the specific CDP type for all the domains.

We need to apply those staged_configs to its peer type on all the domains.

Something like this?

/* Initialize staged_config of the peer type when CDP is enabled */ if (resctrl_arch_get_cdp_enabled(r->rid)) { list_for_each_entry(d, &s->res->ctrl_domains, hdr.list) { cfg = &d->staged_config[s->conf_type]; cfg_peer = &d->staged_config[peer_type]; cfg_peer->new_ctrl = cfg->new_ctrl; cfg_peer->have_new_ctrl = cfg->have_new_ctrl; }
        }

Quoted 7 lines

Generally when feedback is provided it is good to check all places in series where
it is relevant. oh ... but looking ahead you ignored the feedback in the patch
it was given also :(

My bad.

I will address that.

Thanks

Babu

Quoted 28 lines · Babu

Quoted 26 lines · Reinette Chatre

Quoted 11 lines · Babu Moger

+    enum resctrl_conf_type peer_type;
+    struct resctrl_schema *peer_s;
+    int ret;

fs/resctrl/ctrlmondata.c +8 -0

+
+    rdt_staged_configs_clear();
+
+    ret = rdtgroup_init_cat(s, closid);
+    if (ret < 0)
+        goto out;
+
+    /* Initialize schema for both CDP_DATA and CDP_CODE when CDP is

Quoted 6 lines

enabled */
+    if (resctrl_arch_get_cdp_enabled(r->rid)) { +        peer_type = resctrl_peer_type(s->conf_type); +        peer_s = resctrl_get_schema(peer_type); +        if (peer_s) {
+            ret = rdtgroup_init_cat(peer_s, closid);

This is unexpected. In v7 I suggested that when parsing the CBM of one of the CDP
resources it is not necessary to do so again for the peer. The CBM can be parsed *once* and the configuration just copied over. See:
https://lore.kernel.org/
lkml/[email protected]/

Quoted 6 lines

Let met try to understand.

So, rdtgroup_init_cat() sets up the staged _config for the specific CDP type for all the domains.

We need to apply those staged_configs to its peer type on all the domains.

Quoted 13 lines

Something like this?

/* Initialize staged_config of the peer type when CDP is enabled */ if (resctrl_arch_get_cdp_enabled(r->rid)) { list_for_each_entry(d, &s->res->ctrl_domains, hdr.list) { cfg = &d->staged_config[s->conf_type]; cfg_peer = &d->staged_config[peer_type]; cfg_peer->new_ctrl = cfg->new_ctrl; cfg_peer->have_new_ctrl = cfg->have_new_ctrl; }
        }

Replaced with following snippet.

/* Initialize schema for both CDP_DATA and CDP_CODE when CDP is enabled */

Hi Babu,

Quoted 28 lines · Babu

Quoted 27 lines · Babu

Quoted 16 lines · Reinette Chatre

Quoted 15 lines · Babu Moger

+    enum resctrl_conf_type peer_type;
+    struct resctrl_schema *peer_s;
+    int ret;

fs/resctrl/ctrlmondata.c +8 -0

+
+    rdt_staged_configs_clear();
+
+    ret = rdtgroup_init_cat(s, closid);
+    if (ret < 0)
+        goto out;
+
+    /* Initialize schema for both CDP_DATA and CDP_CODE when CDP is

enabled */
+    if (resctrl_arch_get_cdp_enabled(r->rid)) { +        peer_type = resctrl_peer_type(s->conf_type); +        peer_s = resctrl_get_schema(peer_type);

Quoted 8 lines

Quoted 2 lines

+        if (peer_s) {
+            ret = rdtgroup_init_cat(peer_s, closid);

This is unexpected. In v7 I suggested that when parsing the CBM of one of the CDP
resources it is not necessary to do so again for the peer. The CBM can be parsed *once* and the configuration just copied over. See:
https://lore.kernel.org/
lkml/[email protected]/

Let met try to understand.

Quoted 4 lines

Quoted 4 lines

So, rdtgroup_init_cat() sets up the staged _config for the specific CDP type for all the domains.

We need to apply those staged_configs to its peer type on all the domains.

To put it more directly, this implementation keeps the CBM of CDP_CODE and CDP_DATA in sync. Skipping the unnecessary and duplicate parsing and instead copying the CBM from one to the other makes that obvious.

Quoted 13 lines

Quoted 13 lines

Something like this?

/* Initialize staged_config of the peer type when CDP is enabled */
        if (resctrl_arch_get_cdp_enabled(r->rid)) { list_for_each_entry(d, &s->res->ctrl_domains, hdr.list) { cfg = &d->staged_config[s->conf_type]; cfg_peer = &d->staged_config[peer_type]; cfg_peer->new_ctrl = cfg->new_ctrl; cfg_peer->have_new_ctrl = cfg->have_new_ctrl; }
        }

Quoted 3 lines

Replaced with following snippet.

/* Initialize schema for both CDP_DATA and CDP_CODE when CDP is enabled */

Could this be more specific? For example,
"Keep CDP_CODE and CDP_DATA of io_alloc CLOSID's CBM in sync."

Review

Quoted 7 lines

(quoted diff) +6 -0

+	if (resctrl_arch_get_cdp_enabled(r->rid)) {
+		peer_type = resctrl_peer_type(s->conf_type);
+		list_for_each_entry(d, &s->res->ctrl_domains, hdr.list)
+			memcpy(&d->staged_config[peer_type],
+			       &d->staged_config[s->conf_type],
+			       sizeof(*d->staged_config));

This looks good to me. To make it obvious what types are dealt with this can instead use sizeof(d->staged_config[0]).

"io_alloc" feature in resctrl enables direct insertion of data from I/O devices into the cache.

On AMD systems, when io_alloc is enabled, the highest CLOSID is reserved exclusively for I/O allocation traffic and is no longer available for general CPU cache allocation. Users are encouraged to enable it only when running workloads that can benefit from this functionality.

Since CLOSIDs are managed by resctrl fs, it is least invasive to make the "io_alloc is supported by maximum supported CLOSID" part of the initial resctrl fs support for io_alloc. Take care not to expose this use of CLOSID for io_alloc to user space so that this is not required from other architectures that may support io_alloc differently in the future.

Introduce user interface to enable/disable io_alloc feature.

Signed-off-by Babu Moger <[email protected]>

fs/resctrl/ctrlmondata.c +26 -0

+ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
+			       size_t nbytes, loff_t off)
+{
+	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
+	struct rdt_resource *r = s->res;
+	char const *grp_name;
+	u32 io_alloc_closid;
+	bool enable;
+	int ret;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret)
+		return ret;
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+
+	rdt_last_cmd_clear();
+
+	if (!r->cache.io_alloc_capable) {
+		rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+
+	io_alloc_closid = resctrl_io_alloc_closid(r);

fs/resctrl/ctrlmondata.c +29 -0

+
+	if (enable) {
+		if (!closid_alloc_fixed(io_alloc_closid)) {
+			grp_name = rdtgroup_name_by_closid(io_alloc_closid);
+			WARN_ON_ONCE(!grp_name);
+			rdt_last_cmd_printf("CLOSID (ctrl_hw_id) %d for io_alloc is used by %s group\n",
+					    io_alloc_closid, grp_name ? grp_name : "another");
+			ret = -ENOSPC;
+			goto out_unlock;
+		}
+
+		ret = resctrl_io_alloc_init_cbm(s, io_alloc_closid);
+		if (ret) {
+			rdt_last_cmd_puts("Failed to initialize io_alloc allocations\n");
+			closid_free(io_alloc_closid);
+			goto out_unlock;
+		}
+	} else {
+		closid_free(io_alloc_closid);
+	}
+
+	ret = resctrl_arch_io_alloc_enable(r, enable);
+
+out_unlock:
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+
+	return ret ?: nbytes;
+}

Hello Babu,

Quoted 17 lines · Babu Moger

"io_alloc" feature in resctrl enables direct insertion of data from I/O devices into the cache.

On AMD systems, when io_alloc is enabled, the highest CLOSID is reserved exclusively for I/O allocation traffic and is no longer available for general CPU cache allocation. Users are encouraged to enable it only when running workloads that can benefit from this functionality.

Since CLOSIDs are managed by resctrl fs, it is least invasive to make the "io_alloc is supported by maximum supported CLOSID" part of the initial resctrl fs support for io_alloc. Take care not to expose this use of CLOSID for io_alloc to user space so that this is not required from other architectures that may support io_alloc differently in the future.

Introduce user interface to enable/disable io_alloc feature.

Signed-off-by Babu Moger <[email protected]>

[..snip..]

Quoted 29 lines

fs/resctrl/ctrlmondata.c +29 -0

+ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf,
+			       size_t nbytes, loff_t off)
+{
+	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
+	struct rdt_resource *r = s->res;
+	char const *grp_name;
+	u32 io_alloc_closid;
+	bool enable;
+	int ret;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret)
+		return ret;
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+
+	rdt_last_cmd_clear();
+
+	if (!r->cache.io_alloc_capable) {
+		rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+
+	io_alloc_closid = resctrl_io_alloc_closid(r);
+	if (!resctrl_io_alloc_closid_supported(io_alloc_closid)) {
+		rdt_last_cmd_printf("io_alloc CLOSID (ctrl_hw_id) %d is not available\n",
+				    io_alloc_closid);

fs/resctrl/ctrlmondata.c int resctrl_io_alloc_show(struct kernfs_open_file *of, struct seq_file *seq, voi · line 795

Quoted 7 lines

fs/resctrl/ctrlmondata.c +7 -0

+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	/* If the feature is already up to date, no action is needed. */
+	if (resctrl_arch_get_io_alloc_enabled(r) == enable)
+		goto out_unlock;

Does it make sense to move this check before calling resctrl_io_alloc_closid(r) ?

Quoted 31 lines

fs/resctrl/ctrlmondata.c +29 -0

+
+	if (enable) {
+		if (!closid_alloc_fixed(io_alloc_closid)) {
+			grp_name = rdtgroup_name_by_closid(io_alloc_closid);
+			WARN_ON_ONCE(!grp_name);
+			rdt_last_cmd_printf("CLOSID (ctrl_hw_id) %d for io_alloc is used by %s group\n",
+					    io_alloc_closid, grp_name ? grp_name : "another");
+			ret = -ENOSPC;
+			goto out_unlock;
+		}
+
+		ret = resctrl_io_alloc_init_cbm(s, io_alloc_closid);
+		if (ret) {
+			rdt_last_cmd_puts("Failed to initialize io_alloc allocations\n");
+			closid_free(io_alloc_closid);
+			goto out_unlock;
+		}
+	} else {
+		closid_free(io_alloc_closid);
+	}
+
+	ret = resctrl_arch_io_alloc_enable(r, enable);
+
+out_unlock:
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+
+	return ret ?: nbytes;
+}

[..snip..]

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

Add "io_alloc_cbm" resctrl file to display CBMs (Capacity Bit Mask) of io_alloc feature.

The CBM interface file io_alloc_cbm resides in the info directory (e.g., /sys/fs/resctrl/info/L3/). Displaying the resource name is not necessary. Pass the resource name to show_doms() and print it only if

the name is valid. For io_alloc, pass NULL to suppress printing the resource name.

When CDP is enabled, io_alloc routes traffic using the highest CLOSID associated with the L3CODE resource. To ensure consistent cache allocation behavior, the L3CODE and L3DATA resources must remain synchronized.

rdtgroup_init_cat() function takes both L3CODE and L3DATA into account when

initializing CBMs for new groups. The io_alloc feature adheres to this same principle, meaning that the Cache Bit Masks (CBMs) accessed through either L3CODE or L3DATA will reflect identical values.

Signed-off-by Babu Moger <[email protected]>

---

---

fs/resctrl/ctrlmondata.c +13 -0

+int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq, void *v)
+{
+	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
+	struct rdt_resource *r = s->res;
+	int ret = 0;
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+
+	rdt_last_cmd_clear();
+
+	if (!r->cache.io_alloc_capable) {
+		rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);

Hi Babu,

Quoted 3 lines · Babu Moger

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

Quoted 2 lines

Add "io_alloc_cbm" resctrl file to display CBMs (Capacity Bit Mask) of io_alloc feature.

This is a bit vague. How about:
Add "io_alloc_cbm" resctrl file to display the Capacity Bit Masks (CBMs) that represent the portion of each cache instance allocated for I/O traffic.

Quoted 4 lines

The CBM interface file io_alloc_cbm resides in the info directory (e.g., /sys/fs/resctrl/info/L3/). Displaying the resource name is not necessary. Pass the resource name to show_doms() and print it only if

"Displaying the resource name is not necessary." -> "Since the resource name is part of the path it is not necessary to display the resource name as done in the schemata file."?

Quoted 8 lines

the name is valid. For io_alloc, pass NULL to suppress printing the resource name.

When CDP is enabled, io_alloc routes traffic using the highest CLOSID associated with the L3CODE resource. To ensure consistent cache allocation behavior, the L3CODE and L3DATA resources must remain synchronized.

"must remain synchronized" -> "are kept in sync"

Quoted 2 lines

rdtgroup_init_cat() function takes both L3CODE and L3DATA into account when

I do not understand this part. rdtgroup_init_cat() is part of current implementation and it takes L3CODE and L3DATE of _other_ CLOSID into account when determining what CBM to initialize new CLOSID with. How is that relevant here? I wonder if you are not perhaps trying to say:
"resctrl_io_alloc_init_cbm() initializes L3CODE and L3DATA of highest CLOSID
with the same CBM."
I do not think this is necessary to include here though since this is what the previous patch does and just saying that L3CODE and L3DATA are kept in sync is sufficient here.

Quoted 4 lines

initializing CBMs for new groups. The io_alloc feature adheres to this same principle, meaning that the Cache Bit Masks (CBMs) accessed through either L3CODE or L3DATA will reflect identical values.

I do not understand what you are trying to say here. What do you mean with "same principle"? The fact that L3CODE and L3DATA are kept in sync is part of io_alloc only, no?

Quoted 4 lines

Signed-off-by Babu Moger <[email protected]>

---

...

Quoted 1 line

---

...

Quoted 20 lines

fs/resctrl/ctrlmondata.c +20 -0

+int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq, void *v)
+{
+	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
+	struct rdt_resource *r = s->res;
+	int ret = 0;
+
+	cpus_read_lock();
+	mutex_lock(&rdtgroup_mutex);
+
+	rdt_last_cmd_clear();
+
+	if (!r->cache.io_alloc_capable) {
+		rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+
+	if (!resctrl_arch_get_io_alloc_enabled(r)) {
+		rdt_last_cmd_printf("io_alloc is not enabled on %s\n", s->name);
+		ret = -EINVAL;

fs/resctrl/ctrlmondata.c ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf, · line 844

Quoted 3 lines

fs/resctrl/ctrlmondata.c +3 -0

+		goto out_unlock;
+	}
+

Could you please add a comment here that explains to the reader that CBMs of L3CODE and L3DATA are kept in sync elsewhere and the io_alloc CBMs displayed from either CDP resource are thus identical and accurately reflect the CBMs used for I/O.

fs/resctrl/ctrlmondata.c ssize_t resctrl_io_alloc_write(struct kernfs_open_file *of, char *buf, · line 857

Quoted 8 lines

fs/resctrl/ctrlmondata.c +7 -0

+	show_doms(seq, s, NULL, resctrl_io_alloc_closid(r));
+
+out_unlock:
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+	return ret;
+}

Reinette

parse_cbm() and parse_bw() require mode and CLOSID to validate the Capacity

Bit Mask (CBM). It is passed via struct rdtgroup in struct rdt_parse_data.

The io_alloc feature also uses CBMs to indicate which portions of cache are allocated for I/O traffic. The CBMs are provided by user space and need to be validated the same as CBMs provided for general (CPU) cache allocation. parse_cbm() cannot be used as-is since io_alloc does not have rdtgroup context.

Pass the mode and CLOSID directly to parse_cbm() via struct rdt_parse_data instead of through the rdtgroup struct to facilitate calling parse_cbm() to verify the CBM of the io_alloc feature.

Signed-off-by Babu Moger <[email protected]>

---

fs/resctrl/ctrlmondata.c +3 -2

@@ -156,9 +157,10 @@ static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
 static int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
 		     struct rdt_ctrl_domain *d)
 {
-	struct rdtgroup *rdtgrp = data->rdtgrp;
+	enum rdtgrp_mode mode = data->mode;
 	struct resctrl_staged_config *cfg;
 	struct rdt_resource *r = s->res;
+	u32 closid = data->closid;
 	u32 cbm_val;
 
 	cfg = &d->staged_config[s->conf_type];

@@ -171,7 +173,7 @@ static int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
 	 * Cannot set up more than one pseudo-locked region in a cache
 	 * hierarchy.
 	 */
-	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP &&
+	if (mode == RDT_MODE_PSEUDO_LOCKSETUP &&
 	    rdtgroup_pseudo_locked_in_hierarchy(d)) {
 		rdt_last_cmd_puts("Pseudo-locked region in hierarchy\n");
 		return -EINVAL;

Hi Babu,

Quoted 1 line · Babu Moger

parse_cbm() and parse_bw() require mode and CLOSID to validate the Capacity

Again [1], parse_bw() does not validate any CBMs.

To be more specific: "mode" -> "resource group mode"?

Quoted 15 lines

Bit Mask (CBM). It is passed via struct rdtgroup in struct rdt_parse_data.

The io_alloc feature also uses CBMs to indicate which portions of cache are allocated for I/O traffic. The CBMs are provided by user space and need to be validated the same as CBMs provided for general (CPU) cache allocation. parse_cbm() cannot be used as-is since io_alloc does not have rdtgroup context.

Pass the mode and CLOSID directly to parse_cbm() via struct rdt_parse_data instead of through the rdtgroup struct to facilitate calling parse_cbm() to verify the CBM of the io_alloc feature.

Signed-off-by Babu Moger <[email protected]>

---

...

Quoted 25 lines

fs/resctrl/ctrlmondata.c +3 -2

@@ -156,9 +157,10 @@ static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
 static int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
 		     struct rdt_ctrl_domain *d)
 {
-	struct rdtgroup *rdtgrp = data->rdtgrp;
+	enum rdtgrp_mode mode = data->mode;
 	struct resctrl_staged_config *cfg;
 	struct rdt_resource *r = s->res;
+	u32 closid = data->closid;
 	u32 cbm_val;
 
 	cfg = &d->staged_config[s->conf_type];

@@ -171,7 +173,7 @@ static int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
 	 * Cannot set up more than one pseudo-locked region in a cache
 	 * hierarchy.
 	 */
-	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP &&
+	if (mode == RDT_MODE_PSEUDO_LOCKSETUP &&
 	    rdtgroup_pseudo_locked_in_hierarchy(d)) {
 		rdt_last_cmd_puts("Pseudo-locked region in hierarchy\n");
 		return -EINVAL;

@@ -180,8 +182,8 @@ static int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
 	if (!cbm_validate(data->buf, &cbm_val, r))
 		return -EINVAL;
 

fs/resctrl/ctrlmondata.c static int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s, · line 186

Quoted 4 lines

fs/resctrl/ctrlmondata.c +2 -2

-	if ((rdtgrp->mode == RDT_MODE_EXCLUSIVE ||
-	     rdtgrp->mode == RDT_MODE_SHAREABLE) &&
+	if ((mode == RDT_MODE_EXCLUSIVE ||
+	     mode == RDT_MODE_SHAREABLE) &&

This can now be on one line?

fs/resctrl/ctrlmondata.c static int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s, · line 189

Quoted 4 lines

fs/resctrl/ctrlmondata.c +0 -0

 	    rdtgroup_cbm_overlaps_pseudo_locked(d, cbm_val)) {
 		rdt_last_cmd_puts("CBM overlaps with pseudo-locked region\n");
 		return -EINVAL;

Reinette

[1] https://lore.kernel.org/lkml/[email protected]/

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

When CDP is enabled, io_alloc routes traffic using the highest CLOSID associated with the L3CODE resource. To ensure consistent cache allocation behavior, the L3CODE and L3DATA resources must remain synchronized. rdtgroup_init_cat() function takes both L3CODE and L3DATA into account when initializing CBMs for new groups. The io_alloc feature must maintain the same behavior, ensuring that the Cache Bit Masks (CBMs) for both L3CODE and L3DATA are updated together.

Enable users to modify io_alloc CBMs (Capacity Bit Masks) via the io_alloc_cbm resctrl file when io_alloc is enabled.

Signed-off-by Babu Moger <[email protected]>

---

---
 Documentation/filesystems/resctrl.rst |  8 +++
 fs/resctrl/ctrlmondata.c              | 97 +++++++++++++++++++++++++++
 fs/resctrl/internal.h                 |  3 +
 fs/resctrl/rdtgroup.c                 |  3 +-
 4 files changed, 110 insertions(+), 1 deletion(-)

Documentation/filesystems/resctrl.rst +0 -0

index 3002f7fdb2fe..d955e8525af0 100644

Documentation/filesystems/resctrl.rst +3 -0

+			# cat /sys/fs/resctrl/info/L3/io_alloc_cbm
+			0=ffff;1=00ff
+
 		When CDP is enabled "io_alloc_cbm" associated with the DATA and CODE
 		resources may reflect the same values. For example, values read from and
 		written to /sys/fs/resctrl/info/L3DATA/io_alloc_cbm may be reflected by

fs/resctrl/ctrlmondata.c +29 -0

index 641094aac322..1f69117f96f8 100644
@@ -858,3 +858,100 @@ int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq,
 	cpus_read_unlock();
 	return ret;
 }
+
+static int resctrl_io_alloc_parse_line(char *line,  struct rdt_resource *r,
+				       struct resctrl_schema *s, u32 closid)
+{
+	enum resctrl_conf_type peer_type;
+	struct resctrl_schema *peer_s;
+	struct rdt_parse_data data;
+	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) {
+			data.buf = dom;
+			data.mode = RDT_MODE_SHAREABLE;
+			data.closid = closid;
+			if (parse_cbm(&data, s, d))

fs/resctrl/ctrlmondata.c +24 -0

+			}
+			goto next;
+		}
+	}
+
+	return -EINVAL;
+}
+
+ssize_t resctrl_io_alloc_cbm_write(struct kernfs_open_file *of, char *buf,
+				   size_t nbytes, loff_t off)
+{
+	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
+	struct rdt_resource *r = s->res;
+	u32 io_alloc_closid;
+	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);

fs/resctrl/ctrlmondata.c +13 -0

+
+	if (!resctrl_arch_get_io_alloc_enabled(r)) {
+		rdt_last_cmd_printf("io_alloc is not enabled on %s\n", s->name);
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+
+	io_alloc_closid = resctrl_io_alloc_closid(r);
+
+	ret = resctrl_io_alloc_parse_line(buf, r, s, io_alloc_closid);
+
+	if (ret)
+		goto out_unlock;

Hi Babu,

Quoted 5 lines · Babu Moger

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

Quoted 7 lines

When CDP is enabled, io_alloc routes traffic using the highest CLOSID associated with the L3CODE resource. To ensure consistent cache allocation behavior, the L3CODE and L3DATA resources must remain synchronized. rdtgroup_init_cat() function takes both L3CODE and L3DATA into account when initializing CBMs for new groups. The io_alloc feature must maintain the same behavior, ensuring that the Cache Bit Masks (CBMs) for both L3CODE and L3DATA are updated together.

Please rework this copy&pasted text also and make specific to what this patch actually does.

Quoted 3 lines

Enable users to modify io_alloc CBMs (Capacity Bit Masks) via the io_alloc_cbm resctrl file when io_alloc is enabled.

Here the changelog can provide overview of what is done by this patch when a user provides a new CBM. This can include that a CBM provided to CDP enabled resource will copy the CBM to the CDP peer.

Quoted 22 lines

Signed-off-by Babu Moger <[email protected]>

---

---
 Documentation/filesystems/resctrl.rst |  8 +++
 fs/resctrl/ctrlmondata.c              | 97 +++++++++++++++++++++++++++
 fs/resctrl/internal.h                 |  3 +
 fs/resctrl/rdtgroup.c                 |  3 +-
 4 files changed, 110 insertions(+), 1 deletion(-)

Documentation/filesystems/resctrl.rst +0 -0

index 3002f7fdb2fe..d955e8525af0 100644
@@ -187,6 +187,14 @@ related to allocation:
 			# cat /sys/fs/resctrl/info/L3/io_alloc_cbm
 			0=ffff;1=ffff
 

Documentation/filesystems/resctrl.rst related to allocation: · line 194

Quoted 5 lines

Documentation/filesystems/resctrl.rst +5 -0

+		CBMs can be configured by writing to the interface.
+
+		Example::
+
+			# echo 1=FF > /sys/fs/resctrl/info/L3/io_alloc_cbm

It may be useful to demonstrate syntax when more than one CBM is modified.

Quoted 50 lines

Documentation/filesystems/resctrl.rst +3 -0

+			# cat /sys/fs/resctrl/info/L3/io_alloc_cbm
+			0=ffff;1=00ff
+
 		When CDP is enabled "io_alloc_cbm" associated with the DATA and CODE
 		resources may reflect the same values. For example, values read from and
 		written to /sys/fs/resctrl/info/L3DATA/io_alloc_cbm may be reflected by

fs/resctrl/ctrlmondata.c +35 -0

index 641094aac322..1f69117f96f8 100644
@@ -858,3 +858,100 @@ int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq,
 	cpus_read_unlock();
 	return ret;
 }
+
+static int resctrl_io_alloc_parse_line(char *line,  struct rdt_resource *r,
+				       struct resctrl_schema *s, u32 closid)
+{
+	enum resctrl_conf_type peer_type;
+	struct resctrl_schema *peer_s;
+	struct rdt_parse_data data;
+	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) {
+			data.buf = dom;
+			data.mode = RDT_MODE_SHAREABLE;
+			data.closid = closid;
+			if (parse_cbm(&data, s, d))
+				return -EINVAL;
+			/*
+			 * When CDP is enabled, update the schema for both CDP_DATA
+			 * and CDP_CODE.
+			 */
+			if (resctrl_arch_get_cdp_enabled(r->rid)) {

fs/resctrl/ctrlmondata.c int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq, · line 899

Quoted 4 lines

fs/resctrl/ctrlmondata.c +4 -0

+				peer_type = resctrl_peer_type(s->conf_type);
+				peer_s = resctrl_get_schema(peer_type);
+				if (parse_cbm(&data, peer_s, d))
+					return -EINVAL;

CBM is still parsed twice. As I mentioned in v7 the parsing only need to be done once and then the resulting CBM can be copied to CDP peer.
https://lore.kernel.org/lkml/[email protected]/

Quoted 29 lines

fs/resctrl/ctrlmondata.c +28 -0

+			}
+			goto next;
+		}
+	}
+
+	return -EINVAL;
+}
+
+ssize_t resctrl_io_alloc_cbm_write(struct kernfs_open_file *of, char *buf,
+				   size_t nbytes, loff_t off)
+{
+	struct resctrl_schema *s = rdt_kn_parent_priv(of->kn);
+	struct rdt_resource *r = s->res;
+	u32 io_alloc_closid;
+	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();
+
+	if (!r->cache.io_alloc_capable) {

fs/resctrl/ctrlmondata.c int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq, · line 933

Quoted 6 lines

fs/resctrl/ctrlmondata.c +6 -0

+		rdt_last_cmd_printf("io_alloc is not supported on %s\n", s->name);
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+
+	rdt_last_cmd_clear();

Unnecessary rdt_last_cmd_clear().

fs/resctrl/ctrlmondata.c int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq, · line 934

Quoted 2 lines

fs/resctrl/ctrlmondata.c +1 -0

+	rdt_staged_configs_clear();

Placement of this can be improved by putting it closer to the code that populates the staged configs. That is, just before resctrl_io_alloc_parse_line(). The flow is not symmetrical in that the out_unlock exit code always clears the buffer whether it was used or not. I think it will be easier to understand if the out_unlock *only* unlocks the locks and there is a new goto label, for example, "out_clear_configs" that calls rdt_staged_configs_clear() and is used after resctrl_io_alloc_parse_line().

Quoted 14 lines

fs/resctrl/ctrlmondata.c +13 -0

+
+	if (!resctrl_arch_get_io_alloc_enabled(r)) {
+		rdt_last_cmd_printf("io_alloc is not enabled on %s\n", s->name);
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+
+	io_alloc_closid = resctrl_io_alloc_closid(r);
+
+	ret = resctrl_io_alloc_parse_line(buf, r, s, io_alloc_closid);
+
+	if (ret)
+		goto out_unlock;

fs/resctrl/ctrlmondata.c int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq, · line 957

Quoted 10 lines

fs/resctrl/ctrlmondata.c +10 -0

+
+	ret = resctrl_arch_update_domains(r, io_alloc_closid);
+
+out_unlock:
+	rdt_staged_configs_clear();
+	mutex_unlock(&rdtgroup_mutex);
+	cpus_read_unlock();
+
+	return ret ?: nbytes;
+}

Reinette

When the io_alloc feature is enabled, a portion of the cache can be configured for shared use between hardware and software.

Update the bit_usage representation to reflect the io_alloc configuration.

Signed-off-by Babu Moger <[email protected]>

---

---
 Documentation/filesystems/resctrl.rst | 19 +++++++++++++------
 fs/resctrl/ctrlmondata.c              |  2 +-
 fs/resctrl/internal.h                 |  2 ++
 fs/resctrl/rdtgroup.c                 | 18 ++++++++++++++++--
 4 files changed, 32 insertions(+), 9 deletions(-)

Documentation/filesystems/resctrl.rst +4 -6

index d955e8525af0..1e46231b929c 100644
@@ -90,12 +90,19 @@ related to allocation:
 		must be set when writing a mask.
 
 "shareable_bits":
-		Bitmask of shareable resource with other executing
-		entities (e.g. I/O). User can use this when
-		setting up exclusive cache partitions. Note that
-		some platforms support devices that have their
-		own settings for cache use which can over-ride
-		these bits.
+		Bitmask of shareable resource with other executing entities
+		(e.g. I/O). Applies to all instances of this resource. User
+		can use this when setting up exclusive cache partitions.
+		Note that some platforms support devices that have their

Documentation/filesystems/resctrl.rst +0 -0

 		Annotated capacity bitmasks showing how all
 		instances of the resource are used. The legend is:

fs/resctrl/ctrlmondata.c +1 -1

index 1f69117f96f8..cfa766319189 100644
@@ -754,7 +754,7 @@ static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32 closid)
  * resource. Note that if Code Data Prioritization (CDP) is enabled, the number
  * of available CLOSIDs is reduced by half.
  */
-static u32 resctrl_io_alloc_closid(struct rdt_resource *r)
+u32 resctrl_io_alloc_closid(struct rdt_resource *r)
 {
 	if (resctrl_arch_get_cdp_enabled(r->rid))
 		return resctrl_arch_get_num_closid(r) / 2  - 1;

fs/resctrl/internal.h +2 -0

index 5467c3ad1b6d..98b87725508b 100644
@@ -395,6 +395,8 @@ int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq,
 ssize_t resctrl_io_alloc_cbm_write(struct kernfs_open_file *of, char *buf,
 				   size_t nbytes, loff_t off);
 
+u32 resctrl_io_alloc_closid(struct rdt_resource *r);
+
 const char *rdtgroup_name_by_closid(int closid);
 
 #ifdef CONFIG_RESCTRL_FS_PSEUDO_LOCK

fs/resctrl/rdtgroup.c +4 -2

index a3ab2851c6da..b85c2622c94b 100644
@@ -1068,15 +1068,17 @@ static int rdt_bit_usage_show(struct kernfs_open_file *of,
 
 	cpus_read_lock();
 	mutex_lock(&rdtgroup_mutex);
-	hw_shareable = r->cache.shareable_bits;
 	list_for_each_entry(dom, &r->ctrl_domains, hdr.list) {
 		if (sep)
 			seq_putc(seq, ';');
+		hw_shareable = r->cache.shareable_bits;
 		sw_shareable = 0;
 		exclusive = 0;
 		seq_printf(seq, "%d=", dom->hdr.id);
 		for (i = 0; i < closids_supported(); i++) {
-			if (!closid_allocated(i))
+			if (!closid_allocated(i) ||
+			    (resctrl_arch_get_io_alloc_enabled(r) &&
+			     i == resctrl_io_alloc_closid(r)))
 				continue;
 			ctrl_val = resctrl_arch_get_config(r, dom, i,
 							   s->conf_type);

Hi Babu

Quoted 4 lines · Babu Moger

When the io_alloc feature is enabled, a portion of the cache can be configured for shared use between hardware and software.

Update the bit_usage representation to reflect the io_alloc configuration.

... but no update to the bit_usage docs :(

Quoted 3 lines

Signed-off-by Babu Moger <[email protected]>

---

..

Quoted 29 lines

---
  Documentation/filesystems/resctrl.rst | 19 +++++++++++++------
  fs/resctrl/ctrlmondata.c              |  2 +-
  fs/resctrl/internal.h                 |  2 ++
  fs/resctrl/rdtgroup.c                 | 18 ++++++++++++++++--
  4 files changed, 32 insertions(+), 9 deletions(-)

Documentation/filesystems/resctrl.rst +8 -6

index d955e8525af0..1e46231b929c 100644
@@ -90,12 +90,19 @@ related to allocation:
  		must be set when writing a mask.
 
  "shareable_bits":
-		Bitmask of shareable resource with other executing
-		entities (e.g. I/O). User can use this when
-		setting up exclusive cache partitions. Note that
-		some platforms support devices that have their
-		own settings for cache use which can over-ride
-		these bits.
+		Bitmask of shareable resource with other executing entities
+		(e.g. I/O). Applies to all instances of this resource. User
+		can use this when setting up exclusive cache partitions.
+		Note that some platforms support devices that have their
+		own settings for cache use which can over-ride these bits.
+
+		When "io_alloc" is enabled, a portion of each cache instance can
+		be configured for shared use between hardware and software.

Documentation/filesystems/resctrl.rst related to allocation: · line 106

Quoted 6 lines

Documentation/filesystems/resctrl.rst +5 -0

+		"bit_usage" should be used to see which portions of each cache
+		instance is configured for hardware use via "io_alloc" feature
+		because every cache instance can have its "io_alloc" bitmask
+		configured independently via io_alloc_cbm.
+
  "bit_usage":

More ignored feedback. I was expecting changes to the "bit_usage" section.
https://lore.kernel.org/lkml/[email protected]/

Quoted 55 lines

Documentation/filesystems/resctrl.rst +0 -0

  		Annotated capacity bitmasks showing how all
  		instances of the resource are used. The legend is:

fs/resctrl/ctrlmondata.c +1 -1

index 1f69117f96f8..cfa766319189 100644
@@ -754,7 +754,7 @@ static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32 closid)
   * resource. Note that if Code Data Prioritization (CDP) is enabled, the number
   * of available CLOSIDs is reduced by half.
   */
-static u32 resctrl_io_alloc_closid(struct rdt_resource *r)
+u32 resctrl_io_alloc_closid(struct rdt_resource *r)
  {
  	if (resctrl_arch_get_cdp_enabled(r->rid))
  		return resctrl_arch_get_num_closid(r) / 2  - 1;

fs/resctrl/internal.h +2 -0

index 5467c3ad1b6d..98b87725508b 100644
@@ -395,6 +395,8 @@ int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq,
  ssize_t resctrl_io_alloc_cbm_write(struct kernfs_open_file *of, char *buf,
  				   size_t nbytes, loff_t off);
 
+u32 resctrl_io_alloc_closid(struct rdt_resource *r);
+
  const char *rdtgroup_name_by_closid(int closid);
 
  #ifdef CONFIG_RESCTRL_FS_PSEUDO_LOCK

fs/resctrl/rdtgroup.c +4 -2

index a3ab2851c6da..b85c2622c94b 100644
@@ -1068,15 +1068,17 @@ static int rdt_bit_usage_show(struct kernfs_open_file *of,
 
  	cpus_read_lock();
  	mutex_lock(&rdtgroup_mutex);
-	hw_shareable = r->cache.shareable_bits;
  	list_for_each_entry(dom, &r->ctrl_domains, hdr.list) {
  		if (sep)
  			seq_putc(seq, ';');
+		hw_shareable = r->cache.shareable_bits;
  		sw_shareable = 0;
  		exclusive = 0;
  		seq_printf(seq, "%d=", dom->hdr.id);
  		for (i = 0; i < closids_supported(); i++) {
-			if (!closid_allocated(i))
+			if (!closid_allocated(i) ||
+			    (resctrl_arch_get_io_alloc_enabled(r) &&
+			     i == resctrl_io_alloc_closid(r)))
  				continue;
  			ctrl_val = resctrl_arch_get_config(r, dom, i,
  							   s->conf_type);

@@ -1104,6 +1106,18 @@ static int rdt_bit_usage_show(struct kernfs_open_file *of,
  				break;

fs/resctrl/rdtgroup.c static int rdt_bit_usage_show(struct kernfs_open_file *of, · line 1112

Quoted 6 lines

fs/resctrl/rdtgroup.c +4 -0

  			}
  		}
+
+		/*
+		 * When the "io_alloc" feature is enabled, a portion of the
+		 * cache is configured for shared use between hardware and software.

Please add snippet to comment that highlights that CBM of CDP peers are kept in sync and the CBM used for I/O can thus be obtained from either.

Hi Reinette,

Quoted 8 lines · Reinette Chatre

Hi Babu

Quoted 4 lines · Babu Moger

When the io_alloc feature is enabled, a portion of the cache can be configured for shared use between hardware and software.

Update the bit_usage representation to reflect the io_alloc configuration.

... but no update to the bit_usage docs :(

Yes. Missed that.  Will update the text.

Quoted 33 lines

Quoted 3 lines

Signed-off-by Babu Moger <[email protected]>

---

..

Quoted 28 lines

---
  Documentation/filesystems/resctrl.rst | 19 +++++++++++++------ fs/resctrl/ctrlmondata.c              |  2 +- fs/resctrl/internal.h                 |  2 ++ fs/resctrl/rdtgroup.c                 | 18 ++++++++++++++++--

  4 files changed, 32 insertions(+), 9 deletions(-)

Documentation/filesystems/resctrl.rst +0 -0

index d955e8525af0..1e46231b929c 100644
@@ -90,12 +90,19 @@ related to allocation:

          must be set when writing a mask.
    "shareable_bits":
-        Bitmask of shareable resource with other executing
-        entities (e.g. I/O). User can use this when
-        setting up exclusive cache partitions. Note that
-        some platforms support devices that have their
-        own settings for cache use which can over-ride
-        these bits.
+        Bitmask of shareable resource with other executing entities +        (e.g. I/O). Applies to all instances of this resource. User +        can use this when setting up exclusive cache partitions. +        Note that some platforms support devices that have their +        own settings for cache use which can over-ride these bits. +
+        When "io_alloc" is enabled, a portion of each cache instance can +        be configured for shared use between hardware and software.

Quoted 8 lines

Quoted 6 lines

+        "bit_usage" should be used to see which portions of each cache +        instance is configured for hardware use via "io_alloc" feature +        because every cache instance can have its "io_alloc" bitmask +        configured independently via io_alloc_cbm.
+
  "bit_usage":

More ignored feedback. I was expecting changes to the "bit_usage" section.
https://lore.kernel.org/lkml/[email protected]/

Yea.  Added following text now.

Documentation/filesystems/resctrl.rst +0 -0

index 273c9c9389a4..0a56829407c9 100644
@@ -124,8 +124,11 @@ related to allocation:

                        "X":
                              Corresponding region is available for sharing and

                              used by hardware and software. These are the -                             bits that appear in "shareable_bits" as -                             well as a resource group's allocation.
+                             bits that appear in "shareable_bits" as well as a +                             resource group's allocation. When io_alloc is enabled, +                             the capacity bitmasks configured via io_alloc_cbm will +                             also appear here, indicating the cache regions shared +                             with I/O devices.

Hi Babu,

Quoted 63 lines · Babu

Hi Reinette,

Quoted 8 lines · Reinette Chatre

Hi Babu

Quoted 4 lines · Babu Moger

When the io_alloc feature is enabled, a portion of the cache can be configured for shared use between hardware and software.

Update the bit_usage representation to reflect the io_alloc configuration.

... but no update to the bit_usage docs :(

Yes. Missed that.  Will update the text.

Quoted 32 lines

Quoted 2 lines

Signed-off-by Babu Moger <[email protected]>

---

..

Quoted 28 lines

---
   Documentation/filesystems/resctrl.rst | 19 +++++++++++++------
   fs/resctrl/ctrlmondata.c              |  2 +-
   fs/resctrl/internal.h                 |  2 ++
   fs/resctrl/rdtgroup.c                 | 18 ++++++++++++++++--
   4 files changed, 32 insertions(+), 9 deletions(-)

Documentation/filesystems/resctrl.rst +8 -6

index d955e8525af0..1e46231b929c 100644
@@ -90,12 +90,19 @@ related to allocation:
           must be set when writing a mask.
     "shareable_bits":
-        Bitmask of shareable resource with other executing
-        entities (e.g. I/O). User can use this when
-        setting up exclusive cache partitions. Note that
-        some platforms support devices that have their
-        own settings for cache use which can over-ride
-        these bits.
+        Bitmask of shareable resource with other executing entities
+        (e.g. I/O). Applies to all instances of this resource. User
+        can use this when setting up exclusive cache partitions.
+        Note that some platforms support devices that have their
+        own settings for cache use which can over-ride these bits.
+
+        When "io_alloc" is enabled, a portion of each cache instance can
+        be configured for shared use between hardware and software.

Quoted 8 lines

Documentation/filesystems/resctrl.rst related to allocation: · line 105

Quoted 6 lines

Documentation/filesystems/resctrl.rst +5 -0

+        "bit_usage" should be used to see which portions of each cache
+        instance is configured for hardware use via "io_alloc" feature
+        because every cache instance can have its "io_alloc" bitmask
+        configured independently via io_alloc_cbm.
+
   "bit_usage":

More ignored feedback. I was expecting changes to the "bit_usage" section.
https://lore.kernel.org/lkml/[email protected]/

Yea.  Added following text now.

Documentation/filesystems/resctrl.rst +0 -0

index 273c9c9389a4..0a56829407c9 100644

Documentation/filesystems/resctrl.rst related to allocation: · line 131

Quoted 10 lines

Documentation/filesystems/resctrl.rst +5 -2

@@ -124,8 +124,11 @@ related to allocation:
                         "X":
                               Corresponding region is available for sharing and
                               used by hardware and software. These are the
-                             bits that appear in "shareable_bits" as
-                             well as a resource group's allocation.
+                             bits that appear in "shareable_bits" as well as a
+                             resource group's allocation. When io_alloc is enabled,
+                             the capacity bitmasks configured via io_alloc_cbm will
+                             also appear here, indicating the cache regions shared
+                             with I/O devices.

This sounds as though when a bit is set in io_alloc_cbm then its corresponding position will contain an "X". An "X" will only be displayed if io_alloc_cbm has the corresponding bit set *and* the bit appears in a resource group's allocation.

I think that patch #7 adds a very good definition of io_alloc_cbm and that it is unnecessary to duplicate it. Simply referring to io_alloc_cbm should thus be fine with user's search for it in the doc leading to a good description.

Considering this, how about just:
"X":
Corresponding region is available for sharing and used by hardware and software. These are the bits that appear in "shareable_bits" or "io_alloc_cbm" as well as a resource group's allocation.

Please also update the "H" section.