Re: [PATCH v6 17/64] KVM: arm64: nv: Emulate PSTATE.M for a guest hypervisor

Russell King (Oracle)
2022-02-01T18:09:00Z

On Fri, Jan 28, 2022 at 12:18:25PM +0000, Marc Zyngier wrote:
> From: Christoffer Dall <christoffer.dall@arm.com>
>
> We can no longer blindly copy the VCPU's PSTATE into SPSR_EL2 and return
> to the guest and vice versa when taking an exception to the hypervisor,
> because we emulate virtual EL2 in EL1 and therefore have to translate
> the mode field from EL2 to EL1 and vice versa.
>
> This requires keeping track of the state we enter the guest, for which
> we transiently use a dedicated flag.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/include/asm/kvm_host.h | 1 +
> arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 19 ++++++++++++++++-
> arch/arm64/kvm/hyp/vhe/switch.c | 24 ++++++++++++++++++++++
> 3 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 8fffe2888403..fa253f08e0fd 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -472,6 +472,7 @@ struct kvm_vcpu_arch {
> #define KVM_ARM64_DEBUG_STATE_SAVE_SPE (1 << 12) /* Save SPE context if active */
> #define KVM_ARM64_DEBUG_STATE_SAVE_TRBE (1 << 13) /* Save TRBE context if active */
> #define KVM_ARM64_FP_FOREIGN_FPSTATE (1 << 14)
> +#define KVM_ARM64_IN_HYP_CONTEXT (1 << 15) /* Guest running in HYP context */
>
> #define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
> KVM_GUESTDBG_USE_SW_BP | \
> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> index 283f780f5f56..e3689c6ce4cc 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> @@ -157,9 +157,26 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt,
> write_sysreg_el1(ctxt_sys_reg(ctxt, SPSR_EL1), SYS_SPSR);
> }
>
> +/* Read the VCPU state's PSTATE, but translate (v)EL2 to EL1. */
> +static inline u64 to_hw_pstate(const struct kvm_cpu_context *ctxt)
> +{
> + u64 mode = ctxt->regs.pstate & (PSR_MODE_MASK | PSR_MODE32_BIT);
> +
> + switch (mode) {
> + case PSR_MODE_EL2t:
> + mode = PSR_MODE_EL1t;
> + break;
> + case PSR_MODE_EL2h:
> + mode = PSR_MODE_EL1h;
> + break;
> + }
> +
> + return (ctxt->regs.pstate & ~(PSR_MODE_MASK | PSR_MODE32_BIT)) | mode;
> +}
> +

Wondering if it makes sense to also have the reverse translation as an
inline function after the above too, so the two translations are
together - but as it's only used (in this patch at least) in switch.c
there probably isn't too much point.

So:

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!