[v4,30/40] KVM: arm64: Defer saving/restoring 32-bit sysregs to vcpu load/put
diff mbox

Message ID 20180215210332.8648-31-christoffer.dall@linaro.org
State New
Headers show

Commit Message

Christoffer Dall Feb. 15, 2018, 9:03 p.m. UTC
When running a 32-bit VM (EL1 in AArch32), the AArch32 system registers
can be deferred to vcpu load/put on VHE systems because neither
the host kernel nor host userspace uses these registers.

Note that we can not defer saving DBGVCR32_EL2 conditionally based
on the state of the debug dirty flag on VHE, but since we do the
load/put pretty rarely, this comes out as a win anyway.

We can also not defer saving FPEXC32_32 because this register only holds
a guest-valid value for 32-bit guests during the exit path when the
guest has used FPSIMD registers and restored the register in the early
assembly handler from taking the EL2 fault, and therefore we have to
check if fpsimd is enabled for the guest in the exit path and save the
register then, for both VHE and non-VHE guests.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---

Notes:
    Changes since v3:
     - Rework the FPEXC32 save/restore logic to no longer attempt to
       save/restore this register lazily.
    
    Changes since v2:
     - New patch (deferred register handling has been reworked)

 arch/arm64/kvm/hyp/switch.c    | 17 +++++++++++------
 arch/arm64/kvm/hyp/sysreg-sr.c | 15 ++++++++++-----
 2 files changed, 21 insertions(+), 11 deletions(-)

Comments

Marc Zyngier Feb. 21, 2018, 4:27 p.m. UTC | #1
On Thu, 15 Feb 2018 21:03:22 +0000,
Christoffer Dall wrote:
> 
> When running a 32-bit VM (EL1 in AArch32), the AArch32 system registers
> can be deferred to vcpu load/put on VHE systems because neither
> the host kernel nor host userspace uses these registers.
> 
> Note that we can not defer saving DBGVCR32_EL2 conditionally based
> on the state of the debug dirty flag on VHE, but since we do the
> load/put pretty rarely, this comes out as a win anyway.

I'm not sure I understand that comment. We don't have any deferring
for this register, so the load/put reference seems out of place.

> 
> We can also not defer saving FPEXC32_32 because this register only holds
> a guest-valid value for 32-bit guests during the exit path when the
> guest has used FPSIMD registers and restored the register in the early
> assembly handler from taking the EL2 fault, and therefore we have to
> check if fpsimd is enabled for the guest in the exit path and save the
> register then, for both VHE and non-VHE guests.
> 
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> 
> Notes:
>     Changes since v3:
>      - Rework the FPEXC32 save/restore logic to no longer attempt to
>        save/restore this register lazily.
>     
>     Changes since v2:
>      - New patch (deferred register handling has been reworked)
> 
>  arch/arm64/kvm/hyp/switch.c    | 17 +++++++++++------
>  arch/arm64/kvm/hyp/sysreg-sr.c | 15 ++++++++++-----
>  2 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 22e77deb8e2e..909aa3fe9196 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -47,6 +47,15 @@ bool __hyp_text __fpsimd_enabled(void)
>  	return __fpsimd_is_enabled()();
>  }
>  
> +/* Save the 32-bit only FPSIMD system register state */
> +static inline void __hyp_text __fpsimd_save_fpexc32(struct kvm_vcpu *vcpu)
> +{
> +	if (!vcpu_el1_is_32bit(vcpu))
> +		return;
> +
> +	vcpu->arch.ctxt.sys_regs[FPEXC32_EL2] = read_sysreg(fpexc32_el2);
> +}
> +
>  static void __hyp_text __activate_traps_vhe(void)
>  {
>  	u64 val;
> @@ -380,11 +389,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  
>  	__vgic_restore_state(vcpu);
>  
> -	/*
> -	 * We must restore the 32-bit state before the sysregs, thanks
> -	 * to erratum #852523 (Cortex-A57) or #853709 (Cortex-A72).
> -	 */
> -	__sysreg32_restore_state(vcpu);
>  	sysreg_restore_guest_state_vhe(guest_ctxt);
>  	__debug_switch_to_guest(vcpu);
>  
> @@ -398,7 +402,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  	fp_enabled = __fpsimd_enabled();
>  
>  	sysreg_save_guest_state_vhe(guest_ctxt);
> -	__sysreg32_save_state(vcpu);
>  	__vgic_save_state(vcpu);
>  
>  	__deactivate_traps(vcpu);
> @@ -408,6 +411,7 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  	if (fp_enabled) {
>  		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
>  		__fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
> +		__fpsimd_save_fpexc32(vcpu);
>  	}
>  
>  	__debug_switch_to_host(vcpu);
> @@ -475,6 +479,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
>  	if (fp_enabled) {
>  		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
>  		__fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
> +		__fpsimd_save_fpexc32(vcpu);
>  	}
>  
>  	/*
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 9c60b8062724..aacba4636871 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -196,10 +196,7 @@ void __hyp_text __sysreg32_save_state(struct kvm_vcpu *vcpu)
>  	sysreg[DACR32_EL2] = read_sysreg(dacr32_el2);
>  	sysreg[IFSR32_EL2] = read_sysreg(ifsr32_el2);
>  
> -	if (__fpsimd_enabled())
> -		sysreg[FPEXC32_EL2] = read_sysreg(fpexc32_el2);
> -
> -	if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
> +	if (has_vhe() || vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
>  		sysreg[DBGVCR32_EL2] = read_sysreg(dbgvcr32_el2);
>  }
>  
> @@ -221,7 +218,7 @@ void __hyp_text __sysreg32_restore_state(struct kvm_vcpu *vcpu)
>  	write_sysreg(sysreg[DACR32_EL2], dacr32_el2);
>  	write_sysreg(sysreg[IFSR32_EL2], ifsr32_el2);
>  
> -	if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
> +	if (has_vhe() || vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
>  		write_sysreg(sysreg[DBGVCR32_EL2], dbgvcr32_el2);
>  }
>  
> @@ -246,6 +243,13 @@ void kvm_vcpu_load_sysregs(struct kvm_vcpu *vcpu)
>  
>  	__sysreg_save_user_state(host_ctxt);
>  
> +	/*
> +	 * Load guest EL1 and user state
> +	 *
> +	 * We must restore the 32-bit state before the sysregs, thanks
> +	 * to erratum #852523 (Cortex-A57) or #853709 (Cortex-A72).
> +	 */
> +	__sysreg32_restore_state(vcpu);
>  	__sysreg_restore_user_state(guest_ctxt);
>  	__sysreg_restore_el1_state(guest_ctxt);
>  
> @@ -273,6 +277,7 @@ void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu)
>  
>  	__sysreg_save_el1_state(guest_ctxt);
>  	__sysreg_save_user_state(guest_ctxt);
> +	__sysreg32_save_state(vcpu);
>  
>  	/* Restore host user state */
>  	__sysreg_restore_user_state(host_ctxt);
> -- 
> 2.14.2
> 

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
Andrew Jones Feb. 22, 2018, 2:35 p.m. UTC | #2
On Thu, Feb 15, 2018 at 10:03:22PM +0100, Christoffer Dall wrote:
> When running a 32-bit VM (EL1 in AArch32), the AArch32 system registers
> can be deferred to vcpu load/put on VHE systems because neither
> the host kernel nor host userspace uses these registers.
> 
> Note that we can not defer saving DBGVCR32_EL2 conditionally based
> on the state of the debug dirty flag on VHE, but since we do the
> load/put pretty rarely, this comes out as a win anyway.
> 
> We can also not defer saving FPEXC32_32 because this register only holds
> a guest-valid value for 32-bit guests during the exit path when the
> guest has used FPSIMD registers and restored the register in the early
> assembly handler from taking the EL2 fault, and therefore we have to
> check if fpsimd is enabled for the guest in the exit path and save the
> register then, for both VHE and non-VHE guests.
> 
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
> 
> Notes:
>     Changes since v3:
>      - Rework the FPEXC32 save/restore logic to no longer attempt to
>        save/restore this register lazily.
>     
>     Changes since v2:
>      - New patch (deferred register handling has been reworked)
> 
>  arch/arm64/kvm/hyp/switch.c    | 17 +++++++++++------
>  arch/arm64/kvm/hyp/sysreg-sr.c | 15 ++++++++++-----
>  2 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 22e77deb8e2e..909aa3fe9196 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -47,6 +47,15 @@ bool __hyp_text __fpsimd_enabled(void)
>  	return __fpsimd_is_enabled()();
>  }
>  
> +/* Save the 32-bit only FPSIMD system register state */
> +static inline void __hyp_text __fpsimd_save_fpexc32(struct kvm_vcpu *vcpu)
> +{
> +	if (!vcpu_el1_is_32bit(vcpu))
> +		return;
> +
> +	vcpu->arch.ctxt.sys_regs[FPEXC32_EL2] = read_sysreg(fpexc32_el2);
> +}
> +

