Re: [RFC PATCH v2 1/4] KVM: x86: TDX: Track supported configurable CPUID bits

[Binbin Wu]

On 6/26/2026 1:04 AM, Sean Christopherson wrote:
> On Thu, Jun 04, 2026, Binbin Wu wrote:
>> ---
>>  arch/x86/kvm/vmx/tdx.c | 174 +++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 174 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> index ffe9d0db58c5..e0567088ebf5 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
>> @@ -52,6 +52,178 @@
>>  	__TDX_BUG_ON(__err, #__fn, __kvm, ", " #a1 " 0x%llx, " #a2 ", 0x%llx, " #a3 " 0x%llx", \
>>  		     a1, a2, a3)
>>  
>> +#define TDX_CPUID_IGNORE_INDEX	BIT(0)
>> +struct tdx_supported_cpuid_reg {
>> +	u32 function;
>> +	u32 index;
>> +	u8 flags;
>> +	u8 reg;
>> +	u32 mask;
>> +};
>> +
>> +/*
>> + * Multi-bit fields are statically initialized, feature bits are initialized
>> + * in tdx_initialize_cpu_cfg_caps().
>> + */
>> +static struct tdx_supported_cpuid_reg tdx_kvm_supported_cpuid[] __ro_after_init = {
>> +	{ 0x1, 0, 0, CPUID_EAX, GENMASK_U32(27, 16) | GENMASK_U32(13, 0) },
>> +	{ 0x1, 0, 0, CPUID_EBX, GENMASK_U32(23, 16) },
>> +	{ 0x1, 0, 0, CPUID_ECX, 0 },
>> +	{ 0x1, 0, 0, CPUID_EDX, 0 },
>> +	{ 0x4, 0, TDX_CPUID_IGNORE_INDEX, CPUID_EAX, ~GENMASK_U32(13, 10) },
>> +	{ 0x4, 0, TDX_CPUID_IGNORE_INDEX, CPUID_EBX, GENMASK_U32(31, 12) },
>> +	{ 0x4, 0, TDX_CPUID_IGNORE_INDEX, CPUID_ECX, GENMASK_U32(31, 0) },
>> +	{ 0x4, 0, TDX_CPUID_IGNORE_INDEX, CPUID_EDX, GENMASK_U32(2, 0) },
>> +	{ 0x7, 0, 0, CPUID_EBX, 0 },
>> +	{ 0x7, 0, 0, CPUID_ECX, 0 },
>> +	{ 0x7, 0, 0, CPUID_EDX, 0 },
>> +	{ 0x7, 1, 0, CPUID_EAX, 0 },
>> +	{ 0x7, 1, 0, CPUID_EDX, 0 },
>> +	{ 0x7, 2, 0, CPUID_EDX, 0 },
>> +	{ 0x18, 0, TDX_CPUID_IGNORE_INDEX, CPUID_EDX, GENMASK_U32(25, 14) },
>> +	{ 0x1E, 1, 0, CPUID_EAX, 0 },
>> +	{ 0x1F, 0, TDX_CPUID_IGNORE_INDEX, CPUID_EAX, GENMASK_U32(4, 0) },
>> +	{ 0x1F, 0, TDX_CPUID_IGNORE_INDEX, CPUID_EBX, GENMASK_U32(15, 0) },
>> +	{ 0x1F, 0, TDX_CPUID_IGNORE_INDEX, CPUID_ECX, GENMASK_U32(15, 0) },
>> +	/* See comments in td_init_cpuid_entry2() for CPUID 0x80000008 EAX[23:16]. */
>> +	{ 0x80000008, 0, 0, CPUID_EAX, GENMASK_U32(23, 16) | GENMASK_U32(7, 0) },
>> +	{ 0x80000008, 0, 0, CPUID_EBX, 0 },
> 
> For non-feature bits, I think I would rather handle them entirely at runtime via
> switch statement(s).  Realistically, CPUID.0x1.E{A,B}X are never going to be
> repurposed to hold feature bits, and so generating a mask of allowed bits adds
> unnecessary cognitive load and maintenance.  Ditto for CPUID 0x4, 0x18, and 0x1F.

Yes, it's a bit over-engineered with the matter of fact that these are never going
to hold feature bit.

> 
> CPUID.0x1E is a bit different because it's kinda sorta a feature?  That one is
> probably worth restricting, but again that's easy to do in a case-statement.

Only CPUID.0x1E.EAX has TDX directly configurable bits currently, no special
handling needed for the rest of CPUID.0x1E.

> 
> Then for the feature bits, there should be no need to define a separate structure,
> just do "u32 kvm_tdx_cpu_caps[NR_KVM_CPU_CAPS]".  Then KVM can even further
> restrict that array with kvm_cpu_caps (though it might take some creativity to
> deal with things like MWAIT).  Because generally speaking, KVM shouldn't allow
> features that KVM doesn't support for non-TDX VMs.
> 

OK, will go this direction in the next version.
Thanks for the suggestions.