* [PATCH] KVM: selftests: Make guest_code_xsave more friendly @ 2026-06-03 22:55 ` Ackerley Tng 0 siblings, 0 replies; 6+ messages in thread From: Ackerley Tng via B4 Relay @ 2026-06-03 22:55 UTC (permalink / raw) To: Sean Christopherson, Paolo Bonzini, Shuah Khan Cc: kvm, linux-kselftest, linux-kernel, michael.roth, jackyli, liruxin, darwinguo, jacobhxu, Ackerley Tng 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> ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] KVM: selftests: Make guest_code_xsave more friendly @ 2026-06-03 22:55 ` Ackerley Tng 0 siblings, 0 replies; 6+ messages in thread From: Ackerley Tng @ 2026-06-03 22:55 UTC (permalink / raw) To: Sean Christopherson, Paolo Bonzini, Shuah Khan Cc: kvm, linux-kselftest, linux-kernel, michael.roth, jackyli, liruxin, darwinguo, jacobhxu, Ackerley Tng 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> ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: selftests: Make guest_code_xsave more friendly 2026-06-03 22:55 ` Ackerley Tng (?) @ 2026-06-03 23:05 ` Sean Christopherson 2026-06-05 21:45 ` Ackerley Tng -1 siblings, 1 reply; 6+ messages in thread From: Sean Christopherson @ 2026-06-03 23:05 UTC (permalink / raw) To: ackerleytng Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel, michael.roth, jackyli, liruxin, darwinguo, jacobhxu 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> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: selftests: Make guest_code_xsave more friendly 2026-06-03 23:05 ` Sean Christopherson @ 2026-06-05 21:45 ` Ackerley Tng 2026-06-09 3:08 ` Sean Christopherson 0 siblings, 1 reply; 6+ messages in thread From: Ackerley Tng @ 2026-06-05 21:45 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel, michael.roth, jackyli, liruxin, darwinguo, jacobhxu 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: selftests: Make guest_code_xsave more friendly 2026-06-05 21:45 ` Ackerley Tng @ 2026-06-09 3:08 ` Sean Christopherson 0 siblings, 0 replies; 6+ messages in thread From: Sean Christopherson @ 2026-06-09 3:08 UTC (permalink / raw) To: Ackerley Tng Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel, michael.roth, jackyli, liruxin, darwinguo, jacobhxu 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: selftests: Make guest_code_xsave more friendly 2026-06-03 22:55 ` Ackerley Tng (?) (?) @ 2026-06-03 23:05 ` sashiko-bot -1 siblings, 0 replies; 6+ messages in thread From: sashiko-bot @ 2026-06-03 23:05 UTC (permalink / raw) To: Ackerley Tng via B4 Relay; +Cc: kvm 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-06-09 3:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 2026-06-05 21:45 ` Ackerley Tng 2026-06-09 3:08 ` Sean Christopherson 2026-06-03 23:05 ` sashiko-bot
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.