[PATCH 0/3] KVM: arm64: Allow vGICv4 configuration per VM

[PATCH 0/3] KVM: arm64: Allow vGICv4 configuration per VM

[Raghavendra Rao Ananta]

Hello,

When kvm-arm.vgic_v4_enable=1, KVM adds support for direct interrupt
injection by default to all the VMs in the system, aka GICv4. A
shortcoming of the GIC architecture is that there's an absolute limit on
the number of vPEs that can be tracked by the ITS. It is possible that
an operator is running a mix of VMs on a system, only wanting to provide
a specific class of VMs with hardware interrupt injection support.

To support this, introduce a GIC attribute, KVM_DEV_ARM_VGIC_CONFIG_GICV4,
for the userspace to enable or disable vGICv4 for a given VM.

The attribute allows the configuration only when vGICv4 is enabled in KVM,
else it acts a read-only attribute returning
KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE as the value.

On the other hand, if KVM has the vGICv4 enabled via the cmdline, the
VM absorbs this configuration by default to maintain the backward
compatibility. Userspace can get the attribute's value to check if the VM
has vGICv4 support if it sees KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE as the
value. As required, it can disable vGICv4 by setting
KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE as the value.

The patches are distrubuted as:

Patch-1 contains the KVM code that introduces the
KVM_DEV_ARM_VGIC_CONFIG_GICV4 attr, and adds all the support around it.

Patch-2 adds the documentation for the said attribute.

Patch-3 extends the vgic_init kvm/arm64 selftest that tests the get and
set of this attribute in various configurations.

Thank you.
Raghavendra

Raghavendra Rao Ananta (3):
  kvm: arm64: Add support for KVM_DEV_ARM_VGIC_CONFIG_GICV4 attr
  docs: kvm: devices/arm-vgic-v3: Document KVM_DEV_ARM_VGIC_CONFIG_GICV4
    attr
  KVM: selftests: Extend vgic_init to test GICv4 config attr

 .../virt/kvm/devices/arm-vgic-v3.rst          | 24 ++++++--
 arch/arm64/include/uapi/asm/kvm.h             |  7 +++
 arch/arm64/kvm/vgic/vgic-init.c               |  3 +
 arch/arm64/kvm/vgic/vgic-its.c                |  2 +-
 arch/arm64/kvm/vgic/vgic-kvm-device.c         | 39 +++++++++++++
 arch/arm64/kvm/vgic/vgic-mmio-v3.c            | 12 ++--
 arch/arm64/kvm/vgic/vgic-v3.c                 | 16 ++++-
 arch/arm64/kvm/vgic/vgic-v4.c                 |  8 +--
 include/kvm/arm_vgic.h                        |  5 ++
 tools/testing/selftests/kvm/arm64/vgic_init.c | 58 +++++++++++++++++++
 10 files changed, 157 insertions(+), 17 deletions(-)


base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
--
2.49.0.1101.gccaa498523-goog

[PATCH 1/3] kvm: arm64: Add support for KVM_DEV_ARM_VGIC_CONFIG_GICV4 attr

[Raghavendra Rao Ananta]

When kvm-arm.vgic_v4_enable=1, KVM adds support for direct interrupt
injection by default to all the VMs in the system, aka GICv4. A
shortcoming of the GIC architecture is that there's an absolute limit on
the number of vPEs that can be tracked by the ITS. It is possible that
an operator is running a mix of VMs on a system, only wanting to provide
a specific class of VMs with hardware interrupt injection support.

To support this, introduce a GIC attribute, KVM_DEV_ARM_VGIC_CONFIG_GICV4,
for the userspace to enable or disable vGICv4 for a given VM. Make the
interface backward compatible by leaving vGICv4 enabled by default.

