| Message ID | 20241207092136.2488426-5-zengheng4@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm_mpam: Introduce the Narrow-PARTID feature for MPAM driver | expand |
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
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);
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(-)