diff mbox series

[1/3] of: fdt: Add generic support for parsing elf core header properties

Message ID 8661d652968d050489852ccb50df5ad4cc048900.1623780059.git.geert+renesas@glider.be (mailing list archive)
State New, archived
Headers show
Series Add generic-support for linux,elfcorehdr and fix riscv | expand

Commit Message

Geert Uytterhoeven June 15, 2021, 6:17 p.m. UTC
There are two methods to specify the location of the elf core header:
using the "elfcorehdr=" kernel parameter, as handled by generic code in
kernel/crash_dump.c, or using the "linux,elfcorehdr" property under the
"/chosen" node in the Device Tree, as handled by architecture-specific
code in arch/arm64/mm/init.c.

Extend support for "linux,elfcorehdr" to all platforms supporting DT by
adding platform-agnostic handling for parsing this property to the FDT
core code.  This can co-exist safely with the architecture-specific
handling, until the latter has been removed.

This requires moving the call to of_scan_flat_dt() up, as the code
scanning the "/chosen" node now needs to be aware of the values of
"#address-cells" and "#size-cells".

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/devicetree/bindings/chosen.txt |  6 ++--
 drivers/of/fdt.c                             | 37 ++++++++++++++++++--
 2 files changed, 37 insertions(+), 6 deletions(-)

Comments

Rob Herring June 15, 2021, 7:54 p.m. UTC | #1
On Tue, Jun 15, 2021 at 12:17 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> There are two methods to specify the location of the elf core header:
> using the "elfcorehdr=" kernel parameter, as handled by generic code in
> kernel/crash_dump.c, or using the "linux,elfcorehdr" property under the
> "/chosen" node in the Device Tree, as handled by architecture-specific
> code in arch/arm64/mm/init.c.
>
> Extend support for "linux,elfcorehdr" to all platforms supporting DT by
> adding platform-agnostic handling for parsing this property to the FDT
> core code.  This can co-exist safely with the architecture-specific
> handling, until the latter has been removed.
>
> This requires moving the call to of_scan_flat_dt() up, as the code
> scanning the "/chosen" node now needs to be aware of the values of
> "#address-cells" and "#size-cells".
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  Documentation/devicetree/bindings/chosen.txt |  6 ++--
>  drivers/of/fdt.c                             | 37 ++++++++++++++++++--
>  2 files changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
> index 45e79172a646c537..5b0b94eb2d04e79d 100644
> --- a/Documentation/devicetree/bindings/chosen.txt
> +++ b/Documentation/devicetree/bindings/chosen.txt
> @@ -106,9 +106,9 @@ respectively, of the root node.
>  linux,elfcorehdr
>  ----------------
>
> -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.
> +This property 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.
>  e.g.
>
>  / {
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index a03d43f95495d8e1..f13db831c8028cce 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -8,6 +8,7 @@
>
>  #define pr_fmt(fmt)    "OF: fdt: " fmt
>
> +#include <linux/crash_dump.h>
>  #include <linux/crc32.h>
>  #include <linux/kernel.h>
>  #include <linux/initrd.h>
> @@ -909,6 +910,35 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)
>  }
>  #endif /* CONFIG_BLK_DEV_INITRD */
>
> +#ifdef CONFIG_CRASH_DUMP
> +/**
> + * early_init_dt_check_for_elfcorehdr - Decode elfcorehdr location from flat
> + * tree
> + * @node: reference to node containing elfcorehdr location ('chosen')
> + */
> +static void __init early_init_dt_check_for_elfcorehdr(unsigned long node)
> +{
> +       const __be32 *prop;
> +       int len;
> +
> +       pr_debug("Looking for elfcorehdr property... ");
> +
> +       prop = of_get_flat_dt_prop(node, "linux,elfcorehdr", &len);
> +       if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
> +               return;
> +
> +       elfcorehdr_addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
> +       elfcorehdr_size = dt_mem_next_cell(dt_root_size_cells, &prop);

If these declarations were moved outside the '#ifdef
CONFIG_CRASH_DUMP' in crash_dump.h, then IS_ENABLED() could be used in
this function.


> +
> +       pr_debug("elfcorehdr_start=0x%llx elfcorehdr_size=0x%llx\n",
> +                elfcorehdr_addr, elfcorehdr_size);
> +}
> +#else
> +static inline void early_init_dt_check_for_elfcorehdr(unsigned long node)
> +{
> +}
> +#endif
> +
>  #ifdef CONFIG_SERIAL_EARLYCON
>
>  int __init early_init_dt_scan_chosen_stdout(void)
> @@ -1057,6 +1087,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>                 return 0;
>
>         early_init_dt_check_for_initrd(node);
> +       early_init_dt_check_for_elfcorehdr(node);
>
>         /* Retrieve command line */
>         p = of_get_flat_dt_prop(node, "bootargs", &l);
> @@ -1201,14 +1232,14 @@ void __init early_init_dt_scan_nodes(void)
>  {
>         int rc = 0;
>
> +       /* Initialize {size,address}-cells info */
> +       of_scan_flat_dt(early_init_dt_scan_root, NULL);
> +
>         /* Retrieve various information from the /chosen node */
>         rc = of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
>         if (!rc)
>                 pr_warn("No chosen node found, continuing without\n");
>
> -       /* Initialize {size,address}-cells info */
> -       of_scan_flat_dt(early_init_dt_scan_root, NULL);
> -
>         /* Setup memory, calling early_init_dt_add_memory_arch */
>         of_scan_flat_dt(early_init_dt_scan_memory, NULL);
>  }
> --
> 2.25.1
>
Nick Kossifidis June 15, 2021, 11:28 p.m. UTC | #2
Στις 2021-06-15 21:17, Geert Uytterhoeven έγραψε:
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c

