All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Chao Gao <chao.gao@intel.com>
Cc: Binbin Wu <binbin.wu@linux.intel.com>,
	kvm@vger.kernel.org,  linux-kernel@vger.kernel.org,
	x86@kernel.org, pbonzini@redhat.com,  dave.hansen@intel.com,
	kas@kernel.org, rick.p.edgecombe@intel.com,
	 vishal.l.verma@intel.com, xiaoyao.li@intel.com
Subject: Re: [PATCH 1/2] KVM: TDX: Allow TDs to read MSR_IA32_PLATFORM_ID
Date: Tue, 28 Apr 2026 09:30:03 -0700	[thread overview]
Message-ID: <afDgiz38tARlIQNr@google.com> (raw)
In-Reply-To: <afCdoufK0D+1hKAR@intel.com>

On Tue, Apr 28, 2026, Chao Gao wrote:
> On Tue, Apr 28, 2026 at 10:47:45AM +0800, Binbin Wu wrote:
> >Add MSR_IA32_PLATFORM_ID to tdx_has_emulated_msr() so that TDs can read
> >it.
> >
> >Linux kernel reads MSR_IA32_PLATFORM_ID during init since commit
> >d8630b67ca1e ("x86/cpu: Add platform ID to CPU info structure").  KVM
> >already supports this MSR on read for normal VMs by returning 0.
> >Without support for this MSR, TDs get unchecked MSR access errors.
> >
> >    unchecked MSR access error: RDMSR from 0x17 at rIP: 0xffffffffba38d6fc (intel_get_platform_id+0x7c/0xb0)
> >    Call Trace:
> >     <TASK>
> >     ? early_init_intel+0x28/0x2c0
> >     ? early_cpu_init+0x9b/0x930
> >     ? setup_arch+0xbf/0xbb0
> >     ? _printk+0x6b/0x90
> >     ? start_kernel+0x7f/0xaa0
> >     ? x86_64_start_reservations+0x24/0x30
> >     ? x86_64_start_kernel+0xda/0xe0
> >     ? common_startup_64+0x13e/0x141
> >     </TASK>
> >
> >Add MSR_IA32_PLATFORM_ID in tdx_has_emulated_msr() to allow TDs to read
> >the MSR.  MSR_IA32_PLATFORM_ID is read-only by hardware definition, i.e.
> >KVM should never add it as writable, no need to add it in
> >tdx_is_read_only_msr().
> >
> >Fixes: dd50294f3e3c ("KVM: TDX: Implement callbacks for MSR operations")
> >Reported-by: Vishal Verma <vishal.l.verma@intel.com>
> >Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
> >---
> > arch/x86/kvm/vmx/tdx.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> >index 04ce321ebdf3..812ad99b11e5 100644
> >--- a/arch/x86/kvm/vmx/tdx.c
> >+++ b/arch/x86/kvm/vmx/tdx.c
> >@@ -2094,6 +2094,7 @@ void tdx_get_exit_info(struct kvm_vcpu *vcpu, u32 *reason,
> > bool tdx_has_emulated_msr(u32 index)
> > {
> > 	switch (index) {
> >+	case MSR_IA32_PLATFORM_ID:
> > 	case MSR_IA32_UCODE_REV:
> > 	case MSR_IA32_ARCH_CAPABILITIES:
> > 	case MSR_IA32_POWER_CTL:
> 
> This patch looks good to me. But the rule for which MSRs should be emulated
> by KVM for TDX is not very clear to me.

I would strongly prefer to not take this patch, and instead fix the guest.  This
isn't some latent/pre-existing guest behavior, it's brand new functionality.
I.e. Linux-as-a-TDX-guest broke itself.

More importantly from a guest security perspective, consuming MSR_IA32_PLATFORM_ID
is actively dangerous.  E.g. it could allow the untrusted host to steer the guest's
behavior with respect to feature detection and enablement.

And once KVM allows reads from MSR_IA32_PLATFORM_ID, there's no going back.  E.g.
if the TDX-Module wants to emulate MSR_IA32_PLATFORM_ID, because there's an actual
*need* to do so, then we're going to have a (minor) mess with KVM's ABI.

> Maybe we can document the rule here, if there is one. That would make it
> much easier to tell whether future issues like this are guest regressions
> or missing KVM handling.

  reply	other threads:[~2026-04-28 16:30 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-28  2:47 [PATCH 0/2] Fix MSR_IA32_PLATFORM_ID access for TDX guests Binbin Wu
2026-04-28  2:47 ` [PATCH 1/2] KVM: TDX: Allow TDs to read MSR_IA32_PLATFORM_ID Binbin Wu
2026-04-28  5:31   ` Xiaoyao Li
2026-04-28 11:44   ` Chao Gao
2026-04-28 16:30     ` Sean Christopherson [this message]
2026-04-28 18:31       ` Edgecombe, Rick P
2026-04-28 18:44         ` Sean Christopherson
2026-04-28 19:28           ` Edgecombe, Rick P
2026-04-28 18:49   ` Dave Hansen
2026-04-29  9:09     ` Binbin Wu
2026-04-28  2:47 ` [PATCH 2/2] x86/cpu: Skip reading MSR_IA32_PLATFORM_ID in virtualized environment Binbin Wu
2026-04-28  6:01   ` Xiaoyao Li
2026-04-28  9:57     ` Binbin Wu
2026-04-28 18:54     ` Edgecombe, Rick P
2026-04-28 19:13       ` Dave Hansen
2026-04-29 23:14   ` Dave Hansen

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=afDgiz38tARlIQNr@google.com \
    --to=seanjc@google.com \
    --cc=binbin.wu@linux.intel.com \
    --cc=chao.gao@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=kas@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rick.p.edgecombe@intel.com \
    --cc=vishal.l.verma@intel.com \
    --cc=x86@kernel.org \
    --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.