From: Thomas Gleixner tglx@linutronix.de

KVM requires a clear separation of host user space and guest permissions for dynamic XSTATE components.

Add a guest permissions member to struct fpu and a separate set of prctl() arguments: ARCH_GET_XCOMP_GUEST_PERM and ARCH_REQ_XCOMP_GUEST_PERM.

The semantics are equivalent to the host user space permission control except for the following constraints:

1) Permissions have to be requested before the first vCPU is created

2) Permissions are frozen when the first vCPU is created to ensure consistency. Any attempt to expand permissions via the prctl() after that point is rejected.

Signed-off-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Jing Liu jing2.liu@intel.com --- arch/x86/include/asm/fpu/api.h | 2 ++ arch/x86/include/asm/fpu/types.h | 9 ++++++ arch/x86/include/uapi/asm/prctl.h | 26 ++++++++------- arch/x86/kernel/fpu/core.c | 3 ++ arch/x86/kernel/fpu/xstate.c | 53 +++++++++++++++++++++++-------- arch/x86/kernel/fpu/xstate.h | 13 ++++++-- arch/x86/kernel/process.c | 2 ++ 7 files changed, 80 insertions(+), 28 deletions(-)

diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h index c2767a6a387e..d8c222290e68 100644 --- a/arch/x86/include/asm/fpu/api.h +++ b/arch/x86/include/asm/fpu/api.h @@ -132,6 +132,8 @@ static inline void fpstate_free(struct fpu *fpu) { } /* fpstate-related functions which are exported to KVM */ extern void fpstate_clear_xstate_component(struct fpstate *fps, unsigned int xfeature);

+extern inline u64 xstate_get_guest_group_perm(void); + /* KVM specific functions */ extern bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu); extern void fpu_free_guest_fpstate(struct fpu_guest *gfpu); diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h index 3c06c82ab355..6ddf80637697 100644 --- a/arch/x86/include/asm/fpu/types.h +++ b/arch/x86/include/asm/fpu/types.h @@ -387,6 +387,8 @@ struct fpstate { /* @regs is dynamically sized! Don't add anything after @regs! */ } __aligned(64);

+#define FPU_GUEST_PERM_LOCKED BIT_ULL(63) + struct fpu_state_perm { /* * @__state_perm: @@ -476,6 +478,13 @@ struct fpu { */ struct fpu_state_perm perm;

+ /* + * @guest_perm: + * + * Permission related information for guest pseudo FPUs + */ + struct fpu_state_perm guest_perm; + /* * @__fpstate: * diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h index 754a07856817..500b96e71f18 100644 --- a/arch/x86/include/uapi/asm/prctl.h +++ b/arch/x86/include/uapi/asm/prctl.h @@ -2,20 +2,22 @@ #ifndef _ASM_X86_PRCTL_H #define _ASM_X86_PRCTL_H

-#define ARCH_SET_GS 0x1001 -#define ARCH_SET_FS 0x1002 -#define ARCH_GET_FS 0x1003 -#define ARCH_GET_GS 0x1004 +#define ARCH_SET_GS 0x1001 +#define ARCH_SET_FS 0x1002 +#define ARCH_GET_FS 0x1003 +#define ARCH_GET_GS 0x1004

-#define ARCH_GET_CPUID 0x1011 -#define ARCH_SET_CPUID 0x1012 +#define ARCH_GET_CPUID 0x1011 +#define ARCH_SET_CPUID 0x1012

-#define ARCH_GET_XCOMP_SUPP 0x1021 -#define ARCH_GET_XCOMP_PERM 0x1022 -#define ARCH_REQ_XCOMP_PERM 0x1023 +#define ARCH_GET_XCOMP_SUPP 0x1021 +#define ARCH_GET_XCOMP_PERM 0x1022 +#define ARCH_REQ_XCOMP_PERM 0x1023 +#define ARCH_GET_XCOMP_GUEST_PERM 0x1024 +#define ARCH_REQ_XCOMP_GUEST_PERM 0x1025

-#define ARCH_MAP_VDSO_X32 0x2001 -#define ARCH_MAP_VDSO_32 0x2002 -#define ARCH_MAP_VDSO_64 0x2003 +#define ARCH_MAP_VDSO_X32 0x2001 +#define ARCH_MAP_VDSO_32 0x2002 +#define ARCH_MAP_VDSO_64 0x2003

#endif /* _ASM_X86_PRCTL_H */ diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index 8ea306b1bf8e..ab19b3d8b2f7 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -450,6 +450,8 @@ void fpstate_reset(struct fpu *fpu) fpu->perm.__state_perm = fpu_kernel_cfg.default_features; fpu->perm.__state_size = fpu_kernel_cfg.default_size; fpu->perm.__user_state_size = fpu_user_cfg.default_size; + /* Same defaults for guests */ + fpu->guest_perm = fpu->perm; }

static inline void fpu_inherit_perms(struct fpu *dst_fpu) @@ -460,6 +462,7 @@ static inline void fpu_inherit_perms(struct fpu *dst_fpu) spin_lock_irq(&current->sighand->siglock); /* Fork also inherits the permissions of the parent */ dst_fpu->perm = src_fpu->perm; + dst_fpu->guest_perm = src_fpu->guest_perm; spin_unlock_irq(&current->sighand->siglock); } } diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index d28829403ed0..5f01d463859d 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -1595,7 +1595,7 @@ static int validate_sigaltstack(unsigned int usize) return 0; }

-static int __xstate_request_perm(u64 permitted, u64 requested) +static int __xstate_request_perm(u64 permitted, u64 requested, bool guest) { /* * This deliberately does not exclude !XSAVES as we still might @@ -1605,9 +1605,10 @@ static int __xstate_request_perm(u64 permitted, u64 requested) */ bool compacted = cpu_feature_enabled(X86_FEATURE_XSAVES); struct fpu *fpu = &current->group_leader->thread.fpu; + struct fpu_state_perm *perm; unsigned int ksize, usize; u64 mask; - int ret; + int ret = 0;

/* Check whether fully enabled */ if ((permitted & requested) == requested) @@ -1621,15 +1622,18 @@ static int __xstate_request_perm(u64 permitted, u64 requested) mask &= XFEATURE_MASK_USER_SUPPORTED; usize = xstate_calculate_size(mask, false);

- ret = validate_sigaltstack(usize); - if (ret) - return ret; + if (!guest) { + ret = validate_sigaltstack(usize); + if (ret) + return ret; + }

+ perm = guest ? &fpu->guest_perm : &fpu->perm; /* Pairs with the READ_ONCE() in xstate_get_group_perm() */ - WRITE_ONCE(fpu->perm.__state_perm, requested); + WRITE_ONCE(perm->__state_perm, requested); /* Protected by sighand lock */ - fpu->perm.__state_size = ksize; - fpu->perm.__user_state_size = usize; + perm->__state_size = ksize; + perm->__user_state_size = usize; return ret; }

@@ -1640,7 +1644,7 @@ static const u64 xstate_prctl_req[XFEATURE_MAX] = { [XFEATURE_XTILE_DATA] = XFEATURE_MASK_XTILE_DATA, };

-static int xstate_request_perm(unsigned long idx) +static int xstate_request_perm(unsigned long idx, bool guest) { u64 permitted, requested; int ret; @@ -1661,14 +1665,19 @@ static int xstate_request_perm(unsigned long idx) return -EOPNOTSUPP;

/* Lockless quick check */ - permitted = xstate_get_host_group_perm(); + permitted = xstate_get_group_perm(guest); if ((permitted & requested) == requested) return 0;

/* Protect against concurrent modifications */ spin_lock_irq(&current->sighand->siglock); - permitted = xstate_get_host_group_perm(); - ret = __xstate_request_perm(permitted, requested); + permitted = xstate_get_group_perm(guest); + + /* First vCPU allocation locks the permissions. */ + if (guest && (permitted & FPU_GUEST_PERM_LOCKED)) + ret = -EBUSY; + else + ret = __xstate_request_perm(permitted, requested, guest); spin_unlock_irq(&current->sighand->siglock); return ret; } @@ -1713,12 +1722,18 @@ int xfd_enable_feature(u64 xfd_err) return 0; } #else /* CONFIG_X86_64 */ -static inline int xstate_request_perm(unsigned long idx) +static inline int xstate_request_perm(unsigned long idx, bool guest) { return -EPERM; } #endif /* !CONFIG_X86_64 */

