lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <95d219be-c89b-431a-974a-2e2b653f98e4@amd.com>
Date: Thu, 18 Dec 2025 15:32:56 -0600
From: "Moger, Babu" <bmoger@....com>
To: Aaron Tomlin <atomlin@...mlin.com>, tony.luck@...el.com,
 reinette.chatre@...el.com, Dave.Martin@....com, james.morse@....com,
 babu.moger@....com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com
Cc: sean@...e.io, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] x86/resctrl: Add "*" shorthand to set minimum
 io_alloc CBM for all domains

Hi Aaron,

On 12/15/2025 5:02 PM, Aaron Tomlin wrote:
> This patch introduces a special domain ID selector "*" for io_alloc_cbm
> that allows setting the CBM of each cache domain to its minimum number
> of consecutive bits in a single operation. For example, writing "*=0" to
> /sys/fs/resctrl/info/L3/io_alloc_cbm programs each domain's CBM to the
> hardware-defined minimum, greatly simplifying automation and management
> tasks. The user is required to specify the correct minimum stored in
> /sys/fs/resctrl/info/L3/min_cbm_bits.
> 
> Signed-off-by: Aaron Tomlin <atomlin@...mlin.com>
> ---
>   Documentation/filesystems/resctrl.rst     |  10 ++
>   arch/x86/kernel/cpu/resctrl/core.c        |   2 +-
>   arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  23 ++--
>   fs/resctrl/ctrlmondata.c                  | 141 ++++++++++++++++------
>   fs/resctrl/internal.h                     |  13 ++
>   fs/resctrl/rdtgroup.c                     |   2 +-
>   include/linux/resctrl.h                   |  30 ++++-
>   7 files changed, 174 insertions(+), 47 deletions(-)
> 
> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
> index 8c8ce678148a..9c128c8fba86 100644
> --- a/Documentation/filesystems/resctrl.rst
> +++ b/Documentation/filesystems/resctrl.rst
> @@ -215,6 +215,16 @@ related to allocation:
>   			# cat /sys/fs/resctrl/info/L3/io_alloc_cbm
>   			0=00ff;1=000f
>   
> +		Set each CBM to their minimum number of consecutive bits.
> +
> +		A special value "*" is required to represent all cache IDs.
> +		The user should specify the correct minimum stored in
> +		/sys/fs/resctrl/info/L3/min_cbm_bits.
> +
> +		Example::
> +
> +			# echo "*=0" > /sys/fs/resctrl/info/L3/io_alloc_cbm
> +
>   		When CDP is enabled "io_alloc_cbm" associated with the CDP_DATA and CDP_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
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 3792ab4819dc..44aea6b534e0 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -276,7 +276,7 @@ static void rdt_get_cdp_config(int level)
>   
>   static void rdt_set_io_alloc_capable(struct rdt_resource *r)
>   {
> -	r->cache.io_alloc_capable = true;
> +	r->cache.io_alloc.io_alloc_capable = true;
>   }
>   
>   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 b20e705606b8..0f051d848422 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -57,14 +57,19 @@ int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
>   		hw_dom = resctrl_to_arch_ctrl_dom(d);
>   		msr_param.res = NULL;
>   		for (t = 0; t < CDP_NUM_TYPES; t++) {
> -			cfg = &hw_dom->d_resctrl.staged_config[t];
> -			if (!cfg->have_new_ctrl)
> -				continue;
> -
> -			idx = resctrl_get_config_index(closid, t);
> -			if (cfg->new_ctrl == hw_dom->ctrl_val[idx])
> -				continue;
> -			hw_dom->ctrl_val[idx] = cfg->new_ctrl;
> +			if (resctrl_should_io_alloc_min_cbm(r)) {
> +				idx = resctrl_get_config_index(closid, t);
> +				hw_dom->ctrl_val[idx] = apply_io_alloc_min_cbm(r);
> +			} else {
> +				cfg = &hw_dom->d_resctrl.staged_config[t];
> +				if (!cfg->have_new_ctrl)
> +					continue;
> +
> +				idx = resctrl_get_config_index(closid, t);
> +				if (cfg->new_ctrl == hw_dom->ctrl_val[idx])
> +					continue;
> +				hw_dom->ctrl_val[idx] = cfg->new_ctrl;
> +			}
>   
>   			if (!msr_param.res) {
>   				msr_param.low = idx;
> @@ -123,7 +128,7 @@ 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 &&
> +	if (hw_res->r_resctrl.cache.io_alloc.io_alloc_capable &&
>   	    hw_res->sdciae_enabled != enable) {
>   		_resctrl_sdciae_enable(r, enable);
>   		hw_res->sdciae_enabled = enable;
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index d2f9a4f2d887..a36b9055a1be 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -688,7 +688,7 @@ int resctrl_io_alloc_show(struct kernfs_open_file *of, struct seq_file *seq, voi
>   
>   	mutex_lock(&rdtgroup_mutex);
>   
> -	if (r->cache.io_alloc_capable) {
> +	if (r->cache.io_alloc.io_alloc_capable) {
>   		if (resctrl_arch_get_io_alloc_enabled(r))
>   			seq_puts(seq, "enabled\n");
>   		else
> @@ -712,13 +712,31 @@ static bool resctrl_io_alloc_closid_supported(u32 io_alloc_closid)
>   	return io_alloc_closid < closids_supported();
>   }
>   
> +/**
> + * resctrl_sync_cdp_peer - Mirror staged configuration to the CDP peer type
> + *
> + * @s: resctrl schema
> + * @d: resctrl control domain
> + *
> + * The caller must ensure CDP is enabled for the resource and must be
> + * called under the cpu hotplug lock and rdtgroup mutex
> + */
> +static void resctrl_sync_cdp_peer(struct resctrl_schema *s, struct rdt_ctrl_domain *d)
> +{
> +	enum resctrl_conf_type peer_type;
> +
> +	peer_type = resctrl_peer_type(s->conf_type);
> +	memcpy(&d->staged_config[peer_type],
> +	       &d->staged_config[s->conf_type],
> +	       sizeof(d->staged_config[0]));
> +}
> +
>   /*
>    * 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)
>   {
> -	enum resctrl_conf_type peer_type;
>   	struct rdt_resource *r = s->res;
>   	struct rdt_ctrl_domain *d;
>   	int ret;
> @@ -731,11 +749,8 @@ static int resctrl_io_alloc_init_cbm(struct resctrl_schema *s, u32 closid)
>   
>   	/* Keep CDP_CODE and CDP_DATA of io_alloc CLOSID's CBM in sync. */
>   	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[0]));
> +			resctrl_sync_cdp_peer(s, d);
>   	}
>   
>   	ret = resctrl_arch_update_domains(r, closid);
> @@ -903,49 +918,103 @@ int resctrl_io_alloc_cbm_show(struct kernfs_open_file *of, struct seq_file *seq,
>   	return ret;
>   }
>   
> +/**
> + * parse_domain_cbm - Parse "domain=cbm" and return either side
> + *
> + * @line_ptr: Pointer to the input string
> + * @out_val: Output from parsed value (either domain ID or CDM)
> + * @which: Selector for which side to parse
> + *
> + * It is assumed that *line_ptr and *out_val are valid.
> + *
> + * Return: 0 on success and advance *line_ptr to point past the
> + * delimiter, EINVAL on parsing error
> + */
> +static inline int parse_domain_cbm(char **line_ptr, unsigned long *out_val,
> +				   enum resctrl_kv_sel which)
> +{
> +	char *rhs, *lhs, *tok;
> +	unsigned int base;
> +
> +	rhs = *line_ptr;
> +
> +	lhs = strsep(&rhs, "=");
> +	if (!lhs || !rhs)
> +		goto err;
> +
> +	if (which == RDT_PARSE_DOMAIN) {
> +		tok = lhs;
> +		base = 10;
> +	} else {
> +		tok = rhs;
> +		base = 16;
> +	}
> +	tok = strim(tok);
> +
> +	if (kstrtoul(tok, base, out_val))
> +		goto err;
> +
> +	*line_ptr = rhs;
> +	return 0;
> +err:
> +	rdt_last_cmd_puts("Invalid domain=cbm: missing '=' or non-numeric value\n");
> +	return -EINVAL;
> +}
> +
>   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 rdt_parse_data data;
>   	struct rdt_ctrl_domain *d;
> -	char *dom = NULL, *id;
> -	unsigned long dom_id;
> +	char *cbm = NULL;
> +	unsigned long out_val;
> +	int ret;
>   
> -next:
> -	if (!line || line[0] == '\0')
> -		return 0;
> +	if (line[0] == '*') {
> +		ret = parse_domain_cbm(&line, &out_val, RDT_PARSE_CBM);
> +		if (ret)
> +			return ret;
>   
> -	dom = strsep(&line, ";");
> -	id = strsep(&dom, "=");
> -	if (!dom || kstrtoul(id, 10, &dom_id)) {
> -		rdt_last_cmd_puts("Missing '=' or non-numeric domain\n");
> +		if (out_val == r->cache.min_cbm_bits) {
> +			r->cache.io_alloc.io_alloc_min_cbm = true;
> +			return 0;
> +		}
> +		rdt_last_cmd_puts("Invalid io_alloc min CBM\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;
> -			/*
> -			 * Keep io_alloc CLOSID's CBM of CDP_CODE and CDP_DATA
> -			 * in sync.
> -			 */
> -			if (resctrl_arch_get_cdp_enabled(r->rid)) {
> -				peer_type = resctrl_peer_type(s->conf_type);
> -				memcpy(&d->staged_config[peer_type],
> -				       &d->staged_config[s->conf_type],
> -				       sizeof(d->staged_config[0]));
> +	while (line && line[0] != '\0') {
> +		bool found = false;
> +
> +		cbm = strsep(&line, ";");
> +		ret = parse_domain_cbm(&cbm, &out_val, RDT_PARSE_DOMAIN);
> +		if (ret)
> +			return ret;
> +
> +		cbm = strim(cbm);
> +		list_for_each_entry(d, &r->ctrl_domains, hdr.list) {
> +			if (d->hdr.id == out_val) {
> +				data.buf = cbm;
> +				data.mode = RDT_MODE_SHAREABLE;
> +				data.closid = closid;
> +				if (parse_cbm(&data, s, d))
> +					return -EINVAL;
> +				/*
> +				 * Keep io_alloc CLOSID's CBM of CDP_CODE and CDP_DATA
> +				 * in sync.
> +				 */
> +				if (resctrl_arch_get_cdp_enabled(r->rid))
> +					resctrl_sync_cdp_peer(s, d);
> +				found = true;
> +				break;
>   			}
> -			goto next;
>   		}
> +
> +		if (!found)
> +			return -EINVAL;
>   	}
>   
> -	return -EINVAL;
> +	return 0;
>   }

Can't we just simplify to this patch without adding new data structures?

diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
index f7608121f5e3..d113d8a3ee02 100644
--- a/fs/resctrl/ctrlmondata.c
+++ b/fs/resctrl/ctrlmondata.c
@@ -874,6 +874,7 @@ static int resctrl_io_alloc_parse_line(char *line, 
struct rdt_resource *r,
         struct rdt_ctrl_domain *d;
         char *dom = NULL, *id;
         unsigned long dom_id;
+       bool update_all;

  next:
         if (!line || line[0] == '\0')
@@ -881,14 +882,18 @@ static int resctrl_io_alloc_parse_line(char *line, 
  struct rdt_resource *r,

         dom = strsep(&line, ";");
         id = strsep(&dom, "=");
-       if (!dom || kstrtoul(id, 10, &dom_id)) {
+       update_all = false;
+
+       if (id && *id == '*') {
+               update_all = true;
+       } else 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 (update_all || (d->hdr.id == dom_id)) {
                         data.buf = dom;
                         data.mode = RDT_MODE_SHAREABLE;
                         data.closid = closid;


What do you think?

thanks
Babu


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