Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] irqchip/gic-v4, KVM: arm64: Fix the vgic init error paths
@ 2026-08-20 12:50 Fuad Tabba
  2026-08-20 12:50 ` [PATCH 1/3] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them Fuad Tabba
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Fuad Tabba @ 2026-08-20 12:50 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton
  Cc: Thomas Gleixner, Eric Auger, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Will Deacon, Sascha Bischoff,
	Sebastian Ene, Fuad Tabba, kvmarm, linux-arm-kernel, linux-kernel

Hi folks,

Three fixes on the paths that run when vgic init fails partway. Each one
needs an allocation failure to reach.

Sashiko reported the vgic_init() case [1] while reviewing the SPI-array
leak fix. The two irq-gic-v4.c fixes came out of chasing it [2].

Patch 1 clears the irq domain and fwnode pointers at the four GICv4
sites that free them and leave them set. Patch 2 makes
its_alloc_vcpu_irqs() release what it allocated when the SGI loop fails.
Patch 3 releases the SPI array and the vPEs on vgic_init()'s later
failure paths, so KVM_DEV_ARM_VGIC_CTRL_INIT is all or nothing and a
retry starts from scratch.

These patches run across two subsystems, but ordering matters, which is
why this is one series. Patch 3 runs vgic_v4_teardown() on a path it
never ran on before, which adds a route into what patch 1 fixes, and
patch 2 reuses its_free_sgi_irqs() from an error path, which only works
once patch 1 has cleared the pointers.

Based on kvmarm/next (aa8e5dc6a7a2a).

Cheers,
/fuad

[1] https://lore.kernel.org/all/20260807105558.73D701F000E9@smtp.kernel.org/
[2] https://lore.kernel.org/all/CA+EHjTyDix+y6NTLTsXP5j9Sn2VLOZcsw94LntQxg_etRBJZuA@mail.gmail.com/

Fuad Tabba (3):
  irqchip/gic-v4: Clear the domain and fwnode pointers after freeing
    them
  irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure
  KVM: arm64: vgic: Tear down what vgic_init() created when it fails

 arch/arm64/kvm/vgic/vgic-init.c | 11 +++++++++--
 drivers/irqchip/irq-gic-v4.c    | 34 +++++++++++++++++++++++++++------
 2 files changed, 37 insertions(+), 8 deletions(-)


base-commit: aa8e5dc6a7a2a1141ab40706a51010adcd0e57d2
-- 
2.39.5



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them
  2026-08-20 12:50 [PATCH 0/3] irqchip/gic-v4, KVM: arm64: Fix the vgic init error paths Fuad Tabba
@ 2026-08-20 12:50 ` Fuad Tabba
  2026-08-20 12:50 ` [PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure Fuad Tabba
  2026-08-20 12:50 ` [PATCH 3/3] KVM: arm64: vgic: Tear down what vgic_init() created when it fails Fuad Tabba
  2 siblings, 0 replies; 4+ messages in thread
From: Fuad Tabba @ 2026-08-20 12:50 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton
  Cc: Thomas Gleixner, Eric Auger, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Will Deacon, Sascha Bischoff,
	Sebastian Ene, Fuad Tabba, kvmarm, linux-arm-kernel, linux-kernel

The GICv4 allocation and teardown paths free their irq domains and
fwnodes but leave the pointers set, and the allocation error paths test
those pointers before removing them. struct its_vm and struct its_vpe
are embedded in KVM's per-VM and per-vCPU state, so nothing re-zeroes
them between two attempts, and an error path taken after an earlier one
already freed the domain calls irq_domain_remove() on freed memory.

Reaching this takes two allocation failures, one to leave the stale
pointer behind and one to send the next attempt down the error path.

Fixes: 7de5c0af9c7c ("irqchip/gic-v4: Add per-VM VPE domain creation")
Fixes: 6d31b6ff985d ("irqchip/gic-v4.1: Add VSGI allocation/teardown")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/irqchip/irq-gic-v4.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 8455b4a5fbb0d..754839e409f88 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -147,10 +147,14 @@ static int its_alloc_vcpu_sgis(struct its_vpe *vpe, int idx)
 	return 0;
 
 err:
-	if (vpe->sgi_domain)
+	if (vpe->sgi_domain) {
 		irq_domain_remove(vpe->sgi_domain);
-	if (vpe->fwnode)
+		vpe->sgi_domain = NULL;
+	}
+	if (vpe->fwnode) {
 		irq_domain_free_fwnode(vpe->fwnode);
+		vpe->fwnode = NULL;
+	}
 	kfree(name);
 	return -ENOMEM;
 }
@@ -191,10 +195,14 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
 	return 0;
 
 err:
-	if (vm->domain)
+	if (vm->domain) {
 		irq_domain_remove(vm->domain);
-	if (vm->fwnode)
+		vm->domain = NULL;
+	}
+	if (vm->fwnode) {
 		irq_domain_free_fwnode(vm->fwnode);
+		vm->fwnode = NULL;
+	}
 
 	return -ENOMEM;
 }
@@ -215,6 +223,8 @@ static void its_free_sgi_irqs(struct its_vm *vm)
 		irq_domain_free_irqs(irq, 16);
 		irq_domain_remove(vm->vpes[i]->sgi_domain);
 		irq_domain_free_fwnode(vm->vpes[i]->fwnode);
+		vm->vpes[i]->sgi_domain = NULL;
+		vm->vpes[i]->fwnode = NULL;
 	}
 }
 
@@ -224,6 +234,8 @@ void its_free_vcpu_irqs(struct its_vm *vm)
 	irq_domain_free_irqs(vm->vpes[0]->irq, vm->nr_vpes);
 	irq_domain_remove(vm->domain);
 	irq_domain_free_fwnode(vm->fwnode);
+	vm->domain = NULL;
+	vm->fwnode = NULL;
 }
 
 static int its_send_vpe_cmd(struct its_vpe *vpe, struct its_cmd_info *info)
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure
  2026-08-20 12:50 [PATCH 0/3] irqchip/gic-v4, KVM: arm64: Fix the vgic init error paths Fuad Tabba
  2026-08-20 12:50 ` [PATCH 1/3] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them Fuad Tabba
@ 2026-08-20 12:50 ` Fuad Tabba
  2026-08-20 12:50 ` [PATCH 3/3] KVM: arm64: vgic: Tear down what vgic_init() created when it fails Fuad Tabba
  2 siblings, 0 replies; 4+ messages in thread
From: Fuad Tabba @ 2026-08-20 12:50 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton
  Cc: Thomas Gleixner, Eric Auger, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Will Deacon, Sascha Bischoff,
	Sebastian Ene, Fuad Tabba, kvmarm, linux-arm-kernel, linux-kernel

A failure in the its_alloc_vcpu_sgis() loop leaves behind both the SGI
domains created for the vPEs below the failing index and the vPE irqs
allocated before the loop, since irq_domain_remove() frees neither. Each
leaked vPE takes its ITS state with it, a vpe_id and an LPI pending
table.

Free both from a second label before the existing unwind. With the
freed pointers now cleared, its_free_sgi_irqs() can skip a vPE with no
SGI domain rather than warn on it, so the error path can reuse it.

Fixes: 6d31b6ff985d ("irqchip/gic-v4.1: Add VSGI allocation/teardown")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/irqchip/irq-gic-v4.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 754839e409f88..f707a3cb281aa 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -159,6 +159,8 @@ static int its_alloc_vcpu_sgis(struct its_vpe *vpe, int idx)
 	return -ENOMEM;
 }
 
+static void its_free_sgi_irqs(struct its_vm *vm);
+
 int its_alloc_vcpu_irqs(struct its_vm *vm)
 {
 	int vpe_base_irq, i;
@@ -189,11 +191,14 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
 		vm->vpes[i]->irq = vpe_base_irq + i;
 		ret = its_alloc_vcpu_sgis(vm->vpes[i], i);
 		if (ret)
-			goto err;
+			goto err_free_irqs;
 	}
 
 	return 0;
 
+err_free_irqs:
+	its_free_sgi_irqs(vm);
+	irq_domain_free_irqs(vpe_base_irq, vm->nr_vpes);
 err:
 	if (vm->domain) {
 		irq_domain_remove(vm->domain);
@@ -215,8 +220,13 @@ static void its_free_sgi_irqs(struct its_vm *vm)
 		return;
 
 	for (i = 0; i < vm->nr_vpes; i++) {
-		unsigned int irq = irq_find_mapping(vm->vpes[i]->sgi_domain, 0);
+		unsigned int irq;
 
+		/* irq_find_mapping() falls back to the default domain on NULL. */
+		if (!vm->vpes[i]->sgi_domain)
+			continue;
+
+		irq = irq_find_mapping(vm->vpes[i]->sgi_domain, 0);
 		if (WARN_ON(!irq))
 			continue;
 
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] KVM: arm64: vgic: Tear down what vgic_init() created when it fails
  2026-08-20 12:50 [PATCH 0/3] irqchip/gic-v4, KVM: arm64: Fix the vgic init error paths Fuad Tabba
  2026-08-20 12:50 ` [PATCH 1/3] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them Fuad Tabba
  2026-08-20 12:50 ` [PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure Fuad Tabba
@ 2026-08-20 12:50 ` Fuad Tabba
  2 siblings, 0 replies; 4+ messages in thread
From: Fuad Tabba @ 2026-08-20 12:50 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton
  Cc: Thomas Gleixner, Eric Auger, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Will Deacon, Sascha Bischoff,
	Sebastian Ene, Fuad Tabba, kvmarm, linux-arm-kernel, linux-kernel

Once kvm_vgic_dist_init() has succeeded, every later failure in
vgic_init() returns with the SPI array still allocated. A failure after
vgic_v4_init() has also succeeded, which today means only
kvm_vgic_setup_default_irq_routing(), leaves the vPE array behind as
well.

A failed vgic_init() leaves kvm_arch_vcpu_precreate() admitting new
vCPUs, so a retry of KVM_DEV_ARM_VGIC_CTRL_INIT reaches
vgic_v4_init()'s early return with an array that no longer covers every
vCPU, and vgic_v3_load()'s WARN_ON(vgic_v4_load()) fires on the first
one it misses.

Release both on the two paths that can reach them, so the ioctl is all
or nothing and a retry starts from scratch. dist->nr_spis stays frozen,
since the SPI count cannot change once vgic_init() has supplied it.

Fixes: 74fe55dc9ab7 ("KVM: arm/arm64: GICv4: Add init/teardown of the per-VM vPE irq domain")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260807105558.73D701F000E9@smtp.kernel.org/
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-init.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 4012df6002ea6..7493fded53acc 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -462,7 +462,7 @@ int vgic_init(struct kvm *kvm)
 		if (vgic_supports_direct_irqs(kvm)) {
 			ret = vgic_v4_init(kvm);
 			if (ret)
-				return ret;
+				goto out_teardown;
 		}
 	} else {
 		ret = vgic_v5_init(kvm);
@@ -475,12 +475,19 @@ int vgic_init(struct kvm *kvm)
 
 	ret = kvm_vgic_setup_default_irq_routing(kvm);
 	if (ret)
-		return ret;
+		goto out_teardown;
 
 	vgic_debug_init(kvm);
 	dist->initialized = true;
 
 	return 0;
+
+out_teardown:
+	vgic_v4_teardown(kvm);
+	kfree(dist->spis);
+	dist->spis = NULL;
+
+	return ret;
 }
 
 static void kvm_vgic_dist_destroy(struct kvm *kvm)
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-20 12:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 12:50 [PATCH 0/3] irqchip/gic-v4, KVM: arm64: Fix the vgic init error paths Fuad Tabba
2026-08-20 12:50 ` [PATCH 1/3] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them Fuad Tabba
2026-08-20 12:50 ` [PATCH 2/3] irqchip/gic-v4: Unwind what its_alloc_vcpu_irqs() allocated on failure Fuad Tabba
2026-08-20 12:50 ` [PATCH 3/3] KVM: arm64: vgic: Tear down what vgic_init() created when it fails Fuad Tabba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox