[PATCH] arm64/fpsimd: Clarify the purpose of using last in fpsimd_save()

[PATCH] arm64/fpsimd: Clarify the purpose of using last in fpsimd_save()

[Mark Brown]

When saving the floating point context in fpsimd_save() we always reference
the state using last-> rather than using current->. Looking at the FP code
in isolation the reason for this is not entirely obvious, it's done because
when KVM is running it will bind the guest context and rely on the host
writing out the guest state on context switch away from the guest.

There's a slight trick here in that KVM still uses TIF_FOREIGN_FPSTATE and
TIF_SVE to communicate what needs to be saved, it maintains those flags
and restores them when it is done running the guest so that the normal
restore paths function when we return back to userspace.

Add a comment to explain this to help future readers work out what's going
on a bit faster.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/fpsimd.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 5280e098cfb5..47af76e53221 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -348,7 +348,13 @@ static void task_fpsimd_load(void)
 
 /*
  * Ensure FPSIMD/SVE storage in memory for the loaded context is up to
- * date with respect to the CPU registers.
+ * date with respect to the CPU registers. Note carefully that the
+ * current context is the context last bound to the CPU stored in
+ * last, if KVM is involved this may be the guest VM context rather
+ * than the host thread for the VM pointed to by current. This means
+ * that we must always reference the state storage via last rather
+ * than via current, other than the TIF_ flags which KVM will
+ * carefully maintain for us.
  */
 static void fpsimd_save(void)
 {
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/fpsimd: Clarify the purpose of using last in fpsimd_save()

[Catalin Marinas]

On Mon, Jan 24, 2022 at 04:11:15PM +0000, Mark Brown wrote:
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 5280e098cfb5..47af76e53221 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -348,7 +348,13 @@ static void task_fpsimd_load(void)
>  
>  /*
>   * Ensure FPSIMD/SVE storage in memory for the loaded context is up to
> - * date with respect to the CPU registers.
> + * date with respect to the CPU registers. Note carefully that the
> + * current context is the context last bound to the CPU stored in
> + * last, if KVM is involved this may be the guest VM context rather
> + * than the host thread for the VM pointed to by current. This means
> + * that we must always reference the state storage via last rather
> + * than via current, other than the TIF_ flags which KVM will
> + * carefully maintain for us.
>   */
>  static void fpsimd_save(void)
>  {

It's good to have the text here, otherwise it's hidden somewhere in the
commit logs.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/fpsimd: Clarify the purpose of using last in fpsimd_save()

[Marc Zyngier]

On Mon, 24 Jan 2022 16:11:15 +0000, Mark Brown wrote:
> When saving the floating point context in fpsimd_save() we always reference
> the state using last-> rather than using current->. Looking at the FP code
> in isolation the reason for this is not entirely obvious, it's done because
> when KVM is running it will bind the guest context and rely on the host
> writing out the guest state on context switch away from the guest.
> 
> There's a slight trick here in that KVM still uses TIF_FOREIGN_FPSTATE and
> TIF_SVE to communicate what needs to be saved, it maintains those flags
> and restores them when it is done running the guest so that the normal
> restore paths function when we return back to userspace.
> 
> [...]

Applied to next, thanks!

[1/1] arm64/fpsimd: Clarify the purpose of using last in fpsimd_save()
      commit: 432110cd83caa655c646ec5d8ca6a9e9afb9ccba

Cheers,

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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel