[PATCH v4 0/2] Add capability to dump fdt blob for arm64 platforms

[PATCH v4 0/2] Add capability to dump fdt blob for arm64 platforms

[Bhupesh Sharma]

Changes since v3:
----------------
 - Addressed comments from Akashi regarding:
   ~ Using sensible names for variables and pointers.
   ~ Removing usage of local pointer variable in 'dump_fdt()' function.
   ~ Tried addressing a comment regarding converting GET_CELL macro to
     inline function, but the final implementation became more tricky, so
     dropped the same and decided to go ahead with the macro for
     simplicity.
 - v3 can be viewed here: https://www.spinics.net/lists/kexec/msg20544.html

Changes since v2:
----------------
 - Added ascii prints for printing bootargs.
 - v2 can be viewed here: http://lists.infradead.org/pipermail/kexec/2018-April/020532.html

Changes since v1:
----------------
 - No functional changes: Just added a cover letter to explain the
   background better and also capture some details on where I found this
   patchset handy. Also added some dtb dumps logs from 'kexec -p -d' for
   reference (with this patchset applied) for clarity.
 - v1 can be viewed here: http://lists.infradead.org/pipermail/kexec/2018-April/020407.html

While working on a couple of issues related to primary kernel crash on freescale
and huawei arm64 boards, I noticed that the primary kernel crashed before it could reach
the command prompt but was able to launch some early initramfs scriptware. 

In the initial initramfs scriptware crashkernel loading was automated along
with auto load of other userspace applications (for e.g. on the freescale board
there are networking applications like ODP/DPDK which can be launched automatically via
scriptware).

I was hoping that the crashkernel would be able to load when the primary kernel crashes,
and using the crash core dump thus obtained, I would be able to debug the problem which
caused the primary kernel to crash late in the boot flow (before reaching the boot prompt).

Unfortunately currently we can experience an early crash in crashkernel itself
(on such example is the 'acpi table access' issue in the arm64 crashkernel
which we discussed some time back upstream
<https://www.spinics.net/lists/arm-kernel/msg616632.html>):

In such cases, we have no opportunity to obtain the crash core dump which can be
used to debug the primary kernel crash.

Now, looking at just the panic messages from the crashkernel in such cases is sometimes
not very useful in debugging what might have caused it to crash when the primary kernel
is able to atleast boot past that point on the same hardware platform.

Debugging the issue closer (especially on the request for help on the freescale board), I
realized that the crashkernel crash may be caused by improper/buggy fixing of 'dtb'
being passed to the crashkernel - especially the 'linux,usable-memory-range' property.

For such cases, I found that dumping the dtb blob entries from kexec-tools is
a useful debugging tip as I could identify the 'linux,usable-memory-range'
property did not contain ACPI RECLAIM region entries.

Please note that since the primary kernel crashes before the command prompt
can be reached, it is not possible to run a dtc interpreter there (and it
also adds the requirement for an additional 'dtc' tool to be present in the initramfs).

Also, it might not be possible to always correctly time the 'dtc' interpreter loading
via the initramfs scriptware and store the binary/hex output to a storage device
just after the crashkernel is loaded via 'kexec -p' as the storage driver itself
might have panick'ed during the meanwhile.

In view of the above, it would be useful to dump the fdt blob being passed to the second
(kexec/kdump) kernel when '-d' flag is specified while invoking kexec/kdump. This allows
 one to look at the device-tree fields that is being passed to the secondary
kernel and accordingly debug issues.

This can be specially useful for the arm64 case, where we are still fixing up some issues
with the upstream kexec-tools/arm64 kernel.

I loathe to keep this patch locally and apply it locally on top of the upstream 'kexec-tools'
patches when debugging such issues, so it would be probably good to have this feature
available in upstream itself.

Here is an example output of the dtb dump(on an arm64 board), on serial console with
the patchset applied and 'kexec -p' launched used with a '-d' flag using initramfs scriptware:

<..snip..>

setup_2nd_dtb: found /sys/firmware/fdt
 / {
    #size-cells = <0x00000002>;
    #address-cells = <0x00000002>;
    chosen {
        linux,usable-memory-range = <0x00000000 0xdfe00000 0x00000000 0x20000000>;
        linux,elfcorehdr = <0x00000000 0xffdf0000 0x00000000 0x00001400>;
        kaslr-seed = <0x00000000 0x00000000>;
        linux,uefi-mmap-desc-ver = <0x00000001>;
        linux,uefi-mmap-desc-size = <0x00000030>;
        linux,uefi-mmap-size = <0x000020a0>;
        linux,uefi-mmap-start = <0x00000000 0x07a81018>;
        linux,uefi-system-table = <0x00000000 0x17fc0018>;
	bootargs = "root=/dev/mapper/rhel_qualcomm--amberwing--rep--15-root ro rd.lvm.lv=rhel_qualcomm-amberwing-rep-15/root rd.lvm.lv=rhel_qualcomm-amberwing-rep-15/swap";
        linux,initrd-end = <0x00000000 0x05e8a7a1>;
        linux,initrd-start = <0x00000000 0x04b49000>;
    };
 };

<..snip..>

Bhupesh Sharma (2):
  dt-ops: Add helper API to dump fdt blob
  kexec-arm64: Add functionality to dump 2nd dtb

 kexec/arch/arm64/kexec-arm64.c |   2 +
 kexec/dt-ops.c                 | 141 +++++++++++++++++++++++++++++++++++++++++
 kexec/dt-ops.h                 |   1 +
 3 files changed, 144 insertions(+)

-- 
2.7.4


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v4 1/2] dt-ops: Add helper API to dump fdt blob

[Bhupesh Sharma]

At several occasions it would be useful to dump the fdt
blob being passed to the second (kexec/kdump) kernel
when '-d' flag is specified while invoking kexec/kdump.

This allows one to look at the device-tree fields that
is being passed to the secondary kernel and accordingly
debug issues.

This can be specially useful for the arm64 case, where
kexec_load() or kdump passes important information like
'linux,usable-memory' ranges to the second kernel, and
the correctness of the ranges can be verified by
looking at the device-tree dump with '-d' flag specified.

Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
---
 kexec/dt-ops.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kexec/dt-ops.h |   1 +
 2 files changed, 142 insertions(+)

diff --git a/kexec/dt-ops.c b/kexec/dt-ops.c
index 915dbf55afd2..80ebfd31b4b6 100644
--- a/kexec/dt-ops.c
+++ b/kexec/dt-ops.c
@@ -8,6 +8,10 @@
 #include "kexec.h"
 #include "dt-ops.h"
 
