Re: [RFC PATCH 25/36] arm_mpam: Register and enable IRQs

Hi Jonathan,

On 22/07/2025 16:06, Jonathan Cameron wrote:

On Fri, 11 Jul 2025 18:36:37 +0000
James Morse [off-list ref] wrote:

quoted

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.

Sparse gives an imbalance warning in mpam_register_irqs()

quoted

+static int mpam_register_irqs(void)
+{
+	int err, irq, idx;
+	struct mpam_msc *msc;
+
+	lockdep_assert_cpus_held();
+
+	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;
+
+		/* The MPAM spec says the interrupt can be SPI, PPI or LPI */
+		/* We anticipate sharing the interrupt with other MSCs */
+		if (irq_is_percpu(irq)) {
+			err = request_percpu_irq(irq, &mpam_ppi_handler,
+						 "mpam:msc:error",
+						 msc->error_dev_id);
+			if (err)
+				return err;

Looks like the srcu_read_lock is still held.

Oops,

There is a DEFINE_LOCK_GUARD_1() in srcu.h so you can do

	guard(srcu)(&mpam_srcu, idx);

I think and not worry about releasing it in errors or the good path.

Sure ... but having the compiler chose when to release locks makes me nervous!


Thanks,

James