| Message ID | 20220805093948.82561-16-danielhb413@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | QMP/HMP: add 'dumpdtb' and 'info fdt' commands | expand |
On Fri, Aug 05, 2022 at 06:39:43AM -0300, Daniel Henrique Barboza wrote: > Reading the FDT requires that the user saves the fdt_blob and then use > 'dtc' to read the contents. Saving the file and using 'dtc' is a strong > use case when we need to compare two FDTs, but it's a lot of steps if > you want to do quick check on a certain node or property. > > 'info fdt' retrieves FDT nodes (and properties, later on) and print it > to the user. This can be used to check the FDT on running machines > without having to save the blob and use 'dtc'. > > The implementation is based on the premise that the machine thas a FDT > created using libfdt and pointed by 'machine->fdt'. As long as this > pre-requisite is met the machine should be able to support it. > > For now we're going to add the required QMP/HMP boilerplate and the > capability of printing the name of the properties of a given node. Next > patches will extend 'info fdt' to be able to print nodes recursively, > and then individual properties. > > 'info fdt' is not something that we expect to be used aside from debugging, > so we're implementing it in QMP as 'x-query-fdt'. > > This is an example of 'info fdt' fetching the '/chosen' node of the > pSeries machine: > > (qemu) info fdt /chosen > chosen { > ibm,architecture-vec-5; > rng-seed; > ibm,arch-vec-5-platform-support; > linux,pci-probe-only; > stdout-path; > linux,stdout-path; > qemu,graphic-depth; > qemu,graphic-height; > qemu,graphic-width; > } > > And the same node for the aarch64 'virt' machine: > > (qemu) info fdt /chosen > chosen { > stdout-path; > rng-seed; > kaslr-seed; > } So... it's listing the names of the properties, but not the contents? That seems kind of odd. > > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > hmp-commands-info.hx | 13 ++++++++++ > include/monitor/hmp.h | 1 + > include/sysemu/device_tree.h | 4 +++ > monitor/hmp-cmds.c | 13 ++++++++++ > monitor/qmp-cmds.c | 12 +++++++++ > qapi/machine.json | 19 +++++++++++++++ > softmmu/device_tree.c | 47 ++++++++++++++++++++++++++++++++++++ > 7 files changed, 109 insertions(+) > > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx > index 188d9ece3b..743b48865d 100644 > --- a/hmp-commands-info.hx > +++ b/hmp-commands-info.hx > @@ -921,3 +921,16 @@ SRST > ``stats`` > Show runtime-collected statistics > ERST > + > + { > + .name = "fdt", > + .args_type = "nodepath:s", > + .params = "nodepath", > + .help = "show firmware device tree node given its path", > + .cmd = hmp_info_fdt, > + }, > + > +SRST > + ``info fdt`` > + Show a firmware device tree node given its path. Requires libfdt. > +ERST > diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h > index d7f324da59..c0883dd1e3 100644 > --- a/include/monitor/hmp.h > +++ b/include/monitor/hmp.h > @@ -135,6 +135,7 @@ 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_info_fdt(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 bf7684e4ed..057d13e397 100644 > --- a/include/sysemu/device_tree.h > +++ b/include/sysemu/device_tree.h > @@ -14,6 +14,8 @@ > #ifndef DEVICE_TREE_H > #define DEVICE_TREE_H > > +#include "qapi/qapi-types-common.h" > + > void *create_device_tree(int *sizep); > void *load_device_tree(const char *filename_path, int *sizep); > #ifdef CONFIG_LINUX > @@ -137,6 +139,8 @@ int qemu_fdt_add_path(void *fdt, const char *path); > > void qemu_fdt_dumpdtb(void *fdt, int size); > void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp); > +HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, > + Error **errp); > > /** > * qemu_fdt_setprop_sized_cells_from_array: > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c > index d23ec85f9d..accde90380 100644 > --- a/monitor/hmp-cmds.c > +++ b/monitor/hmp-cmds.c > @@ -2484,3 +2484,16 @@ void hmp_dumpdtb(Monitor *mon, const QDict *qdict) > error_report_err(local_err); > } > } > + > +void hmp_info_fdt(Monitor *mon, const QDict *qdict) > +{ > + const char *nodepath = qdict_get_str(qdict, "nodepath"); > + Error *err = NULL; > + g_autoptr(HumanReadableText) info = qmp_x_query_fdt(nodepath, &err); > + > + if (hmp_handle_error(mon, err)) { > + return; > + } > + > + monitor_printf(mon, "%s", info->human_readable_text); > +} > diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c > index 8415aca08c..db2c6aa7da 100644 > --- a/monitor/qmp-cmds.c > +++ b/monitor/qmp-cmds.c > @@ -603,9 +603,21 @@ void qmp_dumpdtb(const char *filename, Error **errp) > { > return qemu_fdt_qmp_dumpdtb(filename, errp); > } > + > +HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp) > +{ > + return qemu_fdt_qmp_query_fdt(nodepath, errp); > +} > #else > void qmp_dumpdtb(const char *filename, Error **errp) > { > error_setg(errp, "dumpdtb requires libfdt"); > } > + > +HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp) > +{ > + error_setg(errp, "this command requires libfdt"); > + > + return NULL; > +} > #endif > diff --git a/qapi/machine.json b/qapi/machine.json > index aeb013f3dd..96cff541ca 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1681,3 +1681,22 @@ > ## > { 'command': 'dumpdtb', > 'data': { 'filename': 'str' } } > + > +## > +# @x-query-fdt: > +# > +# Query for FDT element (node or property). Requires 'libfdt'. > +# > +# @nodepath: the path of the FDT node to be retrieved > +# > +# Features: > +# @unstable: This command is meant for debugging. > +# > +# Returns: FDT node > +# > +# Since: 7.2 > +## > +{ 'command': 'x-query-fdt', > + 'data': { 'nodepath': 'str' }, > + 'returns': 'HumanReadableText', > + 'features': [ 'unstable' ] } A QMP command that returns human readable text, rather than something JSON structured seems... odd. Admittedly, *how* you'd JSON structure the results gets a bit tricky. Listing nodes or property names would be easy enough, but getting the property contents is hairy, since JSON strings are supposed to be Unicode, but DT properties can be arbitrary bytestrings. > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > index cd487ddd4d..3fb07b537f 100644 > --- a/softmmu/device_tree.c > +++ b/softmmu/device_tree.c > @@ -18,6 +18,7 @@ > #endif > > #include "qapi/error.h" > +#include "qapi/type-helpers.h" > #include "qemu/error-report.h" > #include "qemu/option.h" > #include "qemu/bswap.h" > @@ -661,3 +662,49 @@ void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp) > > error_setg(errp, "Error when saving machine FDT to file %s", filename); > } > + > +static void fdt_format_node(GString *buf, int node, int depth) > +{ > + const struct fdt_property *prop = NULL; > + const char *propname = NULL; > + void *fdt = current_machine->fdt; > + int padding = depth * 4; > + int property = 0; > + int prop_size; > + > + g_string_append_printf(buf, "%*s%s {\n", padding, "", > + fdt_get_name(fdt, node, NULL)); > + > + padding += 4; > + > + fdt_for_each_property_offset(property, fdt, node) { > + prop = fdt_get_property_by_offset(fdt, property, &prop_size); > + propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); > + > + g_string_append_printf(buf, "%*s%s;\n", padding, "", propname); > + } > + > + padding -= 4; > + g_string_append_printf(buf, "%*s}\n", padding, ""); So, this lists it in dts format... kind of. Because you don't include the property values, it makes it look like all properties are binary (either absent or present-but-empty). I think that's misleading. If you're only going to list properties, I think you'd be better off using different formatting (and maybe a more specific command name as well). > +} > + > +HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, Error **errp) > +{ > + g_autoptr(GString) buf = g_string_new(""); > + int node; > + > + if (!current_machine->fdt) { > + error_setg(errp, "Unable to find the machine FDT"); > + return NULL; > + } > + > + node = fdt_path_offset(current_machine->fdt, nodepath); > + if (node < 0) { > + error_setg(errp, "node '%s' not found in FDT", nodepath); > + return NULL; > + } > + > + fdt_format_node(buf, node, 0); > + > + return human_readable_text_from_str(buf); > +}
On 8/8/22 01:21, David Gibson wrote: > On Fri, Aug 05, 2022 at 06:39:43AM -0300, Daniel Henrique Barboza wrote: >> Reading the FDT requires that the user saves the fdt_blob and then use >> 'dtc' to read the contents. Saving the file and using 'dtc' is a strong >> use case when we need to compare two FDTs, but it's a lot of steps if >> you want to do quick check on a certain node or property. >> >> 'info fdt' retrieves FDT nodes (and properties, later on) and print it >> to the user. This can be used to check the FDT on running machines >> without having to save the blob and use 'dtc'. >> >> The implementation is based on the premise that the machine thas a FDT >> created using libfdt and pointed by 'machine->fdt'. As long as this >> pre-requisite is met the machine should be able to support it. >> >> For now we're going to add the required QMP/HMP boilerplate and the >> capability of printing the name of the properties of a given node. Next >> patches will extend 'info fdt' to be able to print nodes recursively, >> and then individual properties. >> >> 'info fdt' is not something that we expect to be used aside from debugging, >> so we're implementing it in QMP as 'x-query-fdt'. >> >> This is an example of 'info fdt' fetching the '/chosen' node of the >> pSeries machine: >> >> (qemu) info fdt /chosen >> chosen { >> ibm,architecture-vec-5; >> rng-seed; >> ibm,arch-vec-5-platform-support; >> linux,pci-probe-only; >> stdout-path; >> linux,stdout-path; >> qemu,graphic-depth; >> qemu,graphic-height; >> qemu,graphic-width; >> } >> >> And the same node for the aarch64 'virt' machine: >> >> (qemu) info fdt /chosen >> chosen { >> stdout-path; >> rng-seed; >> kaslr-seed; >> } > > So... it's listing the names of the properties, but not the contents? > That seems kind of odd. > >> >> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> --- >> hmp-commands-info.hx | 13 ++++++++++ >> include/monitor/hmp.h | 1 + >> include/sysemu/device_tree.h | 4 +++ >> monitor/hmp-cmds.c | 13 ++++++++++ >> monitor/qmp-cmds.c | 12 +++++++++ >> qapi/machine.json | 19 +++++++++++++++ >> softmmu/device_tree.c | 47 ++++++++++++++++++++++++++++++++++++ >> 7 files changed, 109 insertions(+) >> >> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx >> index 188d9ece3b..743b48865d 100644 >> --- a/hmp-commands-info.hx >> +++ b/hmp-commands-info.hx >> @@ -921,3 +921,16 @@ SRST >> ``stats`` >> Show runtime-collected statistics >> ERST >> + >> + { >> + .name = "fdt", >> + .args_type = "nodepath:s", >> + .params = "nodepath", >> + .help = "show firmware device tree node given its path", >> + .cmd = hmp_info_fdt, >> + }, >> + >> +SRST >> + ``info fdt`` >> + Show a firmware device tree node given its path. Requires libfdt. >> +ERST >> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h >> index d7f324da59..c0883dd1e3 100644 >> --- a/include/monitor/hmp.h >> +++ b/include/monitor/hmp.h >> @@ -135,6 +135,7 @@ 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_info_fdt(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 bf7684e4ed..057d13e397 100644 >> --- a/include/sysemu/device_tree.h >> +++ b/include/sysemu/device_tree.h >> @@ -14,6 +14,8 @@ >> #ifndef DEVICE_TREE_H >> #define DEVICE_TREE_H >> >> +#include "qapi/qapi-types-common.h" >> + >> void *create_device_tree(int *sizep); >> void *load_device_tree(const char *filename_path, int *sizep); >> #ifdef CONFIG_LINUX >> @@ -137,6 +139,8 @@ int qemu_fdt_add_path(void *fdt, const char *path); >> >> void qemu_fdt_dumpdtb(void *fdt, int size); >> void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp); >> +HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, >> + Error **errp); >> >> /** >> * qemu_fdt_setprop_sized_cells_from_array: >> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c >> index d23ec85f9d..accde90380 100644 >> --- a/monitor/hmp-cmds.c >> +++ b/monitor/hmp-cmds.c >> @@ -2484,3 +2484,16 @@ void hmp_dumpdtb(Monitor *mon, const QDict *qdict) >> error_report_err(local_err); >> } >> } >> + >> +void hmp_info_fdt(Monitor *mon, const QDict *qdict) >> +{ >> + const char *nodepath = qdict_get_str(qdict, "nodepath"); >> + Error *err = NULL; >> + g_autoptr(HumanReadableText) info = qmp_x_query_fdt(nodepath, &err); >> + >> + if (hmp_handle_error(mon, err)) { >> + return; >> + } >> + >> + monitor_printf(mon, "%s", info->human_readable_text); >> +} >> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c >> index 8415aca08c..db2c6aa7da 100644 >> --- a/monitor/qmp-cmds.c >> +++ b/monitor/qmp-cmds.c >> @@ -603,9 +603,21 @@ void qmp_dumpdtb(const char *filename, Error **errp) >> { >> return qemu_fdt_qmp_dumpdtb(filename, errp); >> } >> + >> +HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp) >> +{ >> + return qemu_fdt_qmp_query_fdt(nodepath, errp); >> +} >> #else >> void qmp_dumpdtb(const char *filename, Error **errp) >> { >> error_setg(errp, "dumpdtb requires libfdt"); >> } >> + >> +HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp) >> +{ >> + error_setg(errp, "this command requires libfdt"); >> + >> + return NULL; >> +} >> #endif >> diff --git a/qapi/machine.json b/qapi/machine.json >> index aeb013f3dd..96cff541ca 100644 >> --- a/qapi/machine.json >> +++ b/qapi/machine.json >> @@ -1681,3 +1681,22 @@ >> ## >> { 'command': 'dumpdtb', >> 'data': { 'filename': 'str' } } >> + >> +## >> +# @x-query-fdt: >> +# >> +# Query for FDT element (node or property). Requires 'libfdt'. >> +# >> +# @nodepath: the path of the FDT node to be retrieved >> +# >> +# Features: >> +# @unstable: This command is meant for debugging. >> +# >> +# Returns: FDT node >> +# >> +# Since: 7.2 >> +## >> +{ 'command': 'x-query-fdt', >> + 'data': { 'nodepath': 'str' }, >> + 'returns': 'HumanReadableText', >> + 'features': [ 'unstable' ] } > > > A QMP command that returns human readable text, rather than something > JSON structured seems... odd. > > Admittedly, *how* you'd JSON structure the results gets a bit tricky. > Listing nodes or property names would be easy enough, but getting the > property contents is hairy, since JSON strings are supposed to be > Unicode, but DT properties can be arbitrary bytestrings. > >> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c >> index cd487ddd4d..3fb07b537f 100644 >> --- a/softmmu/device_tree.c >> +++ b/softmmu/device_tree.c >> @@ -18,6 +18,7 @@ >> #endif >> >> #include "qapi/error.h" >> +#include "qapi/type-helpers.h" >> #include "qemu/error-report.h" >> #include "qemu/option.h" >> #include "qemu/bswap.h" >> @@ -661,3 +662,49 @@ void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp) >> >> error_setg(errp, "Error when saving machine FDT to file %s", filename); >> } >> + >> +static void fdt_format_node(GString *buf, int node, int depth) >> +{ >> + const struct fdt_property *prop = NULL; >> + const char *propname = NULL; >> + void *fdt = current_machine->fdt; >> + int padding = depth * 4; >> + int property = 0; >> + int prop_size; >> + >> + g_string_append_printf(buf, "%*s%s {\n", padding, "", >> + fdt_get_name(fdt, node, NULL)); >> + >> + padding += 4; >> + >> + fdt_for_each_property_offset(property, fdt, node) { >> + prop = fdt_get_property_by_offset(fdt, property, &prop_size); >> + propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); >> + >> + g_string_append_printf(buf, "%*s%s;\n", padding, "", propname); >> + } >> + >> + padding -= 4; >> + g_string_append_printf(buf, "%*s}\n", padding, ""); > > So, this lists it in dts format... kind of. Because you don't include > the property values, it makes it look like all properties are binary > (either absent or present-but-empty). I think that's misleading. If > you're only going to list properties, I think you'd be better off > using different formatting (and maybe a more specific command name as > well). As you've already noticed in the next patch, I decided to split between QMP/HMP introduction and the dts formatting to avoid clogging everything in a single patch. In the end the whole fdt tree can be printed with all the properties types. As for using HumanReadableText, I tried to imagine a structured output for 'info fdt'. It would be something like: - struct Property: an union of the possible types (none, string, uint32 array, byte array) - struct Node: an array of properties and subnodes And then 'info fdt <node>' would return a struct node and 'info fdt <node> <prop>' would return a struct Property Adding this stable ABI sounded like too much extra work for a command that would be used mostly for debug/development purposes. Every other command that outputs internal guest state (info roms, info rdma, info irq, info numa ...) are happy with returning HumanReadableFormat and being considered debug only. I decided to do the same thing. Thanks, Daniel > >> +} >> + >> +HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, Error **errp) >> +{ >> + g_autoptr(GString) buf = g_string_new(""); >> + int node; >> + >> + if (!current_machine->fdt) { >> + error_setg(errp, "Unable to find the machine FDT"); >> + return NULL; >> + } >> + >> + node = fdt_path_offset(current_machine->fdt, nodepath); >> + if (node < 0) { >> + error_setg(errp, "node '%s' not found in FDT", nodepath); >> + return NULL; >> + } >> + >> + fdt_format_node(buf, node, 0); >> + >> + return human_readable_text_from_str(buf); >> +} >
On Mon, Aug 15, 2022 at 07:48:14PM -0300, Daniel Henrique Barboza wrote: > > > On 8/8/22 01:21, David Gibson wrote: > > On Fri, Aug 05, 2022 at 06:39:43AM -0300, Daniel Henrique Barboza wrote: > > > Reading the FDT requires that the user saves the fdt_blob and then use > > > 'dtc' to read the contents. Saving the file and using 'dtc' is a strong > > > use case when we need to compare two FDTs, but it's a lot of steps if > > > you want to do quick check on a certain node or property. > > > > > > 'info fdt' retrieves FDT nodes (and properties, later on) and print it > > > to the user. This can be used to check the FDT on running machines > > > without having to save the blob and use 'dtc'. > > > > > > The implementation is based on the premise that the machine thas a FDT > > > created using libfdt and pointed by 'machine->fdt'. As long as this > > > pre-requisite is met the machine should be able to support it. > > > > > > For now we're going to add the required QMP/HMP boilerplate and the > > > capability of printing the name of the properties of a given node. Next > > > patches will extend 'info fdt' to be able to print nodes recursively, > > > and then individual properties. > > > > > > 'info fdt' is not something that we expect to be used aside from debugging, > > > so we're implementing it in QMP as 'x-query-fdt'. > > > > > > This is an example of 'info fdt' fetching the '/chosen' node of the > > > pSeries machine: > > > > > > (qemu) info fdt /chosen > > > chosen { > > > ibm,architecture-vec-5; > > > rng-seed; > > > ibm,arch-vec-5-platform-support; > > > linux,pci-probe-only; > > > stdout-path; > > > linux,stdout-path; > > > qemu,graphic-depth; > > > qemu,graphic-height; > > > qemu,graphic-width; > > > } > > > > > > And the same node for the aarch64 'virt' machine: > > > > > > (qemu) info fdt /chosen > > > chosen { > > > stdout-path; > > > rng-seed; > > > kaslr-seed; > > > } > > > > So... it's listing the names of the properties, but not the contents? > > That seems kind of odd. > > > > > > > > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > > --- > > > hmp-commands-info.hx | 13 ++++++++++ > > > include/monitor/hmp.h | 1 + > > > include/sysemu/device_tree.h | 4 +++ > > > monitor/hmp-cmds.c | 13 ++++++++++ > > > monitor/qmp-cmds.c | 12 +++++++++ > > > qapi/machine.json | 19 +++++++++++++++ > > > softmmu/device_tree.c | 47 ++++++++++++++++++++++++++++++++++++ > > > 7 files changed, 109 insertions(+) > > > > > > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx > > > index 188d9ece3b..743b48865d 100644 > > > --- a/hmp-commands-info.hx > > > +++ b/hmp-commands-info.hx > > > @@ -921,3 +921,16 @@ SRST > > > ``stats`` > > > Show runtime-collected statistics > > > ERST > > > + > > > + { > > > + .name = "fdt", > > > + .args_type = "nodepath:s", > > > + .params = "nodepath", > > > + .help = "show firmware device tree node given its path", > > > + .cmd = hmp_info_fdt, > > > + }, > > > + > > > +SRST > > > + ``info fdt`` > > > + Show a firmware device tree node given its path. Requires libfdt. > > > +ERST > > > diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h > > > index d7f324da59..c0883dd1e3 100644 > > > --- a/include/monitor/hmp.h > > > +++ b/include/monitor/hmp.h > > > @@ -135,6 +135,7 @@ 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_info_fdt(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 bf7684e4ed..057d13e397 100644 > > > --- a/include/sysemu/device_tree.h > > > +++ b/include/sysemu/device_tree.h > > > @@ -14,6 +14,8 @@ > > > #ifndef DEVICE_TREE_H > > > #define DEVICE_TREE_H > > > +#include "qapi/qapi-types-common.h" > > > + > > > void *create_device_tree(int *sizep); > > > void *load_device_tree(const char *filename_path, int *sizep); > > > #ifdef CONFIG_LINUX > > > @@ -137,6 +139,8 @@ int qemu_fdt_add_path(void *fdt, const char *path); > > > void qemu_fdt_dumpdtb(void *fdt, int size); > > > void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp); > > > +HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, > > > + Error **errp); > > > /** > > > * qemu_fdt_setprop_sized_cells_from_array: > > > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c > > > index d23ec85f9d..accde90380 100644 > > > --- a/monitor/hmp-cmds.c > > > +++ b/monitor/hmp-cmds.c > > > @@ -2484,3 +2484,16 @@ void hmp_dumpdtb(Monitor *mon, const QDict *qdict) > > > error_report_err(local_err); > > > } > > > } > > > + > > > +void hmp_info_fdt(Monitor *mon, const QDict *qdict) > > > +{ > > > + const char *nodepath = qdict_get_str(qdict, "nodepath"); > > > + Error *err = NULL; > > > + g_autoptr(HumanReadableText) info = qmp_x_query_fdt(nodepath, &err); > > > + > > > + if (hmp_handle_error(mon, err)) { > > > + return; > > > + } > > > + > > > + monitor_printf(mon, "%s", info->human_readable_text); > > > +} > > > diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c > > > index 8415aca08c..db2c6aa7da 100644 > > > --- a/monitor/qmp-cmds.c > > > +++ b/monitor/qmp-cmds.c > > > @@ -603,9 +603,21 @@ void qmp_dumpdtb(const char *filename, Error **errp) > > > { > > > return qemu_fdt_qmp_dumpdtb(filename, errp); > > > } > > > + > > > +HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp) > > > +{ > > > + return qemu_fdt_qmp_query_fdt(nodepath, errp); > > > +} > > > #else > > > void qmp_dumpdtb(const char *filename, Error **errp) > > > { > > > error_setg(errp, "dumpdtb requires libfdt"); > > > } > > > + > > > +HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp) > > > +{ > > > + error_setg(errp, "this command requires libfdt"); > > > + > > > + return NULL; > > > +} > > > #endif > > > diff --git a/qapi/machine.json b/qapi/machine.json > > > index aeb013f3dd..96cff541ca 100644 > > > --- a/qapi/machine.json > > > +++ b/qapi/machine.json > > > @@ -1681,3 +1681,22 @@ > > > ## > > > { 'command': 'dumpdtb', > > > 'data': { 'filename': 'str' } } > > > + > > > +## > > > +# @x-query-fdt: > > > +# > > > +# Query for FDT element (node or property). Requires 'libfdt'. > > > +# > > > +# @nodepath: the path of the FDT node to be retrieved > > > +# > > > +# Features: > > > +# @unstable: This command is meant for debugging. > > > +# > > > +# Returns: FDT node > > > +# > > > +# Since: 7.2 > > > +## > > > +{ 'command': 'x-query-fdt', > > > + 'data': { 'nodepath': 'str' }, > > > + 'returns': 'HumanReadableText', > > > + 'features': [ 'unstable' ] } > > > > > > A QMP command that returns human readable text, rather than something > > JSON structured seems... odd. > > > > Admittedly, *how* you'd JSON structure the results gets a bit tricky. > > Listing nodes or property names would be easy enough, but getting the > > property contents is hairy, since JSON strings are supposed to be > > Unicode, but DT properties can be arbitrary bytestrings. > > > > > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > > > index cd487ddd4d..3fb07b537f 100644 > > > --- a/softmmu/device_tree.c > > > +++ b/softmmu/device_tree.c > > > @@ -18,6 +18,7 @@ > > > #endif > > > #include "qapi/error.h" > > > +#include "qapi/type-helpers.h" > > > #include "qemu/error-report.h" > > > #include "qemu/option.h" > > > #include "qemu/bswap.h" > > > @@ -661,3 +662,49 @@ void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp) > > > error_setg(errp, "Error when saving machine FDT to file %s", filename); > > > } > > > + > > > +static void fdt_format_node(GString *buf, int node, int depth) > > > +{ > > > + const struct fdt_property *prop = NULL; > > > + const char *propname = NULL; > > > + void *fdt = current_machine->fdt; > > > + int padding = depth * 4; > > > + int property = 0; > > > + int prop_size; > > > + > > > + g_string_append_printf(buf, "%*s%s {\n", padding, "", > > > + fdt_get_name(fdt, node, NULL)); > > > + > > > + padding += 4; > > > + > > > + fdt_for_each_property_offset(property, fdt, node) { > > > + prop = fdt_get_property_by_offset(fdt, property, &prop_size); > > > + propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); > > > + > > > + g_string_append_printf(buf, "%*s%s;\n", padding, "", propname); > > > + } > > > + > > > + padding -= 4; > > > + g_string_append_printf(buf, "%*s}\n", padding, ""); > > > > So, this lists it in dts format... kind of. Because you don't include > > the property values, it makes it look like all properties are binary > > (either absent or present-but-empty). I think that's misleading. If > > you're only going to list properties, I think you'd be better off > > using different formatting (and maybe a more specific command name as > > well). > > As you've already noticed in the next patch, I decided to split between QMP/HMP > introduction and the dts formatting to avoid clogging everything in a single patch. > In the end the whole fdt tree can be printed with all the properties types. > > As for using HumanReadableText, I tried to imagine a structured output for > 'info fdt'. It would be something like: > > - struct Property: an union of the possible types (none, string, uint32 array, > byte array) I don't think it's wise to try to interpret the property any further than a raw bytestring in a machine oriented interface. You can't really know the type of a property without knowing all the DT bindings that apply to it, what we do for formatting in this patch series, and in dtc is a heuristic only. That's useful for debuging output aimed at humans, but not a good idea for a structured interface. Note that even for the human interface it might be a better compromise to just always emit properties essentially as a hexdump. I wouldn't suggest approximating dts format in this case though, to make the distinction clearer. > - struct Node: an array of properties and subnodes > > And then 'info fdt <node>' would return a struct node and 'info fdt <node> <prop>' > would return a struct Property Well, for a "native" QMP interface, I'd suggest splitting up the operations more. Say "fdt-ls" (takes a path, returns array of subnode names), "fdt-properties" (takes a path, returns array of property names), and getprop (takes a path and a property name, returns bytestring). I guess a "dump" operation of some sort to get the whole thing without a million commands might be a good idea too. > Adding this stable ABI sounded like too much extra work for a command that would be > used mostly for debug/development purposes. Every other command that outputs > internal guest state (info roms, info rdma, info irq, info numa ...) are happy with > returning HumanReadableFormat and being considered debug only. I decided to do the > same thing. Ok, I hadn't realized there was precedent for this approach. If that's the case I'm less bothered by this.
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index 188d9ece3b..743b48865d 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -921,3 +921,16 @@ SRST ``stats`` Show runtime-collected statistics ERST + + { + .name = "fdt", + .args_type = "nodepath:s", + .params = "nodepath", + .help = "show firmware device tree node given its path", + .cmd = hmp_info_fdt, + }, + +SRST + ``info fdt`` + Show a firmware device tree node given its path. Requires libfdt. +ERST diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h index d7f324da59..c0883dd1e3 100644 --- a/include/monitor/hmp.h +++ b/include/monitor/hmp.h @@ -135,6 +135,7 @@ 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_info_fdt(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 bf7684e4ed..057d13e397 100644 --- a/include/sysemu/device_tree.h +++ b/include/sysemu/device_tree.h @@ -14,6 +14,8 @@ #ifndef DEVICE_TREE_H #define DEVICE_TREE_H +#include "qapi/qapi-types-common.h" + void *create_device_tree(int *sizep); void *load_device_tree(const char *filename_path, int *sizep); #ifdef CONFIG_LINUX @@ -137,6 +139,8 @@ int qemu_fdt_add_path(void *fdt, const char *path); void qemu_fdt_dumpdtb(void *fdt, int size); void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp); +HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, + Error **errp); /** * qemu_fdt_setprop_sized_cells_from_array: diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index d23ec85f9d..accde90380 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -2484,3 +2484,16 @@ void hmp_dumpdtb(Monitor *mon, const QDict *qdict) error_report_err(local_err); } } + +void hmp_info_fdt(Monitor *mon, const QDict *qdict) +{ + const char *nodepath = qdict_get_str(qdict, "nodepath"); + Error *err = NULL; + g_autoptr(HumanReadableText) info = qmp_x_query_fdt(nodepath, &err); + + if (hmp_handle_error(mon, err)) { + return; + } + + monitor_printf(mon, "%s", info->human_readable_text); +} diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index 8415aca08c..db2c6aa7da 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -603,9 +603,21 @@ void qmp_dumpdtb(const char *filename, Error **errp) { return qemu_fdt_qmp_dumpdtb(filename, errp); } + +HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp) +{ + return qemu_fdt_qmp_query_fdt(nodepath, errp); +} #else void qmp_dumpdtb(const char *filename, Error **errp) { error_setg(errp, "dumpdtb requires libfdt"); } + +HumanReadableText *qmp_x_query_fdt(const char *nodepath, Error **errp) +{ + error_setg(errp, "this command requires libfdt"); + + return NULL; +} #endif diff --git a/qapi/machine.json b/qapi/machine.json index aeb013f3dd..96cff541ca 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1681,3 +1681,22 @@ ## { 'command': 'dumpdtb', 'data': { 'filename': 'str' } } + +## +# @x-query-fdt: +# +# Query for FDT element (node or property). Requires 'libfdt'. +# +# @nodepath: the path of the FDT node to be retrieved +# +# Features: +# @unstable: This command is meant for debugging. +# +# Returns: FDT node +# +# Since: 7.2 +## +{ 'command': 'x-query-fdt', + 'data': { 'nodepath': 'str' }, + 'returns': 'HumanReadableText', + 'features': [ 'unstable' ] } diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c index cd487ddd4d..3fb07b537f 100644 --- a/softmmu/device_tree.c +++ b/softmmu/device_tree.c @@ -18,6 +18,7 @@ #endif #include "qapi/error.h" +#include "qapi/type-helpers.h" #include "qemu/error-report.h" #include "qemu/option.h" #include "qemu/bswap.h" @@ -661,3 +662,49 @@ void qemu_fdt_qmp_dumpdtb(const char *filename, Error **errp) error_setg(errp, "Error when saving machine FDT to file %s", filename); } + +static void fdt_format_node(GString *buf, int node, int depth) +{ + const struct fdt_property *prop = NULL; + const char *propname = NULL; + void *fdt = current_machine->fdt; + int padding = depth * 4; + int property = 0; + int prop_size; + + g_string_append_printf(buf, "%*s%s {\n", padding, "", + fdt_get_name(fdt, node, NULL)); + + padding += 4; + + fdt_for_each_property_offset(property, fdt, node) { + prop = fdt_get_property_by_offset(fdt, property, &prop_size); + propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); + + g_string_append_printf(buf, "%*s%s;\n", padding, "", propname); + } + + padding -= 4; + g_string_append_printf(buf, "%*s}\n", padding, ""); +} + +HumanReadableText *qemu_fdt_qmp_query_fdt(const char *nodepath, Error **errp) +{ + g_autoptr(GString) buf = g_string_new(""); + int node; + + if (!current_machine->fdt) { + error_setg(errp, "Unable to find the machine FDT"); + return NULL; + } + + node = fdt_path_offset(current_machine->fdt, nodepath); + if (node < 0) { + error_setg(errp, "node '%s' not found in FDT", nodepath); + return NULL; + } + + fdt_format_node(buf, node, 0); + + return human_readable_text_from_str(buf); +}
Reading the FDT requires that the user saves the fdt_blob and then use 'dtc' to read the contents. Saving the file and using 'dtc' is a strong use case when we need to compare two FDTs, but it's a lot of steps if you want to do quick check on a certain node or property. 'info fdt' retrieves FDT nodes (and properties, later on) and print it to the user. This can be used to check the FDT on running machines without having to save the blob and use 'dtc'. The implementation is based on the premise that the machine thas a FDT created using libfdt and pointed by 'machine->fdt'. As long as this pre-requisite is met the machine should be able to support it. For now we're going to add the required QMP/HMP boilerplate and the capability of printing the name of the properties of a given node. Next patches will extend 'info fdt' to be able to print nodes recursively, and then individual properties. 'info fdt' is not something that we expect to be used aside from debugging, so we're implementing it in QMP as 'x-query-fdt'. This is an example of 'info fdt' fetching the '/chosen' node of the pSeries machine: (qemu) info fdt /chosen chosen { ibm,architecture-vec-5; rng-seed; ibm,arch-vec-5-platform-support; linux,pci-probe-only; stdout-path; linux,stdout-path; qemu,graphic-depth; qemu,graphic-height; qemu,graphic-width; } And the same node for the aarch64 'virt' machine: (qemu) info fdt /chosen chosen { stdout-path; rng-seed; kaslr-seed; } Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hmp-commands-info.hx | 13 ++++++++++ include/monitor/hmp.h | 1 + include/sysemu/device_tree.h | 4 +++ monitor/hmp-cmds.c | 13 ++++++++++ monitor/qmp-cmds.c | 12 +++++++++ qapi/machine.json | 19 +++++++++++++++ softmmu/device_tree.c | 47 ++++++++++++++++++++++++++++++++++++ 7 files changed, 109 insertions(+)