Re: [PATCH v2 18/29] arm_mpam: Register and enable IRQs

James Morse
2025-10-02T18:03:16Z

Hi Ben,

On 12/09/2025 15:40, Ben Horgan wrote:
> On 9/10/25 21:42, James Morse wrote:
>> Register and enable error IRQs. All the MPAM error interrupts indicate a
>> software bug, e.g. out of range partid. If the error interrupt is ever
>> signalled, attempt to disable MPAM.
>>
>> Only the irq handler accesses the ESR register, so no locking is needed.
>> The work to disable MPAM after an error needs to happen at process
>> context as it takes mutex. It also unregisters the interrupts, meaning
>> it can't be done from the threaded part of a threaded interrupt.
>> Instead, mpam_disable() gets scheduled.
>>
>> Enabling the IRQs in the MSC may involve cross calling to a CPU that
>> can access the MSC.
>>
>> Once the IRQ is requested, the mpam_disable() path can be called
>> asynchronously, which will walk structures sized by max_partid. Ensure
>> this size is fixed before the interrupt is requested.

>> +static int __setup_ppi(struct mpam_msc *msc)
>> +{
>> + int cpu;
>> + struct device *dev = &msc->pdev->dev;
>> +
>> + msc->error_dev_id = alloc_percpu(struct mpam_msc *);
>> + if (!msc->error_dev_id)
>> + return -ENOMEM;
>> +
>> + for_each_cpu(cpu, &msc->accessibility) {
>> + struct mpam_msc *empty = *per_cpu_ptr(msc->error_dev_id, cpu);
>> +
>> + if (empty) {
>
> I'm confused about how this if conditioned can be satisfied. Isn't the
> alloc clearing msc->error_dev_id for each cpu and then it's only getting
> set for each cpu later in the iteration.

Yes, you're right.

I think this was part of the support for PPI partitions, where multiple partitions would
get set up here. This was a sanity check that they didn't overlap...

I've ripped that out.

>> + dev_err_once(dev, "MSC shares PPI with %s!\n",
>> + dev_name(&empty->pdev->dev));
>> + return -EBUSY;
>> + }
>> + *per_cpu_ptr(msc->error_dev_id, cpu) = msc;
>> + }
>> +
>> + return 0;
>> +}

Thanks,

James