x86/entry: fix jump into restore_all_guest without %rbx correctly set

[PATCH] x86/entry: fix jump into restore_all_guest without %rbx correctly set

Posted by Roger Pau Monne 3 months ago

e047b8d0fa05 went too far when limiting obtaining the vCPU pointer.  While the
code in ist_dispatch_done does indeed only need the vCPU pointer when PV32 is
enabled, the !PV32 path will end up jumping into restore_all_guest which does
require rbx == vCPU pointer.

Fix by moving the fetching of the vCPU pointer to be done outside of the PV32
code block.

Reported-by: Edwin Torok <edwin.torok@cloud.com>
Fixes: e047b8d0fa05 ('x86/entry: replace two GET_CURRENT() uses')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/x86_64/entry.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 44f25def86bc..ecdd6e5b47a5 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -1139,8 +1139,9 @@ FUNC(handle_ist_exception)
         movl  $EVENT_CHECK_VECTOR,%edi
         call  send_IPI_self
 1:
-#ifdef CONFIG_PV32
+        /* For restore_all_guest. */
         mov   STACK_CPUINFO_FIELD(current_vcpu)(%r14), %rbx
+#ifdef CONFIG_PV32
         movq  VCPU_domain(%rbx),%rax
         cmpb  $0,DOMAIN_is_32bit_pv(%rax)
         je    restore_all_guest
-- 
2.43.0

Re: [PATCH] x86/entry: fix jump into restore_all_guest without %rbx correctly set

Posted by Andrew Cooper 3 months ago

On 26/01/2024 2:54 pm, Roger Pau Monne wrote:
> e047b8d0fa05 went too far when limiting obtaining the vCPU pointer.  While the
> code in ist_dispatch_done does indeed only need the vCPU pointer when PV32 is
> enabled, the !PV32 path will end up jumping into restore_all_guest which does
> require rbx == vCPU pointer.
>
> Fix by moving the fetching of the vCPU pointer to be done outside of the PV32
> code block.
>
> Reported-by: Edwin Torok <edwin.torok@cloud.com>
> Fixes: e047b8d0fa05 ('x86/entry: replace two GET_CURRENT() uses')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks for getting to the bottom of this.

This asm code is an unwieldy mess.  We need to figure out a way to
simplify it.