[PATCH 0/5] KVM: arm64: Make ICH_VTR_EL2 accesses an inlined literal

[PATCH 0/5] KVM: arm64: Make ICH_VTR_EL2 accesses an inlined literal

[Marc Zyngier]

I did it again.

I was looking at NV traces, and realised that we were trapping a whole
lot of accesses to ICH_VTR_EL2. But ICH_VTR_EL2 really is a constant
from a hypervisor perspective, and we usually cache this in memory, just
like any other ID register.

We do have a variable for this in kvm_vgic_global_state, but we don't
map this one at EL2 since 8aaf3f7dce746 ("KVM: arm64: Don't map
'kvm_vgic_global_state' at EL2 with pKVM").

So what can we do? We can revert back to mapping the global state, but
that's not a very good idea. Or we can be creative and turn the
ICH_VTR_EL2 accesses into a runtime constant patched into the
code. Since we already have a bunch of similar things for the GICv3
traps, that's no big deal.

And for consistency, let's kill kvm_vgic_global_state.ich_vtr_el2
altogether, so that there is only a single efficient way to read
ICH_VTR_EL2.

As usual, this has a significant impact on deeply nested workloads, a
solid 5% reduction in elapsed time for an L3 Linux guest running under
nested KVM instances. I expect this to benefit KVM running under other
hypervisors as well.

Patches on top of -rc3.

Marc Zyngier (5):
  KVM: arm64: vgic-v3: Make vtr_to_* helpers use architectural field
    symbols
  KVM: arm64: Add a helper providing an inlined literal value for
    ICH_VTR_EL2
  KVM: arm64: Convert most ICH_VTR_EL2 accesses to inlined literal value
  KVM: arm64: vgic-v3: Simplify initial GICv3 configuration sampling
  KVM: arm64: vgic-v3: Kill kvm_vgic_global_state.ich_vtr_el2

 arch/arm64/kernel/image-vars.h   |  1 +
 arch/arm64/kvm/hyp/vgic-v3-sr.c  | 29 ++++++--------
 arch/arm64/kvm/nested.c          |  3 +-
 arch/arm64/kvm/vgic-sys-reg-v3.c |  8 ++--
 arch/arm64/kvm/vgic/vgic-v3.c    | 69 +++++++++++++++++++++++++++-----
 arch/arm64/kvm/vgic/vgic-v5.c    |  5 +--
 arch/arm64/kvm/vgic/vgic.h       | 39 +++++++++++++-----
 include/kvm/arm_vgic.h           |  2 -
 8 files changed, 108 insertions(+), 48 deletions(-)

-- 
2.47.3

[PATCH 1/5] KVM: arm64: vgic-v3: Make vtr_to_* helpers use architectural field symbols

[Marc Zyngier]

vgic-v3-sr.c still contains some hardcoded constants for some of
the ICH)VTR_EL2 fields. Bring them into the modern world by using
the named symbols from the sysreg file.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index c4d2f1feea8b6..8da7a40ced58a 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -16,9 +16,9 @@
 
 #include "../../vgic/vgic.h"
 
