[PATCH v5 0/8] KVM: arm64: pKVM vCPU state management at EL2 (series A)

[PATCH v5 0/8] KVM: arm64: pKVM vCPU state management at EL2 (series A)

[Fuad Tabba]

Hi folks,

Changes since v4 [1]:
  - Renamed the sys register accessors: vcpu_{read,write}_sys_reg() are
    now the context-dispatching wrappers valid in any context, and the
    host-only implementations are __vcpu_{read,write}_sysreg_vhe(). This
    avoids introducing a kvm_vcpu_ prefix that would be easy to confuse
    with vcpu_. (Oliver)
  - Dropped Vincent's Reviewed-by on the sys register accessor patch,
    since it changed materially with the rename.
  - Rebased onto v7.2-rc3.

Building on Will's pKVM infrastructure series [2], this series reworks
how pKVM moves vCPU state between the host and EL2, and stops copying a
non-protected guest's state on every world switch.

EL2 gains proper primitives for the state it transfers: vCPU lookup
helpers, and VGIC flush/sync that reduces how much host state EL2
dereferences. The series also moves some preparatory code (such as sys
reg access and PSCI helpers) to shared headers and HYP, and implements
lazy copying of a non-protected guest's register state back to the host
until the host actually needs it, instead of on every exit.

This is the first of two series moving pKVM vCPU state management to
EL2. The follow-up completes the job for protected VMs: state
isolation, PSCI handling at EL2, and the resulting API behaviour.

The series is structured as follows:

  01-04:  Preparatory refactoring (MPIDR, sys reg access, vCPU reset, PSCI
          helpers) to shared headers and HYP.
  05:     Host and hypervisor vCPU lookup primitives.
  06-07:  VGIC: reduce EL2's exposure to host state, add flush/sync primitives.
  08:     Lazy state sync for non-protected guests.

Based on v7.2-rc3 (a13c140cc289c).

Cheers,
/fuad

[1] https://lore.kernel.org/all/20260706095927.560795-1-fuad.tabba@linux.dev/
[2] https://lore.kernel.org/all/20260105154939.11041-1-will@kernel.org/

Fuad Tabba (5):
  KVM: arm64: Extract MPIDR computation into a shared header
  KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code
  KVM: arm64: Factor out reusable vCPU reset helpers
  KVM: arm64: Move PSCI helper functions to a shared header
  KVM: arm64: Implement lazy vCPU state sync for non-protected guests

Marc Zyngier (3):
  KVM: arm64: Add host and hypervisor vCPU lookup primitives
  KVM: arm64: Minimise EL2's exposure of host VGIC state during world
    switch
  KVM: arm64: Add primitives to flush/sync the VGIC state at EL2

 arch/arm64/include/asm/kvm_arm.h     |  12 ++
 arch/arm64/include/asm/kvm_asm.h     |   1 +
 arch/arm64/include/asm/kvm_emulate.h |  77 ++++++++
 arch/arm64/include/asm/kvm_host.h    |   6 +-
 arch/arm64/kvm/arm.c                 |   7 +
 arch/arm64/kvm/handle_exit.c         |  23 +++
 arch/arm64/kvm/hyp/exception.c       |  34 +---
 arch/arm64/kvm/hyp/nvhe/hyp-main.c   | 258 +++++++++++++++++++++++----
 arch/arm64/kvm/psci.c                |  30 +---
 arch/arm64/kvm/reset.c               |  60 +------
 arch/arm64/kvm/sys_regs.c            |  18 +-
 arch/arm64/kvm/sys_regs.h            |  19 ++
 include/kvm/arm_psci.h               |  27 +++
 13 files changed, 408 insertions(+), 164 deletions(-)


base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
-- 
2.39.5

[PATCH v5 1/8] KVM: arm64: Extract MPIDR computation into a shared header

[Fuad Tabba]

Extract the vCPU MPIDR computation embedded in reset_mpidr() into a
kvm_calculate_mpidr() inline in sys_regs.h, so it can be computed
without duplicating the logic. A follow-up series reuses it to reset
protected vCPUs at EL2.

No functional change intended.

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/sys_regs.c | 14 +-------------
 arch/arm64/kvm/sys_regs.h | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 5d5c579d45790..08ba882799d48 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -976,21 +976,9 @@ static u64 reset_actlr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 
 static u64 reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 {
-	u64 mpidr;
+	u64 mpidr = kvm_calculate_mpidr(vcpu);
 
-	/*
-	 * Map the vcpu_id into the first three affinity level fields of
-	 * the MPIDR. We limit the number of VCPUs in level 0 due to a
-	 * limitation to 16 CPUs in that level in the ICC_SGIxR registers
-	 * of the GICv3 to be able to address each CPU directly when
-	 * sending IPIs.
-	 */
-	mpidr = (vcpu->vcpu_id & 0x0f) << MPIDR_LEVEL_SHIFT(0);
-	mpidr |= ((vcpu->vcpu_id >> 4) & 0xff) << MPIDR_LEVEL_SHIFT(1);
-	mpidr |= ((vcpu->vcpu_id >> 12) & 0xff) << MPIDR_LEVEL_SHIFT(2);
-	mpidr |= (1ULL << 31);
 	vcpu_write_sys_reg(vcpu, mpidr, MPIDR_EL1);
-
 	return mpidr;
 }
 
diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
index 2a983664220ce..bd56a45abbf9c 100644
--- a/arch/arm64/kvm/sys_regs.h
+++ b/arch/arm64/kvm/sys_regs.h
@@ -222,6 +222,25 @@ find_reg(const struct sys_reg_params *params, const struct sys_reg_desc table[],
 	return __inline_bsearch((void *)pval, table, num, sizeof(table[0]), match_sys_reg);
 }
 
+static inline u64 kvm_calculate_mpidr(const struct kvm_vcpu *vcpu)
+{
+	u64 mpidr;
+
+	/*
+	 * Map the vcpu_id into the first three affinity level fields of
+	 * the MPIDR. We limit the number of VCPUs in level 0 due to a
+	 * limitation to 16 CPUs in that level in the ICC_SGIxR registers
+	 * of the GICv3 to be able to address each CPU directly when
+	 * sending IPIs.
+	 */
+	mpidr = (vcpu->vcpu_id & 0x0f) << MPIDR_LEVEL_SHIFT(0);
+	mpidr |= ((vcpu->vcpu_id >> 4) & 0xff) << MPIDR_LEVEL_SHIFT(1);
+	mpidr |= ((vcpu->vcpu_id >> 12) & 0xff) << MPIDR_LEVEL_SHIFT(2);
+	mpidr |= (1ULL << 31);
+
+	return mpidr;
+}
+
 const struct sys_reg_desc *get_reg_by_id(u64 id,
 					 const struct sys_reg_desc table[],
 					 unsigned int num);
-- 
2.39.5

[PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

[Fuad Tabba]

The vcpu_{read,write}_sys_reg() accessors are only valid on a VHE host,
so helpers built on them such as kvm_vcpu_set_be()/kvm_vcpu_is_be()
cannot be shared with hyp code. exception.c already wraps them in local
helpers that pick the host- or hyp-side accessor via has_vhe().

Rename the host-only implementations to __vcpu_{read,write}_sysreg_vhe()
and turn vcpu_{read,write}_sys_reg() into the context-dispatching
wrappers, so every caller gets the version valid in any context and a
follow-up series can share that emulation code at EL2.

No functional change intended.

Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
v5:
  - Named the wrappers vcpu_{read,write}_sys_reg() and renamed the
    host-only implementations to __vcpu_{read,write}_sysreg_vhe(), rather
    than introducing a kvm_vcpu_ prefix. (Oliver)
  - Dropped Vincent's Reviewed-by since the patch changed materially.

 arch/arm64/include/asm/kvm_emulate.h | 20 ++++++++++++++++
 arch/arm64/include/asm/kvm_host.h    |  4 ++--
 arch/arm64/kvm/hyp/exception.c       | 34 ++++++++--------------------
 arch/arm64/kvm/sys_regs.c            |  4 ++--
 4 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 5bf3d7e1d92c7..429bda6f48d94 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -506,6 +506,26 @@ static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
 	return __vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
 }
 
+/*
+ * __vcpu_*_sysreg_vhe() are only valid on a VHE host; wrap them so the same
+ * call site also works at EL2 under nVHE.
+ */
+static inline u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
+{
+	if (has_vhe())
+		return __vcpu_read_sysreg_vhe(vcpu, reg);
+
+	return __vcpu_sys_reg(vcpu, reg);
+}
+
+static inline void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
+{
+	if (has_vhe())
+		__vcpu_write_sysreg_vhe(vcpu, val, reg);
+	else
+		__vcpu_assign_sys_reg(vcpu, reg, val);
+}
+
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
 {
 	if (vcpu_mode_is_32bit(vcpu)) {
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index bae2c4f92ef5c..f3c3c86b3d7fb 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1215,8 +1215,8 @@ u64 kvm_vcpu_apply_reg_masks(const struct kvm_vcpu *, enum vcpu_sysreg, u64);
 		__v;							\
 	})
 
-u64 vcpu_read_sys_reg(const struct kvm_vcpu *, enum vcpu_sysreg);
-void vcpu_write_sys_reg(struct kvm_vcpu *, u64, enum vcpu_sysreg);
+u64 __vcpu_read_sysreg_vhe(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg);
+void __vcpu_write_sysreg_vhe(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg);
 
 struct kvm_vm_stat {
 	struct kvm_vm_stat_generic generic;
diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
index bef40ddb16dbc..754e2dc1df54a 100644
--- a/arch/arm64/kvm/hyp/exception.c
+++ b/arch/arm64/kvm/hyp/exception.c
@@ -20,22 +20,6 @@
 #error Hypervisor code only!
 #endif
 
-static inline u64 __vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)
-{
-	if (has_vhe())
-		return vcpu_read_sys_reg(vcpu, reg);
-
-	return __vcpu_sys_reg(vcpu, reg);
-}
-
-static inline void __vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
-{
-	if (has_vhe())
-		vcpu_write_sys_reg(vcpu, val, reg);
-	else
-		__vcpu_assign_sys_reg(vcpu, reg, val);
-}
-
 static void __vcpu_write_spsr(struct kvm_vcpu *vcpu, unsigned long target_mode,
 			      u64 val)
 {
@@ -101,14 +85,14 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
 
 	switch (target_mode) {
 	case PSR_MODE_EL1h:
-		vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL1);
-		sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
-		__vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
+		vbar = vcpu_read_sys_reg(vcpu, VBAR_EL1);
+		sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
+		vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
 		break;
 	case PSR_MODE_EL2h:
-		vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL2);
-		sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL2);
-		__vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
+		vbar = vcpu_read_sys_reg(vcpu, VBAR_EL2);
+		sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL2);
+		vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
 		break;
 	default:
 		/* Don't do that */
@@ -185,7 +169,7 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
  */
 static unsigned long get_except32_cpsr(struct kvm_vcpu *vcpu, u32 mode)
 {
-	u32 sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
+	u32 sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
 	unsigned long old, new;
 
 	old = *vcpu_cpsr(vcpu);
@@ -281,7 +265,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
 {
 	unsigned long spsr = *vcpu_cpsr(vcpu);
 	bool is_thumb = (spsr & PSR_AA32_T_BIT);
-	u32 sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
+	u32 sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
 	u32 return_address;
 
 	*vcpu_cpsr(vcpu) = get_except32_cpsr(vcpu, mode);
@@ -305,7 +289,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
 	if (sctlr & (1 << 13))
 		vect_offset += 0xffff0000;
 	else /* always have security exceptions */
-		vect_offset += __vcpu_read_sys_reg(vcpu, VBAR_EL1);
+		vect_offset += vcpu_read_sys_reg(vcpu, VBAR_EL1);
 
 	*vcpu_pc(vcpu) = vect_offset;
 }
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 08ba882799d48..c6a416974a61f 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -291,7 +291,7 @@ static void write_sr_to_cpu(enum vcpu_sysreg reg, u64 val)
 	}
 }
 
-u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
+u64 __vcpu_read_sysreg_vhe(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
 {
 	struct sr_loc loc = {};
 
@@ -338,7 +338,7 @@ u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
 	return __vcpu_sys_reg(vcpu, reg);
 }
 
-void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
+void __vcpu_write_sysreg_vhe(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
 {
 	struct sr_loc loc = {};
 
-- 
2.39.5

[PATCH v5 3/8] KVM: arm64: Factor out reusable vCPU reset helpers

[Fuad Tabba]

Pull the reusable pieces out of kvm_reset_vcpu(): expose the reset
PSTATE values in kvm_arm.h, and split the core register reset and the
PSCI-driven reset into kvm_reset_vcpu_core() and kvm_reset_vcpu_psci().
A follow-up series reuses these to reset protected vCPUs at EL2.

No functional change intended.

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/include/asm/kvm_arm.h     | 12 ++++++
 arch/arm64/include/asm/kvm_emulate.h | 57 ++++++++++++++++++++++++++
 arch/arm64/kvm/reset.c               | 60 ++--------------------------
 3 files changed, 72 insertions(+), 57 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 3f9233b5a1308..aba4ec09acd23 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -348,4 +348,16 @@
 	{ PSR_AA32_MODE_UND,	"32-bit UND" },	\
 	{ PSR_AA32_MODE_SYS,	"32-bit SYS" }
 
+/*
+ * ARMv8 Reset Values
+ */
+#define VCPU_RESET_PSTATE_EL1	(PSR_MODE_EL1h | PSR_A_BIT | PSR_I_BIT | \
+				 PSR_F_BIT | PSR_D_BIT)
+
+#define VCPU_RESET_PSTATE_EL2	(PSR_MODE_EL2h | PSR_A_BIT | PSR_I_BIT | \
+				 PSR_F_BIT | PSR_D_BIT)
+
+#define VCPU_RESET_PSTATE_SVC	(PSR_AA32_MODE_SVC | PSR_AA32_A_BIT | \
+				 PSR_AA32_I_BIT | PSR_AA32_F_BIT)
+
 #endif /* __ARM64_KVM_ARM_H__ */
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 429bda6f48d94..57e3482ad9a55 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -708,4 +708,61 @@ static inline void vcpu_set_hcrx(struct kvm_vcpu *vcpu)
 			vcpu->arch.hcrx_el2 |= HCRX_EL2_EnASR;
 	}
 }
