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

[RESEND PATCH 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 | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kexec/dt-ops.h |   1 +
 2 files changed, 134 insertions(+)

diff --git a/kexec/dt-ops.c b/kexec/dt-ops.c
index 915dbf55afd2..caf6f8df6e6b 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,132 @@ 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' || (s + 1 - ss) < len) {
+		return 0;
+	}
+
+	return 1;
+}
+
+static void print_data(const char* data, uint64_t len)
+{
+	uint64_t i;
+	const char *p = 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)),
+					i < (len - 4) ? " " : "");
+		}
+		dbgprintf(">");
+	} else {
+		dbgprintf(" = [");
+		for (i = 0; i < len; i++)
+			dbgprintf("%02x%s", *p++,
+					i < len - 1 ? " " : "");
+		dbgprintf("]");
+	}
+}
+
+void dump_fdt(void* fdt)
+{
+	struct fdt_header *bph;
+	const char* p_struct;
+	const char* p_strings;
+	const char* p;
+	const char* s;
+	const char* t;
+	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 = p_struct;
+	while ((tag = fdt32_to_cpu(GET_CELL(p))) != FDT_END) {
+
+		if (tag == FDT_BEGIN_NODE) {
+			s = p;
+			p = PALIGN(p + strlen(s) + 1, 4);
+
+			if (*s == '\0')
+				s = "/";
+
+			dbgprintf("%*s%s {\n", (int)(depth * shift), " ", s);
+
+			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));
+		s = p_strings + fdt32_to_cpu(GET_CELL(p));
+		if (version < 16 && sz >= 8)
+			p = PALIGN(p, 8);
+		t = p;
+
+		p = PALIGN(p + sz, 4);
+
+		dbgprintf("%*s%s", (int)(depth * shift), " ", s);
+		print_data(t, sz);
+		dbgprintf(";\n");
+	}
+}
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

[RESEND PATCH 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 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
index 62f37585b788..1b54718465b9 100644
--- a/kexec/arch/arm64/kexec-arm64.c
+++ b/kexec/arch/arm64/kexec-arm64.c
@@ -477,9 +477,10 @@ static int setup_2nd_dtb(struct dtb *dtb, char *command_line, int on_crash)
 		dtb->size = fdt_totalsize(new_buf);
 	}
 
+	dbgprintf("%s: found %s\n", __func__, dtb->path);
+	dump_fdt(dtb->buf);
 	dump_reservemap(dtb);
 
-
 	return result;
 
 on_error:
-- 
2.7.4


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

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

[AKASHI Takahiro]

On Mon, Mar 19, 2018 at 04:05:38PM +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.

Why not just save binary data to a file and interpret it
with dtc command?

-Takahiro AKASHI


> 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 | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  kexec/dt-ops.h |   1 +
>  2 files changed, 134 insertions(+)
> 
> diff --git a/kexec/dt-ops.c b/kexec/dt-ops.c
> index 915dbf55afd2..caf6f8df6e6b 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,132 @@ 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' || (s + 1 - ss) < len) {
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +static void print_data(const char* data, uint64_t len)
> +{
> +	uint64_t i;
> +	const char *p = 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)),
> +					i < (len - 4) ? " " : "");
> +		}
> +		dbgprintf(">");
> +	} else {
> +		dbgprintf(" = [");
> +		for (i = 0; i < len; i++)
> +			dbgprintf("%02x%s", *p++,
> +					i < len - 1 ? " " : "");
> +		dbgprintf("]");
> +	}
> +}
> +
> +void dump_fdt(void* fdt)
> +{
> +	struct fdt_header *bph;
> +	const char* p_struct;
> +	const char* p_strings;
> +	const char* p;
> +	const char* s;
> +	const char* t;
> +	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 = p_struct;
> +	while ((tag = fdt32_to_cpu(GET_CELL(p))) != FDT_END) {
> +
> +		if (tag == FDT_BEGIN_NODE) {
> +			s = p;
> +			p = PALIGN(p + strlen(s) + 1, 4);
> +
> +			if (*s == '\0')
> +				s = "/";
> +
> +			dbgprintf("%*s%s {\n", (int)(depth * shift), " ", s);
> +
> +			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));
> +		s = p_strings + fdt32_to_cpu(GET_CELL(p));
> +		if (version < 16 && sz >= 8)
> +			p = PALIGN(p, 8);
> +		t = p;
> +
> +		p = PALIGN(p + sz, 4);
> +
> +		dbgprintf("%*s%s", (int)(depth * shift), " ", s);
> +		print_data(t, sz);
> +		dbgprintf(";\n");
> +	}
> +}
> 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: [RESEND PATCH 1/2] dt-ops: Add helper API to dump fdt blob

[Bhupesh Sharma]

On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
> On Mon, Mar 19, 2018 at 04:05:38PM +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.
>
> Why not just save binary data to a file and interpret it
> with dtc command?

Well, there are a couple of reasons for that which can be understood
from a system which is in a production environment (for e.g. I have
worked on one arm64 system in the past which used a yocto based
distribution in which kexec -p was launched with a -d option as a part
of initial ramfs scriptware):

- In a production environment, installing and executing tools like dtc
(which might not have been installed by default via 'yum' or 'apt-get
install' or other means is not only an additional step, but we might
not get a chance to run it even if it is installed if we have a early
crash in kdump itself (for e.g. consider the 'acpi table access' issue
in the arm64 crashkernel we discussed some time back
<https://www.spinics.net/lists/arm-kernel/msg616632.html>):

a) In such a case the primary kernel has already crashed, so we had no
opportunity to run a dtc interpreter there and the kdump kernel itself
has crashed in a very early boot phase. So we didn't get a chance to
execute 'dtc' on the kdump kernel command prompt (if the kdump scripts
are configured not to reboot the primary again).

b) Also when an early arm64 kdump crash is reported by a customer, we
usually only have access to the primary and secondary console log
which also might include the 'kexec -p -d' log messages, which can
point us to a discrepancy in dtc nodes like 'linux,usable-memory"
which might have caused a early crashkernel crash.

