linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] irqchip/gic-v5: Skip deactivate for forwarded PPI interrupts
  2025-06-20 16:07 [PATCH 0/5] KVM: arm64: Enable GICv3 guests on GICv5 hosts using FEAT_GCIE_LEGACY Sascha Bischoff
@ 2025-06-20 16:07 ` Sascha Bischoff
  2025-06-23 15:21   ` Lorenzo Pieralisi
  2025-06-20 16:07 ` [PATCH 2/5] irqchip/gic-v5: Populate struct gic_kvm_info Sascha Bischoff
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-20 16:07 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org
  Cc: nd, maz@kernel.org, oliver.upton@linux.dev, Joey Gouly,
	Suzuki Poulose, yuzenghui@huawei.com, will@kernel.org,
	tglx@linutronix.de, lpieralisi@kernel.org, Timothy Hayes

If a PPI interrupt is forwarded to a guest, skip the deactivate and
only EOI. Rely on the guest deactivating the both the virtual and
physical interrupts (due to ICH_LRx_EL2.HW being set) later on as part
of handling the injected interrupt. This mimics the behaviour seen on
native GICv3.

This is part of adding support for the GICv3 compatibility mode on a
GICv5 host.

Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
 drivers/irqchip/irq-gic-v5.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
index 4a0990f46358..28853d51a2ea 100644
--- a/drivers/irqchip/irq-gic-v5.c
+++ b/drivers/irqchip/irq-gic-v5.c
@@ -213,6 +213,12 @@ static void gicv5_hwirq_eoi(u32 hwirq_id, u8 hwirq_type)
 
 static void gicv5_ppi_irq_eoi(struct irq_data *d)
 {
+	/* Skip deactivate for forwarded PPI interrupts */
+	if (irqd_is_forwarded_to_vcpu(d)) {
+		gic_insn(0, CDEOI);
+		return;
+	}
+
 	gicv5_hwirq_eoi(d->hwirq, GICV5_HWIRQ_TYPE_PPI);
 }
 
@@ -494,6 +500,16 @@ static bool gicv5_ppi_irq_is_level(irq_hw_number_t hwirq)
 	return !!(read_ppi_sysreg_s(hwirq, PPI_HM) & bit);
 }
 
+static int gicv5_ppi_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
+{
+	if (vcpu)
+		irqd_set_forwarded_to_vcpu(d);
+	else
+		irqd_clr_forwarded_to_vcpu(d);
+
+	return 0;
+}
+
 static const struct irq_chip gicv5_ppi_irq_chip = {
 	.name			= "GICv5-PPI",
 	.irq_mask		= gicv5_ppi_irq_mask,
@@ -501,6 +517,7 @@ static const struct irq_chip gicv5_ppi_irq_chip = {
 	.irq_eoi		= gicv5_ppi_irq_eoi,
 	.irq_get_irqchip_state	= gicv5_ppi_irq_get_irqchip_state,
 	.irq_set_irqchip_state	= gicv5_ppi_irq_set_irqchip_state,
+	.irq_set_vcpu_affinity	= gicv5_ppi_irq_set_vcpu_affinity,
 	.flags			= IRQCHIP_SKIP_SET_WAKE	  |
 				  IRQCHIP_MASK_ON_SUSPEND,
 };
-- 
2.34.1

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

* [PATCH 0/5] KVM: arm64: Enable GICv3 guests on GICv5 hosts using FEAT_GCIE_LEGACY
@ 2025-06-20 16:07 Sascha Bischoff
  2025-06-20 16:07 ` [PATCH 1/5] irqchip/gic-v5: Skip deactivate for forwarded PPI interrupts Sascha Bischoff
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-20 16:07 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org
  Cc: nd, maz@kernel.org, oliver.upton@linux.dev, Joey Gouly,
	Suzuki Poulose, yuzenghui@huawei.com, will@kernel.org,
	tglx@linutronix.de, lpieralisi@kernel.org, Timothy Hayes

Hi all,

This series introduces support for running GICv3 guests on GICv5 hosts
by leveraging the GICv5 legacy compatibility feature
(FEAT_GCIE_LEGACY). The main motivation is to enable existing GICv3
VMs on GICv5 system without VM or VMM modifications - things should
work out of the box.

The changes are focused on two main areas:

    KVM GIC support: Enabling detection of a GICv5 host and
    configuring it to support GICv3 guests.

    IRQ chip support: Ensuring forwarded PPIs behave consistently with
    GICv3 expectations.

Summary of the patches:

    Ensure injected guest interrupts behave correctly by deferring
    deactivation to the guest, matching GICv3-native behavior.

    Set up the necessary GIC capabilities to advertise
    FEAT_GCIE_LEGACY to KVM.

    Add missing system register required for enabling GICv3 compat
    mode from EL2.

    Enable full support for running GICv3 VMs on a GICv5 host when
    compat mode is present, covering VHE, nVHE, and protected KVM
    configurations (excluding nested virt).

    Introduce a probe routine to enable GICv5 when FEAT_GCIE_LEGACY is
    detected. This consumes the gic_kvm_info populated earlier.

This support has been co-developed with T.Hayes, indicated with
Co-authored-by tags.

This series is based and dependent on [PATCH v5 00/27] Arm GICv5: Host
driver implementation [1].

Feedback welcome!

Thanks,
Sascha

[1] https://lore.kernel.org/all/20250618-gicv5-host-v5-0-d9e622ac5539@kernel.org/

Sascha Bischoff (5):
  irqchip/gic-v5: Skip deactivate for forwarded PPI interrupts
  irqchip/gic-v5: Populate struct gic_kvm_info
  arm64/sysreg: Add ICH_VCTLR_EL2
  KVM: arm64: gic-v5: Support GICv3 compat
  KVM: arm64: gic-v5: Probe for GICv5

 arch/arm64/include/asm/kvm_asm.h      |  2 +
 arch/arm64/include/asm/kvm_hyp.h      |  2 +
 arch/arm64/kvm/Makefile               |  3 +-
 arch/arm64/kvm/hyp/nvhe/hyp-main.c    | 12 +++++
 arch/arm64/kvm/hyp/vgic-v3-sr.c       | 51 +++++++++++++++++----
 arch/arm64/kvm/sys_regs.c             | 10 ++++-
 arch/arm64/kvm/vgic/vgic-init.c       |  9 +++-
 arch/arm64/kvm/vgic/vgic-v3.c         |  6 +++
 arch/arm64/kvm/vgic/vgic-v5.c         | 64 +++++++++++++++++++++++++++
 arch/arm64/kvm/vgic/vgic.h            |  4 ++
 arch/arm64/tools/sysreg               |  6 +++
 drivers/irqchip/irq-gic-v5.c          | 51 +++++++++++++++++++++
 include/kvm/arm_vgic.h                |  9 +++-
 include/linux/irqchip/arm-vgic-info.h |  4 ++
 14 files changed, 220 insertions(+), 13 deletions(-)
 create mode 100644 arch/arm64/kvm/vgic/vgic-v5.c

-- 
2.34.1

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

* [PATCH 3/5] arm64/sysreg: Add ICH_VCTLR_EL2
  2025-06-20 16:07 [PATCH 0/5] KVM: arm64: Enable GICv3 guests on GICv5 hosts using FEAT_GCIE_LEGACY Sascha Bischoff
  2025-06-20 16:07 ` [PATCH 1/5] irqchip/gic-v5: Skip deactivate for forwarded PPI interrupts Sascha Bischoff
  2025-06-20 16:07 ` [PATCH 2/5] irqchip/gic-v5: Populate struct gic_kvm_info Sascha Bischoff
