linux-kernel - Re: [PATCH v2 40/45] arm_mpam: Generate a configuration for min controls

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: <20260106150907.0000457b@huawei.com>
Date: Tue, 6 Jan 2026 15:09:07 +0000
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Ben Horgan <ben.horgan@....com>
CC: <amitsinght@...vell.com>, <baisheng.gao@...soc.com>,
	<baolin.wang@...ux.alibaba.com>, <carl@...amperecomputing.com>,
	<dave.martin@....com>, <david@...nel.org>, <dfustini@...libre.com>,
	<fenghuay@...dia.com>, <gshan@...hat.com>, <james.morse@....com>,
	<kobak@...dia.com>, <lcherian@...vell.com>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<peternewman@...gle.com>, <punit.agrawal@....qualcomm.com>,
	<quic_jiles@...cinc.com>, <reinette.chatre@...el.com>,
	<rohit.mathew@....com>, <scott@...amperecomputing.com>,
	<sdonthineni@...dia.com>, <tan.shaopeng@...itsu.com>,
	<xhao@...ux.alibaba.com>, <catalin.marinas@....com>, <will@...nel.org>,
	<corbet@....net>, <maz@...nel.org>, <oupton@...nel.org>,
	<joey.gouly@....com>, <suzuki.poulose@....com>, <kvmarm@...ts.linux.dev>,
	"Zeng Heng" <zengheng4@...wei.com>
Subject: Re: [PATCH v2 40/45] arm_mpam: Generate a configuration for min
 controls

On Fri, 19 Dec 2025 18:11:42 +0000
Ben Horgan <ben.horgan@....com> wrote:

> From: James Morse <james.morse@....com>
> 
> MPAM supports a minimum and maximum control for memory bandwidth. The
> purpose of the minimum control is to give priority to tasks that are below
> their minimum value. Resctrl only provides one value for the bandwidth
> configuration, which is used for the maximum.
> 
> The minimum control is always programmed to zero on hardware that supports
> it.
> 
> Generate a minimum bandwidth value that is 5% lower than the value provided
> by resctrl. This means tasks that are not receiving their target bandwidth
> can be prioritised by the hardware.

I'm not sure this policy make sense in general as there will be bandwidth
heavy background tasks where we want to make sure they only use up to X%
of bandwidth, but don't care if they don't get that.  I guess maybe those
are always in the %5 or less category so maybe this policy is fine.
Ah well, can revisit later I guess as long as we keep this as the default
for any new controls.

> 
> For component reset reuse the same calculation so that the default is a
> value resctrl can set.
> 
> CC: Zeng Heng <zengheng4@...wei.com>
> Signed-off-by: James Morse <james.morse@....com>
> Signed-off-by: Ben Horgan <ben.horgan@....com>

Most, maybe all the other cases have split test from implementation. I'd
probably do that here just for consistency.

A few things inline.

J



> ---
> Add reset_mbw_min
> Clear min cfg when setting max
> use mpam_extend_config on component reset
> ---
>  drivers/resctrl/mpam_devices.c      | 76 +++++++++++++++++++++++++++--
>  drivers/resctrl/mpam_internal.h     |  3 ++
>  drivers/resctrl/mpam_resctrl.c      |  2 +
>  drivers/resctrl/test_mpam_devices.c | 66 +++++++++++++++++++++++++
>  4 files changed, 142 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index c20b2757b86b..deea46c2a6c3 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -721,6 +721,13 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
>  			mpam_set_feature(mpam_feat_mbw_part, props);
>  
>  		props->bwa_wd = FIELD_GET(MPAMF_MBW_IDR_BWA_WD, mbw_features);
> +
> +		/*
> +		 * The BWA_WD field can represent 0-63, but the control fields it
> +		 * describes have a maximum of 16 bits.

I'm lost to why this only matters now!  Feels like that makes sense way earlier
in the series. Particularly as that detail confused me no end in the bandwidth
/ percent calculation patches.

> +		 */
> +		props->bwa_wd = min(props->bwa_wd, 16);
> +
>  		if (props->bwa_wd && FIELD_GET(MPAMF_MBW_IDR_HAS_MAX, mbw_features))
>  			mpam_set_feature(mpam_feat_mbw_max, props);
>  

> +static void mpam_extend_config(struct mpam_class *class, struct mpam_config *cfg)
> +{
> +	struct mpam_props *cprops = &class->props;
> +	u16 min, min_hw_granule, delta;
> +	u16 max_hw_value, res0_bits;

Probably makes sense to drag some of these into the scope below.

> +
> +	/*
> +	 * MAX and MIN should be set together. If only one is provided,
> +	 * generate a configuration for the other. If only one control
> +	 * type is supported, the other value will be ignored.
> +	 *
> +	 * Resctrl can only configure the MAX.
> +	 */
> +	if (mpam_has_feature(mpam_feat_mbw_max, cfg) &&
> +	    !mpam_has_feature(mpam_feat_mbw_min, cfg)) {
> +		/*
> +		 * Calculate the values the 'min' control can hold.
> +		 * e.g. on a platform with bwa_wd = 8, min_hw_granule is 0x00ff
> +		 * because those bits are RES0. Configurations of this value
> +		 * are effectively zero. But configurations need to saturate
> +		 * at min_hw_granule on systems with mismatched bwa_wd, where
> +		 * the 'less than 0' values are implemented on some MSC, but
> +		 * not others.
> +		 */
> +		res0_bits = 16 - cprops->bwa_wd;
> +		max_hw_value = ((1 << cprops->bwa_wd) - 1) << res0_bits;
> +		min_hw_granule = ~max_hw_value;
> +
> +		delta = ((5 * MPAMCFG_MBW_MAX_MAX) / 100) - 1;
> +		if (cfg->mbw_max > delta)
> +			min = cfg->mbw_max - delta;
> +		else
> +			min = 0;
> +
> +		cfg->mbw_min = max(min, min_hw_granule);
> +		mpam_set_feature(mpam_feat_mbw_min, cfg);
> +	}
> +}
> +
>