-#define vtr_to_max_lr_idx(v)		((v) & 0xf)
-#define vtr_to_nr_pre_bits(v)		((((u32)(v) >> 26) & 7) + 1)
-#define vtr_to_nr_apr_regs(v)		(1 << (vtr_to_nr_pre_bits(v) - 5))
+#define vtr_to_max_lr_idx(v)		FIELD_GET(ICH_VTR_EL2_ListRegs, (v))
+#define vtr_to_nr_pre_bits(v)		(FIELD_GET(ICH_VTR_EL2_PREbits, (v)) + 1)
+#define vtr_to_nr_apr_regs(v)		BIT(vtr_to_nr_pre_bits(v) - 5)
 
 u64 __gic_v3_get_lr(unsigned int lr)
 {
-- 
2.47.3

[PATCH 2/5] KVM: arm64: Add a helper providing an inlined literal value for ICH_VTR_EL2

[Marc Zyngier]

We already have two ways to deal with ICH_VTR_EL2:

- directly read the system register
- read a cached copy in the vgic global state

Add a third way, in the form of an inlined literal value that will
eventually replace all of the above. This literal value is computed
at boot time, and patched in the relevant code locations.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kernel/image-vars.h |  1 +
 arch/arm64/kvm/vgic/vgic-v3.c  | 51 ++++++++++++++++++++++++++++++++++
 arch/arm64/kvm/vgic/vgic.h     | 17 ++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index d4c7d45ae6bc8..459c33634ebd1 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -92,6 +92,7 @@ KVM_NVHE_ALIAS(spectre_bhb_patch_wa3);
 KVM_NVHE_ALIAS(spectre_bhb_patch_clearbhb);
 KVM_NVHE_ALIAS(alt_cb_patch_nops);
 KVM_NVHE_ALIAS(kvm_compute_ich_hcr_trap_bits);
+KVM_NVHE_ALIAS(kvm_patch_ich_vtr_el2);
 
 /* Global kernel state accessed by nVHE hyp code. */
 KVM_NVHE_ALIAS(kvm_vgic_global_state);
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 9e841e7afd4a7..ef62dd498f5cf 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -876,6 +876,57 @@ void noinstr kvm_compute_ich_hcr_trap_bits(struct alt_instr *alt,
 	*updptr = cpu_to_le32(insn);
 }
 
+void noinstr kvm_patch_ich_vtr_el2(struct alt_instr *alt,
+				   __le32 *origptr, __le32 *updptr,
+				   int nr_inst)
+{
+	struct arm_smccc_res res = {};
+	u32 insn, oinsn, rd, vtr;
+
+	/* No KVM? Nothing to do */
+	if (!is_hyp_mode_available())
+		return;
+
+	/* No v3 (or v3 compat)? Nothing to do either */
+	if (!this_cpu_has_cap(ARM64_HAS_GICV5_LEGACY) &&
+	    !this_cpu_has_cap(ARM64_HAS_GICV3_CPUIF))
+		return;
+
+	/*
+	 * At the point where this is called, we are guaranteed that if
+	 * we're running at EL1, then the EL2 stubs are still in place.
+	 */
+	if (is_kernel_in_hyp_mode())
+		res.a1 = read_sysreg_s(SYS_ICH_VTR_EL2);
+	else
+		arm_smccc_1_1_hvc(HVC_GET_ICH_VTR_EL2, &res);
+
+	if (WARN_ON(res.a0 == HVC_STUB_ERR))
+		return;
+
+	vtr = res.a1;
+
+	/* Compute target register */
+	oinsn = le32_to_cpu(*origptr);
+	rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, oinsn);
+
+	/* movz rd, #(vtr & 0xffff) */
+	insn = aarch64_insn_gen_movewide(rd,
+					 (u16)vtr,
+					 0,
+					 AARCH64_INSN_VARIANT_64BIT,
+					 AARCH64_INSN_MOVEWIDE_ZERO);
+	*updptr++ = cpu_to_le32(insn);
+
+	/* movk rd, #((vtr >> 16) & 0xffff), lsl #16 */
+	insn = aarch64_insn_gen_movewide(rd,
+					 (u16)(vtr >> 16),
+					 16,
+					 AARCH64_INSN_VARIANT_64BIT,
+					 AARCH64_INSN_MOVEWIDE_KEEP);
+	*updptr++ = cpu_to_le32(insn);
+}
+
 void vgic_v3_enable_cpuif_traps(void)
 {
 	u64 traps = vgic_ich_hcr_trap_bits();
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index f45f7e3ec4d6e..4f65a210041ff 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -180,6 +180,23 @@ static inline u64 vgic_ich_hcr_trap_bits(void)
 	return hcr;
 }
 
+void kvm_patch_ich_vtr_el2(struct alt_instr *alt,
+			   __le32 *origptr, __le32 *updptr, int nr_inst);
+
+static inline u64 vgic_ich_vtr(void)
+{
+	u64 vtr;
+
+	/* All non-RES0 bits are in the bottom 32bits */
+	asm volatile(ALTERNATIVE_CB("movz %0, #0\n"
+				    "movk %0, #0, lsl #16\n",
+				    ARM64_ALWAYS_SYSTEM,
+				    kvm_patch_ich_vtr_el2)
+		     : "=r" (vtr));
+
+	return vtr;
+}
+
 /*
  * This struct provides an intermediate representation of the fields contained
  * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
-- 
2.47.3

[PATCH 2/5] KVM: arm64: Add a helper providing an inlined litteral value for ICH_VTR_EL2

[Marc Zyngier]

We already have two ways to deal with ICH_VTR_EL2:

- directly read the system register
- read a cached copy in the vgic global state

Add a third way, in the form of an inlined litteral value that will
eventually replace all of the above. This litteral value is computed
at boot time, and patched in the relevant code locations.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kernel/image-vars.h |  1 +
 arch/arm64/kvm/vgic/vgic-v3.c  | 51 ++++++++++++++++++++++++++++++++++
 arch/arm64/kvm/vgic/vgic.h     | 17 ++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index d4c7d45ae6bc8..459c33634ebd1 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -92,6 +92,7 @@ KVM_NVHE_ALIAS(spectre_bhb_patch_wa3);
 KVM_NVHE_ALIAS(spectre_bhb_patch_clearbhb);
 KVM_NVHE_ALIAS(alt_cb_patch_nops);
 KVM_NVHE_ALIAS(kvm_compute_ich_hcr_trap_bits);
+KVM_NVHE_ALIAS(kvm_patch_ich_vtr_el2);
 
 /* Global kernel state accessed by nVHE hyp code. */
 KVM_NVHE_ALIAS(kvm_vgic_global_state);
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 9e841e7afd4a7..ef62dd498f5cf 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -876,6 +876,57 @@ void noinstr kvm_compute_ich_hcr_trap_bits(struct alt_instr *alt,
 	*updptr = cpu_to_le32(insn);
 }
 
+void noinstr kvm_patch_ich_vtr_el2(struct alt_instr *alt,
+				   __le32 *origptr, __le32 *updptr,
+				   int nr_inst)
+{
+	struct arm_smccc_res res = {};
+	u32 insn, oinsn, rd, vtr;
+
+	/* No KVM? Nothing to do */
+	if (!is_hyp_mode_available())
+		return;
+
+	/* No v3 (or v3 compat)? Nothing to do either */
+	if (!this_cpu_has_cap(ARM64_HAS_GICV5_LEGACY) &&
+	    !this_cpu_has_cap(ARM64_HAS_GICV3_CPUIF))
+		return;
+
+	/*
+	 * At the point where this is called, we are guaranteed that if
+	 * we're running at EL1, then the EL2 stubs are still in place.
+	 */
+	if (is_kernel_in_hyp_mode())
+		res.a1 = read_sysreg_s(SYS_ICH_VTR_EL2);
+	else
+		arm_smccc_1_1_hvc(HVC_GET_ICH_VTR_EL2, &res);
+
+	if (WARN_ON(res.a0 == HVC_STUB_ERR))
+		return;
+
+	vtr = res.a1;
+
+	/* Compute target register */
+	oinsn = le32_to_cpu(*origptr);
+	rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, oinsn);
+
+	/* movz rd, #(vtr & 0xffff) */
+	insn = aarch64_insn_gen_movewide(rd,
+					 (u16)vtr,
+					 0,
+					 AARCH64_INSN_VARIANT_64BIT,
+					 AARCH64_INSN_MOVEWIDE_ZERO);
+	*updptr++ = cpu_to_le32(insn);
+
+	/* movk rd, #((vtr >> 16) & 0xffff), lsl #16 */
+	insn = aarch64_insn_gen_movewide(rd,
+					 (u16)(vtr >> 16),
+					 16,
+					 AARCH64_INSN_VARIANT_64BIT,
+					 AARCH64_INSN_MOVEWIDE_KEEP);
+	*updptr++ = cpu_to_le32(insn);
+}
+
 void vgic_v3_enable_cpuif_traps(void)
 {
 	u64 traps = vgic_ich_hcr_trap_bits();
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index f45f7e3ec4d6e..4f65a210041ff 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -180,6 +180,23 @@ static inline u64 vgic_ich_hcr_trap_bits(void)
 	return hcr;
 }
 
+void kvm_patch_ich_vtr_el2(struct alt_instr *alt,
+			   __le32 *origptr, __le32 *updptr, int nr_inst);
+
+static inline u64 vgic_ich_vtr(void)
+{
+	u64 vtr;
+
+	/* All non-RES0 bits are in the bottom 32bits */
+	asm volatile(ALTERNATIVE_CB("movz %0, #0\n"
+				    "movk %0, #0, lsl #16\n",
+				    ARM64_ALWAYS_SYSTEM,
+				    kvm_patch_ich_vtr_el2)
+		     : "=r" (vtr));
+
+	return vtr;
+}
+
 /*
  * This struct provides an intermediate representation of the fields contained
  * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
-- 
2.47.3

[PATCH 3/5] KVM: arm64: Convert most ICH_VTR_EL2 accesses to inlined literal value

[Marc Zyngier]

Now that we have a new helper that inlines the value of ICH_VTR_EL2
into the code, use this to replace most of the sysreg accesses to
that register.

This avoids a lot of traps when running KVM under NV.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index 8da7a40ced58a..3e5d5ddf35a19 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -367,7 +367,7 @@ void __vgic_v3_save_aprs(struct vgic_v3_cpu_if *cpu_if)
 	u64 val;
 	u32 nr_pre_bits;
 
-	val = read_gicreg(ICH_VTR_EL2);
+	val = vgic_ich_vtr();
 	nr_pre_bits = vtr_to_nr_pre_bits(val);
 
 	switch (nr_pre_bits) {
@@ -400,7 +400,7 @@ static void __vgic_v3_restore_aprs(struct vgic_v3_cpu_if *cpu_if)
 	u64 val;
 	u32 nr_pre_bits;
 
-	val = read_gicreg(ICH_VTR_EL2);
+	val = vgic_ich_vtr();
 	nr_pre_bits = vtr_to_nr_pre_bits(val);
 
 	switch (nr_pre_bits) {
@@ -430,7 +430,7 @@ static void __vgic_v3_restore_aprs(struct vgic_v3_cpu_if *cpu_if)
 
 void __vgic_v3_init_lrs(void)
 {
-	int max_lr_idx = vtr_to_max_lr_idx(read_gicreg(ICH_VTR_EL2));
+	int max_lr_idx = vtr_to_max_lr_idx(vgic_ich_vtr());
 	int i;
 
 	for (i = 0; i <= max_lr_idx; i++)
@@ -455,7 +455,7 @@ u64 __vgic_v3_get_gic_config(void)
 	 * system, so we first check if we have GICv5 support.
 	 */
 	if (cpus_have_final_cap(ARM64_HAS_GICV5_CPUIF))
