On Tue, 2024-06-18 at 22:19 +0000, Huang, Kai wrote: > On Tue, 2024-06-18 at 07:49 -0700, Sean Christopherson wrote: > > On Tue, Jun 18, 2024, Kai Huang wrote: > > > On 15/06/2024 12:04 pm, Sean Christopherson wrote: > > > > On Fri, Jun 14, 2024, Kai Huang wrote: > > > > > > - The "max_vcpus_per_td" can be different depending on module versions. In > > > > > > practice it reflects the maximum physical logical cpus that all the > > > > > > platforms (that the module supports) can possibly have. > > > > > > > > It's a reasonable restriction, e.g. KVM_CAP_NR_VCPUS is already capped at number > > > > of online CPUs, although userspace is obviously allowed to create oversubscribed > > > > VMs. > > > > > > > > I think the sane thing to do is document that TDX VMs are restricted to the number > > > > of logical CPUs in the system, have KVM_CAP_MAX_VCPUS enumerate exactly that, and > > > > then sanity check that max_vcpus_per_td is greater than or equal to what KVM > > > > reports for KVM_CAP_MAX_VCPUS. > > > > > Stating that the maximum number of vCPUs depends on the whims TDX module doesn't > > > > provide a predictable ABI for KVM, i.e. I don't want to simply forward TDX's > > > > max_vcpus_per_td to userspace. > > > > > > This sounds good to me. I think it should be also OK for client too, if TDX > > > ever gets supported for client. > > > > > > IIUC we can consult the @nr_cpu_ids or num_possible_cpus() to get the > > > "number of logical CPUs in the system". And we can reject to use the TDX > > > module if 'max_vcpus_per_td' turns to be smaller. > > > > I assume TDX is incompatible with actual physical CPU hotplug? > > > > Correct. > > > If so, we can and > > should use num_present_cpus(). > > > > On TDX platform num_present_cpus() and num_possible_cpus() should be just > identical, because TDX requires BIOS to mark all all physical LPs the > platform as enabled, and TDX doesn't support physical CPU hotplug. > > Using num_present_cpus() w/o holding CPU hotplug lock is a little bit > annoying from code's perspective, but it's OK to me. We can add a comment > saying TDX doesn't support physical CPU hotplug. > > > If loading the TDX module completely disables > > onlining CPUs, then we can use num_online_cpus(). Missed this one. No loading/initializing TDX module doesn't disable onlining CPUs. TDX module can be initialized on a subset of physical logical CPUs. Also, after module initialization, logical CPU can be put to offline and then brought back to online again.