Re: [PATCH 08/19] x86/fpu: Move xfd_update_state() to xstate.c and export symbol

Thomas Gleixner
2021-12-10T22:44:29Z

On Tue, Dec 07 2021 at 19:03, Yang Zhong wrote:
> From: Jing Liu <jing2.liu@intel.com>
>
> xfd_update_state() is the interface to update IA32_XFD and its per-cpu
> cache. All callers of this interface are currently in fpu core. KVM only
> indirectly triggers IA32_XFD update via a helper function
> (fpu_swap_kvm_fpstate()) when switching between user fpu and guest fpu.
>
> Supporting AMX in guest now requires KVM to directly update IA32_XFD
> with the guest value (when emulating WRMSR) so XSAVE/XRSTOR can manage
> XSTATE components correctly inside guest.
>
> This patch moves xfd_update_state() from fpu/xstate.h to fpu/xstate.c

s/This patch moves/Move/

please. See Documentation/process/submitting-patches.rst and search for
'This patch'

> and export it for reference outside of fpu core.
>
> Signed-off-by: Jing Liu <jing2.liu@intel.com>
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> ---
> arch/x86/include/asm/fpu/api.h | 2 ++
> arch/x86/kernel/fpu/xstate.c | 12 ++++++++++++
> arch/x86/kernel/fpu/xstate.h | 14 +-------------
> 3 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
> index 7532f73c82a6..999d89026be9 100644
> --- a/arch/x86/include/asm/fpu/api.h
> +++ b/arch/x86/include/asm/fpu/api.h
> @@ -131,8 +131,10 @@ DECLARE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx);
> /* Process cleanup */
> #ifdef CONFIG_X86_64
> extern void fpstate_free(struct fpu *fpu);
> +extern void xfd_update_state(struct fpstate *fpstate);
> #else
> static inline void fpstate_free(struct fpu *fpu) { }
> +static void xfd_update_state(struct fpstate *fpstate) { }

Try a 32bit build to see the warnings this causes. That wants to be
'static inline void' obviously.

> #ifdef CONFIG_X86_64
> -static inline void xfd_update_state(struct fpstate *fpstate)
> -{
> - if (fpu_state_size_dynamic()) {
> - u64 xfd = fpstate->xfd;
> -
> - if (__this_cpu_read(xfd_state) != xfd) {
> - wrmsrl(MSR_IA32_XFD, xfd);
> - __this_cpu_write(xfd_state, xfd);
> - }
> - }
> -}
> -#else
> -static inline void xfd_update_state(struct fpstate *fpstate) { }
> +extern void xfd_update_state(struct fpstate *fpstate);

Why? It's already declared in the global header. So all of this has to
be simply removed, no?

Thanks,

tglx