[Xen-devel] [PATCH] x86/boot: Reposition trampoline data

Andrew Cooper posted 1 patch 4 years, 8 months ago
Failed in applying to current master (apply log)
xen/arch/x86/boot/trampoline.S | 67 ++++++++++++++++++------------------------
1 file changed, 28 insertions(+), 39 deletions(-)
[Xen-devel] [PATCH] x86/boot: Reposition trampoline data
Posted by Andrew Cooper 4 years, 8 months ago
... to separate code from data.  In particular, trampoline_realmode_entry's
write to trampoline_cpu_started clobbers the I-cache line containing
trampoline_protmode_entry, which won't be great for AP startup.

Reformat the comments for trampoline_gdt to reduce their volume.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/boot/trampoline.S | 67 ++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 39 deletions(-)

diff --git a/xen/arch/x86/boot/trampoline.S b/xen/arch/x86/boot/trampoline.S
index 1761fc1213..1b11b4757a 100644
--- a/xen/arch/x86/boot/trampoline.S
+++ b/xen/arch/x86/boot/trampoline.S
@@ -59,45 +59,6 @@ GLOBAL(trampoline_realmode_entry)
         lmsw    %ax                       # CR0.PE = 1 (enter protected mode)
         ljmpl   $BOOT_CS32,$bootsym_rel(trampoline_protmode_entry,6)
 
-trampoline_gdt:
-        /* 0x0000: unused */
-        .quad   0x0000000000000000
-        /* 0x0008: ring 0 code, 32-bit mode */
-        .quad   0x00cf9b000000ffff
-        /* 0x0010: ring 0 code, 64-bit mode */
-        .quad   0x00af9b000000ffff
-        /* 0x0018: ring 0 data */
-        .quad   0x00cf93000000ffff
-        /* 0x0020: real-mode code @ BOOT_TRAMPOLINE */
-        .long   0x0000ffff
-        .long   0x00009b00
-        /* 0x0028: real-mode data @ BOOT_TRAMPOLINE */
-        .long   0x0000ffff
-        .long   0x00009300
-        /*
-         * 0x0030: ring 0 Xen data, 16 MiB size, base
-         * address is computed at runtime.
-         */
-        .quad   0x00c0930000000fff
-.Ltramopline_gdt_end:
-
-        .pushsection .trampoline_rel, "a"
-        .long   trampoline_gdt + BOOT_PSEUDORM_CS + 2 - .
-        .long   trampoline_gdt + BOOT_PSEUDORM_DS + 2 - .
-        .popsection
-
-GLOBAL(trampoline_misc_enable_off)
-        .quad   0
-
-GLOBAL(cpuid_ext_features)
-        .long   0
-
-GLOBAL(trampoline_xen_phys_start)
-        .long   0
-
-GLOBAL(trampoline_cpu_started)
-        .byte   0
-
         .code32
 trampoline_protmode_entry:
         /* Set up a few descriptors: on entry only CS is guaranteed good. */
@@ -186,6 +147,34 @@ idt_48: .word   0, 0, 0 # base = limit = 0
 gdt_48: .word   .Ltramopline_gdt_end - trampoline_gdt - 1
         .long   bootsym_rel(trampoline_gdt,4)
 
+trampoline_gdt:
+        .quad   0x0000000000000000 /* 0x0000: unused */
+        .quad   0x00cf9b000000ffff /* 0x0008: ring 0 code, 32-bit mode */
+        .quad   0x00af9b000000ffff /* 0x0010: ring 0 code, 64-bit mode */
+        .quad   0x00cf93000000ffff /* 0x0018: ring 0 data */
+        .quad   0x00009b000000ffff /* 0x0020: real-mode code @ BOOT_TRAMPOLINE */
+        .quad   0x000093000000ffff /* 0x0028: real-mode data @ BOOT_TRAMPOLINE */
+        .quad   0x00c0930000000fff /* 0x0030: ring 0 Xen data, 16M @ XEN */
+.Ltramopline_gdt_end:
+
+        /* Relocations for trampoline Real Mode segments. */
+        .pushsection .trampoline_rel, "a"
+        .long   trampoline_gdt + BOOT_PSEUDORM_CS + 2 - .
+        .long   trampoline_gdt + BOOT_PSEUDORM_DS + 2 - .
+        .popsection
+
+GLOBAL(trampoline_misc_enable_off)
+        .quad   0
+
+GLOBAL(cpuid_ext_features)
+        .long   0
+
+GLOBAL(trampoline_xen_phys_start)
+        .long   0
+
+GLOBAL(trampoline_cpu_started)
+        .byte   0
+
 /* The first page of trampoline is permanent, the rest boot-time only. */
 /* Reuse the boot trampoline on the 1st trampoline page as stack for wakeup. */
         .equ    wakeup_stack, trampoline_start + PAGE_SIZE
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: Reposition trampoline data
Posted by Jan Beulich 4 years, 8 months ago
On 19.08.2019 15:42, Andrew Cooper wrote:
> ... to separate code from data.  In particular, trampoline_realmode_entry's
> write to trampoline_cpu_started clobbers the I-cache line containing
> trampoline_protmode_entry, which won't be great for AP startup.
> 
> Reformat the comments for trampoline_gdt to reduce their volume.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

, ... subject to David's concern wrt his series.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: Reposition trampoline data
Posted by David Woodhouse 4 years, 8 months ago
On Mon, 2019-08-19 at 14:42 +0100, Andrew Cooper wrote:
> ... to separate code from data.  In particular,
> trampoline_realmode_entry's
> write to trampoline_cpu_started clobbers the I-cache line containing
> trampoline_protmode_entry, which won't be great for AP startup.
> 
> Reformat the comments for trampoline_gdt to reduce their volume.
> 
> No functional change.

Please, let's not do this one until my other boot cleanups have landed.
It just hurts. I have also reordered some of these for functional
reasons, because they are used in different contexts (and end up in
completely different trampolines).

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wl@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  xen/arch/x86/boot/trampoline.S | 67 ++++++++++++++++++------------
> ------------
>  1 file changed, 28 insertions(+), 39 deletions(-)
> 
> diff --git a/xen/arch/x86/boot/trampoline.S
> b/xen/arch/x86/boot/trampoline.S
> index 1761fc1213..1b11b4757a 100644
> --- a/xen/arch/x86/boot/trampoline.S
> +++ b/xen/arch/x86/boot/trampoline.S
> @@ -59,45 +59,6 @@ GLOBAL(trampoline_realmode_entry)
>          lmsw    %ax                       # CR0.PE = 1 (enter
> protected mode)
>          ljmpl   $BOOT_CS32,$bootsym_rel(trampoline_protmode_entry,6)
>  
> -trampoline_gdt:
> -        /* 0x0000: unused */
> -        .quad   0x0000000000000000
> -        /* 0x0008: ring 0 code, 32-bit mode */
> -        .quad   0x00cf9b000000ffff
> -        /* 0x0010: ring 0 code, 64-bit mode */
> -        .quad   0x00af9b000000ffff
> -        /* 0x0018: ring 0 data */
> -        .quad   0x00cf93000000ffff
> -        /* 0x0020: real-mode code @ BOOT_TRAMPOLINE */
> -        .long   0x0000ffff
> -        .long   0x00009b00
> -        /* 0x0028: real-mode data @ BOOT_TRAMPOLINE */
> -        .long   0x0000ffff
> -        .long   0x00009300
> -        /*
> -         * 0x0030: ring 0 Xen data, 16 MiB size, base
> -         * address is computed at runtime.
> -         */
> -        .quad   0x00c0930000000fff
> -.Ltramopline_gdt_end:
> -
> -        .pushsection .trampoline_rel, "a"
> -        .long   trampoline_gdt + BOOT_PSEUDORM_CS + 2 - .
> -        .long   trampoline_gdt + BOOT_PSEUDORM_DS + 2 - .
> -        .popsection
> -
> -GLOBAL(trampoline_misc_enable_off)
> -        .quad   0
> -
> -GLOBAL(cpuid_ext_features)
> -        .long   0
> -
> -GLOBAL(trampoline_xen_phys_start)
> -        .long   0
> -
> -GLOBAL(trampoline_cpu_started)
> -        .byte   0
> -
>          .code32
>  trampoline_protmode_entry:
>          /* Set up a few descriptors: on entry only CS is guaranteed
> good. */
> @@ -186,6 +147,34 @@ idt_48: .word   0, 0, 0 # base = limit = 0
>  gdt_48: .word   .Ltramopline_gdt_end - trampoline_gdt - 1
>          .long   bootsym_rel(trampoline_gdt,4)
>  
> +trampoline_gdt:
> +        .quad   0x0000000000000000 /* 0x0000: unused */
> +        .quad   0x00cf9b000000ffff /* 0x0008: ring 0 code, 32-bit
> mode */
> +        .quad   0x00af9b000000ffff /* 0x0010: ring 0 code, 64-bit
> mode */
> +        .quad   0x00cf93000000ffff /* 0x0018: ring 0 data */
> +        .quad   0x00009b000000ffff /* 0x0020: real-mode code @
> BOOT_TRAMPOLINE */
> +        .quad   0x000093000000ffff /* 0x0028: real-mode data @
> BOOT_TRAMPOLINE */
> +        .quad   0x00c0930000000fff /* 0x0030: ring 0 Xen data, 16M @
> XEN */
> +.Ltramopline_gdt_end:
> +
> +        /* Relocations for trampoline Real Mode segments. */
> +        .pushsection .trampoline_rel, "a"
> +        .long   trampoline_gdt + BOOT_PSEUDORM_CS + 2 - .
> +        .long   trampoline_gdt + BOOT_PSEUDORM_DS + 2 - .
> +        .popsection
> +
> +GLOBAL(trampoline_misc_enable_off)
> +        .quad   0
> +
> +GLOBAL(cpuid_ext_features)
> +        .long   0
> +
> +GLOBAL(trampoline_xen_phys_start)
> +        .long   0
> +
> +GLOBAL(trampoline_cpu_started)
> +        .byte   0
> +
>  /* The first page of trampoline is permanent, the rest boot-time
> only. */
>  /* Reuse the boot trampoline on the 1st trampoline page as stack for
> wakeup. */
>          .equ    wakeup_stack, trampoline_start + PAGE_SIZE

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel