2021-04-19 00:56:45

by Nick Kossifidis

[permalink] [raw]
Subject: [PATCH v4 5/5] RISC-V: Add crash kernel support

This patch allows Linux to act as a crash kernel for use with
kdump. Userspace will let the crash kernel know about the
memory region it can use through linux,usable-memory property
on the /memory node (overriding its reg property), and about the
memory region where the elf core header of the previous kernel
is saved, through a reserved-memory node with a compatible string
of "linux,elfcorehdr". This approach is the least invasive and
re-uses functionality already present.

I tested this on riscv64 qemu and it works as expected, you
may test it by retrieving the dmesg of the previous kernel
through /proc/vmcore, using the vmcore-dmesg utility from
kexec-tools.

v4:
* Rebase on top of "fixes" branch

v3:
* Rebase

v2:
* Use linux,usable-memory on /memory instead of a new binding
* Use a reserved-memory node for ELF core header

Signed-off-by: Nick Kossifidis <[email protected]>
---
arch/riscv/Kconfig | 10 ++++++++
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/crash_dump.c | 46 ++++++++++++++++++++++++++++++++++
arch/riscv/kernel/setup.c | 12 +++++++++
arch/riscv/mm/init.c | 33 ++++++++++++++++++++++++
5 files changed, 102 insertions(+)
create mode 100644 arch/riscv/kernel/crash_dump.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 10cc19be0..39aa18ef4 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -400,6 +400,16 @@ config KEXEC

The name comes from the similarity to the exec system call.

+config CRASH_DUMP
+ bool "Build kdump crash kernel"
+ help
+ Generate crash dump after being started by kexec. This should
+ be normally only set in special crash dump kernels which are
+ loaded in the main kernel with kexec-tools into a specially
+ reserved region and then later executed after a crash by
+ kdump/kexec.
+
+ For more details see Documentation/admin-guide/kdump/kdump.rst

endmenu

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 41a1469b2..d3081e4d9 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -60,6 +60,7 @@ endif
obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o
obj-$(CONFIG_KGDB) += kgdb.o
obj-$(CONFIG_KEXEC) += kexec_relocate.o crash_save_regs.o machine_kexec.o
+obj-$(CONFIG_CRASH_DUMP) += crash_dump.o

obj-$(CONFIG_JUMP_LABEL) += jump_label.o

diff --git a/arch/riscv/kernel/crash_dump.c b/arch/riscv/kernel/crash_dump.c
new file mode 100644
index 000000000..86cc0ada5
--- /dev/null
+++ b/arch/riscv/kernel/crash_dump.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This code comes from arch/arm64/kernel/crash_dump.c
+ * Created by: AKASHI Takahiro <[email protected]>
+ * Copyright (C) 2017 Linaro Limited
+ */
+
+#include <linux/crash_dump.h>
+#include <linux/io.h>
+
+/**
+ * copy_oldmem_page() - copy one page from old kernel memory
+ * @pfn: page frame number to be copied
+ * @buf: buffer where the copied page is placed
+ * @csize: number of bytes to copy
+ * @offset: offset in bytes into the page
+ * @userbuf: if set, @buf is in a user address space
+ *
+ * This function copies one page from old kernel memory into buffer pointed by
+ * @buf. If @buf is in userspace, set @userbuf to %1. Returns number of bytes
+ * copied or negative error in case of failure.
+ */
+ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
+ size_t csize, unsigned long offset,
+ int userbuf)
+{
+ void *vaddr;
+
+ if (!csize)
+ return 0;
+
+ vaddr = memremap(__pfn_to_phys(pfn), PAGE_SIZE, MEMREMAP_WB);
+ if (!vaddr)
+ return -ENOMEM;
+
+ if (userbuf) {
+ if (copy_to_user((char __user *)buf, vaddr + offset, csize)) {
+ memunmap(vaddr);
+ return -EFAULT;
+ }
+ } else
+ memcpy(buf, vaddr + offset, csize);
+
+ memunmap(vaddr);
+ return csize;
+}
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 31866dce9..ff398a3d8 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -66,6 +66,9 @@ static struct resource code_res = { .name = "Kernel code", };
static struct resource data_res = { .name = "Kernel data", };
static struct resource rodata_res = { .name = "Kernel rodata", };
static struct resource bss_res = { .name = "Kernel bss", };
+#ifdef CONFIG_CRASH_DUMP
+static struct resource elfcorehdr_res = { .name = "ELF Core hdr", };
+#endif

static int __init add_resource(struct resource *parent,
struct resource *res)
@@ -169,6 +172,15 @@ static void __init init_resources(void)
}
#endif