+
+/* Reset a vcpu's core registers. */
+static inline void kvm_reset_vcpu_core(struct kvm_vcpu *vcpu)
+{
+	u32 pstate;
+
+	if (vcpu_el1_is_32bit(vcpu))
+		pstate = VCPU_RESET_PSTATE_SVC;
+	else if (vcpu_has_nv(vcpu))
+		pstate = VCPU_RESET_PSTATE_EL2;
+	else
+		pstate = VCPU_RESET_PSTATE_EL1;
+
+	/* Reset core registers */
+	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
+	memset(&vcpu->arch.ctxt.fp_regs, 0, sizeof(vcpu->arch.ctxt.fp_regs));
+	vcpu->arch.ctxt.spsr_abt = 0;
+	vcpu->arch.ctxt.spsr_und = 0;
+	vcpu->arch.ctxt.spsr_irq = 0;
+	vcpu->arch.ctxt.spsr_fiq = 0;
+	vcpu_gp_regs(vcpu)->pstate = pstate;
+}
+
+/* PSCI reset handling for a vcpu. */
+static inline void kvm_reset_vcpu_psci(struct kvm_vcpu *vcpu,
+				       struct vcpu_reset_state *reset_state)
+{
+	unsigned long target_pc = reset_state->pc;
+
+	/* Gracefully handle Thumb2 entry point */
+	if (vcpu_mode_is_32bit(vcpu) && (target_pc & 1)) {
+		target_pc &= ~1UL;
+		vcpu_set_thumb(vcpu);
+	}
+
+	/* Propagate caller endianness */
+	if (reset_state->be)
+		kvm_vcpu_set_be(vcpu);
+
+	*vcpu_pc(vcpu) = target_pc;
+
+	/*
+	 * We may come from a state where either a PC update was
+	 * pending (SMC call resulting in PC being increpented to
+	 * skip the SMC) or a pending exception. Make sure we get
+	 * rid of all that, as this cannot be valid out of reset.
+	 *
+	 * Note that clearing the exception mask also clears PC
+	 * updates, but that's an implementation detail, and we
+	 * really want to make it explicit.
+	 */
+	vcpu_clear_flag(vcpu, PENDING_EXCEPTION);
+	vcpu_clear_flag(vcpu, EXCEPT_MASK);
+	vcpu_clear_flag(vcpu, INCREMENT_PC);
+	vcpu_set_reg(vcpu, 0, reset_state->r0);
+}
+
 #endif /* __ARM64_KVM_EMULATE_H__ */
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index b963fd975aaca..10eb7249aa9e8 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -34,18 +34,6 @@
 static u32 __ro_after_init kvm_ipa_limit;
 unsigned int __ro_after_init kvm_host_sve_max_vl;
 
-/*
- * ARMv8 Reset Values
- */
-#define VCPU_RESET_PSTATE_EL1	(PSR_MODE_EL1h | PSR_A_BIT | PSR_I_BIT | \
-				 PSR_F_BIT | PSR_D_BIT)
-
-#define VCPU_RESET_PSTATE_EL2	(PSR_MODE_EL2h | PSR_A_BIT | PSR_I_BIT | \
-				 PSR_F_BIT | PSR_D_BIT)
-
-#define VCPU_RESET_PSTATE_SVC	(PSR_AA32_MODE_SVC | PSR_AA32_A_BIT | \
-				 PSR_AA32_I_BIT | PSR_AA32_F_BIT)
-
 unsigned int __ro_after_init kvm_sve_max_vl;
 
 int __init kvm_arm_init_sve(void)
@@ -191,7 +179,6 @@ void kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_reset_state reset_state;
 	bool loaded;
-	u32 pstate;
 
 	spin_lock(&vcpu->arch.mp_state_lock);
 	reset_state = vcpu->arch.reset_state;
@@ -210,21 +197,8 @@ void kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 		kvm_vcpu_reset_sve(vcpu);
 	}
 
-	if (vcpu_el1_is_32bit(vcpu))
-		pstate = VCPU_RESET_PSTATE_SVC;
-	else if (vcpu_has_nv(vcpu))
-		pstate = VCPU_RESET_PSTATE_EL2;
-	else
-		pstate = VCPU_RESET_PSTATE_EL1;
-
 	/* Reset core registers */
-	memset(vcpu_gp_regs(vcpu), 0, sizeof(*vcpu_gp_regs(vcpu)));
-	memset(&vcpu->arch.ctxt.fp_regs, 0, sizeof(vcpu->arch.ctxt.fp_regs));
-	vcpu->arch.ctxt.spsr_abt = 0;
-	vcpu->arch.ctxt.spsr_und = 0;
-	vcpu->arch.ctxt.spsr_irq = 0;
-	vcpu->arch.ctxt.spsr_fiq = 0;
-	vcpu_gp_regs(vcpu)->pstate = pstate;
+	kvm_reset_vcpu_core(vcpu);
 
 	/* Reset system registers */
 	kvm_reset_sys_regs(vcpu);
@@ -233,36 +207,8 @@ void kvm_reset_vcpu(struct kvm_vcpu *vcpu)
 	 * Additional reset state handling that PSCI may have imposed on us.
 	 * Must be done after all the sys_reg reset.
 	 */
-	if (reset_state.reset) {
-		unsigned long target_pc = reset_state.pc;
-
-		/* Gracefully handle Thumb2 entry point */
-		if (vcpu_mode_is_32bit(vcpu) && (target_pc & 1)) {
-			target_pc &= ~1UL;
-			vcpu_set_thumb(vcpu);
-		}
-
-		/* Propagate caller endianness */
-		if (reset_state.be)
-			kvm_vcpu_set_be(vcpu);
-
-		*vcpu_pc(vcpu) = target_pc;
-
-		/*
-		 * We may come from a state where either a PC update was
-		 * pending (SMC call resulting in PC being increpented to
-		 * skip the SMC) or a pending exception. Make sure we get
-		 * rid of all that, as this cannot be valid out of reset.
-		 *
-		 * Note that clearing the exception mask also clears PC
-		 * updates, but that's an implementation detail, and we
-		 * really want to make it explicit.
-		 */
-		vcpu_clear_flag(vcpu, PENDING_EXCEPTION);
-		vcpu_clear_flag(vcpu, EXCEPT_MASK);
-		vcpu_clear_flag(vcpu, INCREMENT_PC);
-		vcpu_set_reg(vcpu, 0, reset_state.r0);
-	}
+	if (reset_state.reset)
+		kvm_reset_vcpu_psci(vcpu, &reset_state);
 
 	/* Reset timer */
 	kvm_timer_vcpu_reset(vcpu);
-- 
2.39.5

[PATCH v5 4/8] KVM: arm64: Move PSCI helper functions to a shared header

[Fuad Tabba]

Move kvm_psci_valid_affinity() and kvm_psci_narrow_to_32bit() from
psci.c to include/kvm/arm_psci.h, and move psci_affinity_mask() there
too, renaming it kvm_psci_affinity_mask() now that it is no longer
file-local. A follow-up series handles some protected-guest PSCI calls
at EL2 using these helpers.

No functional change intended.

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/psci.c  | 30 +-----------------------------
 include/kvm/arm_psci.h | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c
index 3b5dbe9a0a0ea..e3db84400d1f8 100644
--- a/arch/arm64/kvm/psci.c
+++ b/arch/arm64/kvm/psci.c
@@ -21,16 +21,6 @@
  * as described in ARM document number ARM DEN 0022A.
  */
 
-#define AFFINITY_MASK(level)	~((0x1UL << ((level) * MPIDR_LEVEL_BITS)) - 1)
-
-static unsigned long psci_affinity_mask(unsigned long affinity_level)
-{
-	if (affinity_level <= 3)
-		return MPIDR_HWID_BITMASK & AFFINITY_MASK(affinity_level);
-
-	return 0;
-}
-
 static unsigned long kvm_psci_vcpu_suspend(struct kvm_vcpu *vcpu)
 {
 	/*
@@ -51,12 +41,6 @@ static unsigned long kvm_psci_vcpu_suspend(struct kvm_vcpu *vcpu)
 	return PSCI_RET_SUCCESS;
 }
 
-static inline bool kvm_psci_valid_affinity(struct kvm_vcpu *vcpu,
-					   unsigned long affinity)
-{
-	return !(affinity & ~MPIDR_HWID_BITMASK);
-}
-
 static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 {
 	struct vcpu_reset_state *reset_state;
@@ -135,7 +119,7 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
 		return PSCI_RET_INVALID_PARAMS;
 
 	/* Determine target affinity mask */
-	target_affinity_mask = psci_affinity_mask(lowest_affinity_level);
+	target_affinity_mask = kvm_psci_affinity_mask(lowest_affinity_level);
 	if (!target_affinity_mask)
 		return PSCI_RET_INVALID_PARAMS;
 
@@ -220,18 +204,6 @@ static void kvm_psci_system_suspend(struct kvm_vcpu *vcpu)
 	run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
 }
 
-static void kvm_psci_narrow_to_32bit(struct kvm_vcpu *vcpu)
-{
-	int i;
-
-	/*
-	 * Zero the input registers' upper 32 bits. They will be fully
-	 * zeroed on exit, so we're fine changing them in place.
-	 */
-	for (i = 1; i < 4; i++)
-		vcpu_set_reg(vcpu, i, lower_32_bits(vcpu_get_reg(vcpu, i)));
-}
-
 static unsigned long kvm_psci_check_allowed_function(struct kvm_vcpu *vcpu, u32 fn)
 {
 	/*
diff --git a/include/kvm/arm_psci.h b/include/kvm/arm_psci.h
index cbaec804eb839..f86a006d67136 100644
--- a/include/kvm/arm_psci.h
+++ b/include/kvm/arm_psci.h
@@ -38,6 +38,33 @@ static inline int kvm_psci_version(struct kvm_vcpu *vcpu)
 	return KVM_ARM_PSCI_0_1;
 }
 
+/* Narrow the PSCI register arguments (r1 to r3) to 32 bits. */
+static inline void kvm_psci_narrow_to_32bit(struct kvm_vcpu *vcpu)
+{
+	int i;
+
+	/*
+	 * Zero the input registers' upper 32 bits. They will be fully
+	 * zeroed on exit, so we're fine changing them in place.
+	 */
+	for (i = 1; i < 4; i++)
+		vcpu_set_reg(vcpu, i, lower_32_bits(vcpu_get_reg(vcpu, i)));
+}
+
+static inline bool kvm_psci_valid_affinity(struct kvm_vcpu *vcpu,
+					   unsigned long affinity)
+{
+	return !(affinity & ~MPIDR_HWID_BITMASK);
+}
+
+static inline unsigned long kvm_psci_affinity_mask(unsigned long affinity_level)
+{
+	if (affinity_level <= 3)
+		return MPIDR_HWID_BITMASK &
+			~((0x1UL << (affinity_level * MPIDR_LEVEL_BITS)) - 1);
+
+	return 0;
+}
 
 int kvm_psci_call(struct kvm_vcpu *vcpu);
 
-- 
2.39.5

[PATCH v5 5/8] KVM: arm64: Add host and hypervisor vCPU lookup primitives

[Fuad Tabba]

From: Marc Zyngier <maz@kernel.org>

The nVHE hypervisor repeatedly resolves a host vCPU into the EL2
address space and validates that the loaded hyp vCPU matches it, with
that logic open-coded in each handler.

Add __get_host_hyp_vcpus() and the get_host_hyp_vcpus() macro, which
translate the host vCPU into the hypervisor's address space and, when
pKVM is enabled, also return the loaded hyp vCPU if it matches. If pKVM
is enabled but the loaded hyp vCPU does not correspond to the requested
host vCPU, both the host and hyp vCPU are returned as NULL. Convert
handle___kvm_vcpu_run() to use it.

No functional change intended.

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Co-developed-by: Fuad Tabba <fuad.tabba@linux.dev>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/hyp/nvhe/hyp-main.c | 52 ++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index d3c69de698f48..45d717889f6ea 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -214,14 +214,45 @@ static void handle___pkvm_vcpu_put(struct kvm_cpu_context *host_ctxt)
 		pkvm_put_hyp_vcpu(hyp_vcpu);
 }
 
-static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
+static struct kvm_vcpu *__get_host_hyp_vcpus(struct kvm_vcpu *arg,
+					     struct pkvm_hyp_vcpu **hyp_vcpup)
 {
-	DECLARE_REG(struct kvm_vcpu *, host_vcpu, host_ctxt, 1);
-	int ret;
+	struct kvm_vcpu *host_vcpu = kern_hyp_va(arg);
+	struct pkvm_hyp_vcpu *hyp_vcpu = NULL;
 
 	if (unlikely(is_protected_kvm_enabled())) {
-		struct pkvm_hyp_vcpu *hyp_vcpu = pkvm_get_loaded_hyp_vcpu();
+		hyp_vcpu = pkvm_get_loaded_hyp_vcpu();
 
+		if (!hyp_vcpu || hyp_vcpu->host_vcpu != host_vcpu) {
+			hyp_vcpu = NULL;
+			host_vcpu = NULL;
+		}
+	}
+
+	*hyp_vcpup = hyp_vcpu;
+	return host_vcpu;
+}
+
+#define get_host_hyp_vcpus(ctxt, regnr, hyp_vcpup)			\
+	({								\
+		DECLARE_REG(struct kvm_vcpu *, __vcpu, ctxt, regnr);	\
+		__get_host_hyp_vcpus(__vcpu, hyp_vcpup);		\
+	})
+
+static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
+{
+	struct pkvm_hyp_vcpu *hyp_vcpu;
+	struct kvm_vcpu *host_vcpu;
+	int ret;
+
+	host_vcpu = get_host_hyp_vcpus(host_ctxt, 1, &hyp_vcpu);
+
+	if (!host_vcpu) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (unlikely(hyp_vcpu)) {
 		/*
 		 * KVM (and pKVM) doesn't support SME guests for now, and
 		 * ensures that SME features aren't enabled in pstate when
@@ -233,23 +264,16 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
 			goto out;
 		}
 
-		if (!hyp_vcpu) {
-			ret = -EINVAL;
-			goto out;
-		}
-
 		flush_hyp_vcpu(hyp_vcpu);
 
 		ret = __kvm_vcpu_run(&hyp_vcpu->vcpu);
 
 		sync_hyp_vcpu(hyp_vcpu);
 	} else {
-		struct kvm_vcpu *vcpu = kern_hyp_va(host_vcpu);
-
 		/* The host is fully trusted, run its vCPU directly. */
-		fpsimd_lazy_switch_to_guest(vcpu);
-		ret = __kvm_vcpu_run(vcpu);
-		fpsimd_lazy_switch_to_host(vcpu);
+		fpsimd_lazy_switch_to_guest(host_vcpu);
+		ret = __kvm_vcpu_run(host_vcpu);
+		fpsimd_lazy_switch_to_host(host_vcpu);
 	}
 out:
 	cpu_reg(host_ctxt, 1) =  ret;
-- 
2.39.5

[PATCH v5 6/8] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch

[Fuad Tabba]

From: Marc Zyngier <maz@kernel.org>

The host passes a vgic_v3_cpu_if pointer to the __vgic_v3_save_aprs and
__vgic_v3_restore_vmcr_aprs hypercalls, which EL2 dereferences
wholesale. That exposes the host's full VGIC emulation state to the
hypervisor, against pKVM's isolation goals.

Recover the host vCPU from the supplied cpu_if via container_of() and
copy only vgic_vmcr and the active priority registers between EL2's
hyp-side state and the host vCPU, so EL2 no longer dereferences the
host's vgic_v3_cpu_if directly.

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Co-developed-by: Fuad Tabba <fuad.tabba@linux.dev>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/hyp/nvhe/hyp-main.c | 67 ++++++++++++++++++++++++++++--
 1 file changed, 63 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index 45d717889f6ea..f3233ee343a39 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -7,6 +7,8 @@
 #include <hyp/adjust_pc.h>
 #include <hyp/switch.h>
 
+#include <linux/irqchip/arm-gic-v3.h>
+
 #include <asm/pgtable-types.h>
 #include <asm/kvm_asm.h>
 #include <asm/kvm_emulate.h>
@@ -239,6 +241,16 @@ static struct kvm_vcpu *__get_host_hyp_vcpus(struct kvm_vcpu *arg,
 		__get_host_hyp_vcpus(__vcpu, hyp_vcpup);		\
 	})
 
+#define get_host_hyp_vcpus_from_vgic_v3_cpu_if(ctxt, regnr, hyp_vcpup)		\
+	({									\
+		DECLARE_REG(struct vgic_v3_cpu_if *, cif, ctxt, regnr);\
+		struct kvm_vcpu *__vcpu = container_of(cif,			\
+						       struct kvm_vcpu,		\
+						       arch.vgic_cpu.vgic_v3);	\
+										\
+		__get_host_hyp_vcpus(__vcpu, hyp_vcpup);			\
+	})
+
 static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
 {
 	struct pkvm_hyp_vcpu *hyp_vcpu;
@@ -508,16 +520,63 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)
 
 static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
 {
-	DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
+	struct pkvm_hyp_vcpu *hyp_vcpu;
+	struct kvm_vcpu *host_vcpu;
 
-	__vgic_v3_save_aprs(kern_hyp_va(cpu_if));
+	host_vcpu = get_host_hyp_vcpus_from_vgic_v3_cpu_if(host_ctxt, 1,
+							   &hyp_vcpu);
+	if (!host_vcpu)
+		return;
+
+	if (unlikely(hyp_vcpu)) {
+		struct vgic_v3_cpu_if *hyp_cpu_if, *host_cpu_if;
+		int i;
+
+		hyp_cpu_if = &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
+		__vgic_v3_save_aprs(hyp_cpu_if);
+
+		host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3;
+		host_cpu_if->vgic_vmcr = hyp_cpu_if->vgic_vmcr;
+		for (i = 0; i < ARRAY_SIZE(host_cpu_if->vgic_ap0r); i++) {
+			host_cpu_if->vgic_ap0r[i] = hyp_cpu_if->vgic_ap0r[i];
+			host_cpu_if->vgic_ap1r[i] = hyp_cpu_if->vgic_ap1r[i];
+		}
+	} else {
+		__vgic_v3_save_aprs(&host_vcpu->arch.vgic_cpu.vgic_v3);
+	}
 }
 
 static void handle___vgic_v3_restore_vmcr_aprs(struct kvm_cpu_context *host_ctxt)
 {
-	DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
+	struct pkvm_hyp_vcpu *hyp_vcpu;
+	struct kvm_vcpu *host_vcpu;
 
-	__vgic_v3_restore_vmcr_aprs(kern_hyp_va(cpu_if));
+	host_vcpu = get_host_hyp_vcpus_from_vgic_v3_cpu_if(host_ctxt, 1,
+							   &hyp_vcpu);
+	if (!host_vcpu)
+		return;
+
+	if (unlikely(hyp_vcpu)) {
+		struct vgic_v3_cpu_if *hyp_cpu_if, *host_cpu_if;
+		int i;
+
+		hyp_cpu_if = &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
+		host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3;
+
+		hyp_cpu_if->vgic_vmcr = host_cpu_if->vgic_vmcr;
+		/* Should be a one-off */
+		hyp_cpu_if->vgic_sre = (ICC_SRE_EL1_DIB |
+					ICC_SRE_EL1_DFB |
+					ICC_SRE_EL1_SRE);
+		for (i = 0; i < ARRAY_SIZE(host_cpu_if->vgic_ap0r); i++) {
+			hyp_cpu_if->vgic_ap0r[i] = host_cpu_if->vgic_ap0r[i];
+			hyp_cpu_if->vgic_ap1r[i] = host_cpu_if->vgic_ap1r[i];
+		}
+
+		__vgic_v3_restore_vmcr_aprs(hyp_cpu_if);
+	} else {
+		__vgic_v3_restore_vmcr_aprs(&host_vcpu->arch.vgic_cpu.vgic_v3);
+	}
 }
 
 static void handle___pkvm_init(struct kvm_cpu_context *host_ctxt)
-- 
2.39.5

[PATCH v5 7/8] KVM: arm64: Add primitives to flush/sync the VGIC state at EL2

[Fuad Tabba]

From: Marc Zyngier <maz@kernel.org>

pKVM performs its own world switch for protected VMs but has no
primitives to move the per-vCPU VGIC state between the host and
hypervisor vCPU contexts.

Add flush_hyp_vgic_state() and sync_hyp_vgic_state(). Flush copies
vgic_hcr, the in-use list registers and used_lrs from the host into the
hyp vCPU and pins vgic_sre to a fixed value; sync copies vgic_hcr,
vgic_vmcr and the in-use list registers back. The active priority
registers are handled separately by the save/restore-aprs path.

Bound used_lrs by hyp_gicv3_nr_lr, the cached implemented-LR count,
instead of reading ICH_VTR_EL2 on each entry. That clamps the
host-supplied value and avoids a per-entry sysreg read that is costly
under NV.

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Co-developed-by: Fuad Tabba <fuad.tabba@linux.dev>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/kvm/hyp/nvhe/hyp-main.c | 55 ++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index f3233ee343a39..2db56146ec493 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -104,6 +104,45 @@ static void fpsimd_sve_sync(struct kvm_vcpu *vcpu)
 	*host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED;
 }
 
+static void flush_hyp_vgic_state(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
+	struct vgic_v3_cpu_if *host_cpu_if, *hyp_cpu_if;
+	unsigned int used_lrs, i;
+
+	host_cpu_if	= &host_vcpu->arch.vgic_cpu.vgic_v3;
+	hyp_cpu_if	= &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
+
+	used_lrs	= host_cpu_if->used_lrs;
+	used_lrs	= min(used_lrs, hyp_gicv3_nr_lr);
+
+	hyp_cpu_if->vgic_hcr	= host_cpu_if->vgic_hcr;
+	/* Should be a one-off */
+	hyp_cpu_if->vgic_sre	= (ICC_SRE_EL1_DIB |
+				   ICC_SRE_EL1_DFB |
+				   ICC_SRE_EL1_SRE);
+	hyp_cpu_if->used_lrs	= used_lrs;
+
+	for (i = 0; i < used_lrs; i++)
+		hyp_cpu_if->vgic_lr[i] = host_cpu_if->vgic_lr[i];
+}
+
+static void sync_hyp_vgic_state(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
+	struct vgic_v3_cpu_if *host_cpu_if, *hyp_cpu_if;
+	unsigned int i;
+
+	host_cpu_if	= &host_vcpu->arch.vgic_cpu.vgic_v3;
+	hyp_cpu_if	= &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
+
+	host_cpu_if->vgic_hcr = hyp_cpu_if->vgic_hcr;
+	host_cpu_if->vgic_vmcr = hyp_cpu_if->vgic_vmcr;
+
+	for (i = 0; i < hyp_cpu_if->used_lrs; i++)
+		host_cpu_if->vgic_lr[i] = hyp_cpu_if->vgic_lr[i];
+}
+
 static void flush_debug_state(struct pkvm_hyp_vcpu *hyp_vcpu)
 {
 	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
@@ -152,13 +191,7 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 
 	hyp_vcpu->vcpu.arch.vsesr_el2	= host_vcpu->arch.vsesr_el2;
 
-	hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3 = host_vcpu->arch.vgic_cpu.vgic_v3;
-
-	/* Bound used_lrs by the number of implemented list registers. */
-	hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3.used_lrs =
-		min_t(unsigned int,
-		      hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3.used_lrs,
-		      hyp_gicv3_nr_lr);
+	flush_hyp_vgic_state(hyp_vcpu);
 
 	hyp_vcpu->vcpu.arch.pid = host_vcpu->arch.pid;
 }
@@ -166,9 +199,6 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 {
 	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
-	struct vgic_v3_cpu_if *hyp_cpu_if = &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
-	struct vgic_v3_cpu_if *host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3;
-	unsigned int i;
 
 	fpsimd_sve_sync(&hyp_vcpu->vcpu);
 	sync_debug_state(hyp_vcpu);
@@ -181,10 +211,7 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 
 	host_vcpu->arch.iflags		= hyp_vcpu->vcpu.arch.iflags;
 
-	host_cpu_if->vgic_hcr		= hyp_cpu_if->vgic_hcr;
-	host_cpu_if->vgic_vmcr		= hyp_cpu_if->vgic_vmcr;
-	for (i = 0; i < hyp_cpu_if->used_lrs; ++i)
-		host_cpu_if->vgic_lr[i] = hyp_cpu_if->vgic_lr[i];
+	sync_hyp_vgic_state(hyp_vcpu);
 }
 
 static void handle___pkvm_vcpu_load(struct kvm_cpu_context *host_ctxt)
-- 
2.39.5

[PATCH v5 8/8] KVM: arm64: Implement lazy vCPU state sync for non-protected guests

[Fuad Tabba]

pKVM copies a non-protected guest's register context between the host
and the hypervisor on every world switch, even when the host never
inspects it. Defer the copy: on entry, flush the host context into the
hyp vCPU only when the host marked it dirty (PKVM_HOST_STATE_DIRTY); on
exit, leave it in the hyp vCPU and copy it back only when the host needs
it, via a __pkvm_vcpu_sync_state hypercall or at vcpu put. A protected
guest's context is copied as before, since lazy sync only helps where
the host is trusted to see the guest's registers.

PC and PSTATE are the exception: they are copied back on every exit so
the kvm_exit tracepoint reports the guest's real exit PC, and the run
loop's vcpu_mode_is_bad_32bit() and SError-masking checks evaluate the
guest's current PSTATE rather than the value left by the previous sync.

The host needs the full context when it is about to read it (trap
handling) or write it (the SError injection that writes ESR_EL1). Sync
both from handle_exit_early(), which runs non-preemptible so the loaded
hyp vCPU is stable without a preempt guard.

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 arch/arm64/include/asm/kvm_asm.h   |  1 +
 arch/arm64/include/asm/kvm_host.h  |  2 +
 arch/arm64/kvm/arm.c               |  7 +++
 arch/arm64/kvm/handle_exit.c       | 23 ++++++++
 arch/arm64/kvm/hyp/nvhe/hyp-main.c | 86 ++++++++++++++++++++++++++++--
 5 files changed, 114 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 043495f7fc78b..6e1135b3ded44 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -113,6 +113,7 @@ enum __kvm_host_smccc_func {
 	__KVM_HOST_SMCCC_FUNC___pkvm_finalize_teardown_vm,
 	__KVM_HOST_SMCCC_FUNC___pkvm_vcpu_load,
 	__KVM_HOST_SMCCC_FUNC___pkvm_vcpu_put,
+	__KVM_HOST_SMCCC_FUNC___pkvm_vcpu_sync_state,
 	__KVM_HOST_SMCCC_FUNC___pkvm_tlb_flush_vmid,
 
 	MARKER(__KVM_HOST_SMCCC_FUNC_MAX)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index f3c3c86b3d7fb..1b81bc0a6b374 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1051,6 +1051,8 @@ struct kvm_vcpu_arch {
 #define INCREMENT_PC		__vcpu_single_flag(iflags, BIT(1))
 /* Target EL/MODE (not a single flag, but let's abuse the macro) */
 #define EXCEPT_MASK		__vcpu_single_flag(iflags, GENMASK(3, 1))
+/* Host-set: the hyp flushes the non-protected vCPU state in on entry */
+#define PKVM_HOST_STATE_DIRTY	__vcpu_single_flag(iflags, BIT(4))
 
 /* Helpers to encode exceptions with minimum fuss */
 #define __EXCEPT_MASK_VAL	unpack_vcpu_flag(EXCEPT_MASK)
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 50adfff75be82..ace7b67fe1cc8 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -735,6 +735,10 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 	if (is_protected_kvm_enabled()) {
 		kvm_call_hyp(__vgic_v3_save_aprs, &vcpu->arch.vgic_cpu.vgic_v3);
 		kvm_call_hyp_nvhe(__pkvm_vcpu_put);
+
+		/* __pkvm_vcpu_put implies a sync of the state */
+		if (!kvm_vm_is_protected(vcpu->kvm))
+			vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY);
 	}
 
 	kvm_vcpu_put_debug(vcpu);
@@ -966,6 +970,9 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
 		return ret;
 
 	if (is_protected_kvm_enabled()) {
+		/* Start with the vcpu in a dirty state */
+		if (!kvm_vm_is_protected(vcpu->kvm))
+			vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY);
 		ret = pkvm_create_hyp_vm(kvm);
 		if (ret)
 			return ret;
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 54aedf93c78b6..29108e5c0206e 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -486,9 +486,32 @@ int handle_exit(struct kvm_vcpu *vcpu, int exception_index)
 	}
 }
 
