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 "JSON". > > > That would be quite the curious choice for a TDX module API. > > > > > > So it is easy to convert it to a C struct and embed it in KVM. It's just not > > > that useful because it will not necessarily be valid for future TDX modules. > > > > No, I don't want to embed anything in KVM, that's the exact same as hardcoding > > crud into KVM, which is what I want to avoid. I want to be able to 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 aligns > > with > > userspace's desired the vCPU model, i.e. that the value of fixed bits match up > > with the guest CPUID that userspace wants to define. > > > > Maybe that just means converting the JSON file into some binary format that > > the > > kernel can already parse. But I want Intel to commit to providing that > > metadata > > along with every TDX module. > > Oof. It turns out in one of the JSON files there is a description of a different > interface (TDX module runtime interface) that provides a way to read CPUID 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 resulting > values of specific CPUID leafs. > > This does not provide a way to query the fixed bits specifically, it tells you > what ended up getting configuring in a specific TD, which includes the fixed > bits and anything else. So we need to do KVM_TDX_INIT_VM before KVM_SET_CPUID 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. > > 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 behavior. I think it would be a good (optional?) sanity check, e.g. KVM_BUG_ON() if the 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.