Suggested-by: Oliver Upton <oliver.upton@linux.dev>
Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 arch/arm64/include/uapi/asm/kvm.h     |  7 +++++
 arch/arm64/kvm/vgic/vgic-init.c       |  3 +++
 arch/arm64/kvm/vgic/vgic-its.c        |  2 +-
 arch/arm64/kvm/vgic/vgic-kvm-device.c | 39 +++++++++++++++++++++++++++
 arch/arm64/kvm/vgic/vgic-mmio-v3.c    | 12 ++++-----
 arch/arm64/kvm/vgic/vgic-v3.c         | 16 +++++++++--
 arch/arm64/kvm/vgic/vgic-v4.c         |  8 +++---
 include/kvm/arm_vgic.h                |  5 ++++
 8 files changed, 79 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index af9d9acaf997..6762683f7e0f 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -428,6 +428,13 @@ enum {
 #define   KVM_DEV_ARM_ITS_RESTORE_TABLES        2
 #define   KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES	3
 #define   KVM_DEV_ARM_ITS_CTRL_RESET		4
+#define   KVM_DEV_ARM_VGIC_CONFIG_GICV4	5
+
+enum {
+	KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE = 0,
+	KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE,
+	KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE,
+};
 
 /* Device Control API on vcpu fd */
 #define KVM_ARM_VCPU_PMU_V3_CTRL	0
diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 1f33e71c2a73..cd345df2271f 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -132,6 +132,9 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
 
 	kvm->arch.vgic.in_kernel = true;
 	kvm->arch.vgic.vgic_model = type;
+	kvm->arch.vgic.gicv4_config = kvm_vgic_global_state.has_gicv4 ?
+				      KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE :
+				      KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE;
 
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
 
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index fb96802799c6..bba635e4e851 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -2242,7 +2242,7 @@ static int vgic_its_save_itt(struct vgic_its *its, struct its_device *device)
 		 * have direct access to that state without GICv4.1.
 		 * Let's simply fail the save operation...
 		 */
-		if (ite->irq->hw && !kvm_vgic_global_state.has_gicv4_1)
+		if (ite->irq->hw && !kvm_vm_has_gicv4_1(its->dev->kvm))
 			return -EACCES;
 
 		ret = vgic_its_save_ite(its, device, ite, gpa);
diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
index 359094f68c23..f03b80fc816e 100644
--- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
+++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
@@ -279,6 +279,33 @@ static int vgic_set_common_attr(struct kvm_device *dev,
 			unlock_all_vcpus(dev->kvm);
 			mutex_unlock(&dev->kvm->lock);
 			return r;
+		case KVM_DEV_ARM_VGIC_CONFIG_GICV4: {
+			u8 __user *uaddr = (u8 __user *)(long)attr->addr;
+			u8 val;
+
+			if (!kvm_vgic_global_state.has_gicv4)
+				return -ENXIO;
+
+			if (get_user(val, uaddr))
+				return -EFAULT;
+
+			if (vgic_initialized(dev->kvm) &&
+				val != dev->kvm->arch.vgic.gicv4_config)
+				return -EBUSY;
+
+			switch (val) {
+			case KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE:
+			case KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE:
+				mutex_lock(&dev->kvm->arch.config_lock);
+				dev->kvm->arch.vgic.gicv4_config = val;
+				mutex_unlock(&dev->kvm->arch.config_lock);
+				break;
+			default:
+				return -EINVAL;
+			}
+
+			return 0;
+		}
 		}
 		break;
 	}
@@ -309,6 +336,16 @@ static int vgic_get_common_attr(struct kvm_device *dev,
 		r = put_user(dev->kvm->arch.vgic.mi_intid, uaddr);
 		break;
 	}
+	case KVM_DEV_ARM_VGIC_GRP_CTRL: {
+		switch (attr->attr) {
+		case KVM_DEV_ARM_VGIC_CONFIG_GICV4: {
+			u8 __user *uaddr = (u8 __user *)(long)attr->addr;
+
+			r = put_user(dev->kvm->arch.vgic.gicv4_config, uaddr);
+			break;
+		}
+		}
+	}
 	}
 
 	return r;
@@ -684,6 +721,8 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
 			return 0;
 		case KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES:
 			return 0;
+		case KVM_DEV_ARM_VGIC_CONFIG_GICV4:
+			return 0;
 		}
 	}
 	return -ENXIO;
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index ae4c0593d114..66b365f59c51 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -50,8 +50,8 @@ bool vgic_has_its(struct kvm *kvm)
 
 bool vgic_supports_direct_msis(struct kvm *kvm)
 {
-	return (kvm_vgic_global_state.has_gicv4_1 ||
-		(kvm_vgic_global_state.has_gicv4 && vgic_has_its(kvm)));
+	return kvm_vm_has_gicv4(kvm) &&
+		(kvm_vgic_global_state.has_gicv4_1 || vgic_has_its(kvm));
 }
 
 /*
@@ -86,7 +86,7 @@ static unsigned long vgic_mmio_read_v3_misc(struct kvm_vcpu *vcpu,
 		}
 		break;
 	case GICD_TYPER2:
-		if (kvm_vgic_global_state.has_gicv4_1 && gic_cpuif_has_vsgi())
+		if (kvm_vm_has_gicv4_1(vcpu->kvm) && gic_cpuif_has_vsgi())
 			value = GICD_TYPER2_nASSGIcap;
 		break;
 	case GICD_IIDR:
@@ -119,7 +119,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
 		dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
 
 		/* Not a GICv4.1? No HW SGIs */
-		if (!kvm_vgic_global_state.has_gicv4_1 || !gic_cpuif_has_vsgi())
+		if (!kvm_vm_has_gicv4_1(vcpu->kvm) || !gic_cpuif_has_vsgi())
 			val &= ~GICD_CTLR_nASSGIreq;
 
 		/* Dist stays enabled? nASSGIreq is RO */
@@ -133,7 +133,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
 		if (is_hwsgi != dist->nassgireq)
 			vgic_v4_configure_vsgis(vcpu->kvm);
 
-		if (kvm_vgic_global_state.has_gicv4_1 &&
+		if (kvm_vm_has_gicv4_1(vcpu->kvm) &&
 		    was_enabled != dist->enabled)
 			kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_RELOAD_GICv4);
 		else if (!was_enabled && dist->enabled)
@@ -178,7 +178,7 @@ static int vgic_mmio_uaccess_write_v3_misc(struct kvm_vcpu *vcpu,
 		}
 	case GICD_CTLR:
 		/* Not a GICv4.1? No HW SGIs */
-		if (!kvm_vgic_global_state.has_gicv4_1)
+		if (!kvm_vm_has_gicv4_1(vcpu->kvm))
 			val &= ~GICD_CTLR_nASSGIreq;
 
 		dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index b9ad7c42c5b0..bc8cb9184be9 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -20,6 +20,18 @@ static bool common_trap;
 static bool dir_trap;
 static bool gicv4_enable;
 
+int kvm_vm_has_gicv4(struct kvm *kvm)
+{
+	return kvm->arch.vgic.gicv4_config ==
+		KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE;
+}
+
+int kvm_vm_has_gicv4_1(struct kvm *kvm)
+{
+	return (kvm_vm_has_gicv4(kvm) &&
+		kvm_vgic_global_state.has_gicv4_1);
+}
+
 void vgic_v3_set_underflow(struct kvm_vcpu *vcpu)
 {
 	struct vgic_v3_cpu_if *cpuif = &vcpu->arch.vgic_cpu.vgic_v3;
@@ -404,7 +416,7 @@ int vgic_v3_save_pending_tables(struct kvm *kvm)
 	 * The above vgic initialized check also ensures that the allocation
 	 * and enabling of the doorbells have already been done.
 	 */
-	if (kvm_vgic_global_state.has_gicv4_1) {
+	if (kvm_vm_has_gicv4_1(kvm)) {
 		unmap_all_vpes(kvm);
 		vlpi_avail = true;
 	}
@@ -581,7 +593,7 @@ int vgic_v3_map_resources(struct kvm *kvm)
 		return -EBUSY;
 	}
 
-	if (kvm_vgic_global_state.has_gicv4_1)
+	if (kvm_vm_has_gicv4_1(kvm))
 		vgic_v4_configure_vsgis(kvm);
 
 	return 0;
diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
index c7de6154627c..814d54f4ce13 100644
--- a/arch/arm64/kvm/vgic/vgic-v4.c
+++ b/arch/arm64/kvm/vgic/vgic-v4.c
@@ -86,7 +86,7 @@ static irqreturn_t vgic_v4_doorbell_handler(int irq, void *info)
 	struct kvm_vcpu *vcpu = info;
 
 	/* We got the message, no need to fire again */
-	if (!kvm_vgic_global_state.has_gicv4_1 &&
+	if (!kvm_vm_has_gicv4_1(vcpu->kvm) &&
 	    !irqd_irq_disabled(&irq_to_desc(irq)->irq_data))
 		disable_irq_nosync(irq);
 
@@ -245,7 +245,7 @@ int vgic_v4_init(struct kvm *kvm)
 
 	lockdep_assert_held(&kvm->arch.config_lock);
 
-	if (!kvm_vgic_global_state.has_gicv4)
+	if (!kvm_vm_has_gicv4(kvm))
 		return 0; /* Nothing to see here... move along. */
 
 	if (dist->its_vm.vpes)
@@ -286,7 +286,7 @@ int vgic_v4_init(struct kvm *kvm)
 		 * On GICv4.1, the doorbell is managed in HW and must
 		 * be left enabled.
 		 */
-		if (kvm_vgic_global_state.has_gicv4_1)
+		if (kvm_vm_has_gicv4_1(kvm))
 			irq_flags &= ~IRQ_NOAUTOEN;
 		irq_set_status_flags(irq, irq_flags);
 
@@ -392,7 +392,7 @@ int vgic_v4_load(struct kvm_vcpu *vcpu)
 	 * doorbell interrupt that would still be pending. This is a
 	 * GICv4.0 only "feature"...
 	 */
-	if (!kvm_vgic_global_state.has_gicv4_1)
+	if (!kvm_vm_has_gicv4_1(vcpu->kvm))
 		err = irq_set_irqchip_state(vpe->irq, IRQCHIP_STATE_PENDING, false);
 
 	return err;
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 714cef854c1c..8883dc677674 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -296,6 +296,8 @@ struct vgic_dist {
 	 * else.
 	 */
 	struct its_vm		its_vm;
+
+	u8			gicv4_config;
 };
 
 struct vgic_v2_cpu_if {
@@ -447,4 +449,7 @@ bool vgic_state_is_nested(struct kvm_vcpu *vcpu);
 void kvm_vgic_cpu_up(void);
 void kvm_vgic_cpu_down(void);
 
+int kvm_vm_has_gicv4(struct kvm *kvm);
+int kvm_vm_has_gicv4_1(struct kvm *kvm);
+
 #endif /* __KVM_ARM_VGIC_H */
-- 
2.49.0.1101.gccaa498523-goog

[PATCH 2/3] docs: kvm: devices/arm-vgic-v3: Document KVM_DEV_ARM_VGIC_CONFIG_GICV4 attr

[Raghavendra Rao Ananta]

Document the KVM_DEV_ARM_VGIC_CONFIG_GICV4 attr under
KVM_DEV_ARM_VGIC_GRP_CTRL, that includes the values supported to set/get
and the expected error returns.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 .../virt/kvm/devices/arm-vgic-v3.rst          | 24 +++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/Documentation/virt/kvm/devices/arm-vgic-v3.rst b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
index e860498b1e35..2eed2ac13542 100644
--- a/Documentation/virt/kvm/devices/arm-vgic-v3.rst
+++ b/Documentation/virt/kvm/devices/arm-vgic-v3.rst
@@ -240,17 +240,33 @@ Groups:
       save all LPI pending bits into guest RAM pending tables.
 
       The first kB of the pending table is not altered by this operation.
+    KVM_DEV_ARM_VGIC_CONFIG_GICV4
+      attribute to enable/disable vGICv4 for a VM. It supports three values:
+      KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE: Kernel is not booted with
+      'kvm-arm.vgic_v4_enable=1' cmdline, and hence vGICv4 is unavailable to
+      the VM. The value can only be read by the userspace, but cannot be set.
+      KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE: Kernel is booted with
+      'kvm-arm.vgic_v4_enable=1' cmdline, and vGICv4 is available and enabled
+      for the VM (default config). Userspace can get and set this value.
+      KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE: Kernel is booted with
+      'kvm-arm.vgic_v4_enable=1' cmdline, and vGICv4 is available and disabled
+      for the VM. Userspace can get and set this value.
+
 
   Errors:
 
-    =======  ========================================================
+    =======  ==================================================================
     -ENXIO   VGIC not properly configured as required prior to calling
-             this attribute
+             this attribute or trying to enable/disable vGICv4 for the VM
+             on a vGICv3 configuration in the case of
+             KVM_DEV_ARM_VGIC_CONFIG_GICV4
+    -EINVAL  Invalid configuration supplied by userspace
     -ENODEV  no online VCPU
     -ENOMEM  memory shortage when allocating vgic internal data
     -EFAULT  Invalid guest ram access
-    -EBUSY   One or more VCPUS are running
-    =======  ========================================================
+    -EBUSY   One or more VCPUS are running or vGIC has already been initialized
+             in the case of KVM_DEV_ARM_VGIC_CONFIG_GICV4
+    =======  ==================================================================
 
 
   KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO
-- 
2.49.0.1101.gccaa498523-goog

[PATCH 3/3] KVM: selftests: Extend vgic_init to test GICv4 config attr

[Raghavendra Rao Ananta]

Extend the arm64 vgic_init test to check KVM_DEV_ARM_VGIC_CONFIG_GICV4
attribute. This includes testing the interface with various
configurations when KVM has vGICv4 enabled (kvm-arm.vgic_v4_enable=1
cmdline) and disabled.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 tools/testing/selftests/kvm/arm64/vgic_init.c | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/tools/testing/selftests/kvm/arm64/vgic_init.c b/tools/testing/selftests/kvm/arm64/vgic_init.c
index b3b5fb0ff0a9..adcfaf461b2b 100644
--- a/tools/testing/selftests/kvm/arm64/vgic_init.c
+++ b/tools/testing/selftests/kvm/arm64/vgic_init.c
@@ -675,6 +675,63 @@ static void test_v3_its_region(void)
 	vm_gic_destroy(&v);
 }
 
+static void test_v3_vgicv4_config(void)
+{
+	struct kvm_vcpu *vcpus[NR_VCPUS];
+	uint8_t gicv4_config;
+	struct vm_gic v;
+	int ret;
+
+	v = vm_gic_create_with_vcpus(KVM_DEV_TYPE_ARM_VGIC_V3, NR_VCPUS, vcpus);
+	if (__kvm_has_device_attr(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+					KVM_DEV_ARM_VGIC_CONFIG_GICV4))
+		return;
+
+	kvm_device_attr_get(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+				KVM_DEV_ARM_VGIC_CONFIG_GICV4, &gicv4_config);
+
+	if (gicv4_config == KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE) {
+		gicv4_config = KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE;
+		ret = __kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+				KVM_DEV_ARM_VGIC_CONFIG_GICV4, &gicv4_config);
+		TEST_ASSERT(ret && errno == ENXIO,
+			"vGICv4 allowed to be disabled even though it's unavailable");
+
+		gicv4_config = KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE;
+		ret = __kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+				KVM_DEV_ARM_VGIC_CONFIG_GICV4, &gicv4_config);
+		TEST_ASSERT(ret && errno == ENXIO,
+			"vGICv4 allowed to be enabled even though it's unavailable");
+	} else { /* kvm-arm.vgic_v4_enable=1 */
+		TEST_ASSERT(gicv4_config == KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE,
+				"Expected vGICv4 to be enabled by default");
+
+		gicv4_config = KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE;
+		kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+				KVM_DEV_ARM_VGIC_CONFIG_GICV4, &gicv4_config);
+
+		gicv4_config = KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE;
+		kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+				KVM_DEV_ARM_VGIC_CONFIG_GICV4, &gicv4_config);
+
+		gicv4_config = KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE + 1;
+		ret = __kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+			KVM_DEV_ARM_VGIC_CONFIG_GICV4, &gicv4_config);
+		TEST_ASSERT(ret && errno == EINVAL,
+			"vGICv4 allowed to be configured with unknown value");
+
+		kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+					KVM_DEV_ARM_VGIC_CTRL_INIT, NULL);
+		gicv4_config = KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE;
+		ret = __kvm_device_attr_set(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+			KVM_DEV_ARM_VGIC_CONFIG_GICV4, &gicv4_config);
+		TEST_ASSERT(ret && errno == EBUSY,
+			"Changing vGICv4 config allowed after vGIC initialization");
+	}
+
+	vm_gic_destroy(&v);
+}
+
 /*
  * Returns 0 if it's possible to create GIC device of a given type (V2 or V3).
  */
@@ -730,6 +787,7 @@ void run_tests(uint32_t gic_dev_type)
 		test_v3_last_bit_single_rdist();
 		test_v3_redist_ipa_range_check_at_vcpu_run();
 		test_v3_its_region();
+		test_v3_vgicv4_config();
 	}
 }
 
-- 
2.49.0.1101.gccaa498523-goog

Re: [PATCH 0/3] KVM: arm64: Allow vGICv4 configuration per VM

[Ben Horgan]

Hi,

On 5/14/25 20:21, Raghavendra Rao Ananta wrote:
> Hello,
> 
> When kvm-arm.vgic_v4_enable=1, KVM adds support for direct interrupt
> injection by default to all the VMs in the system, aka GICv4. A
> shortcoming of the GIC architecture is that there's an absolute limit on
> the number of vPEs that can be tracked by the ITS. It is possible that
> an operator is running a mix of VMs on a system, only wanting to provide
> a specific class of VMs with hardware interrupt injection support.
> 
> To support this, introduce a GIC attribute, KVM_DEV_ARM_VGIC_CONFIG_GICV4,
> for the userspace to enable or disable vGICv4 for a given VM.
> 
> The attribute allows the configuration only when vGICv4 is enabled in KVM,
> else it acts a read-only attribute returning
> KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE as the value.
What's the reason for the cmdline enable continuing to be absolute in 
the disable case? I wonder if this is unnecessarily restrictive.

Couldn't KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE be reserved for 
hardware that doesn't support vgic_v4 and if kvm-arm.vgic_v4_enable=0, 
or omitted, on supporting hardware then default to 
KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE but allow it to be overridden? I 
don't think this changes the behaviour when your new attribute is not used.
> 
> On the other hand, if KVM has the vGICv4 enabled via the cmdline, the
> VM absorbs this configuration by default to maintain the backward
> compatibility. Userspace can get the attribute's value to check if the VM
> has vGICv4 support if it sees KVM_DEV_ARM_VGIC_CONFIG_GICV4_ENABLE as the
> value. As required, it can disable vGICv4 by setting
> KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE as the value.
> 
...
> 
> 

Thanks,

Ben

Re: [PATCH 0/3] KVM: arm64: Allow vGICv4 configuration per VM

[Marc Zyngier]

On Thu, 15 May 2025 11:30:33 +0100,
Ben Horgan <ben.horgan@arm.com> wrote:
> 
> Hi,
> 
> On 5/14/25 20:21, Raghavendra Rao Ananta wrote:
> > Hello,
> > 
> > When kvm-arm.vgic_v4_enable=1, KVM adds support for direct interrupt
> > injection by default to all the VMs in the system, aka GICv4. A
> > shortcoming of the GIC architecture is that there's an absolute limit on
> > the number of vPEs that can be tracked by the ITS. It is possible that
> > an operator is running a mix of VMs on a system, only wanting to provide
> > a specific class of VMs with hardware interrupt injection support.
> > 
> > To support this, introduce a GIC attribute, KVM_DEV_ARM_VGIC_CONFIG_GICV4,
> > for the userspace to enable or disable vGICv4 for a given VM.
> > 
> > The attribute allows the configuration only when vGICv4 is enabled in KVM,
> > else it acts a read-only attribute returning
> > KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE as the value.
> What's the reason for the cmdline enable continuing to be absolute in
> the disable case? I wonder if this is unnecessarily restrictive.

Because there are a number of GICv4 implementations that are
absolutely terrible out there, and that will happily lock-up under
some undisclosed circumstances.

So unless you find a good way to retire that HW, GICv4 will continue
to be a buy-in.

	M.

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

Re: [PATCH 0/3] KVM: arm64: Allow vGICv4 configuration per VM

[Raghavendra Rao Ananta]

