> On May 31, 2023, at 5:09 PM, Jon Kohler <jon@xxxxxxxxxxx> wrote:
>
>
>>
>> The CPUID bits that enumerate support for a feature are independent from the CPUID
>> bits that enumerate what XCR0 bits are supported, i.e. what features can be saved
>> and restored via XSAVE/XRSTOR.
>>
>> KVM does mostly account for host XCR0, but in a very ad hoc way. E.g. MPX is
>> handled by manually checking host XCR0.
>>
>> if (kvm_mpx_supported())
>> kvm_cpu_cap_check_and_set(X86_FEATURE_MPX);
>>
>> PKU manually checks too, but indirectly by looking at whether or not the kernel
>> has enabled CR4.OSPKE.
>>
>> if (!tdp_enabled || !boot_cpu_has(X86_FEATURE_OSPKE))
>> kvm_cpu_cap_clear(X86_FEATURE_PKU);
>>
>> But unless I'm missing something, the various AVX and AMX bits rely solely on
>> boot_cpu_data, i.e. would break if someone added CONFIG_X86_AVX or CONFIG_X86_AMX.
>
> What if we simply moved static unsigned short xsave_cpuid_features[] … into xstate.h, which
> is already included in arch/x86/kvm/cpuid.c, and do something similar to what I’m proposing in this
> patch already
>
> This would future proof such breakages I’d imagine?
>
> void kvm_set_cpu_caps(void)
> {
> ...
> /*
> * Clear CPUID for XSAVE features that are disabled.
> */
> for (i = 0; i < ARRAY_SIZE(xsave_cpuid_features); i++) {
> unsigned short cid = xsave_cpuid_features[i];
>
> /* Careful: X86_FEATURE_FPU is 0! */
> if ((i != XFEATURE_FP && !cid) || !boot_cpu_has(cid) ||
> !cpu_feature_enabled(cid))
> kvm_cpu_cap_clear(cid);
> }
> …
> }
>
Sean - following up on this rough idea code above, wanted to validate that this was the
direction you were thinking of having kvm_set_cpu_caps() clear caps when a particular
xsave feature was disabled?