Personally, so-far I have found this dtb dumping facility of use in
debugging atleast a couple of arm64 crashkernel crash/panic issues.
Till the arm64 kexec/kdump implementation matures further, I feel this
dumping facility is of good use to ease crashkernel panic debugs.

Regards,
Bhupesh


>
>> 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 | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  kexec/dt-ops.h |   1 +
>>  2 files changed, 134 insertions(+)
>>
>> diff --git a/kexec/dt-ops.c b/kexec/dt-ops.c
>> index 915dbf55afd2..caf6f8df6e6b 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,132 @@ 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' || (s + 1 - ss) < len) {
>> +             return 0;
>> +     }
>> +
>> +     return 1;
>> +}
>> +
>> +static void print_data(const char* data, uint64_t len)
>> +{
>> +     uint64_t i;
>> +     const char *p = 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)),
>> +                                     i < (len - 4) ? " " : "");
>> +             }
>> +             dbgprintf(">");
>> +     } else {
>> +             dbgprintf(" = [");
>> +             for (i = 0; i < len; i++)
>> +                     dbgprintf("%02x%s", *p++,
>> +                                     i < len - 1 ? " " : "");
>> +             dbgprintf("]");
>> +     }
>> +}
>> +
>> +void dump_fdt(void* fdt)
>> +{
>> +     struct fdt_header *bph;
>> +     const char* p_struct;
>> +     const char* p_strings;
>> +     const char* p;
>> +     const char* s;
>> +     const char* t;
>> +     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 = p_struct;
>> +     while ((tag = fdt32_to_cpu(GET_CELL(p))) != FDT_END) {
>> +
>> +             if (tag == FDT_BEGIN_NODE) {
>> +                     s = p;
>> +                     p = PALIGN(p + strlen(s) + 1, 4);
>> +
>> +                     if (*s == '\0')
>> +                             s = "/";
>> +
>> +                     dbgprintf("%*s%s {\n", (int)(depth * shift), " ", s);
>> +
>> +                     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));
>> +             s = p_strings + fdt32_to_cpu(GET_CELL(p));
>> +             if (version < 16 && sz >= 8)
>> +                     p = PALIGN(p, 8);
>> +             t = p;
>> +
>> +             p = PALIGN(p + sz, 4);
>> +
>> +             dbgprintf("%*s%s", (int)(depth * shift), " ", s);
>> +             print_data(t, sz);
>> +             dbgprintf(";\n");
>> +     }
>> +}
>> 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: [RESEND PATCH 1/2] dt-ops: Add helper API to dump fdt blob

[Bhupesh Sharma]

Hi Akashi,

On Tue, Mar 20, 2018 at 12:36 AM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
> On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
>> On Mon, Mar 19, 2018 at 04:05:38PM +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.
>>
>> Why not just save binary data to a file and interpret it
>> with dtc command?
>
> Well, there are a couple of reasons for that which can be understood
> from a system which is in a production environment (for e.g. I have
> worked on one arm64 system in the past which used a yocto based
> distribution in which kexec -p was launched with a -d option as a part
> of initial ramfs scriptware):
>
> - In a production environment, installing and executing tools like dtc
> (which might not have been installed by default via 'yum' or 'apt-get
> install' or other means is not only an additional step, but we might
> not get a chance to run it even if it is installed if we have a early
> crash in kdump itself (for e.g. consider the 'acpi table access' issue
> in the arm64 crashkernel we discussed some time back
> <https://www.spinics.net/lists/arm-kernel/msg616632.html>):
>
> a) In such a case the primary kernel has already crashed, so we had no
> opportunity to run a dtc interpreter there and the kdump kernel itself
> has crashed in a very early boot phase. So we didn't get a chance to
> execute 'dtc' on the kdump kernel command prompt (if the kdump scripts
> are configured not to reboot the primary again).
>
> b) Also when an early arm64 kdump crash is reported by a customer, we
> usually only have access to the primary and secondary console log
> which also might include the 'kexec -p -d' log messages, which can
> point us to a discrepancy in dtc nodes like 'linux,usable-memory"
> which might have caused a early crashkernel crash.
>
> Personally, so-far I have found this dtb dumping facility of use in
> debugging atleast a couple of arm64 crashkernel crash/panic issues.
> Till the arm64 kexec/kdump implementation matures further, I feel this
> dumping facility is of good use to ease crashkernel panic debugs.

Ping. Do you have any further comments on it?

Regards,
Bhupesh

>>> 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 | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  kexec/dt-ops.h |   1 +
>>>  2 files changed, 134 insertions(+)
>>>
>>> diff --git a/kexec/dt-ops.c b/kexec/dt-ops.c
>>> index 915dbf55afd2..caf6f8df6e6b 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,132 @@ 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' || (s + 1 - ss) < len) {
>>> +             return 0;
>>> +     }
>>> +
>>> +     return 1;
>>> +}
>>> +
>>> +static void print_data(const char* data, uint64_t len)
>>> +{
>>> +     uint64_t i;
>>> +     const char *p = 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)),
>>> +                                     i < (len - 4) ? " " : "");
>>> +             }
>>> +             dbgprintf(">");
>>> +     } else {
>>> +             dbgprintf(" = [");
>>> +             for (i = 0; i < len; i++)
>>> +                     dbgprintf("%02x%s", *p++,
>>> +                                     i < len - 1 ? " " : "");
>>> +             dbgprintf("]");
>>> +     }
>>> +}
>>> +
>>> +void dump_fdt(void* fdt)
>>> +{
>>> +     struct fdt_header *bph;
>>> +     const char* p_struct;
>>> +     const char* p_strings;
>>> +     const char* p;
>>> +     const char* s;
>>> +     const char* t;
>>> +     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 = p_struct;
>>> +     while ((tag = fdt32_to_cpu(GET_CELL(p))) != FDT_END) {
>>> +
>>> +             if (tag == FDT_BEGIN_NODE) {
>>> +                     s = p;
>>> +                     p = PALIGN(p + strlen(s) + 1, 4);
>>> +
>>> +                     if (*s == '\0')
>>> +                             s = "/";
>>> +
>>> +                     dbgprintf("%*s%s {\n", (int)(depth * shift), " ", s);
>>> +
>>> +                     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));
>>> +             s = p_strings + fdt32_to_cpu(GET_CELL(p));
>>> +             if (version < 16 && sz >= 8)
>>> +                     p = PALIGN(p, 8);
>>> +             t = p;
>>> +
>>> +             p = PALIGN(p + sz, 4);
>>> +
>>> +             dbgprintf("%*s%s", (int)(depth * shift), " ", s);
>>> +             print_data(t, sz);
>>> +             dbgprintf(";\n");
>>> +     }
>>> +}
>>> 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: [RESEND PATCH 1/2] dt-ops: Add helper API to dump fdt blob

[AKASHI Takahiro]

On Mon, Mar 26, 2018 at 02:29:31PM +0530, Bhupesh Sharma wrote:
> Hi Akashi,
> 
> On Tue, Mar 20, 2018 at 12:36 AM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
> > On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> >> On Mon, Mar 19, 2018 at 04:05:38PM +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.
> >>
> >> Why not just save binary data to a file and interpret it
> >> with dtc command?
> >
> > Well, there are a couple of reasons for that which can be understood
> > from a system which is in a production environment (for e.g. I have
> > worked on one arm64 system in the past which used a yocto based
> > distribution in which kexec -p was launched with a -d option as a part
> > of initial ramfs scriptware):
> >
> > - In a production environment, installing and executing tools like dtc
> > (which might not have been installed by default via 'yum' or 'apt-get
> > install' or other means is not only an additional step, but we might
> > not get a chance to run it even if it is installed if we have a early
> > crash in kdump itself (for e.g. consider the 'acpi table access' issue
> > in the arm64 crashkernel we discussed some time back
> > <https://www.spinics.net/lists/arm-kernel/msg616632.html>):
> >
> > a) In such a case the primary kernel has already crashed, so we had no
> > opportunity to run a dtc interpreter there and the kdump kernel itself
> > has crashed in a very early boot phase. So we didn't get a chance to
> > execute 'dtc' on the kdump kernel command prompt (if the kdump scripts
> > are configured not to reboot the primary again).
> >
> > b) Also when an early arm64 kdump crash is reported by a customer, we
> > usually only have access to the primary and secondary console log
> > which also might include the 'kexec -p -d' log messages, which can
> > point us to a discrepancy in dtc nodes like 'linux,usable-memory"
> > which might have caused a early crashkernel crash.
> >
> > Personally, so-far I have found this dtb dumping facility of use in
> > debugging atleast a couple of arm64 crashkernel crash/panic issues.
> > Till the arm64 kexec/kdump implementation matures further, I feel this
> > dumping facility is of good use to ease crashkernel panic debugs.
> 
> Ping. Do you have any further comments on it?

No.
While I don't think dumping fdt is so useful as "kexec -d" option
outputs enough information for me, you can go ahead.

Thanks,
-Takahiro AKASHI


> Regards,
> Bhupesh
> 
> >>> 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 | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>  kexec/dt-ops.h |   1 +
> >>>  2 files changed, 134 insertions(+)
> >>>
> >>> diff --git a/kexec/dt-ops.c b/kexec/dt-ops.c
> >>> index 915dbf55afd2..caf6f8df6e6b 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,132 @@ 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' || (s + 1 - ss) < len) {
> >>> +             return 0;
> >>> +     }
> >>> +
> >>> +     return 1;
> >>> +}
> >>> +
> >>> +static void print_data(const char* data, uint64_t len)
> >>> +{
> >>> +     uint64_t i;
> >>> +     const char *p = 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)),
> >>> +                                     i < (len - 4) ? " " : "");
> >>> +             }
> >>> +             dbgprintf(">");
> >>> +     } else {
> >>> +             dbgprintf(" = [");
> >>> +             for (i = 0; i < len; i++)
> >>> +                     dbgprintf("%02x%s", *p++,
> >>> +                                     i < len - 1 ? " " : "");
> >>> +             dbgprintf("]");
> >>> +     }
> >>> +}
> >>> +
> >>> +void dump_fdt(void* fdt)
> >>> +{
> >>> +     struct fdt_header *bph;
> >>> +     const char* p_struct;
> >>> +     const char* p_strings;
> >>> +     const char* p;
> >>> +     const char* s;
> >>> +     const char* t;
> >>> +     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 = p_struct;
> >>> +     while ((tag = fdt32_to_cpu(GET_CELL(p))) != FDT_END) {
> >>> +
> >>> +             if (tag == FDT_BEGIN_NODE) {
> >>> +                     s = p;
> >>> +                     p = PALIGN(p + strlen(s) + 1, 4);
> >>> +
> >>> +                     if (*s == '\0')
> >>> +                             s = "/";
> >>> +
> >>> +                     dbgprintf("%*s%s {\n", (int)(depth * shift), " ", s);
> >>> +
> >>> +                     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));
> >>> +             s = p_strings + fdt32_to_cpu(GET_CELL(p));
> >>> +             if (version < 16 && sz >= 8)
> >>> +                     p = PALIGN(p, 8);
> >>> +             t = p;
> >>> +
> >>> +             p = PALIGN(p + sz, 4);
> >>> +
> >>> +             dbgprintf("%*s%s", (int)(depth * shift), " ", s);
> >>> +             print_data(t, sz);
> >>> +             dbgprintf(";\n");
> >>> +     }
> >>> +}
> >>> 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: [RESEND PATCH 1/2] dt-ops: Add helper API to dump fdt blob

[James Morse]

Hi Akashi, Bhupesh,

On 27/03/18 10:04, AKASHI Takahiro wrote:
> On Mon, Mar 26, 2018 at 02:29:31PM +0530, Bhupesh Sharma wrote:
>> On Tue, Mar 20, 2018 at 12:36 AM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
>>> On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro
>>> <takahiro.akashi@linaro.org> wrote:
>>>> On Mon, Mar 19, 2018 at 04:05:38PM +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.
>>>>
>>>> Why not just save binary data to a file and interpret it
>>>> with dtc command?

I'd prefer this too. It would also let us debug any issue where kexec-tools
produces an invalid DTB. It also lets us test booting the kernel from firmware
with that DTB.


>>> Well, there are a couple of reasons for that which can be understood
>>> from a system which is in a production environment (for e.g. I have
>>> worked on one arm64 system in the past which used a yocto based
>>> distribution in which kexec -p was launched with a -d option as a part
>>> of initial ramfs scriptware):

and panics before you get an interactive prompt or persistent storage? I think
this would be a pretty niche use-case. You could always base64-dump the dtb to
stdout from your script.


>>> - In a production environment, installing and executing tools like dtc
>>> (which might not have been installed by default via 'yum' or 'apt-get
>>> install' or other means is not only an additional step, but we might
>>> not get a chance to run it even if it is installed if we have a early
>>> crash in kdump itself (for e.g. consider the 'acpi table access' issue
>>> in the arm64 crashkernel we discussed some time back
>>> <https://www.spinics.net/lists/arm-kernel/msg616632.html>):

Wouldn't it be possible to transfer the dumped DTB file off the machine before
the behaviour that brings the machine down? Kdump always requires a setup step
to load the kdump kernel/dtb. You have to decide to add the debug flags at this
point. I don't see how choosing to save the modified-DTB would be any different.

I assume the flow here is: do thingX, the kernel crashes. Enable kdump, do
thingX, kdump fails to boot. Enable kdump-debug. Do thingX...


>>> a) In such a case the primary kernel has already crashed, so we had no
>>> opportunity to run a dtc interpreter there and the kdump kernel itself
>>> has crashed in a very early boot phase. So we didn't get a chance to
>>> execute 'dtc' on the kdump kernel command prompt (if the kdump scripts
>>> are configured not to reboot the primary again).

Transfer it off the machine, save it somewhere persistent or print it to stdout
in base64.


>>> b) Also when an early arm64 kdump crash is reported by a customer, we
>>> usually only have access to the primary and secondary console log
>>> which also might include the 'kexec -p -d' log messages, which can
>>> point us to a discrepancy in dtc nodes like 'linux,usable-memory"
>>> which might have caused a early crashkernel crash.

>>> Personally, so-far I have found this dtb dumping facility of use in
>>> debugging atleast a couple of arm64 crashkernel crash/panic issues.
>>> Till the arm64 kexec/kdump implementation matures further, I feel this
>>> dumping facility is of good use to ease crashkernel panic debugs.
>>
>> Ping. Do you have any further comments on it?
> 
> No.
> While I don't think dumping fdt is so useful as "kexec -d" option
> outputs enough information for me, you can go ahead.

(likewise)


Thanks,

James

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

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

[Bhupesh Sharma]

Hi James,

Sorry for the delay, I had a long weekend last week.

On Tue, Mar 27, 2018 at 7:01 PM, James Morse <james.morse@arm.com> wrote:
> Hi Akashi, Bhupesh,
>
> On 27/03/18 10:04, AKASHI Takahiro wrote:
>> On Mon, Mar 26, 2018 at 02:29:31PM +0530, Bhupesh Sharma wrote:
>>> On Tue, Mar 20, 2018 at 12:36 AM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
>>>> On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro
>>>> <takahiro.akashi@linaro.org> wrote:
>>>>> On Mon, Mar 19, 2018 at 04:05:38PM +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.
>>>>>
>>>>> Why not just save binary data to a file and interpret it
>>>>> with dtc command?
>
> I'd prefer this too. It would also let us debug any issue where kexec-tools
> produces an invalid DTB. It also lets us test booting the kernel from firmware
> with that DTB.

I captured the use case where it is not possible to do so. I have seen
primary kernel crash before we can get to the command prompt to save
the dtb blob. Since the arm64 crashkernel still seems to have issues
itself while booting on acpi enabled machines (see
<https://www.spinics.net/lists/arm-kernel/msg616632.html>), so we are
trying to debug a problem which has two undefined variables :)

>>>> Well, there are a couple of reasons for that which can be understood
>>>> from a system which is in a production environment (for e.g. I have
>>>> worked on one arm64 system in the past which used a yocto based
>>>> distribution in which kexec -p was launched with a -d option as a part
>>>> of initial ramfs scriptware):
>
> and panics before you get an interactive prompt or persistent storage? I think
> this would be a pretty niche use-case. You could always base64-dump the dtb to
> stdout from your script.

That is pretty basic case on several new arm64 development boards
(e.g. qualcomm, huawei etc) where we are debugging issues in primary
kernel boot (and we are not even able to reach the command prompt).

If the crashkernel crashes even before the primary kernel does because
of the issues in the way DTB is passed to the crashkernel (which can
include wrong DTB fields), we better have mechanisms to track the same
rather than adding debug prints to the kernels.

Note that the print logs are enabled when -d flag is passed to
conventional 'kexec-tools' (where we already have several log
messages), so I am not sure how adding the dtb print here affects the
default (non-debug) case.

