Re: [PATCH -next v2 1/2] riscv: kdump: Implement crashkernel=X,[high,low]
On 03/30/23 at 09:40pm, chenjiahao (C) wrote:
..
> Agreed, I will clean this up later in next version.
> > > + if (ret || !crash_size)
> > > + return;
> > > +
> > > + /*
> > > + * crashkernel=Y,low is valid only when crashkernel=X,high
> > > + * is passed and high memory is reserved successful.
> > > + */
> > > + ret = parse_crashkernel_low(boot_command_line, 0,
> > > _low_size, _base);
> > > + if (ret == -ENOENT)
> > > + crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
> > > + else if (ret)
> > > + return;
> > > +
> > > + search_start = dma32_phys_limit;
> > > + } else if (ret || !crash_size) {
> > > + /* Invalid argument value specified */
> > > return;
> > > + }
> > > crash_size = PAGE_ALIGN(crash_size);
> > > @@ -1201,16 +1246,26 @@ static void __init reserve_crashkernel(void)
> > >*/
> > > crash_base = memblock_phys_alloc_range(crash_size, PMD_SIZE,
> > > search_start,
> > > -min(search_end, (unsigned long)
> > > SZ_4G));
> > > +min(search_end, (unsigned
> > > long)dma32_phys_limit));
> > > if (crash_base == 0) {
> > The above conditional check isn't right. If crashkernel=size@offset
> > specified, the reservation failure won't trigger retry. This seems to be
> > originally introduced by old commit, while this need be fixed firstly.
>
> Just a little curious about the rule to cope with this specific case. If
> "crashkernel=size@offset" was passed
>
> but reserve failed, should try again to allocate in high memory, regardless
> the specified size@offset,
>
> or just throw a warning and return? Since I noticed the current logic here
> on Arm64 is to check if !fixed_base first
Yeah, we need mark the "crashkernel=size@offset" case and avoid to
retry. Because you won't succeed if memblock has already failed to
reserve an unavailable memory region, retry is meaningless. This has
been done in x86, arm64.
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v5 2/2] x86/purgatory: Add linker script
On Thu, Mar 30, 2023 at 11:31:27AM -0400, Steven Rostedt wrote: > On Thu, 30 Mar 2023 17:18:26 +0200 > Borislav Petkov wrote: > > > On Thu, Mar 30, 2023 at 11:15:23AM -0400, Steven Rostedt wrote: > > > > Make sure that the .text section is not divided in multiple overlapping > > > > sections. This is not supported by kexec_file. > > > > And? > > > > What is the failure scenario? Why are you fixing it? Why do we care? > > > > This is way too laconic. > > > > Yeah, I think the change log in patch 1 needs to be in this patch too, > which gives better context. Just read it. Why did it work with clang version < 16? + toolchains ML. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v5 2/2] x86/purgatory: Add linker script
On Thu, 30 Mar 2023 17:18:26 +0200 Borislav Petkov wrote: > On Thu, Mar 30, 2023 at 11:15:23AM -0400, Steven Rostedt wrote: > > > Make sure that the .text section is not divided in multiple overlapping > > > sections. This is not supported by kexec_file. > > And? > > What is the failure scenario? Why are you fixing it? Why do we care? > > This is way too laconic. > Yeah, I think the change log in patch 1 needs to be in this patch too, which gives better context. -- Steve ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v5 2/2] x86/purgatory: Add linker script
On Thu, Mar 30, 2023 at 11:15:23AM -0400, Steven Rostedt wrote: > > Make sure that the .text section is not divided in multiple overlapping > > sections. This is not supported by kexec_file. And? What is the failure scenario? Why are you fixing it? Why do we care? This is way too laconic. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v5 2/2] x86/purgatory: Add linker script
Hmm, this patch may need some more eyes. At least from the x86 maintainers.
-- Steve
On Thu, 30 Mar 2023 11:44:48 +0200
Ricardo Ribalda wrote:
> Make sure that the .text section is not divided in multiple overlapping
> sections. This is not supported by kexec_file.
>
> Signed-off-by: Ricardo Ribalda
> ---
> arch/x86/purgatory/.gitignore| 2 ++
> arch/x86/purgatory/Makefile | 20 +
> arch/x86/purgatory/kexec-purgatory.S | 2 +-
> arch/x86/purgatory/purgatory.lds.S | 57
>
> 4 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/purgatory/.gitignore b/arch/x86/purgatory/.gitignore
> index d2be1500671d..1fe71fe5945d 100644
> --- a/arch/x86/purgatory/.gitignore
> +++ b/arch/x86/purgatory/.gitignore
> @@ -1 +1,3 @@
> purgatory.chk
> +purgatory.lds
> +purgatory
> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> index 17f09dc26381..4dc96d409bec 100644
> --- a/arch/x86/purgatory/Makefile
> +++ b/arch/x86/purgatory/Makefile
> @@ -16,10 +16,11 @@ CFLAGS_sha256.o := -D__DISABLE_EXPORTS
>
> # When linking purgatory.ro with -r unresolved symbols are not checked,
> # also link a purgatory.chk binary without -r to check for unresolved
> symbols.
> -PURGATORY_LDFLAGS := -e purgatory_start -z nodefaultlib
> -LDFLAGS_purgatory.ro := -r $(PURGATORY_LDFLAGS)
> -LDFLAGS_purgatory.chk := $(PURGATORY_LDFLAGS)
> -targets += purgatory.ro purgatory.chk
> +PURGATORY_LDFLAGS := -nostdlib -z nodefaultlib
> +LDFLAGS_purgatory := -r $(PURGATORY_LDFLAGS) -T
> +LDFLAGS_purgatory.chk := -e purgatory_start $(PURGATORY_LDFLAGS)
> +
> +targets += purgatory.lds purgatory.ro purgatory.chk
>
> # Sanitizer, etc. runtimes are unavailable and cannot be linked here.
> GCOV_PROFILE := n
> @@ -72,10 +73,17 @@ CFLAGS_string.o += $(PURGATORY_CFLAGS)
> AFLAGS_REMOVE_setup-x86_$(BITS).o+= -Wa,-gdwarf-2
> AFLAGS_REMOVE_entry64.o += -Wa,-gdwarf-2
>
> -$(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
> +OBJCOPYFLAGS_purgatory.ro := -O elf64-x86-64
> +OBJCOPYFLAGS_purgatory.ro += --remove-section='*debug*'
> +OBJCOPYFLAGS_purgatory.ro += --remove-section='.comment'
> +OBJCOPYFLAGS_purgatory.ro += --remove-section='.note.*'
> +$(obj)/purgatory.ro: $(obj)/purgatory FORCE
> + $(call if_changed,objcopy)
> +
> +$(obj)/purgatory.chk: $(obj)/purgatory FORCE
> $(call if_changed,ld)
>
> -$(obj)/purgatory.chk: $(obj)/purgatory.ro FORCE
> +$(obj)/purgatory: $(obj)/purgatory.lds $(PURGATORY_OBJS) FORCE
> $(call if_changed,ld)
>
> $(obj)/kexec-purgatory.o: $(obj)/purgatory.ro $(obj)/purgatory.chk
> diff --git a/arch/x86/purgatory/kexec-purgatory.S
> b/arch/x86/purgatory/kexec-purgatory.S
> index 8530fe93b718..54b0d0b4dc42 100644
> --- a/arch/x86/purgatory/kexec-purgatory.S
> +++ b/arch/x86/purgatory/kexec-purgatory.S
> @@ -5,7 +5,7 @@
> .align 8
> kexec_purgatory:
> .globl kexec_purgatory
> - .incbin "arch/x86/purgatory/purgatory.ro"
> + .incbin "arch/x86/purgatory/purgatory"
> .Lkexec_purgatory_end:
>
> .align 8
> diff --git a/arch/x86/purgatory/purgatory.lds.S
> b/arch/x86/purgatory/purgatory.lds.S
> new file mode 100644
> index ..610da88aafa0
> --- /dev/null
> +++ b/arch/x86/purgatory/purgatory.lds.S
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include
> +
> +OUTPUT_FORMAT(CONFIG_OUTPUT_FORMAT)
> +
> +#undef i386
> +
> +#include
> +#include
> +
> +ENTRY(purgatory_start)
> +
> +SECTIONS
> +{
> + . = 0;
> + .head.text : {
> + _head = . ;
> + HEAD_TEXT
> + _ehead = . ;
> + }
> + .rodata : {
> + _rodata = . ;
> + *(.rodata) /* read-only data */
> + *(.rodata.*)
> + _erodata = . ;
> + }
> + .text : {
> + _text = .; /* Text */
> + *(.text)
> + *(.text.*)
> + *(.noinstr.text)
> + _etext = . ;
> + }
> + .data : {
> + _data = . ;
> + *(.data)
> + *(.data.*)
> + *(.bss.efistub)
> + _edata = . ;
> + }
> + . = ALIGN(L1_CACHE_BYTES);
> + .bss : {
> + _bss = . ;
> + *(.bss)
> + *(.bss.*)
> + *(COMMON)
> + . = ALIGN(8); /* For convenience during zeroing */
> + _ebss = .;
> + }
> +
> + /* Sections to be discarded */
> + /DISCARD/ : {
> + *(.eh_frame)
> + *(*__ksymtab*)
> + *(___kcrctab*)
> + }
> +}
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v5 1/2] kexec: Support purgatories with .text.hot sections
On Thu, 30 Mar 2023 11:44:47 +0200
Ricardo Ribalda wrote:
> Clang16 links the purgatory text in two sections:
>
> [ 1] .text PROGBITS 0040
>11a1 AX 0 0 16
> [ 2] .rela.textRELA 3498
>0648 0018 I 24 1 8
> ...
> [17] .text.hot.PROGBITS 3220
>020b AX 0 0 1
> [18] .rela.text.hot. RELA 4428
>0078 0018 I 2417 8
>
> And both of them have their range [sh_addr ... sh_addr+sh_size] on the
> area pointed by `e_entry`.
>
> This causes that image->start is calculated twice, once for .text and
> another time for .text.hot. The second calculation leaves image->start
> in a random location.
>
> Because of this, the system crashes immediately after:
>
> kexec_core: Starting new kernel
>
> Cc: sta...@vger.kernel.org
> Fixes: 930457057abe ("kernel/kexec_file.c: split up __kexec_load_puragory")
> Reviewed-by: Ross Zwisler
> Signed-off-by: Ricardo Ribalda
> ---
> kernel/kexec_file.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index f1a0e4e3fb5c..c7a0e51a6d87 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -901,10 +901,22 @@ static int kexec_purgatory_setup_sechdrs(struct
> purgatory_info *pi,
> }
>
> offset = ALIGN(offset, align);
> +
> + /*
> + * Check if the segment contains the entry point, if so,
> + * calculate the value of image->start based on it.
> + * If the compiler has produced more than one .text section
> + * (Eg: .text.hot), they are generally after the main .text
> + * section, and they shall not be used to calculate
> + * image->start. So do not re-calculate image->start if it
> + * is not set to the initial value, and warn the user so they
> + * have a chance to fix their purgatory's linker script.
> + */
> if (sechdrs[i].sh_flags & SHF_EXECINSTR &&
> pi->ehdr->e_entry >= sechdrs[i].sh_addr &&
> pi->ehdr->e_entry < (sechdrs[i].sh_addr
> - + sechdrs[i].sh_size)) {
> + + sechdrs[i].sh_size) &&
> + !WARN_ON(kbuf->image->start != pi->ehdr->e_entry)) {
> kbuf->image->start -= sechdrs[i].sh_addr;
> kbuf->image->start += kbuf->mem + offset;
> }
>
Reviewed-by: Steven Rostedt (Google)
Thanks Ricardo!
-- Steve
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH -next v2 1/2] riscv: kdump: Implement crashkernel=X,[high,low]
On 2023/3/29 19:19, Baoquan He wrote:
On 03/28/23 at 07:51pm, Chen Jiahao wrote:
Thanks for reviewing.
On riscv, the current crash kernel allocation logic is trying to
allocate within 32bit addressible memory region by default, if
failed, try to allocate without 4G restriction.
In need of saving DMA zone memory while allocating a relatively large
crash kernel region, allocating the reserved memory top down in
high memory, without overlapping the DMA zone, is a mature solution.
Here introduce the parameter option crashkernel=X,[high,low].
One can reserve the crash kernel from high memory above DMA zone range
by explicitly passing "crashkernel=X,high"; or reserve a memory range
below 4G with "crashkernel=X,low".
Signed-off-by: Chen Jiahao
---
arch/riscv/kernel/setup.c | 5
arch/riscv/mm/init.c | 63 ---
2 files changed, 64 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 5d3184cbf518..ea84e5047c23 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -176,6 +176,11 @@ static void __init init_resources(void)
if (ret < 0)
goto error;
}
+ if (crashk_low_res.start != crashk_low_res.end) {
+ ret = add_resource(_resource, _low_res);
+ if (ret < 0)
+ goto error;
+ }
#endif
#ifdef CONFIG_CRASH_DUMP
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 478d6763a01a..b7708cc467fa 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -1152,6 +1152,28 @@ static inline void setup_vm_final(void)
}
#endif /* CONFIG_MMU */
+/* Reserve 128M low memory by default for swiotlb buffer */
+#define DEFAULT_CRASH_KERNEL_LOW_SIZE (128UL << 20)
+
+static int __init reserve_crashkernel_low(unsigned long long low_size)
+{
+ unsigned long long low_base;
+
+ low_base = memblock_phys_alloc_range(low_size, PMD_SIZE, 0,
dma32_phys_limit);
+ if (!low_base) {
+ pr_err("cannot allocate crashkernel low memory
(size:0x%llx).\n", low_size);
+ return -ENOMEM;
+ }
+
+ pr_info("crashkernel low memory reserved: 0x%016llx - 0x%016llx (%lld
MB)\n",
+ low_base, low_base + low_size, low_size >> 20);
+
+ crashk_low_res.start = low_base;
+ crashk_low_res.end = low_base + low_size - 1;
+
+ return 0;
+}
+
/*
* reserve_crashkernel() - reserves memory for crash kernel
*
@@ -1163,6 +1185,7 @@ static void __init reserve_crashkernel(void)
{
unsigned long long crash_base = 0;
unsigned long long crash_size = 0;
+ unsigned long long crash_low_size = 0;
unsigned long search_start = memblock_start_of_DRAM();
unsigned long search_end = memblock_end_of_DRAM();
@@ -1182,8 +1205,30 @@ static void __init reserve_crashkernel(void)
ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
_size, _base);
- if (ret || !crash_size)
+ if (ret == -ENOENT) {
+ /*
+* crashkernel=X,[high,low] can be specified or not, but
+* invalid value is not allowed.
+*/
+ ret = parse_crashkernel_high(boot_command_line, 0, _size,
_base);
I would add a local variable to assign boot_command_line to it just like
arm64 does. Then these lines could be shorter.
char *cmdline = boot_command_line;
Agreed, I will clean this up later in next version.
+ if (ret || !crash_size)
+ return;
+
+ /*
+* crashkernel=Y,low is valid only when crashkernel=X,high
+* is passed and high memory is reserved successful.
+*/
+ ret = parse_crashkernel_low(boot_command_line, 0, _low_size,
_base);
+ if (ret == -ENOENT)
+ crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
+ else if (ret)
+ return;
+
+ search_start = dma32_phys_limit;
+ } else if (ret || !crash_size) {
+ /* Invalid argument value specified */
return;
+ }
crash_size = PAGE_ALIGN(crash_size);
@@ -1201,16 +1246,26 @@ static void __init reserve_crashkernel(void)
*/
crash_base = memblock_phys_alloc_range(crash_size, PMD_SIZE,
search_start,
- min(search_end, (unsigned long)
SZ_4G));
+ min(search_end, (unsigned
long)dma32_phys_limit));
if (crash_base == 0) {
The above conditional check isn't right. If crashkernel=size@offset
specified, the reservation failure won't trigger retry. This seems to be
originally introduced by old commit, while this need be fixed firstly.
Just a little curious about
Re: [PATCH v4 1/2] kexec: Support purgatories with .text.hot sections
Hi Simon
Thanks for your review!
On Thu, 30 Mar 2023 at 09:49, Simon Horman wrote:
>
> On Mon, Mar 27, 2023 at 05:06:53PM +0200, Ricardo Ribalda wrote:
> > Clang16 links the purgatory text in two sections:
> >
> > [ 1] .text PROGBITS 0040
> >11a1 AX 0 0 16
> > [ 2] .rela.textRELA 3498
> >0648 0018 I 24 1 8
> > ...
> > [17] .text.hot.PROGBITS 3220
> >020b AX 0 0 1
> > [18] .rela.text.hot. RELA 4428
> >0078 0018 I 2417 8
> >
> > And both of them have their range [sh_addr ... sh_addr+sh_size] on the
> > area pointed by `e_entry`.
> >
> > This causes that image->start is calculated twice, once for .text and
> > another time for .text.hot. The second calculation leaves image->start
> > in a random location.
> >
> > Because of this, the system crashes inmediatly after:
>
> s/inmediatly/immediately/
>
> >
> > kexec_core: Starting new kernel
> >
> > Cc: sta...@vger.kernel.org
>
> Maybe a fixes tag is warranted here.
>
> > Reviewed-by: Ross Zwisler
> > Signed-off-by: Ricardo Ribalda
> > ---
> > kernel/kexec_file.c | 13 -
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > index f1a0e4e3fb5c..25a37d8f113a 100644
> > --- a/kernel/kexec_file.c
> > +++ b/kernel/kexec_file.c
> > @@ -901,10 +901,21 @@ static int kexec_purgatory_setup_sechdrs(struct
> > purgatory_info *pi,
> > }
> >
> > offset = ALIGN(offset, align);
> > +
> > + /*
> > + * Check if the segment contains the entry point, if so,
> > + * calculate the value of image->start based on it.
> > + * If the compiler has produced more than one .text sections
>
> nit: s/sections/section/
>
> > + * (Eg: .text.hot), they are generally after the main .text
>
> If this is the general case, then are there cases where this doesn't hold?
When looking at this issue, I have only seen .text.hot after .text.
But I cannot warantee that future versions of llvm or gcc decide to
swap the order.
I am going to add a WARN whenever there are two overlapping .text
sections so the user has the chance to update their linker script.
>
> > + * section, and they shall not be used to calculate
> > + * image->start. So do not re-calculate image->start if it
> > + * is not set to the initial value.
> > + */
> > if (sechdrs[i].sh_flags & SHF_EXECINSTR &&
> > pi->ehdr->e_entry >= sechdrs[i].sh_addr &&
> > pi->ehdr->e_entry < (sechdrs[i].sh_addr
> > - + sechdrs[i].sh_size)) {
> > + + sechdrs[i].sh_size) &&
> > + kbuf->image->start == pi->ehdr->e_entry) {
> > kbuf->image->start -= sechdrs[i].sh_addr;
> > kbuf->image->start += kbuf->mem + offset;
> > }
> >
> > --
> > 2.40.0.348.gf938b09366-goog-b4-0.11.0-dev-696ae
> >
> > ___
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec
> >
--
Ricardo Ribalda
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v5 2/2] x86/purgatory: Add linker script
Make sure that the .text section is not divided in multiple overlapping
sections. This is not supported by kexec_file.
Signed-off-by: Ricardo Ribalda
---
arch/x86/purgatory/.gitignore| 2 ++
arch/x86/purgatory/Makefile | 20 +
arch/x86/purgatory/kexec-purgatory.S | 2 +-
arch/x86/purgatory/purgatory.lds.S | 57
4 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/arch/x86/purgatory/.gitignore b/arch/x86/purgatory/.gitignore
index d2be1500671d..1fe71fe5945d 100644
--- a/arch/x86/purgatory/.gitignore
+++ b/arch/x86/purgatory/.gitignore
@@ -1 +1,3 @@
purgatory.chk
+purgatory.lds
+purgatory
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 17f09dc26381..4dc96d409bec 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -16,10 +16,11 @@ CFLAGS_sha256.o := -D__DISABLE_EXPORTS
# When linking purgatory.ro with -r unresolved symbols are not checked,
# also link a purgatory.chk binary without -r to check for unresolved symbols.
-PURGATORY_LDFLAGS := -e purgatory_start -z nodefaultlib
-LDFLAGS_purgatory.ro := -r $(PURGATORY_LDFLAGS)
-LDFLAGS_purgatory.chk := $(PURGATORY_LDFLAGS)
-targets += purgatory.ro purgatory.chk
+PURGATORY_LDFLAGS := -nostdlib -z nodefaultlib
+LDFLAGS_purgatory := -r $(PURGATORY_LDFLAGS) -T
+LDFLAGS_purgatory.chk := -e purgatory_start $(PURGATORY_LDFLAGS)
+
+targets += purgatory.lds purgatory.ro purgatory.chk
# Sanitizer, etc. runtimes are unavailable and cannot be linked here.
GCOV_PROFILE := n
@@ -72,10 +73,17 @@ CFLAGS_string.o += $(PURGATORY_CFLAGS)
AFLAGS_REMOVE_setup-x86_$(BITS).o += -Wa,-gdwarf-2
AFLAGS_REMOVE_entry64.o+= -Wa,-gdwarf-2
-$(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
+OBJCOPYFLAGS_purgatory.ro := -O elf64-x86-64
+OBJCOPYFLAGS_purgatory.ro += --remove-section='*debug*'
+OBJCOPYFLAGS_purgatory.ro += --remove-section='.comment'
+OBJCOPYFLAGS_purgatory.ro += --remove-section='.note.*'
+$(obj)/purgatory.ro: $(obj)/purgatory FORCE
+ $(call if_changed,objcopy)
+
+$(obj)/purgatory.chk: $(obj)/purgatory FORCE
$(call if_changed,ld)
-$(obj)/purgatory.chk: $(obj)/purgatory.ro FORCE
+$(obj)/purgatory: $(obj)/purgatory.lds $(PURGATORY_OBJS) FORCE
$(call if_changed,ld)
$(obj)/kexec-purgatory.o: $(obj)/purgatory.ro $(obj)/purgatory.chk
diff --git a/arch/x86/purgatory/kexec-purgatory.S
b/arch/x86/purgatory/kexec-purgatory.S
index 8530fe93b718..54b0d0b4dc42 100644
--- a/arch/x86/purgatory/kexec-purgatory.S
+++ b/arch/x86/purgatory/kexec-purgatory.S
@@ -5,7 +5,7 @@
.align 8
kexec_purgatory:
.globl kexec_purgatory
- .incbin "arch/x86/purgatory/purgatory.ro"
+ .incbin "arch/x86/purgatory/purgatory"
.Lkexec_purgatory_end:
.align 8
diff --git a/arch/x86/purgatory/purgatory.lds.S
b/arch/x86/purgatory/purgatory.lds.S
new file mode 100644
index ..610da88aafa0
--- /dev/null
+++ b/arch/x86/purgatory/purgatory.lds.S
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include
+
+OUTPUT_FORMAT(CONFIG_OUTPUT_FORMAT)
+
+#undef i386
+
+#include
+#include
+
+ENTRY(purgatory_start)
+
+SECTIONS
+{
+ . = 0;
+ .head.text : {
+ _head = . ;
+ HEAD_TEXT
+ _ehead = . ;
+ }
+ .rodata : {
+ _rodata = . ;
+ *(.rodata) /* read-only data */
+ *(.rodata.*)
+ _erodata = . ;
+ }
+ .text : {
+ _text = .; /* Text */
+ *(.text)
+ *(.text.*)
+ *(.noinstr.text)
+ _etext = . ;
+ }
+ .data : {
+ _data = . ;
+ *(.data)
+ *(.data.*)
+ *(.bss.efistub)
+ _edata = . ;
+ }
+ . = ALIGN(L1_CACHE_BYTES);
+ .bss : {
+ _bss = . ;
+ *(.bss)
+ *(.bss.*)
+ *(COMMON)
+ . = ALIGN(8); /* For convenience during zeroing */
+ _ebss = .;
+ }
+
+ /* Sections to be discarded */
+ /DISCARD/ : {
+ *(.eh_frame)
+ *(*__ksymtab*)
+ *(___kcrctab*)
+ }
+}
--
2.40.0.348.gf938b09366-goog
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v5 0/2] kexec: Fix kexec_file_load for llvm16
When upreving llvm I realised that kexec stopped working on my test platform. This patch fixes it. Signed-off-by: Ricardo Ribalda --- Changes in v5: - Add warning when multiple text sections are found. Thanks Simon! - Add Fixes tag. - Link to v4: https://lore.kernel.org/r/20230321-kexec_clang16-v4-0-1340518f9...@chromium.org Changes in v4: - Add Cc: stable - Add linker script for x86 - Add a warning when the kernel image has overlapping sections. - Link to v3: https://lore.kernel.org/r/20230321-kexec_clang16-v3-0-5f016c8d0...@chromium.org Changes in v3: - Fix initial value. Thanks Ross! - Link to v2: https://lore.kernel.org/r/20230321-kexec_clang16-v2-0-d10e5d517...@chromium.org Changes in v2: - Fix if condition. Thanks Steven!. - Update Philipp email. Thanks Baoquan. - Link to v1: https://lore.kernel.org/r/20230321-kexec_clang16-v1-0-a768fc2c7...@chromium.org --- Ricardo Ribalda (2): kexec: Support purgatories with .text.hot sections x86/purgatory: Add linker script arch/x86/purgatory/.gitignore| 2 ++ arch/x86/purgatory/Makefile | 20 + arch/x86/purgatory/kexec-purgatory.S | 2 +- arch/x86/purgatory/purgatory.lds.S | 57 kernel/kexec_file.c | 14 - 5 files changed, 87 insertions(+), 8 deletions(-) --- base-commit: 17214b70a159c6547df9ae204a6275d983146f6b change-id: 20230321-kexec_clang16-4510c23d129c Best regards, -- Ricardo Ribalda ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v5 1/2] kexec: Support purgatories with .text.hot sections
Clang16 links the purgatory text in two sections:
[ 1] .text PROGBITS 0040
11a1 AX 0 0 16
[ 2] .rela.textRELA 3498
0648 0018 I 24 1 8
...
[17] .text.hot.PROGBITS 3220
020b AX 0 0 1
[18] .rela.text.hot. RELA 4428
0078 0018 I 2417 8
And both of them have their range [sh_addr ... sh_addr+sh_size] on the
area pointed by `e_entry`.
This causes that image->start is calculated twice, once for .text and
another time for .text.hot. The second calculation leaves image->start
in a random location.
Because of this, the system crashes immediately after:
kexec_core: Starting new kernel
Cc: sta...@vger.kernel.org
Fixes: 930457057abe ("kernel/kexec_file.c: split up __kexec_load_puragory")
Reviewed-by: Ross Zwisler
Signed-off-by: Ricardo Ribalda
---
kernel/kexec_file.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f1a0e4e3fb5c..c7a0e51a6d87 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -901,10 +901,22 @@ static int kexec_purgatory_setup_sechdrs(struct
purgatory_info *pi,
}
offset = ALIGN(offset, align);
+
+ /*
+* Check if the segment contains the entry point, if so,
+* calculate the value of image->start based on it.
+* If the compiler has produced more than one .text section
+* (Eg: .text.hot), they are generally after the main .text
+* section, and they shall not be used to calculate
+* image->start. So do not re-calculate image->start if it
+* is not set to the initial value, and warn the user so they
+* have a chance to fix their purgatory's linker script.
+*/
if (sechdrs[i].sh_flags & SHF_EXECINSTR &&
pi->ehdr->e_entry >= sechdrs[i].sh_addr &&
pi->ehdr->e_entry < (sechdrs[i].sh_addr
-+ sechdrs[i].sh_size)) {
++ sechdrs[i].sh_size) &&
+ !WARN_ON(kbuf->image->start != pi->ehdr->e_entry)) {
kbuf->image->start -= sechdrs[i].sh_addr;
kbuf->image->start += kbuf->mem + offset;
}
--
2.40.0.348.gf938b09366-goog
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 1/2] kexec: Support purgatories with .text.hot sections
On Mon, Mar 27, 2023 at 05:06:53PM +0200, Ricardo Ribalda wrote:
> Clang16 links the purgatory text in two sections:
>
> [ 1] .text PROGBITS 0040
>11a1 AX 0 0 16
> [ 2] .rela.textRELA 3498
>0648 0018 I 24 1 8
> ...
> [17] .text.hot.PROGBITS 3220
>020b AX 0 0 1
> [18] .rela.text.hot. RELA 4428
>0078 0018 I 2417 8
>
> And both of them have their range [sh_addr ... sh_addr+sh_size] on the
> area pointed by `e_entry`.
>
> This causes that image->start is calculated twice, once for .text and
> another time for .text.hot. The second calculation leaves image->start
> in a random location.
>
> Because of this, the system crashes inmediatly after:
s/inmediatly/immediately/
>
> kexec_core: Starting new kernel
>
> Cc: sta...@vger.kernel.org
Maybe a fixes tag is warranted here.
> Reviewed-by: Ross Zwisler
> Signed-off-by: Ricardo Ribalda
> ---
> kernel/kexec_file.c | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index f1a0e4e3fb5c..25a37d8f113a 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -901,10 +901,21 @@ static int kexec_purgatory_setup_sechdrs(struct
> purgatory_info *pi,
> }
>
> offset = ALIGN(offset, align);
> +
> + /*
> + * Check if the segment contains the entry point, if so,
> + * calculate the value of image->start based on it.
> + * If the compiler has produced more than one .text sections
nit: s/sections/section/
> + * (Eg: .text.hot), they are generally after the main .text
If this is the general case, then are there cases where this doesn't hold?
> + * section, and they shall not be used to calculate
> + * image->start. So do not re-calculate image->start if it
> + * is not set to the initial value.
> + */
> if (sechdrs[i].sh_flags & SHF_EXECINSTR &&
> pi->ehdr->e_entry >= sechdrs[i].sh_addr &&
> pi->ehdr->e_entry < (sechdrs[i].sh_addr
> - + sechdrs[i].sh_size)) {
> + + sechdrs[i].sh_size) &&
> + kbuf->image->start == pi->ehdr->e_entry) {
> kbuf->image->start -= sechdrs[i].sh_addr;
> kbuf->image->start += kbuf->mem + offset;
> }
>
> --
> 2.40.0.348.gf938b09366-goog-b4-0.11.0-dev-696ae
>
> ___
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
