1. Patchew
  2. linux
  3. [PATCH v4 0/4] KVM: arm64: Fix underallocation of storage for SVE state
  4. View patch

[PATCH v4 3/4] KVM: arm64: Fix FFR offset calculation for pKVM host state save and restore

Mark Brown posted 4 patches 1 year, 7 months ago
There is a newer version of this series
[PATCH v4 3/4] KVM: arm64: Fix FFR offset calculation for pKVM host state save and restore
Posted by Mark Brown 1 year, 7 months ago 
When saving and restoring the SVE state for the host we configure the
hardware for the maximum VL it supports, but when calculating offsets in
memory we use the maximum usable VL for the host. Since these two values
may not be the same this may result in data corruption.  We can just
read the current VL from the hardware with an instruction so do that
instead of a saved value, we need to correct the value and this makes
the consistency obvious.

Fixes: b5b9955617bc ("KVM: arm64: Eagerly restore host fpsimd/sve state in pKVM")
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/kvm_hyp.h        | 1 +
 arch/arm64/kvm/hyp/fpsimd.S             | 5 +++++
 arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
 arch/arm64/kvm/hyp/nvhe/hyp-main.c      | 2 +-
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index b05bceca3385..7510383d78a6 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -113,6 +113,7 @@ void __fpsimd_save_state(struct user_fpsimd_state *fp_regs);
 void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs);
 void __sve_save_state(void *sve_pffr, u32 *fpsr, int save_ffr);
 void __sve_restore_state(void *sve_pffr, u32 *fpsr, int restore_ffr);
+int __sve_get_vl(void);
 
 u64 __guest_enter(struct kvm_vcpu *vcpu);
 
diff --git a/arch/arm64/kvm/hyp/fpsimd.S b/arch/arm64/kvm/hyp/fpsimd.S
index e950875e31ce..d272dbf36da8 100644
--- a/arch/arm64/kvm/hyp/fpsimd.S
+++ b/arch/arm64/kvm/hyp/fpsimd.S
@@ -31,3 +31,8 @@ SYM_FUNC_START(__sve_save_state)
 	sve_save 0, x1, x2, 3
 	ret
 SYM_FUNC_END(__sve_save_state)
+
+SYM_FUNC_START(__sve_get_vl)
+	_sve_rdvl	0, 1
+	ret
+SYM_FUNC_END(__sve_get_vl)
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 0c4de44534b7..06efcca765cc 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -327,7 +327,7 @@ static inline void __hyp_sve_save_host(void)
 
 	sve_state->zcr_el1 = read_sysreg_el1(SYS_ZCR);
 	write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2);
-	__sve_save_state(sve_state->sve_regs + sve_ffr_offset(kvm_host_sve_max_vl),
+	__sve_save_state(sve_state->sve_regs + sve_ffr_offset(__sve_get_vl()),
 			 &sve_state->fpsr,
 			 true);
 }
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index f43d845f3c4e..bd8f671e848c 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -49,7 +49,7 @@ static void __hyp_sve_restore_host(void)
 	 * supported by the system (or limited at EL3).
 	 */
 	write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2);
-	__sve_restore_state(sve_state->sve_regs + sve_ffr_offset(kvm_host_sve_max_vl),
+	__sve_restore_state(sve_state->sve_regs + sve_ffr_offset(__sve_get_vl()),
 			    &sve_state->fpsr,
 			    true);
 	write_sysreg_el1(sve_state->zcr_el1, SYS_ZCR);

-- 
2.39.2
Re: [PATCH v4 3/4] KVM: arm64: Fix FFR offset calculation for pKVM host state save and restore
Posted by Marc Zyngier 1 year, 7 months ago 
On Thu, 04 Jul 2024 18:28:18 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> When saving and restoring the SVE state for the host we configure the
> hardware for the maximum VL it supports, but when calculating offsets in
> memory we use the maximum usable VL for the host. Since these two values
> may not be the same this may result in data corruption.  We can just
> read the current VL from the hardware with an instruction so do that
> instead of a saved value, we need to correct the value and this makes
> the consistency obvious.

Which value are we correcting?

> 
> Fixes: b5b9955617bc ("KVM: arm64: Eagerly restore host fpsimd/sve state in pKVM")
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_hyp.h        | 1 +
>  arch/arm64/kvm/hyp/fpsimd.S             | 5 +++++
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
>  arch/arm64/kvm/hyp/nvhe/hyp-main.c      | 2 +-
>  4 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index b05bceca3385..7510383d78a6 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -113,6 +113,7 @@ void __fpsimd_save_state(struct user_fpsimd_state *fp_regs);
>  void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs);
>  void __sve_save_state(void *sve_pffr, u32 *fpsr, int save_ffr);
>  void __sve_restore_state(void *sve_pffr, u32 *fpsr, int restore_ffr);
> +int __sve_get_vl(void);

Is there any case where you'd consider returning a signed value? Given
the multiplier of '1', I don't see it likely to happen.

>  
>  u64 __guest_enter(struct kvm_vcpu *vcpu);
>  
> diff --git a/arch/arm64/kvm/hyp/fpsimd.S b/arch/arm64/kvm/hyp/fpsimd.S
> index e950875e31ce..d272dbf36da8 100644
> --- a/arch/arm64/kvm/hyp/fpsimd.S
> +++ b/arch/arm64/kvm/hyp/fpsimd.S
> @@ -31,3 +31,8 @@ SYM_FUNC_START(__sve_save_state)
>  	sve_save 0, x1, x2, 3
>  	ret
>  SYM_FUNC_END(__sve_save_state)
> +
> +SYM_FUNC_START(__sve_get_vl)
> +	_sve_rdvl	0, 1
> +	ret
> +SYM_FUNC_END(__sve_get_vl)
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 0c4de44534b7..06efcca765cc 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -327,7 +327,7 @@ static inline void __hyp_sve_save_host(void)
>  
>  	sve_state->zcr_el1 = read_sysreg_el1(SYS_ZCR);
>  	write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2);
> -	__sve_save_state(sve_state->sve_regs + sve_ffr_offset(kvm_host_sve_max_vl),
> +	__sve_save_state(sve_state->sve_regs + sve_ffr_offset(__sve_get_vl()),
>  			 &sve_state->fpsr,
>  			 true);
>  }
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index f43d845f3c4e..bd8f671e848c 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -49,7 +49,7 @@ static void __hyp_sve_restore_host(void)
>  	 * supported by the system (or limited at EL3).
>  	 */
>  	write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2);
> -	__sve_restore_state(sve_state->sve_regs + sve_ffr_offset(kvm_host_sve_max_vl),
> +	__sve_restore_state(sve_state->sve_regs + sve_ffr_offset(__sve_get_vl()),
>  			    &sve_state->fpsr,
>  			    true);
>  	write_sysreg_el1(sve_state->zcr_el1, SYS_ZCR);
> 

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH v4 3/4] KVM: arm64: Fix FFR offset calculation for pKVM host state save and restore
Posted by Mark Brown 1 year, 7 months ago 
On Fri, Jul 05, 2024 at 02:27:01PM +0100, Marc Zyngier wrote:
> On Thu, 04 Jul 2024 18:28:18 +0100,
> Mark Brown <broonie@kernel.org> wrote:

> > When saving and restoring the SVE state for the host we configure the
> > hardware for the maximum VL it supports, but when calculating offsets in
> > memory we use the maximum usable VL for the host. Since these two values
> > may not be the same this may result in data corruption.  We can just
> > read the current VL from the hardware with an instruction so do that
> > instead of a saved value, we need to correct the value and this makes
> > the consistency obvious.

> Which value are we correcting?

The vector length used when laying out the storage, especially in the
case where the hardware VL is larger than the largest VL used by Linux.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.