Re: [PATCH 01/20] x86/tdx: Introduce tdvmcall_trampoline()

Dave Hansen
2024-05-20T15:49:59Z

On 5/20/24 03:32, Kirill A. Shutemov wrote:
>> In other words, I think this as the foundational justification for the
>> rest of the series leaves a little to be desired.
> See the patch below. Is it what you had in mind?
>
> This patch saves ~2K of code, comparing to ~3K for my patchset:
>
> add/remove: 0/0 grow/shrink: 1/17 up/down: 8/-2266 (-2258)
>
> But it is considerably simpler.
>
> arch/x86/boot/compressed/tdx.c | 32 ++++----
> arch/x86/coco/tdx/tdx-shared.c | 3 +-
> arch/x86/coco/tdx/tdx.c | 159 +++++++++++++++++++++-----------------
> arch/x86/hyperv/ivm.c | 32 ++++----
> arch/x86/include/asm/shared/tdx.h | 25 ++++--
> 5 files changed, 142 insertions(+), 109 deletions(-)

The diffstat is a bit misleading because those extra lines really add
very little complexity. The only real risk is that folks end up leaving
the args structure uninitialized, but there are a number of ways to
mitigate that risk.

> +static __always_inline void tdx_arg_init(struct tdx_module_args *args)
> +{
> + asm ("rep stosb"
> + : "+D" (args)
> + : "c" (sizeof(*args)), "a" (0)
> + : "memory");
> +}

There are a bunch of ways to do this. This one certainly isn't _bad_,
but I'd be open to doing it other ways if folks have more ideas.

Either way, I very much prefer this approach to adding a bunch more
assembly and making things less self-documenting. I also suspect you
can get some more text size shrinkage from selectively uninlining a few
things.