Re: [PATCH 1/2] KVM: arm64: vgic: Require matching VGIC model for GICv3 ITS

[Marc Zyngier <maz@kernel.org>]

On Fri, 16 Oct 2026 16:22:23 +0100,
Sascha Bischoff <sascha.bischoff@arm.com> wrote:
> 
> On a GICv5 host with legacy support, KVM registers both VGICv3 and
> VGICv5 devices. A VM can create a GICv3 ITS with another VGIC model,
> setting has_its even though that model cannot use the GICv3 ITS.
> 
> Reject GICv3 ITS creation when the VM already has an incompatible VGIC.
> KVM also permits creating an ITS before its VGIC, so record the model
> required by the ITS and reject a later incompatible VGIC. Preserve
> either creation order when the models match.
> 
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c | 5 +++++
>  arch/arm64/kvm/vgic/vgic-its.c  | 9 +++++++++
>  include/kvm/arm_vgic.h          | 2 ++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index b5ea1e39c6862..48a92555fa6ab 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -125,6 +125,11 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
>  		goto out_unlock;
>  	}
>  
> +	if (kvm->arch.vgic.its_model && kvm->arch.vgic.its_model != type) {
> +		ret = -ENODEV;
> +		goto out_unlock;
> +	}
> +
>  	if (kvm_id_regs_final(kvm))
>  		goto out_unlock;
>  	ret = 0;
> diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
> index e5657cb603567..bf901626ea6df 100644
> --- a/arch/arm64/kvm/vgic/vgic-its.c
> +++ b/arch/arm64/kvm/vgic/vgic-its.c
> @@ -1875,6 +1875,14 @@ static int vgic_its_create(struct kvm_device *dev, u32 type)
>  
>  	mutex_lock(&dev->kvm->arch.config_lock);
>  
> +	if ((irqchip_in_kernel(dev->kvm) && !vgic_is_v3(dev->kvm)) ||
> +	    (dev->kvm->arch.vgic.its_model &&
> +	     dev->kvm->arch.vgic.its_model != KVM_DEV_TYPE_ARM_VGIC_V3)) {
> +		mutex_unlock(&dev->kvm->arch.config_lock);
> +		kfree(its);
> +		return -ENODEV;
> +	}
> +

A comment would go a long way to make it clear what this is checking
for:

	- if a GIC already exists and is not v3, fail
	- if an ITS already exists and is not v3, fail

>  	if (vgic_initialized(dev->kvm)) {
>  		ret = vgic_v4_init(dev->kvm);
>  		if (ret < 0) {
> @@ -1903,6 +1911,7 @@ static int vgic_its_create(struct kvm_device *dev, u32 type)
>  
>  	dev->kvm->arch.vgic.msis_require_devid = true;
>  	dev->kvm->arch.vgic.has_its = true;
> +	dev->kvm->arch.vgic.its_model = KVM_DEV_TYPE_ARM_VGIC_V3;
>  	its->enabled = false;
>  	its->dev = dev;
>  
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 7be81b936bf10..a2b6b328808ba 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -548,6 +548,8 @@ struct vgic_dist {
>  	struct vgic_io_device	cpuif_iodev;
>  
>  	bool			has_its;
> +	/* VGIC model required by the VM's ITS devices. */
> +	u32			its_model;

I reckon that has_its and its_model can be made the same thing, since
its_model == 0 indicates the lack of ITS, and it is otherwise the same
as vgic_model.

I ended up with this (untested). Thoughts?

	M.

diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 2d915769cdc7c..c70390b7fcab6 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -456,6 +456,9 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 		 */
 		if (vcpu->vcpu_id >= kvm_vgic_global_state.max_gicv5_vcpus)
 			return -EINVAL;
+		break;
+	default:
+		unreachable();
 	}
 
 	return ret;
@@ -572,6 +575,8 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm)
 		kfree(dist->vgic_v5_irs_data);
 		dist->vgic_v5_irs_data = NULL;
 		break;
+	default:
+		unreachable();
 	}
 
 	xa_destroy(&dist->lpi_xa);
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 0ec461df74cd9..171e5e345c220 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -1963,7 +1963,6 @@ static int vgic_its_create(struct kvm_device *dev, u32 type)
 	xa_init(&its->translation_cache);
 
 	dev->kvm->arch.vgic.msis_require_devid = true;
-	dev->kvm->arch.vgic.has_its = true;
 	dev->kvm->arch.vgic.its_model = KVM_DEV_TYPE_ARM_VGIC_V3;
 	its->enabled = false;
 	its->dev = dev;
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 636c576b2b99e..282fac37b6642 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -42,10 +42,7 @@ bool vgic_has_its(struct kvm *kvm)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
 
-	if (dist->vgic_model != KVM_DEV_TYPE_ARM_VGIC_V3)
-		return false;
-
-	return dist->has_its;
+	return (dist->its_model == dist->vgic_model);
 }
 
 bool vgic_supports_direct_msis(struct kvm *kvm)
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index c4135815884b6..4b2c41917b77c 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -503,7 +503,9 @@ struct vgic_dist {
 	bool			initialized;
 
 	/* vGIC model the kernel emulates for the guest (GICv2 or GICv3) */
-	u32			vgic_model;
+	enum kvm_device_type	vgic_model;
+	/* VGIC model required by the VM's ITS devices. */
+	enum kvm_device_type	its_model;
 
 	/* Implementation revision as reported in the GICD_IIDR */
 	u32			implementation_rev;
@@ -548,9 +550,6 @@ struct vgic_dist {
 	struct vgic_io_device	dist_iodev;
 	struct vgic_io_device	cpuif_iodev;
 
-	bool			has_its;
-	/* VGIC model required by the VM's ITS devices. */
-	u32			its_model;
 	bool			table_write_in_progress;
 
 	/*

-- 
Jazz isn't dead. It just smells funny.