+inline u64 xstate_get_guest_group_perm(void) +{ + return xstate_get_group_perm(true); +} +EXPORT_SYMBOL_GPL(xstate_get_guest_group_perm); + /** * fpu_xstate_prctl - xstate permission operations * @tsk: Redundant pointer to current @@ -1742,6 +1757,7 @@ long fpu_xstate_prctl(struct task_struct *tsk, int option, unsigned long arg2) u64 __user *uptr = (u64 __user *)arg2; u64 permitted, supported; unsigned long idx = arg2; + bool guest = false;

if (tsk != current) return -EPERM; @@ -1760,11 +1776,20 @@ long fpu_xstate_prctl(struct task_struct *tsk, int option, unsigned long arg2) permitted &= XFEATURE_MASK_USER_SUPPORTED; return put_user(permitted, uptr);

+ case ARCH_GET_XCOMP_GUEST_PERM: + permitted = xstate_get_guest_group_perm(); + permitted &= XFEATURE_MASK_USER_SUPPORTED; + return put_user(permitted, uptr); + + case ARCH_REQ_XCOMP_GUEST_PERM: + guest = true; + fallthrough; + case ARCH_REQ_XCOMP_PERM: if (!IS_ENABLED(CONFIG_X86_64)) return -EOPNOTSUPP;

- return xstate_request_perm(idx); + return xstate_request_perm(idx, guest);

default: return -EINVAL; diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h index 86ea7c0fa2f6..98a472775c97 100644 --- a/arch/x86/kernel/fpu/xstate.h +++ b/arch/x86/kernel/fpu/xstate.h @@ -20,10 +20,19 @@ static inline void xstate_init_xcomp_bv(struct xregs_state *xsave, u64 mask) xsave->header.xcomp_bv = mask | XCOMP_BV_COMPACTED_FORMAT; }

-static inline u64 xstate_get_host_group_perm(void) +static inline u64 xstate_get_group_perm(bool guest) { + struct fpu *fpu = &current->group_leader->thread.fpu; + struct fpu_state_perm *perm; + /* Pairs with WRITE_ONCE() in xstate_request_perm() */ - return READ_ONCE(current->group_leader->thread.fpu.perm.__state_perm); + perm = guest ? &fpu->guest_perm : &fpu->perm; + return READ_ONCE(perm->__state_perm); +} + +static inline u64 xstate_get_host_group_perm(void) +{ + return xstate_get_group_perm(false); }

enum xstate_copy_mode { diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c index 04143a653a8a..d7bc23589062 100644 --- a/arch/x86/kernel/process.c +++ b/arch/x86/kernel/process.c @@ -993,6 +993,8 @@ long do_arch_prctl_common(struct task_struct *task, int option, case ARCH_GET_XCOMP_SUPP: case ARCH_GET_XCOMP_PERM: case ARCH_REQ_XCOMP_PERM: + case ARCH_GET_XCOMP_GUEST_PERM: + case ARCH_REQ_XCOMP_GUEST_PERM: return fpu_xstate_prctl(task, option, arg2); }

CPUID.0xD.1.EBX enumerates the size of the XSAVE area (in compacted format) required by XSAVES. If CPUID.0xD.i.ECX[1] is set for a state component (i), this state component should be located on the next 64-bytes boundary following the preceding state component in the compacted layout.

Fix xstate_required_size() to follow the alignment rule. AMX is the first state component with 64-bytes alignment to catch this bug.

Signed-off-by: Yang Zhong yang.zhong@intel.com Signed-off-by: Jing Liu jing2.liu@intel.com --- arch/x86/kvm/cpuid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 07e9215e911d..148003e26cbb 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -42,7 +42,8 @@ static u32 xstate_required_size(u64 xstate_bv, bool compacted) if (xstate_bv & 0x1) { u32 eax, ebx, ecx, edx, offset; cpuid_count(0xD, feature_bit, &eax, &ebx, &ecx, &edx); - offset = compacted ? ret : ebx; + /* ECX[1]: 64B alignment in compacted form */ + offset = compacted ? ((ecx & 0x2) ? ALIGN(ret, 64) : ret) : ebx; ret = max(ret, offset + eax); }

Guest xstate permissions should be set by userspace VMM before vcpu creation. Extend KVM_SET_CPUID2 to verify that every feature reported in CPUID[0xD] has proper permission set.

Signed-off-by: Jing Liu jing2.liu@intel.com --- arch/x86/kvm/cpuid.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 4855344091b8..a068373a7fbd 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -81,7 +81,9 @@ static inline struct kvm_cpuid_entry2 *cpuid_entry2_find( return NULL; }

-static int kvm_check_cpuid(struct kvm_cpuid_entry2 *entries, int nent) +static int kvm_check_cpuid(struct kvm_vcpu *vcpu, + struct kvm_cpuid_entry2 *entries, + int nent) { struct kvm_cpuid_entry2 *best;

@@ -97,6 +99,16 @@ static int kvm_check_cpuid(struct kvm_cpuid_entry2 *entries, int nent) return -EINVAL; }

+ /* Check guest permissions for dynamically-enabled xfeatures */ + best = cpuid_entry2_find(entries, nent, 0xd, 0); + if (best) { + u64 xfeatures; + + xfeatures = best->eax | ((u64)best->edx << 32); + if (xfeatures & ~vcpu->arch.guest_fpu.perm) + return -ENXIO; + } + return 0; }

@@ -277,21 +289,21 @@ u64 kvm_vcpu_reserved_gpa_bits_raw(struct kvm_vcpu *vcpu) static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2, int nent) { - int r; + int r;

- r = kvm_check_cpuid(e2, nent); - if (r) - return r; + r = kvm_check_cpuid(vcpu, e2, nent); + if (r) + return r;

- kvfree(vcpu->arch.cpuid_entries); - vcpu->arch.cpuid_entries = e2; - vcpu->arch.cpuid_nent = nent; + kvfree(vcpu->arch.cpuid_entries); + vcpu->arch.cpuid_entries = e2; + vcpu->arch.cpuid_nent = nent;

- kvm_update_kvm_cpuid_base(vcpu); - kvm_update_cpuid_runtime(vcpu); - kvm_vcpu_after_set_cpuid(vcpu); + kvm_update_kvm_cpuid_base(vcpu); + kvm_update_cpuid_runtime(vcpu); + kvm_vcpu_after_set_cpuid(vcpu);

- return 0; + return 0; }

/* when an old userspace process fills a new kernel module */

From: Thomas Gleixner tglx@linutronix.de

Guest support for dynamically enabled FPU features requires a few modifications to the enablement function which is currently invoked from the #NM handler:

1) Use guest permissions and sizes for the update

2) Update fpu_guest state accordingly

3) Take into account that the enabling can be triggered either from a running guest via XSETBV and MSR_IA32_XFD write emulation or from a guest restore. In the latter case the guests fpstate is not the current tasks active fpstate.

Split the function and implement the guest mechanics throughout the callchain.

Signed-off-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Jing Liu jing2.liu@intel.com --- arch/x86/kernel/fpu/xstate.c | 93 +++++++++++++++++++++--------------- arch/x86/kernel/fpu/xstate.h | 2 + 2 files changed, 56 insertions(+), 39 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 5f01d463859d..0c0b2323cdec 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -1499,29 +1499,6 @@ void fpstate_free(struct fpu *fpu) vfree(fpu->fpstate); }

-/** - * fpu_install_fpstate - Update the active fpstate in the FPU - * - * @fpu: A struct fpu * pointer - * @newfps: A struct fpstate * pointer - * - * Returns: A null pointer if the last active fpstate is the embedded - * one or the new fpstate is already installed; - * otherwise, a pointer to the old fpstate which has to - * be freed by the caller. - */ -static struct fpstate *fpu_install_fpstate(struct fpu *fpu, - struct fpstate *newfps) -{ - struct fpstate *oldfps = fpu->fpstate; - - if (fpu->fpstate == newfps) - return NULL; - - fpu->fpstate = newfps; - return oldfps != &fpu->__fpstate ? oldfps : NULL; -} - /** * fpstate_realloc - Reallocate struct fpstate for the requested new features * @@ -1529,6 +1506,7 @@ static struct fpstate *fpu_install_fpstate(struct fpu *fpu, * of that task * @ksize: The required size for the kernel buffer * @usize: The required size for user space buffers + * @guest_fpu: Pointer to a guest FPU container. NULL for host allocations * * Note vs. vmalloc(): If the task with a vzalloc()-allocated buffer * terminates quickly, vfree()-induced IPIs may be a concern, but tasks @@ -1537,13 +1515,13 @@ static struct fpstate *fpu_install_fpstate(struct fpu *fpu, * Returns: 0 on success, -ENOMEM on allocation error. */ static int fpstate_realloc(u64 xfeatures, unsigned int ksize, - unsigned int usize) + unsigned int usize, struct fpu_guest *guest_fpu) { struct fpu *fpu = &current->thread.fpu; struct fpstate *curfps, *newfps = NULL; unsigned int fpsize; + bool in_use;

- curfps = fpu->fpstate; fpsize = ksize + ALIGN(offsetof(struct fpstate, regs), 64);

newfps = vzalloc(fpsize); @@ -1553,28 +1531,55 @@ static int fpstate_realloc(u64 xfeatures, unsigned int ksize, newfps->user_size = usize; newfps->is_valloc = true;

+ /* + * When a guest FPU is supplied, use @guest_fpu->fpstate + * as reference independent whether it is in use or not. + */ + curfps = guest_fpu ? guest_fpu->fpstate : fpu->fpstate; + + /* Determine whether @curfps is the active fpstate */ + in_use = fpu->fpstate == curfps; + + if (guest_fpu) { + newfps->is_guest = true; + newfps->is_confidential = curfps->is_confidential; + newfps->in_use = curfps->in_use; + guest_fpu->xfeatures |= xfeatures; + } + fpregs_lock(); /* - * Ensure that the current state is in the registers before - * swapping fpstate as that might invalidate it due to layout - * changes. + * If @curfps is in use, ensure that the current state is in the + * registers before swapping fpstate as that might invalidate it + * due to layout changes. */ - if (test_thread_flag(TIF_NEED_FPU_LOAD)) + if (in_use && test_thread_flag(TIF_NEED_FPU_LOAD)) fpregs_restore_userregs();

newfps->xfeatures = curfps->xfeatures | xfeatures; newfps->user_xfeatures = curfps->user_xfeatures | xfeatures; newfps->xfd = curfps->xfd & ~xfeatures;

- curfps = fpu_install_fpstate(fpu, newfps); - /* Do the final updates within the locked region */ xstate_init_xcomp_bv(&newfps->regs.xsave, newfps->xfeatures); - xfd_update_state(newfps);

+ if (guest_fpu) { + guest_fpu->fpstate = newfps; + /* If curfps is active, update the FPU fpstate pointer */ + if (in_use) + fpu->fpstate = newfps; + } else { + fpu->fpstate = newfps; + } + + if (in_use) + xfd_update_state(fpu->fpstate); fpregs_unlock();

- vfree(curfps); + /* Only free valloc'ed state */ + if (curfps && curfps->is_valloc) + vfree(curfps); + return 0; }

@@ -1682,14 +1687,16 @@ static int xstate_request_perm(unsigned long idx, bool guest) return ret; }

-int xfd_enable_feature(u64 xfd_err) +int __xfd_enable_feature(u64 xfd_err, struct fpu_guest *guest_fpu) { u64 xfd_event = xfd_err & XFEATURE_MASK_USER_DYNAMIC; + struct fpu_state_perm *perm; unsigned int ksize, usize; struct fpu *fpu;

if (!xfd_event) { - pr_err_once("XFD: Invalid xfd error: %016llx\n", xfd_err); + if (!guest_fpu) + pr_err_once("XFD: Invalid xfd error: %016llx\n", xfd_err); return 0; }

@@ -1697,14 +1704,16 @@ int xfd_enable_feature(u64 xfd_err) spin_lock_irq(&current->sighand->siglock);

/* If not permitted let it die */ - if ((xstate_get_host_group_perm() & xfd_event) != xfd_event) { + if ((xstate_get_group_perm(!!guest_fpu) & xfd_event) != xfd_event) { spin_unlock_irq(&current->sighand->siglock); return -EPERM; }

fpu = &current->group_leader->thread.fpu; - ksize = fpu->perm.__state_size; - usize = fpu->perm.__user_state_size; + perm = guest_fpu ? &fpu->guest_perm : &fpu->perm; + ksize = perm->__state_size; + usize = perm->__user_state_size; + /* * The feature is permitted. State size is sufficient. Dropping * the lock is safe here even if more features are added from @@ -1717,10 +1726,16 @@ int xfd_enable_feature(u64 xfd_err) * Try to allocate a new fpstate. If that fails there is no way * out. */ - if (fpstate_realloc(xfd_event, ksize, usize)) + if (fpstate_realloc(xfd_event, ksize, usize, guest_fpu)) return -EFAULT; return 0; } + +int xfd_enable_feature(u64 xfd_err) +{ + return __xfd_enable_feature(xfd_err, NULL); +} + #else /* CONFIG_X86_64 */ static inline int xstate_request_perm(unsigned long idx, bool guest) { diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h index 98a472775c97..3e63f723525b 100644 --- a/arch/x86/kernel/fpu/xstate.h +++ b/arch/x86/kernel/fpu/xstate.h @@ -55,6 +55,8 @@ extern void fpu__init_system_xstate(unsigned int legacy_size);

extern void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr);

+extern int __xfd_enable_feature(u64 which, struct fpu_guest *guest_fpu); + static inline u64 xfeatures_mask_supervisor(void) { return fpu_kernel_cfg.max_features & XFEATURE_MASK_SUPERVISOR_SUPPORTED;

Statically enable all xfeatures allowed by guest perm in KVM_SET_CPUID2, with fpstate buffer sized accordingly. This avoids run-time expansion in the emulation and restore path of XCR0 and XFD MSR [1].

Change kvm_vcpu_after_set_cpuid() to return error given fpstate reallocation may fail.

[1] https://lore.kernel.org/all/20211214024948.048572883@linutronix.de/

Suggested-by: Thomas Gleixner tglx@linutronix.de Suggested-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Jing Liu jing2.liu@intel.com --- arch/x86/kvm/cpuid.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index a068373a7fbd..eb5a5070accb 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -204,10 +204,12 @@ void kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu) } EXPORT_SYMBOL_GPL(kvm_update_cpuid_runtime);

-static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) +static int kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) { struct kvm_lapic *apic = vcpu->arch.apic; struct kvm_cpuid_entry2 *best; + u64 xfeatures; + int r;

best = kvm_find_cpuid_entry(vcpu, 1, 0); if (best && apic) { @@ -222,9 +224,17 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) best = kvm_find_cpuid_entry(vcpu, 0xD, 0); if (!best) vcpu->arch.guest_supported_xcr0 = 0; - else - vcpu->arch.guest_supported_xcr0 = - (best->eax | ((u64)best->edx << 32)) & supported_xcr0; + else { + xfeatures = best->eax | ((u64)best->edx << 32); + + vcpu->arch.guest_supported_xcr0 = xfeatures & supported_xcr0; + + if (xfeatures != vcpu->arch.guest_fpu.xfeatures) { + r = fpu_update_guest_perm_features(&vcpu->arch.guest_fpu); + if (r) + return r; + } + }

/* * Bits 127:0 of the allowed SECS.ATTRIBUTES (CPUID.0x12.0x1) enumerate @@ -260,6 +270,8 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) * adjustments to the reserved GPA bits. */ kvm_mmu_after_set_cpuid(vcpu); + + return 0; }

int cpuid_query_maxphyaddr(struct kvm_vcpu *vcpu) @@ -301,9 +313,7 @@ static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2,

kvm_update_kvm_cpuid_base(vcpu); kvm_update_cpuid_runtime(vcpu); - kvm_vcpu_after_set_cpuid(vcpu); - - return 0; + return kvm_vcpu_after_set_cpuid(vcpu); }

/* when an old userspace process fills a new kernel module */

Intel's eXtended Feature Disable (XFD) feature allows the software to dynamically adjust fpstate buffer size for XSAVE features which have large state.

Because fpstate has been expanded for all possible dynamic xstates at KVM_SET_CPUID2, emulation of the IA32_XFD MSR is straightforward. For write just call fpu_update_guest_xfd() to update the guest fpu container once all the sanity checks are passed. For read then return the cached value in the container.

Signed-off-by: Zeng Guang guang.zeng@intel.com Signed-off-by: Wei Wang wei.w.wang@intel.com Signed-off-by: Jing Liu jing2.liu@intel.com --- arch/x86/kvm/x86.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index e50e97ac4408..36677b754ac9 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1359,6 +1359,7 @@ static const u32 msrs_to_save_all[] = { MSR_F15H_PERF_CTL3, MSR_F15H_PERF_CTL4, MSR_F15H_PERF_CTL5, MSR_F15H_PERF_CTR0, MSR_F15H_PERF_CTR1, MSR_F15H_PERF_CTR2, MSR_F15H_PERF_CTR3, MSR_F15H_PERF_CTR4, MSR_F15H_PERF_CTR5, + MSR_IA32_XFD, };

static u32 msrs_to_save[ARRAY_SIZE(msrs_to_save_all)]; @@ -3669,6 +3670,19 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) return 1; vcpu->arch.msr_misc_features_enables = data; break; +#ifdef CONFIG_X86_64 + case MSR_IA32_XFD: + if (!msr_info->host_initiated && + !guest_cpuid_has(vcpu, X86_FEATURE_XFD)) + return 1; + + if (data & ~(XFEATURE_MASK_USER_DYNAMIC & + vcpu->arch.guest_supported_xcr0)) + return 1; + + fpu_update_guest_xfd(&vcpu->arch.guest_fpu, data); + break; +#endif default: if (kvm_pmu_is_valid_msr(vcpu, msr)) return kvm_pmu_set_msr(vcpu, msr_info); @@ -3989,6 +4003,15 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) case MSR_K7_HWCR: msr_info->data = vcpu->arch.msr_hwcr; break; +#ifdef CONFIG_X86_64 + case MSR_IA32_XFD: + if (!msr_info->host_initiated && + !guest_cpuid_has(vcpu, X86_FEATURE_XFD)) + return 1; + + msr_info->data = vcpu->arch.guest_fpu.fpstate->xfd; + break; +#endif default: if (kvm_pmu_is_valid_msr(vcpu, msr_info->index)) return kvm_pmu_get_msr(vcpu, msr_info); @@ -6422,6 +6445,10 @@ static void kvm_init_msr_list(void) min(INTEL_PMC_MAX_GENERIC, x86_pmu.num_counters_gp)) continue; break; + case MSR_IA32_XFD: + if (!kvm_cpu_cap_has(X86_FEATURE_XFD)) + continue; + break; default: break; }

Guest IA32_XFD_ERR is generally modified in two places:

- Set by CPU when #NM is triggered; - Cleared by guest in its #NM handler;

Intercept #NM for the first case, if guest writes XFD as nonzero for the first time which indicates guest is possible to use XFD generating the exception. #NM is rare if the guest doesn't use dynamic features. Otherwise, there is at most one exception per guest task given a dynamic feature.

Save the current XFD_ERR value to the guest_fpu container in the #NM VM-exit handler. This must be done with interrupt/preemption disabled, otherwise the unsaved MSR value may be clobbered by host operations.

Inject a virtual #NM to the guest after saving the MSR value.

Restore the host value (always ZERO outside of the host #NM handler) before enabling preemption.

Restore the guest value from the guest_fpu container right before entering the guest (with preemption disabled).

Suggested-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Jing Liu jing2.liu@intel.com --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/vmx/vmcs.h | 5 +++++ arch/x86/kvm/vmx/vmx.c | 22 +++++++++++++++++++++- arch/x86/kvm/x86.c | 6 ++++++ 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 555f4de47ef2..f7a661f35d1a 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -640,6 +640,7 @@ struct kvm_vcpu_arch { u64 smi_count; bool tpr_access_reporting; bool xsaves_enabled; + bool trap_nm; u64 ia32_xss; u64 microcode_version; u64 arch_capabilities; diff --git a/arch/x86/kvm/vmx/vmcs.h b/arch/x86/kvm/vmx/vmcs.h index 6e5de2e2b0da..c57798b56f95 100644 --- a/arch/x86/kvm/vmx/vmcs.h +++ b/arch/x86/kvm/vmx/vmcs.h @@ -129,6 +129,11 @@ static inline bool is_machine_check(u32 intr_info) return is_exception_n(intr_info, MC_VECTOR); }

+static inline bool is_nm(u32 intr_info) +{ + return is_exception_n(intr_info, NM_VECTOR); +} + /* Undocumented: icebp/int1 */ static inline bool is_icebp(u32 intr_info) { diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 5974a88c9d35..57d256fbf34b 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -36,6 +36,7 @@ #include <asm/debugreg.h> #include <asm/desc.h> #include <asm/fpu/api.h> +#include <asm/fpu/xstate.h> #include <asm/idtentry.h> #include <asm/io.h> #include <asm/irq_remapping.h> @@ -763,6 +764,9 @@ void vmx_update_exception_bitmap(struct kvm_vcpu *vcpu) vmcs_write32(PAGE_FAULT_ERROR_CODE_MATCH, match); }

+ if (vcpu->arch.trap_nm) + eb |= (1u << NM_VECTOR); + vmcs_write32(EXCEPTION_BITMAP, eb); }

@@ -1960,6 +1964,13 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) case MSR_KERNEL_GS_BASE: vmx_write_guest_kernel_gs_base(vmx, data); break; + case MSR_IA32_XFD: + ret = kvm_set_msr_common(vcpu, msr_info); + if (!ret && data) { + vcpu->arch.trap_nm = true; + vmx_update_exception_bitmap(vcpu); + } + break; #endif case MSR_IA32_SYSENTER_CS: if (is_guest_mode(vcpu)) @@ -4746,7 +4757,7 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu) vect_info = vmx->idt_vectoring_info; intr_info = vmx_get_intr_info(vcpu);

- if (is_machine_check(intr_info) || is_nmi(intr_info)) + if (is_machine_check(intr_info) || is_nmi(intr_info) || is_nm(intr_info)) return 1; /* handled by handle_exception_nmi_irqoff() */

if (is_invalid_opcode(intr_info)) @@ -6350,6 +6361,12 @@ static void handle_interrupt_nmi_irqoff(struct kvm_vcpu *vcpu, kvm_after_interrupt(vcpu); }

+static void handle_exception_nm(struct kvm_vcpu *vcpu) +{ + rdmsrl(MSR_IA32_XFD_ERR, vcpu->arch.guest_fpu.xfd_err); + kvm_queue_exception(vcpu, NM_VECTOR); +} + static void handle_exception_nmi_irqoff(struct vcpu_vmx *vmx) { const unsigned long nmi_entry = (unsigned long)asm_exc_nmi_noist; @@ -6358,6 +6375,9 @@ static void handle_exception_nmi_irqoff(struct vcpu_vmx *vmx) /* if exit due to PF check for async PF */ if (is_page_fault(intr_info)) vmx->vcpu.arch.apf.host_apf_flags = kvm_read_and_reset_apf_flags(); + /* if exit due to NM, handle before preemptions are enabled */ + else if (is_nm(intr_info)) + handle_exception_nm(&vmx->vcpu); /* Handle machine checks before interrupts are enabled */ else if (is_machine_check(intr_info)) kvm_machine_check(); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 36677b754ac9..b22defad5cab 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -9893,6 +9893,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) if (test_thread_flag(TIF_NEED_FPU_LOAD)) switch_fpu_return();

+ if (vcpu->arch.guest_fpu.xfd_err) + wrmsrl(MSR_IA32_XFD_ERR, vcpu->arch.guest_fpu.xfd_err); + if (unlikely(vcpu->arch.switch_db_regs)) { set_debugreg(0, 7); set_debugreg(vcpu->arch.eff_db[0], 0); @@ -9956,6 +9959,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)

static_call(kvm_x86_handle_exit_irqoff)(vcpu);

+ if (vcpu->arch.guest_fpu.xfd_err) + wrmsrl(MSR_IA32_XFD_ERR, 0); + /* * Consume any pending interrupts, including the possible source of * VM-Exit on SVM and any ticks that occur between VM-Exit and now.

Disable read emulation of IA32_XFD_ERR MSR if guest cpuid includes XFD. This saves one unnecessary VM-exit in guest #NM handler, given that the MSR is already restored with the guest value before the guest is resumed.

Suggested-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Jing Liu jing2.liu@intel.com --- arch/x86/kvm/vmx/vmx.c | 6 ++++++ arch/x86/kvm/vmx/vmx.h | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 57d256fbf34b..8595a3e8fbd8 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -162,6 +162,7 @@ static u32 vmx_possible_passthrough_msrs[MAX_POSSIBLE_PASSTHROUGH_MSRS] = { MSR_FS_BASE, MSR_GS_BASE, MSR_KERNEL_GS_BASE, + MSR_IA32_XFD_ERR, #endif MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, @@ -7225,6 +7226,11 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) } }

+ if (boot_cpu_has(X86_FEATURE_XFD)) + vmx_set_intercept_for_msr(vcpu, MSR_IA32_XFD_ERR, MSR_TYPE_R, + !guest_cpuid_has(vcpu, X86_FEATURE_XFD)); + + set_cr4_guest_host_mask(vmx);

vmx_write_encls_bitmap(vcpu, NULL); diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h index 4df2ac24ffc1..bf9d3051cd6c 100644 --- a/arch/x86/kvm/vmx/vmx.h +++ b/arch/x86/kvm/vmx/vmx.h @@ -340,7 +340,7 @@ struct vcpu_vmx { struct lbr_desc lbr_desc;

/* Save desired MSR intercept (read: pass-through) state */ -#define MAX_POSSIBLE_PASSTHROUGH_MSRS 13 +#define MAX_POSSIBLE_PASSTHROUGH_MSRS 14 struct { DECLARE_BITMAP(read, MAX_POSSIBLE_PASSTHROUGH_MSRS); DECLARE_BITMAP(write, MAX_POSSIBLE_PASSTHROUGH_MSRS);

Extend CPUID emulation to support XFD, AMX_TILE, AMX_INT8 and AMX_BF16. Adding those bits into kvm_cpu_caps finally activates all previous logics in this series.

Signed-off-by: Yang Zhong yang.zhong@intel.com Signed-off-by: Jing Liu jing2.liu@intel.com --- arch/x86/include/asm/cpufeatures.h | 2 ++ arch/x86/kvm/cpuid.c | 25 +++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index d5b5f2ab87a0..da872b6f8d8b 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -299,7 +299,9 @@ /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */ #define X86_FEATURE_AVX_VNNI (12*32+ 4) /* AVX VNNI instructions */ #define X86_FEATURE_AVX512_BF16 (12*32+ 5) /* AVX512 BFLOAT16 instructions */ +#define X86_FEATURE_AMX_BF16 (18*32+22) /* AMX bf16 Support */ #define X86_FEATURE_AMX_TILE (18*32+24) /* AMX tile Support */ +#define X86_FEATURE_AMX_INT8 (18*32+25) /* AMX int8 Support */

/* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */ #define X86_FEATURE_CLZERO (13*32+ 0) /* CLZERO instruction */ diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index eb5a5070accb..1d694ead374e 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -515,7 +515,8 @@ void kvm_set_cpu_caps(void) F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) | F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) | F(MD_CLEAR) | F(AVX512_VP2INTERSECT) | F(FSRM) | - F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16) + F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16) | + F(AMX_TILE) | F(AMX_INT8) | F(AMX_BF16) );

/* TSC_ADJUST and ARCH_CAPABILITIES are emulated in software. */ @@ -534,7 +535,7 @@ void kvm_set_cpu_caps(void) );

kvm_cpu_cap_mask(CPUID_D_1_EAX, - F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | F(XSAVES) + F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | F(XSAVES) | F(XFD) );

kvm_cpu_cap_init_scattered(CPUID_12_EAX, @@ -660,6 +661,8 @@ static struct kvm_cpuid_entry2 *do_host_cpuid(struct kvm_cpuid_array *array, case 0x14: case 0x17: case 0x18: + case 0x1d: + case 0x1e: case 0x1f: case 0x8000001d: entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX; @@ -932,6 +935,24 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) goto out; } break; + /* Intel AMX TILE */ + case 0x1d: + if (!kvm_cpu_cap_has(X86_FEATURE_AMX_TILE)) { + entry->eax = entry->ebx = entry->ecx = entry->edx = 0; + break; + } + + for (i = 1, max_idx = entry->eax; i <= max_idx; ++i) { + if (!do_host_cpuid(array, function, i)) + goto out; + } + break; + case 0x1e: /* TMUL information */ + if (!kvm_cpu_cap_has(X86_FEATURE_AMX_TILE)) { + entry->eax = entry->ebx = entry->ecx = entry->edx = 0; + break; + } + break; case KVM_CPUID_SIGNATURE: { const u32 *sigptr = (const u32 *)KVM_SIGNATURE; entry->eax = KVM_CPUID_FEATURES;

From: Guang Zeng guang.zeng@intel.com

When AMX is enabled it requires a larger xstate buffer than the legacy hardcoded 4KB one. Exising kvm ioctls (KVM_[G|S]ET_XSAVE under KVM_CAP_XSAVE) are not suitable for this purpose.

A new capability (KVM_CAP_XSAVE2) is introduced to mark an extended kvm_xsave format to support >4KB fpstate. The expanded fpstate size is returned to userspace when it checks the KVM_CAP_XSAVE2 capability.

Introduce a new KVM_GET_XSAVE2 under this capability to use the new format for copying guest fpstate to userspace. Reuse KVM_SET_XSAVE for both old/new formats by reimplementing it to do properly-sized memdup_user() based on the guest fpu container.

Also, update the api doc with the new KVM_GET_XSAVE2 ioctl.

Signed-off-by: Guang Zeng guang.zeng@intel.com Signed-off-by: Wei Wang wei.w.wang@intel.com Signed-off-by: Jing Liu jing2.liu@intel.com --- Documentation/virt/kvm/api.rst | 42 +++++++++++++++++++++++++++++++-- arch/x86/include/uapi/asm/kvm.h | 16 ++++++++++++- arch/x86/kvm/x86.c | 39 +++++++++++++++++++++++++++++- include/uapi/linux/kvm.h | 4 ++++ 4 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 1cf2483246cd..e48f7de5f23a 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -1566,6 +1566,7 @@ otherwise it will return EBUSY error.

struct kvm_xsave { __u32 region[1024]; + __u32 extra[0]; };

This ioctl would copy current vcpu's xsave struct to the userspace. @@ -1574,7 +1575,7 @@ This ioctl would copy current vcpu's xsave struct to the userspace. 4.43 KVM_SET_XSAVE ------------------

-:Capability: KVM_CAP_XSAVE +:Capability: KVM_CAP_XSAVE and KVM_CAP_XSAVE2 :Architectures: x86 :Type: vcpu ioctl :Parameters: struct kvm_xsave (in) @@ -1585,9 +1586,18 @@ This ioctl would copy current vcpu's xsave struct to the userspace.

struct kvm_xsave { __u32 region[1024]; + __u32 extra[0]; };

-This ioctl would copy userspace's xsave struct to the kernel. +This ioctl would copy userspace's xsave struct to the kernel. It copies +as many bytes as are returned by KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2), +when invoked on the vm file descriptor. The size value returned by +KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2) will always be at least 4096. +Currently, it is only greater than 4096 if a dynamic feature has been +enabled with ``arch_prctl()``, but this may change in the future. + +The offsets of the state save areas in struct kvm_xsave follow the +contents of CPUID leaf 0xD on the host.

4.44 KVM_GET_XCRS @@ -5507,6 +5517,34 @@ the trailing ``'\0'``, is indicated by ``name_size`` in the header. The Stats Data block contains an array of 64-bit values in the same order as the descriptors in Descriptors block.

+4.42 KVM_GET_XSAVE2 +------------------ + +:Capability: KVM_CAP_XSAVE2 +:Architectures: x86 +:Type: vcpu ioctl +:Parameters: struct kvm_xsave (out) +:Returns: 0 on success, -1 on error + + +:: + + struct kvm_xsave { + __u32 region[1024]; + __u32 extra[0]; + }; + +This ioctl would copy current vcpu's xsave struct to the userspace. It +copies as many bytes as are returned by KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2) +when invoked on the vm file descriptor. The size value returned by +KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2) will always be at least 4096. +Currently, it is only greater than 4096 if a dynamic feature has been +enabled with ``arch_prctl()``, but this may change in the future. + +The offsets of the state save areas in struct kvm_xsave follow the contents +of CPUID leaf 0xD on the host. + + 5. The kvm_run structure ========================

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h index 5a776a08f78c..2da3316bb559 100644 --- a/arch/x86/include/uapi/asm/kvm.h +++ b/arch/x86/include/uapi/asm/kvm.h @@ -373,9 +373,23 @@ struct kvm_debugregs { __u64 reserved[9]; };

-/* for KVM_CAP_XSAVE */ +/* for KVM_CAP_XSAVE and KVM_CAP_XSAVE2 */ struct kvm_xsave { + /* + * KVM_GET_XSAVE2 and KVM_SET_XSAVE write and read as many bytes + * as are returned by KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2) + * respectively, when invoked on the vm file descriptor. + * + * The size value returned by KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2) + * will always be at least 4096. Currently, it is only greater + * than 4096 if a dynamic feature has been enabled with + * ``arch_prctl()``, but this may change in the future. + * + * The offsets of the state save areas in struct kvm_xsave follow + * the contents of CPUID leaf 0xD on the host. + */ __u32 region[1024]; + __u32 extra[0]; };

#define KVM_MAX_XCRS 16 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c558c098979a..3b756ff13103 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4297,6 +4297,11 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) else r = 0; break; + case KVM_CAP_XSAVE2: + r = kvm->vcpus[0]->arch.guest_fpu.uabi_size; + if (r < sizeof(struct kvm_xsave)) + r = sizeof(struct kvm_xsave); + break; default: break; } @@ -4900,6 +4905,16 @@ static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu, vcpu->arch.pkru); }

+static void kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu, + u8 *state, unsigned int size) +{ + if (fpstate_is_confidential(&vcpu->arch.guest_fpu)) + return; + + fpu_copy_guest_fpstate_to_uabi(&vcpu->arch.guest_fpu, + state, size, vcpu->arch.pkru); +} + static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu, struct kvm_xsave *guest_xsave) { @@ -5367,7 +5382,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, break; } case KVM_SET_XSAVE: { - u.xsave = memdup_user(argp, sizeof(*u.xsave)); + int size = vcpu->arch.guest_fpu.uabi_size; + + u.xsave = memdup_user(argp, size); if (IS_ERR(u.xsave)) { r = PTR_ERR(u.xsave); goto out_nofree; @@ -5376,6 +5393,26 @@ long kvm_arch_vcpu_ioctl(struct file *filp, r = kvm_vcpu_ioctl_x86_set_xsave(vcpu, u.xsave); break; } + + case KVM_GET_XSAVE2: { + int size = vcpu->arch.guest_fpu.uabi_size; + + u.xsave = kzalloc(size, GFP_KERNEL_ACCOUNT); + if (!u.xsave) { + r = -ENOMEM; + break; + } + + kvm_vcpu_ioctl_x86_get_xsave2(vcpu, u.buffer, size); + + if (copy_to_user(argp, u.xsave, size)) { + r = -EFAULT; + break; + } + r = 0; + break; + } + case KVM_GET_XCRS: { u.xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL_ACCOUNT); r = -ENOMEM; diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 1daa45268de2..9d1c01669560 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1131,6 +1131,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_EXIT_ON_EMULATION_FAILURE 204 #define KVM_CAP_ARM_MTE 205 #define KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM 206 +#define KVM_CAP_XSAVE2 207

#ifdef KVM_CAP_IRQ_ROUTING

@@ -1610,6 +1611,9 @@ struct kvm_enc_region { #define KVM_S390_NORMAL_RESET _IO(KVMIO, 0xc3) #define KVM_S390_CLEAR_RESET _IO(KVMIO, 0xc4)

+/* Available with KVM_CAP_XSAVE2 */ +#define KVM_GET_XSAVE2 _IOR(KVMIO, 0xcf, struct kvm_xsave) + struct kvm_s390_pv_sec_parm { __u64 origin; __u64 length;

From: Thomas Gleixner tglx@linutronix.de

KVM can disable the write emulation for the XFD MSR when the vCPU's fpstate is already correctly sized to reduce the overhead.

When write emulation is disabled the XFD MSR state after a VMEXIT is unknown and therefore not in sync with the software states in fpstate and the per CPU XFD cache.

Provide fpu_sync_guest_vmexit_xfd_state() which has to be invoked after a VMEXIT before enabling interrupts when write emulation is disabled for the XFD MSR.

It could be invoked unconditionally even when write emulation is enabled for the price of a pointless MSR read.

Signed-off-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Jing Liu jing2.liu@intel.com --- arch/x86/include/asm/fpu/api.h | 2 ++ arch/x86/kernel/fpu/core.c | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+)

diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h index 100b535cd3de..dc51809f0e4a 100644 --- a/arch/x86/include/asm/fpu/api.h +++ b/arch/x86/include/asm/fpu/api.h @@ -142,8 +142,10 @@ extern int fpu_update_guest_perm_features(struct fpu_guest *guest_fpu);

#ifdef CONFIG_X86_64 extern void fpu_update_guest_xfd(struct fpu_guest *guest_fpu, u64 xfd); +extern void fpu_sync_guest_vmexit_xfd_state(void); #else static inline void fpu_update_guest_xfd(struct fpu_guest *guest_fpu, u64 xfd) { } +static inline void fpu_sync_guest_vmexit_xfd_state(void) { } #endif

extern void fpu_copy_guest_fpstate_to_uabi(struct fpu_guest *gfpu, void *buf, unsigned int size, u32 pkru); diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c index 89d679cc819b..42a195a6b2ce 100644 --- a/arch/x86/kernel/fpu/core.c +++ b/arch/x86/kernel/fpu/core.c @@ -299,6 +299,30 @@ void fpu_update_guest_xfd(struct fpu_guest *guest_fpu, u64 xfd) fpregs_unlock(); } EXPORT_SYMBOL_GPL(fpu_update_guest_xfd); + +/** + * fpu_sync_guest_vmexit_xfd_state - Synchronize XFD MSR and software state + * + * Must be invoked from KVM after a VMEXIT before enabling interrupts when + * XFD write emulation is disabled. This is required because the guest can + * freely modify XFD and the state at VMEXIT is not guaranteed to be the + * same as the state on VMENTER. So software state has to be udpated before + * any operation which depends on it can take place. + * + * Note: It can be invoked unconditionally even when write emulation is + * enabled for the price of a then pointless MSR read. + */ +void fpu_sync_guest_vmexit_xfd_state(void) +{ + struct fpstate *fps = current->thread.fpu.fpstate; + + lockdep_assert_irqs_disabled(); + if (fpu_state_size_dynamic()) { + rdmsrl(MSR_IA32_XFD, fps->xfd); + __this_cpu_write(xfd_state, fps->xfd); + } +} +EXPORT_SYMBOL_GPL(fpu_sync_guest_vmexit_xfd_state); #endif /* CONFIG_X86_64 */

int fpu_swap_kvm_fpstate(struct fpu_guest *guest_fpu, bool enter_guest)

On Wed, Dec 22, 2021, Jing Liu wrote:

Guest IA32_XFD_ERR is generally modified in two places:

• Set by CPU when #NM is triggered;
• Cleared by guest in its #NM handler;

Intercept #NM for the first case, if guest writes XFD as nonzero for the first time which indicates guest is possible to use XFD generating the exception. #NM is rare if the guest doesn't use dynamic features. Otherwise, there is at most one exception per guest task given a dynamic feature.

Save the current XFD_ERR value to the guest_fpu container in the #NM VM-exit handler. This must be done with interrupt/preemption disabled,

Assuming my below understanding is correct, drop the "preemption" bit, it's misleading.

otherwise the unsaved MSR value may be clobbered by host operations.

Inject a virtual #NM to the guest after saving the MSR value.

Restore the host value (always ZERO outside of the host #NM handler) before enabling preemption.

AIUI, changelog is wrong, code is right. This must be done before _IRQs_ are enabled, same as handling TIF_NEED_FPU_LOAD.

Restore the guest value from the guest_fpu container right before entering the guest (with preemption disabled).

Same complaint about preemption.

Suggested-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Jing Liu jing2.liu@intel.com

---

arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/vmx/vmcs.h | 5 +++++ arch/x86/kvm/vmx/vmx.c | 22 +++++++++++++++++++++- arch/x86/kvm/x86.c | 6 ++++++ 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 555f4de47ef2..f7a661f35d1a 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -640,6 +640,7 @@ struct kvm_vcpu_arch { u64 smi_count; bool tpr_access_reporting; bool xsaves_enabled;

• bool trap_nm; u64 ia32_xss; u64 microcode_version; u64 arch_capabilities;

...

@@ -763,6 +764,9 @@ void vmx_update_exception_bitmap(struct kvm_vcpu *vcpu) vmcs_write32(PAGE_FAULT_ERROR_CODE_MATCH, match); }

• if (vcpu->arch.trap_nm)

• eb |= (1u << NM_VECTOR);
  

• vmcs_write32(EXCEPTION_BITMAP, eb);

} @@ -1960,6 +1964,13 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) case MSR_KERNEL_GS_BASE: vmx_write_guest_kernel_gs_base(vmx, data); break;

• case MSR_IA32_XFD:

• ret = kvm_set_msr_common(vcpu, msr_info);
  

• if (!ret && data) {
  

• 	vcpu->arch.trap_nm = true;
  

• 	vmx_update_exception_bitmap(vcpu);
  

This is wrong, it fails to clear vcpu->arch.trap_nm and update the bitmap if the MSR is cleared.

But why even bother with an extra flag? Can't vmx_update_exception_bitmap() get the guest's MSR_IA32_XFD value and intercept #NM accordingly? Then you could even handle this fully in kvm_set_msr_common(), e.g.

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2c9606380bca..c6c936d2b298 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3704,6 +3704,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) return 1;

fpu_update_guest_xfd(&vcpu->arch.guest_fpu, data); + /* Blah blah blah blah */ + static_call(kvm_x86_update_exception_bitmap)(vcpu); break; case MSR_IA32_XFD_ERR: if (!msr_info->host_initiated &&

• }
  

• break;
  

#endif case MSR_IA32_SYSENTER_CS: if (is_guest_mode(vcpu)) @@ -4746,7 +4757,7 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu) vect_info = vmx->idt_vectoring_info; intr_info = vmx_get_intr_info(vcpu);

• if (is_machine_check(intr_info) || is_nmi(intr_info))

• if (is_machine_check(intr_info) || is_nmi(intr_info) || is_nm(intr_info)) return 1; /* handled by handle_exception_nmi_irqoff() */

if (is_invalid_opcode(intr_info)) @@ -6350,6 +6361,12 @@ static void handle_interrupt_nmi_irqoff(struct kvm_vcpu *vcpu, kvm_after_interrupt(vcpu); } +static void handle_exception_nm(struct kvm_vcpu *vcpu)

This needs a different name, it's waaaay too close to the base handle_exception_nmi(), which runs with IRQs _on_. And please add "_irqoff" at the end. Maybe handle_nm_fault_irqoff()?

+{

• rdmsrl(MSR_IA32_XFD_ERR, vcpu->arch.guest_fpu.xfd_err);
• kvm_queue_exception(vcpu, NM_VECTOR);

+}

