linux-kernel - Re: [RFC PATCH 13/38] arm_mpam: resctrl: Add CDP emulation

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: <20251218115815.00003f2c@huawei.com>
Date: Thu, 18 Dec 2025 11:58:15 +0000
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: James Morse <james.morse@....com>
CC: <linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>, D
 Scott Phillips OS <scott@...amperecomputing.com>,
	<carl@...amperecomputing.com>, <lcherian@...vell.com>,
	<bobo.shaobowang@...wei.com>, <tan.shaopeng@...itsu.com>,
	<baolin.wang@...ux.alibaba.com>, Jamie Iles <quic_jiles@...cinc.com>, "Xin
 Hao" <xhao@...ux.alibaba.com>, <peternewman@...gle.com>,
	<dfustini@...libre.com>, <amitsinght@...vell.com>, David Hildenbrand
	<david@...nel.org>, Dave Martin <dave.martin@....com>, Koba Ko
	<kobak@...dia.com>, Shanker Donthineni <sdonthineni@...dia.com>,
	<fenghuay@...dia.com>, <baisheng.gao@...soc.com>, Gavin Shan
	<gshan@...hat.com>, Ben Horgan <ben.horgan@....com>, <rohit.mathew@....com>,
	<reinette.chatre@...el.com>, Punit Agrawal <punit.agrawal@....qualcomm.com>
Subject: Re: [RFC PATCH 13/38] arm_mpam: resctrl: Add CDP emulation

On Fri, 5 Dec 2025 21:58:36 +0000
James Morse <james.morse@....com> wrote:

> Intel RDT's CDP feature allows the cache to use a different control value
> depending on whether the accesses was for instruction fetch or a data
> access. MPAM's equivalent feature is the other way up: the CPU assigns a
> different partid label to traffic depending on whether it was instruction
> fetch or a data access, which causes the cache to use a different control
> value based solely on the partid.
> 
> MPAM can emulate CDP, with the side effect that the alternative partid is
> seen by all MSC, it can't be enabled per-MSC.
> 
> Add the resctrl hooks to turn this on or off. Add the helpers that
> match a closid against a task, which need to be aware that the value
> written to hardware is not the same as the one resctrl is using.
> 
> Update the 'arm64_mpam_global_default' variable the arch code uses
> during context switch to know when the per-cpu value should be used
> instead.
> 
> Awkwardly, the MB controls don't implement CDP. To emulate this, the
> MPAM equivalent needs programming twice by the resctrl glue, as
> resctrl expects the bandwidth controls to be applied independently for
> both data and isntruction-fetch.
> 
> CC: Dave Martin <Dave.Martin@....com>
> CC: Ben Horgan <ben.horgan@....com>
> CC: Amit Singh Tomar <amitsinght@...vell.com>
> Signed-off-by: James Morse <james.morse@....com>
More trivial stuff. It is clearly that kind of a day.

> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> index c2650abb99ec..d5f75ed67e46 100644
> --- a/drivers/resctrl/mpam_resctrl.c
> +++ b/drivers/resctrl/mpam_resctrl.c


> +int resctrl_arch_set_cdp_enabled(enum resctrl_res_level ignored, bool enable)
> +{
> +	u32 partid_i, partid_d;
> +
> +	cdp_enabled = enable;
> +	partid_i = partid_d = RESCTRL_RESERVED_CLOSID;


I'd do these at the declarations.  The kernel style guide does also
say not to do multiple assignments on one line. Obviously this
one is fairly obviously fine but none the less...

> +
> +	if (enable) {
> +		u32 partid = RESCTRL_RESERVED_CLOSID;
> +
> +		partid_d = resctrl_get_config_index(partid, CDP_CODE);
> +		partid_i = resctrl_get_config_index(partid, CDP_DATA);
> +	}
> +
> +	mpam_set_task_partid_pmg(current, partid_d, partid_i, 0, 0);
> +	WRITE_ONCE(arm64_mpam_global_default, mpam_get_regval(current));
> +
> +	resctrl_reset_task_closids();
> +
> +	return 0;
> +}


> @@ -272,6 +369,7 @@ u32 resctrl_arch_get_config(struct rdt_resource *r, struct rdt_ctrl_domain *d,
>  int resctrl_arch_update_one(struct rdt_resource *r, struct rdt_ctrl_domain *d,
>  			    u32 closid, enum resctrl_conf_type t, u32 cfg_val)
>  {
> +	int err;
>  	u32 partid;
>  	struct mpam_config cfg;
>  	struct mpam_props *cprops;
> @@ -311,7 +409,22 @@ int resctrl_arch_update_one(struct rdt_resource *r, struct rdt_ctrl_domain *d,
>  		return -EINVAL;
>  	}
>  
> -	return mpam_apply_config(dom->ctrl_comp, partid, &cfg);
> +	/*
> +	 * When CDP is enabled, but the resource doesn't support it, we need to
> +	 * apply the same configuration to the other partid.
> +	 */
> +	if (mpam_resctrl_hide_cdp(r->rid)) {
> +		partid = resctrl_get_config_index(closid, CDP_CODE);
> +		err = mpam_apply_config(dom->ctrl_comp, partid, &cfg);
> +		if (err)
> +			return err;
> +
> +		partid = resctrl_get_config_index(closid, CDP_DATA);
> +		return mpam_apply_config(dom->ctrl_comp, partid, &cfg);

Given you returned, maybe drop the else which will reduce the diff
as well as a bonus.  Definitely drop this blank line.

> +
> +	} else {
> +		return mpam_apply_config(dom->ctrl_comp, partid, &cfg);
> +	}
>  }