linux-kernel - Re: [PATCH v19 007/130] x86/virt/tdx: Export SEAMCALL functions

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]

Date: Wed, 20 Mar 2024 12:09:47 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "Hansen, Dave" <dave.hansen@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>
CC: "Zhang, Tina" <tina.zhang@...el.com>, "Edgecombe, Rick P"
	<rick.p.edgecombe@...el.com>, "Yuan, Hang" <hang.yuan@...el.com>,
	"x86@...nel.org" <x86@...nel.org>, "Chen, Bo2" <chen.bo@...el.com>,
	"sagis@...gle.com" <sagis@...gle.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "isaku.yamahata@...il.com"
	<isaku.yamahata@...il.com>, "Aktas, Erdem" <erdemaktas@...gle.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "Yamahata, Isaku" <isaku.yamahata@...el.com>
Subject: Re: [PATCH v19 007/130] x86/virt/tdx: Export SEAMCALL functions

> 
> then loads and zeros a ton of memory (tdx_kvm_hypercall()):
> 
> 
[...]

> 
> and then unpacks all of that memory back into registers, and reverses that last
> part on the way back, (__tdcall_saved_ret()):
> 
> 
[...]

> 
> It's honestly quite amusing, because y'all took one what I see as one of the big
> advantages of TDX over SEV (using registers instead of shared memory), and managed
> to effectively turn it into a disadvantage.
> 
> Again, I completely understand the maintenance and robustness benefits, but IMO
> the pendulum swung a bit too far in that direction.

Having to zero-and-init the structure and store output regs to the structure is
an unfortunately burden if we want to have a single API __seamcall() for all
SEAMCALL leafs.

To (precisely) avoid writing to the unnecessary structure members before and
after the SEAMCALL instruction, we need to use your old way to have bunch of
macros.  But we may end up with *a lot* macros due to needing to cover new
(e.g., live migration) SEAMCALLs.

Because essentially it's a game to implement wrappers for bunch of combinations
of each individual input/output regs.

I don't want to judge which way is better, but to be honest I think completely
switching to the old way (using bunch of macros) isn't a realistic option at
this stage.

However, I think we might be able to change ...

    u64 __seamcall(u64 fn, struct tdx_module_args *args);

... to

    u64 __seamcall(u64 fn, struct tdx_module_args *in,
			struct tdx_module_args *out);

.. so that the assembly can actually skip "storing output regs to the structure"
if the SEAMCALL doesn't really have any output regs except RAX.

I can try to do if you guys believe this should be done, and should be done
earlier than later, but I am not sure _ANY_ optimization around SEAMCALL will
have meaningful performance improvement.