Re: [ANNOUNCE] PUCK Notes - 2024.04.03 - TDX Upstreaming Strategy

[Isaku Yamahata]

On Fri, Apr 12, 2024 at 04:40:37PM +0800,
Xiaoyao Li <xiaoyao.li@xxxxxxxxx> wrote:

> > The second issue is that userspace can’t know what CPUID values are configured
> > in the TD. In the existing API for normal guests, it knows because it tells the
> > guest what CPUID values to have. But for the TDX module that model is
> > complicated to fit into in its API where you tell it some things and it gives
> > you the resulting leaves. How to handle KVM_SET_CPUID kind of follows from this
> > issue.
> > 
> > One option is to demand the TDX module change to be able to efficiently wedge
> > into KVM’s exiting “tell” model. This looks like the metadata API to query the
> > fixed bits. Then userspace can know what bits it has to set, and call
> > KVM_SET_CPUID with them. I think it is still kind of awkward. "Tell me what you
> > want to hear?", "Ok here it is".
> > 
> > Another option would be to add TDX specific KVM APIs that work for the TDX
> > module's “ask” model, and meet the enumerated two goals. It could look something
> > like:
> > 1. KVM_TDX_GET_CONFIG_CPUID provides a list of directly configurable bits by
> > KVM. This is based on static data on what KVM supports, with sanity check of
> > TD_SYSINFO.CPUID_CONFIG[]. Bits that KVM doesn’t know about, but are returned as
> > configurable by TD_SYSINFO.CPUID_CONFIG[] are not exposed as configurable. (they
> > will be set to 1 by KVM, per the recommendation)
> 
> This is not how KVM works. KVM will never enable unknown features blindly.
> If the feature is unknown to KVM, it cannot be enable for guest. That's why
> every new feature needs enabling patch in KVM, even the simplest case that
> needs one patch to enumerate the CPUID of new instruction in
> KVM_GET_SUPPORTED_CPUID.

We can use device attributes as discussed at
https://lore.kernel.org/kvm/CABgObfZzkNiP3q8p=KpvvFnh8m6qcHX4=tATaJc7cvVv2QWpJQ@xxxxxxxxxxxxxx/
https://lore.kernel.org/kvm/20240404121327.3107131-6-pbonzini@xxxxxxxxxx/

Something like

#define KVM_X86_GRP_TDX         2
ioctl(fd, KVM_GET_DEVICE_ATTR, (KVM_X86_GRP_TDX, metadata_field_id))


> > 2. KVM_TDX_INIT_VM is passed userspaces choice of configurable bits, along with
> > XFAM and ATTRIBUTES as dedicated fields. They go into TDH.MNG.INIT.
> > 3. KVM_TDX_INIT_VCPU_CPUID takes a list of CPUID leafs. It pulls the CPUID bits
> > actually configured in the TD for these leafs. They go into the struct kvm_vcpu,
> > and are also passed up to userspace so everyone knows what actually got
> > configured.

Any reason to introduce KVM_TDX_INIT_VCPU_CPUID in addition to
KVM_TDX_INIT_VCPU?  We can make single vCPU KVM TDX ioctl do all.


> > KVM_SET_CPUID is not used for TDX.

What cpuid does KVM_TDX_INIT_VCPU_CPUID accept?  The one that TDX module
accepts with TDH.MNG.INIT()?  Or any cpuids that KVM_SET_CPUID2 accepts?
I'm asking it because TDX module virtualizes only subset of CPUIDs. 
TDG.VP.VMCALL<CPUID> would need info from KVM_SET_CPUID.


> > Then we get TDX module folks to commit to never breaking KVM/userspace that
> > follows this logic. One thing still missing is how to handle unknown future
> > leafs with fixed bits. If a future leaf is defined and gets fixed 1, QEMU
> > wouldn't know to query it.
> 
> We can make KVM_TDX_INIT_VCPU_CPUID provide a large enough CPUID leafs and
> KVM reports every leafs to userpsace. Instead of something that userspace
> cares leafs X,Y,Z and KVM only reports back leafs X,Y,Z via
> KVM_TDX_INIT_VCPU_CPUID.

If new CPUID index is introduced, the userspace will get default values of
CPUIDs and don't touch unknown CPUIDs?  Or KVM_TDX_GET_CONFIG_CPUID will mask
out CPUID unknown to KVM?
-- 
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>