xen/arch/x86/include/asm/machine_kexec.h | 2 +- xen/arch/x86/machine_kexec.c | 2 +- xen/arch/x86/x86_64/kexec_reloc.S | 22 +++++++++++++++++----- xen/arch/x86/xen.lds.S | 3 ++- 4 files changed, 21 insertions(+), 8 deletions(-)
Scanning for embedded endbranch instructions involves parsing the .text
disassembly. Data in the kexec trampoline has no ELF metadata, so objdump
treats it as instructions and tries to disassemble. Convert:
ffff82d040396108 <compatibility_mode_far>:
ffff82d040396108: 00 00 add %al,(%rax)
ffff82d04039610a: 00 00 add %al,(%rax)
ffff82d04039610c: 10 00 adc %al,(%rax)
ffff82d04039610e <compat_mode_gdt_desc>:
ffff82d04039610e: 17 (bad)
...
ffff82d040396118 <compat_mode_gdt>:
...
ffff82d040396120: ff (bad)
ffff82d040396121: ff 00 incl (%rax)
ffff82d040396123: 00 00 add %al,(%rax)
ffff82d040396125: 93 xchg %eax,%ebx
ffff82d040396126: cf iret
ffff82d040396127: 00 ff add %bh,%bh
ffff82d040396129: ff 00 incl (%rax)
ffff82d04039612b: 00 00 add %al,(%rax)
ffff82d04039612d: 9b fwait
ffff82d04039612e: cf iret
...
ffff82d040396130 <compat_mode_idt>:
...
ffff82d0403961b6 <kexec_reloc_size>:
ffff82d0403961b6: b6 01 mov $0x1,%dh
...
to:
ffff82d040396108 <compatibility_mode_far>:
ffff82d040396108: 00 00 00 00 10 00 ......
ffff82d04039610e <compat_mode_gdt_desc>:
ffff82d04039610e: 17 00 00 00 00 00 00 00 00 00 ..........
ffff82d040396118 <compat_mode_gdt>:
...
ffff82d040396120: ff ff 00 00 00 93 cf 00 ff ff 00 00 00 9b cf 00 ................
ffff82d040396130 <compat_mode_idt>:
ffff82d040396130: 00 00 00 00 00 00 ......
ffff82d040396136 <reloc_stack>:
...
Most data just gains type and size metadata. The reloc_stack label is the
wrong end of the data block to have a size, so move it to the lowest address
and introduce .Lreloc_stack_base as a replacement.
While kexec_reloc_size could gain metadata, it's use in the linker
assertion (while correct) is deeply confusing to follow. Drop it entirely,
using a linker symbol instead to denote the end of the trampoline.
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>
The remainder of the 32bit code has mode-invariant lengths, so disassembles
safely as 64bit. The only differences come from 32/64bit implicit register
sizes.
v2.1:
* New
---
xen/arch/x86/include/asm/machine_kexec.h | 2 +-
xen/arch/x86/machine_kexec.c | 2 +-
xen/arch/x86/x86_64/kexec_reloc.S | 22 +++++++++++++++++-----
xen/arch/x86/xen.lds.S | 3 ++-
4 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/include/asm/machine_kexec.h b/xen/arch/x86/include/asm/machine_kexec.h
index ba0d469d077b..d4880818c1d9 100644
--- a/xen/arch/x86/include/asm/machine_kexec.h
+++ b/xen/arch/x86/include/asm/machine_kexec.h
@@ -9,7 +9,7 @@ extern void kexec_reloc(unsigned long reloc_code, unsigned long reloc_pt,
unsigned long ind_maddr, unsigned long entry_maddr,
unsigned long flags);
-extern unsigned int kexec_reloc_size;
+extern const char kexec_reloc_end[];
#endif
diff --git a/xen/arch/x86/machine_kexec.c b/xen/arch/x86/machine_kexec.c
index 08ec9fd43b1d..751a9efcaf6a 100644
--- a/xen/arch/x86/machine_kexec.c
+++ b/xen/arch/x86/machine_kexec.c
@@ -117,7 +117,7 @@ int machine_kexec_load(struct kexec_image *image)
}
code_page = __map_domain_page(image->control_code_page);
- memcpy(code_page, kexec_reloc, kexec_reloc_size);
+ memcpy(code_page, kexec_reloc, kexec_reloc_end - (char *)kexec_reloc);
unmap_domain_page(code_page);
/*
diff --git a/xen/arch/x86/x86_64/kexec_reloc.S b/xen/arch/x86/x86_64/kexec_reloc.S
index d488d127cfb9..05bf8810cee6 100644
--- a/xen/arch/x86/x86_64/kexec_reloc.S
+++ b/xen/arch/x86/x86_64/kexec_reloc.S
@@ -34,7 +34,7 @@ ENTRY(kexec_reloc)
movq %rcx, %rbp
/* Setup stack. */
- leaq (reloc_stack - kexec_reloc)(%rdi), %rsp
+ leaq (.Lreloc_stack_base - kexec_reloc)(%rdi), %rsp
/* Load reloc page table. */
movq %rsi, %cr3
@@ -175,10 +175,16 @@ 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 */
+ .type compat_mode_gdt_desc, @object
+ .size compat_mode_gdt_desc, . - compat_mode_gdt_desc
+
.align 8
compat_mode_gdt:
.quad 0x0000000000000000 /* null */
@@ -186,16 +192,22 @@ compat_mode_gdt:
.quad 0x00cf9b000000ffff /* 0x0010 ring 0 code, compatibility */
.Lcompat_mode_gdt_end:
+ .type compat_mode_gdt, @object
+ .size compat_mode_gdt, . - compat_mode_gdt
+
compat_mode_idt:
.word 0 /* limit */
.long 0 /* base */
+ .type compat_mode_idt, @object
+ .size compat_mode_idt, . - compat_mode_idt
+
/*
* 16 words of stack are more than enough.
*/
- .fill 16,8,0
reloc_stack:
+ .fill 16,8,0
+.Lreloc_stack_base:
- .globl kexec_reloc_size
-kexec_reloc_size:
- .long . - kexec_reloc
+ .type reloc_stack, @object
+ .size reloc_stack, . - reloc_stack
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index c399178ac123..13fc7ee008c1 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -87,6 +87,7 @@ SECTIONS
*(.text.unlikely)
*(.fixup)
*(.text.kexec)
+ kexec_reloc_end = .;
*(.gnu.warning)
_etext = .; /* End of text section */
} PHDR(text) = 0x9090
@@ -433,7 +434,7 @@ ASSERT(__2M_rwdata_end <= XEN_VIRT_END - XEN_VIRT_START + __XEN_VIRT_START -
"Xen image overlaps stubs area")
#ifdef CONFIG_KEXEC
-ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too large")
+ASSERT(kexec_reloc_end - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too large")
#endif
/* The Multiboot setup paths relies on this to simplify superpage PTE creation. */
--
2.11.0
On 17.02.2022 11:01, Andrew Cooper wrote: > Scanning for embedded endbranch instructions involves parsing the .text > disassembly. Data in the kexec trampoline has no ELF metadata, so objdump > treats it as instructions and tries to disassemble. Convert: > > ffff82d040396108 <compatibility_mode_far>: What about the (possible) padding ahead of this? Should the .align there perhaps specify a filler character? > ffff82d040396108: 00 00 add %al,(%rax) > ffff82d04039610a: 00 00 add %al,(%rax) > ffff82d04039610c: 10 00 adc %al,(%rax) > > ffff82d04039610e <compat_mode_gdt_desc>: > ffff82d04039610e: 17 (bad) > ... > > ffff82d040396118 <compat_mode_gdt>: > ... > ffff82d040396120: ff (bad) > ffff82d040396121: ff 00 incl (%rax) > ffff82d040396123: 00 00 add %al,(%rax) > ffff82d040396125: 93 xchg %eax,%ebx > ffff82d040396126: cf iret > ffff82d040396127: 00 ff add %bh,%bh > ffff82d040396129: ff 00 incl (%rax) > ffff82d04039612b: 00 00 add %al,(%rax) > ffff82d04039612d: 9b fwait > ffff82d04039612e: cf iret > ... > > ffff82d040396130 <compat_mode_idt>: > ... > > ffff82d0403961b6 <kexec_reloc_size>: > ffff82d0403961b6: b6 01 mov $0x1,%dh > ... > > to: > > ffff82d040396108 <compatibility_mode_far>: > ffff82d040396108: 00 00 00 00 10 00 ...... > > ffff82d04039610e <compat_mode_gdt_desc>: > ffff82d04039610e: 17 00 00 00 00 00 00 00 00 00 .......... > > ffff82d040396118 <compat_mode_gdt>: > ... > ffff82d040396120: ff ff 00 00 00 93 cf 00 ff ff 00 00 00 9b cf 00 ................ > > ffff82d040396130 <compat_mode_idt>: > ffff82d040396130: 00 00 00 00 00 00 ...... With the .size directives added, can we rely on consistent (past, present, and future) objcopy behavior for padding gaps? It just so happens that there's no 4-byte gap between compat_mode_gdt_desc and compat_mode_gdt. Changing the .align ahead of compatibility_mode_far would eliminate the risk of padding appearing if the code further up changed. > ffff82d040396136 <reloc_stack>: > ... Now this is particularly puzzling: Us setting %rsp to an unaligned address is clearly not ABI-conforming. Since you're fiddling with all of this already anyway, how about fixing this at the same time? Of course there would then appear padding ahead of the stack, unless the stack was moved up some. > @@ -175,10 +175,16 @@ 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 */ > > + .type compat_mode_gdt_desc, @object > + .size compat_mode_gdt_desc, . - compat_mode_gdt_desc Side note: We really ought to gain something like OBJECT(name) to avoid c'n'p mistakes not updating correctly all three symbol name instances. > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -87,6 +87,7 @@ SECTIONS > *(.text.unlikely) > *(.fixup) > *(.text.kexec) > + kexec_reloc_end = .; Does this maybe want aligning on a 4- or even 8-byte boundary? If so, imo preferably not here, but by adding a trailing .align in the .S file. Jan
On 17/02/2022 10:42, Jan Beulich wrote: > On 17.02.2022 11:01, Andrew Cooper wrote: >> Scanning for embedded endbranch instructions involves parsing the .text >> disassembly. Data in the kexec trampoline has no ELF metadata, so objdump >> treats it as instructions and tries to disassemble. Convert: >> >> ffff82d040396108 <compatibility_mode_far>: > What about the (possible) padding ahead of this? Should the .align > there perhaps specify a filler character? What about it? It's just long nops like all other padding in .text ffff82d040396101: ff d5 call *%ebp ffff82d040396103: 0f 0b ud2 ffff82d040396105: 0f 1f 00 nopl (%eax) ffff82d040396108 <compatibility_mode_far>: ffff82d040396108: 00 00 00 00 10 00 ...... And for this problem, we don't need to care about the behaviour of any pre-CET version of binutils. >> ffff82d040396108: 00 00 add %al,(%rax) >> ffff82d04039610a: 00 00 add %al,(%rax) >> ffff82d04039610c: 10 00 adc %al,(%rax) >> >> ffff82d04039610e <compat_mode_gdt_desc>: >> ffff82d04039610e: 17 (bad) >> ... >> >> ffff82d040396118 <compat_mode_gdt>: >> ... >> ffff82d040396120: ff (bad) >> ffff82d040396121: ff 00 incl (%rax) >> ffff82d040396123: 00 00 add %al,(%rax) >> ffff82d040396125: 93 xchg %eax,%ebx >> ffff82d040396126: cf iret >> ffff82d040396127: 00 ff add %bh,%bh >> ffff82d040396129: ff 00 incl (%rax) >> ffff82d04039612b: 00 00 add %al,(%rax) >> ffff82d04039612d: 9b fwait >> ffff82d04039612e: cf iret >> ... >> >> ffff82d040396130 <compat_mode_idt>: >> ... >> >> ffff82d0403961b6 <kexec_reloc_size>: >> ffff82d0403961b6: b6 01 mov $0x1,%dh >> ... >> >> to: >> >> ffff82d040396108 <compatibility_mode_far>: >> ffff82d040396108: 00 00 00 00 10 00 ...... >> >> ffff82d04039610e <compat_mode_gdt_desc>: >> ffff82d04039610e: 17 00 00 00 00 00 00 00 00 00 .......... >> >> ffff82d040396118 <compat_mode_gdt>: >> ... >> ffff82d040396120: ff ff 00 00 00 93 cf 00 ff ff 00 00 00 9b cf 00 ................ >> >> ffff82d040396130 <compat_mode_idt>: >> ffff82d040396130: 00 00 00 00 00 00 ...... > With the .size directives added, can we rely on consistent (past, > present, and future) objcopy behavior for padding gaps? Of course not. We don't know how things will develop in the future. The best we can do is hope that it doesn't change too much. But on that note, the way this would go wrong is the binary scan finding an endbr that wasn't disassembled properly here, for whatever reason. > It just so > happens that there's no 4-byte gap between compat_mode_gdt_desc and > compat_mode_gdt. Changing the .align ahead of compatibility_mode_far > would eliminate the risk of padding appearing if the code further up > changed. Gaps will be formed of long nops because we're in .text, and they merge with the previous data blob (see below). > >> ffff82d040396136 <reloc_stack>: >> ... > Now this is particularly puzzling: Us setting %rsp to an unaligned > address is clearly not ABI-conforming. Since you're fiddling with > all of this already anyway, how about fixing this at the same time? > Of course there would then appear padding ahead of the stack, unless > the stack was moved up some. Oh - I'd not even noticed that. Luckily there is no ABI which matters, because it's the call/push/pop's in this file alone. With an align 8, we get: ffff82d0403a7138 <compat_mode_idt>: ffff82d0403a7138: 00 00 00 00 00 00 66 90 ......f. ffff82d0403a7140 <reloc_stack>: ... where the 66 90 in compat_mode_idt is the padding. Recall c/s 9fd181540c7e6 > >> @@ -175,10 +175,16 @@ 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 */ >> >> + .type compat_mode_gdt_desc, @object >> + .size compat_mode_gdt_desc, . - compat_mode_gdt_desc > Side note: We really ought to gain something like OBJECT(name) to avoid > c'n'p mistakes not updating correctly all three symbol name instances. I've got an intern working on it. > >> --- a/xen/arch/x86/xen.lds.S >> +++ b/xen/arch/x86/xen.lds.S >> @@ -87,6 +87,7 @@ SECTIONS >> *(.text.unlikely) >> *(.fixup) >> *(.text.kexec) >> + kexec_reloc_end = .; > Does this maybe want aligning on a 4- or even 8-byte boundary? If > so, imo preferably not here, but by adding a trailing .align in the > .S file. There's no special need for it to be aligned, and it is anyway as the stack is the last object in it. The sole user is the memcpy() size calculation moving the trampoline to its destination page in the kexec reserved area. ~Andrew
On 17.02.2022 13:06, Andrew Cooper wrote: > On 17/02/2022 10:42, Jan Beulich wrote: >> On 17.02.2022 11:01, Andrew Cooper wrote: >>> Scanning for embedded endbranch instructions involves parsing the .text >>> disassembly. Data in the kexec trampoline has no ELF metadata, so objdump >>> treats it as instructions and tries to disassemble. Convert: >>> >>> ffff82d040396108 <compatibility_mode_far>: >> What about the (possible) padding ahead of this? Should the .align >> there perhaps specify a filler character? > > What about it? It's just long nops like all other padding in .text > > ffff82d040396101: ff d5 call *%ebp > ffff82d040396103: 0f 0b ud2 > ffff82d040396105: 0f 1f 00 nopl (%eax) > > ffff82d040396108 <compatibility_mode_far>: > ffff82d040396108: 00 00 00 00 10 > 00 ...... > > And for this problem, we don't need to care about the behaviour of any > pre-CET version of binutils. I was about to ask, but yes - this is a good point. Reviewed-by: Jan Beulich <jbeulich@suse.com> >>> ffff82d040396108: 00 00 add %al,(%rax) >>> ffff82d04039610a: 00 00 add %al,(%rax) >>> ffff82d04039610c: 10 00 adc %al,(%rax) >>> >>> ffff82d04039610e <compat_mode_gdt_desc>: >>> ffff82d04039610e: 17 (bad) >>> ... >>> >>> ffff82d040396118 <compat_mode_gdt>: >>> ... >>> ffff82d040396120: ff (bad) >>> ffff82d040396121: ff 00 incl (%rax) >>> ffff82d040396123: 00 00 add %al,(%rax) >>> ffff82d040396125: 93 xchg %eax,%ebx >>> ffff82d040396126: cf iret >>> ffff82d040396127: 00 ff add %bh,%bh >>> ffff82d040396129: ff 00 incl (%rax) >>> ffff82d04039612b: 00 00 add %al,(%rax) >>> ffff82d04039612d: 9b fwait >>> ffff82d04039612e: cf iret >>> ... >>> >>> ffff82d040396130 <compat_mode_idt>: >>> ... >>> >>> ffff82d0403961b6 <kexec_reloc_size>: >>> ffff82d0403961b6: b6 01 mov $0x1,%dh >>> ... >>> >>> to: >>> >>> ffff82d040396108 <compatibility_mode_far>: >>> ffff82d040396108: 00 00 00 00 10 00 ...... >>> >>> ffff82d04039610e <compat_mode_gdt_desc>: >>> ffff82d04039610e: 17 00 00 00 00 00 00 00 00 00 .......... >>> >>> ffff82d040396118 <compat_mode_gdt>: >>> ... >>> ffff82d040396120: ff ff 00 00 00 93 cf 00 ff ff 00 00 00 9b cf 00 ................ >>> >>> ffff82d040396130 <compat_mode_idt>: >>> ffff82d040396130: 00 00 00 00 00 00 ...... >> With the .size directives added, can we rely on consistent (past, >> present, and future) objcopy behavior for padding gaps? > > Of course not. We don't know how things will develop in the future. > The best we can do is hope that it doesn't change too much. > > But on that note, the way this would go wrong is the binary scan finding > an endbr that wasn't disassembled properly here, for whatever reason. True; it'll "just" be a false positive build failure. >> It just so >> happens that there's no 4-byte gap between compat_mode_gdt_desc and >> compat_mode_gdt. Changing the .align ahead of compatibility_mode_far >> would eliminate the risk of padding appearing if the code further up >> changed. > > Gaps will be formed of long nops because we're in .text, and they merge > with the previous data blob (see below). > >> >>> ffff82d040396136 <reloc_stack>: >>> ... >> Now this is particularly puzzling: Us setting %rsp to an unaligned >> address is clearly not ABI-conforming. Since you're fiddling with >> all of this already anyway, how about fixing this at the same time? >> Of course there would then appear padding ahead of the stack, unless >> the stack was moved up some. > > Oh - I'd not even noticed that. Luckily there is no ABI which matters, > because it's the call/push/pop's in this file alone. And the entity transitioned to is forbidden to make use of our stack? > With an align 8, we get: > > ffff82d0403a7138 <compat_mode_idt>: > ffff82d0403a7138: 00 00 00 00 00 00 66 > 90 ......f. > > ffff82d0403a7140 <reloc_stack>: > ... > > where the 66 90 in compat_mode_idt is the padding. Recall c/s 9fd181540c7e6 > >>> --- a/xen/arch/x86/xen.lds.S >>> +++ b/xen/arch/x86/xen.lds.S >>> @@ -87,6 +87,7 @@ SECTIONS >>> *(.text.unlikely) >>> *(.fixup) >>> *(.text.kexec) >>> + kexec_reloc_end = .; >> Does this maybe want aligning on a 4- or even 8-byte boundary? If >> so, imo preferably not here, but by adding a trailing .align in the >> .S file. > > There's no special need for it to be aligned, and it is anyway as the > stack is the last object in it. You mean it anyway would be, if the stack was aligned? Or am I to imply that you've amended the patch to add alignment there? Jan
On 17/02/2022 14:48, Jan Beulich wrote: > On 17.02.2022 13:06, Andrew Cooper wrote: >> On 17/02/2022 10:42, Jan Beulich wrote: >>> On 17.02.2022 11:01, Andrew Cooper wrote: >>>> Scanning for embedded endbranch instructions involves parsing the .text >>>> disassembly. Data in the kexec trampoline has no ELF metadata, so objdump >>>> treats it as instructions and tries to disassemble. Convert: >>>> >>>> ffff82d040396108 <compatibility_mode_far>: >>> What about the (possible) padding ahead of this? Should the .align >>> there perhaps specify a filler character? >> What about it? It's just long nops like all other padding in .text >> >> ffff82d040396101: ff d5 call *%ebp >> ffff82d040396103: 0f 0b ud2 >> ffff82d040396105: 0f 1f 00 nopl (%eax) >> >> ffff82d040396108 <compatibility_mode_far>: >> ffff82d040396108: 00 00 00 00 10 >> 00 ...... >> >> And for this problem, we don't need to care about the behaviour of any >> pre-CET version of binutils. > I was about to ask, but yes - this is a good point. > > Reviewed-by: Jan Beulich <jbeulich@suse.com> Thanks. >>> It just so >>> happens that there's no 4-byte gap between compat_mode_gdt_desc and >>> compat_mode_gdt. Changing the .align ahead of compatibility_mode_far >>> would eliminate the risk of padding appearing if the code further up >>> changed. >> Gaps will be formed of long nops because we're in .text, and they merge >> with the previous data blob (see below). >> >>>> ffff82d040396136 <reloc_stack>: >>>> ... >>> Now this is particularly puzzling: Us setting %rsp to an unaligned >>> address is clearly not ABI-conforming. Since you're fiddling with >>> all of this already anyway, how about fixing this at the same time? >>> Of course there would then appear padding ahead of the stack, unless >>> the stack was moved up some. >> Oh - I'd not even noticed that. Luckily there is no ABI which matters, >> because it's the call/push/pop's in this file alone. > And the entity transitioned to is forbidden to make use of our stack? There's no expectation/guarantee of a good stack, no. Purgatory is a very minimal environment before it sets something new up. >> With an align 8, we get: >> >> ffff82d0403a7138 <compat_mode_idt>: >> ffff82d0403a7138: 00 00 00 00 00 00 66 >> 90 ......f. >> >> ffff82d0403a7140 <reloc_stack>: >> ... >> >> where the 66 90 in compat_mode_idt is the padding. Recall c/s 9fd181540c7e6 >> >>>> --- a/xen/arch/x86/xen.lds.S >>>> +++ b/xen/arch/x86/xen.lds.S >>>> @@ -87,6 +87,7 @@ SECTIONS >>>> *(.text.unlikely) >>>> *(.fixup) >>>> *(.text.kexec) >>>> + kexec_reloc_end = .; >>> Does this maybe want aligning on a 4- or even 8-byte boundary? If >>> so, imo preferably not here, but by adding a trailing .align in the >>> .S file. >> There's no special need for it to be aligned, and it is anyway as the >> stack is the last object in it. > You mean it anyway would be, if the stack was aligned? Or am I to imply > that you've amended the patch to add alignment there? I have aligned reloc_stack stack because that's a no-brainer. With that suitably aligned, kexec_reloc_end becomes aligned naturally (because reloc_stack is the final object), and I don't think there's much point putting anything explicit in the linker script. It doesn't matter if subsequent things follow immediately, because this trampoline is copied into the kexec region before being used. In practice, the thing immediately following it is .init.text. ~Andrew
On 17.02.2022 17:06, Andrew Cooper wrote: > On 17/02/2022 14:48, Jan Beulich wrote: >> On 17.02.2022 13:06, Andrew Cooper wrote: >>> On 17/02/2022 10:42, Jan Beulich wrote: >>>> On 17.02.2022 11:01, Andrew Cooper wrote: >>>>> --- a/xen/arch/x86/xen.lds.S >>>>> +++ b/xen/arch/x86/xen.lds.S >>>>> @@ -87,6 +87,7 @@ SECTIONS >>>>> *(.text.unlikely) >>>>> *(.fixup) >>>>> *(.text.kexec) >>>>> + kexec_reloc_end = .; >>>> Does this maybe want aligning on a 4- or even 8-byte boundary? If >>>> so, imo preferably not here, but by adding a trailing .align in the >>>> .S file. >>> There's no special need for it to be aligned, and it is anyway as the >>> stack is the last object in it. >> You mean it anyway would be, if the stack was aligned? Or am I to imply >> that you've amended the patch to add alignment there? > > I have aligned reloc_stack stack because that's a no-brainer. With this ... > With that suitably aligned, kexec_reloc_end becomes aligned naturally > (because reloc_stack is the final object), and I don't think there's > much point putting anything explicit in the linker script. ... I certainly agree with this. Jan
© 2016 - 2024 Red Hat, Inc.