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)) {
@@ -1615,6 +1622,7 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
 
 	mpam_quirk_post_config_change(ris, partid, cfg);
 
+out:
 	mutex_unlock(&msc->part_sel_lock);
 }
 
@@ -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;
 }
diff --git a/drivers/platform/arm64/mpam/mpam_internal.h b/drivers/platform/arm64/mpam/mpam_internal.h
index 5fc9f09b6945..c02365338b21 100644
--- a/drivers/platform/arm64/mpam/mpam_internal.h
+++ b/drivers/platform/arm64/mpam/mpam_internal.h
@@ -773,4 +773,7 @@ static inline void mpam_resctrl_teardown_class(struct mpam_class *class) { }
  */
 #define MSMON_CAPT_EVNT_NOW    BIT(0)
 
+u32 get_num_reqpartid_per_closid(void);
+u32 req2intpartid(u32 reqpartid);
+
 #endif /* MPAM_INTERNAL_H */
diff --git a/drivers/platform/arm64/mpam/mpam_resctrl.c b/drivers/platform/arm64/mpam/mpam_resctrl.c
index 965ff9fd45d3..47e3a51b0942 100644
--- a/drivers/platform/arm64/mpam/mpam_resctrl.c
+++ b/drivers/platform/arm64/mpam/mpam_resctrl.c
@@ -171,11 +171,19 @@ static u32 get_num_reqpartid(void)
 	return mpam_partid_max + 1;
 }
 
-static u32 get_num_reqpartid_per_closid(void)
+u32 get_num_reqpartid_per_closid(void)
 {
 	return get_num_reqpartid() / resctrl_arch_get_num_closid(NULL);
 }
 
+u32 req2intpartid(u32 reqpartid)
+{
+	u8 intpartid_shift = fls(mpam_intpartid_max);
+	u32 intpartid_mask = ~(~0 << intpartid_shift);
+
+	return reqpartid & intpartid_mask;
+}
+
 u32 resctrl_arch_system_num_rmid_idx(void)
 {
 	u8 closid_shift = fls(mpam_pmg_max);
-- 
2.25.1

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/

On 2024/12/13 0:18, Dave Martin wrote:
> Hi,
> 
>> @@ -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.
> 

Yes, the actual functions of these two locations are essentially the
same. However, at the __write_config position, we can reduce the
repeated judgments of cfg[partid] in mpam_update_config() and also
decrease the times of smp_call remote invocations.

What about your option towards it?

Best regards,
Zeng Heng

Hi,

On Fri, Dec 20, 2024 at 05:36:23PM +0800, Zeng Heng wrote:
> 
> 
> On 2024/12/13 0:18, Dave Martin wrote:
> > Hi,
> > 
> > > @@ -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.
> > 
> 
> Yes, the actual functions of these two locations are essentially the
> same. However, at the __write_config position, we can reduce the
> repeated judgments of cfg[partid] in mpam_update_config() and also
> decrease the times of smp_call remote invocations.
> 
> What about your option towards it?

I think either can be done.  I was aiming to keep things as simple as
possible for now, and contain all the mapping logic in mpam_resctrl.c.

I think that with my version of the code, changing the
mpam_apply_config() interface to accept a PARTID range instead of a
single PARTID might be a natural way to do this.

This probably does make sense, in order to avoid excessive SMP cross-
calling; I will have a go and see whether this works.


(Note, there is likely to be redundant cross-calling already, which is
one reason why I did not pay close attention to this issue.  If we
could batch updates separately per group of CPUs then we could reduce
the number of cross-calls, though care would be needed if CPUs can be
hotplugged while processing a batch of updates.  I think that a change
to the resctrl core interface might be necessary if we want the arch
code to be able to queue and schedule updates in this way; resctrl
currently assumes that each update is applied immediately.)

Cheers
---Dave