+#ifdef CONFIG_CRASH_DUMP
+ if (elfcorehdr_size > 0) {
+ elfcorehdr_res.start = elfcorehdr_addr;
+ elfcorehdr_res.end = elfcorehdr_addr + elfcorehdr_size - 1;
+ elfcorehdr_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
+ add_resource(&iomem_resource, &elfcorehdr_res);
+ }
+#endif
+
for_each_reserved_mem_region(region) {
res = &mem_res[res_idx--];

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 8b2b85a57..9c048fccb 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -13,6 +13,7 @@
#include <linux/swap.h>
#include <linux/sizes.h>
#include <linux/of_fdt.h>
+#include <linux/of_reserved_mem.h>
#include <linux/libfdt.h>
#include <linux/set_memory.h>
#include <linux/dma-map-ops.h>
@@ -605,6 +606,18 @@ static void __init reserve_crashkernel(void)

int ret = 0;

+ /*
+ * Don't reserve a region for a crash kernel on a crash kernel
+ * since it doesn't make much sense and we have limited memory
+ * resources.
+ */
+#ifdef CONFIG_CRASH_DUMP
+ if (is_kdump_kernel()) {
+ pr_info("crashkernel: ignoring reservation request\n");
+ return;
+ }
+#endif
+
ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
&crash_size, &crash_base);
if (ret || !crash_size)
@@ -653,6 +666,26 @@ static void __init reserve_crashkernel(void)
}
#endif /* CONFIG_KEXEC_CORE */

