Received-SPF: pass (google.com: domain of linux-kernel+bounces-140635-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 11 Apr 2024 07:22:44 -0700
In-Reply-To: <b1d112bf0ff55073c4e33a76377f17d48dc038ac.camel@intel.com>
Precedence: bulk
Mime-Version: 1.0
References: <ZhQ8UCf40UeGyfE_@google.com> <5faaeaa7bc66dbc4ea86a64ef8e8f9b22fd22ef4.camel@intel.com>
 <ZhRxWxRLbnrqwQYw@google.com> <957b26d18ba7db611ed6582366066667267d10b8.camel@intel.com>
 <ZhSb28hHoyJ55-ga@google.com> <8b40f8b1d1fa915116ef1c95a13db0e55d3d91f2.camel@intel.com>
 <ZhVdh4afvTPq5ssx@google.com> <4ae4769a6f343a2f4d3648e4348810df069f24b7.camel@intel.com>
 <ZhVsHVqaff7AKagu@google.com> <b1d112bf0ff55073c4e33a76377f17d48dc038ac.camel@intel.com>
Message-ID: <ZhfyNLKsTBUOI7Vp@google.com>
Subject: Re: [ANNOUNCE] PUCK Notes - 2024.04.03 - TDX Upstreaming Strategy
From: Sean Christopherson <seanjc@google.com>
To: Rick P Edgecombe <rick.p.edgecombe@intel.com>
Cc: "davidskidmore@google.com" <davidskidmore@google.com>, Xiaoyao Li <xiaoyao.li@intel.com>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
	"srutherford@google.com" <srutherford@google.com>, "pankaj.gupta@amd.com" <pankaj.gupta@amd.com>, 
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>, Isaku Yamahata <isaku.yamahata@intel.com>, 
	Wei W Wang <wei.w.wang@intel.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 11, 2024, Rick P Edgecombe wrote:
> On Tue, 2024-04-09 at 09:26 -0700, Sean Christopherson wrote:
> > > Haha, if this is the confusion, I see why you reacted that way to "JS=
ON".
> > > That would be quite the curious choice for a TDX module API.
> > >=20
> > > So it is easy to convert it to a C struct and embed it in KVM. It's j=
ust not
> > > that useful because it will not necessarily be valid for future TDX m=
odules.
> >=20
> > No, I don't want to embed anything in KVM, that's the exact same as har=
dcoding
> > crud into KVM, which is what I want to avoid.=C2=A0 I want to be able t=
o roll out a
> > new TDX module with any kernel changes, and I want userspace to be able=
 to
> > assert
> > that, for a given TDX module, the effective guest CPUID configuration a=
ligns
> > with
> > userspace's desired the vCPU model, i.e. that the value of fixed bits m=
atch up
> > with the guest CPUID that userspace wants to define.
> >=20
> > Maybe that just means converting the JSON file into some binary format =
that
> > the
> > kernel can already parse.=C2=A0 But I want Intel to commit to providing=
 that
> > metadata
> > along with every TDX module.
>=20
> Oof. It turns out in one of the JSON files there is a description of a di=
fferent
> interface (TDX module runtime interface) that provides a way to read CPUI=
D data
> that is configured in a TD, including fixed bits. It works like:
> 1. VMM queries which CPUID bits are directly configurable.
> 2. VMM provides directly configurable CPUID bits, along with XFAM and
> ATTRIBUTES, via TDH.MNG.INIT. (KVM_TDX_INIT_VM)
> 3. Then VMM can use this other interface via TDH.MNG.RD, to query the res=
ulting
> values of specific CPUID leafs.
>=20
> This does not provide a way to query the fixed bits specifically, it tell=
s you
> what ended up getting configuring in a specific TD, which includes the fi=
xed
> bits and anything else. So we need to do KVM_TDX_INIT_VM before KVM_SET_C=
PUID in
> order to have something to check against. But there was discussion of
> KVM_SET_CPUID on CPU0 having the CPUID state to pass to KVM_TDX_INIT_VM. =
So that
> would need to be sorted.
>=20
> If we pass the directly configurable values with KVM_TDX_INIT_VM, like we=
 do
> today, then the data provided by this interface should allow us to check
> consistency between KVM_SET_CPUID and the actual configured TD CPUID beha=
vior.

I think it would be a good (optional?) sanity check, e.g. KVM_BUG_ON() if t=
he
post-KVM_TDX_INIT_VM CPUID set doesn't match KVM's internal data.  But that=
 alone
provides a terrible experience for userspace.

 - The VMM would still need to hardcode knowledge of fixed bits, without a =
way
   to do a sanity check of its own.

 - Lack of a sanity check means the VMM can't fail VM creation early.

 - KVM_SET_CPUID2 doesn't have a way to inform userspace _which_ CPUID bits=
 are
   "bad".

 - Neither userspace nor KVM can programming detect when bits are fixed vs.
   flexible.  E.g. it's not impossible that userspace would want to do X if=
 a
   feature is fixed, but Y if it's flexible.