Re: [PATCH v4 3/4] KVM: TDX: Filter configurable CPUID bits

[Edgecombe, Rick P]

On Thu, 2026-09-17 at 15:25 +0800, Binbin Wu wrote:
> Filter the directly configurable CPUID bits reported through
> KVM_TDX_CAPABILITIES against KVM's TDX allowlist, and drop the hardcoded
> denylist based filtering.
> 
> The TDX module reports directly configurable CPUID bits that it supports
> for a TD, but KVM must not expose bits that it doesn't support, as blindly
> exposing a host state clobbering feature can lead to host state corruption.
> The existing denylist, which clears only TSX and WAITPKG, is not fail-safe.
> 
> Add tdx_get_cpuid_cfg_mask() to get the mask of directly configurable bits
> allowed by KVM for a given CPUID register, covering both feature bits,
> which come from tdx_cpu_cfg_caps[], and non-feature bits, which are
> enumerated at runtime.  Apply the mask to every CPUID entry reported
> through KVM_TDX_CAPABILITIES.
> 
> With the allowlist in place, newly introduced TDX directly configurable
> CPUID bits stay hidden from userspace until KVM explicitly opts in.
> 
> Note, filtering KVM_TDX_CAPABILITIES intentionally stops advertising the
> directly configurable bits that aren't in the allowlist, as the
> corresponding features are unsupported or cannot be properly virtualized.

I think we need to more loudly call out that this is an ABI change and we expect
it not to impact other VMMs because...

> 
> Update the documentation to reflect the ABI change.
> 
> Signed-off-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
> ---
> v4:
> - Rename functions. (Tony)
>   tdx_cfg_non_feature_mask()       -> tdx_get_cpuid_cfg_non_feature_mask()
>   tdx_cfg_feature_mask()           -> tdx_get_cpuid_cfg_feature_mask()
>   tdx_get_allowed_cfg_cpuid_mask() -> tdx_get_cpuid_cfg_mask()
> - Update documentation. (Xiaoyao, Rick)
> 
> v3:
> - Handle non-feature leafs at runtime. (Sean)
> - Add CPUID.0x24.0.EBX[7:0] into allowlist. There is a mismatch of the
>   description about CPUID.0x24.0.EBX[7:0], which is listed as
>   "XFAM & CPUID_Enabled & Native" but should be "XFAM & CPUID_Enabled &
>   Configured & Native".
> ---
>  Documentation/virt/kvm/x86/intel-tdx.rst |  8 ++
>  arch/x86/kvm/vmx/tdx.c                   | 95 ++++++++++++++++++------
>  2 files changed, 80 insertions(+), 23 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/x86/intel-tdx.rst b/Documentation/virt/kvm/x86/intel-tdx.rst
> index 6a222e9d09541..6beeb89d7f057 100644
> --- a/Documentation/virt/kvm/x86/intel-tdx.rst
> +++ b/Documentation/virt/kvm/x86/intel-tdx.rst
> @@ -69,6 +69,14 @@ Return the TDX capabilities that current KVM supports with the specific TDX
>  module loaded in the system.  It reports what features/capabilities are allowed
>  to be configured to the TDX guest.
>  
> +Note, a CPUID feature bit that the TDX module reports as directly configurable
> +is not necessarily reported as configurable by KVM.  KVM omits features it
> +doesn't support.  Generally speaking, KVM reports a feature as configurable
> +only if KVM supports the feature for both TDX and non-TDX VMs, though there are
> +a handful of exceptions where KVM allows a feature for TDX guests that it never
> +advertises for non-TDX VMs.  Userspace must rely on KVM_TDX_CAPABILITIES to
> +determine what can be passed in cpuid to KVM_TDX_INIT_VM.
> +
>  - id: KVM_TDX_CAPABILITIES
>  - flags: must be 0
>  - data: pointer to struct kvm_tdx_capabilities
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index b34afc52b714e..ad7d70b36ffe5 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -294,36 +294,79 @@ static bool has_tsx(const struct kvm_cpuid_entry2 *entry)
>  	       (entry->ebx & TDX_FEATURE_TSX);
>  }
>  
> -static void clear_tsx(struct kvm_cpuid_entry2 *entry)
> -{
> -	entry->ebx &= ~TDX_FEATURE_TSX;
> -}
> -
>  static bool has_waitpkg(const struct kvm_cpuid_entry2 *entry)
>  {
>  	return entry->function == 7 && entry->index == 0 &&
>  	       (entry->ecx & __feature_bit(X86_FEATURE_WAITPKG));
>  }
>  
> -static void clear_waitpkg(struct kvm_cpuid_entry2 *entry)
> -{
> -	entry->ecx &= ~__feature_bit(X86_FEATURE_WAITPKG);
> -}
> -
> -static void tdx_clear_unsupported_cpuid(struct kvm_cpuid_entry2 *entry)
> -{
> -	if (has_tsx(entry))
> -		clear_tsx(entry);
> -
> -	if (has_waitpkg(entry))
> -		clear_waitpkg(entry);
> -}
> -
>  static bool tdx_unsupported_cpuid(const struct kvm_cpuid_entry2 *entry)
>  {
>  	return has_tsx(entry) || has_waitpkg(entry);
>  }
>  
> +#define TDX_CPUID_ALL_ALLOWED_MASK	GENMASK_U32(31, 0)
> +
> +static u32 tdx_get_cpuid_cfg_non_feature_mask(u32 function, u32 index, int reg)
> +{
> +	/*
> +	 * For a leaf/subleaf/register that will never be repurposed to hold
> +	 * feature bits, it's safe to return TDX_CPUID_ALL_ALLOWED_MASK, i.e.
> +	 * leave the TDX module's CPUID config mask intact.
> +	 */
> +	switch (function) {
> +	case 1:
> +		if (reg == CPUID_EAX || reg == CPUID_EBX)
> +			return TDX_CPUID_ALL_ALLOWED_MASK;
> +		return 0;
> +	case 4:
> +	case 0x18:
> +	case 0x1f:
> +		return TDX_CPUID_ALL_ALLOWED_MASK;
> +	case 0x24:
> +		if (index == 0 && reg == CPUID_EBX)
> +			return GENMASK_U32(7, 0);
> +		return 0;
> +	case 0x80000008:
> +		if (reg == CPUID_EAX)
> +			return TDX_CPUID_ALL_ALLOWED_MASK;

There are reserved bits in some of these. Are these "no feature bit" assumptions
checked somehow?