All of kexec_reloc(), relocate_pages() and compatibility_mode() are
function-like.  Annotate them appropriately.

Furthermore, move the data into a different cacheline from the code, so the
relocation of compat_mode_gdt_desc doesn't trigger self-modifying safety logic
in the pipeline.

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 | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/x86_64/kexec_reloc.S b/xen/arch/x86/x86_64/kexec_reloc.S
index a81f64146190..c7fc11fa5868 100644
--- a/xen/arch/x86/x86_64/kexec_reloc.S
+++ b/xen/arch/x86/x86_64/kexec_reloc.S
@@ -13,6 +13,7 @@
 
         .file __FILE__
 
+#include <xen/cache.h>
 #include <xen/kimage.h>
 
 #include <asm/asm_defns.h>
@@ -90,7 +91,10 @@ ENTRY(kexec_reloc)
         push    %rax
         lretq
 
-relocate_pages:
+        .type kexec_reloc, @function
+        .size kexec_reloc, . - kexec_reloc
+
+ENTRY(relocate_pages)
         /* %rdi - indirection page maddr */
         pushq   %rbx
 
@@ -137,9 +141,12 @@ relocate_pages:
         popq    %rbx
         ret
 
+        .type relocate_pages, @function
+        .size relocate_pages, . - relocate_pages
+
         .code32
 
-compatibility_mode:
+ENTRY(compatibility_mode)
         /* Setup some sane segments. */
         movl    $0x0008, %eax
         movl    %eax, %ds
@@ -167,7 +174,14 @@ compatibility_mode:
         call    *%ebp
         ud2
 
-        .align 4
+        .type compatibility_mode, @function
+        .size compatibility_mode, . - compatibility_mode
+
+        /*
+         * Ensure data is in a different cache line to code.
+         */
+        .align SMP_CACHE_BYTES, 0
+
 compat_mode_gdt_desc:
         .word .Lcompat_mode_gdt_end - compat_mode_gdt -1
         .quad . - kexec_reloc        /* Relocated before use */
-- 
2.30.2

On 17.02.2023 18:48, Andrew Cooper wrote:
> @@ -90,7 +91,10 @@ ENTRY(kexec_reloc)
>          push    %rax
>          lretq
>  
> -relocate_pages:
> +        .type kexec_reloc, @function
> +        .size kexec_reloc, . - kexec_reloc
> +
> +ENTRY(relocate_pages)
>          /* %rdi - indirection page maddr */
>          pushq   %rbx
>  
> @@ -137,9 +141,12 @@ relocate_pages:
>          popq    %rbx
>          ret
>  
> +        .type relocate_pages, @function
> +        .size relocate_pages, . - relocate_pages
> +
>          .code32
>  
> -compatibility_mode:
> +ENTRY(compatibility_mode)

Do you really mean to make both labels global, thus potentially risking
a link-time name collision down the road? In C files we try to move the
other direction after all, making symbols static which can be.

> @@ -167,7 +174,14 @@ compatibility_mode:
>          call    *%ebp
>          ud2
>  
> -        .align 4
> +        .type compatibility_mode, @function
> +        .size compatibility_mode, . - compatibility_mode
> +
> +        /*
> +         * Ensure data is in a different cache line to code.
> +         */

Nit (style): Strictly speaking this is a single-line comment.

Jan

> +        .align SMP_CACHE_BYTES, 0
> +
>  compat_mode_gdt_desc:
>          .word .Lcompat_mode_gdt_end - compat_mode_gdt -1
>          .quad . - kexec_reloc        /* Relocated before use */