+static void handle_exit_pkvm_state(struct kvm_vcpu *vcpu, int exception_index)
+{
+	int exception_code = ARM_EXCEPTION_CODE(exception_index);
+
+	if (!is_protected_kvm_enabled() || kvm_vm_is_protected(vcpu->kvm))
+		return;
+
+	/*
+	 * Sync the context back when the host will read (trap) or write
+	 * (SError) it. Preempt-off here, so the loaded hyp vCPU is stable.
+	 */
+	if (exception_code == ARM_EXCEPTION_TRAP ||
+	    exception_code == ARM_EXCEPTION_EL1_SERROR ||
+	    ARM_SERROR_PENDING(exception_index)) {
+		kvm_call_hyp_nvhe(__pkvm_vcpu_sync_state);
+		vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY);
+	} else {
+		vcpu_clear_flag(vcpu, PKVM_HOST_STATE_DIRTY);
+	}
+}
+
 /* For exit types that need handling before we can be preempted */
 void handle_exit_early(struct kvm_vcpu *vcpu, int exception_index)
 {
+	handle_exit_pkvm_state(vcpu, exception_index);
+
 	if (ARM_SERROR_PENDING(exception_index)) {
 		if (this_cpu_has_cap(ARM64_HAS_RAS_EXTN)) {
 			u64 disr = kvm_vcpu_get_disr(vcpu);
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index 2db56146ec493..4a8445b636e60 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -143,6 +143,48 @@ static void sync_hyp_vgic_state(struct pkvm_hyp_vcpu *hyp_vcpu)
 		host_cpu_if->vgic_lr[i] = hyp_cpu_if->vgic_lr[i];
 }
 
+static void __copy_vcpu_state(const struct kvm_vcpu *from_vcpu,
+			      struct kvm_vcpu *to_vcpu)
+{
+	int i;
+
+	to_vcpu->arch.ctxt.regs		= from_vcpu->arch.ctxt.regs;
+	to_vcpu->arch.ctxt.spsr_abt	= from_vcpu->arch.ctxt.spsr_abt;
+	to_vcpu->arch.ctxt.spsr_und	= from_vcpu->arch.ctxt.spsr_und;
+	to_vcpu->arch.ctxt.spsr_irq	= from_vcpu->arch.ctxt.spsr_irq;
+	to_vcpu->arch.ctxt.spsr_fiq	= from_vcpu->arch.ctxt.spsr_fiq;
+	to_vcpu->arch.ctxt.fp_regs	= from_vcpu->arch.ctxt.fp_regs;
+
+	/*
+	 * Copy the sysregs, but don't mess with the timer state which
+	 * is directly handled by EL1 and is expected to be preserved.
+	 * enum vcpu_sysreg is sparse: VNCR-mapped registers take values
+	 * derived from their VNCR page offset, so the timer registers do
+	 * not form a contiguous numeric range and must be skipped by name.
+	 */
+	for (i = 1; i < NR_SYS_REGS; i++) {
+		switch (i) {
+		case CNTVOFF_EL2:
+		case CNTV_CVAL_EL0:
+		case CNTV_CTL_EL0:
+		case CNTP_CVAL_EL0:
+		case CNTP_CTL_EL0:
+			continue;
+		}
+		to_vcpu->arch.ctxt.sys_regs[i] = from_vcpu->arch.ctxt.sys_regs[i];
+	}
+}
+
+static void sync_hyp_vcpu_state(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	__copy_vcpu_state(&hyp_vcpu->vcpu, hyp_vcpu->host_vcpu);
+}
+
+static void flush_hyp_vcpu_state(struct pkvm_hyp_vcpu *hyp_vcpu)
+{
+	__copy_vcpu_state(hyp_vcpu->host_vcpu, &hyp_vcpu->vcpu);
+}
+
 static void flush_debug_state(struct pkvm_hyp_vcpu *hyp_vcpu)
 {
 	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
@@ -172,7 +214,17 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 	fpsimd_sve_flush();
 	flush_debug_state(hyp_vcpu);
 
-	hyp_vcpu->vcpu.arch.ctxt	= host_vcpu->arch.ctxt;
+	/*
+	 * If we deal with a non-protected guest and the state is potentially
+	 * dirty (from a host perspective), copy the state back into the hyp
+	 * vcpu.
+	 */
+	if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
+		if (vcpu_get_flag(host_vcpu, PKVM_HOST_STATE_DIRTY))
+			flush_hyp_vcpu_state(hyp_vcpu);
+	} else {
+		hyp_vcpu->vcpu.arch.ctxt = host_vcpu->arch.ctxt;
+	}
 
 	/* __hyp_running_vcpu must be NULL in a guest context. */
 	hyp_vcpu->vcpu.arch.ctxt.__hyp_running_vcpu = NULL;
@@ -203,9 +255,13 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
 	fpsimd_sve_sync(&hyp_vcpu->vcpu);
 	sync_debug_state(hyp_vcpu);
 
-	host_vcpu->arch.ctxt		= hyp_vcpu->vcpu.arch.ctxt;
-
-	host_vcpu->arch.hcr_el2		= hyp_vcpu->vcpu.arch.hcr_el2;
+	if (pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
+		host_vcpu->arch.ctxt = hyp_vcpu->vcpu.arch.ctxt;
+	} else {
+		/* Keep PC (tracepoint) and PSTATE (vcpu_mode_is_bad_32bit) current. */
+		host_vcpu->arch.ctxt.regs.pc = hyp_vcpu->vcpu.arch.ctxt.regs.pc;
+		host_vcpu->arch.ctxt.regs.pstate = hyp_vcpu->vcpu.arch.ctxt.regs.pstate;
+	}
 
 	host_vcpu->arch.fault		= hyp_vcpu->vcpu.arch.fault;
 
@@ -239,8 +295,27 @@ static void handle___pkvm_vcpu_put(struct kvm_cpu_context *host_ctxt)
 {
 	struct pkvm_hyp_vcpu *hyp_vcpu = pkvm_get_loaded_hyp_vcpu();
 
-	if (hyp_vcpu)
+	if (hyp_vcpu) {
+		struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
+
+		if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu) &&
+		    !vcpu_get_flag(host_vcpu, PKVM_HOST_STATE_DIRTY)) {
+			sync_hyp_vcpu_state(hyp_vcpu);
+		}
+
 		pkvm_put_hyp_vcpu(hyp_vcpu);
+	}
+}
+
+static void handle___pkvm_vcpu_sync_state(struct kvm_cpu_context *host_ctxt)
+{
+	struct pkvm_hyp_vcpu *hyp_vcpu;
+
+	hyp_vcpu = pkvm_get_loaded_hyp_vcpu();
+	if (!hyp_vcpu || pkvm_hyp_vcpu_is_protected(hyp_vcpu))
+		return;
+
+	sync_hyp_vcpu_state(hyp_vcpu);
 }
 
 static struct kvm_vcpu *__get_host_hyp_vcpus(struct kvm_vcpu *arg,
@@ -871,6 +946,7 @@ static const hcall_t host_hcall[] = {
 	HANDLE_FUNC(__pkvm_finalize_teardown_vm),
 	HANDLE_FUNC(__pkvm_vcpu_load),
 	HANDLE_FUNC(__pkvm_vcpu_put),
+	HANDLE_FUNC(__pkvm_vcpu_sync_state),
 	HANDLE_FUNC(__pkvm_tlb_flush_vmid),
 };
 
-- 
2.39.5

Re: [PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The WARN_ON_ONCE checks in __vcpu_read_sysreg_vhe and __vcpu_write_sysreg_vhe contain impossible conditions.
--

--- Patch [2]: [PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code ---
commit fbd97ecacd267a88fbcb547ae114d7b3bbe56fb4
Author: Fuad Tabba <fuad.tabba@linux.dev>

KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

This patch renames the host-only implementations to
__vcpu_{read,write}_sysreg_vhe() and turns vcpu_{read,write}_sys_reg() into
context-dispatching wrappers. This allows helpers built on them to be shared
with hyp code.

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 08ba882799d48..c6a416974a61f 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -291,7 +291,7 @@ static void write_sr_to_cpu(enum vcpu_sysreg reg, u64 val)
>  	}
>  }
>  
> -u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
> +u64 __vcpu_read_sysreg_vhe(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
>  {
>  	struct sr_loc loc = {};
>  

[Severity: Low]
Since the new vcpu_read_sys_reg() wrapper in kvm_emulate.h explicitly checks
has_vhe(), does this make the WARN_ON_ONCE(!has_vhe() ...) check inside this
function dead code?

Specifically, just below this declaration:

	locate_register(vcpu, reg, &loc);
	WARN_ON_ONCE(!has_vhe() && loc.loc != SR_LOC_MEMORY);

Is it worth removing the check now that the caller guarantees has_vhe()
is true?

[ ... ]

> @@ -338,7 +338,7 @@ u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
>  	return __vcpu_sys_reg(vcpu, reg);
>  }
>  
> -void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
> +void __vcpu_write_sysreg_vhe(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
>  {
>  	struct sr_loc loc = {};
>  

[Severity: Low]
Similarly, does the WARN_ON_ONCE(!has_vhe() ...) check inside this function
become unreachable now that the wrapper only calls this when has_vhe()
is true?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714101601.4142645-1-fuad.tabba@linux.dev?part=2

Re: [PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

[Vincent Donnefort]

On Tue, Jul 14, 2026 at 11:15:55AM +0100, Fuad Tabba wrote:
> The vcpu_{read,write}_sys_reg() accessors are only valid on a VHE host,
> so helpers built on them such as kvm_vcpu_set_be()/kvm_vcpu_is_be()
> cannot be shared with hyp code. exception.c already wraps them in local
> helpers that pick the host- or hyp-side accessor via has_vhe().
> 
> Rename the host-only implementations to __vcpu_{read,write}_sysreg_vhe()
> and turn vcpu_{read,write}_sys_reg() into the context-dispatching
> wrappers, so every caller gets the version valid in any context and a
> follow-up series can share that emulation code at EL2.
> 
> No functional change intended.
> 
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>

> ---
> v5:
>   - Named the wrappers vcpu_{read,write}_sys_reg() and renamed the
>     host-only implementations to __vcpu_{read,write}_sysreg_vhe(), rather
>     than introducing a kvm_vcpu_ prefix. (Oliver)
>   - Dropped Vincent's Reviewed-by since the patch changed materially.
> 
>  arch/arm64/include/asm/kvm_emulate.h | 20 ++++++++++++++++
>  arch/arm64/include/asm/kvm_host.h    |  4 ++--
>  arch/arm64/kvm/hyp/exception.c       | 34 ++++++++--------------------
>  arch/arm64/kvm/sys_regs.c            |  4 ++--
>  4 files changed, 33 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 5bf3d7e1d92c7..429bda6f48d94 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -506,6 +506,26 @@ static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
>  	return __vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
>  }
>  
> +/*
> + * __vcpu_*_sysreg_vhe() are only valid on a VHE host; wrap them so the same
> + * call site also works at EL2 under nVHE.
> + */
> +static inline u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
> +{
> +	if (has_vhe())
> +		return __vcpu_read_sysreg_vhe(vcpu, reg);
> +
> +	return __vcpu_sys_reg(vcpu, reg);
> +}
> +
> +static inline void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
> +{
> +	if (has_vhe())
> +		__vcpu_write_sysreg_vhe(vcpu, val, reg);
> +	else
> +		__vcpu_assign_sys_reg(vcpu, reg, val);
> +}
> +
>  static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
>  {
>  	if (vcpu_mode_is_32bit(vcpu)) {
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index bae2c4f92ef5c..f3c3c86b3d7fb 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -1215,8 +1215,8 @@ u64 kvm_vcpu_apply_reg_masks(const struct kvm_vcpu *, enum vcpu_sysreg, u64);
>  		__v;							\
>  	})
>  
> -u64 vcpu_read_sys_reg(const struct kvm_vcpu *, enum vcpu_sysreg);
> -void vcpu_write_sys_reg(struct kvm_vcpu *, u64, enum vcpu_sysreg);
> +u64 __vcpu_read_sysreg_vhe(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg);
> +void __vcpu_write_sysreg_vhe(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg);
>  
>  struct kvm_vm_stat {
>  	struct kvm_vm_stat_generic generic;
> diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
> index bef40ddb16dbc..754e2dc1df54a 100644
> --- a/arch/arm64/kvm/hyp/exception.c
> +++ b/arch/arm64/kvm/hyp/exception.c
> @@ -20,22 +20,6 @@
>  #error Hypervisor code only!
>  #endif
>  
> -static inline u64 __vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)
> -{
> -	if (has_vhe())
> -		return vcpu_read_sys_reg(vcpu, reg);
> -
> -	return __vcpu_sys_reg(vcpu, reg);
> -}
> -
> -static inline void __vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
> -{
> -	if (has_vhe())
> -		vcpu_write_sys_reg(vcpu, val, reg);
> -	else
> -		__vcpu_assign_sys_reg(vcpu, reg, val);
> -}
> -
>  static void __vcpu_write_spsr(struct kvm_vcpu *vcpu, unsigned long target_mode,
>  			      u64 val)
>  {
> @@ -101,14 +85,14 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
>  
>  	switch (target_mode) {
>  	case PSR_MODE_EL1h:
> -		vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL1);
> -		sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> -		__vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
> +		vbar = vcpu_read_sys_reg(vcpu, VBAR_EL1);
> +		sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> +		vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
>  		break;
>  	case PSR_MODE_EL2h:
> -		vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL2);
> -		sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL2);
> -		__vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
> +		vbar = vcpu_read_sys_reg(vcpu, VBAR_EL2);
> +		sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL2);
> +		vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
>  		break;
>  	default:
>  		/* Don't do that */
> @@ -185,7 +169,7 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
>   */
>  static unsigned long get_except32_cpsr(struct kvm_vcpu *vcpu, u32 mode)
>  {
> -	u32 sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> +	u32 sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
>  	unsigned long old, new;
>  
>  	old = *vcpu_cpsr(vcpu);
> @@ -281,7 +265,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
>  {
>  	unsigned long spsr = *vcpu_cpsr(vcpu);
>  	bool is_thumb = (spsr & PSR_AA32_T_BIT);
> -	u32 sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> +	u32 sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
>  	u32 return_address;
>  
>  	*vcpu_cpsr(vcpu) = get_except32_cpsr(vcpu, mode);
> @@ -305,7 +289,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
>  	if (sctlr & (1 << 13))
>  		vect_offset += 0xffff0000;
>  	else /* always have security exceptions */
> -		vect_offset += __vcpu_read_sys_reg(vcpu, VBAR_EL1);
> +		vect_offset += vcpu_read_sys_reg(vcpu, VBAR_EL1);
>  
>  	*vcpu_pc(vcpu) = vect_offset;
>  }
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 08ba882799d48..c6a416974a61f 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -291,7 +291,7 @@ static void write_sr_to_cpu(enum vcpu_sysreg reg, u64 val)
>  	}
>  }
>  
> -u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
> +u64 __vcpu_read_sysreg_vhe(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
>  {
>  	struct sr_loc loc = {};
>  
> @@ -338,7 +338,7 @@ u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
>  	return __vcpu_sys_reg(vcpu, reg);
>  }
>  
> -void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
> +void __vcpu_write_sysreg_vhe(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
>  {
>  	struct sr_loc loc = {};
>  
> -- 
> 2.39.5
> 

Re: [PATCH v5 4/8] KVM: arm64: Move PSCI helper functions to a shared header

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The header `include/kvm/arm_psci.h` uses architecture-specific functions (`vcpu_get_reg`, `vcpu_set_reg`) without including the necessary header `<asm/kvm_emulate.h>`, breaking header self-containment.
--

--- Patch [4]: [PATCH v5 4/8] KVM: arm64: Move PSCI helper functions to a shared header ---
Note: The format of this report has been altered due to recitation restrictions. The original patch code is not quoted directly.

commit 0c74ab23a0a20b7f1a788c6ac3e03217bd5099cd
Author: Fuad Tabba <fuad.tabba@linux.dev>

KVM: arm64: Move PSCI helper functions to a shared header

This commit moves several PSCI helper functions out of arch/arm64/kvm/psci.c
and into the shared include/kvm/arm_psci.h header. This allows the functions
to be reused, specifically for a follow-up series that handles protected-guest
PSCI calls at EL2.

[Severity: Low]
Does include/kvm/arm_psci.h need to explicitly include <asm/kvm_emulate.h>?

Looking at the moved inline function kvm_psci_narrow_to_32bit(), it relies
on vcpu_get_reg() and vcpu_set_reg(). These functions are provided by
<asm/kvm_emulate.h>, but that header does not appear to be included in 
include/kvm/arm_psci.h.

Will this cause build failures if a new compilation unit (such as the EL2 
pKVM follow-up series mentioned in the commit message) includes 
<kvm/arm_psci.h> directly without having already included <asm/kvm_emulate.h>?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714101601.4142645-1-fuad.tabba@linux.dev?part=4

Re: [PATCH v5 4/8] KVM: arm64: Move PSCI helper functions to a shared header

[Fuad Tabba]

On Tue, 14 Jul 2026 at 12:10, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The header `include/kvm/arm_psci.h` uses architecture-specific functions (`vcpu_get_reg`, `vcpu_set_reg`) without including the necessary header `<asm/kvm_emulate.h>`, breaking header self-containment.

https://lore.kernel.org/all/CA+EHjTwpB2fmoYGfkP5RVZcB4sgbVPQ0=QR5dyUJnpQ_VQa+pA@mail.gmail.com/

/fuad

> --
>
> --- Patch [4]: [PATCH v5 4/8] KVM: arm64: Move PSCI helper functions to a shared header ---
> Note: The format of this report has been altered due to recitation restrictions. The original patch code is not quoted directly.
>
> commit 0c74ab23a0a20b7f1a788c6ac3e03217bd5099cd
> Author: Fuad Tabba <fuad.tabba@linux.dev>
>
> KVM: arm64: Move PSCI helper functions to a shared header
>
> This commit moves several PSCI helper functions out of arch/arm64/kvm/psci.c
> and into the shared include/kvm/arm_psci.h header. This allows the functions
> to be reused, specifically for a follow-up series that handles protected-guest
> PSCI calls at EL2.
>
> [Severity: Low]
> Does include/kvm/arm_psci.h need to explicitly include <asm/kvm_emulate.h>?
>
> Looking at the moved inline function kvm_psci_narrow_to_32bit(), it relies
> on vcpu_get_reg() and vcpu_set_reg(). These functions are provided by
> <asm/kvm_emulate.h>, but that header does not appear to be included in
> include/kvm/arm_psci.h.
>
> Will this cause build failures if a new compilation unit (such as the EL2
> pKVM follow-up series mentioned in the commit message) includes
> <kvm/arm_psci.h> directly without having already included <asm/kvm_emulate.h>?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260714101601.4142645-1-fuad.tabba@linux.dev?part=4

Re: [PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

[Fuad Tabba]

On Tue, 14 Jul 2026 at 11:43, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The WARN_ON_ONCE checks in __vcpu_read_sysreg_vhe and __vcpu_write_sysreg_vhe contain impossible conditions.
> --

That WARN_ON_ONCE() is unchanged from the current
vcpu_read_sys_reg()/vcpu_write_sys_reg() in mainline; this patch only
renames them, no functional change. It's now unreachable via the
wrapper, but since the patch exports the symbol it still guards a
direct non-VHE caller. I'd keep it and remove it in a separate patch
after this series if we decide it's worth it.

/fuad

> --- Patch [2]: [PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code ---
> commit fbd97ecacd267a88fbcb547ae114d7b3bbe56fb4
> Author: Fuad Tabba <fuad.tabba@linux.dev>
>
> KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code
>
> This patch renames the host-only implementations to
> __vcpu_{read,write}_sysreg_vhe() and turns vcpu_{read,write}_sys_reg() into
> context-dispatching wrappers. This allows helpers built on them to be shared
> with hyp code.
>
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 08ba882799d48..c6a416974a61f 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -291,7 +291,7 @@ static void write_sr_to_cpu(enum vcpu_sysreg reg, u64 val)
> >       }
> >  }
> >
> > -u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
> > +u64 __vcpu_read_sysreg_vhe(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
> >  {
> >       struct sr_loc loc = {};
> >
>
> [Severity: Low]
> Since the new vcpu_read_sys_reg() wrapper in kvm_emulate.h explicitly checks
> has_vhe(), does this make the WARN_ON_ONCE(!has_vhe() ...) check inside this
> function dead code?
>
> Specifically, just below this declaration:
>
>         locate_register(vcpu, reg, &loc);
>         WARN_ON_ONCE(!has_vhe() && loc.loc != SR_LOC_MEMORY);
>
> Is it worth removing the check now that the caller guarantees has_vhe()
> is true?
>
> [ ... ]
>
> > @@ -338,7 +338,7 @@ u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg reg)
> >       return __vcpu_sys_reg(vcpu, reg);
> >  }
> >
> > -void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
> > +void __vcpu_write_sysreg_vhe(struct kvm_vcpu *vcpu, u64 val, enum vcpu_sysreg reg)
> >  {
> >       struct sr_loc loc = {};
> >
>
> [Severity: Low]
> Similarly, does the WARN_ON_ONCE(!has_vhe() ...) check inside this function
> become unreachable now that the wrapper only calls this when has_vhe()
> is true?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260714101601.4142645-1-fuad.tabba@linux.dev?part=2

Re: [PATCH v5 6/8] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] handle___vgic_v3_save_aprs erroneously overwrites the host's vgic_vmcr with a stale EL2 value.

Pre-existing issues:
- [Critical] Incomplete fix leaves identical hypervisor memory corruption vulnerability in GICv5 hypercalls.
- [Critical] The `handle___kvm_adjust_pc` hypercall blindly dereferences a host-provided `vcpu` pointer via `kern_hyp_va()` without validation.
--

--- Patch [6]: [PATCH v5 6/8] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch ---
commit bad346f74febb306b17aa647c2da5c6747345fe8
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch

The host passes a vgic_v3_cpu_if pointer to the __vgic_v3_save_aprs and
__vgic_v3_restore_vmcr_aprs hypercalls, which EL2 dereferences wholesale.
This patch copies only vgic_vmcr and the active priority registers between
EL2's hyp-side state and the host vCPU to minimise exposure.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 45d717889f6ea..f3233ee343a39 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
[ ... ]
> @@ -508,16 +520,63 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)
>  
>  static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
>  {
> -	DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
> +	struct pkvm_hyp_vcpu *hyp_vcpu;
> +	struct kvm_vcpu *host_vcpu;
>  
> -	__vgic_v3_save_aprs(kern_hyp_va(cpu_if));
> +	host_vcpu = get_host_hyp_vcpus_from_vgic_v3_cpu_if(host_ctxt, 1,
> +							   &hyp_vcpu);
> +	if (!host_vcpu)
> +		return;
> +
> +	if (unlikely(hyp_vcpu)) {
> +		struct vgic_v3_cpu_if *hyp_cpu_if, *host_cpu_if;
> +		int i;
> +
> +		hyp_cpu_if = &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
> +		__vgic_v3_save_aprs(hyp_cpu_if);
> +
> +		host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3;
> +		host_cpu_if->vgic_vmcr = hyp_cpu_if->vgic_vmcr;

[Severity: High]
Does this overwrite the host's vgic_vmcr with a stale EL2 value?

When the guest performs an MMIO write to the GICv2 VMCR (emulated on a GICv3
host), the trap is handled at EL1, updating
host_vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr.

Later, if the vCPU is preempted or userspace schedules out,
__vgic_v3_save_aprs is invoked. Because __vgic_v3_save_aprs() does not read
VMCR from hardware, hyp_cpu_if->vgic_vmcr contains the stale value from the
previous guest exit.

This would silently clobber and destroy the EL1's MMIO emulation updates.

[ ... ]

This is a pre-existing issue, but I noticed a similar pattern in
handle___kvm_adjust_pc() in the same file:

arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___kvm_adjust_pc() {
        ...
        __kvm_adjust_pc(kern_hyp_va(vcpu));
}

[Severity: Critical]
Could this allow an EL1 host to corrupt hypervisor memory?

An EL1 host can call the __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc hypercall
and provide an arbitrary TTBR1 pointer. EL2's handle___kvm_adjust_pc()
applies kern_hyp_va() without bounds-checking or validation.

When __kvm_adjust_pc() is executed:

arch/arm64/kvm/hyp/exception.c:__kvm_adjust_pc() {
        if (vcpu_get_flag(vcpu, PENDING_EXCEPTION)) {
        ...

This dereferences the unvalidated pointer to read flags and modify memory,
giving EL1 the ability to increment 64-bit values in EL2 memory.


This is also a pre-existing issue, but a similar pattern remains in the
GICv5 counterparts:

arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___vgic_v5_save_apr() {
        ...
        __vgic_v5_save_apr(kern_hyp_va(cpu_if));
}

arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___vgic_v5_restore_vmcr_apr() {
        ...
        __vgic_v5_restore_vmcr_apr(kern_hyp_va(cpu_if));
}

[Severity: Critical]
Does this leave an identical memory corruption regression?

An EL1 host can call these hypercalls with an arbitrary TTBR1 pointer,
which is directly dereferenced without validation, leading to arbitrary
memory read/write against EL2.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714101601.4142645-1-fuad.tabba@linux.dev?part=6

Re: [PATCH v5 6/8] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch

[Fuad Tabba]

On Tue, 14 Jul 2026 at 12:53, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [High] handle___vgic_v3_save_aprs erroneously overwrites the host's vgic_vmcr with a stale EL2 value.

__vgic_v3_save_aprs()  reads only AP0R/AP1R, so the copied  vgic_vmcr
is the host's own value from the prior  restore_vmcr_aprs: redundant,
not stale.

> Pre-existing issues:
> - [Critical] Incomplete fix leaves identical hypervisor memory corruption vulnerability in GICv5 hypercalls.

GICv5 is not supported under pKVM (vgic_v5_probe  ->  skip_v5), so
those handlers are unreachable in protected mode.

> - [Critical] The `handle___kvm_adjust_pc` hypercall blindly dereferences a host-provided `vcpu` pointer via `kern_hyp_va()` without validation.

Pre-existing upstream code, untouched by this series, so it's out of
scope here; the pointer validation is being addressed separately.

/fuad

> --
>
> --- Patch [6]: [PATCH v5 6/8] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch ---
> commit bad346f74febb306b17aa647c2da5c6747345fe8
> Author: Marc Zyngier <maz@kernel.org>
>
> KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch
>
> The host passes a vgic_v3_cpu_if pointer to the __vgic_v3_save_aprs and
> __vgic_v3_restore_vmcr_aprs hypercalls, which EL2 dereferences wholesale.
> This patch copies only vgic_vmcr and the active priority registers between
> EL2's hyp-side state and the host vCPU to minimise exposure.
>
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index 45d717889f6ea..f3233ee343a39 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> [ ... ]
> > @@ -508,16 +520,63 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)
> >
> >  static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
> >  {
> > -     DECLARE_REG(struct vgic_v3_cpu_if *, cpu_if, host_ctxt, 1);
> > +     struct pkvm_hyp_vcpu *hyp_vcpu;
> > +     struct kvm_vcpu *host_vcpu;
> >
> > -     __vgic_v3_save_aprs(kern_hyp_va(cpu_if));
> > +     host_vcpu = get_host_hyp_vcpus_from_vgic_v3_cpu_if(host_ctxt, 1,
> > +                                                        &hyp_vcpu);
> > +     if (!host_vcpu)
> > +             return;
> > +
> > +     if (unlikely(hyp_vcpu)) {
> > +             struct vgic_v3_cpu_if *hyp_cpu_if, *host_cpu_if;
> > +             int i;
> > +
> > +             hyp_cpu_if = &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
> > +             __vgic_v3_save_aprs(hyp_cpu_if);
> > +
> > +             host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3;
> > +             host_cpu_if->vgic_vmcr = hyp_cpu_if->vgic_vmcr;
>
> [Severity: High]
> Does this overwrite the host's vgic_vmcr with a stale EL2 value?
>
> When the guest performs an MMIO write to the GICv2 VMCR (emulated on a GICv3
> host), the trap is handled at EL1, updating
> host_vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr.
>
> Later, if the vCPU is preempted or userspace schedules out,
> __vgic_v3_save_aprs is invoked. Because __vgic_v3_save_aprs() does not read
> VMCR from hardware, hyp_cpu_if->vgic_vmcr contains the stale value from the
> previous guest exit.
>
> This would silently clobber and destroy the EL1's MMIO emulation updates.
>
> [ ... ]
>
> This is a pre-existing issue, but I noticed a similar pattern in
> handle___kvm_adjust_pc() in the same file:
>
> arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___kvm_adjust_pc() {
>         ...
>         __kvm_adjust_pc(kern_hyp_va(vcpu));
> }
>
> [Severity: Critical]
> Could this allow an EL1 host to corrupt hypervisor memory?
>
> An EL1 host can call the __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc hypercall
> and provide an arbitrary TTBR1 pointer. EL2's handle___kvm_adjust_pc()
> applies kern_hyp_va() without bounds-checking or validation.
>
> When __kvm_adjust_pc() is executed:
>
> arch/arm64/kvm/hyp/exception.c:__kvm_adjust_pc() {
>         if (vcpu_get_flag(vcpu, PENDING_EXCEPTION)) {
>         ...
>
> This dereferences the unvalidated pointer to read flags and modify memory,
> giving EL1 the ability to increment 64-bit values in EL2 memory.
>
>
> This is also a pre-existing issue, but a similar pattern remains in the
> GICv5 counterparts:
>
> arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___vgic_v5_save_apr() {
>         ...
>         __vgic_v5_save_apr(kern_hyp_va(cpu_if));
> }
>
> arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___vgic_v5_restore_vmcr_apr() {
>         ...
>         __vgic_v5_restore_vmcr_apr(kern_hyp_va(cpu_if));
> }
>
> [Severity: Critical]
> Does this leave an identical memory corruption regression?
>
> An EL1 host can call these hypercalls with an arbitrary TTBR1 pointer,
> which is directly dereferenced without validation, leading to arbitrary
> memory read/write against EL2.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260714101601.4142645-1-fuad.tabba@linux.dev?part=6

Re: [PATCH v5 7/8] KVM: arm64: Add primitives to flush/sync the VGIC state at EL2

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing its_vpe copy in flush_hyp_vgic_state breaks GICv4 virtual LPI injection

Pre-existing issues:
- [High] KVM_SET_ONE_REG updates to GICv3 VMCR are silently dropped under pKVM during vcpu_put()
--

--- Patch [7]: [PATCH v5 7/8] KVM: arm64: Add primitives to flush/sync the VGIC state at EL2 ---
commit 5a3c12abf480537e61537f1e45492ee3e349846c
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Add primitives to flush/sync the VGIC state at EL2

pKVM performs its own world switch for protected VMs but has no primitives
to move the per-vCPU VGIC state between the host and hypervisor vCPU
contexts. Add flush_hyp_vgic_state() and sync_hyp_vgic_state().

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index f3233ee343a39..2db56146ec493 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -104,6 +104,45 @@ static void fpsimd_sve_sync(struct kvm_vcpu *vcpu)
>  	*host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED;
>  }
>  
> +static void flush_hyp_vgic_state(struct pkvm_hyp_vcpu *hyp_vcpu)
> +{
> +	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
> +	struct vgic_v3_cpu_if *host_cpu_if, *hyp_cpu_if;
> +	unsigned int used_lrs, i;
> +
> +	host_cpu_if	= &host_vcpu->arch.vgic_cpu.vgic_v3;
> +	hyp_cpu_if	= &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
> +
> +	used_lrs	= host_cpu_if->used_lrs;
> +	used_lrs	= min(used_lrs, hyp_gicv3_nr_lr);
> +
> +	hyp_cpu_if->vgic_hcr	= host_cpu_if->vgic_hcr;
> +	/* Should be a one-off */
> +	hyp_cpu_if->vgic_sre	= (ICC_SRE_EL1_DIB |
> +				   ICC_SRE_EL1_DFB |
> +				   ICC_SRE_EL1_SRE);
> +	hyp_cpu_if->used_lrs	= used_lrs;
> +
> +	for (i = 0; i < used_lrs; i++)
> +		hyp_cpu_if->vgic_lr[i] = host_cpu_if->vgic_lr[i];
> +}

[Severity: High]
Does this code miss copying the its_vpe struct?

Previously, this was a full struct assignment. By explicitly copying fields
instead, its_vpe is omitted, leaving its_vpe.its_vm as NULL in the
hypervisor vCPU state.

When the guest runs, __vgic_v3_activate_traps() executes at EL2 and relies
on this field:

arch/arm64/kvm/hyp/vgic-v3-sr.c:__vgic_v3_activate_traps() {
    if (static_branch_unlikely(&vgic_v3_cpuif_trap) ||
        cpu_if->its_vpe.its_vm || !cpu_if->vgic_sre)
        write_gicreg(vgic_ich_hcr_trap_bits() | ICH_HCR_EL2_En, ICH_HCR_EL2);
    ...
}

Since its_vm is NULL, ICH_HCR_EL2_En is not set. Could this break GICv4
direct virtual LPI injection for non-protected guests?

[ ... ]

> @@ -166,9 +199,6 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
>  static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
>  {
>  	struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
> -	struct vgic_v3_cpu_if *hyp_cpu_if = &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
> -	struct vgic_v3_cpu_if *host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3;
> -	unsigned int i;
>  
>  	fpsimd_sve_sync(&hyp_vcpu->vcpu);
>  	sync_debug_state(hyp_vcpu);
> @@ -181,10 +211,7 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
>  
>  	host_vcpu->arch.iflags		= hyp_vcpu->vcpu.arch.iflags;
>  
> -	host_cpu_if->vgic_hcr		= hyp_cpu_if->vgic_hcr;
> -	host_cpu_if->vgic_vmcr		= hyp_cpu_if->vgic_vmcr;
> -	for (i = 0; i < hyp_cpu_if->used_lrs; ++i)
> -		host_cpu_if->vgic_lr[i] = hyp_cpu_if->vgic_lr[i];
> +	sync_hyp_vgic_state(hyp_vcpu);
>  }

[Severity: High]
This is a pre-existing issue, but does handle___vgic_v3_save_aprs()
silently drop userspace modifications to the GICv3 VMCR?

When userspace sets the VMCR register, vcpu_load() first calls
handle___vgic_v3_restore_vmcr_aprs(), which copies host_cpu_if->vgic_vmcr
to the hyp vCPU. KVM_SET_ONE_REG then updates the host copy of the register.

Finally, vcpu_put() calls handle___vgic_v3_save_aprs():

arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___vgic_v3_save_aprs() {
    ...
    hyp_cpu_if = &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
    ...
    host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3;
    host_cpu_if->vgic_vmcr = hyp_cpu_if->vgic_vmcr;
}

Doesn't this unconditionally overwrite the new host value (set by
KVM_SET_ONE_REG) with the old value stored in the hyp vCPU at load time,
potentially breaking VM migration and state restoration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714101601.4142645-1-fuad.tabba@linux.dev?part=7

Re: [PATCH v5 8/8] KVM: arm64: Implement lazy vCPU state sync for non-protected guests

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Removal of `hcr_el2` synchronization back to the host prevents clearance of `HCR_VSE`, causing infinite virtual SError storms in the guest.
- [High] Lazy state sync skips synchronization and clears `PKVM_HOST_STATE_DIRTY` on IRQ and IL exits, silently discarding host-side context modifications.

Pre-existing issues:
- [Critical] Protected guest register state is unconditionally leaked to and overwritten by untrusted host memory (`host_vcpu->arch.ctxt`) on every world switch.
--

--- Patch [8]: [PATCH v5 8/8] KVM: arm64: Implement lazy vCPU state sync for non-protected guests ---
commit e68e83fa70438b2538d1ea836d971c6d52ffb760
Author: Fuad Tabba <fuad.tabba@linux.dev>

KVM: arm64: Implement lazy vCPU state sync for non-protected guests

pKVM copies a non-protected guest's register context between the host
and the hypervisor on every world switch, even when the host never
inspects it. Defer the copy: on entry, flush the host context into the
hyp vCPU only when the host marked it dirty; on exit, leave it in the
hyp vCPU and copy it back only when the host needs it.

> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -486,9 +486,32 @@ int handle_exit(struct kvm_vcpu *vcpu, int exception_index)

[ ... ]

> +static void handle_exit_pkvm_state(struct kvm_vcpu *vcpu, int exception_index)
> +{

[ ... ]

> +	if (exception_code == ARM_EXCEPTION_TRAP ||
> +	    exception_code == ARM_EXCEPTION_EL1_SERROR ||
> +	    ARM_SERROR_PENDING(exception_index)) {
> +		kvm_call_hyp_nvhe(__pkvm_vcpu_sync_state);
> +		vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY);
> +	} else {
> +		vcpu_clear_flag(vcpu, PKVM_HOST_STATE_DIRTY);
> +	}
> +}

[Severity: High]
If a non-protected guest exits due to an IRQ (such as a PMU overflow), this
clears PKVM_HOST_STATE_DIRTY. If the host kernel then handles the IRQ and
modifies the guest context (for example, setting PMOVSSET_EL0 for the
overflow or applying a reset like KVM_REQ_SUSPEND), these updates appear to
be lost.

On the next entry, flush_hyp_vcpu() will skip copying the host state because
the dirty flag is clear. Then, on a subsequent exit or vcpu_put,
handle___pkvm_vcpu_put() will reverse-sync the stale hyp state back over the
host state. Does this clear flag path silently discard host-side context
modifications?

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -172,7 +214,17 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
>  	fpsimd_sve_flush();
>  	flush_debug_state(hyp_vcpu);
>  
> -	hyp_vcpu->vcpu.arch.ctxt	= host_vcpu->arch.ctxt;
> +	/*
> +	 * If we deal with a non-protected guest and the state is potentially
> +	 * dirty (from a host perspective), copy the state back into the hyp
> +	 * vcpu.
> +	 */
> +	if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
> +		if (vcpu_get_flag(host_vcpu, PKVM_HOST_STATE_DIRTY))
> +			flush_hyp_vcpu_state(hyp_vcpu);
> +	} else {
> +		hyp_vcpu->vcpu.arch.ctxt = host_vcpu->arch.ctxt;
> +	}

[Severity: Critical]
This is a pre-existing issue, but doesn't this unconditionally overwrite the
protected guest's private EL2 registers with the values from the untrusted
host memory? By copying host_vcpu->arch.ctxt into the hyp_vcpu on entry,
could the host arbitrarily modify a protected VM's state before execution?

> @@ -203,9 +255,13 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
>  	fpsimd_sve_sync(&hyp_vcpu->vcpu);
>  	sync_debug_state(hyp_vcpu);
>  
> -	host_vcpu->arch.ctxt		= hyp_vcpu->vcpu.arch.ctxt;
> -
> -	host_vcpu->arch.hcr_el2		= hyp_vcpu->vcpu.arch.hcr_el2;
> +	if (pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
> +		host_vcpu->arch.ctxt = hyp_vcpu->vcpu.arch.ctxt;

[Severity: Critical]
This is also a pre-existing issue, but leaking the protected guest's
register state into the untrusted host memory seems to violate pKVM's
confidentiality guarantees. The untrusted host EL1 can read all private EL2
registers via its mapped kvm_vcpu structure. Should this copy be avoided for
protected VMs?

[Severity: High]
By removing the assignment that copies hcr_el2 back to the host, do we break
virtual SError injection?

When the guest takes a virtual SError, the hardware clears the VSE bit in
the hyp vCPU's hcr_el2. Because we no longer sync this back in
sync_hyp_vcpu(), the host's copy remains set. On the next entry,
flush_hyp_vcpu() will read the stale host HCR_VSE bit and set it again,
which seems like it would trap the guest in an infinite virtual SError
storm.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714101601.4142645-1-fuad.tabba@linux.dev?part=8

Re: [PATCH v5 7/8] KVM: arm64: Add primitives to flush/sync the VGIC state at EL2

[Fuad Tabba]

On Tue, 14 Jul 2026 at 13:20, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Missing its_vpe copy in flush_hyp_vgic_state breaks GICv4 virtual LPI injection

pKVM has no vLPI, so  its_vm  is NULL either way; and  En  comes from
the copied  vgic_hcr  anyway.

>
> Pre-existing issues:
> - [High] KVM_SET_ONE_REG updates to GICv3 VMCR are silently dropped under pKVM during vcpu_put()

The uaccess holds  kvm_trylock_all_vcpus , so it can't run between load and put.

/fuad

> --
>
> --- Patch [7]: [PATCH v5 7/8] KVM: arm64: Add primitives to flush/sync the VGIC state at EL2 ---
> commit 5a3c12abf480537e61537f1e45492ee3e349846c
> Author: Marc Zyngier <maz@kernel.org>
>
> KVM: arm64: Add primitives to flush/sync the VGIC state at EL2
>
> pKVM performs its own world switch for protected VMs but has no primitives
> to move the per-vCPU VGIC state between the host and hypervisor vCPU
> contexts. Add flush_hyp_vgic_state() and sync_hyp_vgic_state().
>
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > index f3233ee343a39..2db56146ec493 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > @@ -104,6 +104,45 @@ static void fpsimd_sve_sync(struct kvm_vcpu *vcpu)
> >       *host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED;
> >  }
> >
> > +static void flush_hyp_vgic_state(struct pkvm_hyp_vcpu *hyp_vcpu)
> > +{
> > +     struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
> > +     struct vgic_v3_cpu_if *host_cpu_if, *hyp_cpu_if;
> > +     unsigned int used_lrs, i;
> > +
> > +     host_cpu_if     = &host_vcpu->arch.vgic_cpu.vgic_v3;
> > +     hyp_cpu_if      = &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
> > +
> > +     used_lrs        = host_cpu_if->used_lrs;
> > +     used_lrs        = min(used_lrs, hyp_gicv3_nr_lr);
> > +
> > +     hyp_cpu_if->vgic_hcr    = host_cpu_if->vgic_hcr;
> > +     /* Should be a one-off */
> > +     hyp_cpu_if->vgic_sre    = (ICC_SRE_EL1_DIB |
> > +                                ICC_SRE_EL1_DFB |
> > +                                ICC_SRE_EL1_SRE);
> > +     hyp_cpu_if->used_lrs    = used_lrs;
> > +
> > +     for (i = 0; i < used_lrs; i++)
> > +             hyp_cpu_if->vgic_lr[i] = host_cpu_if->vgic_lr[i];
> > +}
>
> [Severity: High]
> Does this code miss copying the its_vpe struct?
>
> Previously, this was a full struct assignment. By explicitly copying fields
> instead, its_vpe is omitted, leaving its_vpe.its_vm as NULL in the
> hypervisor vCPU state.
>
> When the guest runs, __vgic_v3_activate_traps() executes at EL2 and relies
> on this field:
>
> arch/arm64/kvm/hyp/vgic-v3-sr.c:__vgic_v3_activate_traps() {
>     if (static_branch_unlikely(&vgic_v3_cpuif_trap) ||
>         cpu_if->its_vpe.its_vm || !cpu_if->vgic_sre)
>         write_gicreg(vgic_ich_hcr_trap_bits() | ICH_HCR_EL2_En, ICH_HCR_EL2);
>     ...
> }
>
> Since its_vm is NULL, ICH_HCR_EL2_En is not set. Could this break GICv4
> direct virtual LPI injection for non-protected guests?
>
> [ ... ]
>
> > @@ -166,9 +199,6 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
> >  static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
> >  {
> >       struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
> > -     struct vgic_v3_cpu_if *hyp_cpu_if = &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
> > -     struct vgic_v3_cpu_if *host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3;
> > -     unsigned int i;
> >
> >       fpsimd_sve_sync(&hyp_vcpu->vcpu);
> >       sync_debug_state(hyp_vcpu);
> > @@ -181,10 +211,7 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
> >
> >       host_vcpu->arch.iflags          = hyp_vcpu->vcpu.arch.iflags;
> >
> > -     host_cpu_if->vgic_hcr           = hyp_cpu_if->vgic_hcr;
> > -     host_cpu_if->vgic_vmcr          = hyp_cpu_if->vgic_vmcr;
> > -     for (i = 0; i < hyp_cpu_if->used_lrs; ++i)
> > -             host_cpu_if->vgic_lr[i] = hyp_cpu_if->vgic_lr[i];
> > +     sync_hyp_vgic_state(hyp_vcpu);
> >  }
>
> [Severity: High]
> This is a pre-existing issue, but does handle___vgic_v3_save_aprs()
> silently drop userspace modifications to the GICv3 VMCR?
>
> When userspace sets the VMCR register, vcpu_load() first calls
> handle___vgic_v3_restore_vmcr_aprs(), which copies host_cpu_if->vgic_vmcr
> to the hyp vCPU. KVM_SET_ONE_REG then updates the host copy of the register.
>
> Finally, vcpu_put() calls handle___vgic_v3_save_aprs():
>
> arch/arm64/kvm/hyp/nvhe/hyp-main.c:handle___vgic_v3_save_aprs() {
>     ...
>     hyp_cpu_if = &hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3;
>     ...
>     host_cpu_if = &host_vcpu->arch.vgic_cpu.vgic_v3;
>     host_cpu_if->vgic_vmcr = hyp_cpu_if->vgic_vmcr;
> }
>
> Doesn't this unconditionally overwrite the new host value (set by
> KVM_SET_ONE_REG) with the old value stored in the hyp vCPU at load time,
> potentially breaking VM migration and state restoration?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260714101601.4142645-1-fuad.tabba@linux.dev?part=7

Re: [PATCH v5 8/8] KVM: arm64: Implement lazy vCPU state sync for non-protected guests

[Fuad Tabba]

On Tue, 14 Jul 2026 at 13:33, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [High] Removal of `hcr_el2` synchronization back to the host prevents clearance of `HCR_VSE`, causing infinite virtual SError storms in the guest.
sync_hyp_vcpu  does sync VSE back ( hyp-main.c:712-713 ), host copy is cleared.

> - [High] Lazy state sync skips synchronization and clears `PKVM_HOST_STATE_DIRTY` on IRQ and IL exits, silently discarding host-side context modifications.
the only in-loop ctxt writer is  KVM_REQ_VCPU_RESET  (PSCI CPU_ON
target, not the running vCPU, DIRTY still set); injection state rides
iflags , copied unconditionally. Nothing dropped.

>
> Pre-existing issues:
> - [Critical] Protected guest register state is unconditionally leaked to and overwritten by untrusted host memory (`host_vcpu->arch.ctxt`) on every world switch.

pre-existing upstream placeholder; per-field marshalling for protected
VMs is the next series.

/fuad


> --
>
> --- Patch [8]: [PATCH v5 8/8] KVM: arm64: Implement lazy vCPU state sync for non-protected guests ---
> commit e68e83fa70438b2538d1ea836d971c6d52ffb760
> Author: Fuad Tabba <fuad.tabba@linux.dev>
>
> KVM: arm64: Implement lazy vCPU state sync for non-protected guests
>
> pKVM copies a non-protected guest's register context between the host
> and the hypervisor on every world switch, even when the host never
> inspects it. Defer the copy: on entry, flush the host context into the
> hyp vCPU only when the host marked it dirty; on exit, leave it in the
> hyp vCPU and copy it back only when the host needs it.
>
> > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> > --- a/arch/arm64/kvm/handle_exit.c
> > +++ b/arch/arm64/kvm/handle_exit.c
> > @@ -486,9 +486,32 @@ int handle_exit(struct kvm_vcpu *vcpu, int exception_index)
>
> [ ... ]
>
> > +static void handle_exit_pkvm_state(struct kvm_vcpu *vcpu, int exception_index)
> > +{
>
> [ ... ]
>
> > +     if (exception_code == ARM_EXCEPTION_TRAP ||
> > +         exception_code == ARM_EXCEPTION_EL1_SERROR ||
> > +         ARM_SERROR_PENDING(exception_index)) {
> > +             kvm_call_hyp_nvhe(__pkvm_vcpu_sync_state);
> > +             vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY);
> > +     } else {
> > +             vcpu_clear_flag(vcpu, PKVM_HOST_STATE_DIRTY);
> > +     }
> > +}
>
> [Severity: High]
> If a non-protected guest exits due to an IRQ (such as a PMU overflow), this
> clears PKVM_HOST_STATE_DIRTY. If the host kernel then handles the IRQ and
> modifies the guest context (for example, setting PMOVSSET_EL0 for the
> overflow or applying a reset like KVM_REQ_SUSPEND), these updates appear to
> be lost.
>
> On the next entry, flush_hyp_vcpu() will skip copying the host state because
> the dirty flag is clear. Then, on a subsequent exit or vcpu_put,
> handle___pkvm_vcpu_put() will reverse-sync the stale hyp state back over the
> host state. Does this clear flag path silently discard host-side context
> modifications?
>
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > @@ -172,7 +214,17 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
> >       fpsimd_sve_flush();
> >       flush_debug_state(hyp_vcpu);
> >
> > -     hyp_vcpu->vcpu.arch.ctxt        = host_vcpu->arch.ctxt;
> > +     /*
> > +      * If we deal with a non-protected guest and the state is potentially
> > +      * dirty (from a host perspective), copy the state back into the hyp
> > +      * vcpu.
> > +      */
> > +     if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
> > +             if (vcpu_get_flag(host_vcpu, PKVM_HOST_STATE_DIRTY))
> > +                     flush_hyp_vcpu_state(hyp_vcpu);
> > +     } else {
> > +             hyp_vcpu->vcpu.arch.ctxt = host_vcpu->arch.ctxt;
> > +     }
>
> [Severity: Critical]
> This is a pre-existing issue, but doesn't this unconditionally overwrite the
> protected guest's private EL2 registers with the values from the untrusted
> host memory? By copying host_vcpu->arch.ctxt into the hyp_vcpu on entry,
> could the host arbitrarily modify a protected VM's state before execution?
>
> > @@ -203,9 +255,13 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
> >       fpsimd_sve_sync(&hyp_vcpu->vcpu);
> >       sync_debug_state(hyp_vcpu);
> >
> > -     host_vcpu->arch.ctxt            = hyp_vcpu->vcpu.arch.ctxt;
> > -
> > -     host_vcpu->arch.hcr_el2         = hyp_vcpu->vcpu.arch.hcr_el2;
> > +     if (pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
> > +             host_vcpu->arch.ctxt = hyp_vcpu->vcpu.arch.ctxt;
>
> [Severity: Critical]
> This is also a pre-existing issue, but leaking the protected guest's
> register state into the untrusted host memory seems to violate pKVM's
> confidentiality guarantees. The untrusted host EL1 can read all private EL2
> registers via its mapped kvm_vcpu structure. Should this copy be avoided for
> protected VMs?
>
> [Severity: High]
> By removing the assignment that copies hcr_el2 back to the host, do we break
> virtual SError injection?
>
> When the guest takes a virtual SError, the hardware clears the VSE bit in
> the hyp vCPU's hcr_el2. Because we no longer sync this back in
> sync_hyp_vcpu(), the host's copy remains set. On the next entry,
> flush_hyp_vcpu() will read the stale host HCR_VSE bit and set it again,
> which seems like it would trap the guest in an infinite virtual SError
> storm.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260714101601.4142645-1-fuad.tabba@linux.dev?part=8

Re: [PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

[Marc Zyngier]

On Tue, 14 Jul 2026 11:15:55 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> The vcpu_{read,write}_sys_reg() accessors are only valid on a VHE host,
> so helpers built on them such as kvm_vcpu_set_be()/kvm_vcpu_is_be()
> cannot be shared with hyp code. exception.c already wraps them in local
> helpers that pick the host- or hyp-side accessor via has_vhe().
> 
> Rename the host-only implementations to __vcpu_{read,write}_sysreg_vhe()

I'm a bit puzzled by this. There is nothing that makes these functions
VHE-specific. Look at where they are called from: plenty of non-VHE
uses. These helpers are the canonical accessors for any system
register, and they don't cater for any particular mode.

	m.

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

Re: [PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

[Fuad Tabba]

On Tue, 14 Jul 2026 at 16:19, Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 14 Jul 2026 11:15:55 +0100,
> Fuad Tabba <fuad.tabba@linux.dev> wrote:
> >
> > The vcpu_{read,write}_sys_reg() accessors are only valid on a VHE host,
> > so helpers built on them such as kvm_vcpu_set_be()/kvm_vcpu_is_be()
> > cannot be shared with hyp code. exception.c already wraps them in local
> > helpers that pick the host- or hyp-side accessor via has_vhe().
> >
> > Rename the host-only implementations to __vcpu_{read,write}_sysreg_vhe()
>
> I'm a bit puzzled by this. There is nothing that makes these functions
> VHE-specific. Look at where they are called from: plenty of non-VHE
> uses. These helpers are the canonical accessors for any system
> register, and they don't cater for any particular mode.

I might have misunderstood what Oliver asked me to do here [1]:
> Can you instead name the wrappers vcpu_{read,write}_sys_reg() and rename
> the current implementations, like __vcpu_{read,write}_sysreg_vhe()?

Since they're only used and gated by has_vhe(), they're vhe-specific?

Cheers,
/fuad

[1] https://lore.kernel.org/all/alWDI7uj9YFedXET@kernel.org/


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

Re: [PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

[Marc Zyngier]

On Tue, 14 Jul 2026 16:32:54 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> On Tue, 14 Jul 2026 at 16:19, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Tue, 14 Jul 2026 11:15:55 +0100,
> > Fuad Tabba <fuad.tabba@linux.dev> wrote:
> > >
> > > The vcpu_{read,write}_sys_reg() accessors are only valid on a VHE host,
> > > so helpers built on them such as kvm_vcpu_set_be()/kvm_vcpu_is_be()
> > > cannot be shared with hyp code. exception.c already wraps them in local
> > > helpers that pick the host- or hyp-side accessor via has_vhe().
> > >
> > > Rename the host-only implementations to __vcpu_{read,write}_sysreg_vhe()
> >
> > I'm a bit puzzled by this. There is nothing that makes these functions
> > VHE-specific. Look at where they are called from: plenty of non-VHE
> > uses. These helpers are the canonical accessors for any system
> > register, and they don't cater for any particular mode.
> 
> I might have misunderstood what Oliver asked me to do here [1]:
> > Can you instead name the wrappers vcpu_{read,write}_sys_reg() and rename
> > the current implementations, like __vcpu_{read,write}_sysreg_vhe()?
> 
> Since they're only used and gated by has_vhe(), they're vhe-specific?

But they factually aren't VHE specific. Look at sys_reg.c, for
example. The whole point is that they abstract where the registers are
located, irrespective of the KVM mode.

The code in exception.c is actually a local specialisation of this
code for the nVHE code not to explode. And the exact same effect could
be achieved without repainting the whole thing and renaming *perfectly
named* accessors! ;-)

Something like below (compile-tested only).

	M.

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 9d9e7674d45fe..5ea708ff29436 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -526,6 +526,14 @@ static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
 	return __vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
 }
 
+#if   defined (__KVM_NVHE_HYPERVISOR__)
+#define vcpu_read_sys_reg(v, r)		__vcpu_sys_reg(v, r)
+#define vcpu_write_sys_reg(v, x, r)	__vcpu_assign_sys_reg(v, x, r)
+#elif defined (__KVM_VHE_HYPERVISOR__)
+#define vcpu_read_sys_reg(v, r)		vcpu_read_sys_reg(v, r)
+#define vcpu_write_sys_reg(v, x, r)	vcpu_write_sys_reg(v, x, r)
+#endif
+
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
 {
 	if (vcpu_mode_is_32bit(vcpu)) {
diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
index bef40ddb16dbc..754e2dc1df54a 100644
--- a/arch/arm64/kvm/hyp/exception.c
+++ b/arch/arm64/kvm/hyp/exception.c
@@ -20,22 +20,6 @@
 #error Hypervisor code only!
 #endif
 
-static inline u64 __vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)
-{
-	if (has_vhe())
-		return vcpu_read_sys_reg(vcpu, reg);
-
-	return __vcpu_sys_reg(vcpu, reg);
-}
-
-static inline void __vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
-{
-	if (has_vhe())
-		vcpu_write_sys_reg(vcpu, val, reg);
-	else
-		__vcpu_assign_sys_reg(vcpu, reg, val);
-}
-
 static void __vcpu_write_spsr(struct kvm_vcpu *vcpu, unsigned long target_mode,
 			      u64 val)
 {
@@ -101,14 +85,14 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
 
 	switch (target_mode) {
 	case PSR_MODE_EL1h:
-		vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL1);
-		sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
-		__vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
+		vbar = vcpu_read_sys_reg(vcpu, VBAR_EL1);
+		sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
+		vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
 		break;
 	case PSR_MODE_EL2h:
-		vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL2);
-		sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL2);
-		__vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
+		vbar = vcpu_read_sys_reg(vcpu, VBAR_EL2);
+		sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL2);
+		vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
 		break;
 	default:
 		/* Don't do that */
@@ -185,7 +169,7 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
  */
 static unsigned long get_except32_cpsr(struct kvm_vcpu *vcpu, u32 mode)
 {
-	u32 sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
+	u32 sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
 	unsigned long old, new;
 
 	old = *vcpu_cpsr(vcpu);
@@ -281,7 +265,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
 {
 	unsigned long spsr = *vcpu_cpsr(vcpu);
 	bool is_thumb = (spsr & PSR_AA32_T_BIT);
-	u32 sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
+	u32 sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
 	u32 return_address;
 
 	*vcpu_cpsr(vcpu) = get_except32_cpsr(vcpu, mode);
@@ -305,7 +289,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
 	if (sctlr & (1 << 13))
 		vect_offset += 0xffff0000;
 	else /* always have security exceptions */
-		vect_offset += __vcpu_read_sys_reg(vcpu, VBAR_EL1);
+		vect_offset += vcpu_read_sys_reg(vcpu, VBAR_EL1);
 
 	*vcpu_pc(vcpu) = vect_offset;
 }

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

Re: [PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

[Fuad Tabba]

On Tue, 14 Jul 2026 at 17:39, Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 14 Jul 2026 16:32:54 +0100,
> Fuad Tabba <fuad.tabba@linux.dev> wrote:
> >
> > On Tue, 14 Jul 2026 at 16:19, Marc Zyngier <maz@kernel.org> wrote:
> > >
> > > On Tue, 14 Jul 2026 11:15:55 +0100,
> > > Fuad Tabba <fuad.tabba@linux.dev> wrote:
> > > >
> > > > The vcpu_{read,write}_sys_reg() accessors are only valid on a VHE host,
> > > > so helpers built on them such as kvm_vcpu_set_be()/kvm_vcpu_is_be()
> > > > cannot be shared with hyp code. exception.c already wraps them in local
> > > > helpers that pick the host- or hyp-side accessor via has_vhe().
> > > >
> > > > Rename the host-only implementations to __vcpu_{read,write}_sysreg_vhe()
> > >
> > > I'm a bit puzzled by this. There is nothing that makes these functions
> > > VHE-specific. Look at where they are called from: plenty of non-VHE
> > > uses. These helpers are the canonical accessors for any system
> > > register, and they don't cater for any particular mode.
> >
> > I might have misunderstood what Oliver asked me to do here [1]:
> > > Can you instead name the wrappers vcpu_{read,write}_sys_reg() and rename
> > > the current implementations, like __vcpu_{read,write}_sysreg_vhe()?
> >
> > Since they're only used and gated by has_vhe(), they're vhe-specific?
>
> But they factually aren't VHE specific. Look at sys_reg.c, for
> example. The whole point is that they abstract where the registers are
> located, irrespective of the KVM mode.
>
> The code in exception.c is actually a local specialisation of this
> code for the nVHE code not to explode. And the exact same effect could
> be achieved without repainting the whole thing and renaming *perfectly
> named* accessors! ;-)
>
> Something like below (compile-tested only).
>
>         M.

If it's fine by you and Oliver it works for me. I'll respin with that.

Cheers,
/fuad

>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 9d9e7674d45fe..5ea708ff29436 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -526,6 +526,14 @@ static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
>         return __vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
>  }
>
> +#if   defined (__KVM_NVHE_HYPERVISOR__)
> +#define vcpu_read_sys_reg(v, r)                __vcpu_sys_reg(v, r)
> +#define vcpu_write_sys_reg(v, x, r)    __vcpu_assign_sys_reg(v, x, r)
> +#elif defined (__KVM_VHE_HYPERVISOR__)
> +#define vcpu_read_sys_reg(v, r)                vcpu_read_sys_reg(v, r)
> +#define vcpu_write_sys_reg(v, x, r)    vcpu_write_sys_reg(v, x, r)
> +#endif
> +
>  static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
>  {
>         if (vcpu_mode_is_32bit(vcpu)) {
> diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
> index bef40ddb16dbc..754e2dc1df54a 100644
> --- a/arch/arm64/kvm/hyp/exception.c
> +++ b/arch/arm64/kvm/hyp/exception.c
> @@ -20,22 +20,6 @@
>  #error Hypervisor code only!
>  #endif
>
> -static inline u64 __vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)
> -{
> -       if (has_vhe())
> -               return vcpu_read_sys_reg(vcpu, reg);
> -
> -       return __vcpu_sys_reg(vcpu, reg);
> -}
> -
> -static inline void __vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
> -{
> -       if (has_vhe())
> -               vcpu_write_sys_reg(vcpu, val, reg);
> -       else
> -               __vcpu_assign_sys_reg(vcpu, reg, val);
> -}
> -
>  static void __vcpu_write_spsr(struct kvm_vcpu *vcpu, unsigned long target_mode,
>                               u64 val)
>  {
> @@ -101,14 +85,14 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
>
>         switch (target_mode) {
>         case PSR_MODE_EL1h:
> -               vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL1);
> -               sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> -               __vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
> +               vbar = vcpu_read_sys_reg(vcpu, VBAR_EL1);
> +               sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> +               vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
>                 break;
>         case PSR_MODE_EL2h:
> -               vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL2);
> -               sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL2);
> -               __vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
> +               vbar = vcpu_read_sys_reg(vcpu, VBAR_EL2);
> +               sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL2);
> +               vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
>                 break;
>         default:
>                 /* Don't do that */
> @@ -185,7 +169,7 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
>   */
>  static unsigned long get_except32_cpsr(struct kvm_vcpu *vcpu, u32 mode)
>  {
> -       u32 sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> +       u32 sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
>         unsigned long old, new;
>
>         old = *vcpu_cpsr(vcpu);
> @@ -281,7 +265,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
>  {
>         unsigned long spsr = *vcpu_cpsr(vcpu);
>         bool is_thumb = (spsr & PSR_AA32_T_BIT);
> -       u32 sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> +       u32 sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
>         u32 return_address;
>
>         *vcpu_cpsr(vcpu) = get_except32_cpsr(vcpu, mode);
> @@ -305,7 +289,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
>         if (sctlr & (1 << 13))
>                 vect_offset += 0xffff0000;
>         else /* always have security exceptions */
> -               vect_offset += __vcpu_read_sys_reg(vcpu, VBAR_EL1);
> +               vect_offset += vcpu_read_sys_reg(vcpu, VBAR_EL1);
>
>         *vcpu_pc(vcpu) = vect_offset;
>  }
>
> --
> Without deviation from the norm, progress is not possible.