Re: [PATCH v1 02/11] arm_mpam: Restore the error interrupt enable from mpam_cpu_online()

[Ben Horgan]

Hi Gavin,

On 7/16/26 04:59, Gavin Shan wrote:
> Hi Ben,
> 
> On 7/10/26 9:55 PM, Ben Horgan wrote:
>> When all CPUs affine to an MSC are offline the MSC may lose register state
>> which the driver then restores when an affine CPU comes back online. The
>> error interrupt enable, MPAMF_ECR.INTEN, is missed.
>>
>> Restore MPAMF_ECR at CPU online.
>>
>> Fixes: 49aa621c4dca ("arm_mpam: Register and enable IRQs")
>> Signed-off-by: Ben Horgan <ben.horgan@xxxxxxx>
>> ---
>>   drivers/resctrl/mpam_devices.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>> index 82966ca2a631..acfa9a4dc2fc 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -1852,8 +1852,14 @@ static int mpam_cpu_online(unsigned int cpu)
>>           if (msc->reenable_error_ppi)
>>               _enable_percpu_irq(&msc->reenable_error_ppi);
>>   -        if (atomic_fetch_inc(&msc->online_refs) == 0)
>> +        if (atomic_fetch_inc(&msc->online_refs) == 0) {
>> +            mutex_lock(&msc->error_irq_lock);
>> +            if (msc->error_irq_hw_enabled)
>> +                mpam_touch_msc(msc, mpam_enable_msc_ecr, msc);
>> +            mutex_unlock(&msc->error_irq_lock);
>> +
>>               mpam_reprogram_msc(msc);
>> +        }
>>       }
>>   
> 
> I don't understand how this happened that MPAMF_ECR gets lost, but msc->error_irq_hw_enabled
> is kept as true. mpam_disable_msc_ecr() is triggered in __mpam_irq_handler() or mpam_unregister_irqs().
> Both are related to mpam_broken_work, which is invoked to disable the driver completely.
> I don't see how MPAMF_ECR is cleared at CPU offline time.

The reasoning is the same as to why mpam_reprogram_msc() is already run here. When all the CPUs
affine to an MSC at a cache are offline then the cache instance and so the MSC might be switched off
and lose register state. Hence, any registers we rely on need to set when the MSC comes back online.
For PSCI_CPU_SUSPEND this needs to be handled by the firmware and for PSCI_CPU_OFF the driver
handles it. Does that make any more sense?

Thanks,

Ben

> 
>>       if (mpam_resctrl_enabled)
> 
> Thanks,
> Gavin
>