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@xxxxxxxxxx> wrote: > > On Mon, Mar 19, 2018 at 8:15 PM, AKASHI Takahiro > > <takahiro.akashi@xxxxxxxxxx> 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@xxxxxxxxxx> > >>> --- > >>> 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@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec
