1. Patchew
  2. linux
  3. View series

[PATCH] KVM: selftests: Make guest_code_xsave more friendly

Ackerley Tng via B4 Relay posted 1 patch 4 days, 9 hours ago 
tools/testing/selftests/kvm/x86/sev_smoke_test.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
[PATCH] KVM: selftests: Make guest_code_xsave more friendly
Posted by Ackerley Tng via B4 Relay 4 days, 9 hours ago 
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
Posted by Sean Christopherson 4 days, 9 hours ago 
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
Posted by Ackerley Tng 2 days, 10 hours ago 
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.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.