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));
 
 	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)
-- 
2.25.1

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