| Message ID | 20241207092136.2488426-6-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:36PM +0800, Zeng Heng wrote: > According to the previous patches, add the inverse functions for the > closid/rmid conversion functions to serve as the conversion functions for > reqpartid/pmg. And adapt the matching determination functions > resctrl_arch_match_closid() and resctrl_arch_match_rmid() by the inverse > functions. > > For the same reason, when updating the (req)PARTID/PMG pair for a task, > the new conversion functions also are used for adaptation. > > Signed-off-by: Zeng Heng <zengheng4@huawei.com> > --- > arch/arm64/include/asm/mpam.h | 6 ++- > drivers/platform/arm64/mpam/mpam_resctrl.c | 63 +++++++++++++++------- > 2 files changed, 47 insertions(+), 22 deletions(-) > > diff --git a/arch/arm64/include/asm/mpam.h b/arch/arm64/include/asm/mpam.h > index e5f385767174..9fc095530671 100644 > --- a/arch/arm64/include/asm/mpam.h > +++ b/arch/arm64/include/asm/mpam.h > @@ -93,6 +93,8 @@ static inline u64 mpam_get_regval(struct task_struct *tsk) > #endif > } > > +u32 rmid2pmg(u32 rmid); > + > static inline void resctrl_arch_set_rmid(struct task_struct *tsk, u32 rmid) > { > #ifdef CONFIG_ARM64_MPAM > @@ -100,8 +102,8 @@ static inline void resctrl_arch_set_rmid(struct task_struct *tsk, u32 rmid) > > regval &= ~MPAM1_EL1_PMG_D; > regval &= ~MPAM1_EL1_PMG_I; > - regval |= FIELD_PREP(MPAM1_EL1_PMG_D, rmid); > - regval |= FIELD_PREP(MPAM1_EL1_PMG_I, rmid); > + regval |= FIELD_PREP(MPAM1_EL1_PMG_D, rmid2pmg(rmid)); > + regval |= FIELD_PREP(MPAM1_EL1_PMG_I, rmid2pmg(rmid)); Note, this function does not seem to be used; I added a patch in my series [1] to get rid of it instead of converting it. > > WRITE_ONCE(task_thread_info(tsk)->mpam_partid_pmg, regval); > #endif > diff --git a/drivers/platform/arm64/mpam/mpam_resctrl.c b/drivers/platform/arm64/mpam/mpam_resctrl.c [...] > void resctrl_arch_set_cpu_default_closid_rmid(int cpu, u32 closid, u32 rmid) > { > - BUG_ON(closid > U16_MAX); > - BUG_ON(rmid > U8_MAX); > + u32 reqpartid = closid_rmid2reqpartid(closid, rmid); > + u32 pmg = rmid2pmg(rmid); > + u32 partid_d, partid_i; > + > + BUG_ON(reqpartid > U16_MAX); > + BUG_ON(pmg > U8_MAX); > > if (!cdp_enabled) { > - mpam_set_cpu_defaults(cpu, closid, closid, rmid, rmid); > + mpam_set_cpu_defaults(cpu, reqpartid, reqpartid, pmg, pmg); > } else { > /* > * When CDP is enabled, resctrl halves the closid range and we > * use odd/even partid for one closid. > */ > - u32 partid_d = resctrl_get_config_index(closid, CDP_DATA); > - u32 partid_i = resctrl_get_config_index(closid, CDP_CODE); > + partid_d = resctrl_get_config_index(reqpartid, CDP_DATA); > + partid_i = resctrl_get_config_index(reqpartid, CDP_CODE); > > - mpam_set_cpu_defaults(cpu, partid_d, partid_i, rmid, rmid); > + mpam_set_cpu_defaults(cpu, partid_d, partid_i, pmg, pmg); Prior to this patch, will the PARTID and/or PMG programmed for a control group be different from the PARTID and/or PMG used to program the MSCs? If so, those changes probably need to be in the same patch. [...] > @@ -289,41 +307,46 @@ void resctrl_arch_sync_cpu_closid_rmid(void *info) [...] > /* The task's pmg is not unique, the partid must be considered too */ > bool resctrl_arch_match_rmid(struct task_struct *tsk, u32 closid, u32 rmid) > { > u64 regval = mpam_get_regval(tsk); > - u32 tsk_closid = FIELD_GET(MPAM1_EL1_PARTID_D, regval); > - u32 tsk_rmid = FIELD_GET(MPAM1_EL1_PMG_D, regval); > + u32 tsk_pmg = FIELD_GET(MPAM1_EL1_PMG_D, regval); > + u32 tsk_partid = FIELD_GET(MPAM1_EL1_PARTID_D, regval); > > if (cdp_enabled) > - tsk_closid >>= 1; > + tsk_partid >>= 1; > > - return (tsk_closid == closid) && (tsk_rmid == rmid); > + return (reqpartid2closid(tsk_partid) == closid) && > + (reqpartid_pmg2rmid(tsk_partid, tsk_pmg) == rmid); Do we actually need the reverse mappings here? It doesn't really matter which ID namespace is used for the comparison, so in my version of this I converted the passed-in closid and rmid to PARTID / PMG form and then compared those with tsk's values. (But if I've missed some subtlety here, please let me know!) [...] Cheers ---Dave [1] [RFC PATCH 3/6] arm_mpam: Delete unused function resctrl_arch_set_rmid() https://lore.kernel.org/lkml/20241212154000.330467-4-Dave.Martin@arm.com/
On 2024/12/13 0:19, Dave Martin wrote: > Hi, > > On Sat, Dec 07, 2024 at 05:21:36PM +0800, Zeng Heng wrote: >> According to the previous patches, add the inverse functions for the >> closid/rmid conversion functions to serve as the conversion functions for >> reqpartid/pmg. And adapt the matching determination functions >> resctrl_arch_match_closid() and resctrl_arch_match_rmid() by the inverse >> functions. >> >> For the same reason, when updating the (req)PARTID/PMG pair for a task, >> the new conversion functions also are used for adaptation. >> >> Signed-off-by: Zeng Heng <zengheng4@huawei.com> >> --- >> arch/arm64/include/asm/mpam.h | 6 ++- >> drivers/platform/arm64/mpam/mpam_resctrl.c | 63 +++++++++++++++------- >> 2 files changed, 47 insertions(+), 22 deletions(-) >> >> diff --git a/arch/arm64/include/asm/mpam.h b/arch/arm64/include/asm/mpam.h >> index e5f385767174..9fc095530671 100644 >> --- a/arch/arm64/include/asm/mpam.h >> +++ b/arch/arm64/include/asm/mpam.h >> @@ -93,6 +93,8 @@ static inline u64 mpam_get_regval(struct task_struct *tsk) >> #endif >> } >> >> +u32 rmid2pmg(u32 rmid); >> + >> static inline void resctrl_arch_set_rmid(struct task_struct *tsk, u32 rmid) >> { >> #ifdef CONFIG_ARM64_MPAM >> @@ -100,8 +102,8 @@ static inline void resctrl_arch_set_rmid(struct task_struct *tsk, u32 rmid) >> >> regval &= ~MPAM1_EL1_PMG_D; >> regval &= ~MPAM1_EL1_PMG_I; >> - regval |= FIELD_PREP(MPAM1_EL1_PMG_D, rmid); >> - regval |= FIELD_PREP(MPAM1_EL1_PMG_I, rmid); >> + regval |= FIELD_PREP(MPAM1_EL1_PMG_D, rmid2pmg(rmid)); >> + regval |= FIELD_PREP(MPAM1_EL1_PMG_I, rmid2pmg(rmid)); > > Note, this function does not seem to be used; I added a patch in my > series [1] to get rid of it instead of converting it. > At that time, I was still hesitating whether to directly delete that function. Yes, I will skip the adaptation of this function. >> >> WRITE_ONCE(task_thread_info(tsk)->mpam_partid_pmg, regval); >> #endif >> diff --git a/drivers/platform/arm64/mpam/mpam_resctrl.c b/drivers/platform/arm64/mpam/mpam_resctrl.c > > [...] > >> void resctrl_arch_set_cpu_default_closid_rmid(int cpu, u32 closid, u32 rmid) >> { >> - BUG_ON(closid > U16_MAX); >> - BUG_ON(rmid > U8_MAX); >> + u32 reqpartid = closid_rmid2reqpartid(closid, rmid); >> + u32 pmg = rmid2pmg(rmid); >> + u32 partid_d, partid_i; >> + >> + BUG_ON(reqpartid > U16_MAX); >> + BUG_ON(pmg > U8_MAX); >> >> if (!cdp_enabled) { >> - mpam_set_cpu_defaults(cpu, closid, closid, rmid, rmid); >> + mpam_set_cpu_defaults(cpu, reqpartid, reqpartid, pmg, pmg); >> } else { >> /* >> * When CDP is enabled, resctrl halves the closid range and we >> * use odd/even partid for one closid. >> */ >> - u32 partid_d = resctrl_get_config_index(closid, CDP_DATA); >> - u32 partid_i = resctrl_get_config_index(closid, CDP_CODE); >> + partid_d = resctrl_get_config_index(reqpartid, CDP_DATA); >> + partid_i = resctrl_get_config_index(reqpartid, CDP_CODE); >> >> - mpam_set_cpu_defaults(cpu, partid_d, partid_i, rmid, rmid); >> + mpam_set_cpu_defaults(cpu, partid_d, partid_i, pmg, pmg); > > Prior to this patch, will the PARTID and/or PMG programmed for a > control group be different from the PARTID and/or PMG used to program > the MSCs? > > If so, those changes probably need to be in the same patch. > Thanks to point it out. It's reasonable to put setting PARTID/PMG operations together with the closid/rmid pair remapping. > >> @@ -289,41 +307,46 @@ void resctrl_arch_sync_cpu_closid_rmid(void *info) > > [...] > >> /* The task's pmg is not unique, the partid must be considered too */ >> bool resctrl_arch_match_rmid(struct task_struct *tsk, u32 closid, u32 rmid) >> { >> u64 regval = mpam_get_regval(tsk); >> - u32 tsk_closid = FIELD_GET(MPAM1_EL1_PARTID_D, regval); >> - u32 tsk_rmid = FIELD_GET(MPAM1_EL1_PMG_D, regval); >> + u32 tsk_pmg = FIELD_GET(MPAM1_EL1_PMG_D, regval); >> + u32 tsk_partid = FIELD_GET(MPAM1_EL1_PARTID_D, regval); >> >> if (cdp_enabled) >> - tsk_closid >>= 1; >> + tsk_partid >>= 1; >> >> - return (tsk_closid == closid) && (tsk_rmid == rmid); >> + return (reqpartid2closid(tsk_partid) == closid) && >> + (reqpartid_pmg2rmid(tsk_partid, tsk_pmg) == rmid); > > Do we actually need the reverse mappings here? > > It doesn't really matter which ID namespace is used for the > comparison, so in my version of this I converted the passed-in closid > and rmid to PARTID / PMG form and then compared those with tsk's > values. > > (But if I've missed some subtlety here, please let me know!) > Yes, we actually don't need the reverse mappings, and reuse the closid_rmid2reqpartid() and rmid2pmg() to finish the comparison. Best regards, Zeng Heng
diff --git a/arch/arm64/include/asm/mpam.h b/arch/arm64/include/asm/mpam.h index e5f385767174..9fc095530671 100644 --- a/arch/arm64/include/asm/mpam.h +++ b/arch/arm64/include/asm/mpam.h @@ -93,6 +93,8 @@ static inline u64 mpam_get_regval(struct task_struct *tsk) #endif } +u32 rmid2pmg(u32 rmid); + static inline void resctrl_arch_set_rmid(struct task_struct *tsk, u32 rmid) { #ifdef CONFIG_ARM64_MPAM @@ -100,8 +102,8 @@ static inline void resctrl_arch_set_rmid(struct task_struct *tsk, u32 rmid) regval &= ~MPAM1_EL1_PMG_D; regval &= ~MPAM1_EL1_PMG_I; - regval |= FIELD_PREP(MPAM1_EL1_PMG_D, rmid); - regval |= FIELD_PREP(MPAM1_EL1_PMG_I, rmid); + regval |= FIELD_PREP(MPAM1_EL1_PMG_D, rmid2pmg(rmid)); + regval |= FIELD_PREP(MPAM1_EL1_PMG_I, rmid2pmg(rmid)); WRITE_ONCE(task_thread_info(tsk)->mpam_partid_pmg, regval); #endif diff --git a/drivers/platform/arm64/mpam/mpam_resctrl.c b/drivers/platform/arm64/mpam/mpam_resctrl.c index 47e3a51b0942..cb5da3de907a 100644 --- a/drivers/platform/arm64/mpam/mpam_resctrl.c +++ b/drivers/platform/arm64/mpam/mpam_resctrl.c @@ -239,7 +239,7 @@ static u32 closid_rmid2reqpartid(u32 closid, u32 rmid) return (req_idx << intpartid_shift) | closid; } -static u32 rmid2pmg(u32 rmid) +u32 rmid2pmg(u32 rmid) { u8 pmg_shift = fls(mpam_pmg_max); u32 pmg_mask = ~(~0 << pmg_shift); @@ -247,6 +247,20 @@ static u32 rmid2pmg(u32 rmid) return rmid & pmg_mask; } +static u32 reqpartid2closid(u32 reqpartid) +{ + return req2intpartid(reqpartid); +} + +static u32 reqpartid_pmg2rmid(u32 reqpartid, u32 pmg) +{ + u8 pmg_shift = fls(mpam_pmg_max); + u8 intpartid_shift = fls(mpam_intpartid_max); + u32 req_idx = (reqpartid >> intpartid_shift); + + return (req_idx << pmg_shift) | pmg; +} + void resctrl_arch_sched_in(struct task_struct *tsk) { lockdep_assert_preemption_disabled(); @@ -256,20 +270,24 @@ void resctrl_arch_sched_in(struct task_struct *tsk) void resctrl_arch_set_cpu_default_closid_rmid(int cpu, u32 closid, u32 rmid) { - BUG_ON(closid > U16_MAX); - BUG_ON(rmid > U8_MAX); + u32 reqpartid = closid_rmid2reqpartid(closid, rmid); + u32 pmg = rmid2pmg(rmid); + u32 partid_d, partid_i; + + BUG_ON(reqpartid > U16_MAX); + BUG_ON(pmg > U8_MAX); if (!cdp_enabled) { - mpam_set_cpu_defaults(cpu, closid, closid, rmid, rmid); + mpam_set_cpu_defaults(cpu, reqpartid, reqpartid, pmg, pmg); } else { /* * When CDP is enabled, resctrl halves the closid range and we * use odd/even partid for one closid. */ - u32 partid_d = resctrl_get_config_index(closid, CDP_DATA); - u32 partid_i = resctrl_get_config_index(closid, CDP_CODE); + partid_d = resctrl_get_config_index(reqpartid, CDP_DATA); + partid_i = resctrl_get_config_index(reqpartid, CDP_CODE); - mpam_set_cpu_defaults(cpu, partid_d, partid_i, rmid, rmid); + mpam_set_cpu_defaults(cpu, partid_d, partid_i, pmg, pmg); } } @@ -289,41 +307,46 @@ void resctrl_arch_sync_cpu_closid_rmid(void *info) void resctrl_arch_set_closid_rmid(struct task_struct *tsk, u32 closid, u32 rmid) { - BUG_ON(closid > U16_MAX); - BUG_ON(rmid > U8_MAX); + u32 reqpartid = closid_rmid2reqpartid(closid, rmid); + u32 pmg = rmid2pmg(rmid); + u32 partid_d, partid_i; + + BUG_ON(reqpartid > U16_MAX); + BUG_ON(pmg > U8_MAX); if (!cdp_enabled) { - mpam_set_task_partid_pmg(tsk, closid, closid, rmid, rmid); + mpam_set_task_partid_pmg(tsk, reqpartid, reqpartid, pmg, pmg); } else { - u32 partid_d = resctrl_get_config_index(closid, CDP_DATA); - u32 partid_i = resctrl_get_config_index(closid, CDP_CODE); + partid_d = resctrl_get_config_index(reqpartid, CDP_DATA); + partid_i = resctrl_get_config_index(reqpartid, CDP_CODE); - mpam_set_task_partid_pmg(tsk, partid_d, partid_i, rmid, rmid); + mpam_set_task_partid_pmg(tsk, partid_d, partid_i, pmg, pmg); } } bool resctrl_arch_match_closid(struct task_struct *tsk, u32 closid) { u64 regval = mpam_get_regval(tsk); - u32 tsk_closid = FIELD_GET(MPAM1_EL1_PARTID_D, regval); + u32 tsk_partid = FIELD_GET(MPAM1_EL1_PARTID_D, regval); if (cdp_enabled) - tsk_closid >>= 1; + tsk_partid >>= 1; - return tsk_closid == closid; + return reqpartid2closid(tsk_partid) == closid; } /* The task's pmg is not unique, the partid must be considered too */ bool resctrl_arch_match_rmid(struct task_struct *tsk, u32 closid, u32 rmid) { u64 regval = mpam_get_regval(tsk); - u32 tsk_closid = FIELD_GET(MPAM1_EL1_PARTID_D, regval); - u32 tsk_rmid = FIELD_GET(MPAM1_EL1_PMG_D, regval); + u32 tsk_pmg = FIELD_GET(MPAM1_EL1_PMG_D, regval); + u32 tsk_partid = FIELD_GET(MPAM1_EL1_PARTID_D, regval); if (cdp_enabled) - tsk_closid >>= 1; + tsk_partid >>= 1; - return (tsk_closid == closid) && (tsk_rmid == rmid); + return (reqpartid2closid(tsk_partid) == closid) && + (reqpartid_pmg2rmid(tsk_partid, tsk_pmg) == rmid); } struct rdt_resource *resctrl_arch_get_resource(enum resctrl_res_level l)
According to the previous patches, add the inverse functions for the closid/rmid conversion functions to serve as the conversion functions for reqpartid/pmg. And adapt the matching determination functions resctrl_arch_match_closid() and resctrl_arch_match_rmid() by the inverse functions. For the same reason, when updating the (req)PARTID/PMG pair for a task, the new conversion functions also are used for adaptation. Signed-off-by: Zeng Heng <zengheng4@huawei.com> --- arch/arm64/include/asm/mpam.h | 6 ++- drivers/platform/arm64/mpam/mpam_resctrl.c | 63 +++++++++++++++------- 2 files changed, 47 insertions(+), 22 deletions(-)