linux-kernel - Re: [patch 13/31] x86/fpu: Move KVMs FPU swapping to FPU core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 Oct 2021 18:53:38 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        "Chang S. Bae" <chang.seok.bae@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Arjan van de Ven <arjan@...ux.intel.com>,
        kvm@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [patch 13/31] x86/fpu: Move KVMs FPU swapping to FPU core

Just typos:

On Tue, Oct 12, 2021 at 02:00:17AM +0200, Thomas Gleixner wrote:
> Swapping the host/guest FPU is directly fiddling with FPU internals which
> requires 5 exports. The upcoming support of dymanically enabled states

"dynamically"

>  /*
>   * Use kernel_fpu_begin/end() if you intend to use FPU in kernel context. It
>   * disables preemption so be careful if you intend to use it for long periods
> @@ -108,4 +110,10 @@ extern int cpu_has_xfeatures(u64 xfeatur
>  
>  static inline void update_pasid(void) { }
>  
> +/* FPSTATE related functions which are exported to KVM */

fpstate-related

> +extern void fpu_init_fpstate_user(struct fpu *fpu);
> +
> +/* KVM specific functions */
> +extern void fpu_swap_kvm_fpu(struct fpu *save, struct fpu *rstor, u64 restore_mask);
> +
>  #endif /* _ASM_X86_FPU_API_H */

...

>  /* Swap (qemu) user FPU context for the guest FPU context. */
>  static void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
>  {
> -	fpregs_lock();
> -
> -	kvm_save_current_fpu(vcpu->arch.user_fpu);
> -
>  	/*
> -	 * Guests with protected state can't have it set by the hypervisor,
> -	 * so skip trying to set it.
> +	 * Guest with protected state have guest_fpu == NULL which makes

"Guests ... "

> +	 * the swap only safe the host state. Exclude PKRU from restore as

"save"

> +	 * it is restored separately in kvm_x86_ops.run().
>  	 */
> -	if (vcpu->arch.guest_fpu)
> -		/* PKRU is separately restored in kvm_x86_ops.run. */
> -		__restore_fpregs_from_fpstate(&vcpu->arch.guest_fpu->state,
> -					~XFEATURE_MASK_PKRU);
> -
> -	fpregs_mark_activate();
> -	fpregs_unlock();
> -
> +	fpu_swap_kvm_fpu(vcpu->arch.user_fpu, vcpu->arch.guest_fpu,
> +			 ~XFEATURE_MASK_PKRU);
>  	trace_kvm_fpu(1);
>  }
>  
>  /* When vcpu_run ends, restore user space FPU context. */
>  static void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
>  {
> -	fpregs_lock();
> -
>  	/*
> -	 * Guests with protected state can't have it read by the hypervisor,
> -	 * so skip trying to save it.
> +	 * Guest with protected state have guest_fpu == NULL which makes

"Guests ... "

> +	 * swap only restore the host state.
>  	 */
> -	if (vcpu->arch.guest_fpu)
> -		kvm_save_current_fpu(vcpu->arch.guest_fpu);
> -
> -	restore_fpregs_from_fpstate(&vcpu->arch.user_fpu->state);
> -
> -	fpregs_mark_activate();
> -	fpregs_unlock();
> -
> +	fpu_swap_kvm_fpu(vcpu->arch.guest_fpu, vcpu->arch.user_fpu, ~0ULL);
>  	++vcpu->stat.fpu_reload;
>  	trace_kvm_fpu(0);
>  }
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -47,7 +47,7 @@ static bool ex_handler_fprestore(const s
>  	WARN_ONCE(1, "Bad FPU state detected at %pB, reinitializing FPU registers.",
>  		  (void *)instruction_pointer(regs));
>  
> -	__restore_fpregs_from_fpstate(&init_fpstate, xfeatures_mask_fpstate());
> +	restore_fpregs_from_fpstate(&init_fpstate, xfeatures_mask_fpstate());
>  	return true;
>  }
>  
> 

-- 
Regards/Gruss,
    Boris.

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