Re: [RFC PATCH 33/38] arm_mpam: Generate a configuration for min controls

Ben Horgan
2025-12-09T16:45:22Z

Hi James,

On 12/5/25 21:58, James Morse wrote:
> 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.

To ensure that the min is always programmed we need to add a
reset_mbw_min to the reset_cfg for the ris level reset and give a value
in mpam_reset_component_cfg() for the component level reset.

>
> CC: Zeng Heng <zengheng4@huawei.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> drivers/resctrl/mpam_devices.c | 68 +++++++++++++++++++++++++++--
> drivers/resctrl/mpam_internal.h | 2 +
> drivers/resctrl/test_mpam_devices.c | 66 ++++++++++++++++++++++++++++
> 3 files changed, 132 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 1334093fc03e..741e14e1e6cf 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.
> + */
> + 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);
>
> @@ -1387,7 +1394,7 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>
> if (mpam_has_feature(mpam_feat_mbw_min, rprops) &&
> mpam_has_feature(mpam_feat_mbw_min, cfg))
> - mpam_write_partsel_reg(msc, MBW_MIN, 0);
> + mpam_write_partsel_reg(msc, MBW_MIN, cfg->mbw_min);
>
> if (mpam_has_feature(mpam_feat_mbw_max, rprops) &&
> mpam_has_feature(mpam_feat_mbw_max, cfg)) {
> @@ -2693,24 +2700,77 @@ static bool mpam_update_config(struct mpam_config *cfg,
> maybe_update_config(cfg, mpam_feat_cpor_part, newcfg, cpbm, has_changes);
> maybe_update_config(cfg, mpam_feat_mbw_part, newcfg, mbw_pbm, has_changes);
> maybe_update_config(cfg, mpam_feat_mbw_max, newcfg, mbw_max, has_changes);
> + maybe_update_config(cfg, mpam_feat_mbw_min, newcfg, mbw_min, has_changes);
>
> return has_changes;
> }
>
> +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;
> +
> + /*
> + * 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);
> + }
> +}
> +
> int mpam_apply_config(struct mpam_component *comp, u16 partid,
> - struct mpam_config *cfg)
> + struct mpam_config *user_cfg)
> {
> struct mpam_write_config_arg arg;
> struct mpam_msc_ris *ris;
> + struct mpam_config cfg;
> struct mpam_vmsc *vmsc;
> struct mpam_msc *msc;
>
> lockdep_assert_cpus_held();
>
> +
> /* Don't pass in the current config! */
> - WARN_ON_ONCE(&comp->cfg[partid] == cfg);
> + WARN_ON_ONCE(&comp->cfg[partid] == user_cfg);
>
> - if (!mpam_update_config(&comp->cfg[partid], cfg))
> + /*
> + * Copy the config to avoid writing back the 'extended' version to
> + * the caller.
> + * This avoids mpam_devices.c setting a mbm_min that mpam_resctrl.c
> + * is unaware of ... when it then changes mbm_max to be lower than
> + * mbm_min.
> + */
> + cfg = *user_cfg;
> +
> + mpam_extend_config(comp->class, &cfg);
> +
> + if (!mpam_update_config(&comp->cfg[partid], &cfg))
> return 0;
>
> arg.comp = comp;
> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index b13d5e55e701..d381906545ed 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h
> @@ -277,6 +277,7 @@ struct mpam_config {
> u32 cpbm;
> u32 mbw_pbm;
> u16 mbw_max;
> + u16 mbw_min;
>
> bool reset_cpbm;
> bool reset_mbw_pbm;
> @@ -618,6 +619,7 @@ static inline void mpam_resctrl_teardown_class(struct mpam_class *class) { }
> * MPAMCFG_MBW_MAX - MPAM memory maximum bandwidth partitioning configuration
> * register
> */
> +#define MPAMCFG_MBW_MAX_MAX_NR_BITS 16
> #define MPAMCFG_MBW_MAX_MAX GENMASK(15, 0)
> #define MPAMCFG_MBW_MAX_HARDLIM BIT(31)
>
> diff --git a/drivers/resctrl/test_mpam_devices.c b/drivers/resctrl/test_mpam_devices.c
> index 3e8d564a0c64..2f802fd9f249 100644
> --- a/drivers/resctrl/test_mpam_devices.c
> +++ b/drivers/resctrl/test_mpam_devices.c
> @@ -322,6 +322,71 @@ static void test_mpam_enable_merge_features(struct kunit *test)
> mutex_unlock(&mpam_list_lock);
> }
>
> +static void test_mpam_extend_config(struct kunit *test)
> +{
> + struct mpam_config fake_cfg = { };
> + struct mpam_class fake_class = { };
> +
> + /* Configurations with both are not modified */
> + fake_class.props.bwa_wd = 16;
> + fake_cfg.mbw_max = 0xfeef;
> + fake_cfg.mbw_min = 0xfeef;
> + bitmap_zero(fake_cfg.features, MPAM_FEATURE_LAST);
> + mpam_set_feature(mpam_feat_mbw_max, &fake_cfg);
> + mpam_set_feature(mpam_feat_mbw_min, &fake_cfg);
> + mpam_extend_config(&fake_class, &fake_cfg);
> + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_max, &fake_cfg));
> + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_min, &fake_cfg));
> + KUNIT_EXPECT_EQ(test, fake_cfg.mbw_max, 0xfeef);
> + KUNIT_EXPECT_EQ(test, fake_cfg.mbw_min, 0xfeef);
> +
> + /* When a min is missing, it is generated */
> + fake_class.props.bwa_wd = 16;
> + fake_cfg.mbw_max = 0xfeef;
> + fake_cfg.mbw_min = 0;
> + bitmap_zero(fake_cfg.features, MPAM_FEATURE_LAST);
> + mpam_set_feature(mpam_feat_mbw_max, &fake_cfg);
> + mpam_extend_config(&fake_class, &fake_cfg);
> + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_max, &fake_cfg));
> + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_min, &fake_cfg));
> + KUNIT_EXPECT_EQ(test, fake_cfg.mbw_max, 0xfeef);
> + KUNIT_EXPECT_EQ(test, fake_cfg.mbw_min, 0xf224);
> +
> + fake_class.props.bwa_wd = 8;
> + fake_cfg.mbw_max = 0xfeef;
> + fake_cfg.mbw_min = 0;
> + bitmap_zero(fake_cfg.features, MPAM_FEATURE_LAST);
> + mpam_set_feature(mpam_feat_mbw_max, &fake_cfg);
> + mpam_extend_config(&fake_class, &fake_cfg);
> + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_max, &fake_cfg));
> + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_min, &fake_cfg));
> + KUNIT_EXPECT_EQ(test, fake_cfg.mbw_max, 0xfeef);
> + KUNIT_EXPECT_EQ(test, fake_cfg.mbw_min, 0xf224);
> +
> + /* 5% below the minimum granule, is still the minimum granule */
> + fake_class.props.bwa_wd = 12;
> + fake_cfg.mbw_max = 0xf;
> + fake_cfg.mbw_min = 0;
> + bitmap_zero(fake_cfg.features, MPAM_FEATURE_LAST);
> + mpam_set_feature(mpam_feat_mbw_max, &fake_cfg);
> + mpam_extend_config(&fake_class, &fake_cfg);
> + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_max, &fake_cfg));
> + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_min, &fake_cfg));
> + KUNIT_EXPECT_EQ(test, fake_cfg.mbw_max, 0xf);
> + KUNIT_EXPECT_EQ(test, fake_cfg.mbw_min, 0xf);
> +
> + fake_class.props.bwa_wd = 16;
> + fake_cfg.mbw_max = 0x4;
> + fake_cfg.mbw_min = 0;
> + bitmap_zero(fake_cfg.features, MPAM_FEATURE_LAST);
> + mpam_set_feature(mpam_feat_mbw_max, &fake_cfg);
> + mpam_extend_config(&fake_class, &fake_cfg);
> + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_max, &fake_cfg));
> + KUNIT_EXPECT_TRUE(test, mpam_has_feature(mpam_feat_mbw_min, &fake_cfg));
> + KUNIT_EXPECT_EQ(test, fake_cfg.mbw_max, 0x4);
> + KUNIT_EXPECT_EQ(test, fake_cfg.mbw_min, 0x0);
> +}
> +
> static void test_mpam_reset_msc_bitmap(struct kunit *test)
> {
> char __iomem *buf = kunit_kzalloc(test, SZ_16K, GFP_KERNEL);
> @@ -378,6 +443,7 @@ static struct kunit_case mpam_devices_test_cases[] = {
> KUNIT_CASE(test_mpam_reset_msc_bitmap),
> KUNIT_CASE(test_mpam_enable_merge_features),
> KUNIT_CASE(test__props_mismatch),
> + KUNIT_CASE(test_mpam_extend_config),
> {}
> };
>

Thanks,

Ben