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 17:48:57 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "Hansen, Dave" <dave.hansen@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>
CC: "Zhang, Tina" <tina.zhang@...el.com>, "Yuan, Hang" <hang.yuan@...el.com>,
	"Huang, Kai" <kai.huang@...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

On Fri, 2024-03-15 at 09:33 -0700, Sean Christopherson wrote:
> Heh, Like this one?
> 
>         static inline u64 tdh_sys_lp_shutdown(void)
>         {
>                 struct tdx_module_args in = {
>                 };
>         
>                 return tdx_seamcall(TDH_SYS_LP_SHUTDOWN, &in, NULL);
>         }
> 
> Which isn't actually used...

Looks like is was turned into a NOP in TDX 1.5. So will even forever be
dead code. I see one other that is unused. Thanks for pointing it out.

> 
> > But I'd also defer to the KVM maintainers on this.  They're the
> > ones
> > that have to play the symbol exporting game a lot more than I ever
> > do.
> > If they cringe at the idea of adding 20 (or whatever) exports, then
> > that's a lot more important than the possibility of some other
> > silly
> > module abusing the generic exported __seamcall.
> 
> I don't care much about exports.  What I do care about is sane code,
> and while
> the current code _looks_ pretty, it's actually quite insane.
> 
> I get why y'all put SEAMCALL in assembly subroutines; the macro
> shenanigans I
> originally wrote years ago were their own brand of crazy, and dealing
> with GPRs
> that can't be asm() constraints often results in brittle code.

I guess it must be this, for the initiated:
https://lore.kernel.org/lkml/25f0d2c2f73c20309a1b578cc5fc15f4fd6b9a13.1605232743.git.isaku.yamahata@intel.com/

> 
> But the tdx_module_args structure approach generates truly atrocious
> code.  Yes,
> SEAMCALL is inherently slow, but that doesn't mean that we shouldn't
> at least try
> to generate efficient code.  And it's not just efficiency that is
> lost, the
> generated code ends up being much harder to read than it ought to be.
> 
> 
[snip]
> 
> So my feedback is to not worry about the exports, and instead focus
> on figuring
> out a way to make the generated code less bloated and easier to
> read/debug.
> 

Thanks for the feedback both! It sounds like everyone is flexible on
the exports. As for the generated code, oof.

Kai, I see the solution has gone through some iterations already. First
the macro one linked above, then that was dropped pretty quick to
something that loses the asm constraints:
https://lore.kernel.org/lkml/e777bbbe10b1ec2c37d85dcca2e175fe3bc565ec.1625186503.git.isaku.yamahata@intel.com/

Then next the struct grew here, and here:
https://lore.kernel.org/linux-mm/20230628211132.GS38236@hirez.programming.kicks-ass.net/
https://lore.kernel.org/linux-mm/20230630102141.GA2534364@hirez.programming.kicks-ass.net/

Not sure I understand all of the constraints yet. Do you have any
ideas?