1. Patchew
  2. Xen
  3. [PATCH v6 0/5] Reuse 32 bit C code more safely
  4. View patch

[PATCH v6 2/5] x86/boot: Reuse code to relocate trampoline

Frediano Ziglio posted 5 patches 1 month ago
There is a newer version of this series
[PATCH v6 2/5] x86/boot: Reuse code to relocate trampoline
Posted by Frediano Ziglio 1 month ago 
Move code from efi-boot.h to a separate, new, reloc-trampoline.c file.
Reuse this new code, compiling it for 32bit as well, to replace assembly
code in head.S doing the same thing.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v3:
- fixed a typo in comment;
- added Reviewed-by.

Changes since v5:
- do not add obj64 to targets, already done adding it to obj-bin-y.
---
 xen/arch/x86/boot/Makefile           | 10 +++++---
 xen/arch/x86/boot/build32.lds.S      |  5 ++++
 xen/arch/x86/boot/head.S             | 23 +-----------------
 xen/arch/x86/boot/reloc-trampoline.c | 36 ++++++++++++++++++++++++++++
 xen/arch/x86/efi/efi-boot.h          | 15 ++----------
 5 files changed, 51 insertions(+), 38 deletions(-)
 create mode 100644 xen/arch/x86/boot/reloc-trampoline.c

diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
index 5da19501be..98ceb1983d 100644
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -1,11 +1,15 @@
 obj-bin-y += head.o
 obj-bin-y += built-in-32.o
+obj-bin-y += $(obj64)
 
 obj32 := cmdline.32.o
 obj32 += reloc.32.o
+obj32 += reloc-trampoline.32.o
 
-nocov-y   += $(obj32)
-noubsan-y += $(obj32)
+obj64 := reloc-trampoline.o
+
+nocov-y   += $(obj32) $(obj64)
+noubsan-y += $(obj32) $(obj64)
 targets   += $(obj32)
 
 obj32 := $(addprefix $(obj)/,$(obj32))
@@ -56,7 +60,7 @@ cmd_combine = \
 		--bin1 $(obj)/built-in-32.base.bin \
 		--bin2 $(obj)/built-in-32.offset.bin \
 		--map $(obj)/built-in-32.offset.map \
-		--exports cmdline_parse_early,reloc \
+		--exports cmdline_parse_early,reloc,reloc_trampoline32 \
 		--output $@
 
 targets += built-in-32.S
diff --git a/xen/arch/x86/boot/build32.lds.S b/xen/arch/x86/boot/build32.lds.S
index e3f5e55261..fa282370f4 100644
--- a/xen/arch/x86/boot/build32.lds.S
+++ b/xen/arch/x86/boot/build32.lds.S
@@ -41,6 +41,11 @@ SECTIONS
          * Potentially they should be all variables. */
         DECLARE_IMPORT(__base_relocs_start);
         DECLARE_IMPORT(__base_relocs_end);
+        DECLARE_IMPORT(__trampoline_rel_start);
+        DECLARE_IMPORT(__trampoline_rel_stop);
+        DECLARE_IMPORT(__trampoline_seg_start);
+        DECLARE_IMPORT(__trampoline_seg_stop);
+        DECLARE_IMPORT(trampoline_phys);
         . = . + GAP;
         *(.text)
         *(.text.*)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index e0776e3896..ade2c5c43d 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -706,28 +706,7 @@ trampoline_setup:
         mov     %edx, sym_offs(l1_bootmap)(%esi, %ecx, 8)
 
         /* Apply relocations to bootstrap trampoline. */
-        mov     sym_esi(trampoline_phys), %edx
-        lea     sym_esi(__trampoline_rel_start), %edi
-        lea     sym_esi(__trampoline_rel_stop), %ecx
-1:
-        mov     (%edi), %eax
-        add     %edx, (%edi, %eax)
-        add     $4,%edi
-
-        cmp     %ecx, %edi
-        jb      1b
-
-        /* Patch in the trampoline segment. */
-        shr     $4,%edx
-        lea     sym_esi(__trampoline_seg_start), %edi
-        lea     sym_esi(__trampoline_seg_stop), %ecx
-1:
-        mov     (%edi), %eax
-        mov     %dx, (%edi, %eax)
-        add     $4,%edi
-
-        cmp     %ecx, %edi
-        jb      1b
+        call    reloc_trampoline32
 
         /* Do not parse command line on EFI platform here. */
         cmpb    $0, sym_esi(efi_platform)
diff --git a/xen/arch/x86/boot/reloc-trampoline.c b/xen/arch/x86/boot/reloc-trampoline.c
new file mode 100644
index 0000000000..0a74c1e75a
--- /dev/null
+++ b/xen/arch/x86/boot/reloc-trampoline.c
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/compiler.h>
+#include <xen/stdint.h>
+#include <asm/trampoline.h>
+
+extern const int32_t __trampoline_rel_start[], __trampoline_rel_stop[];
+extern const int32_t __trampoline_seg_start[], __trampoline_seg_stop[];
+
+#if defined(__i386__)
+void reloc_trampoline32(void)
+#elif defined (__x86_64__)
+void reloc_trampoline64(void)
+#else
+#error Unknown architecture
+#endif
+{
+    unsigned long phys = trampoline_phys;
+    const int32_t *trampoline_ptr;
+
+    /*
+     * Apply relocations to trampoline.
+     *
+     * This modifies the trampoline in place within Xen, so that it will
+     * operate correctly when copied into place.
+     */
+    for ( trampoline_ptr = __trampoline_rel_start;
+          trampoline_ptr < __trampoline_rel_stop;
+          ++trampoline_ptr )
+        *(uint32_t *)(*trampoline_ptr + (long)trampoline_ptr) += phys;
+
+    for ( trampoline_ptr = __trampoline_seg_start;
+          trampoline_ptr < __trampoline_seg_stop;
+          ++trampoline_ptr )
+        *(uint16_t *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
+}
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 94f3443364..1acceec471 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -103,27 +103,16 @@ static void __init efi_arch_relocate_image(unsigned long delta)
     }
 }
 
