Re: [PATCH] irqchip/gic-v3-its: Don't acquire rt_spin_lock in allocate_vpe_l1_table()

On Mon, 12 Jan 2026 14:08:37 +0000,
Sebastian Andrzej Siewior [off-list ref] wrote:

On 2026-01-12 11:20:07 [+0000], Marc Zyngier wrote:

quoted

On Sun, 11 Jan 2026 16:20:45 +0000,
Thomas Gleixner [off-list ref] wrote:

quoted

On Sun, Jan 11 2026 at 10:38, Marc Zyngier wrote:

quoted

On Sun, 11 Jan 2026 09:39:07 +0000,
Thomas Gleixner [off-list ref] wrote:

quoted

On Fri, Jan 09 2026 at 16:13, Marc Zyngier wrote:

quoted

On Thu, 08 Jan 2026 22:11:33 +0000,
Thomas Gleixner [off-list ref] wrote:

quoted

At the point where a CPU is brought up, the topology should be known
already, which means this can be allocated on the control CPU _before_
the new CPU comes up, no?

No. Each CPU finds *itself* in the forest of redistributors, and from
there tries to find whether it has some shared resource with a CPU
that has booted before it. That's because firmware is absolutely awful
and can't present a consistent view of the system.

Groan....

quoted

Anyway, I expect it could be solved by moving this part of the init to
an ONLINE HP callback.

Which needs to be before CPUHP_AP_IRQ_AFFINITY_ONLINE, but even that
might be to late because there are callbacks in the STARTING section,
i.e. timer, perf, which might rely on interrupts being accessible.

Nah. This stuff is only for direct injection of vLPIs into guests, so
as long as this is done before we can schedule a vcpu on this physical
CPU, we're good. No physical interrupt is concerned with this code.

That's fine then. vCPUs are considered "user-space" tasks and can't be
scheduled before CPUHP_AP_ACTIVE sets the CPU active for the scheduler.

Waiman, can you please give the following hack a go on your box? The
machines I have are thankfully limited to a single ITS group, so I
can't directly reproduce your issue.

Thanks,

	M.

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index ada585bfa4517..20967000f2348 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c

@@ -2896,7 +2896,7 @@ static bool allocate_vpe_l2_table(int cpu, u32 id)
 	return true;
 }
 
-static int allocate_vpe_l1_table(void)
+static int allocate_vpe_l1_table(unsigned int cpu)
 {
 	void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
 	u64 val, gpsz, npg, pa;

@@ -3012,10 +3012,11 @@ static int allocate_vpe_l1_table(void)
 
 out:
 	gicr_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
-	cpumask_set_cpu(smp_processor_id(), gic_data_rdist()->vpe_table_mask);
+	cpumask_set_cpu(cpu, gic_data_rdist()->vpe_table_mask);
+	dsb(sy);
 
 	pr_debug("CPU%d: VPROPBASER = %llx %*pbl\n",
-		 smp_processor_id(), val,
+		 cpu, val,
 		 cpumask_pr_args(gic_data_rdist()->vpe_table_mask));
 
 	return 0;

@@ -3264,15 +3265,9 @@ static void its_cpu_init_lpis(void)
 		val = its_clear_vpend_valid(vlpi_base, 0, 0);
 	}
 
-	if (allocate_vpe_l1_table()) {
-		/*
-		 * If the allocation has failed, we're in massive trouble.
-		 * Disable direct injection, and pray that no VM was
-		 * already running...
-		 */
-		gic_rdists->has_rvpeid = false;
-		gic_rdists->has_vlpis = false;
-	}
+	if (smp_processor_id() == 0)
+		cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "irqchip/arm/gicv3:vpe",
+				  allocate_vpe_l1_table, NULL);

If you move it the online state then you could also
s/GFP_ATOMIC/GFP_KERNEL.

Also previously you checked the error code set has_rvpeid, has_vlpis on
failure. Now you you should the same in case of a failure during
registration.
This also happens happens on CPU hotplug and I don't see how you avoid a
second allocation. But I also don't understand why this registrations
happens on CPU0. It might be just a test patch…

It's just a test hack. There is way more things that would need to
change in order to cope with moving this to CPUHP, but I want
confirmation that this indeed solves the original issue before I start
breaking more things.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.