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

[Daniel Henrique Barboza]

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. And 'info fdt' is a new command that
makes it easier to inspect specific nodes/props.

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.


Thanks,


Daniel

>