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

[Daniel Henrique Barboza <danielhb413@gmail.com>]

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

> 
> 
>