Re: [PATCH] arm_mpam: Fix MPAMCFG_MBW_PBM register setting

[Fenghua Yu <fenghuay@nvidia.com>]

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