On Sat, 2025-01-04 at 01:43 +0000, Edgecombe, Rick P wrote: > > 18: not sure why the check against num_present_cpus() is needed? > > The per-vm KVM_MAX_VCPUS will be min_t(int, kvm->max_vcpus, num_present_cpus()). > So if td_conf->max_vcpus_per_td < num_present_cpus(), then it might report > supporting more CPUs then actually supported by the TDX module. Right. > > As to why not just report td_conf->max_vcpus_per_td, that value is the max CPUs > that are supported by any platform the TDX module supports. So it is more about > what the TDX module supports, then what userspace cares about (how many vCPUs > they can use). Sean didn't want to make reporting maximum vcpus depend on the whims of TDX module since this doesn't provide a predictable ABI: https://lore.kernel.org/kvm/ZmzaqRy2zjvlsDfL@xxxxxxxxxx/ > > I think we could probably get by without the check and blame the TDX module if > it does something strange. It is seems safer ABI-wise to have the check. But we > are being a bit more cavalier around protecting against TDX supported CPUID bit > changes then originally planned, so the check here now seems inconsistent. > > Let me flag Kai to confirm there was not some known violating configuration. He > explored a bunch of edge cases on this corner. In practice the "max_vcpu_per_td" will never be smaller than the maximum logical CPUs that ALL the platforms that the module supports can possibly have. I got this from the TDX module guys, and I don't think there's any reason for the TDX module to break this. However from module ABI's perspective (from the JSON), it could be any value, so I think we should have a sanity check. I think this is also different from the "array size of CPUID_CONFIGs" ABI breakage (assuming this is what you meant "protecting TDX supported CPUID bits" above) since it is currently documented as 32 in the JSON.