+#define ALIGN(x, a)	(((x) + ((a) - 1)) & ~((a) - 1))
+#define PALIGN(p, a)	((void *)(ALIGN((unsigned long)(p), (a))))
+#define GET_CELL(p)	(p += 4, *((const uint32_t *)(p - 4)))
+
 static const char n_chosen[] = "/chosen";
 
 static const char p_bootargs[] = "bootargs";
@@ -143,3 +147,140 @@ int dtb_delete_property(char *dtb, const char *node, const char *prop)
 
 	return result;
 }
+
+static uint64_t is_printable_string(const void* data, uint64_t len)
+{
+	const char *s = data;
+	const char *ss;
+
+	/* Check for zero length strings */
+	if (len == 0)
+		return 0;
+
+	/* String must be terminated with a '\0' */
+	if (s[len - 1] != '\0')
+		return 0;
+
+	ss = s;
+	while (*s)
+		s++;
+
+	/* Traverse till we hit a '\0' or reach 'len' */
+	if (*s != '\0')
+		return 0;
+
+	if ((s + 1 - ss) < len) {
+		/* Handle special cases such as 'bootargs' properties
+		 * in dtb which are actually strings, but they may have
+		 * a format where (s + 1 - ss) < len remains true.
+		 *
+		 * We can catch such cases by checking if (s + 1 - ss)
+		 * is greater than 1
+		 */
+		if ((s + 1 - ss) > 1)
+			return 1;
+
+		return 0;
+	}
+
+	return 1;
+}
+
+static void print_data(const char* data, uint64_t len)
+{
+	uint64_t i;
+	const char *p_data = data;
+
+	/* Check for non-zero length */
+	if (len == 0)
+		return;
+
+	if (is_printable_string(data, len)) {
+		dbgprintf(" = \"%s\"", (const char *)data);
+	} else if ((len % 4) == 0) {
+		dbgprintf(" = <");
+		for (i = 0; i < len; i += 4) {
+			dbgprintf("0x%08x%s",
+					fdt32_to_cpu(GET_CELL(p_data)),
+					i < (len - 4) ? " " : "");
+		}
+		dbgprintf(">");
+	} else {
+		dbgprintf(" = [");
+		for (i = 0; i < len; i++)
+			dbgprintf("%02x%s", *p_data++,
+					i < len - 1 ? " " : "");
+		dbgprintf("]");
+	}
+}
+
+void dump_fdt(void* fdt)
+{
+	struct fdt_header *bph;
+	const char* p_struct;
+	const char* p_strings;
+	const char* p_data;
+	const char* s_data;
+	uint32_t off_dt;
+	uint32_t off_str;
+	uint32_t tag;
+	uint64_t sz;
+	uint64_t depth;
+	uint64_t shift;
+	uint32_t version;
+
+	depth = 0;
+	shift = 4;
+
+	bph = fdt;
+	off_dt = fdt32_to_cpu(bph->off_dt_struct);
+	off_str = fdt32_to_cpu(bph->off_dt_strings);
+	p_struct = (const char*)fdt + off_dt;
+	p_strings = (const char*)fdt + off_str;
+	version = fdt32_to_cpu(bph->version);
+
+	p_data = p_struct;
+	while ((tag = fdt32_to_cpu(GET_CELL(p_data))) != FDT_END) {
+		if (tag == FDT_BEGIN_NODE) {
+			s_data = p_data;
+			p_data = PALIGN(p_data + strlen(s_data) + 1, 4);
+
+			if (*s_data == '\0')
+				s_data = "/";
+
+			dbgprintf("%*s%s {\n", (int)(depth * shift), " ", s_data);
+
+			depth++;
+			continue;
+		}
+
+		if (tag == FDT_END_NODE) {
+			depth--;
+
+			dbgprintf("%*s};\n", (int)(depth * shift), " ");
+			continue;
+		}
+
+		if (tag == FDT_NOP) {
+			dbgprintf("%*s// [NOP]\n", (int)(depth * shift), " ");
+			continue;
+		}
+
+		if (tag != FDT_PROP) {
+			dbgprintf("%*s ** Unknown tag 0x%08x\n",
+					(int)(depth * shift), " ", tag);
+			break;
+		}
+
+		sz = fdt32_to_cpu(GET_CELL(p_data));
+		s_data = p_strings + fdt32_to_cpu(GET_CELL(p_data));
+		if (version < 16 && sz >= 8)
+			p_data = PALIGN(p_data, 8);
+
+		dbgprintf("%*s%s", (int)(depth * shift), " ", s_data);
+		print_data(p_data, sz);
+		dbgprintf(";\n");
+
+		p_data = PALIGN(p_data + sz, 4);
+	}
+}
diff --git a/kexec/dt-ops.h b/kexec/dt-ops.h
index e70d15d8ffbf..25b9b569f2b7 100644
--- a/kexec/dt-ops.h
+++ b/kexec/dt-ops.h
@@ -9,5 +9,6 @@ int dtb_set_property(char **dtb, off_t *dtb_size, const char *node,
 	const char *prop, const void *value, int value_len);
 
 int dtb_delete_property(char *dtb, const char *node, const char *prop);
+void dump_fdt(void *fdt) ;
 
 #endif
-- 
2.7.4


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v4 2/2] kexec-arm64: Add functionality to dump 2nd dtb

[Bhupesh Sharma]

Since during the arm64 kexec_load()/kdump invocation,
the dtb is passed to the second kernel, it is sometimes useful
to dump the dtb contents (to verify the correctness
of the same).

This patch adds this feature which is enabled when '-d' flag is
used with kexec command line invocation.

Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
---
 kexec/arch/arm64/kexec-arm64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index a206c172b1aa..47df756bd595 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -540,6 +540,8 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
 	dtb->buf = new_buf;
 	dtb->size = fdt_totalsize(new_buf);
 
+	dbgprintf("%s: found %s\n", __func__, dtb->path);
+	dump_fdt(dtb->buf);
 	dump_reservemap(dtb);
 
 	return result;
-- 
2.7.4


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 1/2] dt-ops: Add helper API to dump fdt blob

[Simon Horman]

On Thu, Jun 21, 2018 at 03:54:37PM +0530, Bhupesh Sharma wrote:
> At several occasions it would be useful to dump the fdt
> blob being passed to the second (kexec/kdump) kernel
> when '-d' flag is specified while invoking kexec/kdump.
> 
> This allows one to look at the device-tree fields that
> is being passed to the secondary kernel and accordingly
> debug issues.
> 
> This can be specially useful for the arm64 case, where
> kexec_load() or kdump passes important information like
> 'linux,usable-memory' ranges to the second kernel, and
> the correctness of the ranges can be verified by
> looking at the device-tree dump with '-d' flag specified.
> 
> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> ---
>  kexec/dt-ops.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  kexec/dt-ops.h |   1 +
>  2 files changed, 142 insertions(+)
> 
> diff --git a/kexec/dt-ops.c b/kexec/dt-ops.c
> index 915dbf55afd2..80ebfd31b4b6 100644
> --- a/kexec/dt-ops.c
> +++ b/kexec/dt-ops.c
> @@ -8,6 +8,10 @@
>  #include "kexec.h"
>  #include "dt-ops.h"
>  
> +#define ALIGN(x, a)	(((x) + ((a) - 1)) & ~((a) - 1))
> +#define PALIGN(p, a)	((void *)(ALIGN((unsigned long)(p), (a))))
> +#define GET_CELL(p)	(p += 4, *((const uint32_t *)(p - 4)))
> +

I see the above in scripts/dtc/util.c in the kernel tree, which is licensed
GPL v2, whereas this file is licensed GPL v2 or later. Is there is
also a published GPL v2 or later version somewhere?

>  static const char n_chosen[] = "/chosen";
>  
>  static const char p_bootargs[] = "bootargs";
> @@ -143,3 +147,140 @@ int dtb_delete_property(char *dtb, const char *node, const char *prop)
>  
>  	return result;
>  }
> +
> +static uint64_t is_printable_string(const void* data, uint64_t len)

uint64_t seems to be a strange return type. Perhaps bool would be best.

> +{
> +	const char *s = data;
> +	const char *ss;

I think naming s as str and ss as start would enhance
code readability.

> +
> +	/* Check for zero length strings */
> +	if (len == 0)
> +		return 0;
> +
> +	/* String must be terminated with a '\0' */
> +	if (s[len - 1] != '\0')
> +		return 0;
> +
> +	ss = s;
> +	while (*s)
> +		s++;
> +
> +	/* Traverse till we hit a '\0' or reach 'len' */
> +	if (*s != '\0')
> +		return 0;

The two conditions above are: while s is not 0; if s != 0.
How can the if condition ever be true?

Also, do you want to check that the characters traversed are printable?
If not perhaps the name of the function should change.

> +
> +	if ((s + 1 - ss) < len) {
> +		/* Handle special cases such as 'bootargs' properties
> +		 * in dtb which are actually strings, but they may have
> +		 * a format where (s + 1 - ss) < len remains true.
> +		 *
> +		 * We can catch such cases by checking if (s + 1 - ss)
> +		 * is greater than 1
> +		 */
> +		if ((s + 1 - ss) > 1)
> +			return 1;

Is this covering the case where null-terminated strings are imeded in 'data'?
And ensuring that the is at least one non-null byte in the first string?
If so, I think the comment could be improved, explaining things in terms of
multiple strings. And I'm not sure you need the outermost if condition.

> +
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +static void print_data(const char* data, uint64_t len)
> +{
> +	uint64_t i;
> +	const char *p_data = data;
> +
> +	/* Check for non-zero length */
> +	if (len == 0)
> +		return;
> +
> +	if (is_printable_string(data, len)) {
> +		dbgprintf(" = \"%s\"", (const char *)data);
> +	} else if ((len % 4) == 0) {
> +		dbgprintf(" = <");
> +		for (i = 0; i < len; i += 4) {
> +			dbgprintf("0x%08x%s",
> +					fdt32_to_cpu(GET_CELL(p_data)),
> +					i < (len - 4) ? " " : "");
> +		}
> +		dbgprintf(">");

Is there a performance benefit of using fdt32_to_cpu()
that justifies the extra complexity here?

> +	} else {
> +		dbgprintf(" = [");
> +		for (i = 0; i < len; i++)
> +			dbgprintf("%02x%s", *p_data++,
> +					i < len - 1 ? " " : "");
> +		dbgprintf("]");
> +	}
> +}
> +
> +void dump_fdt(void* fdt)
> +{
> +	struct fdt_header *bph;
> +	const char* p_struct;
> +	const char* p_strings;
> +	const char* p_data;
> +	const char* s_data;
> +	uint32_t off_dt;
> +	uint32_t off_str;
> +	uint32_t tag;
> +	uint64_t sz;
> +	uint64_t depth;
> +	uint64_t shift;

Can they type of depth and shift be int?
It would avoid the (int) casting below.

> +	uint32_t version;
> +
> +	depth = 0;
> +	shift = 4;
> +
> +	bph = fdt;
> +	off_dt = fdt32_to_cpu(bph->off_dt_struct);
> +	off_str = fdt32_to_cpu(bph->off_dt_strings);
> +	p_struct = (const char*)fdt + off_dt;
> +	p_strings = (const char*)fdt + off_str;
> +	version = fdt32_to_cpu(bph->version);
> +
> +	p_data = p_struct;
> +	while ((tag = fdt32_to_cpu(GET_CELL(p_data))) != FDT_END) {
> +		if (tag == FDT_BEGIN_NODE) {
> +			s_data = p_data;
> +			p_data = PALIGN(p_data + strlen(s_data) + 1, 4);
> +
> +			if (*s_data == '\0')
> +				s_data = "/";
> +
> +			dbgprintf("%*s%s {\n", (int)(depth * shift), " ", s_data);
> +
> +			depth++;
> +			continue;
> +		}
> +
> +		if (tag == FDT_END_NODE) {
> +			depth--;
> +
> +			dbgprintf("%*s};\n", (int)(depth * shift), " ");
> +			continue;
> +		}
> +
> +		if (tag == FDT_NOP) {
> +			dbgprintf("%*s// [NOP]\n", (int)(depth * shift), " ");
> +			continue;
> +		}
> +
> +		if (tag != FDT_PROP) {
> +			dbgprintf("%*s ** Unknown tag 0x%08x\n",
> +					(int)(depth * shift), " ", tag);
> +			break;
> +		}
> +
> +		sz = fdt32_to_cpu(GET_CELL(p_data));
> +		s_data = p_strings + fdt32_to_cpu(GET_CELL(p_data));
> +		if (version < 16 && sz >= 8)
> +			p_data = PALIGN(p_data, 8);
> +
> +		dbgprintf("%*s%s", (int)(depth * shift), " ", s_data);
> +		print_data(p_data, sz);
> +		dbgprintf(";\n");
> +
> +		p_data = PALIGN(p_data + sz, 4);
> +	}
> +}
> diff --git a/kexec/dt-ops.h b/kexec/dt-ops.h
> index e70d15d8ffbf..25b9b569f2b7 100644
> --- a/kexec/dt-ops.h
> +++ b/kexec/dt-ops.h
> @@ -9,5 +9,6 @@ int dtb_set_property(char **dtb, off_t *dtb_size, const char *node,
>  	const char *prop, const void *value, int value_len);
>  
>  int dtb_delete_property(char *dtb, const char *node, const char *prop);
> +void dump_fdt(void *fdt) ;
>  
>  #endif
> -- 
> 2.7.4
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 2/2] kexec-arm64: Add functionality to dump 2nd dtb

[Simon Horman]

On Thu, Jun 21, 2018 at 03:54:38PM +0530, Bhupesh Sharma wrote:
> Since during the arm64 kexec_load()/kdump invocation,
> the dtb is passed to the second kernel, it is sometimes useful
> to dump the dtb contents (to verify the correctness
> of the same).
> 
> This patch adds this feature which is enabled when '-d' flag is
> used with kexec command line invocation.

This seems significantly more verbose than what -d already outputs.
Perhaps a second debug level is warranted here?

> 
> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> ---
>  kexec/arch/arm64/kexec-arm64.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index a206c172b1aa..47df756bd595 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -540,6 +540,8 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
>  	dtb->buf = new_buf;
>  	dtb->size = fdt_totalsize(new_buf);
>  
> +	dbgprintf("%s: found %s\n", __func__, dtb->path);
> +	dump_fdt(dtb->buf);
>  	dump_reservemap(dtb);
>  
>  	return result;
> -- 
> 2.7.4
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 2/2] kexec-arm64: Add functionality to dump 2nd dtb

[AKASHI Takahiro]

On Wed, Jun 27, 2018 at 02:04:38PM +0200, Simon Horman wrote:
> On Thu, Jun 21, 2018 at 03:54:38PM +0530, Bhupesh Sharma wrote:
> > Since during the arm64 kexec_load()/kdump invocation,
> > the dtb is passed to the second kernel, it is sometimes useful
> > to dump the dtb contents (to verify the correctness
> > of the same).
> > 
> > This patch adds this feature which is enabled when '-d' flag is
> > used with kexec command line invocation.
> 
> This seems significantly more verbose than what -d already outputs.
> Perhaps a second debug level is warranted here?

+1
Debug messages on arm64 are already noisy due to ones from
machine_apply_elf_rel(), which are not very useful in most cases,
even making logs hard to understand.
Some sort of level be helpful here, too.

Thanks,
-Takahiro AKASHI

> 
> > 
> > Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> > ---
> >  kexec/arch/arm64/kexec-arm64.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> > index a206c172b1aa..47df756bd595 100644
> > --- a/kexec/arch/arm64/kexec-arm64.c
> > +++ b/kexec/arch/arm64/kexec-arm64.c
> > @@ -540,6 +540,8 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
> >  	dtb->buf = new_buf;
> >  	dtb->size = fdt_totalsize(new_buf);
> >  
> > +	dbgprintf("%s: found %s\n", __func__, dtb->path);
> > +	dump_fdt(dtb->buf);
> >  	dump_reservemap(dtb);
> >  
> >  	return result;
> > -- 
> > 2.7.4
> > 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 2/2] kexec-arm64: Add functionality to dump 2nd dtb

[Bhupesh Sharma]

Hi Simon and Akashi,

Sorry for the delay, I was out of office most of last week.

On 06/28/2018 05:06 AM, AKASHI Takahiro wrote:
> On Wed, Jun 27, 2018 at 02:04:38PM +0200, Simon Horman wrote:
>> On Thu, Jun 21, 2018 at 03:54:38PM +0530, Bhupesh Sharma wrote:
>>> Since during the arm64 kexec_load()/kdump invocation,
>>> the dtb is passed to the second kernel, it is sometimes useful
>>> to dump the dtb contents (to verify the correctness
>>> of the same).
>>>
>>> This patch adds this feature which is enabled when '-d' flag is
>>> used with kexec command line invocation.
>>
>> This seems significantly more verbose than what -d already outputs.
>> Perhaps a second debug level is warranted here?
> 
> +1
> Debug messages on arm64 are already noisy due to ones from
> machine_apply_elf_rel(), which are not very useful in most cases,
> even making logs hard to understand.
> Some sort of level be helpful here, too.

Ok, will work on the same and send out a v5 with the fixes.

Thanks,
Bhupesh

>>
>>>
>>> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
>>> ---
>>>   kexec/arch/arm64/kexec-arm64.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
>>> index a206c172b1aa..47df756bd595 100644
>>> --- a/kexec/arch/arm64/kexec-arm64.c
>>> +++ b/kexec/arch/arm64/kexec-arm64.c
>>> @@ -540,6 +540,8 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
>>>   	dtb->buf = new_buf;
>>>   	dtb->size = fdt_totalsize(new_buf);
>>>   
>>> +	dbgprintf("%s: found %s\n", __func__, dtb->path);
>>> +	dump_fdt(dtb->buf);
>>>   	dump_reservemap(dtb);
>>>   
>>>   	return result;
>>> -- 
>>> 2.7.4
>>>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 1/2] dt-ops: Add helper API to dump fdt blob

[Bhupesh Sharma]

Hi Simon,

Thanks for the review. Please see my comments inline.

On 06/27/2018 05:32 PM, Simon Horman wrote:
> On Thu, Jun 21, 2018 at 03:54:37PM +0530, Bhupesh Sharma wrote:
>> At several occasions it would be useful to dump the fdt
>> blob being passed to the second (kexec/kdump) kernel
>> when '-d' flag is specified while invoking kexec/kdump.
>>
>> This allows one to look at the device-tree fields that
>> is being passed to the secondary kernel and accordingly
>> debug issues.
>>
>> This can be specially useful for the arm64 case, where
>> kexec_load() or kdump passes important information like
>> 'linux,usable-memory' ranges to the second kernel, and
>> the correctness of the ranges can be verified by
>> looking at the device-tree dump with '-d' flag specified.
>>
>> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
>> ---
>>   kexec/dt-ops.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   kexec/dt-ops.h |   1 +
>>   2 files changed, 142 insertions(+)
>>
>> diff --git a/kexec/dt-ops.c b/kexec/dt-ops.c
>> index 915dbf55afd2..80ebfd31b4b6 100644
>> --- a/kexec/dt-ops.c
>> +++ b/kexec/dt-ops.c
>> @@ -8,6 +8,10 @@
>>   #include "kexec.h"
>>   #include "dt-ops.h"
>>   
>> +#define ALIGN(x, a)	(((x) + ((a) - 1)) & ~((a) - 1))
>> +#define PALIGN(p, a)	((void *)(ALIGN((unsigned long)(p), (a))))
>> +#define GET_CELL(p)	(p += 4, *((const uint32_t *)(p - 4)))
>> +
> 
> I see the above in scripts/dtc/util.c in the kernel tree, which is licensed
> GPL v2, whereas this file is licensed GPL v2 or later. Is there is
> also a published GPL v2 or later version somewhere?

