Re: [PATCH v4 18/41] arm_mpam: resctrl: Implement helpers to update configuration

Zeng Heng
2026-02-14T10:39:26Z

Hi Ben,

On 2026/2/4 5:43, Ben Horgan wrote:
> From: James Morse <james.morse@arm.com>
>
> resctrl has two helpers for updating the configuration.
> resctrl_arch_update_one() updates a single value, and is used by the
> software-controller to apply feedback to the bandwidth controls, it has to
> be called on one of the CPUs in the resctrl:domain.
>
> resctrl_arch_update_domains() copies multiple staged configurations, it can
> be called from anywhere.
>
> Both helpers should update any changes to the underlying hardware.
>
> Implement resctrl_arch_update_domains() to use
> resctrl_arch_update_one(). Neither need to be called on a specific CPU as
> the mpam driver will send IPIs as needed.
>
> Tested-by: Gavin Shan <gshan@redhat.com>
> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Tested-by: Peter Newman <peternewman@google.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
> ---
> Changes since rfc:
> list_for_each_entry -> list_for_each_entry_rcu
> return 0
> Restrict scope of local variables
>
> Changes since v2:
> whitespace fix
> ---
> drivers/resctrl/mpam_resctrl.c | 70 ++++++++++++++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> index ecf00386edca..48d047510089 100644
> --- a/drivers/resctrl/mpam_resctrl.c
> +++ b/drivers/resctrl/mpam_resctrl.c
> @@ -212,6 +212,76 @@ 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)
> +{
> + u32 partid;
> + struct mpam_config cfg;
> + struct mpam_props *cprops;
> + struct mpam_resctrl_res *res;
> + struct mpam_resctrl_dom *dom;
> +
> + lockdep_assert_cpus_held();
> + lockdep_assert_irqs_enabled();
> +
> + /*
> + * No need to check the CPU as mpam_apply_config() doesn't care, and
> + * resctrl_arch_update_domains() relies on this.
> + */
> + res = container_of(r, struct mpam_resctrl_res, resctrl_res);
> + dom = container_of(d, struct mpam_resctrl_dom, resctrl_ctrl_dom);
> + cprops = &res->class->props;
> +
> + partid = resctrl_get_config_index(closid, t);

As a victim, I must admit I cannot verify this feedback on my local
Kunpeng environment since MB functionality is not yet supported by the
driver. However, after careful consideration, I believe this is worth
bringing up for discussion.

Regarding the MB configuration flow, the partid conversion should
include the mpam_resctrl_hide_cdp() condition check. Here's the
rationale:

After resctrl parsing schemata update, MB configuration is set via
parse_bw() or rdtgroup_init_mba(), which stores the updated
configuration in dom->staged_config[CDP_NONE]. If the MB configuration
update directly uses t = CDP_NONE, it would result in MB obtaining the
wrong partid and cfg[partid].

The specific fix would be like:

- partid = resctrl_get_config_index(closid, t);
+ if (mpam_resctrl_hide_cdp(r->rid))
+ /* The configuration of CDP_DATA is same as the CDP_CODE one. */
+ partid = resctrl_get_config_index(closid, CDP_DATA);
+ else
+ partid = resctrl_get_config_index(closid, t);

Similarly, in resctrl_arch_get_config() requires the same treatment to
ensure consistency.

Best regards,
and happy Lunar New Year!

Zeng Heng

> + if (!r->alloc_capable || partid >= resctrl_arch_get_num_closid(r)) {
> + pr_debug("Not alloc capable or computed PARTID out of range\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Copy the current config to avoid clearing other resources when the
> + * same component is exposed multiple times through resctrl.
> + */
> + cfg = dom->ctrl_comp->cfg[partid]; > +
> + switch (r->rid) {
> + case RDT_RESOURCE_L2:
> + case RDT_RESOURCE_L3:
> + cfg.cpbm = cfg_val;
> + mpam_set_feature(mpam_feat_cpor_part, &cfg);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return mpam_apply_config(dom->ctrl_comp, partid, &cfg);
> +}
> +
> +int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid)
> +{
> + int err;
> + struct rdt_ctrl_domain *d;
> +
> + lockdep_assert_cpus_held();
> + lockdep_assert_irqs_enabled();
> +
> + list_for_each_entry_rcu(d, &r->ctrl_domains, hdr.list) {
> + for (enum resctrl_conf_type t = 0; t < CDP_NUM_TYPES; t++) {
> + struct resctrl_staged_config *cfg = &d->staged_config[t];
> +
> + if (!cfg->have_new_ctrl)
> + continue;
> +
> + err = resctrl_arch_update_one(r, d, closid, t,
> + cfg->new_ctrl);
> + if (err)
> + return err;
> + }
> + }
> +
> + return 0;
> +}