Re: [patch 14/31] x86/fpu: Replace KVMs homebrewn FPU copy from user

From: Borislav Petkov <bp@alien8.de>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, "Chang S. Bae" <chang.seok.bae@intel.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Arjan van de Ven <arjan@linux.intel.com>,
	kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [patch 14/31] x86/fpu: Replace KVMs homebrewn FPU copy from user
Date: Tue, 12 Oct 2021 19:00:44 +0200	[thread overview]
Message-ID: <YWW/PEQyQAwS9/qv@zn.tnic> (raw)
In-Reply-To: <20211011223611.129308001@linutronix.de>

On Tue, Oct 12, 2021 at 02:00:19AM +0200, Thomas Gleixner wrote:
> Copying a user space buffer to the memory buffer is already available in
> the FPU core. The copy mechanism in KVM lacks sanity checks and needs to
> use cpuid() to lookup the offset of each component, while the FPU core has
> this information cached.
> 
> Make the FPU core variant accessible for KVM and replace the homebrewn
> mechanism.

I think you mean "homebred" in that patch... or "home brewed", that
works too, I think.

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: kvm@vger.kernel.org
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/asm/fpu/api.h |    3 +
>  arch/x86/kernel/fpu/core.c     |   38 ++++++++++++++++++++-
>  arch/x86/kernel/fpu/xstate.c   |    3 -
>  arch/x86/kvm/x86.c             |   74 +----------------------------------------
>  4 files changed, 44 insertions(+), 74 deletions(-)
> 
> --- a/arch/x86/include/asm/fpu/api.h
> +++ b/arch/x86/include/asm/fpu/api.h
> @@ -116,4 +116,7 @@ extern void fpu_init_fpstate_user(struct
>  /* KVM specific functions */
>  extern void fpu_swap_kvm_fpu(struct fpu *save, struct fpu *rstor, u64 restore_mask);
>  
> +struct kvm_vcpu;
> +extern int fpu_copy_kvm_uabi_to_vcpu(struct fpu *fpu, const void *buf, u64 xcr0, u32 *pkru);
> +
>  #endif /* _ASM_X86_FPU_API_H */
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
> @@ -174,7 +174,43 @@ void fpu_swap_kvm_fpu(struct fpu *save,
>  	fpregs_unlock();
>  }
>  EXPORT_SYMBOL_GPL(fpu_swap_kvm_fpu);
> -#endif
> +
> +int fpu_copy_kvm_uabi_to_vcpu(struct fpu *fpu, const void *buf, u64 xcr0,
> +			      u32 *vpkru)

Right, except that there's no @vcpu in the args of that function. I
guess you could call it

fpu_copy_kvm_uabi_to_buf()

and that @buf can be

vcpu->arch.guest_fpu

...

Just a nitpick anyway.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette