[PATCH 2/3] x86/kexec: Simplify the relocation of compat_mode_gdt_desc

Andrew Cooper posted 3 patches 2 years, 11 months ago
[PATCH 2/3] x86/kexec: Simplify the relocation of compat_mode_gdt_desc
Posted by Andrew Cooper 2 years, 11 months ago
Assemble the GDT base relative to kexec_reloc, and simply add the identity map
base address to relocate.

Adjust a stale comment, and drop the unused matching label.

No functional change.

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>
---
 xen/arch/x86/x86_64/kexec_reloc.S | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/x86_64/kexec_reloc.S b/xen/arch/x86/x86_64/kexec_reloc.S
index 035164e96f38..a81f64146190 100644
--- a/xen/arch/x86/x86_64/kexec_reloc.S
+++ b/xen/arch/x86/x86_64/kexec_reloc.S
@@ -72,7 +72,6 @@ ENTRY(kexec_reloc)
         testq   $KEXEC_RELOC_FLAG_COMPAT, %r8
         jnz     .L_call_32_bit
 
-.L_call_64_bit:
         /* Call the image entry point.  This should never return. */
         callq   *%rbp
         ud2
@@ -81,9 +80,8 @@ ENTRY(kexec_reloc)
         /* Setup IDT. */
         lidt    compat_mode_idt(%rip)
 
-        /* Load compat GDT. */
-        leaq    compat_mode_gdt(%rip), %rax
-        movq    %rax, (compat_mode_gdt_desc + 2)(%rip)
+        /* Relocate and load compat GDT. */
+        add     %rdi, 2 + compat_mode_gdt_desc(%rip)
         lgdt    compat_mode_gdt_desc(%rip)
 
         /* Enter compatibility mode. */
@@ -172,7 +170,7 @@ compatibility_mode:
         .align 4
 compat_mode_gdt_desc:
         .word .Lcompat_mode_gdt_end - compat_mode_gdt -1
-        .quad 0x0000000000000000     /* set in call_32_bit above */
+        .quad . - kexec_reloc        /* Relocated before use */
 
         .type compat_mode_gdt_desc, @object
         .size compat_mode_gdt_desc, . - compat_mode_gdt_desc
-- 
2.30.2


Re: [PATCH 2/3] x86/kexec: Simplify the relocation of compat_mode_gdt_desc
Posted by Jan Beulich 2 years, 11 months ago
On 17.02.2023 18:48, Andrew Cooper wrote:
> Assemble the GDT base relative to kexec_reloc, and simply add the identity map
> base address to relocate.
> 
> Adjust a stale comment, and drop the unused matching label.

Only kind of - the comment is referencing call_32_bit, and hence wasn't
really stale. And what was (and would remain to be) dead is call_64_bit.
May want slightly re-wording.

> @@ -81,9 +80,8 @@ ENTRY(kexec_reloc)
>          /* Setup IDT. */
>          lidt    compat_mode_idt(%rip)
>  
> -        /* Load compat GDT. */
> -        leaq    compat_mode_gdt(%rip), %rax
> -        movq    %rax, (compat_mode_gdt_desc + 2)(%rip)
> +        /* Relocate and load compat GDT. */
> +        add     %rdi, 2 + compat_mode_gdt_desc(%rip)
>          lgdt    compat_mode_gdt_desc(%rip)

Where's %rdi being populated for this? At kexec_reloc %rdi points at
the code page, but prior to calling relocate_pages the register is
overwritten (and the original value is lost). relocate_pages also
has normal C calling convention afaict; kind of as a result %rdi is
actually being clobbered there.

Jan