Re: [PATCH 0/3] KVM: x86: SGX vs. XCR0 cleanups

[Sean Christopherson]

On Wed, Apr 12, 2023, Kai Huang wrote:
> On Thu, 2023-04-06 at 13:01 +0300, Zhi Wang wrote:
> > On Wed, 5 Apr 2023 19:10:40 -0700
> > Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > > TL;DR: trying to enforce "sane" CPUID/feature configuration is a gigantic can of worms.
> > 
> > Interesting point. I was digging the CPUID virtualization OF TDX/SNP.
> > It would be nice to have a conclusion of what is "sane" and what is the
> > proper role for KVM, as firmware/TDX module is going to validate the "sane"
> > CPUID.
> > 
> > TDX/SNP requires the CPUID to be pre-configured and validated before creating
> > a CC guest. (It is done via TDH.MNG.INIT in TDX and inserting a CPUID page in
> > SNP_LAUNCH_UPDATE in SNP).
> > 
> > IIUC according to what you mentioned, KVM should be treated like "CPUID box"
> > for QEMU and the checks in KVM is only to ensure the requirements of a chosen
> > one is literally possible and correct. KVM should not care if the
> > combination, the usage of the chosen ones is insane or not, which gives
> > QEMU flexibility.
> > 
> > As the valid CPUIDs have been decided when creating a CC guest, what should be
> > the proper behavior (basically any new checks?) of KVM for the later
> > SET_CPUID2? My gut feeling is KVM should know the "CPUID box" is reduced
> > at least, because some KVM code paths rely on guest CPUID configuration.
> 
> For TDX guest my preference is KVM to save all CPUID entries in TDH.MNG.INIT and
> manually make vcpu's CPUID point to the saved CPUIDs.  And then KVM just ignore
> the SET_CPUID2 for TDX guest.

It's been a long while since I looked at TDX's CPUID management, but IIRC ignoring
SET_CPUID2 is not an option becuase the TDH.MNG.INIT only allows leafs that are
known to the TDX Module, e.g. KVM's paravirt CPUID leafs can't be communicated via
TDH.MNG.INIT.  KVM's uAPI for initiating TDH.MNG.INIT could obviously filter out
unsupported leafs, but doing so would lead to potential ABI breaks, e.g. if a leaf
that KVM filters out becomes known to the TDX Module, then upgrading the TDX Module
could result in previously allowed input becoming invalid.

Even if that weren't the case, ignoring KVM_SET_CPUID{2} would be a bad option
becuase it doesn't allow KVM to open behavior in the future, i.e. ignoring the
leaf would effectively make _everything_ valid input.  If KVM were to rely solely
on TDH.MNG.INIT, then KVM would want to completely disallow KVM_SET_CPUID{2}.

Back to Zhi's question, the best thing to do for TDX and SNP is likely to require
that overlap between KVM_SET_CPUID{2} and the "trusted" CPUID be consistent.  The
key difference is that KVM would be enforcing consistency, not sanity.  I.e. KVM
isn't making arbitrary decisions on what is/isn't sane, KVM is simply requiring
that userspace provide a CPUID model that's consistent with what userspace provided
earlier.