-extern const int32_t __trampoline_rel_start[], __trampoline_rel_stop[];
-extern const int32_t __trampoline_seg_start[], __trampoline_seg_stop[];
+void reloc_trampoline64(void);
 
 static void __init relocate_trampoline(unsigned long phys)
 {
-    const int32_t *trampoline_ptr;
-
     trampoline_phys = phys;
 
     if ( !efi_enabled(EFI_LOADER) )
         return;
 
-    /* Apply relocations to trampoline. */
-    for ( trampoline_ptr = __trampoline_rel_start;
-          trampoline_ptr < __trampoline_rel_stop;
-          ++trampoline_ptr )
-        *(u32 *)(*trampoline_ptr + (long)trampoline_ptr) += phys;
-    for ( trampoline_ptr = __trampoline_seg_start;
-          trampoline_ptr < __trampoline_seg_stop;
-          ++trampoline_ptr )
-        *(u16 *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
+    reloc_trampoline64();
 }
 
 static void __init place_string(u32 *addr, const char *s)
-- 
2.34.1
Re: [PATCH v6 2/5] x86/boot: Reuse code to relocate trampoline
Posted by Marek Marczykowski-Górecki 1 month ago 
On Thu, Oct 17, 2024 at 02:31:20PM +0100, Frediano Ziglio wrote:
> Move code from efi-boot.h to a separate, new, reloc-trampoline.c file.
> Reuse this new code, compiling it for 32bit as well, to replace assembly
> code in head.S doing the same thing.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

For the EFI part:
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> ---
> Changes since v3:
> - fixed a typo in comment;
> - added Reviewed-by.
> 
> Changes since v5:
> - do not add obj64 to targets, already done adding it to obj-bin-y.
> ---
>  xen/arch/x86/boot/Makefile           | 10 +++++---
>  xen/arch/x86/boot/build32.lds.S      |  5 ++++
>  xen/arch/x86/boot/head.S             | 23 +-----------------
>  xen/arch/x86/boot/reloc-trampoline.c | 36 ++++++++++++++++++++++++++++
>  xen/arch/x86/efi/efi-boot.h          | 15 ++----------
>  5 files changed, 51 insertions(+), 38 deletions(-)
>  create mode 100644 xen/arch/x86/boot/reloc-trampoline.c
> 
> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> index 5da19501be..98ceb1983d 100644
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -1,11 +1,15 @@
>  obj-bin-y += head.o
>  obj-bin-y += built-in-32.o
> +obj-bin-y += $(obj64)
>  
>  obj32 := cmdline.32.o
>  obj32 += reloc.32.o
> +obj32 += reloc-trampoline.32.o
>  
> -nocov-y   += $(obj32)
> -noubsan-y += $(obj32)
> +obj64 := reloc-trampoline.o
> +
> +nocov-y   += $(obj32) $(obj64)
> +noubsan-y += $(obj32) $(obj64)
>  targets   += $(obj32)
>  
>  obj32 := $(addprefix $(obj)/,$(obj32))
> @@ -56,7 +60,7 @@ cmd_combine = \
>  		--bin1 $(obj)/built-in-32.base.bin \
>  		--bin2 $(obj)/built-in-32.offset.bin \
>  		--map $(obj)/built-in-32.offset.map \
> -		--exports cmdline_parse_early,reloc \
> +		--exports cmdline_parse_early,reloc,reloc_trampoline32 \
>  		--output $@
>  
>  targets += built-in-32.S
> diff --git a/xen/arch/x86/boot/build32.lds.S b/xen/arch/x86/boot/build32.lds.S
> index e3f5e55261..fa282370f4 100644
> --- a/xen/arch/x86/boot/build32.lds.S
> +++ b/xen/arch/x86/boot/build32.lds.S
> @@ -41,6 +41,11 @@ SECTIONS
>           * Potentially they should be all variables. */
>          DECLARE_IMPORT(__base_relocs_start);
>          DECLARE_IMPORT(__base_relocs_end);
> +        DECLARE_IMPORT(__trampoline_rel_start);
> +        DECLARE_IMPORT(__trampoline_rel_stop);
> +        DECLARE_IMPORT(__trampoline_seg_start);
> +        DECLARE_IMPORT(__trampoline_seg_stop);
> +        DECLARE_IMPORT(trampoline_phys);
>          . = . + GAP;
>          *(.text)
>          *(.text.*)
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index e0776e3896..ade2c5c43d 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -706,28 +706,7 @@ trampoline_setup:
>          mov     %edx, sym_offs(l1_bootmap)(%esi, %ecx, 8)
>  
>          /* Apply relocations to bootstrap trampoline. */
> -        mov     sym_esi(trampoline_phys), %edx
> -        lea     sym_esi(__trampoline_rel_start), %edi
> -        lea     sym_esi(__trampoline_rel_stop), %ecx
> -1:
> -        mov     (%edi), %eax
> -        add     %edx, (%edi, %eax)
> -        add     $4,%edi
> -
> -        cmp     %ecx, %edi
> -        jb      1b
> -
> -        /* Patch in the trampoline segment. */
> -        shr     $4,%edx
> -        lea     sym_esi(__trampoline_seg_start), %edi
> -        lea     sym_esi(__trampoline_seg_stop), %ecx
> -1:
> -        mov     (%edi), %eax
> -        mov     %dx, (%edi, %eax)
> -        add     $4,%edi
> -
> -        cmp     %ecx, %edi
> -        jb      1b
> +        call    reloc_trampoline32
>  
>          /* Do not parse command line on EFI platform here. */
>          cmpb    $0, sym_esi(efi_platform)
> diff --git a/xen/arch/x86/boot/reloc-trampoline.c b/xen/arch/x86/boot/reloc-trampoline.c
> new file mode 100644
> index 0000000000..0a74c1e75a
> --- /dev/null
> +++ b/xen/arch/x86/boot/reloc-trampoline.c
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/compiler.h>
> +#include <xen/stdint.h>
> +#include <asm/trampoline.h>
> +
> +extern const int32_t __trampoline_rel_start[], __trampoline_rel_stop[];
> +extern const int32_t __trampoline_seg_start[], __trampoline_seg_stop[];
> +
> +#if defined(__i386__)
> +void reloc_trampoline32(void)
> +#elif defined (__x86_64__)
> +void reloc_trampoline64(void)
> +#else
> +#error Unknown architecture
> +#endif
> +{
> +    unsigned long phys = trampoline_phys;
> +    const int32_t *trampoline_ptr;
> +
> +    /*
> +     * Apply relocations to trampoline.
> +     *
> +     * This modifies the trampoline in place within Xen, so that it will
> +     * operate correctly when copied into place.
> +     */
> +    for ( trampoline_ptr = __trampoline_rel_start;
> +          trampoline_ptr < __trampoline_rel_stop;
> +          ++trampoline_ptr )
> +        *(uint32_t *)(*trampoline_ptr + (long)trampoline_ptr) += phys;
> +
> +    for ( trampoline_ptr = __trampoline_seg_start;
> +          trampoline_ptr < __trampoline_seg_stop;
> +          ++trampoline_ptr )
> +        *(uint16_t *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
> +}
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index 94f3443364..1acceec471 100644
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -103,27 +103,16 @@ static void __init efi_arch_relocate_image(unsigned long delta)
>      }
>  }
>  
> -extern const int32_t __trampoline_rel_start[], __trampoline_rel_stop[];
> -extern const int32_t __trampoline_seg_start[], __trampoline_seg_stop[];
> +void reloc_trampoline64(void);
>  
>  static void __init relocate_trampoline(unsigned long phys)
>  {
> -    const int32_t *trampoline_ptr;
> -
>      trampoline_phys = phys;
>  
>      if ( !efi_enabled(EFI_LOADER) )
>          return;
>  
> -    /* Apply relocations to trampoline. */
> -    for ( trampoline_ptr = __trampoline_rel_start;
> -          trampoline_ptr < __trampoline_rel_stop;
> -          ++trampoline_ptr )
> -        *(u32 *)(*trampoline_ptr + (long)trampoline_ptr) += phys;
> -    for ( trampoline_ptr = __trampoline_seg_start;
> -          trampoline_ptr < __trampoline_seg_stop;
> -          ++trampoline_ptr )
> -        *(u16 *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
> +    reloc_trampoline64();
>  }
>  
>  static void __init place_string(u32 *addr, const char *s)
> -- 
> 2.34.1
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.