Hi James, (:p)
On 22/08/2025 16:30, 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, use a threaded interrupt.
>
> There is no support for percpu threaded interrupts, for now schedule
> the work to be done from the irq handler.
>
> 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.
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 3516cbe8623e..210d64fad0b1 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1547,11 +1640,171 @@ static void mpam_enable_merge_features(struct list_head *all_classes_list)
> +static irqreturn_t __mpam_irq_handler(int irq, struct mpam_msc *msc)
> +{
> + u64 reg;
> + u16 partid;
> + u8 errcode, pmg, ris;
> +
> + if (WARN_ON_ONCE(!msc) ||
> + WARN_ON_ONCE(!cpumask_test_cpu(smp_processor_id(),
> + &msc->accessibility)))
> + return IRQ_NONE;
> +
> + reg = mpam_msc_read_esr(msc);
> +
> + errcode = FIELD_GET(MPAMF_ESR_ERRCODE, reg);
> + if (!errcode)
> + return IRQ_NONE;
> +
> + /* Clear level triggered irq */
> + mpam_msc_zero_esr(msc);
> +
> + partid = FIELD_GET(MPAMF_ESR_PARTID_MON, reg);
> + pmg = FIELD_GET(MPAMF_ESR_PMG, reg);
> + ris = FIELD_GET(MPAMF_ESR_RIS, reg);
> +
> + pr_err("error irq from msc:%u '%s', partid:%u, pmg: %u, ris: %u\n",
> + msc->id, mpam_errcode_names[errcode], partid, pmg, ris);
> +
> + if (irq_is_percpu(irq)) {
> + mpam_disable_msc_ecr(msc);
> + schedule_work(&mpam_broken_work);
> + return IRQ_HANDLED;
> + }
> +
> + return IRQ_WAKE_THREAD;
> +}
> +static void mpam_unregister_irqs(void)
> +{
> + int irq, idx;
> + struct mpam_msc *msc;
> +
> + cpus_read_lock();
> + /* take the lock as free_irq() can sleep */
> + idx = srcu_read_lock(&mpam_srcu);
> + list_for_each_entry_srcu(msc, &mpam_all_msc, glbl_list, srcu_read_lock_held(&mpam_srcu)) {
> + irq = platform_get_irq_byname_optional(msc->pdev, "error");
> + if (irq <= 0)
> + continue;
> +
> + if (msc->error_irq_hw_enabled) {
> + mpam_touch_msc(msc, mpam_disable_msc_ecr, msc);
> + msc->error_irq_hw_enabled = false;
> + }
> +
> + if (msc->error_irq_requested) {
> + if (irq_is_percpu(irq)) {
> + msc->reenable_error_ppi = 0;
> + free_percpu_irq(irq, msc->error_dev_id);
> + } else {
> + devm_free_irq(&msc->pdev->dev, irq, msc);
> + }
> + msc->error_irq_requested = false;
> + }
> + }
> + srcu_read_unlock(&mpam_srcu, idx);
> + cpus_read_unlock();
> +}
> @@ -1615,16 +1889,39 @@ static void mpam_reset_class(struct mpam_class *class)
> * All of MPAMs errors indicate a software bug, restore any modified
> * controls to their reset values.
> */
> -void mpam_disable(void)
> +static irqreturn_t mpam_disable_thread(int irq, void *dev_id)
> {
> int idx;
> struct mpam_class *class;
> + struct mpam_msc *msc, *tmp;
> +
> + mutex_lock(&mpam_cpuhp_state_lock);
> + if (mpam_cpuhp_state) {
> + cpuhp_remove_state(mpam_cpuhp_state);
> + mpam_cpuhp_state = 0;
> + }
> + mutex_unlock(&mpam_cpuhp_state_lock);
> + mpam_unregister_irqs();
When out-of-range PARTID get used, all the MSC go off at once - which means the interrupts
can be delivered to multiple CPUs at the same time. This unregister call is outside any
lock, and the msc->error_irq_* flags aren't atomic - leading to hilarity as this races
with itself.
Also turns out you can't devm_free_irq() from a threaded irq as it blocks forever in
syncrhonise_irq().
Naturally I didn't hit either of these issues when scheduling the thread from debugfs.
I've made the flags atomic, and thrown the threaded-irq away - instead the work always
gets scheduled.
Thanks,
James
> idx = srcu_read_lock(&mpam_srcu);
> list_for_each_entry_srcu(class, &mpam_classes, classes_list,
> srcu_read_lock_held(&mpam_srcu))
> mpam_reset_class(class);
> srcu_read_unlock(&mpam_srcu, idx);
> +
> + mutex_lock(&mpam_list_lock);
> + list_for_each_entry_safe(msc, tmp, &mpam_all_msc, glbl_list)
> + mpam_msc_destroy(msc);
> + mutex_unlock(&mpam_list_lock);
> + mpam_free_garbage();
> +
> + return IRQ_HANDLED;
> +}
/*error_irq_requested