Indeed, it is published in several places.
libfdt is GPL/BSD dual-licensed, i.e., it may be used
either under the terms of the GPL, or under the terms of the 2-clause
BSD license (aka the ISC license). See 
<https://github.com/dgibson/dtc/blob/master/README.license> for details.

I am not sure if we will have licensing issues with including a GPL/BSD 
dual-licensed code in a GPL v2 or later code. I think it should be ok, 
but IANAL :)

Do you see any possible licensing conflicts here?

>>   static const char n_chosen[] = "/chosen";
>>   
>>   static const char p_bootargs[] = "bootargs";
>> @@ -143,3 +147,140 @@ int dtb_delete_property(char *dtb, const char *node, const char *prop)
>>   
>>   	return result;
>>   }
>> +
>> +static uint64_t is_printable_string(const void* data, uint64_t len)
> 
> uint64_t seems to be a strange return type. Perhaps bool would be best.
> 
>> +{
>> +	const char *s = data;
>> +	const char *ss;
> 
> I think naming s as str and ss as start would enhance
> code readability.

Sure, will fix this in v5.

>> +
>> +	/* Check for zero length strings */
>> +	if (len == 0)
>> +		return 0;
>> +
>> +	/* String must be terminated with a '\0' */
>> +	if (s[len - 1] != '\0')
>> +		return 0;
>> +
>> +	ss = s;
>> +	while (*s)
>> +		s++;
>> +
>> +	/* Traverse till we hit a '\0' or reach 'len' */
>> +	if (*s != '\0')
>> +		return 0;
> 
> The two conditions above are: while s is not 0; if s != 0.
> How can the if condition ever be true?
> 
> Also, do you want to check that the characters traversed are printable?
> If not perhaps the name of the function should change.

Ok.

>> +
>> +	if ((s + 1 - ss) < len) {
>> +		/* Handle special cases such as 'bootargs' properties
>> +		 * in dtb which are actually strings, but they may have
>> +		 * a format where (s + 1 - ss) < len remains true.
>> +		 *
>> +		 * We can catch such cases by checking if (s + 1 - ss)
>> +		 * is greater than 1
>> +		 */
>> +		if ((s + 1 - ss) > 1)
>> +			return 1;
> 
> Is this covering the case where null-terminated strings are imeded in 'data'?
> And ensuring that the is at least one non-null byte in the first string?
> If so, I think the comment could be improved, explaining things in terms of
> multiple strings. And I'm not sure you need the outermost if condition.

Yes, I can improve the comment to explain the condition you mentioned above.

>> +
>> +		return 0;
>> +	}
>> +
>> +	return 1;
>> +}
>> +
>> +static void print_data(const char* data, uint64_t len)
>> +{
>> +	uint64_t i;
>> +	const char *p_data = data;
>> +
>> +	/* Check for non-zero length */
>> +	if (len == 0)
>> +		return;
>> +
>> +	if (is_printable_string(data, len)) {
>> +		dbgprintf(" = \"%s\"", (const char *)data);
>> +	} else if ((len % 4) == 0) {
>> +		dbgprintf(" = <");
>> +		for (i = 0; i < len; i += 4) {
>> +			dbgprintf("0x%08x%s",
>> +					fdt32_to_cpu(GET_CELL(p_data)),
>> +					i < (len - 4) ? " " : "");
>> +		}
>> +		dbgprintf(">");
> 
> Is there a performance benefit of using fdt32_to_cpu()
> that justifies the extra complexity here?

Yes, I see that fdt32_to_cpu() is considerably faster as compared to 
other implementations I tried.

>> +	} else {
>> +		dbgprintf(" = [");
>> +		for (i = 0; i < len; i++)
>> +			dbgprintf("%02x%s", *p_data++,
>> +					i < len - 1 ? " " : "");
>> +		dbgprintf("]");
>> +	}
>> +}
>> +
>> +void dump_fdt(void* fdt)
>> +{
>> +	struct fdt_header *bph;
>> +	const char* p_struct;
>> +	const char* p_strings;
>> +	const char* p_data;
>> +	const char* s_data;
>> +	uint32_t off_dt;
>> +	uint32_t off_str;
>> +	uint32_t tag;
>> +	uint64_t sz;
>> +	uint64_t depth;
>> +	uint64_t shift;
> 
> Can they type of depth and shift be int?
> It would avoid the (int) casting below.

Hmm. I am not sure, but I can try and use 'int' in v5, if things work 
fine during local checks.

>> +	uint32_t version;
>> +
>> +	depth = 0;
>> +	shift = 4;
>> +
>> +	bph = fdt;
>> +	off_dt = fdt32_to_cpu(bph->off_dt_struct);
>> +	off_str = fdt32_to_cpu(bph->off_dt_strings);
>> +	p_struct = (const char*)fdt + off_dt;
>> +	p_strings = (const char*)fdt + off_str;
>> +	version = fdt32_to_cpu(bph->version);
>> +
>> +	p_data = p_struct;
>> +	while ((tag = fdt32_to_cpu(GET_CELL(p_data))) != FDT_END) {
>> +		if (tag == FDT_BEGIN_NODE) {
>> +			s_data = p_data;
>> +			p_data = PALIGN(p_data + strlen(s_data) + 1, 4);
>> +
>> +			if (*s_data == '\0')
>> +				s_data = "/";
>> +
>> +			dbgprintf("%*s%s {\n", (int)(depth * shift), " ", s_data);
>> +
>> +			depth++;
>> +			continue;
>> +		}
>> +
>> +		if (tag == FDT_END_NODE) {
>> +			depth--;
>> +
>> +			dbgprintf("%*s};\n", (int)(depth * shift), " ");
>> +			continue;
>> +		}
>> +
>> +		if (tag == FDT_NOP) {
>> +			dbgprintf("%*s// [NOP]\n", (int)(depth * shift), " ");
>> +			continue;
>> +		}
>> +
>> +		if (tag != FDT_PROP) {
>> +			dbgprintf("%*s ** Unknown tag 0x%08x\n",
>> +					(int)(depth * shift), " ", tag);
>> +			break;
>> +		}
>> +
>> +		sz = fdt32_to_cpu(GET_CELL(p_data));
>> +		s_data = p_strings + fdt32_to_cpu(GET_CELL(p_data));
>> +		if (version < 16 && sz >= 8)
>> +			p_data = PALIGN(p_data, 8);
>> +
>> +		dbgprintf("%*s%s", (int)(depth * shift), " ", s_data);
>> +		print_data(p_data, sz);
>> +		dbgprintf(";\n");
>> +
>> +		p_data = PALIGN(p_data + sz, 4);
>> +	}
>> +}
>> diff --git a/kexec/dt-ops.h b/kexec/dt-ops.h
>> index e70d15d8ffbf..25b9b569f2b7 100644
>> --- a/kexec/dt-ops.h
>> +++ b/kexec/dt-ops.h
>> @@ -9,5 +9,6 @@ int dtb_set_property(char **dtb, off_t *dtb_size, const char *node,
>>   	const char *prop, const void *value, int value_len);
>>   
>>   int dtb_delete_property(char *dtb, const char *node, const char *prop);
>> +void dump_fdt(void *fdt) ;
>>   
>>   #endif
>> -- 
>> 2.7.4
>>

