xen/arch/x86/x86_64/compat/entry.S | 9 +-------- xen/arch/x86/x86_64/entry.S | 9 +-------- 2 files changed, 2 insertions(+), 16 deletions(-)
The patch adding check_ist_exit() neglected to consider reset_stack_and_jump()
leaving C and entering one of the Xen exit paths. The value in %r12 is stale,
and depending on compiler decisions may not be 0.
This shows up in Gitlab CI for the Clang build:
https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/5112783827
and in OSSTest for GCC 8:
http://logs.test-lab.xenproject.org/osstest/logs/183045/test-amd64-amd64-xl-qemuu-debianhvm-amd64/serial-pinot0.log
There's no straightforward way to reconstruct the IST-exit-ness on the
exit-to-guest path after a context switch. For now, we only need IST-exit on
the return-to-Xen path.
Fixes: 21bdc25b05a0 ("x86/entry: Track the IST-ness of an entry for the exit paths")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
v2:
* Rewrite.
---
xen/arch/x86/x86_64/compat/entry.S | 9 +--------
xen/arch/x86/x86_64/entry.S | 9 +--------
2 files changed, 2 insertions(+), 16 deletions(-)
diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index 7504bfb4f326..bd5abd8040bd 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -117,15 +117,8 @@ compat_process_trap:
call compat_create_bounce_frame
jmp compat_test_all_events
-/* %rbx: struct vcpu, %r12: ist_exit, interrupts disabled */
+/* %rbx: struct vcpu, interrupts disabled */
ENTRY(compat_restore_all_guest)
-
-#ifdef CONFIG_DEBUG
- mov %rsp, %rdi
- mov %r12, %rsi
- call check_ist_exit
-#endif
-
ASSERT_INTERRUPTS_DISABLED
mov $~(X86_EFLAGS_IOPL | X86_EFLAGS_VM), %r11d
and UREGS_eflags(%rsp),%r11d
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 988ef6cbc628..5ca74f5f62b2 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -142,15 +142,8 @@ process_trap:
.section .text.entry, "ax", @progbits
-/* %rbx: struct vcpu, %r12: ist_exit, interrupts disabled */
+/* %rbx: struct vcpu, interrupts disabled */
restore_all_guest:
-
-#ifdef CONFIG_DEBUG
- mov %rsp, %rdi
- mov %r12, %rsi
- call check_ist_exit
-#endif
-
ASSERT_INTERRUPTS_DISABLED
/* Stash guest SPEC_CTRL value while we can read struct vcpu. */
base-commit: ea36ac0de27c2a7c847a2a52c3e0f97a45864d81
--
2.30.2
On 19.09.2023 17:01, Andrew Cooper wrote: > The patch adding check_ist_exit() neglected to consider reset_stack_and_jump() > leaving C and entering one of the Xen exit paths. The value in %r12 is stale, > and depending on compiler decisions may not be 0. And it may also not be zero that we would be looking for. I think this wants expressing differently. The value in %r12 simply doesn't survive, and this has at best little to do with compiler decisions. > This shows up in Gitlab CI for the Clang build: > > https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/5112783827 > > and in OSSTest for GCC 8: > > http://logs.test-lab.xenproject.org/osstest/logs/183045/test-amd64-amd64-xl-qemuu-debianhvm-amd64/serial-pinot0.log > > There's no straightforward way to reconstruct the IST-exit-ness on the > exit-to-guest path after a context switch. For now, we only need IST-exit on > the return-to-Xen path. > > Fixes: 21bdc25b05a0 ("x86/entry: Track the IST-ness of an entry for the exit paths") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Code change itself: Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
© 2016 - 2024 Red Hat, Inc.