Re: [PATCH 7/7] KVM: TDX: Add TSX_CTRL msr into uret_msrs list

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Mon, 2024-12-02 at 16:34 -0800, Sean Christopherson wrote:
> > Small point - the last conversation[0] we had on this was to let *userspace*
> > ensure consistency between KVM's CPUID (i.e. KVM_SET_CPUID2) and the TDX
> > Module's view.
> 
> I'm all for that, right up until KVM needs to protect itself again userspace
> and
> flawed TDX architecture.  A relevant comment I made in that thread:
> 
>  : If the upgrade breaks a setup because it confuses _KVM_, then I'll care
> 
> As it applies here, letting vCPU CPUID and actual guest functionality diverge
> for
> features that KVM cares about _will_ cause problems.

Right, just wanted to make sure we don't need to re-open the major design.

> 
> This will be less ugly to handle once kvm_vcpu_arch.cpu_caps is a thing.  KVM
> can simply force set/clear bits to match the actual guest functionality that's
> hardcoded by the TDX Module or defined by TDPARAMS.
> 
> > So the configuration goes:
> > 1. Userspace configures per-VM CPU features
> > 2. Userspace gets TDX Module's final per-vCPU version of CPUID configuration
> > via
> > KVM API
> > 3. Userspace calls KVM_SET_CPUID2 with the merge of TDX Module's version,
> > and
> > userspace's desired values for KVM "owned" CPUID leads (pv features, etc)
> > 
> > But KVM's knowledge of CPUID bits still remains per-vcpu for TDX in any
> > case.
> > 
> > > 
> > >  - Don't hardcode fixed/required CPUID values in KVM, use available
> > > metadata
> > >    from TDX Module to reject "bad" guest CPUID (or let the TDX module
> > > reject?).
> > >    I.e. don't let a guest silently run with a CPUID that diverges from
> > > what
> > >    userspace provided.
> > 
> > The latest QEMU patches have this fixed bit data hardcoded in QEMU. Then the
> > long term solution is to make the TDX module return this data. Xiaoyao will
> > post
> > a proposal on how the TDX module should expose this soon.
> 
> Punting the "merge" to userspace is fine, but KVM still needs to ensure it
> doesn't
> have holes where userspace can attack the kernel by lying about what features
> the
> guest has access to.  And that means forcing bits in kvm_vcpu_arch.cpu_caps;
> anything else is just asking for problems.

Ok, then for now let's just address them on a case-by-case basis for logic that
protects KVM. I'll add to look at using kvm_vcpu_arch.cpu_caps to our future-
things todo list.

I think Adrian is going post a proposal for how to handle this case better.