On Fri, 2024-03-22 at 16:06 +0000, Edgecombe, Rick P wrote: > On Fri, 2024-03-22 at 07:10 +0000, Huang, Kai wrote: > > > I see that this was suggested by Sean, but can you explain the > > > problem > > > that this is working around? From the linked thread, it seems like > > > the > > > problem is what to do when userspace also calls SET_CPUID after > > > already > > > configuring CPUID to the TDX module in the special way. The choices > > > discussed included: > > > 1. Reject the call > > > 2. Check the consistency between the first CPUID configuration and > > > the > > > second one. > > > > > > 1 is a lot simpler, but the reasoning for 2 is because "some KVM > > > code > > > paths rely on guest CPUID configuration" it seems. Is this a > > > hypothetical or real issue? Which code paths are problematic for > > > TDX/SNP? > > > > There might be use case that TDX guest wants to use some CPUID which > > isn't handled by the TDX module but purely by KVM. These (PV) CPUIDs > > need to be > > provided via KVM_SET_CPUID2. > > Right, but are there any needed today? > I am not sure. Isaku may know better? > I read that Sean's point was > that KVM_SET_CPUID2 can't accept anything today what we would want to > block later, otherwise it would introduce a regression. This was the > major constraint IIUC, and means the base series requires *something* > here. > > If we want to support only the most basic support first, we don't need > to support PV CPUIDs on day 1, right? > > So I'm wondering, if we could shrink the base series by going with > option 1 to start, and then expanding it with this solution later to > enable more features. Do you see a problem or conflict with Sean's > comments? > > To confirm, I mean you want to simply make KVM_SET_CPUID2 return error for TDX guest? It is acceptable to me, and I don't see any conflict with Sean's comments. But I don't know Sean's perference. As he said, I think the consistency checking is quite straight-forward: " 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. " So to me it's not a big deal. Either way, we need a patch to handle SET_CPUID2: 1) if we go option 1) -- that is reject SET_CPUID2 completely -- we need to make vcpu's CPUID point to KVM's saved CPUID during TDH.MNG.INIT. 2) if we do consistency check, we do a for loop and reject when in-consistency found. I'll leave to you to judge :-)
