[PATCH] arm_mpam: Fix MPAMCFG_MBW_PBM register setting

[PATCH] arm_mpam: Fix MPAMCFG_MBW_PBM register setting

[Fenghua Yu]

MPAMCFG_MBW_PBM is written from cfg if cfg has the MBW partition feature.
It is reset when cfg does not have the MBW partition feature.

But the register handling is reversed. This may cause an incorrect
register setting. For example, during an MPAM reset, reset_cfg is
empty (no MBW partition feature set), and cfg->mbw_pbm is 0. Instead of
resetting MPAMCFG_MBW_PBM to all 1's, the current logic will set it to
cfg->mbw_pbm, which is 0.

Fix the issue by swapping the if/else branches.

Fixes: a1cb6577f575 ("arm_mpam: Reset when feature configuration bit unset")
Reported-by: Matt Ochs <mochs@nvidia.com>
Signed-off-by: Fenghua Yu <fenghuay@nvidia.com>
---
 drivers/resctrl/mpam_devices.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 4b93e89c2678..d8b0383cee92 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -1570,9 +1570,9 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
 
 	if (mpam_has_feature(mpam_feat_mbw_part, rprops)) {
 		if (mpam_has_feature(mpam_feat_mbw_part, cfg))
-			mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops->mbw_pbm_bits);
-		else
 			mpam_write_partsel_reg(msc, MBW_PBM, cfg->mbw_pbm);
+		else
+			mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops->mbw_pbm_bits);
 	}
 
 	if (mpam_has_feature(mpam_feat_mbw_min, rprops)) {
-- 
2.43.0

Re: [PATCH] arm_mpam: Fix MPAMCFG_MBW_PBM register setting

[Gavin Shan]

Hi Fenghua,

On 6/7/26 3:09 PM, Fenghua Yu wrote:
> MPAMCFG_MBW_PBM is written from cfg if cfg has the MBW partition feature.
> It is reset when cfg does not have the MBW partition feature.
> 
> But the register handling is reversed. This may cause an incorrect
> register setting. For example, during an MPAM reset, reset_cfg is
> empty (no MBW partition feature set), and cfg->mbw_pbm is 0. Instead of
> resetting MPAMCFG_MBW_PBM to all 1's, the current logic will set it to
> cfg->mbw_pbm, which is 0.
> 
> Fix the issue by swapping the if/else branches.
> 
> Fixes: a1cb6577f575 ("arm_mpam: Reset when feature configuration bit unset")
> Reported-by: Matt Ochs <mochs@nvidia.com>
> Signed-off-by: Fenghua Yu <fenghuay@nvidia.com>
> ---
>   drivers/resctrl/mpam_devices.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 

The fix itself looks reasonable to me, but two questions below.

Reviewed-by: Gavin Shan <gshan@redhat.com>

> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 4b93e89c2678..d8b0383cee92 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1570,9 +1570,9 @@ static void mpam_reprogram_ris_partid(struct mpam_msc_ris *ris, u16 partid,
>   
>   	if (mpam_has_feature(mpam_feat_mbw_part, rprops)) {
>   		if (mpam_has_feature(mpam_feat_mbw_part, cfg))
> -			mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops->mbw_pbm_bits);
> -		else
>   			mpam_write_partsel_reg(msc, MBW_PBM, cfg->mbw_pbm);
> +		else
> +			mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops->mbw_pbm_bits);
>   	}
>   
>   	if (mpam_has_feature(mpam_feat_mbw_min, rprops)) {

Which machine or system where mpam_feat_mbw_part is set on RIS? As I can
remember, this feature isn't available on grace-hopper.

Besides, I don't think this feature is well handled at present because
mpam_config::mbw_pbm is only 32-bits in length, which doesn't match with
the maximal length of the bit map (4096) as documented in the spec.

Thanks,
Gavin

Re: [PATCH] arm_mpam: Fix MPAMCFG_MBW_PBM register setting

[Fenghua Yu]

Hi, Gavin,

On 6/7/26 21:46, Gavin Shan wrote:
> Hi Fenghua,
> 
> On 6/7/26 3:09 PM, Fenghua Yu wrote:
>> MPAMCFG_MBW_PBM is written from cfg if cfg has the MBW partition feature.
>> It is reset when cfg does not have the MBW partition feature.
>>
>> But the register handling is reversed. This may cause an incorrect
>> register setting. For example, during an MPAM reset, reset_cfg is
>> empty (no MBW partition feature set), and cfg->mbw_pbm is 0. Instead of
>> resetting MPAMCFG_MBW_PBM to all 1's, the current logic will set it to
>> cfg->mbw_pbm, which is 0.
>>
>> Fix the issue by swapping the if/else branches.
>>
>> Fixes: a1cb6577f575 ("arm_mpam: Reset when feature configuration bit 
>> unset")
>> Reported-by: Matt Ochs <mochs@nvidia.com>
>> Signed-off-by: Fenghua Yu <fenghuay@nvidia.com>
>> ---
>>   drivers/resctrl/mpam_devices.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
> 
> The fix itself looks reasonable to me, but two questions below.
> 
> Reviewed-by: Gavin Shan <gshan@redhat.com>

Thank you for reviewing the patch!

> 
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/ 
>> mpam_devices.c
>> index 4b93e89c2678..d8b0383cee92 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -1570,9 +1570,9 @@ static void mpam_reprogram_ris_partid(struct 
>> mpam_msc_ris *ris, u16 partid,
>>       if (mpam_has_feature(mpam_feat_mbw_part, rprops)) {
>>           if (mpam_has_feature(mpam_feat_mbw_part, cfg))
>> -            mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops- 
>> >mbw_pbm_bits);
>> -        else
>>               mpam_write_partsel_reg(msc, MBW_PBM, cfg->mbw_pbm);
>> +        else
>> +            mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops- 
>> >mbw_pbm_bits);
>>       }
>>       if (mpam_has_feature(mpam_feat_mbw_min, rprops)) {
> 
> Which machine or system where mpam_feat_mbw_part is set on RIS? As I can
> remember, this feature isn't available on grace-hopper.

Neither Grace nor Vera supports this feature.

> 
> Besides, I don't think this feature is well handled at present because
> mpam_config::mbw_pbm is only 32-bits in length, which doesn't match with
> the maximal length of the bit map (4096) as documented in the spec.

Right. The current code only can support up to 32 bits of PBM bitmap. If 
PBM bitmp length is bigger than 32 bits, it's broken. I guess we will 
need to handle this feature properly when hardware supports more than 32 
bits of PBM bitmap.

Ditto for CPBM bitmap.

This patch currently only fixes the regression issue introduced in 
commit a1cb6577f575 regardless size of PBM bitmap.

Thanks.

-Fenghua

Re: [PATCH] arm_mpam: Fix MPAMCFG_MBW_PBM register setting

[Ben Horgan]

Hi Fenghua,

Thanks for the fix!

On 6/9/26 03:06, Fenghua Yu wrote:
> Hi, Gavin,
> 
> On 6/7/26 21:46, Gavin Shan wrote:
>> Hi Fenghua,
>>
>> On 6/7/26 3:09 PM, Fenghua Yu wrote:
>>> MPAMCFG_MBW_PBM is written from cfg if cfg has the MBW partition feature.
>>> It is reset when cfg does not have the MBW partition feature.
>>>
>>> But the register handling is reversed. This may cause an incorrect
>>> register setting. For example, during an MPAM reset, reset_cfg is
>>> empty (no MBW partition feature set), and cfg->mbw_pbm is 0. Instead of
>>> resetting MPAMCFG_MBW_PBM to all 1's, the current logic will set it to
>>> cfg->mbw_pbm, which is 0.
>>>
>>> Fix the issue by swapping the if/else branches.
>>>
>>> Fixes: a1cb6577f575 ("arm_mpam: Reset when feature configuration bit unset")
>>> Reported-by: Matt Ochs <mochs@nvidia.com>
>>> Signed-off-by: Fenghua Yu <fenghuay@nvidia.com>
>>> ---
>>>   drivers/resctrl/mpam_devices.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>
>> The fix itself looks reasonable to me, but two questions below.
>>
>> Reviewed-by: Gavin Shan <gshan@redhat.com>
> 
> Thank you for reviewing the patch!
> 
>>
>>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/ mpam_devices.c
>>> index 4b93e89c2678..d8b0383cee92 100644
>>> --- a/drivers/resctrl/mpam_devices.c
>>> +++ b/drivers/resctrl/mpam_devices.c
>>> @@ -1570,9 +1570,9 @@ static void mpam_reprogram_ris_partid(struct
>>> mpam_msc_ris *ris, u16 partid,
>>>       if (mpam_has_feature(mpam_feat_mbw_part, rprops)) {
>>>           if (mpam_has_feature(mpam_feat_mbw_part, cfg))
>>> -            mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops- >mbw_pbm_bits);
>>> -        else
>>>               mpam_write_partsel_reg(msc, MBW_PBM, cfg->mbw_pbm);
>>> +        else
>>> +            mpam_reset_msc_bitmap(msc, MPAMCFG_MBW_PBM, rprops- >mbw_pbm_bits);
>>>       }
>>>       if (mpam_has_feature(mpam_feat_mbw_min, rprops)) {
>>
>> Which machine or system where mpam_feat_mbw_part is set on RIS? As I can
>> remember, this feature isn't available on grace-hopper.
> 
> Neither Grace nor Vera supports this feature.
> 
>>
>> Besides, I don't think this feature is well handled at present because
>> mpam_config::mbw_pbm is only 32-bits in length, which doesn't match with
>> the maximal length of the bit map (4096) as documented in the spec.
> > Right. The current code only can support up to 32 bits of PBM bitmap. If PBM
> bitmp length is bigger than 32 bits, it's broken. I guess we will need to handle
> this feature properly when hardware supports more than 32 bits of PBM bitmap.

mpam_config::mbw_pbm shouldn't have made it into the driver as nothing ever sets
it. Otherwise, there is no attempt to make use of the MBW_PBM other than to
reset it to an appropriate value, which this patch fixes. I guess this is a
lesson for me on the perils of dead code.

> 
> Ditto for CPBM bitmap.

If the CPBM bitmap has more than 32 bits, as checked by
cache_has_usable_cpor(), then we don't attempt to configure it other than to
reset. If there is hardware with more than 32 bits we'd first consider extending
this to 64bits.

> 
> This patch currently only fixes the regression issue introduced in commit
> a1cb6577f575 regardless size of PBM bitmap.

Looks good to me.

Reviewed-by: Ben Horgan <ben.horgan@arm.com>

Thanks,

Ben

> 
> Thanks.
> 
> -Fenghua

Re: [PATCH] arm_mpam: Fix MPAMCFG_MBW_PBM register setting

[Will Deacon]

On Sat, 06 Jun 2026 22:09:25 -0700, Fenghua Yu wrote:
> MPAMCFG_MBW_PBM is written from cfg if cfg has the MBW partition feature.
> It is reset when cfg does not have the MBW partition feature.
> 
> But the register handling is reversed. This may cause an incorrect
> register setting. For example, during an MPAM reset, reset_cfg is
> empty (no MBW partition feature set), and cfg->mbw_pbm is 0. Instead of
> resetting MPAMCFG_MBW_PBM to all 1's, the current logic will set it to
> cfg->mbw_pbm, which is 0.
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm_mpam: Fix MPAMCFG_MBW_PBM register setting
      https://git.kernel.org/arm64/c/021118ce5ea9

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev