All of lore.kernel.org
 help / color / mirror / Atom feed
From: Binbin Wu <binbin.wu@linux.intel.com>
To: Sean Christopherson <seanjc@google.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	pbonzini@redhat.com, dave.hansen@linux.intel.com,
	andrew.cooper3@citrix.com, nik.borisov@suse.com, kas@kernel.org,
	rick.p.edgecombe@intel.com, xiaoyao.li@intel.com,
	chao.gao@intel.com, tony.lindgren@linux.intel.com,
	kishen.maloor@intel.com, dedekind1@gmail.com,
	adrian.hunter@intel.com
Subject: Re: [PATCH v5 1/5] KVM: TDX: Check unsupported CPUID bits against the TDX module's subleaf
Date: Sat, 17 Oct 2026 06:15:15 +0800	[thread overview]
Message-ID: <e1553e03-7ca8-4507-b608-d9d663759c1f@linux.intel.com> (raw)
In-Reply-To: <atJB7kDzKsvzrMau@google.com>

On 10/16/2026 11:25 PM, Sean Christopherson wrote:
> On Fri, Oct 16, 2026, Binbin Wu wrote:
>> Don't reject the mismatch outright, as the ABI allows userspace to provide
>> an arbitrary index when KVM_CPUID_FLAG_SIGNIFCANT_INDEX is cleared.  It's
>> userspace's responsibility to provide the intended entries; KVM only needs
>> to ensure that the values it validates are the values it applies.
>>
>> Fixes: 6d415778f106 ("KVM: TDX: Disable support for TSX and WAITPKG")
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Closes: https://lore.kernel.org/kvm/20260604024927.E22311F00893@smtp.kernel.org
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
>> ---
>> v5:
>> - Split out from "KVM: TDX: Validate userspace CPUID input for
>>   KVM_TDX_INIT_VM" as a standalone fix.  Sashiko reported the issue
>>   against the TDX CPUID validation patch, but it also applies to the
>>   existing code, so split it out to allow backporting to stable
>>   independently of the rest of the series.
>> - Check the userspace values against the subleaf reported by the TDX
>>   module instead of rejecting the request when there is a mismatch.
>>
>>   A simpler alternative would be to require a zero index when
>>   KVM_CPUID_FLAG_SIGNIFCANT_INDEX is cleared, but that would be an ABI
>>   change.
> 
> The even simpler change is to reject the incoming CPUID if userspace doesn't set
> KVM_CPUID_FLAG_SIGNIFCANT_INDEX as expected.  Yes, that's an ABI change.  No, we
> don't care.  The rule isn't "never change the ABI", it's "don't break userspace",
> and there's basically zero chance a TDX guest can function properly if userspace
> doesn't set KVM_CPUID_FLAG_SIGNIFCANT_INDEX as appropriate.
> 
> diff --git arch/x86/kvm/vmx/tdx.c arch/x86/kvm/vmx/tdx.c
> index 75a86d64892a..7e0d81066420 100644
> --- arch/x86/kvm/vmx/tdx.c
> +++ arch/x86/kvm/vmx/tdx.c
> @@ -151,8 +151,12 @@ static void tdx_clear_unsupported_cpuid(struct kvm_cpuid_entry2 *entry)
>                 clear_waitpkg(entry);
>  }
>  
> -static bool tdx_unsupported_cpuid(const struct kvm_cpuid_entry2 *entry)
> +static bool tdx_is_unsupported_cpuid_entry(const struct kvm_cpuid_entry2 *entry)
>  {
> +       if (cpuid_function_is_indexed(entry->function) !=
> +           !!(entry->flags & KVM_CPUID_FLAG_SIGNIFCANT_INDEX))
> +               return true;
> +
>         return has_tsx(entry) || has_waitpkg(entry);
>  }
>  
> @@ -2386,7 +2390,7 @@ static int setup_tdparams_cpuids(struct kvm_cpuid2 *cpuid,
>                 if (!entry)
>                         continue;
>  
> -               if (tdx_unsupported_cpuid(entry))
> +               if (tdx_is_unsupported_cpuid_entry(entry))
>                         return -EINVAL;
>  
>                 copy_cnt++;
> 
> Can you test the attached patches?  If they boot TDX with QEMU, I'll go ahead
> and post.

Patch 3 "KVM: TDX: Don't zero non-significant CPUID entry indices" breaks QEMU.

QEMU uses zero as index for cpuid entries when index is not significant.
And in QEMU, the lookup function doesn't consult the flags at all.

struct kvm_cpuid_entry2 *cpuid_find_entry(struct kvm_cpuid2 *cpuid,
                                          uint32_t function,
                                          uint32_t index)
{
    int i;
    for (i = 0; i < cpuid->nent; ++i) {
        if (cpuid->entries[i].function == function &&
            cpuid->entries[i].index == index) {
            return &cpuid->entries[i];
        }
    }
    /* not found: */
    return NULL;
}

If KVM doesn't zero the index for the non-significant, QEMU cannot find
any entry for non-significant entries returned by KVM_TDX_CAPABILITIES.

And 0x80000008 is used to pass GPAW, and KVM_TDX_INIT_VM fails when QEMU
doesn't pass it.

After dropping patch 3, QEMU can boot TD.
Also our internal TDX CI cases passed.

  reply	other threads:[~2026-10-16 22:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16  9:14 [PATCH v5 0/5] KVM: TDX: Validate directly configurable CPUID bits Binbin Wu
2026-10-16  9:14 ` [PATCH v5 1/5] KVM: TDX: Check unsupported CPUID bits against the TDX module's subleaf Binbin Wu
2026-10-16 15:25   ` Sean Christopherson
2026-10-16 22:15     ` Binbin Wu [this message]
2026-10-16 23:26       ` Sean Christopherson
2026-10-16  9:14 ` [PATCH v5 2/5] KVM: TDX: Track configurable CPUID bits allowed by KVM Binbin Wu
2026-10-16  9:14 ` [PATCH v5 3/5] KVM: TDX: Add CORE_CAPABILITIES to the allowlist Binbin Wu
2026-10-16  9:14 ` [PATCH v5 4/5] KVM: TDX: Report only KVM-allowed CPUIDs in KVM_TDX_CAPABILITIES Binbin Wu
2026-10-16  9:14 ` [PATCH v5 5/5] KVM: TDX: Validate userspace CPUID input for KVM_TDX_INIT_VM Binbin Wu
2026-10-16  9:24   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e1553e03-7ca8-4507-b608-d9d663759c1f@linux.intel.com \
    --to=binbin.wu@linux.intel.com \
    --cc=adrian.hunter@intel.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=chao.gao@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dedekind1@gmail.com \
    --cc=kas@kernel.org \
    --cc=kishen.maloor@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nik.borisov@suse.com \
    --cc=pbonzini@redhat.com \
    --cc=rick.p.edgecombe@intel.com \
    --cc=seanjc@google.com \
    --cc=tony.lindgren@linux.intel.com \
    --cc=xiaoyao.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.