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: Fri, 15 Mar 2024 11:28:52 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Rick P Edgecombe <rick.p.edgecombe@...el.com>,
 "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 Kai Huang <kai.huang@...el.com>, Isaku Yamahata <isaku.yamahata@...el.com>,
 "x86@...nel.org" <x86@...nel.org>, Tina Zhang <tina.zhang@...el.com>,
 Hang Yuan <hang.yuan@...el.com>, Bo2 Chen <chen.bo@...el.com>,
 "sagis@...gle.com" <sagis@...gle.com>,
 "isaku.yamahata@...il.com" <isaku.yamahata@...il.com>,
 Erdem Aktas <erdemaktas@...gle.com>,
 "pbonzini@...hat.com" <pbonzini@...hat.com>
Subject: Re: [PATCH v19 007/130] x86/virt/tdx: Export SEAMCALL functions

On 3/15/24 09:33, Sean Christopherson wrote:
>         static inline u64 tdh_mem_page_remove(hpa_t tdr, gpa_t gpa, int level,
>         				      struct tdx_module_args *out)
>         {
>         	struct tdx_module_args in = {
>         		.rcx = gpa | level,
>         		.rdx = tdr,
>         	};
> 
>         	return tdx_seamcall_sept(TDH_MEM_PAGE_REMOVE, &in, out);
>         }
> 
> generates the below monstrosity with gcc-13.  And that's just one SEAMCALL wrapper,
> *every* single one generates the same mess.  clang-16 is kinda sorta a little
> better, as it at least inlines the helpers that have single callers.

Yeah, that's really awful.

Is all the inlining making the compiler too ambitious?  Why is this all
inlined in the first place?

tdh_mem_page_remove() _should_ just be logically:

	* initialize tdx_module_args.  Move a few things into place on
	  the stack and zero the rest.
	* Put a pointer to tdx_module_args in a register
	* Put TDH_MEM_PAGE_REMOVE immediate in a register
	* Some register preservation, maybe
	* call
	* maybe some cleanup
	* return

Those logical things are *NOT* easy to spot in the disassembly.