Re: [PATCH 1/2] xen/device-tree: add a helper to dump an FDT in DTS-like format

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 10/8/26 1:57 PM, Orzel, Michal wrote:
> On 01-Oct-26 13:16, Oleksii Kurochko wrote:
> > The device tree Xen builds for a dom0less domain is only ever consumed
> > by the guest, so a mistake in it surfaces as an obscure failure inside
> > the domain, far away from the code which produced the tree.
> >
> > Add device_tree_dump(). The output follows dtc's format so that a dump
> > can be diffed against the source DTS or fed back to dtc; the value
> > formatting heuristics are ported from dtc's utilfdt_print_data() for
> > that reason. The walk itself goes through libfdt rather than decoding
> > the tag stream by hand as dtc's fdtdump does.
> >
> > Dumping a whole tree is only useful while debugging and costs both code
> > size and a lot of console output, hence CONFIG_DEVICE_TREE_DEBUG. Mention
> > the dump in the help text of the option.
> We should also dump the DTB for dom0/hwdom in prepare_dtb_hwdom(). Please add
> that to this patch.
It makes sense to me. I will add it.
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > ---
> >   xen/Kconfig.debug                       |   3 +-
> >   xen/common/device-tree/Makefile         |   1 +
> >   xen/common/device-tree/dom0less-build.c |   4 +
> >   xen/common/device-tree/fdt-dump.c       | 180 ++++++++++++++++++++++++
> >   xen/include/xen/bootfdt.h               |  13 ++
> >   5 files changed, 200 insertions(+), 1 deletion(-)
> >   create mode 100644 xen/common/device-tree/fdt-dump.c
> >
> > diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> > index d900d926c555..664f50bf8a96 100644
> > --- a/xen/Kconfig.debug
> > +++ b/xen/Kconfig.debug
> > @@ -95,7 +95,8 @@ config DEVICE_TREE_DEBUG
> >         depends on DEVICE_TREE_PARSE
> >         help
> >           Device tree parsing and DOM0 device tree building messages are
> > -         logged in the Xen ring buffer.
> > +         logged in the Xen ring buffer. The device tree generated for each
> > +         dom0less domU is dumped there as well, in a DTS-like format.
> I think we could have a separate option DEVICE_TREE_DUMP that depends on
> DEVICE_TREE_PARSE. Sometimes all we need is just to see how the generated DTB
> look like without seeing thousands of other DT debug messages. Especially that
> nothing in fdt-dump.c depends on DEVICE_TREE_DEBUG.
Agree, it will be better to have DEVICE_TREE_DUMP. I will introduce the 
following config:
config DEVICE_TREE_DUMP
  bool "Dump generated device trees"
  depends on DEVICE_TREE_PARSE
  help
    The device tree Xen generates for dom0/hwdom and for each om0less
    domain is printed on the Xen console in a DTS-like format.
    The dump is logged at debug level, so on a non-debug build it is
    only shown with "loglvl=all".
    If unsure, say N here.


> >           If unsure, say N here.
> >   
> >   config SCRUB_DEBUG
> > diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
> > index 9036e455d66a..959dd2a9fd30 100644
> > --- a/xen/common/device-tree/Makefile
> > +++ b/xen/common/device-tree/Makefile
> > @@ -6,6 +6,7 @@ obj-$(CONFIG_DOMAIN_BUILD_HELPERS) += domain-build.init.o
> >   obj-$(filter $(CONFIG_DOM0LESS_BOOT),$(CONFIG_HAS_DEVICE_TREE_DISCOVERY)) += 
> > dom0less-build.init.o
> >   obj-$(CONFIG_DOM0LESS_BOOT) += dom0less-bindings.init.o
> >   obj-$(CONFIG_OVERLAY_DTB) += dt-overlay.o
> > +obj-$(CONFIG_DEVICE_TREE_DEBUG) += fdt-dump.init.o
> >   obj-$(CONFIG_HAS_DEVICE_TREE_DISCOVERY) += intc.o
> >   obj-$(CONFIG_DOMAIN_BUILD_HELPERS) += kernel.o
> >   obj-$(CONFIG_STATIC_EVTCHN) += static-evtchn.init.o
> > diff --git a/xen/common/device-tree/dom0less-build.c 
> > b/xen/common/device-tree/dom0less-build.c
> > index fcbeb8adbd73..182201384faf 100644
> > --- a/xen/common/device-tree/dom0less-build.c
> > +++ b/xen/common/device-tree/dom0less-build.c
> > @@ -1,5 +1,6 @@
> >   /* SPDX-License-Identifier: GPL-2.0-only */
> >   
> > +#include <xen/bootfdt.h>
> >   #include <xen/bootinfo.h>
> >   #include <xen/device_tree.h>
> >   #include <xen/dom0less-build.h>
> > @@ -587,6 +588,9 @@ static int __init prepare_dtb_domU(struct domain *d, struct 
> > kernel_info *kinfo)
> >       if ( ret < 0 )
> >           goto err;
> >   
> > +    dt_dprintk("Device tree for %pd:\n", d);
> Move this message to device_tree_dump.
I'll move.

> > +    device_tree_dump(kinfo->fdt);
> > +
> >       return 0;
> >   
> >     err:
> > diff --git a/xen/common/device-tree/fdt-dump.c 
> > b/xen/common/device-tree/fdt-dump.c
> > new file mode 100644
> > index 000000000000..fc3d9eca5891
> > --- /dev/null
> > +++ b/xen/common/device-tree/fdt-dump.c
> > @@ -0,0 +1,180 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Dump a flattened device tree blob in a DTS-like format.
> > + *
> > + * is_printable_string() and print_value() are derived from
> > + * util_is_printable_string() and utilfdt_print_data() in the device tree
> > + * compiler (dtc).
> > + *
> > + * Copyright 2011 The Chromium Authors, All Rights Reserved.
> > + * Copyright 2008 Jon Loeliger, Freescale Semiconductor, Inc.
> > + * util_is_printable_string contributed by
> > + *      Pantelis Antoniou <pantelis.antoniou AT gmail.com>
> > + */
> > +
> > +#include <xen/bootfdt.h>
> > +#include <xen/ctype.h>
> > +#include <xen/init.h>
> > +#include <xen/lib.h>
> > +#include <xen/libfdt/libfdt.h>
> > +#include <xen/types.h>
> > +
> > +/* Deeper nodes are still printed, but not indented any further. */
> > +#define DUMP_MAX_INDENT_DEPTH 32
> > +
> > +/* Indentation, in characters, of a node at depth @depth. */
> > +static int __init indent_of(int depth)
> > +{
> > +    return 2 * min(depth, DUMP_MAX_INDENT_DEPTH);
> > +}
> > +
> > +static bool __init is_printable_string(const void *data, unsigned int len)
> > +{
> > +    const char *s = data;
> > +    const char *se = s + len;
> > +
> > +    /* A zero length property, or one which isn't NUL terminated, isn't one. */
> > +    if ( (len == 0) || (s[len - 1] != '\0') )
> > +        return false;
> > +
> > +    while ( s < se )
> > +    {
> > +        const char *ss = s;
> > +
> > +        while ( (s < se) && *s && isprint(*s) )
> Xen's is_print() behaves differently than what dtc uses. You need to combine it
> with is_ascii. Otherwise 0xff would be marked as printable.
I'll add the is_ascii() before isprint():
     /*
      * Xen's isprint() also accepts the Latin-1 characters 0xa0-0xff,
      * while the one dtc uses only accepts ASCII ones.
      */
     while ( (s < se) && *s && isascii(*s) && isprint(*s) )
        ...

Thanks for review!

~ Oleksii