linux-kernel - Re: [PATCH 25/25] KVM: x86: Add CPUID bits missing from KVM_GET_SUPPORTED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZrtEvEh4UJ6ZbPq5@chao-email>
Date: Tue, 13 Aug 2024 19:34:20 +0800
From: Chao Gao <chao.gao@...el.com>
To: Rick Edgecombe <rick.p.edgecombe@...el.com>
CC: <seanjc@...gle.com>, <pbonzini@...hat.com>, <kvm@...r.kernel.org>,
	<kai.huang@...el.com>, <isaku.yamahata@...il.com>,
	<tony.lindgren@...ux.intel.com>, <xiaoyao.li@...el.com>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 25/25] KVM: x86: Add CPUID bits missing from
 KVM_GET_SUPPORTED_CPUID

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