[PATCH 0/2] KVM: arm64: Reject mismatched GICv3 ITS/VGIC models

[PATCH 0/2] KVM: arm64: Reject mismatched GICv3 ITS/VGIC models

[Sascha Bischoff]

On a GICv5 host with legacy GICv3 support, KVM exposes both VGIC
models to userspace. A VM can create a GICv3 ITS alongside a GICv5
VGIC, because neither creation path checks the model of the other
device. The ITS sets has_its, but a GICv5 VM cannot use a GICv3 ITS
for MSI translation. Creating the ITS before the VGIC also lets
userspace select an incompatible VGIC model later.

Record the VGIC model required by a GICv3 ITS and reject incompatible
device creation in either order. The second patch tests both orderings,
skipping the test if the host does not offer a GICv3 ITS.

This series is based on kvmarm/next-7.4+fixes-7.3-2.

Sascha

Sascha Bischoff (2):
  KVM: arm64: vgic: Require matching VGIC model for GICv3 ITS
  KVM: selftests: Reject GICv3 ITS with a GICv5 VGIC

 arch/arm64/kvm/vgic/vgic-init.c             |  5 ++++
 arch/arm64/kvm/vgic/vgic-its.c              |  9 ++++++
 include/kvm/arm_vgic.h                      |  2 ++
 tools/testing/selftests/kvm/arm64/vgic_v5.c | 32 +++++++++++++++++++++
 4 files changed, 48 insertions(+)


base-commit: 678f16b6e4767096aa3e3e6eef60e981b3f61e12
-- 
2.34.1

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

[Sascha Bischoff]

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;
+	}
+
 	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;
 	bool			table_write_in_progress;
 
 	/*
-- 
2.34.1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: vgic: its_model state leak on ITS creation failure prevents VGIC fallback
--

commit 1db9490bd1d4f94f4d1c1d3aee70dca948f96eb7
Author: Sascha Bischoff <sascha.bischoff@arm.com>

KVM: arm64: vgic: Require matching VGIC model for GICv3 ITS

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.

> 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

[ ... ]

> @@ -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;

[Severity: Medium]
When vgic_its_create() successfully executes, it updates the VM-wide state
(its_model, has_its, msis_require_devid) to reflect the ITS device. If the
generic kvm_ioctl_create_device() subsequently fails at anon_inode_getfd()
due to process FD exhaustion, it aborts the device creation and calls the
vgic_its_destroy() cleanup callback.

However, vgic_its_destroy() does not roll back the its_model VM-wide property:

arch/arm64/kvm/vgic/vgic-its.c:vgic_its_destroy() {
    ...
	vgic_its_free_device_list(kvm, its);
	vgic_its_free_collection_list(kvm, its);
	vgic_its_invalidate_cache(its);
	xa_destroy(&its->translation_cache);

	mutex_unlock(&its->its_lock);
	kfree(its);
	kfree(kvm_dev);
}

Consequently, the VM permanently records that it requires a GICv3 VGIC model.

Does this state inconsistency prevent userspace from recovering and falling
back to a different VGIC model (such as VGICv5) after an ephemeral FD
allocation error, or if userspace intentionally closes the ITS file descriptor
before creating the VGIC?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016152225.898761-1-sascha.bischoff@arm.com?part=1

[PATCH 2/2] KVM: selftests: Reject GICv3 ITS with a GICv5 VGIC

[Sascha Bischoff]

Check that a GICv3 ITS and a GICv5 VGIC cannot be paired, regardless
of which device is created first. Skip the check when the host does not
expose a GICv3 ITS.

Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
 tools/testing/selftests/kvm/arm64/vgic_v5.c | 32 +++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/tools/testing/selftests/kvm/arm64/vgic_v5.c b/tools/testing/selftests/kvm/arm64/vgic_v5.c
index d5df742b23001..5bf7d99d29147 100644
--- a/tools/testing/selftests/kvm/arm64/vgic_v5.c
+++ b/tools/testing/selftests/kvm/arm64/vgic_v5.c
@@ -2003,8 +2003,40 @@ int test_kvm_device(u32 gic_dev_type)
 	return 0;
 }
 
+static void test_vgic_v3_its_with_vgic_v5(void)
+{
+	struct kvm_vm *vm = vm_create_barebones();
+	int gic_fd, its_fd, ret;
+
+	if (__kvm_test_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_ITS)) {
+		pr_info("No GICv3 ITS support; skipping mixed-model test.\n");
+		kvm_vm_free(vm);
+		return;
+	}
+
+	gic_fd = kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V5);
+	ret = __kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_ITS);
+	TEST_ASSERT(ret < 0 && errno == ENODEV,
+		    "GICv3 ITS created after GICv5 VGIC: ret=%d errno=%d",
+		    ret, errno);
+	close(gic_fd);
+	kvm_vm_free(vm);
+
+	vm = vm_create_barebones();
+	its_fd = kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_ITS);
+	ret = __kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V5);
+	TEST_ASSERT(ret < 0 && errno == ENODEV,
+		    "GICv5 VGIC created after GICv3 ITS: ret=%d errno=%d",
+		    ret, errno);
+	close(its_fd);
+	kvm_vm_free(vm);
+}
+
 void run_tests(u32 gic_dev_type)
 {
+	pr_info("Test VGICv5 and GICv3 ITS model mismatch\n");
+	test_vgic_v3_its_with_vgic_v5();
+
 	pr_info("Test VGICv5 address attrs\n");
 	test_vgic_v5_addr_attrs();
 
-- 
2.34.1

Re: [PATCH 2/2] KVM: selftests: Reject GICv3 ITS with a GICv5 VGIC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: vgic_v5.c uses ad hoc skip messages and direct exit() instead of TAP
--

commit 1b97dfda1bac7a30c8395c0645ac8efb945b4639
Author: Sascha Bischoff <sascha.bischoff@arm.com>

KVM: selftests: Reject GICv3 ITS with a GICv5 VGIC

Check that a GICv3 ITS and a GICv5 VGIC cannot be paired, regardless
of which device is created first. Skip the check when the host does not
expose a GICv3 ITS.

> diff --git a/tools/testing/selftests/kvm/arm64/vgic_v5.c b/tools/testing/selftests/kvm/arm64/vgic_v5.c
> index d5df742b23001..5bf7d99d29147 100644
> --- a/tools/testing/selftests/kvm/arm64/vgic_v5.c
> +++ b/tools/testing/selftests/kvm/arm64/vgic_v5.c
[ ... ]
> @@ -2003,8 +2003,40 @@ int test_kvm_device(u32 gic_dev_type)
>  	return 0;
>  }
>  
> +static void test_vgic_v3_its_with_vgic_v5(void)
> +{
> +	struct kvm_vm *vm = vm_create_barebones();
> +	int gic_fd, its_fd, ret;
> +
> +	if (__kvm_test_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_ITS)) {
> +		pr_info("No GICv3 ITS support; skipping mixed-model test.\n");
> +		kvm_vm_free(vm);
> +		return;
> +	}

[Severity: Medium]
When running test_vgic_v3_its_with_vgic_v5() on a host without GICv3 ITS
support, this prints an unstructured skip message and returns directly.

Could this be converted to use a TAP reporting macro like
ksft_test_result_skip() instead of an ad-hoc pr_info() and return?

Additionally, running the test suite on a host without GICv5 support calls
exit(KSFT_SKIP) directly in main() instead of using ksft_exit_skip().
Are there plans to update the main() exit path as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016152225.898761-1-sascha.bischoff@arm.com?part=2