| Message ID | 20190308013222.12524-14-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | fw_cfg: reduce memleaks, add QMP/HMP info + edk2_add_host_crypto_policy | expand |
On 3/7/19 7:32 PM, Philippe Mathieu-Daudé wrote: > When debugging a paravirtualized guest firmware, it results very > useful to dump the fw_cfg table. > Add a QMP command which returns the most useful fields. > Since the QMP protocol is not designed for passing stream data, > we don't display a fw_cfg item data, only it's size: > > { "execute": "query-fw_cfg-items" } > { > "return": [ > { > "architecture_specific": false, > "key": 0, > "writeable": false, > "size": 4, > "keyname": "signature" You could return a base64-encoded representation of the field (we do that in other interfaces where raw binary can't be passed reliably over JSON). For 4 bytes, that makes sense, > { > "architecture_specific": true, > "key": 3, > "writeable": false, > "size": 324, > "keyname": "e820_tables" for 324 bytes, it gets a bit long. Still, may be worth it for the added debug visibility. > +++ b/qapi/misc.json > @@ -3051,3 +3051,47 @@ > 'data': 'NumaOptions', > 'allow-preconfig': true > } > + > +## > +# @FirmwareConfigurationItem: > +# > +# Firmware Configuration (fw_cfg) item. > +# > +# @key: The uint16 selector key. > +# @keyname: The stringified name if the selector refers to a well-known > +# numerically defined item. > +# @architecture_specific: Indicates whether the configuration setting is Prefer '-' over '_' in new interfaces. > +# architecture specific. > +# false: The item is a generic configuration item. > +# true: The item is specific to a particular architecture. > +# @writeable: Indicates whether the configuration setting is writeable by > +# the guest. writable > +# @size: The length of @data associated with the item. > +# @data: A string representating the firmware configuration data. representing > +# Note: This field is currently not used. Either drop the field until it has a use, or let it be the base64 encoding and use it now. > +# @path: If the key is a 'file', the named file path. > +# @order: If the key is a 'file', the named file order. > +# > +# Since 4.0 > +## > +{ 'struct': 'FirmwareConfigurationItem', > + 'data': { 'key': 'uint16', > + '*keyname': 'str', > + 'architecture_specific': 'bool', > + 'writeable': 'bool', > + '*data': 'str', > + 'size': 'int', > + '*path': 'str', > + '*order': 'int' } } Is it worth making 'keyname' an enum type, and turning this into a flat union where 'path' and 'order' appear on the branch associated with 'file', and where all other well-known keynames have smaller branches? > + > + > +## > +# @query-fw_cfg-items: That looks weird to mix - and _. Any reason we can't just go with query-firmware-config? > +# > +# Returns the list of Firmware Configuration items. > +# > +# Returns: A list of @FirmwareConfigurationItem for each entry. > +# > +# Since 4.0 > +## > +{ 'command': 'query-fw_cfg-items', 'returns': ['FirmwareConfigurationItem']} >
Hi Eric, On 3/8/19 3:04 AM, Eric Blake wrote: > On 3/7/19 7:32 PM, Philippe Mathieu-Daudé wrote: >> When debugging a paravirtualized guest firmware, it results very >> useful to dump the fw_cfg table. >> Add a QMP command which returns the most useful fields. >> Since the QMP protocol is not designed for passing stream data, >> we don't display a fw_cfg item data, only it's size: >> >> { "execute": "query-fw_cfg-items" } >> { >> "return": [ >> { >> "architecture_specific": false, >> "key": 0, >> "writeable": false, >> "size": 4, >> "keyname": "signature" > > You could return a base64-encoded representation of the field (we do > that in other interfaces where raw binary can't be passed reliably over > JSON). For 4 bytes, that makes sense, > > >> { >> "architecture_specific": true, >> "key": 3, >> "writeable": false, >> "size": 324, >> "keyname": "e820_tables" > > for 324 bytes, it gets a bit long. Still, may be worth it for the added > debug visibility. Until you see values in the next patch...: $ (echo info fw_cfg; echo q) | qemu-system-x86_64 -S -monitor stdio Selector Well-Known Key Pathname ArchSpec Perm Size Order Hex Data [...] 0x0019 file_dir RO 2052 0000000b.. 0x0022 file: etc/acpi/tables RO 131072 130 46414353.. 0x0028 file: etc/table-loader RO 4096 140 01000000.. What about using a 512B limit on this QMP answer? >> +++ b/qapi/misc.json >> @@ -3051,3 +3051,47 @@ >> 'data': 'NumaOptions', >> 'allow-preconfig': true >> } >> + >> +## >> +# @FirmwareConfigurationItem: >> +# >> +# Firmware Configuration (fw_cfg) item. >> +# >> +# @key: The uint16 selector key. >> +# @keyname: The stringified name if the selector refers to a well-known >> +# numerically defined item. >> +# @architecture_specific: Indicates whether the configuration setting is > > Prefer '-' over '_' in new interfaces. OK! > >> +# architecture specific. >> +# false: The item is a generic configuration item. >> +# true: The item is specific to a particular architecture. >> +# @writeable: Indicates whether the configuration setting is writeable by >> +# the guest. > > writable > >> +# @size: The length of @data associated with the item. >> +# @data: A string representating the firmware configuration data. > > representing > >> +# Note: This field is currently not used. > > Either drop the field until it has a use, or let it be the base64 > encoding and use it now. Well, it is used by the HMP command, to pass the hexdump. I'm rather fine using the base64 encoding. >> +# @path: If the key is a 'file', the named file path. >> +# @order: If the key is a 'file', the named file order. >> +# >> +# Since 4.0 >> +## >> +{ 'struct': 'FirmwareConfigurationItem', >> + 'data': { 'key': 'uint16', >> + '*keyname': 'str', >> + 'architecture_specific': 'bool', >> + 'writeable': 'bool', >> + '*data': 'str', >> + 'size': 'int', >> + '*path': 'str', >> + '*order': 'int' } } > > Is it worth making 'keyname' an enum type, and turning this into a flat > union where 'path' and 'order' appear on the branch associated with > 'file', and where all other well-known keynames have smaller branches? I have no idea about that, I will have a look at QMP flat unions. >> + >> + >> +## >> +# @query-fw_cfg-items: > > That looks weird to mix - and _. Any reason we can't just go with > query-firmware-config? Way better! I'll use query-firmware-config-items. Thanks for the review, Phil. >> +# >> +# Returns the list of Firmware Configuration items. >> +# >> +# Returns: A list of @FirmwareConfigurationItem for each entry. >> +# >> +# Since 4.0 >> +## >> +{ 'command': 'query-fw_cfg-items', 'returns': ['FirmwareConfigurationItem']} >>
On 3/8/19 5:08 AM, Philippe Mathieu-Daudé wrote: >>> { >>> "return": [ >>> { >>> "architecture_specific": false, >>> "key": 0, >>> "writeable": false, >>> "size": 4, >>> "keyname": "signature" >> >> You could return a base64-encoded representation of the field (we do >> that in other interfaces where raw binary can't be passed reliably over >> JSON). For 4 bytes, that makes sense, >> >> >>> { >>> "architecture_specific": true, >>> "key": 3, >>> "writeable": false, >>> "size": 324, >>> "keyname": "e820_tables" >> >> for 324 bytes, it gets a bit long. Still, may be worth it for the added >> debug visibility. > > Until you see values in the next patch...: > > $ (echo info fw_cfg; echo q) | qemu-system-x86_64 -S -monitor stdio > Selector Well-Known Key Pathname ArchSpec Perm Size Order Hex Data > [...] > 0x0019 file_dir RO 2052 0000000b.. > 0x0022 file: etc/acpi/tables RO 131072 130 46414353.. Yeah, that's a no-go. > > What about using a 512B limit on this QMP answer? Seems reasonable. Either omit the field when its size exceeds the limit, or use the field to give a truncated version (where the size field still shows the full length, either way). >>> +# @path: If the key is a 'file', the named file path. >>> +# @order: If the key is a 'file', the named file order. >>> +# >>> +# Since 4.0 >>> +## >>> +{ 'struct': 'FirmwareConfigurationItem', >>> + 'data': { 'key': 'uint16', >>> + '*keyname': 'str', >>> + 'architecture_specific': 'bool', >>> + 'writeable': 'bool', >>> + '*data': 'str', >>> + 'size': 'int', >>> + '*path': 'str', >>> + '*order': 'int' } } >> >> Is it worth making 'keyname' an enum type, and turning this into a flat >> union where 'path' and 'order' appear on the branch associated with >> 'file', and where all other well-known keynames have smaller branches? > > I have no idea about that, I will have a look at QMP flat unions. Markus can help if you get stuck on what it might look like. But by now, there are several examples in .qapi files to copy from (look for 'discriminator'). > >>> + >>> + >>> +## >>> +# @query-fw_cfg-items: >> >> That looks weird to mix - and _. Any reason we can't just go with >> query-firmware-config? > > Way better! I'll use query-firmware-config-items. Do we need the -items suffix? Also, is there a chance that we might ever want to extend the command to return more information that is global to firmware-config rather than per-item? >>> +{ 'command': 'query-fw_cfg-items', 'returns': ['FirmwareConfigurationItem']} As written, this results in: { "return": [ { ... }, { ... } ] } but if you add a layer of nesting, you could have easier extensions: { "return": { "global": {}, "items": [ { ... }, { ... } ] } }
On 3/8/19 6:31 PM, Eric Blake wrote: > On 3/8/19 5:08 AM, Philippe Mathieu-Daudé wrote: > >>>> { >>>> "return": [ >>>> { >>>> "architecture_specific": false, >>>> "key": 0, >>>> "writeable": false, >>>> "size": 4, >>>> "keyname": "signature" >>> >>> You could return a base64-encoded representation of the field (we do >>> that in other interfaces where raw binary can't be passed reliably over >>> JSON). For 4 bytes, that makes sense, >>> >>> >>>> { >>>> "architecture_specific": true, >>>> "key": 3, >>>> "writeable": false, >>>> "size": 324, >>>> "keyname": "e820_tables" >>> >>> for 324 bytes, it gets a bit long. Still, may be worth it for the added >>> debug visibility. >> >> Until you see values in the next patch...: >> >> $ (echo info fw_cfg; echo q) | qemu-system-x86_64 -S -monitor stdio >> Selector Well-Known Key Pathname ArchSpec Perm Size Order Hex Data >> [...] >> 0x0019 file_dir RO 2052 0000000b.. >> 0x0022 file: etc/acpi/tables RO 131072 130 46414353.. > > Yeah, that's a no-go. >> >> What about using a 512B limit on this QMP answer? > > Seems reasonable. Either omit the field when its size exceeds the limit, > or use the field to give a truncated version (where the size field still > shows the full length, either way). OK. >>>> +# @path: If the key is a 'file', the named file path. >>>> +# @order: If the key is a 'file', the named file order. >>>> +# >>>> +# Since 4.0 >>>> +## >>>> +{ 'struct': 'FirmwareConfigurationItem', >>>> + 'data': { 'key': 'uint16', >>>> + '*keyname': 'str', >>>> + 'architecture_specific': 'bool', >>>> + 'writeable': 'bool', >>>> + '*data': 'str', >>>> + 'size': 'int', >>>> + '*path': 'str', >>>> + '*order': 'int' } } >>> >>> Is it worth making 'keyname' an enum type, and turning this into a flat >>> union where 'path' and 'order' appear on the branch associated with >>> 'file', and where all other well-known keynames have smaller branches? >> >> I have no idea about that, I will have a look at QMP flat unions. > > Markus can help if you get stuck on what it might look like. But by now, > there are several examples in .qapi files to copy from (look for > 'discriminator'). OK. >> >>>> + >>>> + >>>> +## >>>> +# @query-fw_cfg-items: >>> >>> That looks weird to mix - and _. Any reason we can't just go with >>> query-firmware-config? >> >> Way better! I'll use query-firmware-config-items. > > Do we need the -items suffix? Also, is there a chance that we might ever > want to extend the command to return more information that is global to > firmware-config rather than per-item? Laszlo suggested it could be useful to ask for a specific item (with the full data encoded?). >>>> +{ 'command': 'query-fw_cfg-items', 'returns': ['FirmwareConfigurationItem']} > > As written, this results in: > > { "return": [ { ... }, { ... } ] } > > but if you add a layer of nesting, you could have easier extensions: > > { "return": { "global": {}, "items": [ { ... }, { ... } ] } } > Oh I guess I got it, I'll try it. Thanks! Phil.
On 03/08/19 19:07, Philippe Mathieu-Daudé wrote: > On 3/8/19 6:31 PM, Eric Blake wrote: >> On 3/8/19 5:08 AM, Philippe Mathieu-Daudé wrote: >> >>>>> { >>>>> "return": [ >>>>> { >>>>> "architecture_specific": false, >>>>> "key": 0, >>>>> "writeable": false, >>>>> "size": 4, >>>>> "keyname": "signature" >>>> >>>> You could return a base64-encoded representation of the field (we do >>>> that in other interfaces where raw binary can't be passed reliably over >>>> JSON). For 4 bytes, that makes sense, >>>> >>>> >>>>> { >>>>> "architecture_specific": true, >>>>> "key": 3, >>>>> "writeable": false, >>>>> "size": 324, >>>>> "keyname": "e820_tables" >>>> >>>> for 324 bytes, it gets a bit long. Still, may be worth it for the added >>>> debug visibility. >>> >>> Until you see values in the next patch...: >>> >>> $ (echo info fw_cfg; echo q) | qemu-system-x86_64 -S -monitor stdio >>> Selector Well-Known Key Pathname ArchSpec Perm Size Order Hex Data >>> [...] >>> 0x0019 file_dir RO 2052 0000000b.. >>> 0x0022 file: etc/acpi/tables RO 131072 130 46414353.. >> >> Yeah, that's a no-go. >>> >>> What about using a 512B limit on this QMP answer? >> >> Seems reasonable. Either omit the field when its size exceeds the limit, >> or use the field to give a truncated version (where the size field still >> shows the full length, either way). > > OK. > >>>>> +# @path: If the key is a 'file', the named file path. >>>>> +# @order: If the key is a 'file', the named file order. >>>>> +# >>>>> +# Since 4.0 >>>>> +## >>>>> +{ 'struct': 'FirmwareConfigurationItem', >>>>> + 'data': { 'key': 'uint16', >>>>> + '*keyname': 'str', >>>>> + 'architecture_specific': 'bool', >>>>> + 'writeable': 'bool', >>>>> + '*data': 'str', >>>>> + 'size': 'int', >>>>> + '*path': 'str', >>>>> + '*order': 'int' } } >>>> >>>> Is it worth making 'keyname' an enum type, and turning this into a flat >>>> union where 'path' and 'order' appear on the branch associated with >>>> 'file', and where all other well-known keynames have smaller branches? >>> >>> I have no idea about that, I will have a look at QMP flat unions. >> >> Markus can help if you get stuck on what it might look like. But by now, >> there are several examples in .qapi files to copy from (look for >> 'discriminator'). > > OK. > >>> >>>>> + >>>>> + >>>>> +## >>>>> +# @query-fw_cfg-items: >>>> >>>> That looks weird to mix - and _. Any reason we can't just go with >>>> query-firmware-config? >>> >>> Way better! I'll use query-firmware-config-items. >> >> Do we need the -items suffix? Also, is there a chance that we might ever >> want to extend the command to return more information that is global to >> firmware-config rather than per-item? > > Laszlo suggested it could be useful to ask for a specific item (with the > full data encoded?). It's possible I referred to that a long time ago, but most recently, I think it has been raised by Dave. Thanks Laszlo > >>>>> +{ 'command': 'query-fw_cfg-items', 'returns': ['FirmwareConfigurationItem']} >> >> As written, this results in: >> >> { "return": [ { ... }, { ... } ] } >> >> but if you add a layer of nesting, you could have easier extensions: >> >> { "return": { "global": {}, "items": [ { ... }, { ... } ] } } >> > > Oh I guess I got it, I'll try it. > > Thanks! > > Phil. >
On 3/8/19 9:00 PM, Laszlo Ersek wrote: > On 03/08/19 19:07, Philippe Mathieu-Daudé wrote: >> On 3/8/19 6:31 PM, Eric Blake wrote: >>> On 3/8/19 5:08 AM, Philippe Mathieu-Daudé wrote: >>>>>> +## >>>>>> +# @query-fw_cfg-items: >>>>> >>>>> That looks weird to mix - and _. Any reason we can't just go with >>>>> query-firmware-config? >>>> >>>> Way better! I'll use query-firmware-config-items. >>> >>> Do we need the -items suffix? Also, is there a chance that we might ever >>> want to extend the command to return more information that is global to >>> firmware-config rather than per-item? >> >> Laszlo suggested it could be useful to ask for a specific item (with the >> full data encoded?). > > It's possible I referred to that a long time ago, but most recently, I > think it has been raised by Dave. One of your 'random' ideas :) https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg01959.html (6) If we want bells and whistles, two optional parameters could be considered: one for identifying one specific key to request info about (identify by selector? well-known macro name? file pathname?), and another param for placing a limit, different from 8, on the individual hexdump size. Important: these are not "requirements", just random ideas, food for thought. I'm fine if you reject any subset of them, after consideration. > > Thanks > Laszlo
The subject "Add QMP 'info fw_cfg' command" misspells query-fw_cfg-items :) Eric Blake <eblake@redhat.com> writes: > On 3/7/19 7:32 PM, Philippe Mathieu-Daudé wrote: >> When debugging a paravirtualized guest firmware, it results very >> useful to dump the fw_cfg table. >> Add a QMP command which returns the most useful fields. >> Since the QMP protocol is not designed for passing stream data, >> we don't display a fw_cfg item data, only it's size: >> >> { "execute": "query-fw_cfg-items" } >> { >> "return": [ >> { >> "architecture_specific": false, >> "key": 0, >> "writeable": false, >> "size": 4, >> "keyname": "signature" > > You could return a base64-encoded representation of the field (we do > that in other interfaces where raw binary can't be passed reliably over > JSON). For 4 bytes, that makes sense, > > >> { >> "architecture_specific": true, >> "key": 3, >> "writeable": false, >> "size": 324, >> "keyname": "e820_tables" > > for 324 bytes, it gets a bit long. Still, may be worth it for the added > debug visibility. > > >> +++ b/qapi/misc.json >> @@ -3051,3 +3051,47 @@ >> 'data': 'NumaOptions', >> 'allow-preconfig': true >> } >> + >> +## >> +# @FirmwareConfigurationItem: >> +# >> +# Firmware Configuration (fw_cfg) item. >> +# >> +# @key: The uint16 selector key. >> +# @keyname: The stringified name if the selector refers to a well-known >> +# numerically defined item. Ignorant questions, I'm afraid... What determines the possible values of @key? What's the difference between a "well-known" and a "not well-known" value? Examples? >> +# @architecture_specific: Indicates whether the configuration setting is > > Prefer '-' over '_' in new interfaces. > >> +# architecture specific. >> +# false: The item is a generic configuration item. >> +# true: The item is specific to a particular architecture. >> +# @writeable: Indicates whether the configuration setting is writeable by >> +# the guest. > > writable > >> +# @size: The length of @data associated with the item. >> +# @data: A string representating the firmware configuration data. > > representing > >> +# Note: This field is currently not used. > > Either drop the field until it has a use, or let it be the base64 > encoding and use it now. > >> +# @path: If the key is a 'file', the named file path. >> +# @order: If the key is a 'file', the named file order. >> +# >> +# Since 4.0 >> +## >> +{ 'struct': 'FirmwareConfigurationItem', >> + 'data': { 'key': 'uint16', >> + '*keyname': 'str', >> + 'architecture_specific': 'bool', >> + 'writeable': 'bool', >> + '*data': 'str', >> + 'size': 'int', >> + '*path': 'str', >> + '*order': 'int' } } > > Is it worth making 'keyname' an enum type, and turning this into a flat > union where 'path' and 'order' appear on the branch associated with > 'file', and where all other well-known keynames have smaller branches? Discriminator can't be optional. Obvious solution: add a suitabable enum value for "other" key values. Leads to something like this (untested): { 'union': 'FirmwareConfigurationItem', 'base': { 'key': 'uint16', 'keyname': 'FirmwareConfigurationKey', ... more members that make sense regardless of @key ... }, 'discriminator': 'key', 'data': { 'file': 'FirmwareConfigurationItemFile', ... more variants, if any ... } } where 'FirmwareConfigurationItemFile' is a struct type containing the members that make sense just for 'file'. >> + >> + >> +## >> +# @query-fw_cfg-items: > > That looks weird to mix - and _. Any reason we can't just go with > query-firmware-config? > >> +# >> +# Returns the list of Firmware Configuration items. >> +# >> +# Returns: A list of @FirmwareConfigurationItem for each entry. >> +# >> +# Since 4.0 >> +## >> +{ 'command': 'query-fw_cfg-items', 'returns': ['FirmwareConfigurationItem']} >>
Hi Markus, On 03/09/19 16:04, Markus Armbruster wrote: > The subject "Add QMP 'info fw_cfg' command" misspells query-fw_cfg-items > :) > > Eric Blake <eblake@redhat.com> writes: > >> On 3/7/19 7:32 PM, Philippe Mathieu-Daudé wrote: >>> +## >>> +# @FirmwareConfigurationItem: >>> +# >>> +# Firmware Configuration (fw_cfg) item. >>> +# >>> +# @key: The uint16 selector key. >>> +# @keyname: The stringified name if the selector refers to a well-known >>> +# numerically defined item. > > Ignorant questions, I'm afraid... > > What determines the possible values of @key? the text file "docs/specs/fw_cfg.txt" explains this somewhat. - In section "Selector (Control) Register", it lays out the structure of the key space. > What's the difference between a "well-known" and a "not well-known" > value? Examples? - In section "Firmware Configuration Items", it explains the selectors FW_CFG_SIGNATURE, FW_CFG_ID, FW_CFG_FILE_DIR. - Then it "implies" readers should consult "include/standard-headers/linux/qemu_fw_cfg.h". :) All in all, a selector is "well known" if the QEMU source code defines -- perhaps through incorporating a kernel include file -- a macro directly for the numeric value of the selector key. Such a selector is "well known" because guest firmware is allowed / required to refer directly to the numeric value, rather than to look it up in the fw_cfg file directory, by name (such as "etc/table-loader", "bootorder", ...). Thanks! Laszlo
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index fc392cb7e0..2a8d69ba07 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -35,6 +35,7 @@ #include "qemu/config-file.h" #include "qemu/cutils.h" #include "qapi/error.h" +#include "qapi/qapi-commands-misc.h" #define FW_CFG_FILE_SLOTS_DFLT 0x20 @@ -1229,3 +1230,78 @@ static void fw_cfg_register_types(void) } type_init(fw_cfg_register_types) + +static FirmwareConfigurationItem *create_qmp_fw_cfg_item(FWCfgState *s, + FWCfgEntry *e, + bool is_arch_specific, + uint16_t key, + size_t hex_length) +{ + FirmwareConfigurationItem *item = g_malloc0(sizeof(*item)); + + item->key = key; + item->writeable = e->allow_write; + item->architecture_specific = is_arch_specific; + item->size = e->len; + if (hex_length) { + item->has_data = true; + item->data = qemu_strdup_hexlify(e->data, hex_length); + } + + if (!is_arch_specific && key >= FW_CFG_FILE_FIRST) { + int id = key - FW_CFG_FILE_FIRST; + const char *path = s->files->f[id].name; + + item->has_keyname = true; + item->keyname = g_strdup("file"); + item->has_order = true; + item->order = get_fw_cfg_order(s, path); + item->has_path = true; + item->path = g_strdup(path); + } else { + const char *name; + + if (is_arch_specific) { + key |= FW_CFG_ARCH_LOCAL; + } + name = key_name(key); + if (name) { + item->has_keyname = true; + item->keyname = g_strdup(name); + } + } + return item; +} + +FirmwareConfigurationItemList *qmp_query_fw_cfg_items(Error **errp) +{ + FirmwareConfigurationItemList *item_list = NULL; + uint32_t max_entries; + int arch, key; + FWCfgState *s = fw_cfg_find(); + + if (s == NULL) { + return NULL; + } + + max_entries = fw_cfg_max_entry(s); + for (arch = ARRAY_SIZE(s->entries) - 1; arch >= 0 ; --arch) { + for (key = max_entries - 1; key >= 0; --key) { + FirmwareConfigurationItemList *info; + FWCfgEntry *e = &s->entries[arch][key]; + size_t qmp_hex_length = 0; + + if (!e->len) { + continue; + } + + info = g_malloc0(sizeof(*info)); + info->value = create_qmp_fw_cfg_item(s, e, arch, key, + qmp_hex_length); + info->next = item_list; + item_list = info; + } + } + + return item_list; +} diff --git a/qapi/misc.json b/qapi/misc.json index 8b3ca4fdd3..9d1da7c766 100644 --- a/qapi/misc.json +++ b/qapi/misc.json @@ -3051,3 +3051,47 @@ 'data': 'NumaOptions', 'allow-preconfig': true } + +## +# @FirmwareConfigurationItem: +# +# Firmware Configuration (fw_cfg) item. +# +# @key: The uint16 selector key. +# @keyname: The stringified name if the selector refers to a well-known +# numerically defined item. +# @architecture_specific: Indicates whether the configuration setting is +# architecture specific. +# false: The item is a generic configuration item. +# true: The item is specific to a particular architecture. +# @writeable: Indicates whether the configuration setting is writeable by +# the guest. +# @size: The length of @data associated with the item. +# @data: A string representating the firmware configuration data. +# Note: This field is currently not used. +# @path: If the key is a 'file', the named file path. +# @order: If the key is a 'file', the named file order. +# +# Since 4.0 +## +{ 'struct': 'FirmwareConfigurationItem', + 'data': { 'key': 'uint16', + '*keyname': 'str', + 'architecture_specific': 'bool', + 'writeable': 'bool', + '*data': 'str', + 'size': 'int', + '*path': 'str', + '*order': 'int' } } + + +## +# @query-fw_cfg-items: +# +# Returns the list of Firmware Configuration items. +# +# Returns: A list of @FirmwareConfigurationItem for each entry. +# +# Since 4.0 +## +{ 'command': 'query-fw_cfg-items', 'returns': ['FirmwareConfigurationItem']}
When debugging a paravirtualized guest firmware, it results very useful to dump the fw_cfg table. Add a QMP command which returns the most useful fields. Since the QMP protocol is not designed for passing stream data, we don't display a fw_cfg item data, only it's size: { "execute": "query-fw_cfg-items" } { "return": [ { "architecture_specific": false, "key": 0, "writeable": false, "size": 4, "keyname": "signature" }, { "architecture_specific": false, "key": 1, "writeable": false, "size": 4, "keyname": "id" }, { "architecture_specific": false, "key": 2, "writeable": false, "size": 16, "keyname": "uuid" }, ... { "order": 40, "architecture_specific": false, "key": 36, "writeable": false, "path": "etc/e820", "size": 20, "keyname": "file" }, { "order": 30, "architecture_specific": false, "key": 37, "writeable": false, "path": "etc/smbios/smbios-anchor", "size": 31, "keyname": "file" }, ... { "architecture_specific": true, "key": 3, "writeable": false, "size": 324, "keyname": "e820_tables" }, { "architecture_specific": true, "key": 4, "writeable": false, "size": 121, "keyname": "hpet" } ] } Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- v2: New commit, asked by Eric/Michael, using Laszlo suggestions --- hw/nvram/fw_cfg.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++ qapi/misc.json | 44 +++++++++++++++++++++++++++ 2 files changed, 120 insertions(+)