* [PATCH 0/3] x86/kexec: ASM improvements
@ 2023-02-17 17:48 Andrew Cooper
2023-02-17 17:48 ` [PATCH 1/3] x86/kexec: Drop compatibility_mode_far Andrew Cooper
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Andrew Cooper @ 2023-02-17 17:48 UTC (permalink / raw
To: Xen-devel; +Cc: Andrew Cooper
Mostly to get ELF metadata, but some other easy improvements too.
Andrew Cooper (3):
x86/kexec: Drop compatibility_mode_far
x86/kexec: Simplify the relocation of compat_mode_gdt_desc
x86/kexec: Annotate functions with ELF metadata
xen/arch/x86/x86_64/kexec_reloc.S | 40 +++++++++++++++++--------------
1 file changed, 22 insertions(+), 18 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] x86/kexec: Drop compatibility_mode_far
2023-02-17 17:48 [PATCH 0/3] x86/kexec: ASM improvements Andrew Cooper
@ 2023-02-17 17:48 ` Andrew Cooper
2023-02-21 10:34 ` Jan Beulich
2023-02-17 17:48 ` [PATCH 2/3] x86/kexec: Simplify the relocation of compat_mode_gdt_desc Andrew Cooper
2023-02-17 17:48 ` [PATCH 3/3] x86/kexec: Annotate functions with ELF metadata Andrew Cooper
2 siblings, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2023-02-17 17:48 UTC (permalink / raw
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
ljmp is (famously?) incompatible between Intel and AMD CPUs, and while we're
using one of the compatible forms, we've got a good stack and lret is the far
more common way of doing this.
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 | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/xen/arch/x86/x86_64/kexec_reloc.S b/xen/arch/x86/x86_64/kexec_reloc.S
index f4842025eb56..035164e96f38 100644
--- a/xen/arch/x86/x86_64/kexec_reloc.S
+++ b/xen/arch/x86/x86_64/kexec_reloc.S
@@ -86,12 +86,11 @@ ENTRY(kexec_reloc)
movq %rax, (compat_mode_gdt_desc + 2)(%rip)
lgdt compat_mode_gdt_desc(%rip)
- /* Relocate compatibility mode entry point address. */
- leal compatibility_mode(%rip), %eax
- movl %eax, compatibility_mode_far(%rip)
-
/* Enter compatibility mode. */
- ljmp *compatibility_mode_far(%rip)
+ lea compatibility_mode(%rip), %rax
+ push $0x10
+ push %rax
+ lretq
relocate_pages:
/* %rdi - indirection page maddr */
@@ -171,13 +170,6 @@ compatibility_mode:
ud2
.align 4
-compatibility_mode_far:
- .long 0x00000000 /* set in call_32_bit above */
- .word 0x0010
-
- .type compatibility_mode_far, @object
- .size compatibility_mode_far, . - compatibility_mode_far
-
compat_mode_gdt_desc:
.word .Lcompat_mode_gdt_end - compat_mode_gdt -1
.quad 0x0000000000000000 /* set in call_32_bit above */
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] x86/kexec: Simplify the relocation of compat_mode_gdt_desc
2023-02-17 17:48 [PATCH 0/3] x86/kexec: ASM improvements Andrew Cooper
2023-02-17 17:48 ` [PATCH 1/3] x86/kexec: Drop compatibility_mode_far Andrew Cooper
@ 2023-02-17 17:48 ` Andrew Cooper
2023-02-21 10:48 ` Jan Beulich
2023-02-17 17:48 ` [PATCH 3/3] x86/kexec: Annotate functions with ELF metadata Andrew Cooper
2 siblings, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2023-02-17 17:48 UTC (permalink / raw
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] x86/kexec: Annotate functions with ELF metadata
2023-02-17 17:48 [PATCH 0/3] x86/kexec: ASM improvements Andrew Cooper
2023-02-17 17:48 ` [PATCH 1/3] x86/kexec: Drop compatibility_mode_far Andrew Cooper
2023-02-17 17:48 ` [PATCH 2/3] x86/kexec: Simplify the relocation of compat_mode_gdt_desc Andrew Cooper
@ 2023-02-17 17:48 ` Andrew Cooper
2023-02-21 10:54 ` Jan Beulich
2 siblings, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2023-02-17 17:48 UTC (permalink / raw
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] x86/kexec: Drop compatibility_mode_far
2023-02-17 17:48 ` [PATCH 1/3] x86/kexec: Drop compatibility_mode_far Andrew Cooper
@ 2023-02-21 10:34 ` Jan Beulich
0 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2023-02-21 10:34 UTC (permalink / raw
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 17.02.2023 18:48, Andrew Cooper wrote:
> ljmp is (famously?) incompatible between Intel and AMD CPUs, and while we're
> using one of the compatible forms, we've got a good stack and lret is the far
> more common way of doing this.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
One question though:
> --- a/xen/arch/x86/x86_64/kexec_reloc.S
> +++ b/xen/arch/x86/x86_64/kexec_reloc.S
> @@ -86,12 +86,11 @@ ENTRY(kexec_reloc)
> movq %rax, (compat_mode_gdt_desc + 2)(%rip)
> lgdt compat_mode_gdt_desc(%rip)
>
> - /* Relocate compatibility mode entry point address. */
> - leal compatibility_mode(%rip), %eax
> - movl %eax, compatibility_mode_far(%rip)
> -
> /* Enter compatibility mode. */
> - ljmp *compatibility_mode_far(%rip)
> + lea compatibility_mode(%rip), %rax
> + push $0x10
Any thought about making this literal number a proper expression,
rendering the code a little less fragile?
Jan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] x86/kexec: Simplify the relocation of compat_mode_gdt_desc
2023-02-17 17:48 ` [PATCH 2/3] x86/kexec: Simplify the relocation of compat_mode_gdt_desc Andrew Cooper
@ 2023-02-21 10:48 ` Jan Beulich
0 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2023-02-21 10:48 UTC (permalink / raw
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] x86/kexec: Annotate functions with ELF metadata
2023-02-17 17:48 ` [PATCH 3/3] x86/kexec: Annotate functions with ELF metadata Andrew Cooper
@ 2023-02-21 10:54 ` Jan Beulich
0 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2023-02-21 10:54 UTC (permalink / raw
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
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 */
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-21 10:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-17 17:48 [PATCH 0/3] x86/kexec: ASM improvements Andrew Cooper
2023-02-17 17:48 ` [PATCH 1/3] x86/kexec: Drop compatibility_mode_far Andrew Cooper
2023-02-21 10:34 ` Jan Beulich
2023-02-17 17:48 ` [PATCH 2/3] x86/kexec: Simplify the relocation of compat_mode_gdt_desc Andrew Cooper
2023-02-21 10:48 ` Jan Beulich
2023-02-17 17:48 ` [PATCH 3/3] x86/kexec: Annotate functions with ELF metadata Andrew Cooper
2023-02-21 10:54 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).