Regards,
Bhupesh

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 0/2] Add capability to dump fdt blob for arm64 platforms

[AKASHI Takahiro]

Bhupesh, Simon,

#I'm afraid that I gonna rehash the old discussion.

Looking into ppc's kexec-tools code, I found that ppc version of
this tool has a similar feature and it saves a new dtb into a file,
named "debug.dtb."
See save_fixed_up_dtb() in kexec/arch/ppc/fixup_dtb.c

Even if it is a debug feature, we'd better go in the same way
(if possible).

Thanks,
-Takahiro AKASHI


On Thu, Jun 21, 2018 at 03:54:36PM +0530, Bhupesh Sharma wrote:
> Changes since v3:
> ----------------
>  - Addressed comments from Akashi regarding:
>    ~ Using sensible names for variables and pointers.
>    ~ Removing usage of local pointer variable in 'dump_fdt()' function.
>    ~ Tried addressing a comment regarding converting GET_CELL macro to
>      inline function, but the final implementation became more tricky, so
>      dropped the same and decided to go ahead with the macro for
>      simplicity.
>  - v3 can be viewed here: https://www.spinics.net/lists/kexec/msg20544.html
> 
> Changes since v2:
> ----------------
>  - Added ascii prints for printing bootargs.
>  - v2 can be viewed here: http://lists.infradead.org/pipermail/kexec/2018-April/020532.html
> 
> Changes since v1:
> ----------------
>  - No functional changes: Just added a cover letter to explain the
>    background better and also capture some details on where I found this
>    patchset handy. Also added some dtb dumps logs from 'kexec -p -d' for
>    reference (with this patchset applied) for clarity.
>  - v1 can be viewed here: http://lists.infradead.org/pipermail/kexec/2018-April/020407.html
> 
> While working on a couple of issues related to primary kernel crash on freescale
> and huawei arm64 boards, I noticed that the primary kernel crashed before it could reach
> the command prompt but was able to launch some early initramfs scriptware. 
> 
> In the initial initramfs scriptware crashkernel loading was automated along
> with auto load of other userspace applications (for e.g. on the freescale board
> there are networking applications like ODP/DPDK which can be launched automatically via
> scriptware).
> 
> I was hoping that the crashkernel would be able to load when the primary kernel crashes,
> and using the crash core dump thus obtained, I would be able to debug the problem which
> caused the primary kernel to crash late in the boot flow (before reaching the boot prompt).
> 
> Unfortunately currently we can experience an early crash in crashkernel itself
> (on such example is the 'acpi table access' issue in the arm64 crashkernel
> which we discussed some time back upstream
> <https://www.spinics.net/lists/arm-kernel/msg616632.html>):
> 
> In such cases, we have no opportunity to obtain the crash core dump which can be
> used to debug the primary kernel crash.
> 
> Now, looking at just the panic messages from the crashkernel in such cases is sometimes
> not very useful in debugging what might have caused it to crash when the primary kernel
> is able to atleast boot past that point on the same hardware platform.
> 
> Debugging the issue closer (especially on the request for help on the freescale board), I
> realized that the crashkernel crash may be caused by improper/buggy fixing of 'dtb'
> being passed to the crashkernel - especially the 'linux,usable-memory-range' property.
> 
> For such cases, I found that dumping the dtb blob entries from kexec-tools is
> a useful debugging tip as I could identify the 'linux,usable-memory-range'
> property did not contain ACPI RECLAIM region entries.
> 
> Please note that since the primary kernel crashes before the command prompt
> can be reached, it is not possible to run a dtc interpreter there (and it
> also adds the requirement for an additional 'dtc' tool to be present in the initramfs).
> 
> Also, it might not be possible to always correctly time the 'dtc' interpreter loading
> via the initramfs scriptware and store the binary/hex output to a storage device
> just after the crashkernel is loaded via 'kexec -p' as the storage driver itself
> might have panick'ed during the meanwhile.
> 
> In view of the above, it would be useful to dump the fdt blob being passed to the second
> (kexec/kdump) kernel when '-d' flag is specified while invoking kexec/kdump. This allows
>  one to look at the device-tree fields that is being passed to the secondary
> kernel and accordingly debug issues.
> 
> This can be specially useful for the arm64 case, where we are still fixing up some issues
> with the upstream kexec-tools/arm64 kernel.
> 
> I loathe to keep this patch locally and apply it locally on top of the upstream 'kexec-tools'
> patches when debugging such issues, so it would be probably good to have this feature
> available in upstream itself.
> 
> Here is an example output of the dtb dump(on an arm64 board), on serial console with
> the patchset applied and 'kexec -p' launched used with a '-d' flag using initramfs scriptware:
> 
> <..snip..>
> 
> setup_2nd_dtb: found /sys/firmware/fdt
>  / {
>     #size-cells = <0x00000002>;
>     #address-cells = <0x00000002>;
>     chosen {
>         linux,usable-memory-range = <0x00000000 0xdfe00000 0x00000000 0x20000000>;
>         linux,elfcorehdr = <0x00000000 0xffdf0000 0x00000000 0x00001400>;
>         kaslr-seed = <0x00000000 0x00000000>;
>         linux,uefi-mmap-desc-ver = <0x00000001>;
>         linux,uefi-mmap-desc-size = <0x00000030>;
>         linux,uefi-mmap-size = <0x000020a0>;
>         linux,uefi-mmap-start = <0x00000000 0x07a81018>;
>         linux,uefi-system-table = <0x00000000 0x17fc0018>;
> 	bootargs = "root=/dev/mapper/rhel_qualcomm--amberwing--rep--15-root ro rd.lvm.lv=rhel_qualcomm-amberwing-rep-15/root rd.lvm.lv=rhel_qualcomm-amberwing-rep-15/swap";
>         linux,initrd-end = <0x00000000 0x05e8a7a1>;
>         linux,initrd-start = <0x00000000 0x04b49000>;
>     };
>  };
> 
> <..snip..>
> 
> Bhupesh Sharma (2):
>   dt-ops: Add helper API to dump fdt blob
>   kexec-arm64: Add functionality to dump 2nd dtb
> 
>  kexec/arch/arm64/kexec-arm64.c |   2 +
>  kexec/dt-ops.c                 | 141 +++++++++++++++++++++++++++++++++++++++++
>  kexec/dt-ops.h                 |   1 +
>  3 files changed, 144 insertions(+)
> 
> -- 
> 2.7.4
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 0/2] Add capability to dump fdt blob for arm64 platforms

