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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux