Re: [PATCH for-7.2 v4 15/21] qmp/hmp, device_tree.c: introduce 'info fdt' command

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4510 bytes --]

On Tue, Aug 30, 2022 at 12:43:23PM +0200, Markus Armbruster wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > On Mon, Aug 29, 2022 at 07:00:55PM -0300, Daniel Henrique Barboza wrote:
> >> 
> >> 
> >> On 8/29/22 00:34, David Gibson wrote:
> >> > On Fri, Aug 26, 2022 at 11:11:44AM -0300, Daniel Henrique Barboza wrote:
> >> > > Reading the FDT requires that the user saves the fdt_blob and then use
> >> > > 'dtc' to read the contents. Saving the file and using 'dtc' is a strong
> >> > > use case when we need to compare two FDTs, but it's a lot of steps if
> >> > > you want to do quick check on a certain node or property.
> >> > > 
> >> > > 'info fdt' retrieves FDT nodes (and properties, later on) and print it
> >> > > to the user. This can be used to check the FDT on running machines
> >> > > without having to save the blob and use 'dtc'.
> >> > > 
> >> > > The implementation is based on the premise that the machine thas a FDT
> >> > > created using libfdt and pointed by 'machine->fdt'. As long as this
> >> > > pre-requisite is met the machine should be able to support it.
> >> > > 
> >> > > For now we're going to add the required QMP/HMP boilerplate and the
> >> > > capability of printing the name of the properties of a given node. Next
> >> > > patches will extend 'info fdt' to be able to print nodes recursively,
> >> > > and then individual properties.
> >> > > 
> >> > > This command will always be executed in-band (i.e. holding BQL),
> >> > > avoiding potential race conditions with machines that might change the
> >> > > FDT during runtime (e.g. PowerPC 'pseries' machine).
> >> > > 
> >> > > 'info fdt' is not something that we expect to be used aside from debugging,
> >> > > so we're implementing it in QMP as 'x-query-fdt'.
> >> > > 
> >> > > This is an example of 'info fdt' fetching the '/chosen' node of the
> >> > > pSeries machine:
> >> > > 
> >> > > (qemu) info fdt /chosen
> >> > > chosen {
> >> > >      ibm,architecture-vec-5;
> >> > >      rng-seed;
> >> > >      ibm,arch-vec-5-platform-support;
> >> > >      linux,pci-probe-only;
> >> > >      stdout-path;
> >> > >      linux,stdout-path;
> >> > >      qemu,graphic-depth;
> >> > >      qemu,graphic-height;
> >> > >      qemu,graphic-width;
> >> > > };
> >> > > 
> >> > > And the same node for the aarch64 'virt' machine:
> >> > > 
> >> > > (qemu) info fdt /chosen
> >> > > chosen {
> >> > >      stdout-path;
> >> > >      rng-seed;
> >> > >      kaslr-seed;
> >> > > };
> >> > 
> >> > So, I'm reasonably convinced allowing dumping the whole dtb from
> >> > qmp/hmp is useful.  I'm less convined that info fdt is worth the
> >> > additional complexity it incurs.  Note that as well as being able to
> >> > decompile a whole dtb using dtc, you can also extract and list
> >> > specific properties from a dtb blob using the 'fdtget' tool which is
> >> > part of the dtc tree.
> >> 
> >> What's your opinion on patch 21/21, where 'dumpdtb' can write a formatted
> >> FDT in a file with an extra option? That was possible because of the
> >> format helpers introduced for 'info fdt'. The idea is that since we're
> >> able to format a FDT in DTS format, we can also write the FDT in text
> >> format without relying on DTC to decode it.
> >
> > Since it's mostly the same code, I think it's reasonable to throw in
> > if the info fdt stuff is there, but I don't think it's worth including
> > without that.  As a whole, I remain dubious that (info fdt + dumpdts)
> > is worth the complexity cost.
> 
> How much code does it take, and who's going to maintain it?

It's not especially big, but it's not negligible.  Perhaps the part
that I'm most uncomfortable about is that it requires a bunch of messy
heuristics to guess how to format the output - DT properties are just
bytestrings, any internal interpretation is based on the specific
bindings for them.

dtc already has these and I don't love having a second, potentially
different copy of necessarily imperfect heuristics out in the wild.

> > People with more practical experience debugging the embedded ARM
> > platforms might have a different opinion if they thing info fdt would
> > be really useful though.
> 
> They better speak up then :)

Just so.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]