Kernel KVM virtualization development
 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: 5+ 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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox