Re: [PATCH v4 06/21] x86/fpu: Ignore APX when copying from/to guest FPU

[Paolo Bonzini <pbonzini@redhat.com>]

On 5/12/26 03:14, Chang S. Bae wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> KVM will store APX extended GPRs directly in the regs[] field of struct
> kvm_vcpu.  This is done to make accessors more uniform between GPRs and
> EGPRs, and partly because x86 maintainers expressed the desire to compile
> Linux with APX enabled sooner or later; accessing guest EGPRs from KVM's
> C code would get in the way.
> 
> Therefore, let KVM handle the APX feature on its own when executing
> ioctls to get and set the virtual machine's XSAVE state.
> 
> Cc: x86@kernel.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> [ chang: adjust function prototype ]
> Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>

Boris, Dave, can you please ACK these changes to core FPU code?

Thanks,

Paolo

> ---
> V3 -> V4: New patch
> ---
>   arch/x86/include/asm/fpu/api.h | 2 +-
>   arch/x86/kernel/fpu/core.c     | 9 +++++++--
>   2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
> index cd6f194a912b..0b218f5eaafd 100644
> --- a/arch/x86/include/asm/fpu/api.h
> +++ b/arch/x86/include/asm/fpu/api.h
> @@ -160,7 +160,7 @@ static inline void fpu_sync_guest_vmexit_xfd_state(void) { }
>   
>   extern void fpu_copy_guest_fpstate_to_uabi(struct fpu_guest *gfpu, void *buf,
>   					   unsigned int size, u64 xfeatures, u32 pkru);
> -extern int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, const void *buf, u64 xcr0, u32 *vpkru);
> +extern int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, void *buf, u64 xcr0, u32 *vpkru);
>   
>   static inline void fpstate_set_confidential(struct fpu_guest *gfpu)
>   {
> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> index 608983806fd7..ae5c73faed69 100644
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
> @@ -420,6 +420,8 @@ void fpu_copy_guest_fpstate_to_uabi(struct fpu_guest *gfpu, void *buf,
>   	struct membuf mb = { .p = buf, .left = size };
>   
>   	if (cpu_feature_enabled(X86_FEATURE_XSAVE)) {
> +		/* Up to date APX registers are in struct kvm_vcpu anyway.  */
> +		xfeatures &= ~XFEATURE_MASK_APX;
>   		__copy_xstate_to_uabi_buf(mb, kstate, xfeatures, pkru,
>   					  XSTATE_COPY_XSAVE);
>   	} else {
> @@ -431,11 +433,11 @@ void fpu_copy_guest_fpstate_to_uabi(struct fpu_guest *gfpu, void *buf,
>   }
>   EXPORT_SYMBOL_FOR_KVM(fpu_copy_guest_fpstate_to_uabi);
>   
> -int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, const void *buf,
> +int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, void *buf,
>   				   u64 xcr0, u32 *vpkru)
>   {
>   	struct fpstate *kstate = gfpu->fpstate;
> -	const union fpregs_state *ustate = buf;
> +	union fpregs_state *ustate = buf;
>   
>   	if (!cpu_feature_enabled(X86_FEATURE_XSAVE)) {
>   		if (ustate->xsave.header.xfeatures & ~XFEATURE_MASK_FPSSE)
> @@ -464,6 +466,9 @@ int fpu_copy_uabi_to_guest_fpstate(struct fpu_guest *gfpu, const void *buf,
>   	if (!(ustate->xsave.header.xfeatures & XFEATURE_MASK_PKRU))
>   		vpkru = NULL;
>   
> +	/* APX registers are copied to and from struct kvm_vcpu, not the FPU.  */
> +	ustate->xsave.header.xfeatures &= ~XFEATURE_MASK_APX;
> +
>   	return copy_uabi_from_kernel_to_xstate(kstate, ustate, vpkru);
>   }
>   EXPORT_SYMBOL_FOR_KVM(fpu_copy_uabi_to_guest_fpstate);