Re: [PATCH v5 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code
From: Fuad Tabba <hidden>
Date: 2026-07-14 16:45:40
Also in:
kvmarm, lkml
On Tue, 14 Jul 2026 at 17:39, Marc Zyngier [off-list ref] wrote:
On Tue, 14 Jul 2026 16:32:54 +0100, Fuad Tabba [off-list ref] wrote:quoted
On Tue, 14 Jul 2026 at 16:19, Marc Zyngier [off-list ref] wrote:quoted
On Tue, 14 Jul 2026 11:15:55 +0100, Fuad Tabba [off-list ref] wrote:quoted
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]:quoted
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
quoted hunk ↗ jump to hunk
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.