[Bhupesh Sharma]

Hi Akashi,

On 07/09/2018 12:41 PM, AKASHI Takahiro wrote:
> Bhupesh, Simon,
> 
> #I'm afraid that I gonna rehash the old discussion.
> 
> Looking into ppc's kexec-tools code, I found that ppc version of
> this tool has a similar feature and it saves a new dtb into a file,
> named "debug.dtb."
> See save_fixed_up_dtb() in kexec/arch/ppc/fixup_dtb.c
> 
> Even if it is a debug feature, we'd better go in the same way
> (if possible).

Thanks for sharing the same.

I had a look earlier also at the ppc - "debug.dtb" code in kexec-tools, 
but this is again fails to fulfill the same purpose that we are trying 
to resolve for early primary kernel crash(es) on arm64 machines, because 
of which we are not able to reach the command prompt with the primary 
kernel itself.

We would need to either reach the command prompt to dump out/save the 
"debug.dtb" file when we are running the primary kernel (which is 
unfortunately not always possible wit latest kernels on some of the 
arm64 machines), or use some initramfs scriptware to dump out the same 
to some secondary storage device (nfs server or usb stick).

So, I would still suggest to stick with the approach I proposed for now 
(and we can take a similar approach as ppc for arm64 later on) once we 
have kexec, kdump and other user-space utilities (which rely on the 
kexec kernel framework) working stably on arm64 machines (with latest 
upstream kernels).

Thanks,
Bhupesh

> On Thu, Jun 21, 2018 at 03:54:36PM +0530, Bhupesh Sharma wrote:
>> Changes since v3:
>> ----------------
>>   - Addressed comments from Akashi regarding:
>>     ~ Using sensible names for variables and pointers.
>>     ~ Removing usage of local pointer variable in 'dump_fdt()' function.
>>     ~ Tried addressing a comment regarding converting GET_CELL macro to
>>       inline function, but the final implementation became more tricky, so
>>       dropped the same and decided to go ahead with the macro for
>>       simplicity.
>>   - v3 can be viewed here: https://www.spinics.net/lists/kexec/msg20544.html
>>
>> Changes since v2:
>> ----------------
>>   - Added ascii prints for printing bootargs.
>>   - v2 can be viewed here: http://lists.infradead.org/pipermail/kexec/2018-April/020532.html
>>
>> Changes since v1:
>> ----------------
>>   - No functional changes: Just added a cover letter to explain the
>>     background better and also capture some details on where I found this
>>     patchset handy. Also added some dtb dumps logs from 'kexec -p -d' for
>>     reference (with this patchset applied) for clarity.
>>   - v1 can be viewed here: http://lists.infradead.org/pipermail/kexec/2018-April/020407.html
>>
>> While working on a couple of issues related to primary kernel crash on freescale
>> and huawei arm64 boards, I noticed that the primary kernel crashed before it could reach
>> the command prompt but was able to launch some early initramfs scriptware.
>>
>> In the initial initramfs scriptware crashkernel loading was automated along
>> with auto load of other userspace applications (for e.g. on the freescale board
>> there are networking applications like ODP/DPDK which can be launched automatically via
>> scriptware).
>>
>> I was hoping that the crashkernel would be able to load when the primary kernel crashes,
>> and using the crash core dump thus obtained, I would be able to debug the problem which
>> caused the primary kernel to crash late in the boot flow (before reaching the boot prompt).
>>
>> Unfortunately currently we can experience an early crash in crashkernel itself
>> (on such example is the 'acpi table access' issue in the arm64 crashkernel
>> which we discussed some time back upstream
>> <https://www.spinics.net/lists/arm-kernel/msg616632.html>):
>>
>> In such cases, we have no opportunity to obtain the crash core dump which can be
>> used to debug the primary kernel crash.
>>
>> Now, looking at just the panic messages from the crashkernel in such cases is sometimes
>> not very useful in debugging what might have caused it to crash when the primary kernel
>> is able to atleast boot past that point on the same hardware platform.
>>
>> Debugging the issue closer (especially on the request for help on the freescale board), I
>> realized that the crashkernel crash may be caused by improper/buggy fixing of 'dtb'
>> being passed to the crashkernel - especially the 'linux,usable-memory-range' property.
>>
>> For such cases, I found that dumping the dtb blob entries from kexec-tools is
>> a useful debugging tip as I could identify the 'linux,usable-memory-range'
>> property did not contain ACPI RECLAIM region entries.
>>
>> Please note that since the primary kernel crashes before the command prompt
>> can be reached, it is not possible to run a dtc interpreter there (and it
>> also adds the requirement for an additional 'dtc' tool to be present in the initramfs).
>>
>> Also, it might not be possible to always correctly time the 'dtc' interpreter loading
>> via the initramfs scriptware and store the binary/hex output to a storage device
>> just after the crashkernel is loaded via 'kexec -p' as the storage driver itself
>> might have panick'ed during the meanwhile.
>>
>> In view of the above, it would be useful to dump the fdt blob being passed to the second
>> (kexec/kdump) kernel when '-d' flag is specified while invoking kexec/kdump. This allows
>>   one to look at the device-tree fields that is being passed to the secondary
>> kernel and accordingly debug issues.
>>
>> This can be specially useful for the arm64 case, where we are still fixing up some issues
>> with the upstream kexec-tools/arm64 kernel.
>>
>> I loathe to keep this patch locally and apply it locally on top of the upstream 'kexec-tools'
>> patches when debugging such issues, so it would be probably good to have this feature
>> available in upstream itself.
>>
>> Here is an example output of the dtb dump(on an arm64 board), on serial console with
>> the patchset applied and 'kexec -p' launched used with a '-d' flag using initramfs scriptware:
>>
>> <..snip..>
>>
>> setup_2nd_dtb: found /sys/firmware/fdt
>>   / {
>>      #size-cells = <0x00000002>;
>>      #address-cells = <0x00000002>;
>>      chosen {
>>          linux,usable-memory-range = <0x00000000 0xdfe00000 0x00000000 0x20000000>;
>>          linux,elfcorehdr = <0x00000000 0xffdf0000 0x00000000 0x00001400>;
>>          kaslr-seed = <0x00000000 0x00000000>;
>>          linux,uefi-mmap-desc-ver = <0x00000001>;
>>          linux,uefi-mmap-desc-size = <0x00000030>;
>>          linux,uefi-mmap-size = <0x000020a0>;
>>          linux,uefi-mmap-start = <0x00000000 0x07a81018>;
>>          linux,uefi-system-table = <0x00000000 0x17fc0018>;
>> 	bootargs = "root=/dev/mapper/rhel_qualcomm--amberwing--rep--15-root ro rd.lvm.lv=rhel_qualcomm-amberwing-rep-15/root rd.lvm.lv=rhel_qualcomm-amberwing-rep-15/swap";
>>          linux,initrd-end = <0x00000000 0x05e8a7a1>;
>>          linux,initrd-start = <0x00000000 0x04b49000>;
>>      };
>>   };
>>
>> <..snip..>
>>
>> Bhupesh Sharma (2):
>>    dt-ops: Add helper API to dump fdt blob
>>    kexec-arm64: Add functionality to dump 2nd dtb
>>
>>   kexec/arch/arm64/kexec-arm64.c |   2 +
>>   kexec/dt-ops.c                 | 141 +++++++++++++++++++++++++++++++++++++++++
>>   kexec/dt-ops.h                 |   1 +
>>   3 files changed, 144 insertions(+)
>>
>> -- 
>> 2.7.4
>>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 0/2] Add capability to dump fdt blob for arm64 platforms

