Re: [RFC PATCH mpam mpam/snapshot/v6.12-rc1 v3 4/5] arm_mpam: Automatically synchronize the configuration of all sub-monitoring groups

[Dave Martin <Dave.Martin@arm.com>]

Hi,

On Sat, Dec 07, 2024 at 05:21:35PM +0800, Zeng Heng wrote:
> After the system expands the narrow-partid feature and statically assigns
> all (req)PARTIDs to each control group, the following scenarios require
> configuration synchronization operations:
> 
>   1. MSCs that support narrow-partid need to establish a mapping between
>      reqPARTID and intPARTID after creating a new monitoring group.
>   2. MSCs that do not support narrow-partid need to synchronize the
>      configuration of sub-monitoring groups after users update the control
>      group configuration.
> 
> In __write_config(), we synchronize a control group's configuration to each
> sub-monitoring group.
> 
> Signed-off-by: Zeng Heng <zengheng4@huawei.com>
> ---
>  drivers/platform/arm64/mpam/mpam_devices.c  | 25 ++++++++++++++++++---
>  drivers/platform/arm64/mpam/mpam_internal.h |  3 +++
>  drivers/platform/arm64/mpam/mpam_resctrl.c  | 10 ++++++++-
>  3 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/arm64/mpam/mpam_devices.c b/drivers/platform/arm64/mpam/mpam_devices.c
> index 781c9146718d..91c5849f76e3 100644
> --- a/drivers/platform/arm64/mpam/mpam_devices.c
> +++ b/drivers/platform/arm64/mpam/mpam_devices.c
> @@ -1544,6 +1544,7 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>  	u32 pri_val = 0;
>  	u16 cmax = MPAMCFG_CMAX_CMAX;
>  	u16 bwa_fract = MPAMCFG_MBW_MAX_MAX;
> +	u16 intpartid = req2intpartid(partid);
>  	struct mpam_msc *msc = ris->vmsc->msc;
>  	struct mpam_props *rprops = &ris->props;
>  	u16 dspri = GENMASK(rprops->dspri_wd, 0);
> @@ -1554,8 +1555,14 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>  
>  	if (mpam_has_feature(mpam_feat_partid_nrw, rprops)) {
>  		mpam_write_partsel_reg(msc, INTPARTID,
> -				       MPAMCFG_INTPARTID_INTERNAL | partid);
> -		__mpam_intpart_sel(ris->ris_idx, partid, msc);
> +				       MPAMCFG_INTPARTID_INTERNAL |
> +				       intpartid);
> +
> +		/* Already finish mapping reqPARTID to intPARTID */
> +		if (partid != intpartid)
> +			goto out;
> +
> +		__mpam_intpart_sel(ris->ris_idx, intpartid, msc);
>  	}
>  
>  	if (mpam_has_feature(mpam_feat_cpor_part, rprops)) {

[...]

> @@ -3072,9 +3080,20 @@ struct mpam_write_config_arg {
>  
>  static int __write_config(void *arg)
>  {
> +	int closid_num = resctrl_arch_get_num_closid(NULL);
>  	struct mpam_write_config_arg *c = arg;
> +	u32 reqpartid, req_idx;
> +
> +	WARN_ON(c->partid >= closid_num);
>  
> -	mpam_reprogram_ris_partid(c->ris, c->partid, &c->comp->cfg[c->partid]);
> +	/* Synchronize the configuration to each sub-monitoring group. */
> +	for (req_idx = 0; req_idx < get_num_reqpartid_per_closid();
> +	     req_idx++) {
> +		reqpartid = req_idx * closid_num + c->partid;
> +
> +		mpam_reprogram_ris_partid(c->ris, reqpartid,
> +					 &c->comp->cfg[c->partid]);
> +	}
>  
>  	return 0;
>  }

I haven't decided whether this iteration belongs here or in
mpam_resctrl.c.

Your approach looks like it should work; I do it in
resctrl_arch_update_one() instead [1], but I think the approaches are
pretty much equivalent -- but let me know if you have any thoughts on
it.

[...]

Cheers
---Dave


[1] [RFC PATCH 4/6] arm_mpam: Introduce flexible CLOSID/RMID translation
https://lore.kernel.org/lkml/20241212154000.330467-5-Dave.Martin@arm.com/