linux-kernel - Re: [PATCH 01/20] x86/tdx: Introduce tdvmcall

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: Mon, 20 May 2024 08:49:58 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc: Sean Christopherson <seanjc@...gle.com>,
 Paolo Bonzini <pbonzini@...hat.com>,
 Dave Hansen <dave.hansen@...ux.intel.com>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, x86@...nel.org,
 "H. Peter Anvin" <hpa@...or.com>, "K. Y. Srinivasan" <kys@...rosoft.com>,
 Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
 Dexuan Cui <decui@...rosoft.com>, Josh Poimboeuf <jpoimboe@...nel.org>,
 Peter Zijlstra <peterz@...radead.org>, linux-coco@...ts.linux.dev,
 linux-kernel@...r.kernel.org, linux-hyperv@...r.kernel.org
Subject: Re: [PATCH 01/20] x86/tdx: Introduce tdvmcall_trampoline()

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.