[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'i
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command |
Date: |
Sat, 09 Mar 2019 16:04:27 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
The subject "Add QMP 'info fw_cfg' command" misspells query-fw_cfg-items
:)
Eric Blake <address@hidden> 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']}
>>
- [Qemu-ppc] [PATCH v2 12/18] hw/nvram/fw_cfg: Keep reference of file_data in FWCfgState, (continued)
- [Qemu-ppc] [PATCH v2 12/18] hw/nvram/fw_cfg: Keep reference of file_data in FWCfgState, Philippe Mathieu-Daudé, 2019/03/07
- [Qemu-ppc] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command, Philippe Mathieu-Daudé, 2019/03/07
- Re: [Qemu-ppc] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command, Eric Blake, 2019/03/07
- Re: [Qemu-ppc] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command, Philippe Mathieu-Daudé, 2019/03/08
- Re: [Qemu-ppc] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command, Eric Blake, 2019/03/08
- Re: [Qemu-ppc] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command, Philippe Mathieu-Daudé, 2019/03/08
- Re: [Qemu-ppc] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command, Laszlo Ersek, 2019/03/08
- Re: [Qemu-ppc] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command, Philippe Mathieu-Daudé, 2019/03/08
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command,
Markus Armbruster <=
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 13/18] hw/nvram/fw_cfg: Add QMP 'info fw_cfg' command, Laszlo Ersek, 2019/03/12
[Qemu-ppc] [PATCH v2 15/18] hw/nvram/fw_cfg: Add fw_cfg_add_file_from_host(), Philippe Mathieu-Daudé, 2019/03/07
[Qemu-ppc] [PATCH v2 14/18] hw/nvram/fw_cfg: Add HMP 'info fw_cfg' command, Philippe Mathieu-Daudé, 2019/03/07
[Qemu-ppc] [PATCH v2 16/18] hw/firmware: Add Edk2Crypto and edk2_add_host_crypto_policy(), Philippe Mathieu-Daudé, 2019/03/07
[Qemu-ppc] [PATCH v2 17/18] hw/i386: Use edk2_add_host_crypto_policy(), Philippe Mathieu-Daudé, 2019/03/07
[Qemu-ppc] [PATCH v2 18/18] hw/arm/virt: Use edk2_add_host_crypto_policy(), Philippe Mathieu-Daudé, 2019/03/07
Re: [Qemu-ppc] [PATCH v2 00/18] fw_cfg: reduce memleaks, add QMP/HMP info + edk2_add_host_crypto_policy, Laszlo Ersek, 2019/03/08