On Thu, May 15, 2025 at 3:30 AM Ben Horgan <ben.horgan@arm.com> wrote:
>
> Hi,
>
> On 5/14/25 20:21, Raghavendra Rao Ananta wrote:
> > Hello,
> >
> > When kvm-arm.vgic_v4_enable=1, KVM adds support for direct interrupt
> > injection by default to all the VMs in the system, aka GICv4. A
> > shortcoming of the GIC architecture is that there's an absolute limit on
> > the number of vPEs that can be tracked by the ITS. It is possible that
> > an operator is running a mix of VMs on a system, only wanting to provide
> > a specific class of VMs with hardware interrupt injection support.
> >
> > To support this, introduce a GIC attribute, KVM_DEV_ARM_VGIC_CONFIG_GICV4,
> > for the userspace to enable or disable vGICv4 for a given VM.
> >
> > The attribute allows the configuration only when vGICv4 is enabled in KVM,
> > else it acts a read-only attribute returning
> > KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE as the value.
> What's the reason for the cmdline enable continuing to be absolute in
> the disable case? I wonder if this is unnecessarily restrictive.
>
> Couldn't KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE be reserved for
> hardware that doesn't support vgic_v4 and if kvm-arm.vgic_v4_enable=0,
> or omitted, on supporting hardware then default to
> KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE but allow it to be overridden? I
> don't think this changes the behaviour when your new attribute is not used.

KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE is reserved for the exact
situation that you mentioned (no GICv4 h/w support  or if cmdline is
disabled/omitted).
Regarding defaulting to KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE,
wouldn't it change the existing expectations, i.e., vGICv4 is enabled
if available and set by cmdline?

Thank you.
Raghavendra

Re: [PATCH 0/3] KVM: arm64: Allow vGICv4 configuration per VM

[Ben Horgan]

Hi,

On 5/15/25 16:55, Raghavendra Rao Ananta wrote:
> On Thu, May 15, 2025 at 3:30 AM Ben Horgan <ben.horgan@arm.com> wrote:
>>
>> Hi,
>>
>> On 5/14/25 20:21, Raghavendra Rao Ananta wrote:
>>> Hello,
>>>
>>> When kvm-arm.vgic_v4_enable=1, KVM adds support for direct interrupt
>>> injection by default to all the VMs in the system, aka GICv4. A
>>> shortcoming of the GIC architecture is that there's an absolute limit on
>>> the number of vPEs that can be tracked by the ITS. It is possible that
>>> an operator is running a mix of VMs on a system, only wanting to provide
>>> a specific class of VMs with hardware interrupt injection support.
>>>
>>> To support this, introduce a GIC attribute, KVM_DEV_ARM_VGIC_CONFIG_GICV4,
>>> for the userspace to enable or disable vGICv4 for a given VM.
>>>
>>> The attribute allows the configuration only when vGICv4 is enabled in KVM,
>>> else it acts a read-only attribute returning
>>> KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE as the value.
>> What's the reason for the cmdline enable continuing to be absolute in
>> the disable case? I wonder if this is unnecessarily restrictive.
>>
>> Couldn't KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE be reserved for
>> hardware that doesn't support vgic_v4 and if kvm-arm.vgic_v4_enable=0,
>> or omitted, on supporting hardware then default to
>> KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE but allow it to be overridden? I
>> don't think this changes the behaviour when your new attribute is not used.
> 
> KVM_DEV_ARM_VGIC_CONFIG_GICV4_UNAVAILABLE is reserved for the exact
> situation that you mentioned (no GICv4 h/w support  or if cmdline is
> disabled/omitted).
> Regarding defaulting to KVM_DEV_ARM_VGIC_CONFIG_GICV4_DISABLE,
> wouldn't it change the existing expectations, i.e., vGICv4 is enabled
> if available and set by cmdline?
I was suggesting keeping the defaults the same when your new gic 
attribute is untouched but in the same way that it overrides enable to 
disable you could also allow it to override disable to enable.

Based on Marc's comments this does not seem desirable. As things are 
now, and with your changes, setting kvm-arm.vgic_v4_enable=1 at boot 
implies a promise that vgic_v4 works on the system. As there is broken 
hardware we can't take this promise for granted.
> 
> Thank you.
> Raghavendra

Thanks,

Ben