| 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
| ||
|
Message-ID: <e5e8a8d4-3996-4513-abb8-ae632c416a24@arm.com>
Date: Thu, 8 Jan 2026 15:35:01 +0000
From: Ben Horgan <ben.horgan@....com>
To: Jonathan Cameron <jonathan.cameron@...wei.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
Hi Jonathan,
On 1/6/26 15:09, Jonathan Cameron wrote:
> 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.
Yeah, I don't think the default can make everyone happy.
>
>>
>> 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.
Fair point, I've moved this earlier.
>
>> + */
>> + 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.
I've squashed in a change from a later quirk which moves things out of
the 'if' scope and so only min and delta get moved inside.
>
>> +
>> + /*
>> + * 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);
>> + }
>> +}
>> +
>>
Thanks,
Ben
Powered by blists - more mailing lists