[PATCH] KVM: selftests: Make guest_code_xsave more friendly

[PATCH] KVM: selftests: Make guest_code_xsave more friendly

[Ackerley Tng via B4 Relay]

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.

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.  */
-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"
+		"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>

Re: [PATCH] KVM: selftests: Make guest_code_xsave more friendly

[Sean Christopherson]

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>
> 
> 

Re: [PATCH] KVM: selftests: Make guest_code_xsave more friendly

[Ackerley Tng]

Sean Christopherson <seanjc@google.com> writes:

> 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?
>

I was commenting out function calls while working on another part of
this selftest and ran into linking issues.

>> 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.
>

And omg, I thought I was running the tests!!

>> -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"
> 	 );
>

My bad, I was compiling this while having the caller of this function
commented out. Didn't realise that even within the same file, the inline
assembly won't even be compiled if there's no caller.

>> +		"xsave (%0)"
>> +		:
>> +		: "r"(addr)
>> +		: "eax", "edx", "memory"
>> +	 );
>> +}
>> +
>>
>> [...snip...]
>>

My bad, this patch is such a goof!

I still feel that it'd be nice to allow commenting out functions parts
of selftests while developing other parts, but let's shelve this for
now.

I think to clean this up I would do something like

1. Enter the guest, GUEST_SYNC(STAGE_READY)
2. In the host, write VMSA
3. vcpu_run(), guest will perform that __asm__ block, then GUEST_DONE()
4. Host performs compare_vmsa() to verify

But this relies on ucall support for SNP.

Re: [PATCH] KVM: selftests: Make guest_code_xsave more friendly

[Sean Christopherson]

On Fri, Jun 05, 2026, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
> > 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.
> >
> 
> And omg, I thought I was running the tests!!

FWIW, you probably were, I had to force it to be inline via __always_inline.

> >> +		"xsave (%0)"
> >> +		:
> >> +		: "r"(addr)
> >> +		: "eax", "edx", "memory"
> >> +	 );
> >> +}
> >> +
> >>
> >> [...snip...]
> >>
> 
> My bad, this patch is such a goof!
> 
> I still feel that it'd be nice to allow commenting out functions parts
> of selftests while developing other parts, but let's shelve this for
> now.

I mean, it's always "allowed", but that's firmly "Here be dragons!" territory.
There are any number of things that can go sideways when adding and/or removing
code for development and debug purposes, many of which are far more nefarious
and subtle than obscure compiler errors.

Re: [PATCH] KVM: selftests: Make guest_code_xsave more friendly

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Converting `guest_code_xsave` from a top-level pure assembly block to a standard C function invalidates the test's core assumption that no compiled code runs before the `xsave` instruction, exposing the pristine VMSA state to compiler perturbation.
--

commit 45df73f97588ddd25e44034bc6f46ec39b28af24
Author: Ackerley Tng <ackerleytng@google.com>

KVM: selftests: Make guest_code_xsave more friendly

This commit converts the guest_code_xsave entry point from naked inline
assembly to a standard C function to prevent linker errors when
guest_sev_es_code is optimized out. It also refactors the xsave instruction
into a separate C helper function.

> 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.  */
> -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)
> +{

[ ... ]

> +static void guest_code_xsave(void *vmsa_gva)
> +{
> +	xsave_all_registers(vmsa_gva);

[Severity: Medium]
Will this change allow the compiler to corrupt the VMSA state before the
xsave instruction executes?

The original code was explicitly written as top-level assembly to stash
the VMSA state before any compiled code runs. By converting guest_code_xsave()
to a standard C function, the compiler is free to insert function prologues
or instrumentation (like ASAN or fentry) before the xsave instruction
executes.

Since the x86_64 ABI treats YMM registers as caller-saved (volatile), any
compiler-inserted function calls could clobber the FPU/AVX state (such as ymm4)
that the test relies on, causing spurious test failures.

If the goal is to prevent guest_sev_es_code() from being optimized out,
would it be better to keep the raw assembly and apply __attribute__((used))
to guest_sev_es_code() instead?

> +	guest_sev_es_code();
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-snp-selftest-cleanup-v1-1-73b29fe31ce6@google.com?part=1