> > static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
> > {
> > struct kvm_cpuid_entry2 *entry;
> > @@ -982,6 +992,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
> > u64 permitted_xcr0 = kvm_caps.supported_xcr0 & xstate_get_guest_group_perm();
> > u64 permitted_xss = kvm_caps.supported_xss;
> >
> > + permitted_xcr0 = sanitize_xcr0(permitted_xcr0);
>
>
> This isn't 100% correct, all usage needs to be sanitized so that KVM provides a
> consistent view. E.g. KVM_CAP_XSAVE2 would report the wrong size.
>
> case KVM_CAP_XSAVE2: {
> u64 guest_perm = xstate_get_guest_group_perm();
>
> r = xstate_required_size(kvm_caps.supported_xcr0 & guest_perm, false);
> if (r < sizeof(struct kvm_xsave))
> r = sizeof(struct kvm_xsave);
> break;
> }
>
> Barring a kernel bug, xstate_get_guest_group_perm() will never report partial
> support, so I think the easy solution is to sanitize kvm_caps.suport_xcr0.
>
When I run xcr0_cpuid_test it fails because
xstate_get_guest_group_perm() reports partial support on SPR. It's
reporting 0x206e7 rather than the 0x6e7 I was hoping for. That's why
I went down the road of sanitizing xcr0. Though, if it's expected for
that to report something valid then sanitizing seems like the wrong
approach. If xcr0 is invalid it should stay invalid, and it should
cause a test to fail.
Looking at how xstate_get_guest_group_perm() comes through with
invalid bits I came across this commit:
2308ee57d93d ("x86/fpu/amx: Enable the AMX feature in 64-bit mode")
- /* [XFEATURE_XTILE_DATA] = XFEATURE_MASK_XTILE, */
+ [XFEATURE_XTILE_DATA] = XFEATURE_MASK_XTILE_DATA,
Seems like it should really be:
+ [XFEATURE_XTILE_DATA] = XFEATURE_MASK_XTILE,
With that change xstate_get_guest_group_perm() should no longer report
partial support.
That means this entire series can be simplified to a 'fixes patch' for
commit 2308ee57d93d and xcr0_cpuid_test to demonstrate the fix.
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2480b8027a45..7ea06c58eaf6 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -9344,6 +9344,10 @@ int kvm_arch_init(void *opaque)
> if (boot_cpu_has(X86_FEATURE_XSAVE)) {
> host_xcr0 = xgetbv(XCR_XFEATURE_ENABLED_MASK);
> kvm_caps.supported_xcr0 = host_xcr0 & KVM_SUPPORTED_XCR0;
> + if (!(kvm_caps.supported_xcr0 & XFEATURE_MASK_BNDREGS) ||
> + !(kvm_caps.supported_xcr0 & XFEATURE_MASK_BNDCSR))
> + kvm_caps.supported_xcr0 &= ~(XFEATURE_MASK_BNDREGS |
> + XFEATURE_MASK_BNDCSR);
> }
>
> if (pi_inject_timer == -1)
>
>
> > +
> > entry->eax &= permitted_xcr0;
> > entry->ebx = xstate_required_size(permitted_xcr0, false);
> > entry->ecx = entry->ebx;
> > --
> > 2.39.0.314.g84b9a713c41-goog
> >
