[PATCH] arm_mpam: Restore the cached configuration when a write to an MSC fails

[PATCH] arm_mpam: Restore the cached configuration when a write to an MSC fails

[Jinjie Ruan]

mpam_apply_config() caches the new configuration before writing it to
the RISes. When mpam_touch_msc() fails, the cache keeps the new values,
so a retry with the same schemata is skipped as a no-op and never
reaches the hardware.

Restore the cached configuration on failure so the retry writes it to
the RISes again.

Only arm64 MPAM systems are affected; x86 does not support MPAM.

Fixes: 60e33a1864fd ("arm_mpam: propagate MSC access errors for mpam_reprogram_ris_partid()")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: <stable@vger.kernel.org> # 7.4.x
---
 drivers/resctrl/mpam_devices.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index c067b6d57d71..bb667a416660 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -3412,6 +3412,7 @@ static bool mpam_update_config(struct mpam_config *cfg,
 int mpam_apply_config(struct mpam_component *comp, u16 partid,
 		      struct mpam_config *cfg)
 {
+	struct mpam_config old_cfg = comp->cfg[partid];
 	struct mpam_write_config_arg arg;
 	struct mpam_msc_ris *ris;
 	struct mpam_vmsc *vmsc;
@@ -3439,8 +3440,10 @@ int mpam_apply_config(struct mpam_component *comp, u16 partid,
 					 srcu_read_lock_held(&mpam_srcu)) {
 			arg.ris = ris;
 			ret = mpam_touch_msc(msc, __write_config, &arg);
-			if (ret)
+			if (ret) {
+				comp->cfg[partid] = old_cfg;
 				return ret;
+			}
 			ris->in_reset_state = false;
 		}
 	}
-- 
2.34.1

Re: [PATCH] arm_mpam: Restore the cached configuration when a write to an MSC fails

[Andre Przywara]

Hi,

On 11/4/26 11:15, Jinjie Ruan wrote:
> mpam_apply_config() caches the new configuration before writing it to
> the RISes. When mpam_touch_msc() fails, the cache keeps the new values,
> so a retry with the same schemata is skipped as a no-op and never
> reaches the hardware.

Do we really need this? Has this been observed to solve a problem?
If any of the MSC accesses fail (for MPAM-Fb only), we already tear down 
the whole MPAM driver immediately, so any subsequent accesses wouldn't 
work. So I think there is little point in trying to fix or restore any 
configuration?

Propagating errors up the call chain is another issue, but I don't think 
we need to care about any hardware settings in case of a failure.

Cheers,
Andre

> 
> Restore the cached configuration on failure so the retry writes it to
> the RISes again.
> 
> Only arm64 MPAM systems are affected; x86 does not support MPAM.
> 
> Fixes: 60e33a1864fd ("arm_mpam: propagate MSC access errors for mpam_reprogram_ris_partid()")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Cc: <stable@vger.kernel.org> # 7.4.x
> ---
>   drivers/resctrl/mpam_devices.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index c067b6d57d71..bb667a416660 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -3412,6 +3412,7 @@ static bool mpam_update_config(struct mpam_config *cfg,
>   int mpam_apply_config(struct mpam_component *comp, u16 partid,
>   		      struct mpam_config *cfg)
>   {
> +	struct mpam_config old_cfg = comp->cfg[partid];
>   	struct mpam_write_config_arg arg;
>   	struct mpam_msc_ris *ris;
>   	struct mpam_vmsc *vmsc;
> @@ -3439,8 +3440,10 @@ int mpam_apply_config(struct mpam_component *comp, u16 partid,
>   					 srcu_read_lock_held(&mpam_srcu)) {
>   			arg.ris = ris;
>   			ret = mpam_touch_msc(msc, __write_config, &arg);
> -			if (ret)
> +			if (ret) {
> +				comp->cfg[partid] = old_cfg;
>   				return ret;
> +			}
>   			ris->in_reset_state = false;
>   		}
>   	}

Re: [PATCH] arm_mpam: Restore the cached configuration when a write to an MSC fails

[Jinjie Ruan]

在 2026/11/4 19:24, Andre Przywara 写道:
> Hi,
> 
> On 11/4/26 11:15, Jinjie Ruan wrote:
>> mpam_apply_config() caches the new configuration before writing it to
>> the RISes. When mpam_touch_msc() fails, the cache keeps the new values,
>> so a retry with the same schemata is skipped as a no-op and never
>> reaches the hardware.
> 
> Do we really need this? Has this been observed to solve a problem?
> If any of the MSC accesses fail (for MPAM-Fb only), we already tear down
> the whole MPAM driver immediately, so any subsequent accesses wouldn't

Yes we call mpam_fb_disable_mpam() for MPAM-Fb .

I originally worried about the MMIO path and it turns out not need
the rollback too and the current behavior is correct, see below.

last CPU of the MSC related cpu goes offline
       ↓
    hardware back to default value
       ↓
  user writes schemata  → mpam_apply_config() update comp->cfg[partid]
        ↓
   mpam_touch_msc() → smp_call_on_cpu()
        → -ENXIO only if accessibility has no online CPU
        → comp->cfg[partid] keeps the new values     <- this is right
          ↓
  a CPU of the MSC comes back online
     → mpam_reprogram_msc() replays new comp->cfg[] into the hardware


> work. So I think there is little point in trying to fix or restore any
> configuration?

I think we should drop this patch.

> 
> Propagating errors up the call chain is another issue, but I don't think
> we need to care about any hardware settings in case of a failure.
> 
> Cheers,
> Andre
> 
>>
>> Restore the cached configuration on failure so the retry writes it to
>> the RISes again.
>>
>> Only arm64 MPAM systems are affected; x86 does not support MPAM.
>>
>> Fixes: 60e33a1864fd ("arm_mpam: propagate MSC access errors for
>> mpam_reprogram_ris_partid()")
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> Cc: <stable@vger.kernel.org> # 7.4.x
>> ---
>>   drivers/resctrl/mpam_devices.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/
>> mpam_devices.c
>> index c067b6d57d71..bb667a416660 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -3412,6 +3412,7 @@ static bool mpam_update_config(struct
>> mpam_config *cfg,
>>   int mpam_apply_config(struct mpam_component *comp, u16 partid,
>>                 struct mpam_config *cfg)
>>   {
>> +    struct mpam_config old_cfg = comp->cfg[partid];
>>       struct mpam_write_config_arg arg;
>>       struct mpam_msc_ris *ris;
>>       struct mpam_vmsc *vmsc;
>> @@ -3439,8 +3440,10 @@ int mpam_apply_config(struct mpam_component
>> *comp, u16 partid,
>>                        srcu_read_lock_held(&mpam_srcu)) {
>>               arg.ris = ris;
>>               ret = mpam_touch_msc(msc, __write_config, &arg);
>> -            if (ret)
>> +            if (ret) {
>> +                comp->cfg[partid] = old_cfg;
>>                   return ret;
>> +            }
>>               ris->in_reset_state = false;
>>           }
>>       }
> 

-- 
Best regards,
Jinjie