+#ifdef CONFIG_CRASH_DUMP
+/*
+ * We keep track of the ELF core header of the crashed
+ * kernel with a reserved-memory region with compatible
+ * string "linux,elfcorehdr". Here we register a callback
+ * to populate elfcorehdr_addr/size when this region is
+ * present. Note that this region will be marked as
+ * reserved once we call early_init_fdt_scan_reserved_mem()
+ * later on.
+ */
+static int elfcore_hdr_setup(struct reserved_mem *rmem)
+{
+ elfcorehdr_addr = rmem->base;
+ elfcorehdr_size = rmem->size;
+ return 0;
+}
+
+RESERVEDMEM_OF_DECLARE(elfcorehdr, "linux,elfcorehdr", elfcore_hdr_setup);
+#endif
+
void __init paging_init(void)
{
setup_vm_final();
--
2.26.2


2021-06-15 13:23:51

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

Hi Nick,

CC rob, dt

Thanks for your patch, which is now commit 5640975003d0234d ("RISC-V:
Add crash kernel support") in v5.13-rc1.

On Mon, Apr 19, 2021 at 2:56 AM Nick Kossifidis <[email protected]> wrote:
> This patch allows Linux to act as a crash kernel for use with
> kdump. Userspace will let the crash kernel know about the
> memory region it can use through linux,usable-memory property
> on the /memory node (overriding its reg property), and about the
> memory region where the elf core header of the previous kernel
> is saved, through a reserved-memory node with a compatible string
> of "linux,elfcorehdr". This approach is the least invasive and
> re-uses functionality already present.

This does not match
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:

$ref: types.yaml#/definitions/uint64-array
maxItems: 2
description:
This property (currently used only on arm64) holds the memory range,
the address and the size, of the elf core header which mainly describes
the panicked kernel\'s memory layout as PT_LOAD segments of elf format.

Hence "linux,elfcorehdr" should be a property of the /chosen node,
instead of a memory node with a compatible value of "linux,elfcorehdr".

> I tested this on riscv64 qemu and it works as expected, you
> may test it by retrieving the dmesg of the previous kernel
> through /proc/vmcore, using the vmcore-dmesg utility from
> kexec-tools.
>
> v4:
> * Rebase on top of "fixes" branch
>
> v3:
> * Rebase
>
> v2:
> * Use linux,usable-memory on /memory instead of a new binding

This part seems to have been removed in v3 and later?
Note that "linux,usable-memory-range" should be a property of the
/chosen node, too, cfr.
https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L85

> * Use a reserved-memory node for ELF core header
>
> Signed-off-by: Nick Kossifidis <[email protected]>


> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -653,6 +666,26 @@ static void __init reserve_crashkernel(void)
> }
> #endif /* CONFIG_KEXEC_CORE */
>
> +#ifdef CONFIG_CRASH_DUMP
> +/*
> + * We keep track of the ELF core header of the crashed
> + * kernel with a reserved-memory region with compatible
> + * string "linux,elfcorehdr". Here we register a callback
> + * to populate elfcorehdr_addr/size when this region is
> + * present. Note that this region will be marked as
> + * reserved once we call early_init_fdt_scan_reserved_mem()
> + * later on.
> + */
> +static int elfcore_hdr_setup(struct reserved_mem *rmem)
> +{
> + elfcorehdr_addr = rmem->base;
> + elfcorehdr_size = rmem->size;
> + return 0;
> +}
> +
> +RESERVEDMEM_OF_DECLARE(elfcorehdr, "linux,elfcorehdr", elfcore_hdr_setup);
> +#endif
> +
> void __init paging_init(void)
> {
> setup_vm_final();
> --
> 2.26.2

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-06-15 18:30:46

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

Hello Geert,

Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:
>
> This does not match
> https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:
>
> $ref: types.yaml#/definitions/uint64-array
> maxItems: 2
> description:
> This property (currently used only on arm64) holds the memory
> range,
> the address and the size, of the elf core header which mainly
> describes
> the panicked kernel\'s memory layout as PT_LOAD segments of elf
> format.
>
> Hence "linux,elfcorehdr" should be a property of the /chosen node,
> instead of a memory node with a compatible value of "linux,elfcorehdr".
>

That's a binding for a property on the /chosen node, that as the text
says it's defined for arm64 only and the code that handled it was also
on arm64. Instead the reserved-region binding I used is a standard
binding, if you don't like the name used for the compatible string
because it overlaps with that property we can change it. I want to use a
reserved-region for this because we'll have to reserve it anyway so
using a property on /chosen and then using that property to reserve the
region seemed suboptimal.

>> v2:
>> * Use linux,usable-memory on /memory instead of a new binding
>
> This part seems to have been removed in v3 and later?
> Note that "linux,usable-memory-range" should be a property of the
> /chosen node, too, cfr.
> https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L85
>

No special handling is needed when using linux,usable-memory on /memory,
limiting the available memory is handled by generic code at
drivers/of/fdt.c

Regards,
Nick

2021-06-15 18:49:08

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

Hi Nick,

On Tue, Jun 15, 2021 at 8:29 PM Nick Kossifidis <[email protected]> wrote:
> Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:
> > This does not match
> > https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:
> >
> > $ref: types.yaml#/definitions/uint64-array
> > maxItems: 2
> > description:
> > This property (currently used only on arm64) holds the memory
> > range,
> > the address and the size, of the elf core header which mainly
> > describes
> > the panicked kernel\'s memory layout as PT_LOAD segments of elf
> > format.
> >
> > Hence "linux,elfcorehdr" should be a property of the /chosen node,
> > instead of a memory node with a compatible value of "linux,elfcorehdr".
> >
>
> That's a binding for a property on the /chosen node, that as the text
> says it's defined for arm64 only and the code that handled it was also

That doesn't mean it must not be used on other architectures ;-)
Arm64 was just the first one to use it...

> on arm64. Instead the reserved-region binding I used is a standard
> binding, if you don't like the name used for the compatible string
> because it overlaps with that property we can change it. I want to use a
> reserved-region for this because we'll have to reserve it anyway so
> using a property on /chosen and then using that property to reserve the
> region seemed suboptimal.
>
> >> v2:
> >> * Use linux,usable-memory on /memory instead of a new binding
> >
> > This part seems to have been removed in v3 and later?
> > Note that "linux,usable-memory-range" should be a property of the
> > /chosen node, too, cfr.
> > https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L85
> >
>
> No special handling is needed when using linux,usable-memory on /memory,
> limiting the available memory is handled by generic code at
> drivers/of/fdt.c

It was my understanding both properties under /chosen are the
recommended methods for new platforms... Let's see what Rob has
to say...

Anyway, I sent a patch series to switch to generic "linux,elfcorehdr"
handling
https://lore.kernel.org/r/[email protected]/

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-06-15 19:23:05

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

On Tue, Jun 15, 2021 at 12:48 PM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Nick,
>
> On Tue, Jun 15, 2021 at 8:29 PM Nick Kossifidis <[email protected]> wrote:
> > Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:
> > > This does not match
> > > https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:
> > >
> > > $ref: types.yaml#/definitions/uint64-array
> > > maxItems: 2
> > > description:
> > > This property (currently used only on arm64) holds the memory
> > > range,
> > > the address and the size, of the elf core header which mainly
> > > describes
> > > the panicked kernel\'s memory layout as PT_LOAD segments of elf
> > > format.
> > >
> > > Hence "linux,elfcorehdr" should be a property of the /chosen node,
> > > instead of a memory node with a compatible value of "linux,elfcorehdr".
> > >
> >
> > That's a binding for a property on the /chosen node, that as the text
> > says it's defined for arm64 only and the code that handled it was also
>
> That doesn't mean it must not be used on other architectures ;-)
> Arm64 was just the first one to use it...

It is used on arm64 because memory is often passed by UEFI tables and
not with /memory node. As riscv is also supporting EFI, I'd think they
would do the same.

> > on arm64. Instead the reserved-region binding I used is a standard
> > binding, if you don't like the name used for the compatible string
> > because it overlaps with that property we can change it. I want to use a
> > reserved-region for this because we'll have to reserve it anyway so
> > using a property on /chosen and then using that property to reserve the
> > region seemed suboptimal.
> >
> > >> v2:
> > >> * Use linux,usable-memory on /memory instead of a new binding
> > >
> > > This part seems to have been removed in v3 and later?
> > > Note that "linux,usable-memory-range" should be a property of the
> > > /chosen node, too, cfr.
> > > https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L85
> > >
> >
> > No special handling is needed when using linux,usable-memory on /memory,
> > limiting the available memory is handled by generic code at
> > drivers/of/fdt.c
>
> It was my understanding both properties under /chosen are the
> recommended methods for new platforms... Let's see what Rob has
> to say...
>
> Anyway, I sent a patch series to switch to generic "linux,elfcorehdr"
> handling
> https://lore.kernel.org/r/[email protected]/
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds

2021-06-15 23:31:02

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

Στις 2021-06-15 22:21, Rob Herring έγραψε:
> On Tue, Jun 15, 2021 at 12:48 PM Geert Uytterhoeven
> <[email protected]> wrote:
>>
>> Hi Nick,
>>
>> On Tue, Jun 15, 2021 at 8:29 PM Nick Kossifidis <[email protected]>
>> wrote:
>> > Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:
>> > > This does not match
>> > > https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:
>> > >
>> > > $ref: types.yaml#/definitions/uint64-array
>> > > maxItems: 2
>> > > description:
>> > > This property (currently used only on arm64) holds the memory
>> > > range,
>> > > the address and the size, of the elf core header which mainly
>> > > describes
>> > > the panicked kernel\'s memory layout as PT_LOAD segments of elf
>> > > format.
>> > >
>> > > Hence "linux,elfcorehdr" should be a property of the /chosen node,
>> > > instead of a memory node with a compatible value of "linux,elfcorehdr".
>> > >
>> >
>> > That's a binding for a property on the /chosen node, that as the text
>> > says it's defined for arm64 only and the code that handled it was also
>>
>> That doesn't mean it must not be used on other architectures ;-)
>> Arm64 was just the first one to use it...
>
> It is used on arm64 because memory is often passed by UEFI tables and
> not with /memory node. As riscv is also supporting EFI, I'd think they
> would do the same.
>

We've had this discussion before, riscv uses /memory for now and even if
we switched to getting memory from ACPI/UEFI tables, the elf core header
is passed from the crashed kernel to the kdump kernel, it has nothing to
do with UEFI since the bootloader is the kernel itself. Am I missing
something ?

Regards,
Nick

2021-06-16 19:13:12

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

+Ard

On Tue, Jun 15, 2021 at 5:29 PM Nick Kossifidis <[email protected]> wrote:
>
> Στις 2021-06-15 22:21, Rob Herring έγραψε:
> > On Tue, Jun 15, 2021 at 12:48 PM Geert Uytterhoeven
> > <[email protected]> wrote:
> >>
> >> Hi Nick,
> >>
> >> On Tue, Jun 15, 2021 at 8:29 PM Nick Kossifidis <[email protected]>
> >> wrote:
> >> > Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:
> >> > > This does not match
> >> > > https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:
> >> > >
> >> > > $ref: types.yaml#/definitions/uint64-array
> >> > > maxItems: 2
> >> > > description:
> >> > > This property (currently used only on arm64) holds the memory
> >> > > range,
> >> > > the address and the size, of the elf core header which mainly
> >> > > describes
> >> > > the panicked kernel\'s memory layout as PT_LOAD segments of elf
> >> > > format.
> >> > >
> >> > > Hence "linux,elfcorehdr" should be a property of the /chosen node,
> >> > > instead of a memory node with a compatible value of "linux,elfcorehdr".
> >> > >
> >> >
> >> > That's a binding for a property on the /chosen node, that as the text
> >> > says it's defined for arm64 only and the code that handled it was also
> >>
> >> That doesn't mean it must not be used on other architectures ;-)
> >> Arm64 was just the first one to use it...
> >
> > It is used on arm64 because memory is often passed by UEFI tables and
> > not with /memory node. As riscv is also supporting EFI, I'd think they
> > would do the same.
> >
>
> We've had this discussion before, riscv uses /memory for now and even if
> we switched to getting memory from ACPI/UEFI tables, the elf core header
> is passed from the crashed kernel to the kdump kernel, it has nothing to
> do with UEFI since the bootloader is the kernel itself. Am I missing
> something ?

I believe if we originally booted using UEFI tables, then those are
passed the kdump kernel as well. The original DT may have had a
/memory node, but it's possible it didn't match what was in the UEFI
tables. So using the DT /memory nodes for kdump could give surprising
results. I think reserved regions also come from UEFI. Ard can
probably comment better.

Rob

2021-06-16 23:56:00

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

On Wed, 16 Jun 2021 at 16:55, Rob Herring <[email protected]> wrote:
>
> +Ard
>
> On Tue, Jun 15, 2021 at 5:29 PM Nick Kossifidis <[email protected]> wrote:
> >
> > Στις 2021-06-15 22:21, Rob Herring έγραψε:
> > > On Tue, Jun 15, 2021 at 12:48 PM Geert Uytterhoeven
> > > <[email protected]> wrote:
> > >>
> > >> Hi Nick,
> > >>
> > >> On Tue, Jun 15, 2021 at 8:29 PM Nick Kossifidis <[email protected]>
> > >> wrote:
> > >> > Στις 2021-06-15 16:19, Geert Uytterhoeven έγραψε:
> > >> > > This does not match
> > >> > > https://github.com/devicetree-org/dt-schema/blob/master/schemas/chosen.yaml#L77:
> > >> > >
> > >> > > $ref: types.yaml#/definitions/uint64-array
> > >> > > maxItems: 2
> > >> > > description:
> > >> > > This property (currently used only on arm64) holds the memory
> > >> > > range,
> > >> > > the address and the size, of the elf core header which mainly
> > >> > > describes
> > >> > > the panicked kernel\'s memory layout as PT_LOAD segments of elf
> > >> > > format.
> > >> > >
> > >> > > Hence "linux,elfcorehdr" should be a property of the /chosen node,
> > >> > > instead of a memory node with a compatible value of "linux,elfcorehdr".
> > >> > >
> > >> >
> > >> > That's a binding for a property on the /chosen node, that as the text
> > >> > says it's defined for arm64 only and the code that handled it was also
> > >>
> > >> That doesn't mean it must not be used on other architectures ;-)
> > >> Arm64 was just the first one to use it...
> > >
> > > It is used on arm64 because memory is often passed by UEFI tables and
> > > not with /memory node. As riscv is also supporting EFI, I'd think they
> > > would do the same.
> > >
> >
> > We've had this discussion before, riscv uses /memory for now and even if
> > we switched to getting memory from ACPI/UEFI tables, the elf core header
> > is passed from the crashed kernel to the kdump kernel, it has nothing to
> > do with UEFI since the bootloader is the kernel itself. Am I missing
> > something ?
>
> I believe if we originally booted using UEFI tables, then those are
> passed the kdump kernel as well. The original DT may have had a
> /memory node, but it's possible it didn't match what was in the UEFI
> tables. So using the DT /memory nodes for kdump could give surprising
> results. I think reserved regions also come from UEFI. Ard can
> probably comment better.
>

Anything that executes in the context of the UEFI boot firmware
(loaders, drivers, etc) may use the UEFI memory allocation routines to
allocate memory, and these allocations are communicated via the UEFI
memory map, not via the /memory node.

So it depends whether it matters if the kexec kernel tramples over
those regions. For kdump scenarios, it might be reasonable, but in the
general case, we should really respect what UEFI tells us about the
memory map when booting via UEFI.

2023-10-30 10:45:06

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] RISC-V: Add crash kernel support

On Apr 19 2021, Nick Kossifidis wrote:

> @@ -605,6 +606,18 @@ static void __init reserve_crashkernel(void)
>
> int ret = 0;
>
> + /*
> + * Don't reserve a region for a crash kernel on a crash kernel
> + * since it doesn't make much sense and we have limited memory
> + * resources.
> + */
> +#ifdef CONFIG_CRASH_DUMP
> + if (is_kdump_kernel()) {
> + pr_info("crashkernel: ignoring reservation request\n");
> + return;
> + }
> +#endif
> +

Why is this necessary? It breaks
https://github.com/openSUSE/kdump/tree/master/calibrate which wants to
run the kernel with both elfcorehdr and crashkernel.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."