@ 2025-06-20 16:07 ` Sascha Bischoff
  2025-06-20 16:07 ` [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat Sascha Bischoff
  2025-06-20 16:07 ` [PATCH 5/5] KVM: arm64: gic-v5: Probe for GICv5 Sascha Bischoff
  4 siblings, 0 replies; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-20 16:07 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org
  Cc: nd, maz@kernel.org, oliver.upton@linux.dev, Joey Gouly,
	Suzuki Poulose, yuzenghui@huawei.com, will@kernel.org,
	tglx@linutronix.de, lpieralisi@kernel.org, Timothy Hayes

This system register is required to enable/disable V3 legacy mode when
running on a GICv5 host.

Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
 arch/arm64/tools/sysreg | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index aab58bf4ed9c..dd1ae04eb033 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -4530,6 +4530,12 @@ Field	1	U
 Field	0	EOI
 EndSysreg
 
+Sysreg	ICH_VCTLR_EL2	3	4	12	11	4
+Res0	63:2
+Field	1	V3
+Field	0	En
+EndSysreg
+
 Sysreg	CONTEXTIDR_EL2	3	4	13	0	1
 Fields	CONTEXTIDR_ELx
 EndSysreg
-- 
2.34.1

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

* [PATCH 2/5] irqchip/gic-v5: Populate struct gic_kvm_info
  2025-06-20 16:07 [PATCH 0/5] KVM: arm64: Enable GICv3 guests on GICv5 hosts using FEAT_GCIE_LEGACY Sascha Bischoff
  2025-06-20 16:07 ` [PATCH 1/5] irqchip/gic-v5: Skip deactivate for forwarded PPI interrupts Sascha Bischoff
@ 2025-06-20 16:07 ` Sascha Bischoff
  2025-06-23 15:14   ` Lorenzo Pieralisi
  2025-06-20 16:07 ` [PATCH 3/5] arm64/sysreg: Add ICH_VCTLR_EL2 Sascha Bischoff
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-20 16:07 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org
  Cc: nd, maz@kernel.org, oliver.upton@linux.dev, Joey Gouly,
	Suzuki Poulose, yuzenghui@huawei.com, will@kernel.org,
	tglx@linutronix.de, lpieralisi@kernel.org, Timothy Hayes

Populate the gic_kvm_info struct based on support for
FEAT_GCIE_LEGACY.  The struct is used by KVM to probe for a compatible
GIC.

Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
 drivers/irqchip/irq-gic-v5.c          | 34 +++++++++++++++++++++++++++
 include/linux/irqchip/arm-vgic-info.h |  4 ++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
index 28853d51a2ea..6aecd2343fee 100644
--- a/drivers/irqchip/irq-gic-v5.c
+++ b/drivers/irqchip/irq-gic-v5.c
@@ -13,6 +13,7 @@
 
 #include <linux/irqchip.h>
 #include <linux/irqchip/arm-gic-v5.h>
+#include <linux/irqchip/arm-vgic-info.h>
 
 #include <asm/cpufeature.h>
 #include <asm/exception.h>
@@ -1049,6 +1050,37 @@ static void gicv5_set_cpuif_idbits(void)
 	}
 }
 
+#ifdef CONFIG_KVM
+static struct gic_kvm_info gic_v5_kvm_info __initdata;
+
+static bool gicv5_cpuif_has_gcie_legacy(void)
+{
+	u64 idr0 = read_sysreg_s(SYS_ICC_IDR0_EL1);
+
+	return !!FIELD_GET(ICC_IDR0_EL1_GCIE_LEGACY, idr0);
+}
+
+static void __init gic_of_setup_kvm_info(struct device_node *node)
+{
+	gic_v5_kvm_info.type = GIC_V5;
+	gic_v5_kvm_info.has_gcie_v3_compat = gicv5_cpuif_has_gcie_legacy();
+
+	/* GIC Virtual CPU interface maintenance interrupt */
+	gic_v5_kvm_info.no_maint_irq_mask = false;
+	gic_v5_kvm_info.maint_irq = irq_of_parse_and_map(node, 0);
+	if (!gic_v5_kvm_info.maint_irq) {
+		pr_warn("cannot find GICv5 virtual CPU interface maintenance interrupt\n");
+		return;
+	}
+
+	vgic_set_kvm_info(&gic_v5_kvm_info);
+}
+#else
+static void __init gic_of_setup_kvm_info(struct device_node *node)
+{
+}
+#endif // CONFIG_KVM
+
 static int __init gicv5_of_init(struct device_node *node, struct device_node *parent)
 {
 	int ret = gicv5_irs_of_probe(node);
@@ -1081,6 +1113,8 @@ static int __init gicv5_of_init(struct device_node *node, struct device_node *pa
 
 	gicv5_irs_its_probe();
 
+	gic_of_setup_kvm_info(node);
+
 	return 0;
 out_int:
 	gicv5_cpu_disable_interrupts();
diff --git a/include/linux/irqchip/arm-vgic-info.h b/include/linux/irqchip/arm-vgic-info.h
index a75b2c7de69d..ca1713fac6e3 100644
--- a/include/linux/irqchip/arm-vgic-info.h
+++ b/include/linux/irqchip/arm-vgic-info.h
@@ -15,6 +15,8 @@ enum gic_type {
 	GIC_V2,
 	/* Full GICv3, optionally with v2 compat */
 	GIC_V3,
+	/* Full GICv5, optionally with v3 compat */
+	GIC_V5,
 };
 
 struct gic_kvm_info {
@@ -34,6 +36,8 @@ struct gic_kvm_info {
 	bool		has_v4_1;
 	/* Deactivation impared, subpar stuff */
 	bool		no_hw_deactivation;
+	/* v3 compat support (GICv5 hosts, only) */
+	bool		has_gcie_v3_compat;
 };
 
 #ifdef CONFIG_KVM
-- 
2.34.1

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

* [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat
  2025-06-20 16:07 [PATCH 0/5] KVM: arm64: Enable GICv3 guests on GICv5 hosts using FEAT_GCIE_LEGACY Sascha Bischoff
                   ` (2 preceding siblings ...)
  2025-06-20 16:07 ` [PATCH 3/5] arm64/sysreg: Add ICH_VCTLR_EL2 Sascha Bischoff
@ 2025-06-20 16:07 ` Sascha Bischoff
  2025-06-20 20:20   ` Oliver Upton
  2025-06-20 16:07 ` [PATCH 5/5] KVM: arm64: gic-v5: Probe for GICv5 Sascha Bischoff
  4 siblings, 1 reply; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-20 16:07 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org
  Cc: nd, maz@kernel.org, oliver.upton@linux.dev, Joey Gouly,
	Suzuki Poulose, yuzenghui@huawei.com, will@kernel.org,
	tglx@linutronix.de, lpieralisi@kernel.org, Timothy Hayes

Add support for GICv3 compat mode (FEAT_GCIE_LEGACY) which allows a
GICv5 host to run GICv3-based VMs. This change enables the
VHE/nVHE/hVHE/protected modes, but does not support nested
virtualization.

Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
 arch/arm64/include/asm/kvm_asm.h   |  2 ++
 arch/arm64/include/asm/kvm_hyp.h   |  2 ++
 arch/arm64/kvm/Makefile            |  3 +-
 arch/arm64/kvm/hyp/nvhe/hyp-main.c | 12 +++++++
 arch/arm64/kvm/hyp/vgic-v3-sr.c    | 51 +++++++++++++++++++++++++-----
 arch/arm64/kvm/sys_regs.c          | 10 +++++-
 arch/arm64/kvm/vgic/vgic-init.c    |  6 ++--
 arch/arm64/kvm/vgic/vgic-v3.c      |  6 ++++
 arch/arm64/kvm/vgic/vgic-v5.c      | 14 ++++++++
 arch/arm64/kvm/vgic/vgic.h         |  2 ++
 include/kvm/arm_vgic.h             |  9 +++++-
 11 files changed, 104 insertions(+), 13 deletions(-)
 create mode 100644 arch/arm64/kvm/vgic/vgic-v5.c

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index bec227f9500a..ad1ef0460fd6 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -81,6 +81,8 @@ enum __kvm_host_smccc_func {
 	__KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff,
 	__KVM_HOST_SMCCC_FUNC___vgic_v3_save_vmcr_aprs,
 	__KVM_HOST_SMCCC_FUNC___vgic_v3_restore_vmcr_aprs,
+	__KVM_HOST_SMCCC_FUNC___vgic_v3_compat_mode_enable,
+	__KVM_HOST_SMCCC_FUNC___vgic_v3_compat_mode_disable,
 	__KVM_HOST_SMCCC_FUNC___pkvm_init_vm,
 	__KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu,
 	__KVM_HOST_SMCCC_FUNC___pkvm_teardown_vm,
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index e6be1f5d0967..9c8adc5186ec 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -85,6 +85,8 @@ void __vgic_v3_deactivate_traps(struct vgic_v3_cpu_if *cpu_if);
 void __vgic_v3_save_vmcr_aprs(struct vgic_v3_cpu_if *cpu_if);
 void __vgic_v3_restore_vmcr_aprs(struct vgic_v3_cpu_if *cpu_if);
 int __vgic_v3_perform_cpuif_access(struct kvm_vcpu *vcpu);
+void __vgic_v3_compat_mode_enable(void);
+void __vgic_v3_compat_mode_disable(void);
 
 #ifdef __KVM_NVHE_HYPERVISOR__
 void __timer_enable_traps(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 7c329e01c557..3ebc0570345c 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -23,7 +23,8 @@ kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \
 	 vgic/vgic-v3.o vgic/vgic-v4.o \
 	 vgic/vgic-mmio.o vgic/vgic-mmio-v2.o \
 	 vgic/vgic-mmio-v3.o vgic/vgic-kvm-device.o \
-	 vgic/vgic-its.o vgic/vgic-debug.o vgic/vgic-v3-nested.o
+	 vgic/vgic-its.o vgic/vgic-debug.o vgic/vgic-v3-nested.o \
+	 vgic/vgic-v5.o
 
 kvm-$(CONFIG_HW_PERF_EVENTS)  += pmu-emul.o pmu.o
 kvm-$(CONFIG_ARM64_PTR_AUTH)  += pauth.o
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index e9198e56e784..61af55df60a9 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -475,6 +475,16 @@ static void handle___vgic_v3_restore_vmcr_aprs(struct kvm_cpu_context *host_ctxt
 	__vgic_v3_restore_vmcr_aprs(kern_hyp_va(cpu_if));
 }
 
+static void handle___vgic_v3_compat_mode_enable(struct kvm_cpu_context *host_ctxt)
+{
+	__vgic_v3_compat_mode_enable();
+}
+
+static void handle___vgic_v3_compat_mode_disable(struct kvm_cpu_context *host_ctxt)
+{
+	__vgic_v3_compat_mode_disable();
+}
+
 static void handle___pkvm_init(struct kvm_cpu_context *host_ctxt)
 {
 	DECLARE_REG(phys_addr_t, phys, host_ctxt, 1);
@@ -603,6 +613,8 @@ static const hcall_t host_hcall[] = {
 	HANDLE_FUNC(__kvm_timer_set_cntvoff),
 	HANDLE_FUNC(__vgic_v3_save_vmcr_aprs),
 	HANDLE_FUNC(__vgic_v3_restore_vmcr_aprs),
+	HANDLE_FUNC(__vgic_v3_compat_mode_enable),
+	HANDLE_FUNC(__vgic_v3_compat_mode_disable),
 	HANDLE_FUNC(__pkvm_init_vm),
 	HANDLE_FUNC(__pkvm_init_vcpu),
 	HANDLE_FUNC(__pkvm_teardown_vm),
diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index f162b0df5cae..b03b5f012226 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -257,6 +257,18 @@ void __vgic_v3_restore_state(struct vgic_v3_cpu_if *cpu_if)
 	}
 }
 
+void __vgic_v3_compat_mode_enable(void)
+{
+	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, 0, ICH_VCTLR_EL2_V3);
+	isb();
+}
+
+void __vgic_v3_compat_mode_disable(void)
+{
+	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, ICH_VCTLR_EL2_V3, 0);
+	isb();
+}
+
 void __vgic_v3_activate_traps(struct vgic_v3_cpu_if *cpu_if)
 {
 	/*
@@ -296,12 +308,19 @@ void __vgic_v3_activate_traps(struct vgic_v3_cpu_if *cpu_if)
 	}
 
 	/*
-	 * Prevent the guest from touching the ICC_SRE_EL1 system
-	 * register. Note that this may not have any effect, as
-	 * ICC_SRE_EL2.Enable being RAO/WI is a valid implementation.
+	 * GICv5 BET0 FEAT_GCIE_LEGACY doesn't include ICC_SRE_EL2. This is due
+	 * to be relaxed in a future spec release, likely BET1, at which point
+	 * this in condition can be dropped again.
 	 */
-	write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE,
-		     ICC_SRE_EL2);
+	if (!static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif)) {
+		/*
+		 * Prevent the guest from touching the ICC_SRE_EL1 system
+		 * register. Note that this may not have any effect, as
+		 * ICC_SRE_EL2.Enable being RAO/WI is a valid implementation.
+		 */
+		write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE,
+			     ICC_SRE_EL2);
+	}
 
 	/*
 	 * If we need to trap system registers, we must write
@@ -322,8 +341,14 @@ void __vgic_v3_deactivate_traps(struct vgic_v3_cpu_if *cpu_if)
 		cpu_if->vgic_vmcr = read_gicreg(ICH_VMCR_EL2);
 	}
 
-	val = read_gicreg(ICC_SRE_EL2);
-	write_gicreg(val | ICC_SRE_EL2_ENABLE, ICC_SRE_EL2);
+	/*
+	 * Can be dropped in the future when GICv5 BET1 is released. See
+	 * comment above.
+	 */
+	if (!static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif)) {
+		val = read_gicreg(ICC_SRE_EL2);
+		write_gicreg(val | ICC_SRE_EL2_ENABLE, ICC_SRE_EL2);
+	}
 
 	if (!cpu_if->vgic_sre) {
 		/* Make sure ENABLE is set at EL2 before setting SRE at EL1 */
@@ -423,9 +448,19 @@ void __vgic_v3_init_lrs(void)
  */
 u64 __vgic_v3_get_gic_config(void)
 {
-	u64 val, sre = read_gicreg(ICC_SRE_EL1);
+	u64 val, sre;
 	unsigned long flags = 0;
 
+	/*
+	 * In compat mode, we cannot access ICC_SRE_EL1 at any EL
+	 * other than EL1 itself; just return the
+	 * ICH_VTR_EL2. ICC_IDR0_EL1 is only implemented on a GICv5
+	 * system, so we first check if we have GICv5 support.
+	 */
+	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif))
+		return read_gicreg(ICH_VTR_EL2);
+
+	sre = read_gicreg(ICC_SRE_EL1);
 	/*
 	 * To check whether we have a MMIO-based (GICv2 compatible)
 	 * CPU interface, we need to disable the system register
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 76c2f0da821f..de8297ccb31c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1811,7 +1811,7 @@ static u64 sanitise_id_aa64pfr0_el1(const struct kvm_vcpu *vcpu, u64 val)
 		val |= SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, CSV3, IMP);
 	}
 
-	if (kvm_vgic_global_state.type == VGIC_V3) {
+	if (kvm_vgic_global_state.type == VGIC_V3 || kvm_vgic_in_v3_compat_mode()) {
 		val &= ~ID_AA64PFR0_EL1_GIC_MASK;
 		val |= SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, GIC, IMP);
 	}
@@ -1953,6 +1953,14 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
 	    (vcpu_has_nv(vcpu) && !FIELD_GET(ID_AA64PFR0_EL1_EL2, user_val)))
 		return -EINVAL;
 
+	/*
+	 * If we are running on a GICv5 host and support FEAT_GCIE_LEGACY, then
+	 * we support GICv3. Fail attempts to do anything but set that to IMP.
+	 */
+	if (kvm_vgic_in_v3_compat_mode() &&
+	    FIELD_GET(ID_AA64PFR0_EL1_GIC_MASK, user_val) != ID_AA64PFR0_EL1_GIC_IMP)
+		return -EINVAL;
+
 	return set_id_reg(vcpu, rd, user_val);
 }
 
diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index eb1205654ac8..5f6506e297c1 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -674,10 +674,12 @@ void kvm_vgic_init_cpu_hardware(void)
 	 * We want to make sure the list registers start out clear so that we
 	 * only have the program the used registers.
 	 */
-	if (kvm_vgic_global_state.type == VGIC_V2)
+	if (kvm_vgic_global_state.type == VGIC_V2) {
 		vgic_v2_init_lrs();
-	else
+	} else if (kvm_vgic_global_state.type == VGIC_V3 ||
+		   kvm_vgic_in_v3_compat_mode()) {
 		kvm_call_hyp(__vgic_v3_init_lrs);
+	}
 }
 
 /**
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index b9ad7c42c5b0..b5df4d36821d 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -734,6 +734,9 @@ void vgic_v3_load(struct kvm_vcpu *vcpu)
 {
 	struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3;
 
+	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif))
+		kvm_call_hyp(__vgic_v3_compat_mode_enable);
+
 	/* If the vgic is nested, perform the full state loading */
 	if (vgic_state_is_nested(vcpu)) {
 		vgic_v3_load_nested(vcpu);
@@ -764,4 +767,7 @@ void vgic_v3_put(struct kvm_vcpu *vcpu)
 
 	if (has_vhe())
 		__vgic_v3_deactivate_traps(cpu_if);
+
+	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif))
+		kvm_call_hyp(__vgic_v3_compat_mode_disable);
 }
diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
new file mode 100644
index 000000000000..57199449ca0f
--- /dev/null
+++ b/arch/arm64/kvm/vgic/vgic-v5.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <kvm/arm_vgic.h>
+
+#include "vgic.h"
+
+inline bool kvm_vgic_in_v3_compat_mode(void)
+{
+	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif) &&
+	    kvm_vgic_global_state.has_gcie_v3_compat)
+		return true;
+
+	return false;
+}
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index 4349084cb9a6..5c78eb915a22 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -389,6 +389,8 @@ void vgic_v3_put_nested(struct kvm_vcpu *vcpu);
 void vgic_v3_handle_nested_maint_irq(struct kvm_vcpu *vcpu);
 void vgic_v3_nested_update_mi(struct kvm_vcpu *vcpu);
 
+inline bool kvm_vgic_in_v3_compat_mode(void);
+
 int vgic_its_debug_init(struct kvm_device *dev);
 void vgic_its_debug_destroy(struct kvm_device *dev);
 
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 4a34f7f0a864..6e16e303d80f 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -38,6 +38,7 @@
 enum vgic_type {
 	VGIC_V2,		/* Good ol' GICv2 */
 	VGIC_V3,		/* New fancy GICv3 */
+	VGIC_V5,		/* Newer, fancier GICv5 */
 };
 
 /* same for all guests, as depending only on the _host's_ GIC model */
@@ -77,9 +78,15 @@ struct vgic_global {
 	/* Pseudo GICv3 from outer space */
 	bool			no_hw_deactivation;
 
-	/* GIC system register CPU interface */
+	/* GICv3 system register CPU interface */
 	struct static_key_false gicv3_cpuif;
 
+	/* GICv5 host system */
+	struct static_key_false gicv5_cpuif;
+
+	/* GICv3 compat mode on a GICv5 host */
+	bool			has_gcie_v3_compat;
+
 	u32			ich_vtr_el2;
 };
 
-- 
2.34.1

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

* [PATCH 5/5] KVM: arm64: gic-v5: Probe for GICv5
  2025-06-20 16:07 [PATCH 0/5] KVM: arm64: Enable GICv3 guests on GICv5 hosts using FEAT_GCIE_LEGACY Sascha Bischoff
                   ` (3 preceding siblings ...)
  2025-06-20 16:07 ` [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat Sascha Bischoff
@ 2025-06-20 16:07 ` Sascha Bischoff
  2025-06-20 20:25   ` Oliver Upton
  4 siblings, 1 reply; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-20 16:07 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org
  Cc: nd, maz@kernel.org, oliver.upton@linux.dev, Joey Gouly,
	Suzuki Poulose, yuzenghui@huawei.com, will@kernel.org,
	tglx@linutronix.de, lpieralisi@kernel.org, Timothy Hayes

Add in a probe function for GICv5 which enables support for GICv3
guests on a GICv5 host, if FEAT_GCIE_LEGACY is support.

Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
---
 arch/arm64/kvm/vgic/vgic-init.c |  3 ++
 arch/arm64/kvm/vgic/vgic-v5.c   | 50 +++++++++++++++++++++++++++++++++
 arch/arm64/kvm/vgic/vgic.h      |  2 ++
 3 files changed, 55 insertions(+)

diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 5f6506e297c1..2ec3da04a607 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -724,6 +724,9 @@ int kvm_vgic_hyp_init(void)
 			kvm_info("GIC system register CPU interface enabled\n");
 		}
 		break;
+	case GIC_V5:
+		ret = vgic_v5_probe(gic_kvm_info);
+		break;
 	default:
 		ret = -ENODEV;
 	}
diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
index 57199449ca0f..5d0cfcbbefa7 100644
--- a/arch/arm64/kvm/vgic/vgic-v5.c
+++ b/arch/arm64/kvm/vgic/vgic-v5.c
@@ -1,9 +1,59 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
 #include <kvm/arm_vgic.h>
+#include <linux/irqchip/arm-vgic-info.h>
 
 #include "vgic.h"
 
+/**
+ * vgic_v5_probe - probe for a VGICv5 compatible interrupt controller
+ * @info:	pointer to the GIC description
+ *
+ * Returns 0 if the VGICv5 has been probed successfully, returns an error code
+ * otherwise.
+ */
+int vgic_v5_probe(const struct gic_kvm_info *info)
+{
+	u64 ich_vtr_el2;
+	int ret;
+
+	if (!info->has_gcie_v3_compat)
+		return -ENODEV;
+
+	kvm_vgic_global_state.type = VGIC_V5;
+	kvm_vgic_global_state.has_gcie_v3_compat = true;
+	static_branch_enable(&kvm_vgic_global_state.gicv5_cpuif);
+
+	/* We only support v3 compat mode - use vGICv3 limits */
+	kvm_vgic_global_state.max_gic_vcpus = VGIC_V3_MAX_CPUS;
+
+	kvm_vgic_global_state.vcpu_base = 0;
+	kvm_vgic_global_state.vctrl_base = NULL;
+	kvm_vgic_global_state.can_emulate_gicv2 = false;
+	kvm_vgic_global_state.has_gicv4 = false;
+	kvm_vgic_global_state.has_gicv4_1 = false;
+
+	ich_vtr_el2 =  kvm_call_hyp_ret(__vgic_v3_get_gic_config);
+	kvm_vgic_global_state.ich_vtr_el2 = (u32)ich_vtr_el2;
+
+	/*
+	 * The ListRegs field is 5 bits, but there is an architectural
+	 * maximum of 16 list registers. Just ignore bit 4...
+	 */
+	kvm_vgic_global_state.nr_lr = (ich_vtr_el2 & 0xf) + 1;
+
+	ret = kvm_register_vgic_device(KVM_DEV_TYPE_ARM_VGIC_V3);
+	if (ret) {
+		kvm_err("Cannot register GICv3-legacy KVM device.\n");
+		return ret;
+	}
+
+	static_branch_enable(&kvm_vgic_global_state.gicv3_cpuif);
+	kvm_info("GCIE legacy system register CPU interface\n");
+
+	return 0;
+}
+
 inline bool kvm_vgic_in_v3_compat_mode(void)
 {
 	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif) &&
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index 5c78eb915a22..a5292cad60ff 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -308,6 +308,8 @@ int vgic_init(struct kvm *kvm);
 void vgic_debug_init(struct kvm *kvm);
 void vgic_debug_destroy(struct kvm *kvm);
 
+int vgic_v5_probe(const struct gic_kvm_info *info);
+
 static inline int vgic_v3_max_apr_idx(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *cpu_if = &vcpu->arch.vgic_cpu;
-- 
2.34.1

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

* Re: [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat
  2025-06-20 16:07 ` [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat Sascha Bischoff
@ 2025-06-20 20:20   ` Oliver Upton
  2025-06-20 23:02     ` Oliver Upton
  2025-06-22 12:19     ` Marc Zyngier
  0 siblings, 2 replies; 18+ messages in thread
From: Oliver Upton @ 2025-06-20 20:20 UTC (permalink / raw)
  To: Sascha Bischoff
  Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org, nd,
	maz@kernel.org, Joey Gouly, Suzuki Poulose, yuzenghui@huawei.com,
	will@kernel.org, tglx@linutronix.de, lpieralisi@kernel.org,
	Timothy Hayes

Hi Sascha,

Thank you for posting this. Very excited to see the GICv5 enablement get
started.

On Fri, Jun 20, 2025 at 04:07:51PM +0000, Sascha Bischoff wrote:
> Add support for GICv3 compat mode (FEAT_GCIE_LEGACY) which allows a
> GICv5 host to run GICv3-based VMs. This change enables the
> VHE/nVHE/hVHE/protected modes, but does not support nested
> virtualization.

Can't we just load the shadow state into the compat VGICv3? I'm worried
this has sharp edges on the UAPI side as well as users wanting to
migrate VMs to new hardware.

The guest hypervisor should only see GICv3-only or GICv5-only, we can
pretend FEAT_GCIE_LEGACY never existed :)

> Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
> Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
>  arch/arm64/include/asm/kvm_asm.h   |  2 ++
>  arch/arm64/include/asm/kvm_hyp.h   |  2 ++
>  arch/arm64/kvm/Makefile            |  3 +-
>  arch/arm64/kvm/hyp/nvhe/hyp-main.c | 12 +++++++
>  arch/arm64/kvm/hyp/vgic-v3-sr.c    | 51 +++++++++++++++++++++++++-----
>  arch/arm64/kvm/sys_regs.c          | 10 +++++-
>  arch/arm64/kvm/vgic/vgic-init.c    |  6 ++--
>  arch/arm64/kvm/vgic/vgic-v3.c      |  6 ++++
>  arch/arm64/kvm/vgic/vgic-v5.c      | 14 ++++++++
>  arch/arm64/kvm/vgic/vgic.h         |  2 ++
>  include/kvm/arm_vgic.h             |  9 +++++-
>  11 files changed, 104 insertions(+), 13 deletions(-)
>  create mode 100644 arch/arm64/kvm/vgic/vgic-v5.c
> 
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index bec227f9500a..ad1ef0460fd6 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -81,6 +81,8 @@ enum __kvm_host_smccc_func {
>  	__KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff,
>  	__KVM_HOST_SMCCC_FUNC___vgic_v3_save_vmcr_aprs,
>  	__KVM_HOST_SMCCC_FUNC___vgic_v3_restore_vmcr_aprs,
> +	__KVM_HOST_SMCCC_FUNC___vgic_v3_compat_mode_enable,
> +	__KVM_HOST_SMCCC_FUNC___vgic_v3_compat_mode_disable,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_init_vm,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu,
>  	__KVM_HOST_SMCCC_FUNC___pkvm_teardown_vm,
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index e6be1f5d0967..9c8adc5186ec 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -85,6 +85,8 @@ void __vgic_v3_deactivate_traps(struct vgic_v3_cpu_if *cpu_if);
>  void __vgic_v3_save_vmcr_aprs(struct vgic_v3_cpu_if *cpu_if);
>  void __vgic_v3_restore_vmcr_aprs(struct vgic_v3_cpu_if *cpu_if);
>  int __vgic_v3_perform_cpuif_access(struct kvm_vcpu *vcpu);
> +void __vgic_v3_compat_mode_enable(void);
> +void __vgic_v3_compat_mode_disable(void);
>  
>  #ifdef __KVM_NVHE_HYPERVISOR__
>  void __timer_enable_traps(struct kvm_vcpu *vcpu);
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 7c329e01c557..3ebc0570345c 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -23,7 +23,8 @@ kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \
>  	 vgic/vgic-v3.o vgic/vgic-v4.o \
>  	 vgic/vgic-mmio.o vgic/vgic-mmio-v2.o \
>  	 vgic/vgic-mmio-v3.o vgic/vgic-kvm-device.o \
> -	 vgic/vgic-its.o vgic/vgic-debug.o vgic/vgic-v3-nested.o
> +	 vgic/vgic-its.o vgic/vgic-debug.o vgic/vgic-v3-nested.o \
> +	 vgic/vgic-v5.o
>  
>  kvm-$(CONFIG_HW_PERF_EVENTS)  += pmu-emul.o pmu.o
>  kvm-$(CONFIG_ARM64_PTR_AUTH)  += pauth.o
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index e9198e56e784..61af55df60a9 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -475,6 +475,16 @@ static void handle___vgic_v3_restore_vmcr_aprs(struct kvm_cpu_context *host_ctxt
>  	__vgic_v3_restore_vmcr_aprs(kern_hyp_va(cpu_if));
>  }
>  
> +static void handle___vgic_v3_compat_mode_enable(struct kvm_cpu_context *host_ctxt)
> +{
> +	__vgic_v3_compat_mode_enable();
> +}
> +
> +static void handle___vgic_v3_compat_mode_disable(struct kvm_cpu_context *host_ctxt)
> +{
> +	__vgic_v3_compat_mode_disable();
> +}
> +
>  static void handle___pkvm_init(struct kvm_cpu_context *host_ctxt)
>  {
>  	DECLARE_REG(phys_addr_t, phys, host_ctxt, 1);
> @@ -603,6 +613,8 @@ static const hcall_t host_hcall[] = {
>  	HANDLE_FUNC(__kvm_timer_set_cntvoff),
>  	HANDLE_FUNC(__vgic_v3_save_vmcr_aprs),
>  	HANDLE_FUNC(__vgic_v3_restore_vmcr_aprs),
> +	HANDLE_FUNC(__vgic_v3_compat_mode_enable),
> +	HANDLE_FUNC(__vgic_v3_compat_mode_disable),
>  	HANDLE_FUNC(__pkvm_init_vm),
>  	HANDLE_FUNC(__pkvm_init_vcpu),
>  	HANDLE_FUNC(__pkvm_teardown_vm),
> diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> index f162b0df5cae..b03b5f012226 100644
> --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> @@ -257,6 +257,18 @@ void __vgic_v3_restore_state(struct vgic_v3_cpu_if *cpu_if)
>  	}
>  }
>  
> +void __vgic_v3_compat_mode_enable(void)
> +{
> +	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, 0, ICH_VCTLR_EL2_V3);
> +	isb();
> +}
> +
> +void __vgic_v3_compat_mode_disable(void)
> +{
> +	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, ICH_VCTLR_EL2_V3, 0);
> +	isb();
> +}
> +

It isn't clear to me what these ISBs are synchonizing against. AFAICT,
the whole compat thing is always visible and we can restore the rest of
the VGICv3 context before guaranteeing the enable bit has been observed.

Can we consolidate this into a single hyp call along with
__vgic_v3_*_vmcr_aprs()?

Last bit as an FYI, kvm_call_hyp() has an implied context synchronization upon
return, either because of ERET in nVHE or an explicit ISB on VHE.

>  void __vgic_v3_activate_traps(struct vgic_v3_cpu_if *cpu_if)
>  {
>  	/*
> @@ -296,12 +308,19 @@ void __vgic_v3_activate_traps(struct vgic_v3_cpu_if *cpu_if)
>  	}
>  
>  	/*
> -	 * Prevent the guest from touching the ICC_SRE_EL1 system
> -	 * register. Note that this may not have any effect, as
> -	 * ICC_SRE_EL2.Enable being RAO/WI is a valid implementation.
> +	 * GICv5 BET0 FEAT_GCIE_LEGACY doesn't include ICC_SRE_EL2. This is due
> +	 * to be relaxed in a future spec release, likely BET1, at which point
> +	 * this in condition can be dropped again.
>  	 */
> -	write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE,
> -		     ICC_SRE_EL2);
> +	if (!static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif)) {
> +		/*
> +		 * Prevent the guest from touching the ICC_SRE_EL1 system
> +		 * register. Note that this may not have any effect, as
> +		 * ICC_SRE_EL2.Enable being RAO/WI is a valid implementation.
> +		 */
> +		write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE,
> +			     ICC_SRE_EL2);
> +	}
>  
>  	/*
>  	 * If we need to trap system registers, we must write
> @@ -322,8 +341,14 @@ void __vgic_v3_deactivate_traps(struct vgic_v3_cpu_if *cpu_if)
>  		cpu_if->vgic_vmcr = read_gicreg(ICH_VMCR_EL2);
>  	}
>  
> -	val = read_gicreg(ICC_SRE_EL2);
> -	write_gicreg(val | ICC_SRE_EL2_ENABLE, ICC_SRE_EL2);
> +	/*
> +	 * Can be dropped in the future when GICv5 BET1 is released. See
> +	 * comment above.
> +	 */
> +	if (!static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif)) {

Can we use the GCIE cpucap instead, possibly via a shared helper with
the driver?

> -	if (kvm_vgic_global_state.type == VGIC_V3) {
> +	if (kvm_vgic_global_state.type == VGIC_V3 || kvm_vgic_in_v3_compat_mode()) {

Can we do a helper for this too?

>  		val &= ~ID_AA64PFR0_EL1_GIC_MASK;
>  		val |= SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, GIC, IMP);
>  	}
> @@ -1953,6 +1953,14 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
>  	    (vcpu_has_nv(vcpu) && !FIELD_GET(ID_AA64PFR0_EL1_EL2, user_val)))
>  		return -EINVAL;
>  
> +	/*
> +	 * If we are running on a GICv5 host and support FEAT_GCIE_LEGACY, then
> +	 * we support GICv3. Fail attempts to do anything but set that to IMP.
> +	 */
> +	if (kvm_vgic_in_v3_compat_mode() &&
> +	    FIELD_GET(ID_AA64PFR0_EL1_GIC_MASK, user_val) != ID_AA64PFR0_EL1_GIC_IMP)
> +		return -EINVAL;
> +



>  	return set_id_reg(vcpu, rd, user_val);
>  }
>  
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index eb1205654ac8..5f6506e297c1 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -674,10 +674,12 @@ void kvm_vgic_init_cpu_hardware(void)
>  	 * We want to make sure the list registers start out clear so that we
>  	 * only have the program the used registers.
>  	 */
> -	if (kvm_vgic_global_state.type == VGIC_V2)
> +	if (kvm_vgic_global_state.type == VGIC_V2) {
>  		vgic_v2_init_lrs();
> -	else
> +	} else if (kvm_vgic_global_state.type == VGIC_V3 ||
> +		   kvm_vgic_in_v3_compat_mode()) {
>  		kvm_call_hyp(__vgic_v3_init_lrs);
> +	}
>  }
>  
>  /**
> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> index b9ad7c42c5b0..b5df4d36821d 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> @@ -734,6 +734,9 @@ void vgic_v3_load(struct kvm_vcpu *vcpu)
>  {
>  	struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3;
>  
> +	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif))
> +		kvm_call_hyp(__vgic_v3_compat_mode_enable);
> +
>  	/* If the vgic is nested, perform the full state loading */
>  	if (vgic_state_is_nested(vcpu)) {
>  		vgic_v3_load_nested(vcpu);
> @@ -764,4 +767,7 @@ void vgic_v3_put(struct kvm_vcpu *vcpu)
>  
>  	if (has_vhe())
>  		__vgic_v3_deactivate_traps(cpu_if);
> +
> +	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif))
> +		kvm_call_hyp(__vgic_v3_compat_mode_disable);
>  }
> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> new file mode 100644
> index 000000000000..57199449ca0f
> --- /dev/null
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <kvm/arm_vgic.h>
> +
> +#include "vgic.h"
> +
> +inline bool kvm_vgic_in_v3_compat_mode(void)a

nit: we're generally trusting of the compiler to 'do the right thing'
and avoid explicit inline specifiers unless necessary.

> +{
> +	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif) &&
> +	    kvm_vgic_global_state.has_gcie_v3_compat)
> +		return true;
> +
> +	return false;
> +}

This should be a per-VM thing once KVM support for GICv5 lands. Can you
get ahead of that and take a KVM pointer that goes unused. Maybe rename
it:

bool vgic_is_v3_compat(struct kvm *kvm)

Or something similar.

Thanks,
Oliver

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

* Re: [PATCH 5/5] KVM: arm64: gic-v5: Probe for GICv5
  2025-06-20 16:07 ` [PATCH 5/5] KVM: arm64: gic-v5: Probe for GICv5 Sascha Bischoff
@ 2025-06-20 20:25   ` Oliver Upton
  2025-06-23 13:12     ` Sascha Bischoff
  0 siblings, 1 reply; 18+ messages in thread
From: Oliver Upton @ 2025-06-20 20:25 UTC (permalink / raw)
  To: Sascha Bischoff
  Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org, nd,
	maz@kernel.org, Joey Gouly, Suzuki Poulose, yuzenghui@huawei.com,
	will@kernel.org, tglx@linutronix.de, lpieralisi@kernel.org,
	Timothy Hayes

On Fri, Jun 20, 2025 at 04:07:52PM +0000, Sascha Bischoff wrote:
> +/**
> + * vgic_v5_probe - probe for a VGICv5 compatible interrupt controller
> + * @info:	pointer to the GIC description
> + *
> + * Returns 0 if the VGICv5 has been probed successfully, returns an error code
> + * otherwise.
> + */

nit: avoid kerneldoc style

This actually generates documentation as well as build warnings when we
screw up the format. I'd only do this sort of thing for sufficiently
public functions.

Thanks,
Oliver

> +int vgic_v5_probe(const struct gic_kvm_info *info)
> +{
> +	u64 ich_vtr_el2;
> +	int ret;
> +
> +	if (!info->has_gcie_v3_compat)
> +		return -ENODEV;
> +
> +	kvm_vgic_global_state.type = VGIC_V5;
> +	kvm_vgic_global_state.has_gcie_v3_compat = true;
> +	static_branch_enable(&kvm_vgic_global_state.gicv5_cpuif);
> +
> +	/* We only support v3 compat mode - use vGICv3 limits */
> +	kvm_vgic_global_state.max_gic_vcpus = VGIC_V3_MAX_CPUS;
> +
> +	kvm_vgic_global_state.vcpu_base = 0;
> +	kvm_vgic_global_state.vctrl_base = NULL;
> +	kvm_vgic_global_state.can_emulate_gicv2 = false;
> +	kvm_vgic_global_state.has_gicv4 = false;
> +	kvm_vgic_global_state.has_gicv4_1 = false;
> +
> +	ich_vtr_el2 =  kvm_call_hyp_ret(__vgic_v3_get_gic_config);
> +	kvm_vgic_global_state.ich_vtr_el2 = (u32)ich_vtr_el2;
> +
> +	/*
> +	 * The ListRegs field is 5 bits, but there is an architectural
> +	 * maximum of 16 list registers. Just ignore bit 4...
> +	 */
> +	kvm_vgic_global_state.nr_lr = (ich_vtr_el2 & 0xf) + 1;
> +
> +	ret = kvm_register_vgic_device(KVM_DEV_TYPE_ARM_VGIC_V3);
> +	if (ret) {
> +		kvm_err("Cannot register GICv3-legacy KVM device.\n");
> +		return ret;
> +	}
> +
> +	static_branch_enable(&kvm_vgic_global_state.gicv3_cpuif);
> +	kvm_info("GCIE legacy system register CPU interface\n");
> +
> +	return 0;
> +}
> +
>  inline bool kvm_vgic_in_v3_compat_mode(void)
>  {
>  	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif) &&
> diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
> index 5c78eb915a22..a5292cad60ff 100644
> --- a/arch/arm64/kvm/vgic/vgic.h
> +++ b/arch/arm64/kvm/vgic/vgic.h
> @@ -308,6 +308,8 @@ int vgic_init(struct kvm *kvm);
>  void vgic_debug_init(struct kvm *kvm);
>  void vgic_debug_destroy(struct kvm *kvm);
>  
> +int vgic_v5_probe(const struct gic_kvm_info *info);
> +
>  static inline int vgic_v3_max_apr_idx(struct kvm_vcpu *vcpu)
>  {
>  	struct vgic_cpu *cpu_if = &vcpu->arch.vgic_cpu;
> -- 
> 2.34.1

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

* Re: [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat
  2025-06-20 20:20   ` Oliver Upton
@ 2025-06-20 23:02     ` Oliver Upton
  2025-06-23 13:11       ` Sascha Bischoff
  2025-06-22 12:19     ` Marc Zyngier
  1 sibling, 1 reply; 18+ messages in thread
From: Oliver Upton @ 2025-06-20 23:02 UTC (permalink / raw)
  To: Sascha Bischoff
  Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org, nd,
	maz@kernel.org, Joey Gouly, Suzuki Poulose, yuzenghui@huawei.com,
	will@kernel.org, tglx@linutronix.de, lpieralisi@kernel.org,
	Timothy Hayes

On Fri, Jun 20, 2025 at 01:20:36PM -0700, Oliver Upton wrote:
> Hi Sascha,
> 
> Thank you for posting this. Very excited to see the GICv5 enablement get
> started.
> 
> On Fri, Jun 20, 2025 at 04:07:51PM +0000, Sascha Bischoff wrote:
> > Add support for GICv3 compat mode (FEAT_GCIE_LEGACY) which allows a
> > GICv5 host to run GICv3-based VMs. This change enables the
> > VHE/nVHE/hVHE/protected modes, but does not support nested
> > virtualization.
> 
> Can't we just load the shadow state into the compat VGICv3? I'm worried
> this has sharp edges on the UAPI side as well as users wanting to
> migrate VMs to new hardware.
> 
> The guest hypervisor should only see GICv3-only or GICv5-only, we can
> pretend FEAT_GCIE_LEGACY never existed :)
> 
> > Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > ---
> >  arch/arm64/include/asm/kvm_asm.h   |  2 ++
> >  arch/arm64/include/asm/kvm_hyp.h   |  2 ++
> >  arch/arm64/kvm/Makefile            |  3 +-
> >  arch/arm64/kvm/hyp/nvhe/hyp-main.c | 12 +++++++
> >  arch/arm64/kvm/hyp/vgic-v3-sr.c    | 51 +++++++++++++++++++++++++-----
> >  arch/arm64/kvm/sys_regs.c          | 10 +++++-
> >  arch/arm64/kvm/vgic/vgic-init.c    |  6 ++--
> >  arch/arm64/kvm/vgic/vgic-v3.c      |  6 ++++
> >  arch/arm64/kvm/vgic/vgic-v5.c      | 14 ++++++++
> >  arch/arm64/kvm/vgic/vgic.h         |  2 ++
> >  include/kvm/arm_vgic.h             |  9 +++++-
> >  11 files changed, 104 insertions(+), 13 deletions(-)
> >  create mode 100644 arch/arm64/kvm/vgic/vgic-v5.c
> > 
> > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> > index bec227f9500a..ad1ef0460fd6 100644
> > --- a/arch/arm64/include/asm/kvm_asm.h
> > +++ b/arch/arm64/include/asm/kvm_asm.h
> > @@ -81,6 +81,8 @@ enum __kvm_host_smccc_func {
> >  	__KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff,
> >  	__KVM_HOST_SMCCC_FUNC___vgic_v3_save_vmcr_aprs,
> >  	__KVM_HOST_SMCCC_FUNC___vgic_v3_restore_vmcr_aprs,
> > +	__KVM_HOST_SMCCC_FUNC___vgic_v3_compat_mode_enable,
> > +	__KVM_HOST_SMCCC_FUNC___vgic_v3_compat_mode_disable,
> >  	__KVM_HOST_SMCCC_FUNC___pkvm_init_vm,
> >  	__KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu,
> >  	__KVM_HOST_SMCCC_FUNC___pkvm_teardown_vm,
> > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> > index e6be1f5d0967..9c8adc5186ec 100644
> > --- a/arch/arm64/include/asm/kvm_hyp.h
> > +++ b/arch/arm64/include/asm/kvm_hyp.h
> > @@ -85,6 +85,8 @@ void __vgic_v3_deactivate_traps(struct vgic_v3_cpu_if *cpu_if);
> >  void __vgic_v3_save_vmcr_aprs(struct vgic_v3_cpu_if *cpu_if);
> >  void __vgic_v3_restore_vmcr_aprs(struct vgic_v3_cpu_if *cpu_if);
> >  int __vgic_v3_perform_cpuif_access(struct kvm_vcpu *vcpu);
> > +void __vgic_v3_compat_mode_enable(void);
> > +void __vgic_v3_compat_mode_disable(void);
> >  
> >  #ifdef __KVM_NVHE_HYPERVISOR__
> >  void __timer_enable_traps(struct kvm_vcpu *vcpu);
> > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> > index 7c329e01c557..3ebc0570345c 100644
> > --- a/arch/arm64/kvm/Makefile
> > +++ b/arch/arm64/kvm/Makefile
> > @@ -23,7 +23,8 @@ kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \
> >  	 vgic/vgic-v3.o vgic/vgic-v4.o \
> >  	 vgic/vgic-mmio.o vgic/vgic-mmio-v2.o \
> >  	 vgic/vgic-mmio-v3.o vgic/vgic-kvm-device.o \
> > -	 vgic/vgic-its.o vgic/vgic-debug.o vgic/vgic-v3-nested.o
> > +	 vgic/vgic-its.o vgic/vgic-debug.o vgic/vgic-v3-nested.o \
> > +	 vgic/vgic-v5.o
> >  
> >  kvm-$(CONFIG_HW_PERF_EVENTS)  += pmu-emul.o pmu.o
> >  kvm-$(CONFIG_ARM64_PTR_AUTH)  += pauth.o
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index e9198e56e784..61af55df60a9 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > @@ -475,6 +475,16 @@ static void handle___vgic_v3_restore_vmcr_aprs(struct kvm_cpu_context *host_ctxt
> >  	__vgic_v3_restore_vmcr_aprs(kern_hyp_va(cpu_if));
> >  }
> >  
> > +static void handle___vgic_v3_compat_mode_enable(struct kvm_cpu_context *host_ctxt)
> > +{
> > +	__vgic_v3_compat_mode_enable();
> > +}
> > +
> > +static void handle___vgic_v3_compat_mode_disable(struct kvm_cpu_context *host_ctxt)
> > +{
> > +	__vgic_v3_compat_mode_disable();
> > +}
> > +
> >  static void handle___pkvm_init(struct kvm_cpu_context *host_ctxt)
> >  {
> >  	DECLARE_REG(phys_addr_t, phys, host_ctxt, 1);
> > @@ -603,6 +613,8 @@ static const hcall_t host_hcall[] = {
> >  	HANDLE_FUNC(__kvm_timer_set_cntvoff),
> >  	HANDLE_FUNC(__vgic_v3_save_vmcr_aprs),
> >  	HANDLE_FUNC(__vgic_v3_restore_vmcr_aprs),
> > +	HANDLE_FUNC(__vgic_v3_compat_mode_enable),
> > +	HANDLE_FUNC(__vgic_v3_compat_mode_disable),
> >  	HANDLE_FUNC(__pkvm_init_vm),
> >  	HANDLE_FUNC(__pkvm_init_vcpu),
> >  	HANDLE_FUNC(__pkvm_teardown_vm),
> > diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> > index f162b0df5cae..b03b5f012226 100644
> > --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> > +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> > @@ -257,6 +257,18 @@ void __vgic_v3_restore_state(struct vgic_v3_cpu_if *cpu_if)
> >  	}
> >  }
> >  
> > +void __vgic_v3_compat_mode_enable(void)
> > +{
> > +	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, 0, ICH_VCTLR_EL2_V3);
> > +	isb();
> > +}
> > +
> > +void __vgic_v3_compat_mode_disable(void)
> > +{
> > +	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, ICH_VCTLR_EL2_V3, 0);
> > +	isb();
> > +}
> > +
> 
> It isn't clear to me what these ISBs are synchonizing against. AFAICT,
> the whole compat thing is always visible and we can restore the rest of
> the VGICv3 context before guaranteeing the enable bit has been observed.
> 
> Can we consolidate this into a single hyp call along with
> __vgic_v3_*_vmcr_aprs()?
> 
> Last bit as an FYI, kvm_call_hyp() has an implied context synchronization upon
> return, either because of ERET in nVHE or an explicit ISB on VHE.

Ah, reading the spec was a useful exercise. ICH_VMCR_EL2 is a modal
register... I hear implementation folks *love* those :)

Please do the aforementioned consolidation, at which point the purpose
of the ISB should be apparent.

> >  void __vgic_v3_activate_traps(struct vgic_v3_cpu_if *cpu_if)
> >  {
> >  	/*
> > @@ -296,12 +308,19 @@ void __vgic_v3_activate_traps(struct vgic_v3_cpu_if *cpu_if)
> >  	}
> >  
> >  	/*
> > -	 * Prevent the guest from touching the ICC_SRE_EL1 system
> > -	 * register. Note that this may not have any effect, as
> > -	 * ICC_SRE_EL2.Enable being RAO/WI is a valid implementation.
> > +	 * GICv5 BET0 FEAT_GCIE_LEGACY doesn't include ICC_SRE_EL2. This is due
> > +	 * to be relaxed in a future spec release, likely BET1, at which point
> > +	 * this in condition can be dropped again.
> >  	 */
> > -	write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE,
> > -		     ICC_SRE_EL2);
> > +	if (!static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif)) {
> > +		/*
> > +		 * Prevent the guest from touching the ICC_SRE_EL1 system
> > +		 * register. Note that this may not have any effect, as
> > +		 * ICC_SRE_EL2.Enable being RAO/WI is a valid implementation.
> > +		 */
> > +		write_gicreg(read_gicreg(ICC_SRE_EL2) & ~ICC_SRE_EL2_ENABLE,
> > +			     ICC_SRE_EL2);
> > +	}
> >  
> >  	/*
> >  	 * If we need to trap system registers, we must write
> > @@ -322,8 +341,14 @@ void __vgic_v3_deactivate_traps(struct vgic_v3_cpu_if *cpu_if)
> >  		cpu_if->vgic_vmcr = read_gicreg(ICH_VMCR_EL2);
> >  	}
> >  
> > -	val = read_gicreg(ICC_SRE_EL2);
> > -	write_gicreg(val | ICC_SRE_EL2_ENABLE, ICC_SRE_EL2);
> > +	/*
> > +	 * Can be dropped in the future when GICv5 BET1 is released. See
> > +	 * comment above.
> > +	 */
> > +	if (!static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif)) {
> 
> Can we use the GCIE cpucap instead, possibly via a shared helper with
> the driver?
> 
> > -	if (kvm_vgic_global_state.type == VGIC_V3) {
> > +	if (kvm_vgic_global_state.type == VGIC_V3 || kvm_vgic_in_v3_compat_mode()) {
> 
> Can we do a helper for this too?
> 
> >  		val &= ~ID_AA64PFR0_EL1_GIC_MASK;
> >  		val |= SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, GIC, IMP);
> >  	}
> > @@ -1953,6 +1953,14 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> >  	    (vcpu_has_nv(vcpu) && !FIELD_GET(ID_AA64PFR0_EL1_EL2, user_val)))
> >  		return -EINVAL;
> >  
> > +	/*
> > +	 * If we are running on a GICv5 host and support FEAT_GCIE_LEGACY, then
> > +	 * we support GICv3. Fail attempts to do anything but set that to IMP.
> > +	 */
> > +	if (kvm_vgic_in_v3_compat_mode() &&
> > +	    FIELD_GET(ID_AA64PFR0_EL1_GIC_MASK, user_val) != ID_AA64PFR0_EL1_GIC_IMP)
> > +		return -EINVAL;
> > +
> 
> 
> 
> >  	return set_id_reg(vcpu, rd, user_val);
> >  }
> >  
> > diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> > index eb1205654ac8..5f6506e297c1 100644
> > --- a/arch/arm64/kvm/vgic/vgic-init.c
> > +++ b/arch/arm64/kvm/vgic/vgic-init.c
> > @@ -674,10 +674,12 @@ void kvm_vgic_init_cpu_hardware(void)
> >  	 * We want to make sure the list registers start out clear so that we
> >  	 * only have the program the used registers.
> >  	 */
> > -	if (kvm_vgic_global_state.type == VGIC_V2)
> > +	if (kvm_vgic_global_state.type == VGIC_V2) {
> >  		vgic_v2_init_lrs();
> > -	else
> > +	} else if (kvm_vgic_global_state.type == VGIC_V3 ||
> > +		   kvm_vgic_in_v3_compat_mode()) {
> >  		kvm_call_hyp(__vgic_v3_init_lrs);
> > +	}
> >  }
> >  
> >  /**
> > diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> > index b9ad7c42c5b0..b5df4d36821d 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> > @@ -734,6 +734,9 @@ void vgic_v3_load(struct kvm_vcpu *vcpu)
> >  {
> >  	struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3;
> >  
> > +	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif))
> > +		kvm_call_hyp(__vgic_v3_compat_mode_enable);
> > +
> >  	/* If the vgic is nested, perform the full state loading */
> >  	if (vgic_state_is_nested(vcpu)) {
> >  		vgic_v3_load_nested(vcpu);
> > @@ -764,4 +767,7 @@ void vgic_v3_put(struct kvm_vcpu *vcpu)
> >  
> >  	if (has_vhe())
> >  		__vgic_v3_deactivate_traps(cpu_if);
> > +
> > +	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif))
> > +		kvm_call_hyp(__vgic_v3_compat_mode_disable);

Do we need to eagerly disable compat mode or can we just configure the
VGIC correctly for the intended vCPU at load()?

> >  }
> > diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> > new file mode 100644
> > index 000000000000..57199449ca0f
> > --- /dev/null
> > +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <kvm/arm_vgic.h>
> > +
> > +#include "vgic.h"
> > +
> > +inline bool kvm_vgic_in_v3_compat_mode(void)a
> 
> nit: we're generally trusting of the compiler to 'do the right thing'
> and avoid explicit inline specifiers unless necessary.
> 
> > +{
> > +	if (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif) &&
> > +	    kvm_vgic_global_state.has_gcie_v3_compat)
> > +		return true;
> > +
> > +	return false;
> > +}
> 
> This should be a per-VM thing once KVM support for GICv5 lands. Can you
> get ahead of that and take a KVM pointer that goes unused. Maybe rename
> it:
> 
> bool vgic_is_v3_compat(struct kvm *kvm)
> 
> Or something similar.
> 
> Thanks,
> Oliver
> 

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

* Re: [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat
  2025-06-20 20:20   ` Oliver Upton
  2025-06-20 23:02     ` Oliver Upton
@ 2025-06-22 12:19     ` Marc Zyngier
  2025-06-22 12:37       ` Oliver Upton
  1 sibling, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2025-06-22 12:19 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Sascha Bischoff, linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, nd, Joey Gouly, Suzuki Poulose,
	yuzenghui@huawei.com, will@kernel.org, tglx@linutronix.de,
	lpieralisi@kernel.org, Timothy Hayes

On Fri, 20 Jun 2025 21:20:36 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Hi Sascha,
> 
> Thank you for posting this. Very excited to see the GICv5 enablement get
> started.
> 
> On Fri, Jun 20, 2025 at 04:07:51PM +0000, Sascha Bischoff wrote:
> > Add support for GICv3 compat mode (FEAT_GCIE_LEGACY) which allows a
> > GICv5 host to run GICv3-based VMs. This change enables the
> > VHE/nVHE/hVHE/protected modes, but does not support nested
> > virtualization.
> 
> Can't we just load the shadow state into the compat VGICv3? I'm worried
> this has sharp edges on the UAPI side as well as users wanting to
> migrate VMs to new hardware.
>
> The guest hypervisor should only see GICv3-only or GICv5-only, we can
> pretend FEAT_GCIE_LEGACY never existed :)

That's exactly what this does. And the only reason NV isn't supported
yet is the current BET0 spec makes ICC_SRE_EL2 UNDEF at EL1 with NV,
which breaks NV in a spectacular way.

This will be addressed in a future revision of the architecture, and
no HW will actually be built with this defect. As such, there is no
UAPI to break.

> 
> > Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > ---
> >  arch/arm64/include/asm/kvm_asm.h   |  2 ++
> >  arch/arm64/include/asm/kvm_hyp.h   |  2 ++
> >  arch/arm64/kvm/Makefile            |  3 +-
> >  arch/arm64/kvm/hyp/nvhe/hyp-main.c | 12 +++++++
> >  arch/arm64/kvm/hyp/vgic-v3-sr.c    | 51 +++++++++++++++++++++++++-----
> >  arch/arm64/kvm/sys_regs.c          | 10 +++++-
> >  arch/arm64/kvm/vgic/vgic-init.c    |  6 ++--
> >  arch/arm64/kvm/vgic/vgic-v3.c      |  6 ++++
> >  arch/arm64/kvm/vgic/vgic-v5.c      | 14 ++++++++
> >  arch/arm64/kvm/vgic/vgic.h         |  2 ++
> >  include/kvm/arm_vgic.h             |  9 +++++-
> >  11 files changed, 104 insertions(+), 13 deletions(-)
> >  create mode 100644 arch/arm64/kvm/vgic/vgic-v5.c
> > 
> > diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> > index bec227f9500a..ad1ef0460fd6 100644
> > --- a/arch/arm64/include/asm/kvm_asm.h
> > +++ b/arch/arm64/include/asm/kvm_asm.h
> > @@ -81,6 +81,8 @@ enum __kvm_host_smccc_func {
> >  	__KVM_HOST_SMCCC_FUNC___kvm_timer_set_cntvoff,
> >  	__KVM_HOST_SMCCC_FUNC___vgic_v3_save_vmcr_aprs,
> >  	__KVM_HOST_SMCCC_FUNC___vgic_v3_restore_vmcr_aprs,
> > +	__KVM_HOST_SMCCC_FUNC___vgic_v3_compat_mode_enable,
> > +	__KVM_HOST_SMCCC_FUNC___vgic_v3_compat_mode_disable,
> >  	__KVM_HOST_SMCCC_FUNC___pkvm_init_vm,
> >  	__KVM_HOST_SMCCC_FUNC___pkvm_init_vcpu,
> >  	__KVM_HOST_SMCCC_FUNC___pkvm_teardown_vm,
> > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> > index e6be1f5d0967..9c8adc5186ec 100644
> > --- a/arch/arm64/include/asm/kvm_hyp.h
> > +++ b/arch/arm64/include/asm/kvm_hyp.h
> > @@ -85,6 +85,8 @@ void __vgic_v3_deactivate_traps(struct vgic_v3_cpu_if *cpu_if);
> >  void __vgic_v3_save_vmcr_aprs(struct vgic_v3_cpu_if *cpu_if);
> >  void __vgic_v3_restore_vmcr_aprs(struct vgic_v3_cpu_if *cpu_if);
> >  int __vgic_v3_perform_cpuif_access(struct kvm_vcpu *vcpu);
> > +void __vgic_v3_compat_mode_enable(void);
> > +void __vgic_v3_compat_mode_disable(void);
> >  
> >  #ifdef __KVM_NVHE_HYPERVISOR__
> >  void __timer_enable_traps(struct kvm_vcpu *vcpu);
> > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> > index 7c329e01c557..3ebc0570345c 100644
> > --- a/arch/arm64/kvm/Makefile
> > +++ b/arch/arm64/kvm/Makefile
> > @@ -23,7 +23,8 @@ kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \
> >  	 vgic/vgic-v3.o vgic/vgic-v4.o \
> >  	 vgic/vgic-mmio.o vgic/vgic-mmio-v2.o \
> >  	 vgic/vgic-mmio-v3.o vgic/vgic-kvm-device.o \
> > -	 vgic/vgic-its.o vgic/vgic-debug.o vgic/vgic-v3-nested.o
> > +	 vgic/vgic-its.o vgic/vgic-debug.o vgic/vgic-v3-nested.o \
> > +	 vgic/vgic-v5.o
> >  
> >  kvm-$(CONFIG_HW_PERF_EVENTS)  += pmu-emul.o pmu.o
> >  kvm-$(CONFIG_ARM64_PTR_AUTH)  += pauth.o
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index e9198e56e784..61af55df60a9 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > @@ -475,6 +475,16 @@ static void handle___vgic_v3_restore_vmcr_aprs(struct kvm_cpu_context *host_ctxt
> >  	__vgic_v3_restore_vmcr_aprs(kern_hyp_va(cpu_if));
> >  }
> >  
> > +static void handle___vgic_v3_compat_mode_enable(struct kvm_cpu_context *host_ctxt)
> > +{
> > +	__vgic_v3_compat_mode_enable();
> > +}
> > +
> > +static void handle___vgic_v3_compat_mode_disable(struct kvm_cpu_context *host_ctxt)
> > +{
> > +	__vgic_v3_compat_mode_disable();
> > +}
> > +
> >  static void handle___pkvm_init(struct kvm_cpu_context *host_ctxt)
> >  {
> >  	DECLARE_REG(phys_addr_t, phys, host_ctxt, 1);
> > @@ -603,6 +613,8 @@ static const hcall_t host_hcall[] = {
> >  	HANDLE_FUNC(__kvm_timer_set_cntvoff),
> >  	HANDLE_FUNC(__vgic_v3_save_vmcr_aprs),
> >  	HANDLE_FUNC(__vgic_v3_restore_vmcr_aprs),
> > +	HANDLE_FUNC(__vgic_v3_compat_mode_enable),
> > +	HANDLE_FUNC(__vgic_v3_compat_mode_disable),
> >  	HANDLE_FUNC(__pkvm_init_vm),
> >  	HANDLE_FUNC(__pkvm_init_vcpu),
> >  	HANDLE_FUNC(__pkvm_teardown_vm),
> > diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> > index f162b0df5cae..b03b5f012226 100644
> > --- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
> > +++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
> > @@ -257,6 +257,18 @@ void __vgic_v3_restore_state(struct vgic_v3_cpu_if *cpu_if)
> >  	}
> >  }
> >  
> > +void __vgic_v3_compat_mode_enable(void)
> > +{
> > +	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, 0, ICH_VCTLR_EL2_V3);
> > +	isb();
> > +}
> > +
> > +void __vgic_v3_compat_mode_disable(void)
> > +{
> > +	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, ICH_VCTLR_EL2_V3, 0);
> > +	isb();
> > +}
> > +
> 
> It isn't clear to me what these ISBs are synchonizing against. AFAICT,
> the whole compat thing is always visible and we can restore the rest of
> the VGICv3 context before guaranteeing the enable bit has been observed.

No, some registers have a behaviour that is dependent on the status of
the V3 bit (ICH_VMCR_EL2 being one), so that synchronisation is
absolutely needed before accessing this register.

The disabling is probably the wrong way around though, and I'd expect
the clearing of V3 to have an ISB *before* the write to the sysreg,

> Can we consolidate this into a single hyp call along with
> __vgic_v3_*_vmcr_aprs()?

I agree that we should be able to move this to be driven by
load/put entirely.

But we first need to fix the whole WFI sequencing first, because this
is a bit of a train wreck at the moment (entering the WFI emulation
results in *two* "put" sequences on the vgic, and exiting WFI results
in two loads).

Thanks,

	M.

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

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

* Re: [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat
  2025-06-22 12:19     ` Marc Zyngier
@ 2025-06-22 12:37       ` Oliver Upton
  2025-06-23 13:02         ` Sascha Bischoff
  0 siblings, 1 reply; 18+ messages in thread
From: Oliver Upton @ 2025-06-22 12:37 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Sascha Bischoff, linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, nd, Joey Gouly, Suzuki Poulose,
	yuzenghui@huawei.com, will@kernel.org, tglx@linutronix.de,
	lpieralisi@kernel.org, Timothy Hayes

On Sun, Jun 22, 2025 at 01:19:13PM +0100, Marc Zyngier wrote:
> On Fri, 20 Jun 2025 21:20:36 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > Hi Sascha,
> > 
> > Thank you for posting this. Very excited to see the GICv5 enablement get
> > started.
> > 
> > On Fri, Jun 20, 2025 at 04:07:51PM +0000, Sascha Bischoff wrote:
> > > Add support for GICv3 compat mode (FEAT_GCIE_LEGACY) which allows a
> > > GICv5 host to run GICv3-based VMs. This change enables the
> > > VHE/nVHE/hVHE/protected modes, but does not support nested
> > > virtualization.
> > 
> > Can't we just load the shadow state into the compat VGICv3? I'm worried
> > this has sharp edges on the UAPI side as well as users wanting to
> > migrate VMs to new hardware.
> >
> > The guest hypervisor should only see GICv3-only or GICv5-only, we can
> > pretend FEAT_GCIE_LEGACY never existed :)
> 
> That's exactly what this does. And the only reason NV isn't supported
> yet is the current BET0 spec makes ICC_SRE_EL2 UNDEF at EL1 with NV,
> which breaks NV in a spectacular way.

Gee, I wonder how... :)

> This will be addressed in a future revision of the architecture, and
> no HW will actually be built with this defect. As such, there is no
> UAPI to break.

That's fine by me. TBH, when I left this comment I hadn't fully read the
patch yet and was more curious about the intent.

> > > +void __vgic_v3_compat_mode_disable(void)
> > > +{
> > > +	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, ICH_VCTLR_EL2_V3, 0);
> > > +	isb();
> > > +}
> > > +
> > 
> > It isn't clear to me what these ISBs are synchonizing against. AFAICT,
> > the whole compat thing is always visible and we can restore the rest of
> > the VGICv3 context before guaranteeing the enable bit has been observed.
> 
> No, some registers have a behaviour that is dependent on the status of
> the V3 bit (ICH_VMCR_EL2 being one), so that synchronisation is
> absolutely needed before accessing this register.

Yeah, I had followed up on this after reading the spec, modal registers
are great. Putting all the constituent registers together in the common
load/put helpers will clear that up.

> The disabling is probably the wrong way around though, and I'd expect
> the clearing of V3 to have an ISB *before* the write to the sysreg,
> 
> > Can we consolidate this into a single hyp call along with
> > __vgic_v3_*_vmcr_aprs()?
> 
> I agree that we should be able to move this to be driven by
> load/put entirely.
> 
> But we first need to fix the whole WFI sequencing first, because this
> is a bit of a train wreck at the moment (entering the WFI emulation
> results in *two* "put" sequences on the vgic, and exiting WFI results
> in two loads).

You're talking about the case where halt polling fails and we do a
put/load on the whole vCPU to schedule right? i.e. in addition to the
explicit put on the vgic for faithful emulation.

Thanks,
Oliver

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

* Re: [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat
  2025-06-22 12:37       ` Oliver Upton
@ 2025-06-23 13:02         ` Sascha Bischoff
  0 siblings, 0 replies; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-23 13:02 UTC (permalink / raw)
  To: maz@kernel.org, oliver.upton@linux.dev
  Cc: yuzenghui@huawei.com, Timothy Hayes, nd, Suzuki Poulose,
	lpieralisi@kernel.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	Joey Gouly, will@kernel.org, tglx@linutronix.de

On Sun, 2025-06-22 at 05:37 -0700, Oliver Upton wrote:
> On Sun, Jun 22, 2025 at 01:19:13PM +0100, Marc Zyngier wrote:
> > On Fri, 20 Jun 2025 21:20:36 +0100,
> > Oliver Upton <oliver.upton@linux.dev> wrote:
> > > 
> > > Hi Sascha,
> > > 
> > > Thank you for posting this. Very excited to see the GICv5
> > > enablement get
> > > started.

Hi Oliver. Me too. Thanks for all of your comments!

> > > 
> > > On Fri, Jun 20, 2025 at 04:07:51PM +0000, Sascha Bischoff wrote:
> > > > Add support for GICv3 compat mode (FEAT_GCIE_LEGACY) which
> > > > allows a
> > > > GICv5 host to run GICv3-based VMs. This change enables the
> > > > VHE/nVHE/hVHE/protected modes, but does not support nested
> > > > virtualization.
> > > 
> > > Can't we just load the shadow state into the compat VGICv3? I'm
> > > worried
> > > this has sharp edges on the UAPI side as well as users wanting to
> > > migrate VMs to new hardware.
> > > 
> > > The guest hypervisor should only see GICv3-only or GICv5-only, we
> > > can
> > > pretend FEAT_GCIE_LEGACY never existed :)
> > 
> > That's exactly what this does. And the only reason NV isn't
> > supported
> > yet is the current BET0 spec makes ICC_SRE_EL2 UNDEF at EL1 with
> > NV,
> > which breaks NV in a spectacular way.
> 
> Gee, I wonder how... :)
> 
> > This will be addressed in a future revision of the architecture,
> > and
> > no HW will actually be built with this defect. As such, there is no
> > UAPI to break.
> 
> That's fine by me. TBH, when I left this comment I hadn't fully read
> the
> patch yet and was more curious about the intent.

As Marc said, this can't work right now. Once that's been relaxed it
should largely be a case of dropping the two compat mode checks in
__vgic_v3_activate_traps & __vgic_v3_deactivate_traps, which are
present to make sure we don't access ICC_SRE_EL2 and hit an UNDEF.

And yes, the idea is to expose a pure GICv3 or GICv5 system, and not
tell the guest hyp about FEAT_GCIE_LEGACY. :)

> 
> > > > +void __vgic_v3_compat_mode_disable(void)
> > > > +{
> > > > +	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2,
> > > > ICH_VCTLR_EL2_V3, 0);
> > > > +	isb();
> > > > +}
> > > > +
> > > 
> > > It isn't clear to me what these ISBs are synchonizing against.
> > > AFAICT,
> > > the whole compat thing is always visible and we can restore the
> > > rest of
> > > the VGICv3 context before guaranteeing the enable bit has been
> > > observed.
> > 
> > No, some registers have a behaviour that is dependent on the status
> > of
> > the V3 bit (ICH_VMCR_EL2 being one), so that synchronisation is
> > absolutely needed before accessing this register.
> 
> Yeah, I had followed up on this after reading the spec, modal
> registers
> are great. Putting all the constituent registers together in the
> common
> load/put helpers will clear that up.
> 
> > The disabling is probably the wrong way around though, and I'd
> > expect
> > the clearing of V3 to have an ISB *before* the write to the sysreg,

Ah, that's a good spot. I'll check these and rework accordingly, taking
into account the explicit syncs from the ERET & ISBs already present.

> > 
> > > Can we consolidate this into a single hyp call along with
> > > __vgic_v3_*_vmcr_aprs()?
> > 
> > I agree that we should be able to move this to be driven by
> > load/put entirely.
> > 
> > But we first need to fix the whole WFI sequencing first, because
> > this
> > is a bit of a train wreck at the moment (entering the WFI emulation
> > results in *two* "put" sequences on the vgic, and exiting WFI
> > results
> > in two loads).
> 
> You're talking about the case where halt polling fails and we do a
> put/load on the whole vCPU to schedule right? i.e. in addition to the
> explicit put on the vgic for faithful emulation.

That's the one. When it comes to compat mode, it causes issues due to
sampling the ICH_VMCR_EL2 twice - once with the compat mode enabled
giving the correct VMCR representation, and a second time with compat
mode disabled clobbering the saved state.

Which brings me to...

> Do we need to eagerly disable compat mode or can we just configure
> the VGIC correctly for the intended vCPU at load()?

This isn't something I'd considered, actually. I think this could work,
and would coincidentally work around the issues caused by the double
vgic put.

My thinking was that whenever we're executing on the host, we put the
system back to a "clean" state where the default guest GIC matches the
host, and thereby we'd need to do nothing with compat mode in the
vgic_v5_load/put path when it is added. If we move to toggling compat
mode in the load path only, we need to enable compat in the v3 load and
disable in the v5 load.

Are we happy with this?


Thanks,
Sascha

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

* Re: [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat
  2025-06-20 23:02     ` Oliver Upton
@ 2025-06-23 13:11       ` Sascha Bischoff
  0 siblings, 0 replies; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-23 13:11 UTC (permalink / raw)
  To: oliver.upton@linux.dev
  Cc: yuzenghui@huawei.com, Timothy Hayes, Suzuki Poulose, nd,
	lpieralisi@kernel.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	Joey Gouly, maz@kernel.org, tglx@linutronix.de, will@kernel.org

On Fri, 2025-06-20 at 16:02 -0700, Oliver Upton wrote:
> On Fri, Jun 20, 2025 at 01:20:36PM -0700, Oliver Upton wrote:
> > > +void __vgic_v3_compat_mode_enable(void)
> > > +{
> > > +	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, 0,
> > > ICH_VCTLR_EL2_V3);
> > > +	isb();
> > > +}
> > > +
> > > +void __vgic_v3_compat_mode_disable(void)
> > > +{
> > > +	sysreg_clear_set_s(SYS_ICH_VCTLR_EL2, ICH_VCTLR_EL2_V3,
> > > 0);
> > > +	isb();
> > > +}
> > > +
> > 
> > It isn't clear to me what these ISBs are synchonizing against.
> > AFAICT,
> > the whole compat thing is always visible and we can restore the
> > rest of
> > the VGICv3 context before guaranteeing the enable bit has been
> > observed.
> > 
> > Can we consolidate this into a single hyp call along with
> > __vgic_v3_*_vmcr_aprs()?
> > 
> > Last bit as an FYI, kvm_call_hyp() has an implied context
> > synchronization upon
> > return, either because of ERET in nVHE or an explicit ISB on VHE.
> 
> Ah, reading the spec was a useful exercise. ICH_VMCR_EL2 is a modal
> register... I hear implementation folks *love* those :)
> 
> Please do the aforementioned consolidation, at which point the
> purpose
> of the ISB should be apparent.

If we are happy to move to only touching the ICH_VCTLR_EL2.V3 compat
mode control in the v3/v5 load path, I think this change can happen
now. However, we otherwise need to clean up the WFI load/put path first
to avoid the double calls.

> > > +	if
> > > (!static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif)) {
> > 
> > Can we use the GCIE cpucap instead, possibly via a shared helper
> > with
> > the driver?

I'll look into it.

> > > -	if (kvm_vgic_global_state.type == VGIC_V3) {
> > > +	if (kvm_vgic_global_state.type == VGIC_V3 ||
> > > kvm_vgic_in_v3_compat_mode()) {
> > 
> > Can we do a helper for this too?

Will do.

> > 
> > >  		val &= ~ID_AA64PFR0_EL1_GIC_MASK;
> > >  		val |= SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, GIC,
> > > IMP);
> > >  	}
> > > 

> > > +	if
> > > (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif))
> > > +		kvm_call_hyp(__vgic_v3_compat_mode_disable);
> 
> Do we need to eagerly disable compat mode or can we just configure
> the
> VGIC correctly for the intended vCPU at load()?

I've responded to this in the other thread (I think we could do it
purely on load, and drop eager disable).

> 
> > >  }
> > > diff --git a/arch/arm64/kvm/vgic/vgic-v5.c
> > > b/arch/arm64/kvm/vgic/vgic-v5.c
> > > new file mode 100644
> > > index 000000000000..57199449ca0f
> > > --- /dev/null
> > > +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> > > @@ -0,0 +1,14 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +#include <kvm/arm_vgic.h>
> > > +
> > > +#include "vgic.h"
> > > +
> > > +inline bool kvm_vgic_in_v3_compat_mode(void)a
> > 
> > nit: we're generally trusting of the compiler to 'do the right
> > thing'
> > and avoid explicit inline specifiers unless necessary.

Dropped the explicit inline.

> > 
> > > +{
> > > +	if
> > > (static_branch_unlikely(&kvm_vgic_global_state.gicv5_cpuif) &&
> > > +	    kvm_vgic_global_state.has_gcie_v3_compat)
> > > +		return true;
> > > +
> > > +	return false;
> > > +}
> > 
> > This should be a per-VM thing once KVM support for GICv5 lands. Can
> > you
> > get ahead of that and take a KVM pointer that goes unused. Maybe
> > rename
> > it:
> > 
> > bool vgic_is_v3_compat(struct kvm *kvm)
> > 
> > Or something similar.

OK, will do. There's one case were we use this without access to a
struct kvm* in kvm_vgic_init_cpu_hardware, so that will need to be done
without this helper, but we could fall back to using cpucaps directly
there.

Thanks,
Sascha

> 

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

* Re: [PATCH 5/5] KVM: arm64: gic-v5: Probe for GICv5
  2025-06-20 20:25   ` Oliver Upton
@ 2025-06-23 13:12     ` Sascha Bischoff
  0 siblings, 0 replies; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-23 13:12 UTC (permalink / raw)
  To: oliver.upton@linux.dev
  Cc: yuzenghui@huawei.com, Timothy Hayes, Suzuki Poulose, nd,
	lpieralisi@kernel.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	Joey Gouly, maz@kernel.org, tglx@linutronix.de, will@kernel.org

On Fri, 2025-06-20 at 13:25 -0700, Oliver Upton wrote:
> On Fri, Jun 20, 2025 at 04:07:52PM +0000, Sascha Bischoff wrote:
> > +/**
> > + * vgic_v5_probe - probe for a VGICv5 compatible interrupt
> > controller
> > + * @info:	pointer to the GIC description
> > + *
> > + * Returns 0 if the VGICv5 has been probed successfully, returns
> > an error code
> > + * otherwise.
> > + */
> 
> nit: avoid kerneldoc style
> 
> This actually generates documentation as well as build warnings when
> we
> screw up the format. I'd only do this sort of thing for sufficiently
> public functions.
> 
> Thanks,
> Oliver

Done. Have made this into a non-kernel-doc comment.

FWIW, this was mimicking what is done for vgic_v3_probe.


Thanks,
Sascha
> 

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

* Re: [PATCH 2/5] irqchip/gic-v5: Populate struct gic_kvm_info
  2025-06-20 16:07 ` [PATCH 2/5] irqchip/gic-v5: Populate struct gic_kvm_info Sascha Bischoff
@ 2025-06-23 15:14   ` Lorenzo Pieralisi
  2025-06-27  9:49     ` Sascha Bischoff
  0 siblings, 1 reply; 18+ messages in thread
From: Lorenzo Pieralisi @ 2025-06-23 15:14 UTC (permalink / raw)
  To: Sascha Bischoff
  Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org, nd,
	maz@kernel.org, oliver.upton@linux.dev, Joey Gouly,
	Suzuki Poulose, yuzenghui@huawei.com, will@kernel.org,
	tglx@linutronix.de, Timothy Hayes

On Fri, Jun 20, 2025 at 04:07:51PM +0000, Sascha Bischoff wrote:
> Populate the gic_kvm_info struct based on support for
> FEAT_GCIE_LEGACY.  The struct is used by KVM to probe for a compatible
> GIC.
> 
> Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
> Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
>  drivers/irqchip/irq-gic-v5.c          | 34 +++++++++++++++++++++++++++
>  include/linux/irqchip/arm-vgic-info.h |  4 ++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> index 28853d51a2ea..6aecd2343fee 100644
> --- a/drivers/irqchip/irq-gic-v5.c
> +++ b/drivers/irqchip/irq-gic-v5.c
> @@ -13,6 +13,7 @@
>  
>  #include <linux/irqchip.h>
>  #include <linux/irqchip/arm-gic-v5.h>
> +#include <linux/irqchip/arm-vgic-info.h>
>  
>  #include <asm/cpufeature.h>
>  #include <asm/exception.h>
> @@ -1049,6 +1050,37 @@ static void gicv5_set_cpuif_idbits(void)
>  	}
>  }
>  
> +#ifdef CONFIG_KVM
> +static struct gic_kvm_info gic_v5_kvm_info __initdata;
> +
> +static bool gicv5_cpuif_has_gcie_legacy(void)

__init ?

> +{
> +	u64 idr0 = read_sysreg_s(SYS_ICC_IDR0_EL1);
> +
> +	return !!FIELD_GET(ICC_IDR0_EL1_GCIE_LEGACY, idr0);
> +}
> +
> +static void __init gic_of_setup_kvm_info(struct device_node *node)
> +{
> +	gic_v5_kvm_info.type = GIC_V5;
> +	gic_v5_kvm_info.has_gcie_v3_compat = gicv5_cpuif_has_gcie_legacy();
> +
> +	/* GIC Virtual CPU interface maintenance interrupt */
> +	gic_v5_kvm_info.no_maint_irq_mask = false;
> +	gic_v5_kvm_info.maint_irq = irq_of_parse_and_map(node, 0);
> +	if (!gic_v5_kvm_info.maint_irq) {
> +		pr_warn("cannot find GICv5 virtual CPU interface maintenance interrupt\n");
> +		return;
> +	}
> +
> +	vgic_set_kvm_info(&gic_v5_kvm_info);
> +}
> +#else
> +static void __init gic_of_setup_kvm_info(struct device_node *node)

static inline

Thanks,
Lorenzo

> +{
> +}
> +#endif // CONFIG_KVM
> +
>  static int __init gicv5_of_init(struct device_node *node, struct device_node *parent)
>  {
>  	int ret = gicv5_irs_of_probe(node);
> @@ -1081,6 +1113,8 @@ static int __init gicv5_of_init(struct device_node *node, struct device_node *pa
>  
>  	gicv5_irs_its_probe();
>  
> +	gic_of_setup_kvm_info(node);
> +
>  	return 0;
>  out_int:
>  	gicv5_cpu_disable_interrupts();
> diff --git a/include/linux/irqchip/arm-vgic-info.h b/include/linux/irqchip/arm-vgic-info.h
> index a75b2c7de69d..ca1713fac6e3 100644
> --- a/include/linux/irqchip/arm-vgic-info.h
> +++ b/include/linux/irqchip/arm-vgic-info.h
> @@ -15,6 +15,8 @@ enum gic_type {
>  	GIC_V2,
>  	/* Full GICv3, optionally with v2 compat */
>  	GIC_V3,
> +	/* Full GICv5, optionally with v3 compat */
> +	GIC_V5,
>  };
>  
>  struct gic_kvm_info {
> @@ -34,6 +36,8 @@ struct gic_kvm_info {
>  	bool		has_v4_1;
>  	/* Deactivation impared, subpar stuff */
>  	bool		no_hw_deactivation;
> +	/* v3 compat support (GICv5 hosts, only) */
> +	bool		has_gcie_v3_compat;
>  };
>  
>  #ifdef CONFIG_KVM
> -- 
> 2.34.1

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

* Re: [PATCH 1/5] irqchip/gic-v5: Skip deactivate for forwarded PPI interrupts
  2025-06-20 16:07 ` [PATCH 1/5] irqchip/gic-v5: Skip deactivate for forwarded PPI interrupts Sascha Bischoff
@ 2025-06-23 15:21   ` Lorenzo Pieralisi
  2025-06-27  9:49     ` Sascha Bischoff
  0 siblings, 1 reply; 18+ messages in thread
From: Lorenzo Pieralisi @ 2025-06-23 15:21 UTC (permalink / raw)
  To: Sascha Bischoff
  Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org, nd,
	maz@kernel.org, oliver.upton@linux.dev, Joey Gouly,
	Suzuki Poulose, yuzenghui@huawei.com, will@kernel.org,
	tglx@linutronix.de, Timothy Hayes

On Fri, Jun 20, 2025 at 04:07:50PM +0000, Sascha Bischoff wrote:
> If a PPI interrupt is forwarded to a guest, skip the deactivate and
> only EOI. Rely on the guest deactivating the both the virtual and

"deactivating both"

> physical interrupts (due to ICH_LRx_EL2.HW being set) later on as part
> of handling the injected interrupt. This mimics the behaviour seen on
> native GICv3.
> 
> This is part of adding support for the GICv3 compatibility mode on a
> GICv5 host.
> 
> Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
> Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
>  drivers/irqchip/irq-gic-v5.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Reviewed-by: Lorenzo Pieralisi <lpieralisi@kernel.org>

> diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> index 4a0990f46358..28853d51a2ea 100644
> --- a/drivers/irqchip/irq-gic-v5.c
> +++ b/drivers/irqchip/irq-gic-v5.c
> @@ -213,6 +213,12 @@ static void gicv5_hwirq_eoi(u32 hwirq_id, u8 hwirq_type)
>  
>  static void gicv5_ppi_irq_eoi(struct irq_data *d)
>  {
> +	/* Skip deactivate for forwarded PPI interrupts */
> +	if (irqd_is_forwarded_to_vcpu(d)) {
> +		gic_insn(0, CDEOI);
> +		return;
> +	}
> +
>  	gicv5_hwirq_eoi(d->hwirq, GICV5_HWIRQ_TYPE_PPI);
>  }
>  
> @@ -494,6 +500,16 @@ static bool gicv5_ppi_irq_is_level(irq_hw_number_t hwirq)
>  	return !!(read_ppi_sysreg_s(hwirq, PPI_HM) & bit);
>  }
>  
> +static int gicv5_ppi_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
> +{
> +	if (vcpu)
> +		irqd_set_forwarded_to_vcpu(d);
> +	else
> +		irqd_clr_forwarded_to_vcpu(d);
> +
> +	return 0;
> +}
> +
>  static const struct irq_chip gicv5_ppi_irq_chip = {
>  	.name			= "GICv5-PPI",
>  	.irq_mask		= gicv5_ppi_irq_mask,
> @@ -501,6 +517,7 @@ static const struct irq_chip gicv5_ppi_irq_chip = {
>  	.irq_eoi		= gicv5_ppi_irq_eoi,
>  	.irq_get_irqchip_state	= gicv5_ppi_irq_get_irqchip_state,
>  	.irq_set_irqchip_state	= gicv5_ppi_irq_set_irqchip_state,
> +	.irq_set_vcpu_affinity	= gicv5_ppi_irq_set_vcpu_affinity,
>  	.flags			= IRQCHIP_SKIP_SET_WAKE	  |
>  				  IRQCHIP_MASK_ON_SUSPEND,
>  };
> -- 
> 2.34.1

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

* Re: [PATCH 1/5] irqchip/gic-v5: Skip deactivate for forwarded PPI interrupts
  2025-06-23 15:21   ` Lorenzo Pieralisi
@ 2025-06-27  9:49     ` Sascha Bischoff
  0 siblings, 0 replies; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-27  9:49 UTC (permalink / raw)
  To: lpieralisi@kernel.org
  Cc: yuzenghui@huawei.com, tglx@linutronix.de, Timothy Hayes, nd,
	oliver.upton@linux.dev, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	Joey Gouly, maz@kernel.org, Suzuki Poulose, will@kernel.org

On Mon, 2025-06-23 at 17:21 +0200, Lorenzo Pieralisi wrote:
> On Fri, Jun 20, 2025 at 04:07:50PM +0000, Sascha Bischoff wrote:
> > If a PPI interrupt is forwarded to a guest, skip the deactivate and
> > only EOI. Rely on the guest deactivating the both the virtual and
> 
> "deactivating both"

Done.

> 
> > physical interrupts (due to ICH_LRx_EL2.HW being set) later on as
> > part
> > of handling the injected interrupt. This mimics the behaviour seen
> > on
> > native GICv3.
> > 
> > This is part of adding support for the GICv3 compatibility mode on
> > a
> > GICv5 host.
> > 
> > Co-authored-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > ---
> >  drivers/irqchip/irq-gic-v5.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> 
> Reviewed-by: Lorenzo Pieralisi <lpieralisi@kernel.org>

Done. Thanks!
Sascha



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

* Re: [PATCH 2/5] irqchip/gic-v5: Populate struct gic_kvm_info
  2025-06-23 15:14   ` Lorenzo Pieralisi
@ 2025-06-27  9:49     ` Sascha Bischoff
  0 siblings, 0 replies; 18+ messages in thread
From: Sascha Bischoff @ 2025-06-27  9:49 UTC (permalink / raw)
  To: lpieralisi@kernel.org
  Cc: yuzenghui@huawei.com, tglx@linutronix.de, Timothy Hayes, nd,
	oliver.upton@linux.dev, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	Joey Gouly, maz@kernel.org, Suzuki Poulose, will@kernel.org

On Mon, 2025-06-23 at 17:14 +0200, Lorenzo Pieralisi wrote:
> On Fri, Jun 20, 2025 at 04:07:51PM +0000, Sascha Bischoff wrote:
> >  
> > +#ifdef CONFIG_KVM
> > +static struct gic_kvm_info gic_v5_kvm_info __initdata;
> > +
> > +static bool gicv5_cpuif_has_gcie_legacy(void)
> 
> __init ?

Done.

> > 
> > +}
> > +#else
> > +static void __init gic_of_setup_kvm_info(struct device_node *node)
> 
> static inline

Done.

Thanks,
Sascha



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

end of thread, other threads:[~2025-06-27  9:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-20 16:07 [PATCH 0/5] KVM: arm64: Enable GICv3 guests on GICv5 hosts using FEAT_GCIE_LEGACY Sascha Bischoff
2025-06-20 16:07 ` [PATCH 1/5] irqchip/gic-v5: Skip deactivate for forwarded PPI interrupts Sascha Bischoff
2025-06-23 15:21   ` Lorenzo Pieralisi
2025-06-27  9:49     ` Sascha Bischoff
2025-06-20 16:07 ` [PATCH 2/5] irqchip/gic-v5: Populate struct gic_kvm_info Sascha Bischoff
2025-06-23 15:14   ` Lorenzo Pieralisi
2025-06-27  9:49     ` Sascha Bischoff
2025-06-20 16:07 ` [PATCH 3/5] arm64/sysreg: Add ICH_VCTLR_EL2 Sascha Bischoff
2025-06-20 16:07 ` [PATCH 4/5] KVM: arm64: gic-v5: Support GICv3 compat Sascha Bischoff
2025-06-20 20:20   ` Oliver Upton
2025-06-20 23:02     ` Oliver Upton
2025-06-23 13:11       ` Sascha Bischoff
2025-06-22 12:19     ` Marc Zyngier
2025-06-22 12:37       ` Oliver Upton
2025-06-23 13:02         ` Sascha Bischoff
2025-06-20 16:07 ` [PATCH 5/5] KVM: arm64: gic-v5: Probe for GICv5 Sascha Bischoff
2025-06-20 20:25   ` Oliver Upton
2025-06-23 13:12     ` Sascha Bischoff

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).