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

[David Gibson]

On Mon, Aug 22, 2022 at 07:30:36AM -0300, 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. 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.

I believe what we were thinking is if the dtb (as a single bytestring) can be
retrieved with a qom-get on a suitable property on the machine, that
might make things marginally simpler than adding a new command.  I'm
not certain if the JSON format of the QMP responses can safely encode
an arbitrary bytestring, though (as opoosed to a Unicode string).

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

signature.asc
Description: PGP signature