I believe drivers/of/kexec.c is better suited for this.
Geert Uytterhoeven June 16, 2021, 7:58 a.m. UTC | #3
Hi Nick,

On Wed, Jun 16, 2021 at 1:28 AM Nick Kossifidis <mick@ics.forth.gr> wrote:
>
> Στις 2021-06-15 21:17, Geert Uytterhoeven έγραψε:
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>
> I believe drivers/of/kexec.c is better suited for this.

From an earlier response from me to a comment from Rob:

| > Also, note that there is now a drivers/of/kexec.c (in -next) though
| > not sure if all this would go there or stay in fdt.c with the rest of
| > the memory parsing.
|
| It's gonna be the latter, as that file handles the FDT during early
| kernel startup, for both normal and kdump kernels.
|
| Despite the name, drivers/of/kexec.c is not for kexec, but for
| kexec_file.  This is the "new" fancy syscall that prepares the DTB
| for the new kernel itself, unlike the classic kexec syscall, where
| userspace is responsible for preparing the DTB for the new kernel.

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
index 45e79172a646c537..5b0b94eb2d04e79d 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -106,9 +106,9 @@  respectively, of the root node.
 linux,elfcorehdr
 ----------------
 
-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.
+This property 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.
 e.g.
 
 / {
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index a03d43f95495d8e1..f13db831c8028cce 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -8,6 +8,7 @@ 
 
 #define pr_fmt(fmt)	"OF: fdt: " fmt
 
+#include <linux/crash_dump.h>
 #include <linux/crc32.h>
 #include <linux/kernel.h>
 #include <linux/initrd.h>
@@ -909,6 +910,35 @@  static inline void early_init_dt_check_for_initrd(unsigned long node)
 }
 #endif /* CONFIG_BLK_DEV_INITRD */
 
+#ifdef CONFIG_CRASH_DUMP
+/**
+ * early_init_dt_check_for_elfcorehdr - Decode elfcorehdr location from flat
+ * tree
+ * @node: reference to node containing elfcorehdr location ('chosen')
+ */
+static void __init early_init_dt_check_for_elfcorehdr(unsigned long node)
+{
+	const __be32 *prop;
+	int len;
+
+	pr_debug("Looking for elfcorehdr property... ");
+
+	prop = of_get_flat_dt_prop(node, "linux,elfcorehdr", &len);
+	if (!prop || (len < (dt_root_addr_cells + dt_root_size_cells)))
+		return;
+
+	elfcorehdr_addr = dt_mem_next_cell(dt_root_addr_cells, &prop);
+	elfcorehdr_size = dt_mem_next_cell(dt_root_size_cells, &prop);
+
+	pr_debug("elfcorehdr_start=0x%llx elfcorehdr_size=0x%llx\n",
+		 elfcorehdr_addr, elfcorehdr_size);
+}
+#else
+static inline void early_init_dt_check_for_elfcorehdr(unsigned long node)
+{
+}
+#endif
+
 #ifdef CONFIG_SERIAL_EARLYCON
 
 int __init early_init_dt_scan_chosen_stdout(void)
@@ -1057,6 +1087,7 @@  int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 		return 0;
 
 	early_init_dt_check_for_initrd(node);
+	early_init_dt_check_for_elfcorehdr(node);
 
 	/* Retrieve command line */
 	p = of_get_flat_dt_prop(node, "bootargs", &l);
@@ -1201,14 +1232,14 @@  void __init early_init_dt_scan_nodes(void)
 {
 	int rc = 0;
 
+	/* Initialize {size,address}-cells info */
+	of_scan_flat_dt(early_init_dt_scan_root, NULL);
+
 	/* Retrieve various information from the /chosen node */
 	rc = of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line);
 	if (!rc)
 		pr_warn("No chosen node found, continuing without\n");
 
-	/* Initialize {size,address}-cells info */
-	of_scan_flat_dt(early_init_dt_scan_root, NULL);
-
 	/* Setup memory, calling early_init_dt_add_memory_arch */
 	of_scan_flat_dt(early_init_dt_scan_memory, NULL);
 }