linux-kernel - Re: [PATCH v2 00/25] TDX vCPU/VM creation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9b08c078e9809827718d25ba99513b43ed4f1589.camel@intel.com>
Date: Sun, 5 Jan 2025 21:32:52 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "pbonzini@...hat.com" <pbonzini@...hat.com>, "Edgecombe, Rick P"
	<rick.p.edgecombe@...el.com>
CC: "isaku.yamahata@...il.com" <isaku.yamahata@...il.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "Li, Xiaoyao" <xiaoyao.li@...el.com>, "Chatre, Reinette"
	<reinette.chatre@...el.com>, "Zhao, Yan Y" <yan.y.zhao@...el.com>,
	"tony.lindgren@...ux.intel.com" <tony.lindgren@...ux.intel.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 00/25] TDX vCPU/VM creation

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@google.com/

> 
> 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.