All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: ackerleytng@google.com
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Shuah Khan <shuah@kernel.org>,
	kvm@vger.kernel.org,  linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org,  michael.roth@amd.com,
	jackyli@google.com, liruxin@google.com,  darwinguo@google.com,
	jacobhxu@google.com
Subject: Re: [PATCH] KVM: selftests: Make guest_code_xsave more friendly
Date: Wed, 3 Jun 2026 16:05:25 -0700	[thread overview]
Message-ID: <aiCzNbtQ4FPu2yCR@google.com> (raw)
In-Reply-To: <20260603-snp-selftest-cleanup-v1-1-73b29fe31ce6@google.com>

On Wed, Jun 03, 2026, Ackerley Tng via B4 Relay wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> The original implementation of guest_code_xsave makes a jmp to
> guest_sev_es_code in inline assembly. When code that uses guest_sev_es_code
> is removed, guest_sev_es_code will be optimized out, leading to a linking
> error since guest_code_xsave still tries to jmp to guest_sev_es_code.

So, don't do that?

> Rewrite guest_code_xsave() to instead make a call, in C, to
> guest_sev_es_code(), so that usage of guest_sev_es_code() is made known to
> the compiler.
> 
> This rewriting also gives a name to the xsave inline assembly, improving
> readability.
> 
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
>  tools/testing/selftests/kvm/x86/sev_smoke_test.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
> index 1a49ee3915864..8b859adf4cf6f 100644
> --- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c
> +++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
> @@ -80,13 +80,23 @@ static void guest_sev_code(void)
>  	GUEST_DONE();
>  }
>  
> -/* Stash state passed via VMSA before any compiled code runs.  */

Uh, so as the comment says, the goal is to stash state before _any_ compiled
code runs.  Shoving the code into inline asm breaks that.  The compiler *probably*
won't shove anything before the first inline assembly, but there are absolutely
no guarantees.  E.g. you're subtly relying on a tail-call optimization to avoid
any stack operations.  If I force guest_sev_es_code() to be inlined, then the
prologue becomes:

0000000000402a10 <guest_code_xsave>:
  402a10:       48 83 ec 08                                     sub    $0x8,%rsp
  402a14:       b8 07 00 00 00                                  mov    $0x7,%eax
  402a19:       31 d2                                           xor    %edx,%edx
  402a1b:       0f ae 27                                        xsave  (%rdi)
  402a1e:       b9 31 01 01 c0                                  mov    $0xc0010131,%ecx

and we're hosed.

> -extern void guest_code_xsave(void);
> -asm("guest_code_xsave:\n"
> -    "mov $" __stringify(XFEATURE_MASK_X87_AVX) ", %eax\n"
> -    "xor %edx, %edx\n"
> -    "xsave (%rdi)\n"
> -    "jmp guest_sev_es_code");
> +static void xsave_all_registers(void *addr)
> +{
> +	__asm__ __volatile__(
> +		"mov $" __stringify(XFEATURE_MASK_X87_AVX) ", %eax\n"
> +		"xor %edx, %edx\n"

This doesn't even build.  When using input and/or output params, named registers
like eax and edx need an extra '%' to escape them, e.g.

	asm volatile("mov $" __stringify(XFEATURE_MASK_X87_AVX) ", %%eax\n\t"
		     "xor %%edx, %%edx\n\t"
		     "xsave (%0)"
		:
		: "r"(addr)
		: "eax", "edx", "memory"
	 );

> +		"xsave (%0)"
> +		:
> +		: "r"(addr)
> +		: "eax", "edx", "memory"
> +	 );
> +}
> +
> +static void guest_code_xsave(void *vmsa_gva)
> +{
> +	xsave_all_registers(vmsa_gva);
> +	guest_sev_es_code();
> +}
>  
>  static void compare_xsave(u8 *from_host, u8 *from_guest)
>  {
> 
> ---
> base-commit: 0d9b37717aaa4a73362520af5ba4db7febf09123
> change-id: 20260603-snp-selftest-cleanup-bf97734c6902
> 
> Best regards,
> --
> Ackerley Tng <ackerleytng@google.com>
> 
> 

  reply	other threads:[~2026-06-03 23:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-03 22:55 [PATCH] KVM: selftests: Make guest_code_xsave more friendly Ackerley Tng via B4 Relay
2026-06-03 22:55 ` Ackerley Tng
2026-06-03 23:05 ` Sean Christopherson [this message]
2026-06-05 21:45   ` Ackerley Tng
2026-06-09  3:08     ` Sean Christopherson
2026-06-03 23:05 ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aiCzNbtQ4FPu2yCR@google.com \
    --to=seanjc@google.com \
    --cc=ackerleytng@google.com \
    --cc=darwinguo@google.com \
    --cc=jackyli@google.com \
    --cc=jacobhxu@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=liruxin@google.com \
    --cc=michael.roth@amd.com \
    --cc=pbonzini@redhat.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.