Re: [PATCH 2/2] KVM: arm64: vgic-v4: Don't allocate vPEs when userspace clears nASSGIcap

[Fuad Tabba <fuad.tabba@linux.dev>]

Hi Oliver,

On Mon, 26 Oct 2026 19:39:21 +0000, Oliver Upton <oupton@kernel.org> wrote:
[...]
> Looking at both of the patches now, I'd rather we just fix the
> predicates and use those to consistently gate GICv4. Untested as always:

I open coded the nASSGIcap check because vgic_its_create() only sets
has_its after vgic_v4_init(), but the predicate is the better place for
it.

> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
[...]
> @@ -1866,9 +1866,16 @@ static int vgic_its_create(struct kvm_device *dev, u32 type)
>  
>          mutex_lock(&dev->kvm->arch.config_lock);
>  
> +        /*
> +         * Make an early indication that this VM has an ITS so calls to
> +         * vgic_supports_direct_irqs() work as intended, rolling back state
> +         * if we fail to setup GICv4.
> +         */
> +        dev->kvm->arch.vgic.has_its = true;

This makes vgic_supports_direct_irqs() true for running vCPUs before
vgic_v4_init() has allocated their vPEs. On GICv4.0, a vCPU loaded in
that window hits the WARN_ON(vgic_v4_load()) in vgic_v3_load()
(vpe->irq is still 0). With an msleep() holding vgic_its_create()
there, my reproducer, which creates an ITS while a vCPU runs, hits it
on the FVP. Keeping the store after vgic_v4_init(), where it is today,
avoids that window (diff below).

[...]
> diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
[...]
> @@ -248,8 +248,8 @@ int vgic_v4_init(struct kvm *kvm)
>  
>          lockdep_assert_held(&kvm->arch.config_lock);
>  
> -        if (!kvm_vgic_global_state.has_gicv4)
> -                return 0; /* Nothing to see here... move along. */
> +        if (!vgic_supports_direct_irqs(kvm))
> +                return 0;

vgic_supports_direct_irqs() is also true for any vGICv5 VM, so with
this, creating an ITS on an initialised one, on a GICv5 host with
legacy support, reaches its_alloc_vcpu_irqs() with no GICv4 irqdomain,
where irq_domain_create_hierarchy() dereferences a NULL vpe_domain_ops.
The diff below keeps the has_gicv4 test.

It gates vgic_v4_init() on the part of vgic_supports_direct_msis()
that doesn't read has_its. That part is false for the GICv2 VM from
patch 1 and for a vGICv5 VM, so the diff drops your vgic.h hunk:

diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 5913a20d83019..a60697238a7a7 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -48,7 +48,7 @@ bool vgic_has_its(struct kvm *kvm)
 	return dist->has_its;
 }
 
-bool vgic_supports_direct_msis(struct kvm *kvm)
+bool vgic_supports_vpes(struct kvm *kvm)
 {
 	/*
 	 * Deliberately conflate vLPI and vSGI support on GICv4.1 hardware,
@@ -58,7 +58,12 @@ bool vgic_supports_direct_msis(struct kvm *kvm)
 	if (system_supports_direct_sgis() && !vgic_supports_direct_sgis(kvm))
 		return false;
 
-	return kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm);
+	return kvm_vgic_global_state.has_gicv4 && vgic_is_v3(kvm);
+}
+
+bool vgic_supports_direct_msis(struct kvm *kvm)
+{
+	return vgic_supports_vpes(kvm) && vgic_has_its(kvm);
 }
 
 bool system_supports_direct_sgis(void)
diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
index ed236f083f0d7..0a4b9a74425b4 100644
--- a/arch/arm64/kvm/vgic/vgic-v4.c
+++ b/arch/arm64/kvm/vgic/vgic-v4.c
@@ -248,7 +248,7 @@ int vgic_v4_init(struct kvm *kvm)
 
 	lockdep_assert_held(&kvm->arch.config_lock);
 
-	if (!kvm_vgic_global_state.has_gicv4)
+	if (!vgic_supports_vpes(kvm))
 		return 0; /* Nothing to see here... move along. */
 
 	if (dist->its_vm.vpes)
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index b71d486ae5145..7fc29a395a7db 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -500,6 +500,7 @@ static inline bool vgic_host_has_gicv5(void)
 }
 
 bool system_supports_direct_sgis(void);
+bool vgic_supports_vpes(struct kvm *kvm);
 bool vgic_supports_direct_msis(struct kvm *kvm);
 bool vgic_supports_direct_sgis(struct kvm *kvm);
 

Tested on the FVP (GICv4.0 and GICv4.1): KVM doesn't leak doorbells in
either patch's case, a GICv3 VM still gets its vPEs when the ITS is
created, and the reproducer above doesn't hit the WARN. Does that work
for you? If so, I'll send it as v2.

Cheers,
/fuad