linux-kernel - Re: [RFC/PATCH] drivers/of: add debug for early dump of the dtb strcutrue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <cd6ed5b1-619c-4ca9-8fe0-6b47c7d641a7@codethink.co.uk>
Date: Mon, 11 Aug 2025 09:02:34 +0100
From: Ben Dooks <ben.dooks@...ethink.co.uk>
To: Saravana Kannan <saravanak@...gle.com>, Rob Herring <robh@...nel.org>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC/PATCH] drivers/of: add debug for early dump of the dtb
 strcutrue

On 08/08/2025 22:10, Saravana Kannan wrote:
> On Fri, Aug 8, 2025 at 9:25 AM Rob Herring <robh@...nel.org> wrote:
>>
>> On Fri, Aug 8, 2025 at 9:26 AM Ben Dooks <ben.dooks@...ethink.co.uk> wrote:
>>>
>>> When testing for boot issues, it was helpful to dump the
>>> list of nodes and properties in the device-tree passed into
>>> the kernel.
>>
>> Shouldn't the bootloader be able to dump that?
>>
>>> Add CONFIG_OF_EARLY_DUMP option to dump the list of nodes
>>> and properties to the standard console output early in the
>>> boot sequence. Note, you may need to have some sort of
>>
>> s/may/will/
>>
>>> early or debug console output if there are issues stopping
>>> the kernel starting properly.
>>
>> Seems to me this is giving the user the haystack to find the needle...
> 
> Completely agree with Rob.
> 
> Ben, can you give more context on what kind of issues this has helped
> you (or anticipate it will) solve? Maybe there are better ways of
> getting what you need.

We where having issues with u-boot on big-endian riscv.

turns out the string functions had issues with endian-ness and
where corrupting the dtb when doing the final changes when booting
into the kernel.

the kernel wouldn;t boot as sometimes depending on data alignment
the #size-cells and #address-cells where having their names corrupted
and thus the kernel would bail very early.

> -Saravana
> 
>>
>>> Signed-off-by: Ben Dooks <ben.dooks@...ethink.co.uk>
>>> ---
>>>   drivers/of/Kconfig |  8 ++++++++
>>>   drivers/of/fdt.c   | 39 +++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 47 insertions(+)
>>>
>>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>>> index 50697cc3b07e..ed2c52c54a7d 100644
>>> --- a/drivers/of/Kconfig
>>> +++ b/drivers/of/Kconfig
>>> @@ -126,4 +126,12 @@ config OF_OVERLAY_KUNIT_TEST
>>>   config OF_NUMA
>>>          bool
>>>
>>> +config OF_EARLY_DUMP
>>> +       bool "Dump node list at startup"
>>
>> This needs to depend on OF_EARLY_FLATTREE.
>>
>>> +       help
>>> +         This debug feature runs through all the nodes/properties at startup
>>> +         to show if the dtb has been passed correctly from the boot stage.
>>> +
>>> +         If unsure, say N here
>>> +
>>>   endif # OF
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 0edd639898a6..ab40db0e71a5 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -1164,6 +1164,43 @@ static void * __init early_init_dt_alloc_memory_arch(u64 size, u64 align)
>>>          return memblock_alloc_or_panic(size, align);
>>>   }
>>>
>>> +#ifdef CONFIG_OF_EARLY_DUMP
>>> +static int __init early_init_iterate_nodes(unsigned long node,
>>> +                                          const char *uname,
>>> +                                          int depth, void *data)
>>> +{
>>> +       void *blob = initial_boot_params;
>>> +       int cur;
>>> +
>>> +       pr_info("node '%s', depth %d\n", uname, depth);
>>
>> Can you add indentation for the depth rather than printing the depth?
>>
>> I'm not completely sure if calling this is safe always. How early this
>> will run depends on the architecture. So need to test on a variety of
>> architectures. Or perhaps limit in kconfig to tested architectures.
>> I'd rather not do that though.
>>
>>> +
>>> +       for (cur = fdt_first_property_offset(blob, node);
>>> +            cur >= 0;
>>> +            cur = fdt_next_property_offset(blob, cur)) {
>>> +               const char *pname;
>>> +               const __be32 *val;
>>> +               u32 sz;
>>> +
>>> +               val = fdt_getprop_by_offset(blob, cur, &pname, &sz);
>>> +               if (!val) {
>>> +                       pr_warn(" Cannot locate property at 0x%x\n", cur);
>>> +                       continue;
>>
>> If this fails, you should probably just stop and bail out.
>>
>>> +               }
>>> +
>>> +               pr_info("node %s: property %s\n", uname, pname ? pname : "unnamed");
>>
>> Can unnamed actually happen?
>>
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static inline void early_init_dump_dt(void)
>>> +{
>>> +       of_scan_flat_dt(early_init_iterate_nodes, NULL);
>>
>> This way to iterate is left over from before having libfdt. Is there
>> not a libfdt way to iterate thru all nodes?
>>
>>> +}
>>> +#else
>>> +static inline void early_init_dump_dt(void) { }
>>> +#endif /* CONFIG_OF_EARLY_DUMP */
>>> +
>>>   bool __init early_init_dt_verify(void *dt_virt, phys_addr_t dt_phys)
>>>   {
>>>          if (!dt_virt)
>>> @@ -1178,6 +1215,8 @@ bool __init early_init_dt_verify(void *dt_virt, phys_addr_t dt_phys)
>>>          initial_boot_params_pa = dt_phys;
>>>          of_fdt_crc32 = crc32_be(~0, initial_boot_params,
>>>                                  fdt_totalsize(initial_boot_params));
>>> +
>>> +       early_init_dump_dt();
>>
>> Use "if (IS_ENABLED(CONFIG_OF_EARLY_DUMP))" here instead of #ifdef.
>>
>>>
>>>          /* Initialize {size,address}-cells info */
>>>          early_init_dt_scan_root();
>>> --
>>> 2.37.2.352.g3c44437643
>>>
> 


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html