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

Tian, Kevin
2021-12-14T06:25:49Z

> 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