| Message ID | 20220904233456.209027-15-danielhb413@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | QMP/HMP: introduce 'dumpdtb' | expand |
Daniel Henrique Barboza <danielhb413@gmail.com> writes: > 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. > > This command will always be executed in-band (i.e. holding BQL), > avoiding potential race conditions with machines that might change the > FDT during runtime (e.g. PowerPC 'pseries' machine). > > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Alistair Francis <alistair.francis@wdc.com> > Cc: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > hmp-commands.hx | 15 +++++++++++++++ > include/sysemu/device_tree.h | 1 + > monitor/misc.c | 1 + > qapi/machine.json | 18 ++++++++++++++++++ > softmmu/device_tree.c | 31 +++++++++++++++++++++++++++++++ > 5 files changed, 66 insertions(+) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 182e639d14..9a3e57504f 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1800,3 +1800,18 @@ ERST > "\n\t\t\t\t\t limit on a specified virtual cpu", > .cmd = hmp_cancel_vcpu_dirty_limit, > }, > + > +#if defined(CONFIG_FDT) > +SRST > +``dumpdtb`` *filename* > + Save the FDT in the 'filename' file to be decoded using dtc. > + Requires 'libfdt' support. Does the #if suppress the documentation snippet when !defined(CONFIG_FDT)? If yes, the "Requires" line is redundant. Other conditional commands don't have such lines. > +ERST > + { > + .name = "dumpdtb", > + .args_type = "filename:F", > + .params = "filename", > + .help = "save the FDT in the 'filename' file to be decoded using dtc", > + .cmd = hmp_dumpdtb, > + }, > +#endif > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h > index ef060a9759..e7c5441f56 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 hmp_dumpdtb(Monitor *mon, const QDict *qdict); > > /** > * qemu_fdt_setprop_sized_cells_from_array: > diff --git a/monitor/misc.c b/monitor/misc.c > index 3d2312ba8d..e7dd63030b 100644 > --- a/monitor/misc.c > +++ b/monitor/misc.c > @@ -49,6 +49,7 @@ > #include "sysemu/blockdev.h" > #include "sysemu/sysemu.h" > #include "sysemu/tpm.h" > +#include "sysemu/device_tree.h" > #include "qapi/qmp/qdict.h" > #include "qapi/qmp/qerror.h" > #include "qapi/qmp/qstring.h" > diff --git a/qapi/machine.json b/qapi/machine.json > index 6afd1936b0..f968a5d343 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1664,3 +1664,21 @@ > '*size': 'size', > '*max-size': 'size', > '*slots': 'uint64' } } > + > +## > +# @dumpdtb: > +# > +# Save the FDT in dtb format. Requires 'libfdt' support. We rarely document conditionals like this. It's kind of redundant with the If CONFIG_FDT part in the generated documentation. Aside: this "If" part could certainly be improved. > +# > +# @filename: name of the FDT file to be created > +# > +# Since: 7.2 > +# > +# Example: > +# {"execute": "dumpdtb"} > +# "arguments": { "filename": "/tmp/fdt.dtb" } } Excuse my nitpicking... Creating files with a fixed filename in /tmp is a bad habit, and this example promotes it. We have similar examples elsewhere. Still, I'd prefer not to add more. > +# > +## > +{ 'command': 'dumpdtb', > + 'data': { 'filename': 'str' }, > + 'if': 'CONFIG_FDT' } > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > index 6ca3fad285..cdd41b6de6 100644 > --- a/softmmu/device_tree.c > +++ b/softmmu/device_tree.c > @@ -26,6 +26,9 @@ > #include "hw/loader.h" > #include "hw/boards.h" > #include "qemu/config-file.h" > +#include "qapi/qapi-commands-machine.h" > +#include "qapi/qmp/qdict.h" > +#include "monitor/hmp.h" > > #include <libfdt.h> > > @@ -643,3 +646,31 @@ out: > g_free(propcells); > return ret; > } > + > +void qmp_dumpdtb(const char *filename, Error **errp) > +{ > + g_autoptr(GError) err = NULL; > + int size; > + > + if (!current_machine->fdt) { > + error_setg(errp, "Unable to find the machine FDT"); Perhaps: "this machine doesn't have an FDT". > + return; > + } > + > + size = fdt_totalsize(current_machine->fdt); > + > + if (!g_file_set_contents(filename, current_machine->fdt, size, &err)) { > + error_setg(errp, "Error saving FDT to file %s: %s", > + filename, err->message); > + } > +} > + > +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); > + > + hmp_handle_error(mon, local_err); > +}
On 9/5/22 10:41, Markus Armbruster wrote: > Daniel Henrique Barboza <danielhb413@gmail.com> writes: > >> 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. >> >> This command will always be executed in-band (i.e. holding BQL), >> avoiding potential race conditions with machines that might change the >> FDT during runtime (e.g. PowerPC 'pseries' machine). >> >> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> >> Cc: Markus Armbruster <armbru@redhat.com> >> Cc: Alistair Francis <alistair.francis@wdc.com> >> Cc: David Gibson <david@gibson.dropbear.id.au> >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> --- >> hmp-commands.hx | 15 +++++++++++++++ >> include/sysemu/device_tree.h | 1 + >> monitor/misc.c | 1 + >> qapi/machine.json | 18 ++++++++++++++++++ >> softmmu/device_tree.c | 31 +++++++++++++++++++++++++++++++ >> 5 files changed, 66 insertions(+) >> >> diff --git a/hmp-commands.hx b/hmp-commands.hx >> index 182e639d14..9a3e57504f 100644 >> --- a/hmp-commands.hx >> +++ b/hmp-commands.hx >> @@ -1800,3 +1800,18 @@ ERST >> "\n\t\t\t\t\t limit on a specified virtual cpu", >> .cmd = hmp_cancel_vcpu_dirty_limit, >> }, >> + >> +#if defined(CONFIG_FDT) >> +SRST >> +``dumpdtb`` *filename* >> + Save the FDT in the 'filename' file to be decoded using dtc. >> + Requires 'libfdt' support. > > Does the #if suppress the documentation snippet when > !defined(CONFIG_FDT)? > > If yes, the "Requires" line is redundant. > > Other conditional commands don't have such lines. I tested using a target that doesn't rely on libfdt (in this case I picked the tricore softmmu) and disabled FDT support in ./configure: configure --target-list=tricore-softmmu --enable-fdt=disabled This didn't prevent the 'dumpdtb' section from appearing in the generated docs at build/docs/manual/system/monitor.html. So I guess the documentation isn't being suppressed by 'CONFIG_FDT'. That said, I can remove the "Requires libfdt" observations to be in line with other commands, and then we can then discuss in separate whether the documentation should follow these preprocessing macros. Thanks, Daniel > >> +ERST >> + { >> + .name = "dumpdtb", >> + .args_type = "filename:F", >> + .params = "filename", >> + .help = "save the FDT in the 'filename' file to be decoded using dtc", >> + .cmd = hmp_dumpdtb, >> + }, >> +#endif >> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h >> index ef060a9759..e7c5441f56 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 hmp_dumpdtb(Monitor *mon, const QDict *qdict); >> >> /** >> * qemu_fdt_setprop_sized_cells_from_array: >> diff --git a/monitor/misc.c b/monitor/misc.c >> index 3d2312ba8d..e7dd63030b 100644 >> --- a/monitor/misc.c >> +++ b/monitor/misc.c >> @@ -49,6 +49,7 @@ >> #include "sysemu/blockdev.h" >> #include "sysemu/sysemu.h" >> #include "sysemu/tpm.h" >> +#include "sysemu/device_tree.h" >> #include "qapi/qmp/qdict.h" >> #include "qapi/qmp/qerror.h" >> #include "qapi/qmp/qstring.h" >> diff --git a/qapi/machine.json b/qapi/machine.json >> index 6afd1936b0..f968a5d343 100644 >> --- a/qapi/machine.json >> +++ b/qapi/machine.json >> @@ -1664,3 +1664,21 @@ >> '*size': 'size', >> '*max-size': 'size', >> '*slots': 'uint64' } } >> + >> +## >> +# @dumpdtb: >> +# >> +# Save the FDT in dtb format. Requires 'libfdt' support. > > We rarely document conditionals like this. It's kind of redundant with > the > > If > > CONFIG_FDT > > part in the generated documentation. Aside: this "If" part could > certainly be improved. > >> +# >> +# @filename: name of the FDT file to be created >> +# >> +# Since: 7.2 >> +# >> +# Example: >> +# {"execute": "dumpdtb"} >> +# "arguments": { "filename": "/tmp/fdt.dtb" } } > > Excuse my nitpicking... Creating files with a fixed filename in /tmp is > a bad habit, and this example promotes it. We have similar examples > elsewhere. Still, I'd prefer not to add more. > >> +# >> +## >> +{ 'command': 'dumpdtb', >> + 'data': { 'filename': 'str' }, >> + 'if': 'CONFIG_FDT' } >> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c >> index 6ca3fad285..cdd41b6de6 100644 >> --- a/softmmu/device_tree.c >> +++ b/softmmu/device_tree.c >> @@ -26,6 +26,9 @@ >> #include "hw/loader.h" >> #include "hw/boards.h" >> #include "qemu/config-file.h" >> +#include "qapi/qapi-commands-machine.h" >> +#include "qapi/qmp/qdict.h" >> +#include "monitor/hmp.h" >> >> #include <libfdt.h> >> >> @@ -643,3 +646,31 @@ out: >> g_free(propcells); >> return ret; >> } >> + >> +void qmp_dumpdtb(const char *filename, Error **errp) >> +{ >> + g_autoptr(GError) err = NULL; >> + int size; >> + >> + if (!current_machine->fdt) { >> + error_setg(errp, "Unable to find the machine FDT"); > > Perhaps: "this machine doesn't have an FDT". > >> + return; >> + } >> + >> + size = fdt_totalsize(current_machine->fdt); >> + >> + if (!g_file_set_contents(filename, current_machine->fdt, size, &err)) { >> + error_setg(errp, "Error saving FDT to file %s: %s", >> + filename, err->message); >> + } >> +} >> + >> +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); >> + >> + hmp_handle_error(mon, local_err); >> +} >
* Daniel Henrique Barboza (danielhb413@gmail.com) 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. > > This command will always be executed in-band (i.e. holding BQL), > avoiding potential race conditions with machines that might change the > FDT during runtime (e.g. PowerPC 'pseries' machine). > > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Alistair Francis <alistair.francis@wdc.com> > Cc: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > hmp-commands.hx | 15 +++++++++++++++ > include/sysemu/device_tree.h | 1 + > monitor/misc.c | 1 + > qapi/machine.json | 18 ++++++++++++++++++ > softmmu/device_tree.c | 31 +++++++++++++++++++++++++++++++ > 5 files changed, 66 insertions(+) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 182e639d14..9a3e57504f 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1800,3 +1800,18 @@ ERST > "\n\t\t\t\t\t limit on a specified virtual cpu", > .cmd = hmp_cancel_vcpu_dirty_limit, > }, > + > +#if defined(CONFIG_FDT) > +SRST > +``dumpdtb`` *filename* > + Save the FDT in the 'filename' file to be decoded using dtc. > + Requires 'libfdt' support. > +ERST Can you please put the SRST/ERST below the { }'s please; most of the hmp-commands.hx has it that way around but it looks like the diry rate/limit set at the bottom have them flipped which we need to fix. > + { > + .name = "dumpdtb", > + .args_type = "filename:F", > + .params = "filename", > + .help = "save the FDT in the 'filename' file to be decoded using dtc", > + .cmd = hmp_dumpdtb, > + }, > +#endif Other than the flip, Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h > index ef060a9759..e7c5441f56 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 hmp_dumpdtb(Monitor *mon, const QDict *qdict); > > /** > * qemu_fdt_setprop_sized_cells_from_array: > diff --git a/monitor/misc.c b/monitor/misc.c > index 3d2312ba8d..e7dd63030b 100644 > --- a/monitor/misc.c > +++ b/monitor/misc.c > @@ -49,6 +49,7 @@ > #include "sysemu/blockdev.h" > #include "sysemu/sysemu.h" > #include "sysemu/tpm.h" > +#include "sysemu/device_tree.h" > #include "qapi/qmp/qdict.h" > #include "qapi/qmp/qerror.h" > #include "qapi/qmp/qstring.h" > diff --git a/qapi/machine.json b/qapi/machine.json > index 6afd1936b0..f968a5d343 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1664,3 +1664,21 @@ > '*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' }, > + 'if': 'CONFIG_FDT' } > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > index 6ca3fad285..cdd41b6de6 100644 > --- a/softmmu/device_tree.c > +++ b/softmmu/device_tree.c > @@ -26,6 +26,9 @@ > #include "hw/loader.h" > #include "hw/boards.h" > #include "qemu/config-file.h" > +#include "qapi/qapi-commands-machine.h" > +#include "qapi/qmp/qdict.h" > +#include "monitor/hmp.h" > > #include <libfdt.h> > > @@ -643,3 +646,31 @@ out: > g_free(propcells); > return ret; > } > + > +void qmp_dumpdtb(const char *filename, Error **errp) > +{ > + g_autoptr(GError) err = NULL; > + 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, &err)) { > + error_setg(errp, "Error saving FDT to file %s: %s", > + filename, err->message); > + } > +} > + > +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); > + > + hmp_handle_error(mon, local_err); > +} > -- > 2.37.2 >
diff --git a/hmp-commands.hx b/hmp-commands.hx index 182e639d14..9a3e57504f 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1800,3 +1800,18 @@ ERST "\n\t\t\t\t\t limit on a specified virtual cpu", .cmd = hmp_cancel_vcpu_dirty_limit, }, + +#if defined(CONFIG_FDT) +SRST +``dumpdtb`` *filename* + Save the FDT in the 'filename' file to be decoded using dtc. + Requires 'libfdt' support. +ERST + { + .name = "dumpdtb", + .args_type = "filename:F", + .params = "filename", + .help = "save the FDT in the 'filename' file to be decoded using dtc", + .cmd = hmp_dumpdtb, + }, +#endif diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h index ef060a9759..e7c5441f56 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 hmp_dumpdtb(Monitor *mon, const QDict *qdict); /** * qemu_fdt_setprop_sized_cells_from_array: diff --git a/monitor/misc.c b/monitor/misc.c index 3d2312ba8d..e7dd63030b 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -49,6 +49,7 @@ #include "sysemu/blockdev.h" #include "sysemu/sysemu.h" #include "sysemu/tpm.h" +#include "sysemu/device_tree.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qerror.h" #include "qapi/qmp/qstring.h" diff --git a/qapi/machine.json b/qapi/machine.json index 6afd1936b0..f968a5d343 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1664,3 +1664,21 @@ '*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' }, + 'if': 'CONFIG_FDT' } diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c index 6ca3fad285..cdd41b6de6 100644 --- a/softmmu/device_tree.c +++ b/softmmu/device_tree.c @@ -26,6 +26,9 @@ #include "hw/loader.h" #include "hw/boards.h" #include "qemu/config-file.h" +#include "qapi/qapi-commands-machine.h" +#include "qapi/qmp/qdict.h" +#include "monitor/hmp.h" #include <libfdt.h> @@ -643,3 +646,31 @@ out: g_free(propcells); return ret; } + +void qmp_dumpdtb(const char *filename, Error **errp) +{ + g_autoptr(GError) err = NULL; + 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, &err)) { + error_setg(errp, "Error saving FDT to file %s: %s", + filename, err->message); + } +} + +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); + + hmp_handle_error(mon, local_err); +}
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. This command will always be executed in-band (i.e. holding BQL), avoiding potential race conditions with machines that might change the FDT during runtime (e.g. PowerPC 'pseries' machine). Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> Cc: Markus Armbruster <armbru@redhat.com> Cc: Alistair Francis <alistair.francis@wdc.com> Cc: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hmp-commands.hx | 15 +++++++++++++++ include/sysemu/device_tree.h | 1 + monitor/misc.c | 1 + qapi/machine.json | 18 ++++++++++++++++++ softmmu/device_tree.c | 31 +++++++++++++++++++++++++++++++ 5 files changed, 66 insertions(+)