RE: [patch 5/6] x86/fpu: Provide fpu_update_guest_xcr0/xfd()

["Tian, Kevin" <kevin.tian@intel.com>]

> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Tuesday, December 14, 2021 10:50 AM
> 
> KVM can require fpstate expansion due to updates to XCR0 and to the XFD
> MSR. In both cases it is required to check whether:
> 
>   - the requested values are correct or permitted
> 
>   - the resulting xfeature mask which is relevant for XSAVES is a subset of
>     the guests fpstate xfeature mask for which the register buffer is sized.
> 
>     If the feature mask does not fit into the guests fpstate then
>     reallocation is required.
> 
> Provide a common update function which utilizes the existing XFD
> enablement
> mechanics and two wrapper functions, one for XCR0 and one for XFD.
> 
> These wrappers have to be invoked from XSETBV emulation and the XFD
> MSR
> write emulation.
> 
> XCR0 modification can only proceed when fpu_update_guest_xcr0() returns
> success.

Currently XCR0 is modified right before entering guest with preemption 
disabled (see kvm_load_guest_xsave_state()). So this assumption is met.

> 
> XFD modification is done by the FPU core code as it requires to update the
> software state as well.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

[...]
> +static inline int fpu_update_guest_xfd(struct fpu_guest *guest_fpu, u64 xcr0,
> u64 xfd)
> +{
> +	return __fpu_update_guest_features(guest_fpu, xcr0, xfd);
> +}

no need to pass in xcr0. It can be fetched from vcpu->arch.xcr0.

> +int __fpu_update_guest_features(struct fpu_guest *guest_fpu, u64 xcr0,
> u64 xfd)
> +{
> +	u64 expand, requested;
> +
> +	lockdep_assert_preemption_enabled();
> +
> +	/* Only permitted features are allowed in XCR0 */
> +	if (xcr0 & ~guest_fpu->perm)
> +		return -EPERM;
> +
> +	/* Check for unsupported XFD values */
> +	if (xfd & ~XFEATURE_MASK_USER_DYNAMIC || xfd &
> ~fpu_user_cfg.max_features)
> +		return -ENOTSUPP;
> +
> +	if (!IS_ENABLED(CONFIG_X86_64))
> +		return 0;

this could be checked first.

Thanks
Kevin