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>
>
>
next prev parent 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