I realize it's much more convenient to have this function here, but it
feels a bit out of place, being a _save_ function. Its logical place is
an -sr file.

>  static void __hyp_text __activate_traps_vhe(void)
>  {
>  	u64 val;
> @@ -380,11 +389,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  
>  	__vgic_restore_state(vcpu);
>  
> -	/*
> -	 * We must restore the 32-bit state before the sysregs, thanks
> -	 * to erratum #852523 (Cortex-A57) or #853709 (Cortex-A72).
> -	 */
> -	__sysreg32_restore_state(vcpu);
>  	sysreg_restore_guest_state_vhe(guest_ctxt);
>  	__debug_switch_to_guest(vcpu);
>  
> @@ -398,7 +402,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  	fp_enabled = __fpsimd_enabled();
>  
>  	sysreg_save_guest_state_vhe(guest_ctxt);
> -	__sysreg32_save_state(vcpu);
>  	__vgic_save_state(vcpu);
>  
>  	__deactivate_traps(vcpu);
> @@ -408,6 +411,7 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  	if (fp_enabled) {
>  		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
>  		__fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
> +		__fpsimd_save_fpexc32(vcpu);
>  	}
>  
>  	__debug_switch_to_host(vcpu);
> @@ -475,6 +479,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
>  	if (fp_enabled) {
>  		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
>  		__fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
> +		__fpsimd_save_fpexc32(vcpu);
>  	}
>  
>  	/*
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 9c60b8062724..aacba4636871 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -196,10 +196,7 @@ void __hyp_text __sysreg32_save_state(struct kvm_vcpu *vcpu)
>  	sysreg[DACR32_EL2] = read_sysreg(dacr32_el2);
>  	sysreg[IFSR32_EL2] = read_sysreg(ifsr32_el2);
>  
> -	if (__fpsimd_enabled())
> -		sysreg[FPEXC32_EL2] = read_sysreg(fpexc32_el2);
> -
> -	if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
> +	if (has_vhe() || vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
>  		sysreg[DBGVCR32_EL2] = read_sysreg(dbgvcr32_el2);
>  }
>  
> @@ -221,7 +218,7 @@ void __hyp_text __sysreg32_restore_state(struct kvm_vcpu *vcpu)
>  	write_sysreg(sysreg[DACR32_EL2], dacr32_el2);
>  	write_sysreg(sysreg[IFSR32_EL2], ifsr32_el2);
>  
> -	if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
> +	if (has_vhe() || vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
>  		write_sysreg(sysreg[DBGVCR32_EL2], dbgvcr32_el2);
>  }
>  
> @@ -246,6 +243,13 @@ void kvm_vcpu_load_sysregs(struct kvm_vcpu *vcpu)
>  
>  	__sysreg_save_user_state(host_ctxt);
>  
> +	/*
> +	 * Load guest EL1 and user state
> +	 *
> +	 * We must restore the 32-bit state before the sysregs, thanks
> +	 * to erratum #852523 (Cortex-A57) or #853709 (Cortex-A72).
> +	 */
> +	__sysreg32_restore_state(vcpu);
>  	__sysreg_restore_user_state(guest_ctxt);
>  	__sysreg_restore_el1_state(guest_ctxt);
>  
> @@ -273,6 +277,7 @@ void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu)
>  
>  	__sysreg_save_el1_state(guest_ctxt);
>  	__sysreg_save_user_state(guest_ctxt);
> +	__sysreg32_save_state(vcpu);
>  
>  	/* Restore host user state */
>  	__sysreg_restore_user_state(host_ctxt);
> -- 
> 2.14.2
>

Besides the function location being a bit debatable, it looks good to me

Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew
Christoffer Dall Feb. 22, 2018, 2:58 p.m. UTC | #3
On Thu, Feb 22, 2018 at 03:35:06PM +0100, Andrew Jones wrote:
> On Thu, Feb 15, 2018 at 10:03:22PM +0100, Christoffer Dall wrote:
> > When running a 32-bit VM (EL1 in AArch32), the AArch32 system registers
> > can be deferred to vcpu load/put on VHE systems because neither
> > the host kernel nor host userspace uses these registers.
> > 
> > Note that we can not defer saving DBGVCR32_EL2 conditionally based
> > on the state of the debug dirty flag on VHE, but since we do the
> > load/put pretty rarely, this comes out as a win anyway.
> > 
> > We can also not defer saving FPEXC32_32 because this register only holds
> > a guest-valid value for 32-bit guests during the exit path when the
> > guest has used FPSIMD registers and restored the register in the early
> > assembly handler from taking the EL2 fault, and therefore we have to
> > check if fpsimd is enabled for the guest in the exit path and save the
> > register then, for both VHE and non-VHE guests.
> > 
> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> > ---
> > 
> > Notes:
> >     Changes since v3:
> >      - Rework the FPEXC32 save/restore logic to no longer attempt to
> >        save/restore this register lazily.
> >     
> >     Changes since v2:
> >      - New patch (deferred register handling has been reworked)
> > 
> >  arch/arm64/kvm/hyp/switch.c    | 17 +++++++++++------
> >  arch/arm64/kvm/hyp/sysreg-sr.c | 15 ++++++++++-----
> >  2 files changed, 21 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > index 22e77deb8e2e..909aa3fe9196 100644
> > --- a/arch/arm64/kvm/hyp/switch.c
> > +++ b/arch/arm64/kvm/hyp/switch.c
> > @@ -47,6 +47,15 @@ bool __hyp_text __fpsimd_enabled(void)
> >  	return __fpsimd_is_enabled()();
> >  }
> >  
> > +/* Save the 32-bit only FPSIMD system register state */
> > +static inline void __hyp_text __fpsimd_save_fpexc32(struct kvm_vcpu *vcpu)
> > +{
> > +	if (!vcpu_el1_is_32bit(vcpu))
> > +		return;
> > +
> > +	vcpu->arch.ctxt.sys_regs[FPEXC32_EL2] = read_sysreg(fpexc32_el2);
> > +}
> > +
> 
> I realize it's much more convenient to have this function here, but it
> feels a bit out of place, being a _save_ function. Its logical place is
> an -sr file.
> 

Yes, maybe, but that would make it a function call to perform a mrs and
a store, which is a bit unfortunate.

> >  static void __hyp_text __activate_traps_vhe(void)
> >  {
> >  	u64 val;
> > @@ -380,11 +389,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
> >  
> >  	__vgic_restore_state(vcpu);
> >  
> > -	/*
> > -	 * We must restore the 32-bit state before the sysregs, thanks
> > -	 * to erratum #852523 (Cortex-A57) or #853709 (Cortex-A72).
> > -	 */
> > -	__sysreg32_restore_state(vcpu);
> >  	sysreg_restore_guest_state_vhe(guest_ctxt);
> >  	__debug_switch_to_guest(vcpu);
> >  
> > @@ -398,7 +402,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
> >  	fp_enabled = __fpsimd_enabled();
> >  
> >  	sysreg_save_guest_state_vhe(guest_ctxt);
> > -	__sysreg32_save_state(vcpu);
> >  	__vgic_save_state(vcpu);
> >  
> >  	__deactivate_traps(vcpu);
> > @@ -408,6 +411,7 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
> >  	if (fp_enabled) {
> >  		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
> >  		__fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
> > +		__fpsimd_save_fpexc32(vcpu);
> >  	}
> >  
> >  	__debug_switch_to_host(vcpu);
> > @@ -475,6 +479,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
> >  	if (fp_enabled) {
> >  		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
> >  		__fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
> > +		__fpsimd_save_fpexc32(vcpu);
> >  	}
> >  
> >  	/*
> > diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> > index 9c60b8062724..aacba4636871 100644
> > --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> > +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> > @@ -196,10 +196,7 @@ void __hyp_text __sysreg32_save_state(struct kvm_vcpu *vcpu)
> >  	sysreg[DACR32_EL2] = read_sysreg(dacr32_el2);
> >  	sysreg[IFSR32_EL2] = read_sysreg(ifsr32_el2);
> >  
> > -	if (__fpsimd_enabled())
> > -		sysreg[FPEXC32_EL2] = read_sysreg(fpexc32_el2);
> > -
> > -	if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
> > +	if (has_vhe() || vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
> >  		sysreg[DBGVCR32_EL2] = read_sysreg(dbgvcr32_el2);
> >  }
> >  
> > @@ -221,7 +218,7 @@ void __hyp_text __sysreg32_restore_state(struct kvm_vcpu *vcpu)
> >  	write_sysreg(sysreg[DACR32_EL2], dacr32_el2);
> >  	write_sysreg(sysreg[IFSR32_EL2], ifsr32_el2);
> >  
> > -	if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
> > +	if (has_vhe() || vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
> >  		write_sysreg(sysreg[DBGVCR32_EL2], dbgvcr32_el2);
> >  }
> >  
> > @@ -246,6 +243,13 @@ void kvm_vcpu_load_sysregs(struct kvm_vcpu *vcpu)
> >  
> >  	__sysreg_save_user_state(host_ctxt);
> >  
> > +	/*
> > +	 * Load guest EL1 and user state
> > +	 *
> > +	 * We must restore the 32-bit state before the sysregs, thanks
> > +	 * to erratum #852523 (Cortex-A57) or #853709 (Cortex-A72).
> > +	 */
> > +	__sysreg32_restore_state(vcpu);
> >  	__sysreg_restore_user_state(guest_ctxt);
> >  	__sysreg_restore_el1_state(guest_ctxt);
> >  
> > @@ -273,6 +277,7 @@ void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu)
> >  
> >  	__sysreg_save_el1_state(guest_ctxt);
> >  	__sysreg_save_user_state(guest_ctxt);
> > +	__sysreg32_save_state(vcpu);
> >  
> >  	/* Restore host user state */
> >  	__sysreg_restore_user_state(host_ctxt);
> > -- 
> > 2.14.2
> >
> 
> Besides the function location being a bit debatable, it looks good to me
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> 
Thanks,
-Christoffer
Christoffer Dall Feb. 22, 2018, 6:15 p.m. UTC | #4
On Wed, Feb 21, 2018 at 04:27:25PM +0000, Marc Zyngier wrote:
> On Thu, 15 Feb 2018 21:03:22 +0000,
> Christoffer Dall wrote:
> > 
> > When running a 32-bit VM (EL1 in AArch32), the AArch32 system registers
> > can be deferred to vcpu load/put on VHE systems because neither
> > the host kernel nor host userspace uses these registers.
> > 
> > Note that we can not defer saving DBGVCR32_EL2 conditionally based
> > on the state of the debug dirty flag on VHE, but since we do the
> > load/put pretty rarely, this comes out as a win anyway.
> 
> I'm not sure I understand that comment. We don't have any deferring
> for this register, so the load/put reference seems out of place.
> 

Yeah, this is a patch description editing snafu.  I'll fix it.

Thanks,
-Christoffer

Patch
diff mbox

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 22e77deb8e2e..909aa3fe9196 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -47,6 +47,15 @@  bool __hyp_text __fpsimd_enabled(void)
 	return __fpsimd_is_enabled()();
 }
 
+/* Save the 32-bit only FPSIMD system register state */
+static inline void __hyp_text __fpsimd_save_fpexc32(struct kvm_vcpu *vcpu)
+{
+	if (!vcpu_el1_is_32bit(vcpu))
+		return;
+
+	vcpu->arch.ctxt.sys_regs[FPEXC32_EL2] = read_sysreg(fpexc32_el2);
+}
+
 static void __hyp_text __activate_traps_vhe(void)
 {
 	u64 val;
@@ -380,11 +389,6 @@  int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
 	__vgic_restore_state(vcpu);
 
-	/*
-	 * We must restore the 32-bit state before the sysregs, thanks
-	 * to erratum #852523 (Cortex-A57) or #853709 (Cortex-A72).
-	 */
-	__sysreg32_restore_state(vcpu);
 	sysreg_restore_guest_state_vhe(guest_ctxt);
 	__debug_switch_to_guest(vcpu);
 
@@ -398,7 +402,6 @@  int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 	fp_enabled = __fpsimd_enabled();
 
 	sysreg_save_guest_state_vhe(guest_ctxt);
-	__sysreg32_save_state(vcpu);
 	__vgic_save_state(vcpu);
 
 	__deactivate_traps(vcpu);
@@ -408,6 +411,7 @@  int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 	if (fp_enabled) {
 		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
 		__fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
+		__fpsimd_save_fpexc32(vcpu);
 	}
 
 	__debug_switch_to_host(vcpu);
@@ -475,6 +479,7 @@  int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 	if (fp_enabled) {
 		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
 		__fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
+		__fpsimd_save_fpexc32(vcpu);
 	}
 
 	/*
diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index 9c60b8062724..aacba4636871 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -196,10 +196,7 @@  void __hyp_text __sysreg32_save_state(struct kvm_vcpu *vcpu)
 	sysreg[DACR32_EL2] = read_sysreg(dacr32_el2);
 	sysreg[IFSR32_EL2] = read_sysreg(ifsr32_el2);
 
-	if (__fpsimd_enabled())
-		sysreg[FPEXC32_EL2] = read_sysreg(fpexc32_el2);
-
-	if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
+	if (has_vhe() || vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
 		sysreg[DBGVCR32_EL2] = read_sysreg(dbgvcr32_el2);
 }
 
@@ -221,7 +218,7 @@  void __hyp_text __sysreg32_restore_state(struct kvm_vcpu *vcpu)
 	write_sysreg(sysreg[DACR32_EL2], dacr32_el2);
 	write_sysreg(sysreg[IFSR32_EL2], ifsr32_el2);
 
-	if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
+	if (has_vhe() || vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
 		write_sysreg(sysreg[DBGVCR32_EL2], dbgvcr32_el2);
 }
 
@@ -246,6 +243,13 @@  void kvm_vcpu_load_sysregs(struct kvm_vcpu *vcpu)
 
 	__sysreg_save_user_state(host_ctxt);
 
+	/*
+	 * Load guest EL1 and user state
+	 *
+	 * We must restore the 32-bit state before the sysregs, thanks
+	 * to erratum #852523 (Cortex-A57) or #853709 (Cortex-A72).
+	 */
+	__sysreg32_restore_state(vcpu);
 	__sysreg_restore_user_state(guest_ctxt);
 	__sysreg_restore_el1_state(guest_ctxt);
 
@@ -273,6 +277,7 @@  void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu)
 
 	__sysreg_save_el1_state(guest_ctxt);
 	__sysreg_save_user_state(guest_ctxt);
+	__sysreg32_save_state(vcpu);
 
 	/* Restore host user state */
 	__sysreg_restore_user_state(host_ctxt);