[Simon Horman]

On Tue, Jul 17, 2018 at 02:13:42AM +0530, Bhupesh Sharma wrote:
> Hi Akashi,
> 
> On 07/09/2018 12:41 PM, AKASHI Takahiro wrote:
> > Bhupesh, Simon,
> > 
> > #I'm afraid that I gonna rehash the old discussion.
> > 
> > Looking into ppc's kexec-tools code, I found that ppc version of
> > this tool has a similar feature and it saves a new dtb into a file,
> > named "debug.dtb."
> > See save_fixed_up_dtb() in kexec/arch/ppc/fixup_dtb.c
> > 
> > Even if it is a debug feature, we'd better go in the same way
> > (if possible).
> 
> Thanks for sharing the same.
> 
> I had a look earlier also at the ppc - "debug.dtb" code in kexec-tools, but
> this is again fails to fulfill the same purpose that we are trying to
> resolve for early primary kernel crash(es) on arm64 machines, because of
> which we are not able to reach the command prompt with the primary kernel
> itself.
> 
> We would need to either reach the command prompt to dump out/save the
> "debug.dtb" file when we are running the primary kernel (which is
> unfortunately not always possible wit latest kernels on some of the arm64
> machines), or use some initramfs scriptware to dump out the same to some
> secondary storage device (nfs server or usb stick).
> 
> So, I would still suggest to stick with the approach I proposed for now (and
> we can take a similar approach as ppc for arm64 later on) once we have
> kexec, kdump and other user-space utilities (which rely on the kexec kernel
> framework) working stably on arm64 machines (with latest upstream kernels).

That sounds like a reasonable argument to me.

Akashi, what do you think?

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v4 0/2] Add capability to dump fdt blob for arm64 platforms

[AKASHI Takahiro]

On Tue, Jul 24, 2018 at 08:31:54AM +0200, Simon Horman wrote:
> On Tue, Jul 17, 2018 at 02:13:42AM +0530, Bhupesh Sharma wrote:
> > Hi Akashi,
> > 
> > On 07/09/2018 12:41 PM, AKASHI Takahiro wrote:
> > > Bhupesh, Simon,
> > > 
> > > #I'm afraid that I gonna rehash the old discussion.
> > > 
> > > Looking into ppc's kexec-tools code, I found that ppc version of
> > > this tool has a similar feature and it saves a new dtb into a file,
> > > named "debug.dtb."
> > > See save_fixed_up_dtb() in kexec/arch/ppc/fixup_dtb.c
> > > 
> > > Even if it is a debug feature, we'd better go in the same way
> > > (if possible).
> > 
> > Thanks for sharing the same.
> > 
> > I had a look earlier also at the ppc - "debug.dtb" code in kexec-tools, but
> > this is again fails to fulfill the same purpose that we are trying to
> > resolve for early primary kernel crash(es) on arm64 machines, because of
> > which we are not able to reach the command prompt with the primary kernel
> > itself.
> > 
> > We would need to either reach the command prompt to dump out/save the
> > "debug.dtb" file when we are running the primary kernel (which is
> > unfortunately not always possible wit latest kernels on some of the arm64
> > machines),

Although I'm not sure how it cannot be possible,

> or use some initramfs scriptware to dump out the same to some
> > secondary storage device (nfs server or usb stick).
> > 
> > So, I would still suggest to stick with the approach I proposed for now (and
> > we can take a similar approach as ppc for arm64 later on) once we have
> > kexec, kdump and other user-space utilities (which rely on the kexec kernel
> > framework) working stably on arm64 machines (with latest upstream kernels).
> 
> That sounds like a reasonable argument to me.
> 
> Akashi, what do you think?

it's fine to me, too.

-Takahiro AKASHI

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec