Re: [PATCH for-7.2 v2 10/20] hw/ppc: set machine->fdt in spapr machine

[Alexey Kardashevskiy]

On 22/08/2022 20:30, Daniel Henrique Barboza wrote:
> On 8/22/22 00:29, Alexey Kardashevskiy wrote:
> > On 22/08/2022 13:05, David Gibson wrote:
> > > On Fri, Aug 19, 2022 at 06:42:34AM -0300, Daniel Henrique Barboza wrote:
> > > > On 8/18/22 23:11, Alexey Kardashevskiy wrote:
> > > > > On 05/08/2022 19:39, Daniel Henrique Barboza wrote:
> > > > > > The pSeries machine never bothered with the common machine->fdt
> > > > > > attribute. We do all the FDT related work using spapr->fdt_blob.
> > > > > >
> > > > > > We're going to introduce HMP commands to read and save the FDT, which
> > > > > > will rely on setting machine->fdt properly to work across all machine
> > > > > > archs/types.
> > > > >
> > > > >
> > > > > Out of curiosity - why new HMP command, is not QOM'ing this ms::fdt 
> > > > > property enough?
> > > >
> > > > I tried to do the minimal changes needed for the commands to work. 
> > > > ms::fdt is
> > > > one of the few MachineState fields that hasn't been QOMified by
> > > > machine_class_init() yet. All pre-existing code that uses ms::fdt 
> > > > are using the
> > > > pointer directly. To make a QOMified use of it would require extra 
> > > > patches
> > > > in machine.c to QOMify the property first.
> > > >
> > > > There's also the issue with how each machine is creating the FDT. 
> > > > Most are using
> > > > helpers from device_tree.c, some are creating it from scratch, 
> > > > others required
> > > > a .dtb file, most of them are not doing a fdt_pack() and so on. To 
> > > > really QOMify
> > > > the use of ms::fdt we would need some machine hooks that standardize 
> > > > all that.
> > > > I believe it's worth the trouble, but it would be too much to do
> > > > right now.
> > >
> > > Hmm.. I think this depends on what you mean by "QOM"ify exactly.  If
> > > you're meaning make the full DT representation QOM objects, that you
> > > can look into in detail, then, yes, that's pretty complicated.
> > >
> > > I suspect what Alexey was suggesting though, was merely to make
> > > ms::fdt accessible as a single bytestring property on the machine QOM
> > > object.  Effectively it's just "dumpdtb" but as a property get.
> >
> >
> > Yes, I meant the bytestream, as DTC can easily decompile it onto a DTS.
> >
> >
> > > I'm not 100% certain if QOM can safely represent arbitrary bytestrings
> > > as QOM properties, which would need checking.
> >
> > I am not sure either but rather than adding another command to HMP, 
> > I'd explore this option first.
>
>
> I'm not sure what you mean by that. The HMP version of 'dumpdtb' is more 
> flexible
> that the current "-machine dumpdtb", an extra machine option that would 
> cause
> the guest to exit after writing the dtb

True. Especially with CAS :)

> And 'info fdt' is a new command 
> that
> makes it easier to inspect specific nodes/props.

btw what is this new command going to do? decompile the tree or save dtb?

> I don't see how making ms::fdt being retrievable by 
> object_property_get() internally
> (remember that ms::fdt it's not fully QOMified, so there's no 
> introspection of its
> value from the QEMU monitor) would make any of these new HMP commands 
> obsolete.

Well, there are QMP and HMP and my feeling was that HMP is slowly 
getting deprecated or something and QMP is the superior one. So I 
thought since this FDT is a property and there is no associated action 
with it, making it a property would do.

For ages I've been using a python3 script to talk to QMP as HMP is 
really quite limited, the only thing in HMP which is not in QMP is 
dumping memory ("x", "xp"), in this case I wrap HMP into QMP and keep 
using QMP :)


> Thanks,
>
>
> Daniel
>
> >

--
Alexey