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

[Markus Armbruster]

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?

> 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 :)

>> If we think that this 'dumpdtb' capability is worth having, I can respin
>> the patches without 'info fdt' but adding these helpers to enable this
>> 'dumpdtb' support. If not, then we can just remove patches 15-21 and
>> be done with it.
>> 
>> 
>> Thanks,
>> 
>> 
>> Daniel