-		return read_gicreg(ICH_VTR_EL2);
+		return vgic_ich_vtr();
 
 	sre = read_gicreg(ICC_SRE_EL1);
 	/*
@@ -498,7 +498,7 @@ u64 __vgic_v3_get_gic_config(void)
 	}
 
 	val  = (val & ICC_SRE_EL1_SRE) ? 0 : (1ULL << 63);
-	val |= read_gicreg(ICH_VTR_EL2);
+	val |= vgic_ich_vtr();
 
 	return val;
 }
@@ -540,7 +540,7 @@ void __vgic_v3_restore_vmcr_aprs(struct vgic_v3_cpu_if *cpu_if)
 static int __vgic_v3_bpr_min(void)
 {
 	/* See Pseudocode for VPriorityGroup */
-	return 8 - vtr_to_nr_pre_bits(read_gicreg(ICH_VTR_EL2));
+	return 8 - vtr_to_nr_pre_bits(vgic_ich_vtr());
 }
 
 static int __vgic_v3_get_group(struct kvm_vcpu *vcpu)
@@ -614,7 +614,7 @@ static int __vgic_v3_find_active_lr(struct kvm_vcpu *vcpu, int intid,
 
 static int __vgic_v3_get_highest_active_priority(void)
 {
-	u8 nr_apr_regs = vtr_to_nr_apr_regs(read_gicreg(ICH_VTR_EL2));
+	u8 nr_apr_regs = vtr_to_nr_apr_regs(vgic_ich_vtr());
 	u32 hap = 0;
 	int i;
 
@@ -707,7 +707,7 @@ static void __vgic_v3_set_active_priority(u8 pri, u32 vmcr, int grp)
 
 static int __vgic_v3_clear_highest_active_priority(void)
 {
-	u8 nr_apr_regs = vtr_to_nr_apr_regs(read_gicreg(ICH_VTR_EL2));
+	u8 nr_apr_regs = vtr_to_nr_apr_regs(vgic_ich_vtr());
 	u32 hap = 0;
 	int i;
 
@@ -1039,7 +1039,7 @@ static void __vgic_v3_read_ctlr(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 {
 	u32 vtr, val;
 
-	vtr = read_gicreg(ICH_VTR_EL2);
+	vtr = vgic_ich_vtr();
 	/* PRIbits */
 	val = ((vtr >> 29) & 7) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
 	/* IDbits */
-- 
2.47.3

[PATCH 4/5] KVM: arm64: vgic-v3: Simplify initial GICv3 configuration sampling

[Marc Zyngier]

Now that we have our magic inline helper for ICH_VTR_EL2, we can
get rid of the hack that was reporting a combination of that
register and of the indication of the CPU interface supporting
GICv2 compatibility. We now only report the latter.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c |  9 ++-------
 arch/arm64/kvm/vgic/vgic-v3.c   | 13 +++++++------
 arch/arm64/kvm/vgic/vgic-v5.c   |  2 +-
 3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/kvm/hyp/vgic-v3-sr.c b/arch/arm64/kvm/hyp/vgic-v3-sr.c
index 3e5d5ddf35a19..8a15d6854d1bd 100644
--- a/arch/arm64/kvm/hyp/vgic-v3-sr.c
+++ b/arch/arm64/kvm/hyp/vgic-v3-sr.c
@@ -439,8 +439,6 @@ void __vgic_v3_init_lrs(void)
 
 /*
  * Return the GIC CPU configuration:
- * - [31:0]  ICH_VTR_EL2
- * - [62:32] RES0
  * - [63]    MMIO (GICv2) capable
  */
 u64 __vgic_v3_get_gic_config(void)
@@ -455,7 +453,7 @@ u64 __vgic_v3_get_gic_config(void)
 	 * system, so we first check if we have GICv5 support.
 	 */
 	if (cpus_have_final_cap(ARM64_HAS_GICV5_CPUIF))
-		return vgic_ich_vtr();
+		return 0;
 
 	sre = read_gicreg(ICC_SRE_EL1);
 	/*
@@ -497,10 +495,7 @@ u64 __vgic_v3_get_gic_config(void)
 		isb();
 	}
 
-	val  = (val & ICC_SRE_EL1_SRE) ? 0 : (1ULL << 63);
-	val |= vgic_ich_vtr();
-
-	return val;
+	return (val & ICC_SRE_EL1_SRE) ? 0 : (1ULL << 63);
 }
 
 static void __vgic_v3_compat_mode_enable(void)
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index ef62dd498f5cf..68d4a486df0a1 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -906,6 +906,9 @@ void noinstr kvm_patch_ich_vtr_el2(struct alt_instr *alt,
 
 	vtr = res.a1;
 
+	if (vgic_v3_broken_seis())
+		vtr &= ~ICH_VTR_EL2_SEIS;
+
 	/* Compute target register */
 	oinsn = le32_to_cpu(*origptr);
 	rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, oinsn);
@@ -950,12 +953,12 @@ void vgic_v3_enable_cpuif_traps(void)
  */
 int vgic_v3_probe(const struct gic_kvm_info *info)
 {
-	u64 ich_vtr_el2 = kvm_call_hyp_ret(__vgic_v3_get_gic_config);
+	u64 ich_vtr_el2;
 	bool has_v2;
 	int ret;
 
-	has_v2 = ich_vtr_el2 >> 63;
-	ich_vtr_el2 = (u32)ich_vtr_el2;
+	has_v2 = kvm_call_hyp_ret(__vgic_v3_get_gic_config) >> 63;
+	ich_vtr_el2 = vgic_ich_vtr();
 
 	/*
 	 * The ListRegs field is 5 bits, but there is an architectural
@@ -1010,10 +1013,8 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
 	if (has_v2)
 		static_branch_enable(&vgic_v3_has_v2_compat);
 
-	if (vgic_v3_broken_seis()) {
+	if (vgic_v3_broken_seis())
 		kvm_info("GICv3 with broken locally generated SEI\n");
-		kvm_vgic_global_state.ich_vtr_el2 &= ~ICH_VTR_EL2_SEIS;
-	}
 
 	vgic_v3_enable_cpuif_traps();
 
diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
index d4789ff3e7402..16bc0a670d3e5 100644
--- a/arch/arm64/kvm/vgic/vgic-v5.c
+++ b/arch/arm64/kvm/vgic/vgic-v5.c
@@ -83,7 +83,7 @@ int vgic_v5_probe(const struct gic_kvm_info *info)
 	}
 
 	kvm_vgic_global_state.has_gcie_v3_compat = true;
-	ich_vtr_el2 =  kvm_call_hyp_ret(__vgic_v3_get_gic_config);
+	ich_vtr_el2 = vgic_ich_vtr();
 	kvm_vgic_global_state.ich_vtr_el2 = (u32)ich_vtr_el2;
 
 	/*
-- 
2.47.3

[PATCH 5/5] KVM: arm64: vgic-v3: Kill kvm_vgic_global_state.ich_vtr_el2

[Marc Zyngier]

kvm_vgic_global_state.ich_vtr_el2 is the last bit of caching that
we can get rid of. Not as bad as a sysreg access, but still worse
than a constant.

Move over to the inlined stuff and remove the cached value.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/nested.c          |  3 ++-
 arch/arm64/kvm/vgic-sys-reg-v3.c |  8 ++++----
 arch/arm64/kvm/vgic/vgic-v3.c    |  5 ++---
 arch/arm64/kvm/vgic/vgic-v5.c    |  5 +----
 arch/arm64/kvm/vgic/vgic.h       | 22 +++++++++++-----------
 include/kvm/arm_vgic.h           |  2 --
 6 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index fb54f6dad995c..54defcd8240df 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -16,6 +16,7 @@
 #include <asm/sysreg.h>
 
 #include "sys_regs.h"
+#include "vgic/vgic.h"
 
 struct vncr_tlb {
 	/* The guest's VNCR_EL2 */
@@ -1904,7 +1905,7 @@ int kvm_init_nv_sysregs(struct kvm_vcpu *vcpu)
 	/* ICH_HCR_EL2 */
 	resx.res0 = ICH_HCR_EL2_RES0;
 	resx.res1 = ICH_HCR_EL2_RES1;
-	if (!(kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_EL2_TDS))
+	if (!(vgic_ich_vtr() & ICH_VTR_EL2_TDS))
 		resx.res0 |= ICH_HCR_EL2_TDIR;
 	/* No GICv4 is presented to the guest */
 	resx.res0 |= ICH_HCR_EL2_DVIM | ICH_HCR_EL2_vSGIEOICount;
diff --git a/arch/arm64/kvm/vgic-sys-reg-v3.c b/arch/arm64/kvm/vgic-sys-reg-v3.c
index bdc2d57370b27..89315336b9d3c 100644
--- a/arch/arm64/kvm/vgic-sys-reg-v3.c
+++ b/arch/arm64/kvm/vgic-sys-reg-v3.c
@@ -35,12 +35,12 @@ static int set_gic_ctlr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
 
 	vgic_v3_cpu->num_id_bits = host_id_bits;
 
-	host_seis = FIELD_GET(ICH_VTR_EL2_SEIS, kvm_vgic_global_state.ich_vtr_el2);
+	host_seis = FIELD_GET(ICH_VTR_EL2_SEIS, vgic_ich_vtr());
 	seis = FIELD_GET(ICC_CTLR_EL1_SEIS_MASK, val);
 	if (host_seis != seis)
 		return -EINVAL;
 
-	host_a3v = FIELD_GET(ICH_VTR_EL2_A3V, kvm_vgic_global_state.ich_vtr_el2);
+	host_a3v = FIELD_GET(ICH_VTR_EL2_A3V, vgic_ich_vtr());
 	a3v = FIELD_GET(ICC_CTLR_EL1_A3V_MASK, val);
 	if (host_a3v != a3v)
 		return -EINVAL;
@@ -69,9 +69,9 @@ static int get_gic_ctlr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
 	val |= FIELD_PREP(ICC_CTLR_EL1_ID_BITS_MASK, vgic_v3_cpu->num_id_bits);
 	val |= FIELD_PREP(ICC_CTLR_EL1_SEIS_MASK,
 			  FIELD_GET(ICH_VTR_EL2_SEIS,
-				    kvm_vgic_global_state.ich_vtr_el2));
+				    vgic_ich_vtr()));
 	val |= FIELD_PREP(ICC_CTLR_EL1_A3V_MASK,
-			  FIELD_GET(ICH_VTR_EL2_A3V, kvm_vgic_global_state.ich_vtr_el2));
+			  FIELD_GET(ICH_VTR_EL2_A3V, vgic_ich_vtr()));
 	/*
 	 * The VMCR.CTLR value is in ICC_CTLR_EL1 layout.
 	 * Extract it directly using ICC_CTLR_EL1 reg definitions.
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 68d4a486df0a1..7682cbbe3d8bf 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -490,9 +490,9 @@ void vgic_v3_reset(struct kvm_vcpu *vcpu)
 	}
 
 	vcpu->arch.vgic_cpu.num_id_bits = FIELD_GET(ICH_VTR_EL2_IDbits,
-						    kvm_vgic_global_state.ich_vtr_el2);
+						    vgic_ich_vtr());
 	vcpu->arch.vgic_cpu.num_pri_bits = FIELD_GET(ICH_VTR_EL2_PRIbits,
-						     kvm_vgic_global_state.ich_vtr_el2) + 1;
+						     vgic_ich_vtr()) + 1;
 }
 
 void vcpu_set_ich_hcr(struct kvm_vcpu *vcpu)
@@ -966,7 +966,6 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
 	 */
 	kvm_vgic_global_state.nr_lr = (ich_vtr_el2 & 0xf) + 1;
 	kvm_vgic_global_state.can_emulate_gicv2 = false;
-	kvm_vgic_global_state.ich_vtr_el2 = ich_vtr_el2;
 
 	/* GICv4 support? */
 	if (info->has_v4) {
diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
index 16bc0a670d3e5..4e3e4db24e773 100644
--- a/arch/arm64/kvm/vgic/vgic-v5.c
+++ b/arch/arm64/kvm/vgic/vgic-v5.c
@@ -40,7 +40,6 @@ static void vgic_v5_get_implemented_ppis(void)
 int vgic_v5_probe(const struct gic_kvm_info *info)
 {
 	bool v5_registered = false;
-	u64 ich_vtr_el2;
 	int ret;
 
 	kvm_vgic_global_state.type = VGIC_V5;
@@ -83,14 +82,12 @@ int vgic_v5_probe(const struct gic_kvm_info *info)
 	}
 
 	kvm_vgic_global_state.has_gcie_v3_compat = true;
-	ich_vtr_el2 = vgic_ich_vtr();
-	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;
+	kvm_vgic_global_state.nr_lr = (vgic_ich_vtr() & 0xf) + 1;
 
 	ret = kvm_register_vgic_device(KVM_DEV_TYPE_ARM_VGIC_V3);
 	if (ret) {
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index 4f65a210041ff..e709ef8dd97af 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -71,17 +71,6 @@
 				 ICH_VTR_EL2_IDbits)
 #define KVM_ICH_VTR_EL2_RES1	ICH_VTR_EL2_nV4
 
-static inline u64 kvm_get_guest_vtr_el2(void)
-{
-	u64 vtr;
-
-	vtr  = kvm_vgic_global_state.ich_vtr_el2;
-	vtr &= ~KVM_ICH_VTR_EL2_RES0;
-	vtr |= KVM_ICH_VTR_EL2_RES1;
-
-	return vtr;
-}
-
 /*
  * As per Documentation/virt/kvm/devices/arm-vgic-its.rst,
  * below macros are defined for ITS table entry encoding.
@@ -197,6 +186,17 @@ static inline u64 vgic_ich_vtr(void)
 	return vtr;
 }
 
+static inline u64 kvm_get_guest_vtr_el2(void)
+{
+	u64 vtr;
+
+	vtr  = vgic_ich_vtr();
+	vtr &= ~KVM_ICH_VTR_EL2_RES0;
+	vtr |= KVM_ICH_VTR_EL2_RES1;
+
+	return vtr;
+}
+
 /*
  * This struct provides an intermediate representation of the fields contained
  * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index fe49fb56dc3c9..bd1bb03500b30 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -176,8 +176,6 @@ struct vgic_global {
 	/* GICv3 compat mode on a GICv5 host */
 	bool			has_gcie_v3_compat;
 
-	u32			ich_vtr_el2;
-
 	/* GICv5 PPI capabilities */
 	struct {
 		DECLARE_BITMAP(impl_ppi_mask, VGIC_V5_NR_PRIVATE_IRQS);
-- 
2.47.3

Re: [PATCH 4/5] KVM: arm64: vgic-v3: Simplify initial GICv3 configuration sampling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `noinstr` function `kvm_patch_ich_vtr_el2()` calls a non-`noinstr` function (`vgic_v3_broken_seis()`), causing a `noinstr` section violation.
- [Medium] `WARN_ON` inside the `noinstr` alternative patching callback `kvm_patch_ich_vtr_el2()` violates instrumentation rules and risks deadlocks.
--

--- Patch [4]: [PATCH 4/5] KVM: arm64: vgic-v3: Simplify initial GICv3 configuration sampling ---
commit bf26cd3d96184399493f95667d1786d9b2823890
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: vgic-v3: Simplify initial GICv3 configuration sampling

This commit simplifies the initial GICv3 configuration sampling by returning
early if GICv5 CPU interface support is present. It also inlines a workaround
for a broken locally generated SEI directly into the alternative patching
callback.

[ ... ]

> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> index ef62dd498f5cf..68d4a486df0a1 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> @@ -906,6 +906,9 @@ void noinstr kvm_patch_ich_vtr_el2(struct alt_instr *alt,
>  
>  	vtr = res.a1;
>  
> +	if (vgic_v3_broken_seis())
> +		vtr &= ~ICH_VTR_EL2_SEIS;

[Severity: Medium]
Does this code call an instrumentable function from within a noinstr section?

Looking at vgic_v3_broken_seis(), it calls is_midr_in_range_list(). Calling an
instrumentable function from a noinstr function like kvm_patch_ich_vtr_el2()
breaks safety invariants, which can result in objtool build warnings or
ftrace/kprobe recursion deadlocks if the patching code is traced.

[Severity: Medium]
Also, just before this addition in kvm_patch_ich_vtr_el2(), there is a WARN_ON
call:

	if (WARN_ON(res.a0 == HVC_STUB_ERR))
		return;

	vtr = res.a1;

Is it safe to use WARN_ON inside a noinstr alternative patching callback?

Alternative patching callbacks like kvm_patch_ich_vtr_el2() typically run under
stop_machine() with CPUs parked and interrupts disabled. If WARN_ON triggers,
it invokes the warning exception machinery which can attempt to acquire console
locks. If a parked CPU holds these locks, it could cause a hard deadlock.
Additionally, WARN_ON expands to instrumentable paths, directly violating
noinstr requirements.

> +
>  	/* Compute target register */
>  	oinsn = le32_to_cpu(*origptr);
>  	rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, oinsn);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720132220.2143486-1-maz@kernel.org?part=4

Re: [PATCH 2/5] KVM: arm64: Add a helper providing an inlined literal value for ICH_VTR_EL2

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `kvm_patch_ich_vtr_el2()` helper fails to apply the `vgic_v3_broken_seis()` workaround mask to the `ICH_VTR_EL2` value.
- [High] The `noinstr` function `kvm_patch_ich_vtr_el2()` calls out-of-line instrumentable functions, which violates `.noinstr.text` safety rules.
--

--- Patch [2]: [PATCH 2/5] KVM: arm64: Add a helper providing an inlined literal value for ICH_VTR_EL2 ---
commit aedd950f23aaa5667277a7a495b598168c06eb84
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Add a helper providing an inlined literal value for ICH_VTR_EL2

This commit adds an alternative patching callback to provide an inlined
literal value for the ICH_VTR_EL2 system register. It computes the value
at boot time and patches it into the relevant code locations.

> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> index 9e841e7afd4a7..ef62dd498f5cf 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> @@ -876,6 +876,57 @@ void noinstr kvm_compute_ich_hcr_trap_bits(struct alt_instr *alt,
>  	*updptr = cpu_to_le32(insn);
>  }
>  
> +void noinstr kvm_patch_ich_vtr_el2(struct alt_instr *alt,
> +				   __le32 *origptr, __le32 *updptr,
> +				   int nr_inst)
> +{
> +	struct arm_smccc_res res = {};
> +	u32 insn, oinsn, rd, vtr;
> +
> +	/* No KVM? Nothing to do */
> +	if (!is_hyp_mode_available())
> +		return;
> +
> +	/* No v3 (or v3 compat)? Nothing to do either */
> +	if (!this_cpu_has_cap(ARM64_HAS_GICV5_LEGACY) &&
> +	    !this_cpu_has_cap(ARM64_HAS_GICV3_CPUIF))
> +		return;

[Severity: High]
Is it safe to call out-of-line and instrumentable functions like
this_cpu_has_cap() from a noinstr context?

This violates the .noinstr.text safety rules and might expose the
alternative patching sequence to ftrace, KASAN, and kprobes, potentially
leading to recursive faults.

[ ... ]

> +	if (WARN_ON(res.a0 == HVC_STUB_ERR))
> +		return;

[Severity: High]
Could WARN_ON() also expand to out-of-line instrumentable trap or warning
functions, further violating the noinstr constraints of
kvm_patch_ich_vtr_el2()?

> +
> +	vtr = res.a1;
> +
> +	/* Compute target register */
> +	oinsn = le32_to_cpu(*origptr);

[Severity: High]
Does this sequence miss the workaround mask for broken locally generated
System Error Interrupts (SEI)?

The code reads SYS_ICH_VTR_EL2 and directly bakes it into instructions
without applying the vgic_v3_broken_seis() mask to clear the
ICH_VTR_EL2_SEIS bit. Affected hardware might incorrectly advertise SEI
support as a result.

> +	rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, oinsn);
> +
> +	/* movz rd, #(vtr & 0xffff) */
> +	insn = aarch64_insn_gen_movewide(rd,
> +					 (u16)vtr,
> +					 0,
> +					 AARCH64_INSN_VARIANT_64BIT,
> +					 AARCH64_INSN_MOVEWIDE_ZERO);
> +	*updptr++ = cpu_to_le32(insn);

[Severity: High]
Like this_cpu_has_cap() above, aarch64_insn_decode_register() and
aarch64_insn_gen_movewide() are out-of-line C functions. Should these
calls be avoided inside a noinstr patching callback to prevent recursive
faulting during instruction patching?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720132220.2143486-1-maz@kernel.org?part=2

Re: [PATCH 2/5] KVM: arm64: Add a helper providing an inlined literal value for ICH_VTR_EL2

[Marc Zyngier]

On Mon, 20 Jul 2026 14:46:28 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The `kvm_patch_ich_vtr_el2()` helper fails to apply the `vgic_v3_broken_seis()` workaround mask to the `ICH_VTR_EL2` value.
> - [High] The `noinstr` function `kvm_patch_ich_vtr_el2()` calls out-of-line instrumentable functions, which violates `.noinstr.text` safety rules.
> --
> 
> --- Patch [2]: [PATCH 2/5] KVM: arm64: Add a helper providing an inlined literal value for ICH_VTR_EL2 ---
> commit aedd950f23aaa5667277a7a495b598168c06eb84
> Author: Marc Zyngier <maz@kernel.org>
> 
> KVM: arm64: Add a helper providing an inlined literal value for ICH_VTR_EL2
> 
> This commit adds an alternative patching callback to provide an inlined
> literal value for the ICH_VTR_EL2 system register. It computes the value
> at boot time and patches it into the relevant code locations.
> 
> > diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> > index 9e841e7afd4a7..ef62dd498f5cf 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> > @@ -876,6 +876,57 @@ void noinstr kvm_compute_ich_hcr_trap_bits(struct alt_instr *alt,
> >  	*updptr = cpu_to_le32(insn);
> >  }
> >  
> > +void noinstr kvm_patch_ich_vtr_el2(struct alt_instr *alt,
> > +				   __le32 *origptr, __le32 *updptr,
> > +				   int nr_inst)
> > +{
> > +	struct arm_smccc_res res = {};
> > +	u32 insn, oinsn, rd, vtr;
> > +
> > +	/* No KVM? Nothing to do */
> > +	if (!is_hyp_mode_available())
> > +		return;
> > +
> > +	/* No v3 (or v3 compat)? Nothing to do either */
> > +	if (!this_cpu_has_cap(ARM64_HAS_GICV5_LEGACY) &&
> > +	    !this_cpu_has_cap(ARM64_HAS_GICV3_CPUIF))
> > +		return;
> 
> [Severity: High]
> Is it safe to call out-of-line and instrumentable functions like
> this_cpu_has_cap() from a noinstr context?

Yeah, that's borderline stupid. This should be "cpus_have_cap()".

> 
> This violates the .noinstr.text safety rules and might expose the
> alternative patching sequence to ftrace, KASAN, and kprobes, potentially
> leading to recursive faults.
> 
> [ ... ]
> 
> > +	if (WARN_ON(res.a0 == HVC_STUB_ERR))
> > +		return;
> 
> [Severity: High]
> Could WARN_ON() also expand to out-of-line instrumentable trap or warning
> functions, further violating the noinstr constraints of
> kvm_patch_ich_vtr_el2()?

I guess that if that fails, we will already have failed much earlier,
while probing for TDIR. I'll drop that.

> 
> > +
> > +	vtr = res.a1;
> > +
> > +	/* Compute target register */
> > +	oinsn = le32_to_cpu(*origptr);
> 
> [Severity: High]
> Does this sequence miss the workaround mask for broken locally generated
> System Error Interrupts (SEI)?

It isn't missing. It is just that the relevant hunk is in the wrong
patch and needs to be moved here.

> 
> The code reads SYS_ICH_VTR_EL2 and directly bakes it into instructions
> without applying the vgic_v3_broken_seis() mask to clear the
> ICH_VTR_EL2_SEIS bit. Affected hardware might incorrectly advertise SEI
> support as a result.
> 
> > +	rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, oinsn);
> > +
> > +	/* movz rd, #(vtr & 0xffff) */
> > +	insn = aarch64_insn_gen_movewide(rd,
> > +					 (u16)vtr,
> > +					 0,
> > +					 AARCH64_INSN_VARIANT_64BIT,
> > +					 AARCH64_INSN_MOVEWIDE_ZERO);
> > +	*updptr++ = cpu_to_le32(insn);
> 
> [Severity: High]
> Like this_cpu_has_cap() above, aarch64_insn_decode_register() and
> aarch64_insn_gen_movewide() are out-of-line C functions. Should these
> calls be avoided inside a noinstr patching callback to prevent recursive
> faulting during instruction patching?

That's something that needs to be separately addressed. There was some
work from Ada a long while ago that needs to be brought back to life.

	M.

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

Re: [PATCH 4/5] KVM: arm64: vgic-v3: Simplify initial GICv3 configuration sampling

[Marc Zyngier]

On Mon, 20 Jul 2026 14:38:26 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The `noinstr` function `kvm_patch_ich_vtr_el2()` calls a non-`noinstr` function (`vgic_v3_broken_seis()`), causing a `noinstr` section violation.
> - [Medium] `WARN_ON` inside the `noinstr` alternative patching callback `kvm_patch_ich_vtr_el2()` violates instrumentation rules and risks deadlocks.

[...]

See response to patch #2.

	M.

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