Re: [PATCH v3 1/4] KVM: TDX: Track configurable CPUID bits allowed by KVM

[Xiaoyao Li]

On 9/3/2026 12:19 AM, Binbin Wu wrote:
> On 9/2/2026 11:09 PM, Xiaoyao Li wrote:
>> On 9/2/2026 8:33 AM, Binbin Wu wrote:
>>>>> +static void __init tdx_initialize_cpu_cfg_caps(void)
>>>>> +{
>>>>> +    tdx_cpu_cfg_cap_init(CPUID_1_ECX,
>>>>> +        TDX_CFG_EXTRA_F(MWAIT),
>>>>> +        TDX_CFG_F(TSC_DEADLINE_TIMER),
>>>>> +        TDX_CFG_F(AVX),
>>>>> +        TDX_CFG_F(F16C),
>>>>> +    );
>>>> TDX 1.5.24 on SPR report configurable bits of CPUID_1_ECX as
>>>> 0x31044988, which have
>>>>
>>>> - bit 3        MWAIT
>>>> - bit 7        EST
>>>> - bit 8        TM2
>>>> - bit 11    SDBG
>>>> - bit 14    XTPR
>>>> - bit 18    DCA
>>>> - bit 24    TSC_DEADLINE_TIMER
>>>> - bit 28    AVX
>>>> - bit 29    F16C
>>>>
>>>> but EST/TM2/SDBG/XTPR/DCA are not list here. I guess the reason is kvm_cpu_cap[] doesn't support it. If so, it seems to guard twice:
>>>> 1. mentally/manually check if it a feature is supported in kvm_cpu_caps[]
>>>>
>>>> 2. kvm_cpu_caps guarding in tdx_cpu_cfg_cap_init().
>>>>
>>>> I think 1) is not necessary, we can rely on 2)
>>> In general, if a feature is not supported by the common KVM CPU caps, 
>> For kvm-intel.ko, kvm_cpu_caps[] just means the supported CPUID features
>> for VMX VMs. Treat it as the common KVM CPU caps is a bit arguable.
>>
>>> I prefer not
>>> to add it to the list to save a few lines of code, which probably is dead code,
>> I don't think it's dead code. It shows that these features are
>> virtualizable to TDs from the POV. of TDX.
> It depends on whether KVM allows userspace to set features for TDs that are not support
> for non-TDX VMs (,except for a few exceptions).
> 
> In this version, TDX_CFG_F() already check against kvm_cpu_caps[], if these features
> are not in kvm_cpu_caps[], it will not be exposed to userspace anyway.
>

I still think the reasoning that we omit them because they are not
contained in kvm_cpu_caps[] sounds not right. Based on it, so when we are
going to add a new feature for TDX, we need to first manually check the KVM
code to see if that feature is contained in kvm_cpu_caps[] already. If not,
we just don't add it to TDX's list. Then why need to cap the result
kvm_cpu_caps[] for TDX_CFG_F() again? Just for safety in case human make
mistake and misread the code of kvm_cpu_caps[]?

I think they are two independent steps:
1. list the CPUID features that KVM can support for TDs.
2. apply additional restrictions, e.g., if a feature is not allowed for
non-TDX VMs, it cannot be allowed for TDs.

>> In the end, they might be disallowed to be configured to TDs because KVM
>> doesn't allow them for VMX VMs. This is also the point I want to discuss.
>> Do we really want to make such restriction that KVM cannot enable/allow a
>> feature for TDs unless KVM first enables/allows it for VMX VMs? What's
>> reason behind it?
> Sean mentioned it that "generally speaking, KVM shouldn't allow features
> that KVM doesn't support for non-TDX VMs" in
> https://lore.kernel.org/kvm/aj1fi_0SBxMK5WOB@xxxxxxxxxx/

For existing features, it might make some sense. But for new features, I
don't think so. It defines the enabling order for new features that we must
enable a feature for non-TDX VMs first and then TDs. And people might want
to bypass this rule by abusing the TDX_CFG_EXTRA_F() when only one line of
TDX_CFG_EXTRA_F() is enough to enable a feature for TDs but more effort
required to enable it for non-TDX VMs.

Maybe I miss somthing. I would like to see stronger reasons for such decision.