Re: [RFC PATCH mpam mpam/snapshot/v6.12-rc1 v3 5/5] arm_mpam: Adapting the closid/rmid matching determination functions

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

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/