Messages in this thread | | | From | "Edgecombe, Rick P" <> | Subject | Re: [PATCH v19 007/130] x86/virt/tdx: Export SEAMCALL functions | Date | Fri, 15 Mar 2024 17:48:57 +0000 |
| |
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?
| |