>>>> - In a production environment, installing and executing tools like dtc
>>>> (which might not have been installed by default via 'yum' or 'apt-get
>>>> install' or other means is not only an additional step, but we might
>>>> not get a chance to run it even if it is installed if we have a early
>>>> crash in kdump itself (for e.g. consider the 'acpi table access' issue
>>>> in the arm64 crashkernel we discussed some time back
>>>> <https://www.spinics.net/lists/arm-kernel/msg616632.html>):
>
> Wouldn't it be possible to transfer the dumped DTB file off the machine before
> the behaviour that brings the machine down? Kdump always requires a setup step
> to load the kdump kernel/dtb. You have to decide to add the debug flags at this
> point. I don't see how choosing to save the modified-DTB would be any different.

Its not always possible to do the same in a customer setup or on
remote board-farms.

> I assume the flow here is: do thingX, the kernel crashes. Enable kdump, do
> thingX, kdump fails to boot. Enable kdump-debug. Do thingX...

Please see the use case I shared above

>
>>>> a) In such a case the primary kernel has already crashed, so we had no
>>>> opportunity to run a dtc interpreter there and the kdump kernel itself
>>>> has crashed in a very early boot phase. So we didn't get a chance to
>>>> execute 'dtc' on the kdump kernel command prompt (if the kdump scripts
>>>> are configured not to reboot the primary again).
>
> Transfer it off the machine, save it somewhere persistent or print it to stdout
> in base64.
>
>
>>>> b) Also when an early arm64 kdump crash is reported by a customer, we
>>>> usually only have access to the primary and secondary console log
>>>> which also might include the 'kexec -p -d' log messages, which can
>>>> point us to a discrepancy in dtc nodes like 'linux,usable-memory"
>>>> which might have caused a early crashkernel crash.
>
>>>> Personally, so-far I have found this dtb dumping facility of use in
>>>> debugging atleast a couple of arm64 crashkernel crash/panic issues.
>>>> Till the arm64 kexec/kdump implementation matures further, I feel this
>>>> dumping facility is of good use to ease crashkernel panic debugs.
>>>
>>> Ping. Do you have any further comments on it?
>>
>> No.
>> While I don't think dumping fdt is so useful as "kexec -d" option
>> outputs enough information for me, you can go ahead.
>
> (likewise)
>

Regards,
Bhupesh

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

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

[Bhupesh Sharma]

Hello,

On Tue, Apr 3, 2018 at 7:28 PM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
> Hi James,
>
> Sorry for the delay, I had a long weekend last week.
>
> On Tue, Mar 27, 2018 at 7:01 PM, James Morse <james.morse@arm.com> wrote:
>> Hi Akashi, Bhupesh,
>>
>> On 27/03/18 10:04, AKASHI Takahiro wrote:
>>> On Mon, Mar 26, 2018 at 02:29:31PM +0530, Bhupesh Sharma wrote:
>>>> On Tue, Mar 20, 2018 at 12:36 AM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
>>>>> On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro
>>>>> <takahiro.akashi@linaro.org> wrote:
>>>>>> On Mon, Mar 19, 2018 at 04:05:38PM +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.
>>>>>>
>>>>>> Why not just save binary data to a file and interpret it
>>>>>> with dtc command?
>>
>> I'd prefer this too. It would also let us debug any issue where kexec-tools
>> produces an invalid DTB. It also lets us test booting the kernel from firmware
>> with that DTB.
>
> I captured the use case where it is not possible to do so. I have seen
> primary kernel crash before we can get to the command prompt to save
> the dtb blob. Since the arm64 crashkernel still seems to have issues
> itself while booting on acpi enabled machines (see
> <https://www.spinics.net/lists/arm-kernel/msg616632.html>), so we are
> trying to debug a problem which has two undefined variables :)
>
>>>>> Well, there are a couple of reasons for that which can be understood
>>>>> from a system which is in a production environment (for e.g. I have
>>>>> worked on one arm64 system in the past which used a yocto based
>>>>> distribution in which kexec -p was launched with a -d option as a part
>>>>> of initial ramfs scriptware):
>>
>> and panics before you get an interactive prompt or persistent storage? I think
>> this would be a pretty niche use-case. You could always base64-dump the dtb to
>> stdout from your script.
>
> That is pretty basic case on several new arm64 development boards
> (e.g. qualcomm, huawei etc) where we are debugging issues in primary
> kernel boot (and we are not even able to reach the command prompt).
>
> If the crashkernel crashes even before the primary kernel does because
> of the issues in the way DTB is passed to the crashkernel (which can
> include wrong DTB fields), we better have mechanisms to track the same
> rather than adding debug prints to the kernels.
>
> Note that the print logs are enabled when -d flag is passed to
> conventional 'kexec-tools' (where we already have several log
> messages), so I am not sure how adding the dtb print here affects the
> default (non-debug) case.
>
>>>>> - In a production environment, installing and executing tools like dtc
>>>>> (which might not have been installed by default via 'yum' or 'apt-get
>>>>> install' or other means is not only an additional step, but we might
>>>>> not get a chance to run it even if it is installed if we have a early
>>>>> crash in kdump itself (for e.g. consider the 'acpi table access' issue
>>>>> in the arm64 crashkernel we discussed some time back
>>>>> <https://www.spinics.net/lists/arm-kernel/msg616632.html>):
>>
>> Wouldn't it be possible to transfer the dumped DTB file off the machine before
>> the behaviour that brings the machine down? Kdump always requires a setup step
>> to load the kdump kernel/dtb. You have to decide to add the debug flags at this
>> point. I don't see how choosing to save the modified-DTB would be any different.
>
> Its not always possible to do the same in a customer setup or on
> remote board-farms.
>
>> I assume the flow here is: do thingX, the kernel crashes. Enable kdump, do
>> thingX, kdump fails to boot. Enable kdump-debug. Do thingX...
>
> Please see the use case I shared above
>
>>
>>>>> a) In such a case the primary kernel has already crashed, so we had no
>>>>> opportunity to run a dtc interpreter there and the kdump kernel itself
>>>>> has crashed in a very early boot phase. So we didn't get a chance to
>>>>> execute 'dtc' on the kdump kernel command prompt (if the kdump scripts
>>>>> are configured not to reboot the primary again).
>>
>> Transfer it off the machine, save it somewhere persistent or print it to stdout
>> in base64.
>>
>>
>>>>> b) Also when an early arm64 kdump crash is reported by a customer, we
>>>>> usually only have access to the primary and secondary console log
>>>>> which also might include the 'kexec -p -d' log messages, which can
>>>>> point us to a discrepancy in dtc nodes like 'linux,usable-memory"
>>>>> which might have caused a early crashkernel crash.
>>
>>>>> Personally, so-far I have found this dtb dumping facility of use in
>>>>> debugging atleast a couple of arm64 crashkernel crash/panic issues.
>>>>> Till the arm64 kexec/kdump implementation matures further, I feel this
>>>>> dumping facility is of good use to ease crashkernel panic debugs.
>>>>
>>>> Ping. Do you have any further comments on it?
>>>
>>> No.
>>> While I don't think dumping fdt is so useful as "kexec -d" option
>>> outputs enough information for me, you can go ahead.
>>
>> (likewise)
>>

Ping. Any comments on the above points?

If the use case is not clear in the git log message I can try and
resend this patchset and explain the use case and also maybe capture
the dtb dump that happens when -d option is used for clarity.

IMO getting this in upstream kexec-tools will really help distribution
guys like us who are debugging early crashes in the crashdump kernel
itself and enabling upstream primary kernels on newer arm64 boards (in
such cases, if both the primary and secondary happen to crash very
early during the boot flow, its very useful to have the dtb logs from
the kdump tool itself, which can be easily run in debug mode via
scriptware available in distributions like Fedora/Yocto).

Please let me know.

Thanks,
Bhupesh

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

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

[Dave Young]

Hi Bhupesh,

On 04/03/18 at 07:28pm, Bhupesh Sharma wrote:
> Hi James,
> 
> Sorry for the delay, I had a long weekend last week.
> 
> On Tue, Mar 27, 2018 at 7:01 PM, James Morse <james.morse@arm.com> wrote:
> > Hi Akashi, Bhupesh,
> >
> > On 27/03/18 10:04, AKASHI Takahiro wrote:
> >> On Mon, Mar 26, 2018 at 02:29:31PM +0530, Bhupesh Sharma wrote:
> >>> On Tue, Mar 20, 2018 at 12:36 AM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
> >>>> On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro
> >>>> <takahiro.akashi@linaro.org> wrote:
> >>>>> On Mon, Mar 19, 2018 at 04:05:38PM +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.
> >>>>>
> >>>>> Why not just save binary data to a file and interpret it
> >>>>> with dtc command?
> >
> > I'd prefer this too. It would also let us debug any issue where kexec-tools
> > produces an invalid DTB. It also lets us test booting the kernel from firmware
> > with that DTB.
> 
> I captured the use case where it is not possible to do so. I have seen
> primary kernel crash before we can get to the command prompt to save
> the dtb blob. Since the arm64 crashkernel still seems to have issues
> itself while booting on acpi enabled machines (see
> <https://www.spinics.net/lists/arm-kernel/msg616632.html>), so we are
> trying to debug a problem which has two undefined variables :)
> 
> >>>> Well, there are a couple of reasons for that which can be understood
> >>>> from a system which is in a production environment (for e.g. I have
> >>>> worked on one arm64 system in the past which used a yocto based
> >>>> distribution in which kexec -p was launched with a -d option as a part
> >>>> of initial ramfs scriptware):
> >
> > and panics before you get an interactive prompt or persistent storage? I think
> > this would be a pretty niche use-case. You could always base64-dump the dtb to
> > stdout from your script.
> 
> That is pretty basic case on several new arm64 development boards
> (e.g. qualcomm, huawei etc) where we are debugging issues in primary
> kernel boot (and we are not even able to reach the command prompt).
> 
> If the crashkernel crashes even before the primary kernel does because
> of the issues in the way DTB is passed to the crashkernel (which can
> include wrong DTB fields), we better have mechanisms to track the same
> rather than adding debug prints to the kernels.

In this case, since userspace always need to run 'kexec -l', there
should be chance to save dtb and dump them in scripts if needed.
If this is doable in scripts I also tend not to add the code in kexec-tools.

[snip]

Thanks
Dave

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

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

[Bhupesh Sharma]

Hi Dave,

On Mon, Apr 16, 2018 at 1:43 PM, Dave Young <dyoung@redhat.com> wrote:
> Hi Bhupesh,
>
> On 04/03/18 at 07:28pm, Bhupesh Sharma wrote:
>> Hi James,
>>
>> Sorry for the delay, I had a long weekend last week.
>>
>> On Tue, Mar 27, 2018 at 7:01 PM, James Morse <james.morse@arm.com> wrote:
>> > Hi Akashi, Bhupesh,
>> >
>> > On 27/03/18 10:04, AKASHI Takahiro wrote:
>> >> On Mon, Mar 26, 2018 at 02:29:31PM +0530, Bhupesh Sharma wrote:
>> >>> On Tue, Mar 20, 2018 at 12:36 AM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
>> >>>> On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro
>> >>>> <takahiro.akashi@linaro.org> wrote:
>> >>>>> On Mon, Mar 19, 2018 at 04:05:38PM +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.
>> >>>>>
>> >>>>> Why not just save binary data to a file and interpret it
>> >>>>> with dtc command?
>> >
>> > I'd prefer this too. It would also let us debug any issue where kexec-tools
>> > produces an invalid DTB. It also lets us test booting the kernel from firmware
>> > with that DTB.
>>
>> I captured the use case where it is not possible to do so. I have seen
>> primary kernel crash before we can get to the command prompt to save
>> the dtb blob. Since the arm64 crashkernel still seems to have issues
>> itself while booting on acpi enabled machines (see
>> <https://www.spinics.net/lists/arm-kernel/msg616632.html>), so we are
>> trying to debug a problem which has two undefined variables :)
>>
>> >>>> Well, there are a couple of reasons for that which can be understood
>> >>>> from a system which is in a production environment (for e.g. I have
>> >>>> worked on one arm64 system in the past which used a yocto based
>> >>>> distribution in which kexec -p was launched with a -d option as a part
>> >>>> of initial ramfs scriptware):
>> >
>> > and panics before you get an interactive prompt or persistent storage? I think
>> > this would be a pretty niche use-case. You could always base64-dump the dtb to
>> > stdout from your script.
>>
>> That is pretty basic case on several new arm64 development boards
>> (e.g. qualcomm, huawei etc) where we are debugging issues in primary
>> kernel boot (and we are not even able to reach the command prompt).
>>
>> If the crashkernel crashes even before the primary kernel does because
>> of the issues in the way DTB is passed to the crashkernel (which can
>> include wrong DTB fields), we better have mechanisms to track the same
>> rather than adding debug prints to the kernels.
>
> In this case, since userspace always need to run 'kexec -l', there
> should be chance to save dtb and dump them in scripts if needed.
> If this is doable in scripts I also tend not to add the code in kexec-tools.

