Re: [PATCH for-7.2 v2 14/20] qmp/hmp, device_tree.c: introduce dumpdtb

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

On 8/8/22 00:30, David Gibson wrote:
> On Fri, Aug 05, 2022 at 06:39:42AM -0300, Daniel Henrique Barboza wrote:
>> To save the FDT blob we have the '-machine dumpdtb=<file>' property.
>> With this property set, the machine saves the FDT in <file> and exit.
>> The created file can then be converted to plain text dts format using
>> 'dtc'.
>>
>> There's nothing particularly sophisticated into saving the FDT that
>> can't be done with the machine at any state, as long as the machine has
>> a valid FDT to be saved.
>>
>> The 'dumpdtb' command receives a 'filename' paramenter and, if a valid
>> FDT is available, it'll save it in a file 'filename'. In short, this is
>> a '-machine dumpdtb' that can be fired on demand via QMP/HMP.
>>
>> A valid FDT consists of a FDT that was created using libfdt being
>> retrieved via 'current_machine->fdt' in device_tree.c. This condition is
>> met by most FDT users in QEMU.
> 
> On spapr at least, the fdt can change at runtime (due to hotplugs).
> So we need to think about concurrency between fdt updates and dumping
> it with this command.  I'm assuming it's protected by the BQL, but I'm
> wondering if we should outright state that somewhere for clarity.

Your assumption is right. It is protected by BQL since it's always executed
in-band. To not hold BQL we would need to declare an extra flag "allow-oob"
to execute the command out-of-band.

I'll mention in the commit msg that we're holding BQL. I'll do the same for
the 'info fdt' commit msg.


Thanks,


Daniel


> 
>>
>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   hmp-commands.hx              | 13 +++++++++++++
>>   include/monitor/hmp.h        |  1 +
>>   include/sysemu/device_tree.h |  1 +
>>   monitor/hmp-cmds.c           | 12 ++++++++++++
>>   monitor/qmp-cmds.c           | 13 +++++++++++++
>>   qapi/machine.json            | 17 +++++++++++++++++
>>   softmmu/device_tree.c        | 18 ++++++++++++++++++
>>   7 files changed, 75 insertions(+)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 182e639d14..d2554e9701 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1800,3 +1800,16 @@ ERST
>>                         "\n\t\t\t\t\t limit on a specified virtual cpu",
>>           .cmd        = hmp_cancel_vcpu_dirty_limit,
>>       },
>> +
>> +SRST
>> +``dumpdtb`` *filename*
>> +  Save the FDT in the 'filename' file to be decoded using dtc.
>> +  Requires 'libfdt' support.
>> +ERST
>> +    {
>> +        .name       = "dumpdtb",
>> +        .args_type  = "filename:s",
>> +        .params     = "[filename] file to save the FDT",
>> +        .help       = "save the FDT in the 'filename' file to be decoded using dtc",
>> +        .cmd        = hmp_dumpdtb,
>> +    },
>> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
>> index a618eb1e4e..d7f324da59 100644
>> --- a/include/monitor/hmp.h
>> +++ b/include/monitor/hmp.h
>> @@ -134,6 +134,7 @@ void hmp_calc_dirty_rate(Monitor *mon, const QDict *qdict);
>>   void hmp_set_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
>>   void hmp_cancel_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
>>   void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
>> +void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
>>   void hmp_human_readable_text_helper(Monitor *mon,
>>                                       HumanReadableText *(*qmp_handler)(Error **));
>>   void hmp_info_stats(Monitor *mon, const QDict *qdict);
>> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
>> index ef060a9759..bf7684e4ed 100644
>> --- a/include/sysemu/device_tree.h
>> +++ b/include/sysemu/device_tree.h
>> @@ -136,6 +136,7 @@ int qemu_fdt_add_path(void *fdt, const char *path);
>>       } while (0)
>>   
>>   void qemu_fdt_dumpdtb(void *fdt, int size);
>> +void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp);
>>   
>>   /**
>>    * qemu_fdt_setprop_sized_cells_from_array:
>> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>> index c6cd6f91dd..d23ec85f9d 100644
>> --- a/monitor/hmp-cmds.c
>> +++ b/monitor/hmp-cmds.c
>> @@ -2472,3 +2472,15 @@ exit:
>>   exit_no_print:
>>       error_free(err);
>>   }
>> +
>> +void hmp_dumpdtb(Monitor *mon, const QDict *qdict)
>> +{
>> +    const char *filename = qdict_get_str(qdict, "filename");
>> +    Error *local_err = NULL;
>> +
>> +    qmp_dumpdtb(filename, &local_err);
>> +
>> +    if (local_err) {
>> +        error_report_err(local_err);
>> +    }
>> +}
>> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
>> index 7314cd813d..8415aca08c 100644
>> --- a/monitor/qmp-cmds.c
>> +++ b/monitor/qmp-cmds.c
>> @@ -45,6 +45,7 @@
>>   #include "hw/intc/intc.h"
>>   #include "hw/rdma/rdma.h"
>>   #include "monitor/stats.h"
>> +#include "sysemu/device_tree.h"
>>   
>>   NameInfo *qmp_query_name(Error **errp)
>>   {
>> @@ -596,3 +597,15 @@ bool apply_str_list_filter(const char *string, strList *list)
>>       }
>>       return false;
>>   }
>> +
>> +#ifdef CONFIG_FDT
>> +void qmp_dumpdtb(const char *filename, Error **errp)
>> +{
>> +    return qemu_fdt_qmp_dumpdtb(filename, errp);
>> +}
>> +#else
>> +void qmp_dumpdtb(const char *filename, Error **errp)
>> +{
>> +    error_setg(errp, "dumpdtb requires libfdt");
>> +}
>> +#endif
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 6afd1936b0..aeb013f3dd 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -1664,3 +1664,20 @@
>>        '*size': 'size',
>>        '*max-size': 'size',
>>        '*slots': 'uint64' } }
>> +
>> +##
>> +# @dumpdtb:
>> +#
>> +# Save the FDT in dtb format. Requires 'libfdt' support.
>> +#
>> +# @filename: name of the FDT file to be created
>> +#
>> +# Since: 7.2
>> +#
>> +# Example:
>> +#   {"execute": "dumpdtb"}
>> +#    "arguments": { "filename": "/tmp/fdt.dtb" } }
>> +#
>> +##
>> +{ 'command': 'dumpdtb',
>> +  'data': { 'filename': 'str' } }
>> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
>> index 6ca3fad285..cd487ddd4d 100644
>> --- a/softmmu/device_tree.c
>> +++ b/softmmu/device_tree.c
>> @@ -643,3 +643,21 @@ out:
>>       g_free(propcells);
>>       return ret;
>>   }
>> +
>> +void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp)
>> +{
>> +    int size;
>> +
>> +    if (!current_machine->fdt) {
>> +        error_setg(errp, "Unable to find the machine FDT");
>> +        return;
>> +    }
>> +
>> +    size = fdt_totalsize(current_machine->fdt);
>> +
>> +    if (g_file_set_contents(filename, current_machine->fdt, size, NULL)) {
>> +        return;
>> +    }
>> +
>> +    error_setg(errp, "Error when saving machine FDT to file %s", filename);
>> +}
>