static void handle_exception_nmi_irqoff(struct vcpu_vmx *vmx) { const unsigned long nmi_entry = (unsigned long)asm_exc_nmi_noist; @@ -6358,6 +6375,9 @@ static void handle_exception_nmi_irqoff(struct vcpu_vmx *vmx) /* if exit due to PF check for async PF */ if (is_page_fault(intr_info)) vmx->vcpu.arch.apf.host_apf_flags = kvm_read_and_reset_apf_flags();

• /* if exit due to NM, handle before preemptions are enabled */
• else if (is_nm(intr_info))

Same naming complaint about this helper, it looks like an is_nmi() typo. is_nm_fault()?

• handle_exception_nm(&vmx->vcpu);
  

  /* Handle machine checks before interrupts are enabled */ else if (is_machine_check(intr_info)) kvm_machine_check();

Shortlog needs to have a verb somewhere.

On Wed, Dec 22, 2021, Jing Liu wrote:

From: Guang Zeng guang.zeng@intel.com

When AMX is enabled it requires a larger xstate buffer than the legacy hardcoded 4KB one. Exising kvm ioctls

Existing

(KVM_[G|S]ET_XSAVE under KVM_CAP_XSAVE) are not suitable for this purpose.

...

Reuse KVM_SET_XSAVE for both old/new formats by reimplementing it to do properly-sized memdup_user() based on the guest fpu container.

I'm confused, the first sentence says KVM_SET_XSAVE isn't suitable, the second says it can be reused with minimal effort.

Also, update the api doc with the new KVM_GET_XSAVE2 ioctl.

...

@@ -5367,7 +5382,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp, break; } case KVM_SET_XSAVE: {

• u.xsave = memdup_user(argp, sizeof(*u.xsave));
  

• int size = vcpu->arch.guest_fpu.uabi_size;
  

IIUC, reusing KVM_SET_XSAVE works by requiring that userspace use KVM_GET_XSAVE2 if userspace has expanded the guest FPU size by exposing relevant features to the guest via guest CPUID. If so, then that needs to be enforced in KVM_GET_XSAVE, otherwise userspace will get subtle corruption by invoking the wrong ioctl, e.g.

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2c9606380bca..5d2acbd52df5 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5386,6 +5386,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp, break; } case KVM_GET_XSAVE: { + r -EINVAL; + if (vcpu->arch.guest_fpu.uabi_size > sizeof(struct kvm_xsave)) + break; + u.xsave = kzalloc(sizeof(struct kvm_xsave), GFP_KERNEL_ACCOUNT); r = -ENOMEM; if (!u.xsave)

• u.xsave = memdup_user(argp, size);
  

  if (IS_ERR(u.xsave)) { r = PTR_ERR(u.xsave); goto out_nofree;

@@ -5376,6 +5393,26 @@ long kvm_arch_vcpu_ioctl(struct file *filp, r = kvm_vcpu_ioctl_x86_set_xsave(vcpu, u.xsave); break; }

• case KVM_GET_XSAVE2: {

• int size = vcpu->arch.guest_fpu.uabi_size;
  

• u.xsave = kzalloc(size, GFP_KERNEL_ACCOUNT);
  

• if (!u.xsave) {
  

• 	r = -ENOMEM;
  

I hate the odd patterns in this code as much as anyone, but for better or worse the style throughout is:

r = -ENOMEM; u.xsave = kzalloc(size, GFP_KERNEL_ACCOUNT); if (u.xsave) break;

• 	break;
  

• }
  

• kvm_vcpu_ioctl_x86_get_xsave2(vcpu, u.buffer, size);
  

• if (copy_to_user(argp, u.xsave, size)) {
  

• 	r = -EFAULT;
  

• 	break;
  

Same style thing here.

• }
  

• r = 0;
  

• break;
  

• }
• case KVM_GET_XCRS: { u.xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL_ACCOUNT); r = -ENOMEM;

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 1daa45268de2..9d1c01669560 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1131,6 +1131,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_EXIT_ON_EMULATION_FAILURE 204 #define KVM_CAP_ARM_MTE 205 #define KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM 206 +#define KVM_CAP_XSAVE2 207 #ifdef KVM_CAP_IRQ_ROUTING @@ -1610,6 +1611,9 @@ struct kvm_enc_region { #define KVM_S390_NORMAL_RESET _IO(KVMIO, 0xc3) #define KVM_S390_CLEAR_RESET _IO(KVMIO, 0xc4) +/* Available with KVM_CAP_XSAVE2 */ +#define KVM_GET_XSAVE2 _IOR(KVMIO, 0xcf, struct kvm_xsave)

struct kvm_s390_pv_sec_parm { __u64 origin; __u64 length; -- 2.27.0

From: Sean Christopherson seanjc@google.com Sent: Wednesday, December 29, 2021 8:10 AM

On Wed, Dec 22, 2021, Jing Liu wrote:

...

Guest IA32_XFD_ERR is generally modified in two places:

• Set by CPU when #NM is triggered;
• Cleared by guest in its #NM handler;

Intercept #NM for the first case, if guest writes XFD as nonzero for the first time which indicates guest is possible to use XFD generating the exception. #NM is rare if the guest doesn't use dynamic features. Otherwise, there is at most one exception per guest task given a dynamic feature.

Save the current XFD_ERR value to the guest_fpu container in the #NM VM-exit handler. This must be done with interrupt/preemption disabled,

Assuming my below understanding is correct, drop the "preemption" bit, it's misleading.

code-wise yes. In concept we just want to highlight that this operation must be completed when both interrupt and preemption are disabled.

But we can also drop preemption if you prefer to, since preemption is certainly disabled when interrupt is disabled.

...

otherwise the unsaved MSR value may be clobbered by host operations.

Inject a virtual #NM to the guest after saving the MSR value.

Restore the host value (always ZERO outside of the host #NM handler) before enabling preemption.

AIUI, changelog is wrong, code is right. This must be done before _IRQs_ are enabled, same as handling TIF_NEED_FPU_LOAD.

yes

...

Restore the guest value from the guest_fpu container right before entering the guest (with preemption disabled).

Same complaint about preemption.

...

Suggested-by: Thomas Gleixner tglx@linutronix.de Signed-off-by: Jing Liu jing2.liu@intel.com

---

arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/vmx/vmcs.h | 5 +++++ arch/x86/kvm/vmx/vmx.c | 22 +++++++++++++++++++++- arch/x86/kvm/x86.c | 6 ++++++ 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h

b/arch/x86/include/asm/kvm_host.h

...

index 555f4de47ef2..f7a661f35d1a 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -640,6 +640,7 @@ struct kvm_vcpu_arch { u64 smi_count; bool tpr_access_reporting; bool xsaves_enabled;

• bool trap_nm; u64 ia32_xss; u64 microcode_version; u64 arch_capabilities;

...

...

@@ -763,6 +764,9 @@ void vmx_update_exception_bitmap(struct

kvm_vcpu *vcpu)

...

vmcs_write32(PAGE_FAULT_ERROR_CODE_MATCH, match);

}

• if (vcpu->arch.trap_nm)

• eb |= (1u << NM_VECTOR);
  

• vmcs_write32(EXCEPTION_BITMAP, eb);

}

@@ -1960,6 +1964,13 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu,

struct msr_data *msr_info)

...

case MSR_KERNEL_GS_BASE: vmx_write_guest_kernel_gs_base(vmx, data); break;

• case MSR_IA32_XFD:

• ret = kvm_set_msr_common(vcpu, msr_info);
  

• if (!ret && data) {
  

• 	vcpu->arch.trap_nm = true;
  

• 	vmx_update_exception_bitmap(vcpu);
  

This is wrong, it fails to clear vcpu->arch.trap_nm and update the bitmap if the MSR is cleared.

In concept you are right if just looking at this patch. It's pointless to trap #NM if guest xfd is cleared.

But here we need think about patch22 which disables write interception for xfd. With that in consideration we use the 1st non-zero write as the hint indicating that guest might enable xfd-related usages thus always trap #NM after this point.

It's not a good ordering, but Paolo wants to put the optimization in the end of this series. But we do need to put a clear comment here explaining the always-trap policy.

But why even bother with an extra flag? Can't vmx_update_exception_bitmap() get the guest's MSR_IA32_XFD value and intercept #NM accordingly? Then you

Above is the reason for the extra flag

could even handle this fully in kvm_set_msr_common(), e.g.

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2c9606380bca..c6c936d2b298 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3704,6 +3704,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) return 1;

            fpu_update_guest_xfd(&vcpu->arch.guest_fpu, data);

•           /* Blah blah blah blah */
  

•           static_call(kvm_x86_update_exception_bitmap)(vcpu);
            break;
    case MSR_IA32_XFD_ERR:
            if (!msr_info->host_initiated &&
  
  

...

• }
  

• break;
  

#endif case MSR_IA32_SYSENTER_CS: if (is_guest_mode(vcpu)) @@ -4746,7 +4757,7 @@ static int handle_exception_nmi(struct kvm_vcpu

*vcpu)

...

vect_info = vmx->idt_vectoring_info; intr_info = vmx_get_intr_info(vcpu);

• if (is_machine_check(intr_info) || is_nmi(intr_info))

• if (is_machine_check(intr_info) || is_nmi(intr_info) ||

is_nm(intr_info))

...

return 1; /* handled by handle_exception_nmi_irqoff() */

if (is_invalid_opcode(intr_info)) @@ -6350,6 +6361,12 @@ static void handle_interrupt_nmi_irqoff(struct

kvm_vcpu *vcpu,

...

kvm_after_interrupt(vcpu); }

+static void handle_exception_nm(struct kvm_vcpu *vcpu)

This needs a different name, it's waaaay too close to the base handle_exception_nmi(), which runs with IRQs _on_. And please add "_irqoff" at the end. Maybe handle_nm_fault_irqoff()?

sounds good.

...

+{

• rdmsrl(MSR_IA32_XFD_ERR, vcpu->arch.guest_fpu.xfd_err);
• kvm_queue_exception(vcpu, NM_VECTOR);

+}

static void handle_exception_nmi_irqoff(struct vcpu_vmx *vmx) { const unsigned long nmi_entry = (unsigned long)asm_exc_nmi_noist; @@ -6358,6 +6375,9 @@ static void handle_exception_nmi_irqoff(struct

vcpu_vmx *vmx)

...

/* if exit due to PF check for async PF */ if (is_page_fault(intr_info)) vmx->vcpu.arch.apf.host_apf_flags =

kvm_read_and_reset_apf_flags();

...

• /* if exit due to NM, handle before preemptions are enabled */
• else if (is_nm(intr_info))

Same naming complaint about this helper, it looks like an is_nmi() typo. is_nm_fault()?

will fix

...

• handle_exception_nm(&vmx->vcpu);
  

  /* Handle machine checks before interrupts are enabled */ else if (is_machine_check(intr_info)) kvm_machine_check();

From: Sean Christopherson seanjc@google.com Sent: Wednesday, December 29, 2021 8:21 AM

On Wed, Dec 22, 2021, Jing Liu wrote:

...

Two XCR0 bits are defined for AMX to support XSAVE mechanism. Bit 17 is for tilecfg and bit 18 is for tiledata.

The value of XCR0[17:18] is always either 00b or 11b.

Is that an SDM requirement, or an arbitrary Linux/KVM requirement?

SDM requirement

...

Also, SDM recommends that only 64-bit operating systems enable Intel AMX by setting XCR0[18:17]. If a 32-bit guest tries to set dynamic bits, it

This is wrong. It has nothing to do with 32-bit guests. The restriction is on 32-bit _host kernels_, which I'm guessing never set the tile bits in _host_ XCR0.

make sense.

...

fails to pass vcpu->arch.guest_supported_xcr0 check and gets a #GP.

Signed-off-by: Yang Zhong yang.zhong@intel.com Signed-off-by: Jing Liu jing2.liu@intel.com

---

arch/x86/kvm/x86.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index a48a89f73027..c558c098979a 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -210,7 +210,7 @@ static struct kvm_user_return_msrs __percpu

*user_return_msrs;

...

#define KVM_SUPPORTED_XCR0 (XFEATURE_MASK_FP |

XFEATURE_MASK_SSE \

...

		| XFEATURE_MASK_YMM |

XFEATURE_MASK_BNDREGS \

...

		| XFEATURE_MASK_BNDCSR |

XFEATURE_MASK_AVX512 \

...

• 		| XFEATURE_MASK_PKRU)
  

• 		| XFEATURE_MASK_PKRU |
  

XFEATURE_MASK_XTILE)

...

u64 __read_mostly host_efer; EXPORT_SYMBOL_GPL(host_efer); @@ -990,6 +990,12 @@ static int __kvm_set_xcr(struct kvm_vcpu *vcpu,

u32 index, u64 xcr)

...

if ((xcr0 & XFEATURE_MASK_AVX512) !=

XFEATURE_MASK_AVX512)

...

	return 1;

}

+#ifdef CONFIG_X86_64

Drop the #ifdef, it adds no meaningful value and requires the reader to think far harder than they should have. Yes, it's technically dead code for 32-bit KVM, but no one cares about performance of 32-bit KVM, and in any case it's extremely unlikely this will be anything but noise.

ok

...

• if ((xcr0 & XFEATURE_MASK_XTILE) &&

•    ((xcr0 & XFEATURE_MASK_XTILE) != XFEATURE_MASK_XTILE))
  

• return 1;
  

+#endif vcpu->arch.xcr0 = xcr0;

if ((xcr0 ^ old_xcr0) & XFEATURE_MASK_EXTEND)

2.27.0