Perhaps you missed my latest email which explained the use case better
(please see [1]).
Please note that I am talking about the 'kexec -p' or the kdump use
case rather than the 'kexec -l' or the warm reboot to the second
kernel.

In several cases its useful if we are not able to reach the command
prompt to just modify the scriptware to launch 'kexec -p' when the
ramfs starts up so that we can catch crashes which happen before the
primary kernel reaches the command prompt. For example, on freescale
boards I found this scriptware mechanism quite useful on yocto based
distributions, as you can be porting a new upstream kernel on the
board and thus is not very stable while booting and the crashkernel
also has issues which causes it to crash. In such cases although we
see a panic message from the crashkernel it is very difficult to
deduce that it was because of some wrongly fixed up dtb property.

Getting a debug log which contains the dtb dump is very useful in
above cases for debugging.

Also saving the dtb requires 'dtc' tool to be installed which is an
additional dependency.

Since we have debug facility/messages already available when we
execute 'kexec -p' with '-d' flag we can use the dtb dumps from the
tool to debug crashes than happen in the crashkernel also because of
wrong dtb fields fixed up by 'kexec-tools'. I was loathe to keep this
patch locally and apply to the upstream 'kexec-tools' when debugging
these issues as I think it makes sense to improve our debugging
capabilities if we know there are issues around the same.

Ok, let me resend this patch with a cover letter this time to explain
the use case better and also to capture the dtb logs.

[1] https://marc.info/?l=kexec&m=152382169120505&w=2

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

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

[Dave Young]

On 04/16/18 at 08:39pm, Bhupesh Sharma wrote:
> Hi Dave,
> 
> On Mon, Apr 16, 2018 at 1:43 PM, Dave Young <dyoung@redhat.com> wrote:
> > Hi Bhupesh,
> >
> > On 04/03/18 at 07:28pm, Bhupesh Sharma wrote:
> >> Hi James,
> >>
> >> Sorry for the delay, I had a long weekend last week.
> >>
> >> On Tue, Mar 27, 2018 at 7:01 PM, James Morse <james.morse@arm.com> wrote:
> >> > Hi Akashi, Bhupesh,
> >> >
> >> > On 27/03/18 10:04, AKASHI Takahiro wrote:
> >> >> On Mon, Mar 26, 2018 at 02:29:31PM +0530, Bhupesh Sharma wrote:
> >> >>> On Tue, Mar 20, 2018 at 12:36 AM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
> >> >>>> On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro
> >> >>>> <takahiro.akashi@linaro.org> wrote:
> >> >>>>> On Mon, Mar 19, 2018 at 04:05:38PM +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.
> >> >>>>>
> >> >>>>> Why not just save binary data to a file and interpret it
> >> >>>>> with dtc command?
> >> >
> >> > I'd prefer this too. It would also let us debug any issue where kexec-tools
> >> > produces an invalid DTB. It also lets us test booting the kernel from firmware
> >> > with that DTB.
> >>
> >> I captured the use case where it is not possible to do so. I have seen
> >> primary kernel crash before we can get to the command prompt to save
> >> the dtb blob. Since the arm64 crashkernel still seems to have issues
> >> itself while booting on acpi enabled machines (see
> >> <https://www.spinics.net/lists/arm-kernel/msg616632.html>), so we are
> >> trying to debug a problem which has two undefined variables :)
> >>
> >> >>>> Well, there are a couple of reasons for that which can be understood
> >> >>>> from a system which is in a production environment (for e.g. I have
> >> >>>> worked on one arm64 system in the past which used a yocto based
> >> >>>> distribution in which kexec -p was launched with a -d option as a part
> >> >>>> of initial ramfs scriptware):
> >> >
> >> > and panics before you get an interactive prompt or persistent storage? I think
> >> > this would be a pretty niche use-case. You could always base64-dump the dtb to
> >> > stdout from your script.
> >>
> >> That is pretty basic case on several new arm64 development boards
> >> (e.g. qualcomm, huawei etc) where we are debugging issues in primary
> >> kernel boot (and we are not even able to reach the command prompt).
> >>
> >> If the crashkernel crashes even before the primary kernel does because
> >> of the issues in the way DTB is passed to the crashkernel (which can
> >> include wrong DTB fields), we better have mechanisms to track the same
> >> rather than adding debug prints to the kernels.
> >
> > In this case, since userspace always need to run 'kexec -l', there
> > should be chance to save dtb and dump them in scripts if needed.
> > If this is doable in scripts I also tend not to add the code in kexec-tools.
> 
> Perhaps you missed my latest email which explained the use case better
> (please see [1]).
> Please note that I am talking about the 'kexec -p' or the kdump use
> case rather than the 'kexec -l' or the warm reboot to the second
> kernel.

It is a typo, I meant about 'kexec -p' as well

> 
> In several cases its useful if we are not able to reach the command
> prompt to just modify the scriptware to launch 'kexec -p' when the
> ramfs starts up so that we can catch crashes which happen before the
> primary kernel reaches the command prompt. For example, on freescale
> boards I found this scriptware mechanism quite useful on yocto based
> distributions, as you can be porting a new upstream kernel on the
> board and thus is not very stable while booting and the crashkernel
> also has issues which causes it to crash. In such cases although we
> see a panic message from the crashkernel it is very difficult to
> deduce that it was because of some wrongly fixed up dtb property.

Since kexec -p need to be automated in scripts, add something to dump
dtb is also doable.

> 
> Getting a debug log which contains the dtb dump is very useful in
> above cases for debugging.
> 
> Also saving the dtb requires 'dtc' tool to be installed which is an
> additional dependency.

I think it should be not a big problem,  dtc is designed to do this.

