On Thu, Apr 13, 2023, Kai Huang wrote: > On Wed, 2023-04-12 at 08:22 -0700, Sean Christopherson wrote: > > 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. > > How about only filtering out PV related CPUIDs when applying CPUIDs to > TDH.MNG.INIT? I think we can assume they are not gonna be known to TDX module > anyway. Nope, not going down that road. Fool me once[*], shame on you. Fool me twice, shame on me :-) Objections to hardware vendors defining PV interfaces aside, there exist leafs that are neither PV related nor known to the TDX module, e.g. Centaur leafs. I think it's extremely unlikely (understatement) that anyone will want to expose Centaur leafs to a TDX guest, but again I want to say out of the business of telling userspace what is and isn't sane CPUID models. [*] https://lore.kernel.org/all/20221210160046.2608762-6-chen.zhang@xxxxxxxxx > > 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}. > > Right. Disallowing SET_CPUID{2} probably is better, as it gives userspace a > more concrete result. > > > > > 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. > > So IIUC, you prefer to verifying the CPUIDs in SET_CPUID{2} are a super set of > the CPUIDs provided in TDH.MNG.INIT? And KVM manually verifies all CPUIDs for > all vcpus are consistent (the same) in SET_CPUID{2}? Yes, except KVM doesn't need to verify vCPUs are consistent with respect to each other, just that each vCPU is consistent with respect to what was reported to the TDX Module. > Looks this is over-complicated, _if_ the "only filtering out PV related CPUIDs > when applying CPUIDs to TDH.MNG.INIT" approach works. It's not complicated at all. Walk through the leafs defined during TDH.MNG.INIT, reject KVM_SET_CPUID if a leaf isn't present or doesn't match exactly. Or has the TDX spec changed and it's no longer that simple?