Re: [PATCH 25/25] KVM: x86: Add CPUID bits missing from KVM_GET_SUPPORTED_CPUID

[Chao Gao <chao.gao@intel.com>]

On Mon, Aug 12, 2024 at 03:48:20PM -0700, Rick Edgecombe wrote:
>Originally, the plan was to filter the directly configurable CPUID bits
>exposed by KVM_TDX_CAPABILITIES, and the final configured bit values
>provided by KVM_TDX_GET_CPUID. However, several issues were found with
>this. Both the filtering done with KVM_TDX_CAPABILITIES and
>KVM_TDX_GET_CPUID had the issue that the get_supported_cpuid() provided
>default values instead of supported masks for multi-bit fields (i.e. those
>encoding a multi-bit number).
>
>For KVM_TDX_CAPABILITIES, there was also the problem of bits that are
>actually supported by KVM, but missing from get_supported_cpuid() for one
>reason or another. These include X86_FEATURE_MWAIT, X86_FEATURE_HT and
>X86_FEATURE_TSC_DEADLINE_TIMER. This is currently worked around in QEMU by
>adjusting which features are expected. Some of these are going to be added
>to get_supported_cpuid(), and that is probably the right long term fix.
>
>For KVM_TDX_GET_CPUID, there is another problem. Some CPUID bits are fixed
>on by the TDX module, but unsupported by KVM. This means that the TD will
>have them set, but KVM and userspace won't know about them. This class of

What's the problem of having KVM and userspace see some unsupported bits set?

>bits is dealt with by having QEMU expect not to see them. The bits include:
>X86_FEATURE_HYPERVISOR. The proper fix for this specifically is probably to
>change KVM to show it as supported (currently a patch exists). But this
>scenario could be expected in the end of TDX module ever setting and
>default 1, or fixed 1 bits. It would be good to have discussion on whether
>KVM community should mandate that this doesn't happen.

Just my two cents:

Mandating that all fixed-1 bits be supported by KVM would be a burden for both
KVM and the TDX module: the TDX module couldn't add any fixed-1 bits until KVM
supports them, and KVM shouldn't drop any feature that was ever a fixed-1 bit
in any TDX module. I don't think this is a good idea. TDX module support for a
feature will likely be ready earlier than KVM's, as TDX module is smaller and
is developed inside Intel. Requiring the TDX module to avoid adding fixed-1
bits doesn't make much sense, as making all features configurable would
increase its complexity.

I think adding new fixed-1 bits is fine as long as they don't break KVM, i.e.,
KVM shouldn't need to take any action for the new fixed-1 bits, like
saving/restoring more host CPU states across TD-enter/exit or emulating
CPUID/MSR accesses from guests