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

Edgecombe, Rick P
2024-03-15T17:49:02Z

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?