> 
> Since we have debug facility/messages already available when we
> execute 'kexec -p' with '-d' flag we can use the dtb dumps from the
> tool to debug crashes than happen in the crashkernel also because of
> wrong dtb fields fixed up by 'kexec-tools'. I was loathe to keep this
> patch locally and apply to the upstream 'kexec-tools' when debugging
> these issues as I think it makes sense to improve our debugging
> capabilities if we know there are issues around the same.

Hmm, do you have the example kexec-tools fix-up the dtb content?

> 
> Ok, let me resend this patch with a cover letter this time to explain
> the use case better and also to capture the dtb logs.
> 
> [1] https://marc.info/?l=kexec&m=152382169120505&w=2

Thanks
Dave

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

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

[Bhupesh Sharma]

On Tue, Apr 17, 2018 at 10:47 AM, Dave Young <dyoung@redhat.com> wrote:
> On 04/16/18 at 08:39pm, Bhupesh Sharma wrote:
>> Hi Dave,
>>
>> On Mon, Apr 16, 2018 at 1:43 PM, Dave Young <dyoung@redhat.com> wrote:
>> > Hi Bhupesh,
>> >
>> > On 04/03/18 at 07:28pm, Bhupesh Sharma wrote:
>> >> Hi James,
>> >>
>> >> Sorry for the delay, I had a long weekend last week.
>> >>
>> >> On Tue, Mar 27, 2018 at 7:01 PM, James Morse <james.morse@arm.com> wrote:
>> >> > Hi Akashi, Bhupesh,
>> >> >
>> >> > On 27/03/18 10:04, AKASHI Takahiro wrote:
>> >> >> On Mon, Mar 26, 2018 at 02:29:31PM +0530, Bhupesh Sharma wrote:
>> >> >>> On Tue, Mar 20, 2018 at 12:36 AM, Bhupesh Sharma <bhsharma@redhat.com> wrote:
>> >> >>>> On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro
>> >> >>>> <takahiro.akashi@linaro.org> wrote:
>> >> >>>>> On Mon, Mar 19, 2018 at 04:05:38PM +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.
>> >> >>>>>
>> >> >>>>> Why not just save binary data to a file and interpret it
>> >> >>>>> with dtc command?
>> >> >
>> >> > I'd prefer this too. It would also let us debug any issue where kexec-tools
>> >> > produces an invalid DTB. It also lets us test booting the kernel from firmware
>> >> > with that DTB.
>> >>
>> >> I captured the use case where it is not possible to do so. I have seen
>> >> primary kernel crash before we can get to the command prompt to save
>> >> the dtb blob. Since the arm64 crashkernel still seems to have issues
>> >> itself while booting on acpi enabled machines (see
>> >> <https://www.spinics.net/lists/arm-kernel/msg616632.html>), so we are
>> >> trying to debug a problem which has two undefined variables :)
>> >>
>> >> >>>> Well, there are a couple of reasons for that which can be understood
>> >> >>>> from a system which is in a production environment (for e.g. I have
>> >> >>>> worked on one arm64 system in the past which used a yocto based
>> >> >>>> distribution in which kexec -p was launched with a -d option as a part
>> >> >>>> of initial ramfs scriptware):
>> >> >
>> >> > and panics before you get an interactive prompt or persistent storage? I think
>> >> > this would be a pretty niche use-case. You could always base64-dump the dtb to
>> >> > stdout from your script.
>> >>
>> >> That is pretty basic case on several new arm64 development boards
>> >> (e.g. qualcomm, huawei etc) where we are debugging issues in primary
>> >> kernel boot (and we are not even able to reach the command prompt).
>> >>
>> >> If the crashkernel crashes even before the primary kernel does because
>> >> of the issues in the way DTB is passed to the crashkernel (which can
>> >> include wrong DTB fields), we better have mechanisms to track the same
>> >> rather than adding debug prints to the kernels.
>> >
>> > In this case, since userspace always need to run 'kexec -l', there
>> > should be chance to save dtb and dump them in scripts if needed.
>> > If this is doable in scripts I also tend not to add the code in kexec-tools.
>>
>> Perhaps you missed my latest email which explained the use case better
>> (please see [1]).
>> Please note that I am talking about the 'kexec -p' or the kdump use
>> case rather than the 'kexec -l' or the warm reboot to the second
>> kernel.
>
> It is a typo, I meant about 'kexec -p' as well
>
>>
>> In several cases its useful if we are not able to reach the command
>> prompt to just modify the scriptware to launch 'kexec -p' when the
>> ramfs starts up so that we can catch crashes which happen before the
>> primary kernel reaches the command prompt. For example, on freescale
>> boards I found this scriptware mechanism quite useful on yocto based
>> distributions, as you can be porting a new upstream kernel on the
>> board and thus is not very stable while booting and the crashkernel
>> also has issues which causes it to crash. In such cases although we
>> see a panic message from the crashkernel it is very difficult to
>> deduce that it was because of some wrongly fixed up dtb property.
>
> Since kexec -p need to be automated in scripts, add something to dump
> dtb is also doable.
>
>>
>> Getting a debug log which contains the dtb dump is very useful in
>> above cases for debugging.
>>
>> Also saving the dtb requires 'dtc' tool to be installed which is an
>> additional dependency.
>
> I think it should be not a big problem,  dtc is designed to do this.
>
>>
>> Since we have debug facility/messages already available when we
>> execute 'kexec -p' with '-d' flag we can use the dtb dumps from the
>> tool to debug crashes than happen in the crashkernel also because of
>> wrong dtb fields fixed up by 'kexec-tools'. I was loathe to keep this
>> patch locally and apply to the upstream 'kexec-tools' when debugging
>> these issues as I think it makes sense to improve our debugging
>> capabilities if we know there are issues around the same.
>
> Hmm, do you have the example kexec-tools fix-up the dtb content?

Yes, I have sent out the v2 patchset 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.

You can view the v2 here
<http://lists.infradead.org/pipermail/kexec/2018-April/020532.html>

Regards,
Bhupesh

>> Ok, let me resend this patch with a cover letter this time to explain
>> the use case better and also to capture the dtb logs.
>>
>> [1] https://marc.info/?l=kexec&m=152382169120505&w=2
>
> Thanks
> Dave

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