$ qemu-system-x86_64 -S -monitor stdio
(qemu) info fw_cfg
Type Perm Size Specific Order Info
signature RO 4 QEMU
id RO 4 0x00000003
uuid RO 16 00000000-0000-0000-0000-000000000000
ram_size RO 8 0x0000000008000000
nographic RO 2 0x0000
nb_cpus RO 2 0x0001
numa RO 16
boot_menu RO 2 0x0000
max_cpus RO 2 0x0001
file_dir RO 2052
file (id 1) RO 36 160 etc/acpi/rsdp
file (id 2) RO 131072 130 etc/acpi/tables
file (id 3) RO 4 15 etc/boot-fail-wait
file (id 4) RO 20 40 etc/e820
file (id 5) RO 31 30 etc/smbios/smbios-anchor
file (id 6) RO 320 20 etc/smbios/smbios-tables
file (id 7) RO 6 90 etc/system-states
file (id 8) RO 4096 140 etc/table-loader
file (id 10) RO 9216 55 genroms/kvmvapic.bin
uuid RO 4 (arch spec) 01000000-0000-0000-0000-000000000000
ram_size RO 324 (arch spec)
nographic RO 121 (arch spec)
(qemu)
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hmp-commands-info.hx | 14 +++++
hw/nvram/fw_cfg.c | 115 ++++++++++++++++++++++++++++++++++++++
include/hw/nvram/fw_cfg.h | 2 +
3 files changed, 131 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index cbee8b944d..9e86dceeb4 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -916,6 +916,20 @@ STEXI
@item info sev
@findex info sev
Show SEV information.
+ETEXI
+
+ {
+ .name = "fw_cfg",
+ .args_type = "",
+ .params = "",
+ .help = "Display the table of the fw_cfg entries registered",
+ .cmd = hmp_info_fw_cfg,
+ },
+
+STEXI
+@item info fw_cfg
+@findex info fw_cfg
+Show roms.
ETEXI
STEXI
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 582653f07e..50525ec1dd 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -36,6 +36,7 @@
#include "qemu/config-file.h"
#include "qemu/cutils.h"
#include "qapi/error.h"
+#include "monitor/monitor.h"
#define FW_CFG_FILE_SLOTS_DFLT 0x20
@@ -1164,3 +1165,117 @@ static void fw_cfg_register_types(void)
}
type_init(fw_cfg_register_types)
+
+static const char *fw_cfg_wellknown_entries[0x20] = {
+ [FW_CFG_SIGNATURE] = "signature",
+ [FW_CFG_ID] = "id",
+ [FW_CFG_UUID] = "uuid",
+ [FW_CFG_RAM_SIZE] = "ram_size",
+ [FW_CFG_NOGRAPHIC] = "nographic",
+ [FW_CFG_NB_CPUS] = "nb_cpus",
+ [FW_CFG_MACHINE_ID] = "machine_id",
+ [FW_CFG_KERNEL_ADDR] = "kernel_addr",
+ [FW_CFG_KERNEL_SIZE] = "kernel_size",
+ [FW_CFG_KERNEL_CMDLINE] = "kernel_cmdline",
+ [FW_CFG_INITRD_ADDR] = "initrd_addr",
+ [FW_CFG_INITRD_SIZE] = "initdr_size",
+ [FW_CFG_BOOT_DEVICE] = "boot_device",
+ [FW_CFG_NUMA] = "numa",
+ [FW_CFG_BOOT_MENU] = "boot_menu",
+ [FW_CFG_MAX_CPUS] = "max_cpus",
+ [FW_CFG_KERNEL_ENTRY] = "kernel_entry",
+ [FW_CFG_KERNEL_DATA] = "kernel_data",
+ [FW_CFG_INITRD_DATA] = "initrd_data",
+ [FW_CFG_CMDLINE_ADDR] = "cmdline_addr",
+ [FW_CFG_CMDLINE_SIZE] = "cmdline_size",
+ [FW_CFG_CMDLINE_DATA] = "cmdline_data",
+ [FW_CFG_SETUP_ADDR] = "setup_addr",
+ [FW_CFG_SETUP_SIZE] = "setup_size",
+ [FW_CFG_SETUP_DATA] = "setup_data",
+ [FW_CFG_FILE_DIR] = "file_dir",
+};
+
+void hmp_info_fw_cfg(Monitor *mon, const QDict *qdict)
+{
+ FWCfgState *s = fw_cfg_find();
+ int arch, key;
+
+ monitor_printf(mon, "%12s %5s %7s %9s %6s %-24s\n",
+ "Type", "Perm", "Size", "Specific", "Order", "Info");
+ for (arch = 0; arch < ARRAY_SIZE(s->entries); ++arch) {
+ for (key = 0; key < fw_cfg_max_entry(s); ++key) {
+ FWCfgEntry *e = &s->entries[arch][key];
+ const char *perm = e->allow_write ? "RW" : "RO";
+ const char *arch_spec = arch ? "arch_spec" : "";
+ char *type, *info = NULL;
+
+ if (!e->len) {
+ continue;
+ }
+ if (key >= FW_CFG_FILE_FIRST) {
+ int id = key - FW_CFG_FILE_FIRST;
+ const char *path = s->files->f[id].name;
+
+ type = g_strdup_printf("file (id %d)", id);
+ monitor_printf(mon, "%12s %5s %7d %10s %5d %-24s\n",
+ type, perm, e->len, arch_spec,
+ get_fw_cfg_order(s, path), path);
+ g_free(type);
+ continue;
+ }
+ type = g_strdup(fw_cfg_wellknown_entries[key]);
+ if (!type) {
+ type = g_strdup_printf("entry_%d", key);
+ }
+
+ switch (key) {
+ case FW_CFG_SIGNATURE:
+ info = g_strndup((const gchar *)e->data, e->len);
+ break;
+ case FW_CFG_UUID:
+ info = qemu_uuid_unparse_strdup((const QemuUUID *)e->data);
+ break;
+ case FW_CFG_ID:
+ case FW_CFG_NOGRAPHIC:
+ case FW_CFG_NB_CPUS:
+ case FW_CFG_BOOT_MENU:
+ case FW_CFG_MAX_CPUS:
+ case FW_CFG_RAM_SIZE:
+ case FW_CFG_KERNEL_ADDR:
+ case FW_CFG_KERNEL_SIZE:
+ case FW_CFG_KERNEL_CMDLINE:
+ case FW_CFG_KERNEL_ENTRY:
+ case FW_CFG_KERNEL_DATA:
+ case FW_CFG_INITRD_ADDR:
+ case FW_CFG_INITRD_SIZE:
+ case FW_CFG_INITRD_DATA:
+ case FW_CFG_CMDLINE_ADDR:
+ case FW_CFG_CMDLINE_SIZE:
+ case FW_CFG_CMDLINE_DATA:
+ case FW_CFG_SETUP_ADDR:
+ case FW_CFG_SETUP_SIZE:
+ case FW_CFG_SETUP_DATA:
+ switch (e->len) {
+ case 2:
+ info = g_strdup_printf("0x%04x", lduw_le_p(e->data));
+ break;
+ case 4:
+ info = g_strdup_printf("0x%08x", ldl_le_p(e->data));
+ break;
+ case 8:
+ info = g_strdup_printf("0x%016" PRIx64, ldq_le_p(e->data));
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+ monitor_printf(mon, "%12s %5s %7d %10s %5s %-24s\n",
+ type, perm, e->len, arch_spec, "", info ? info : "");
+ g_free(type);
+ g_free(info);
+ }
+ }
+}
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index f5a6895a74..28630b2f26 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -226,4 +226,6 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
FWCfgState *fw_cfg_find(void);
bool fw_cfg_dma_enabled(void *opaque);
+void hmp_info_fw_cfg(Monitor *mon, const QDict *qdict);
+
#endif
--
2.17.2
On 12/7/18 11:03 AM, Philippe Mathieu-Daudé wrote: > $ qemu-system-x86_64 -S -monitor stdio > (qemu) info fw_cfg > Type Perm Size Specific Order Info > signature RO 4 QEMU > id RO 4 0x00000003 > uuid RO 16 00000000-0000-0000-0000-000000000000 > ram_size RO 8 0x0000000008000000 > nographic RO 2 0x0000 > nb_cpus RO 2 0x0001 > numa RO 16 > boot_menu RO 2 0x0000 > max_cpus RO 2 0x0001 > file_dir RO 2052 > file (id 1) RO 36 160 etc/acpi/rsdp > file (id 2) RO 131072 130 etc/acpi/tables > file (id 3) RO 4 15 etc/boot-fail-wait > file (id 4) RO 20 40 etc/e820 > file (id 5) RO 31 30 etc/smbios/smbios-anchor > file (id 6) RO 320 20 etc/smbios/smbios-tables > file (id 7) RO 6 90 etc/system-states > file (id 8) RO 4096 140 etc/table-loader > file (id 10) RO 9216 55 genroms/kvmvapic.bin > uuid RO 4 (arch spec) 01000000-0000-0000-0000-000000000000 > ram_size RO 324 (arch spec) > nographic RO 121 (arch spec) > (qemu) > In general, when adding something to HMP but not QMP, it is worth a comment explaining WHY we did not want QMP. (Here, I think the reason is that this is a debugging aid likely to be useful to developers but unlikely to be needed by a management app, and matches similar other HMP-only info commands, and is therefore not worth the effort of turning it into QAPI). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Fri, Dec 07, 2018 at 06:03:59PM +0100, Philippe Mathieu-Daudé wrote:
> $ qemu-system-x86_64 -S -monitor stdio
> (qemu) info fw_cfg
> Type Perm Size Specific Order Info
Can we do better than "Info"?
> signature RO 4 QEMU
> id RO 4 0x00000003
> uuid RO 16 00000000-0000-0000-0000-000000000000
> ram_size RO 8 0x0000000008000000
> nographic RO 2 0x0000
> nb_cpus RO 2 0x0001
> numa RO 16
> boot_menu RO 2 0x0000
> max_cpus RO 2 0x0001
> file_dir RO 2052
> file (id 1) RO 36 160 etc/acpi/rsdp
> file (id 2) RO 131072 130 etc/acpi/tables
> file (id 3) RO 4 15 etc/boot-fail-wait
> file (id 4) RO 20 40 etc/e820
> file (id 5) RO 31 30 etc/smbios/smbios-anchor
> file (id 6) RO 320 20 etc/smbios/smbios-tables
> file (id 7) RO 6 90 etc/system-states
> file (id 8) RO 4096 140 etc/table-loader
> file (id 10) RO 9216 55 genroms/kvmvapic.bin
> uuid RO 4 (arch spec) 01000000-0000-0000-0000-000000000000
> ram_size RO 324 (arch spec)
> nographic RO 121 (arch spec)
Weird. Your code has arch_spec.
> (qemu)
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Looks helpful but generally IMHO whatever is exposed through hmp
should be in the qmp as well.
> ---
> hmp-commands-info.hx | 14 +++++
> hw/nvram/fw_cfg.c | 115 ++++++++++++++++++++++++++++++++++++++
> include/hw/nvram/fw_cfg.h | 2 +
> 3 files changed, 131 insertions(+)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index cbee8b944d..9e86dceeb4 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -916,6 +916,20 @@ STEXI
> @item info sev
> @findex info sev
> Show SEV information.
> +ETEXI
> +
> + {
> + .name = "fw_cfg",
> + .args_type = "",
> + .params = "",
> + .help = "Display the table of the fw_cfg entries registered",
I think the help line should be a bit more verbose.
Mention it's a paravirtualized interface, and why
would one want to display it (debugging guest firmware?).
> + .cmd = hmp_info_fw_cfg,
> + },
> +
> +STEXI
> +@item info fw_cfg
> +@findex info fw_cfg
> +Show roms.
> ETEXI
>
> STEXI
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 582653f07e..50525ec1dd 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -36,6 +36,7 @@
> #include "qemu/config-file.h"
> #include "qemu/cutils.h"
> #include "qapi/error.h"
> +#include "monitor/monitor.h"
>
> #define FW_CFG_FILE_SLOTS_DFLT 0x20
>
> @@ -1164,3 +1165,117 @@ static void fw_cfg_register_types(void)
> }
>
> type_init(fw_cfg_register_types)
> +
> +static const char *fw_cfg_wellknown_entries[0x20] = {
> + [FW_CFG_SIGNATURE] = "signature",
> + [FW_CFG_ID] = "id",
> + [FW_CFG_UUID] = "uuid",
> + [FW_CFG_RAM_SIZE] = "ram_size",
> + [FW_CFG_NOGRAPHIC] = "nographic",
> + [FW_CFG_NB_CPUS] = "nb_cpus",
> + [FW_CFG_MACHINE_ID] = "machine_id",
> + [FW_CFG_KERNEL_ADDR] = "kernel_addr",
> + [FW_CFG_KERNEL_SIZE] = "kernel_size",
> + [FW_CFG_KERNEL_CMDLINE] = "kernel_cmdline",
> + [FW_CFG_INITRD_ADDR] = "initrd_addr",
> + [FW_CFG_INITRD_SIZE] = "initdr_size",
> + [FW_CFG_BOOT_DEVICE] = "boot_device",
> + [FW_CFG_NUMA] = "numa",
> + [FW_CFG_BOOT_MENU] = "boot_menu",
> + [FW_CFG_MAX_CPUS] = "max_cpus",
> + [FW_CFG_KERNEL_ENTRY] = "kernel_entry",
> + [FW_CFG_KERNEL_DATA] = "kernel_data",
> + [FW_CFG_INITRD_DATA] = "initrd_data",
> + [FW_CFG_CMDLINE_ADDR] = "cmdline_addr",
> + [FW_CFG_CMDLINE_SIZE] = "cmdline_size",
> + [FW_CFG_CMDLINE_DATA] = "cmdline_data",
> + [FW_CFG_SETUP_ADDR] = "setup_addr",
> + [FW_CFG_SETUP_SIZE] = "setup_size",
> + [FW_CFG_SETUP_DATA] = "setup_data",
> + [FW_CFG_FILE_DIR] = "file_dir",
> +};
> +
> +void hmp_info_fw_cfg(Monitor *mon, const QDict *qdict)
> +{
> + FWCfgState *s = fw_cfg_find();
> + int arch, key;
Looks like this will crash on a machine without fw cfg.
> +
> + monitor_printf(mon, "%12s %5s %7s %9s %6s %-24s\n",
> + "Type", "Perm", "Size", "Specific", "Order", "Info");
> + for (arch = 0; arch < ARRAY_SIZE(s->entries); ++arch) {
> + for (key = 0; key < fw_cfg_max_entry(s); ++key) {
> + FWCfgEntry *e = &s->entries[arch][key];
> + const char *perm = e->allow_write ? "RW" : "RO";
> + const char *arch_spec = arch ? "arch_spec" : "";
> + char *type, *info = NULL;
> +
> + if (!e->len) {
> + continue;
> + }
> + if (key >= FW_CFG_FILE_FIRST) {
> + int id = key - FW_CFG_FILE_FIRST;
> + const char *path = s->files->f[id].name;
> +
> + type = g_strdup_printf("file (id %d)", id);
> + monitor_printf(mon, "%12s %5s %7d %10s %5d %-24s\n",
> + type, perm, e->len, arch_spec,
> + get_fw_cfg_order(s, path), path);
> + g_free(type);
> + continue;
> + }
> + type = g_strdup(fw_cfg_wellknown_entries[key]);
> + if (!type) {
> + type = g_strdup_printf("entry_%d", key);
> + }
> +
> + switch (key) {
> + case FW_CFG_SIGNATURE:
> + info = g_strndup((const gchar *)e->data, e->len);
> + break;
> + case FW_CFG_UUID:
> + info = qemu_uuid_unparse_strdup((const QemuUUID *)e->data);
> + break;
> + case FW_CFG_ID:
> + case FW_CFG_NOGRAPHIC:
> + case FW_CFG_NB_CPUS:
> + case FW_CFG_BOOT_MENU:
> + case FW_CFG_MAX_CPUS:
> + case FW_CFG_RAM_SIZE:
> + case FW_CFG_KERNEL_ADDR:
> + case FW_CFG_KERNEL_SIZE:
> + case FW_CFG_KERNEL_CMDLINE:
> + case FW_CFG_KERNEL_ENTRY:
> + case FW_CFG_KERNEL_DATA:
> + case FW_CFG_INITRD_ADDR:
> + case FW_CFG_INITRD_SIZE:
> + case FW_CFG_INITRD_DATA:
> + case FW_CFG_CMDLINE_ADDR:
> + case FW_CFG_CMDLINE_SIZE:
> + case FW_CFG_CMDLINE_DATA:
> + case FW_CFG_SETUP_ADDR:
> + case FW_CFG_SETUP_SIZE:
> + case FW_CFG_SETUP_DATA:
> + switch (e->len) {
> + case 2:
> + info = g_strdup_printf("0x%04x", lduw_le_p(e->data));
> + break;
> + case 4:
> + info = g_strdup_printf("0x%08x", ldl_le_p(e->data));
> + break;
> + case 8:
> + info = g_strdup_printf("0x%016" PRIx64, ldq_le_p(e->data));
> + break;
> + default:
> + break;
> + }
> + break;
> + default:
> + break;
> + }
> + monitor_printf(mon, "%12s %5s %7d %10s %5s %-24s\n",
> + type, perm, e->len, arch_spec, "", info ? info : "");
> + g_free(type);
> + g_free(info);
> + }
> + }
> +}
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index f5a6895a74..28630b2f26 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -226,4 +226,6 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
> FWCfgState *fw_cfg_find(void);
> bool fw_cfg_dma_enabled(void *opaque);
>
> +void hmp_info_fw_cfg(Monitor *mon, const QDict *qdict);
> +
> #endif
> --
> 2.17.2
On 12/7/18 6:54 PM, Michael S. Tsirkin wrote:
> On Fri, Dec 07, 2018 at 06:03:59PM +0100, Philippe Mathieu-Daudé wrote:
>> $ qemu-system-x86_64 -S -monitor stdio
>> (qemu) info fw_cfg
>> Type Perm Size Specific Order Info
>
> Can we do better than "Info"?
For some entry this is the "content", but for the files this is the
"path". Do you prefer "Content or path"?
>
>> signature RO 4 QEMU
>> id RO 4 0x00000003
>> uuid RO 16 00000000-0000-0000-0000-000000000000
>> ram_size RO 8 0x0000000008000000
>> nographic RO 2 0x0000
>> nb_cpus RO 2 0x0001
>> numa RO 16
>> boot_menu RO 2 0x0000
>> max_cpus RO 2 0x0001
>> file_dir RO 2052
>> file (id 1) RO 36 160 etc/acpi/rsdp
>> file (id 2) RO 131072 130 etc/acpi/tables
>> file (id 3) RO 4 15 etc/boot-fail-wait
>> file (id 4) RO 20 40 etc/e820
>> file (id 5) RO 31 30 etc/smbios/smbios-anchor
>> file (id 6) RO 320 20 etc/smbios/smbios-tables
>> file (id 7) RO 6 90 etc/system-states
>> file (id 8) RO 4096 140 etc/table-loader
>> file (id 10) RO 9216 55 genroms/kvmvapic.bin
>> uuid RO 4 (arch spec) 01000000-0000-0000-0000-000000000000
>> ram_size RO 324 (arch spec)
>> nographic RO 121 (arch spec)
>
> Weird. Your code has arch_spec.
Hmmm I'll check that.
>
>> (qemu)
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>
> Looks helpful but generally IMHO whatever is exposed through hmp
> should be in the qmp as well.
OK.
>
>
>> ---
>> hmp-commands-info.hx | 14 +++++
>> hw/nvram/fw_cfg.c | 115 ++++++++++++++++++++++++++++++++++++++
>> include/hw/nvram/fw_cfg.h | 2 +
>> 3 files changed, 131 insertions(+)
>>
>> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
>> index cbee8b944d..9e86dceeb4 100644
>> --- a/hmp-commands-info.hx
>> +++ b/hmp-commands-info.hx
>> @@ -916,6 +916,20 @@ STEXI
>> @item info sev
>> @findex info sev
>> Show SEV information.
>> +ETEXI
>> +
>> + {
>> + .name = "fw_cfg",
>> + .args_type = "",
>> + .params = "",
>> + .help = "Display the table of the fw_cfg entries registered",
>
> I think the help line should be a bit more verbose.
> Mention it's a paravirtualized interface, and why
> would one want to display it (debugging guest firmware?).
OK.
>
>
>> + .cmd = hmp_info_fw_cfg,
>> + },
>> +
>> +STEXI
>> +@item info fw_cfg
>> +@findex info fw_cfg
>> +Show roms.
>> ETEXI
>>
>> STEXI
>> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
>> index 582653f07e..50525ec1dd 100644
>> --- a/hw/nvram/fw_cfg.c
>> +++ b/hw/nvram/fw_cfg.c
>> @@ -36,6 +36,7 @@
>> #include "qemu/config-file.h"
>> #include "qemu/cutils.h"
>> #include "qapi/error.h"
>> +#include "monitor/monitor.h"
>>
>> #define FW_CFG_FILE_SLOTS_DFLT 0x20
>>
>> @@ -1164,3 +1165,117 @@ static void fw_cfg_register_types(void)
>> }
>>
>> type_init(fw_cfg_register_types)
>> +
>> +static const char *fw_cfg_wellknown_entries[0x20] = {
>> + [FW_CFG_SIGNATURE] = "signature",
>> + [FW_CFG_ID] = "id",
>> + [FW_CFG_UUID] = "uuid",
>> + [FW_CFG_RAM_SIZE] = "ram_size",
>> + [FW_CFG_NOGRAPHIC] = "nographic",
>> + [FW_CFG_NB_CPUS] = "nb_cpus",
>> + [FW_CFG_MACHINE_ID] = "machine_id",
>> + [FW_CFG_KERNEL_ADDR] = "kernel_addr",
>> + [FW_CFG_KERNEL_SIZE] = "kernel_size",
>> + [FW_CFG_KERNEL_CMDLINE] = "kernel_cmdline",
>> + [FW_CFG_INITRD_ADDR] = "initrd_addr",
>> + [FW_CFG_INITRD_SIZE] = "initdr_size",
>> + [FW_CFG_BOOT_DEVICE] = "boot_device",
>> + [FW_CFG_NUMA] = "numa",
>> + [FW_CFG_BOOT_MENU] = "boot_menu",
>> + [FW_CFG_MAX_CPUS] = "max_cpus",
>> + [FW_CFG_KERNEL_ENTRY] = "kernel_entry",
>> + [FW_CFG_KERNEL_DATA] = "kernel_data",
>> + [FW_CFG_INITRD_DATA] = "initrd_data",
>> + [FW_CFG_CMDLINE_ADDR] = "cmdline_addr",
>> + [FW_CFG_CMDLINE_SIZE] = "cmdline_size",
>> + [FW_CFG_CMDLINE_DATA] = "cmdline_data",
>> + [FW_CFG_SETUP_ADDR] = "setup_addr",
>> + [FW_CFG_SETUP_SIZE] = "setup_size",
>> + [FW_CFG_SETUP_DATA] = "setup_data",
>> + [FW_CFG_FILE_DIR] = "file_dir",
>> +};
>> +
>> +void hmp_info_fw_cfg(Monitor *mon, const QDict *qdict)
>> +{
>> + FWCfgState *s = fw_cfg_find();
>> + int arch, key;
>
> Looks like this will crash on a machine without fw cfg.
Oops, good catch :)
>
>
>> +
>> + monitor_printf(mon, "%12s %5s %7s %9s %6s %-24s\n",
>> + "Type", "Perm", "Size", "Specific", "Order", "Info");
>> + for (arch = 0; arch < ARRAY_SIZE(s->entries); ++arch) {
>> + for (key = 0; key < fw_cfg_max_entry(s); ++key) {
>> + FWCfgEntry *e = &s->entries[arch][key];
>> + const char *perm = e->allow_write ? "RW" : "RO";
>> + const char *arch_spec = arch ? "arch_spec" : "";
>> + char *type, *info = NULL;
>> +
>> + if (!e->len) {
>> + continue;
>> + }
>> + if (key >= FW_CFG_FILE_FIRST) {
>> + int id = key - FW_CFG_FILE_FIRST;
>> + const char *path = s->files->f[id].name;
>> +
>> + type = g_strdup_printf("file (id %d)", id);
>> + monitor_printf(mon, "%12s %5s %7d %10s %5d %-24s\n",
>> + type, perm, e->len, arch_spec,
>> + get_fw_cfg_order(s, path), path);
>> + g_free(type);
>> + continue;
>> + }
>> + type = g_strdup(fw_cfg_wellknown_entries[key]);
>> + if (!type) {
>> + type = g_strdup_printf("entry_%d", key);
>> + }
>> +
>> + switch (key) {
>> + case FW_CFG_SIGNATURE:
>> + info = g_strndup((const gchar *)e->data, e->len);
>> + break;
>> + case FW_CFG_UUID:
>> + info = qemu_uuid_unparse_strdup((const QemuUUID *)e->data);
>> + break;
>> + case FW_CFG_ID:
>> + case FW_CFG_NOGRAPHIC:
>> + case FW_CFG_NB_CPUS:
>> + case FW_CFG_BOOT_MENU:
>> + case FW_CFG_MAX_CPUS:
>> + case FW_CFG_RAM_SIZE:
>> + case FW_CFG_KERNEL_ADDR:
>> + case FW_CFG_KERNEL_SIZE:
>> + case FW_CFG_KERNEL_CMDLINE:
>> + case FW_CFG_KERNEL_ENTRY:
>> + case FW_CFG_KERNEL_DATA:
>> + case FW_CFG_INITRD_ADDR:
>> + case FW_CFG_INITRD_SIZE:
>> + case FW_CFG_INITRD_DATA:
>> + case FW_CFG_CMDLINE_ADDR:
>> + case FW_CFG_CMDLINE_SIZE:
>> + case FW_CFG_CMDLINE_DATA:
>> + case FW_CFG_SETUP_ADDR:
>> + case FW_CFG_SETUP_SIZE:
>> + case FW_CFG_SETUP_DATA:
>> + switch (e->len) {
>> + case 2:
>> + info = g_strdup_printf("0x%04x", lduw_le_p(e->data));
>> + break;
>> + case 4:
>> + info = g_strdup_printf("0x%08x", ldl_le_p(e->data));
>> + break;
>> + case 8:
>> + info = g_strdup_printf("0x%016" PRIx64, ldq_le_p(e->data));
>> + break;
>> + default:
>> + break;
>> + }
>> + break;
>> + default:
>> + break;
>> + }
>> + monitor_printf(mon, "%12s %5s %7d %10s %5s %-24s\n",
>> + type, perm, e->len, arch_spec, "", info ? info : "");
>> + g_free(type);
>> + g_free(info);
>> + }
>> + }
>> +}
>> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
>> index f5a6895a74..28630b2f26 100644
>> --- a/include/hw/nvram/fw_cfg.h
>> +++ b/include/hw/nvram/fw_cfg.h
>> @@ -226,4 +226,6 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
>> FWCfgState *fw_cfg_find(void);
>> bool fw_cfg_dma_enabled(void *opaque);
>>
>> +void hmp_info_fw_cfg(Monitor *mon, const QDict *qdict);
>> +
>> #endif
>> --
>> 2.17.2
Hi Michael,
On 12/10/18 10:18 AM, Philippe Mathieu-Daudé wrote:
> On 12/7/18 6:54 PM, Michael S. Tsirkin wrote:
>> On Fri, Dec 07, 2018 at 06:03:59PM +0100, Philippe Mathieu-Daudé wrote:
>>> $ qemu-system-x86_64 -S -monitor stdio
>>> (qemu) info fw_cfg
>>> Type Perm Size Specific Order Info
>>> signature RO 4 QEMU
>>> id RO 4 0x00000003
>>> uuid RO 16 00000000-0000-0000-0000-000000000000
>>> ram_size RO 8 0x0000000008000000
>>> nographic RO 2 0x0000
>>> nb_cpus RO 2 0x0001
>>> numa RO 16
>>> boot_menu RO 2 0x0000
>>> max_cpus RO 2 0x0001
>>> file_dir RO 2052
>>> file (id 1) RO 36 160 etc/acpi/rsdp
>>> file (id 2) RO 131072 130 etc/acpi/tables
>>> file (id 3) RO 4 15 etc/boot-fail-wait
>>> file (id 4) RO 20 40 etc/e820
>>> file (id 5) RO 31 30 etc/smbios/smbios-anchor
>>> file (id 6) RO 320 20 etc/smbios/smbios-tables
>>> file (id 7) RO 6 90 etc/system-states
>>> file (id 8) RO 4096 140 etc/table-loader
>>> file (id 10) RO 9216 55 genroms/kvmvapic.bin
>>> uuid RO 4 (arch spec) 01000000-0000-0000-0000-000000000000
>>> ram_size RO 324 (arch spec)
>>> nographic RO 121 (arch spec)
>>
>> Weird. Your code has arch_spec.
>
> Hmmm I'll check that.
These are the entries used for Bochs:
#define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
#define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
#define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
#define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
#define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
{
[...]
fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
acpi_tables, acpi_tables_len);
fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE,
kvm_allows_irq0_override());
fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
&e820_reserve, sizeof(e820_reserve));
So what happened here is the 'type' name is incorrect (I'll fix), but
the arch_spec flag is correct. Thanks for noticing this :)
Regards,
Phil.
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> $ qemu-system-x86_64 -S -monitor stdio
> (qemu) info fw_cfg
> Type Perm Size Specific Order Info
> signature RO 4 QEMU
> id RO 4 0x00000003
> uuid RO 16 00000000-0000-0000-0000-000000000000
> ram_size RO 8 0x0000000008000000
> nographic RO 2 0x0000
> nb_cpus RO 2 0x0001
> numa RO 16
> boot_menu RO 2 0x0000
> max_cpus RO 2 0x0001
> file_dir RO 2052
> file (id 1) RO 36 160 etc/acpi/rsdp
> file (id 2) RO 131072 130 etc/acpi/tables
> file (id 3) RO 4 15 etc/boot-fail-wait
> file (id 4) RO 20 40 etc/e820
> file (id 5) RO 31 30 etc/smbios/smbios-anchor
> file (id 6) RO 320 20 etc/smbios/smbios-tables
> file (id 7) RO 6 90 etc/system-states
> file (id 8) RO 4096 140 etc/table-loader
> file (id 10) RO 9216 55 genroms/kvmvapic.bin
> uuid RO 4 (arch spec) 01000000-0000-0000-0000-000000000000
> ram_size RO 324 (arch spec)
> nographic RO 121 (arch spec)
> (qemu)
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hmp-commands-info.hx | 14 +++++
> hw/nvram/fw_cfg.c | 115 ++++++++++++++++++++++++++++++++++++++
> include/hw/nvram/fw_cfg.h | 2 +
> 3 files changed, 131 insertions(+)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index cbee8b944d..9e86dceeb4 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -916,6 +916,20 @@ STEXI
> @item info sev
> @findex info sev
> Show SEV information.
> +ETEXI
> +
> + {
> + .name = "fw_cfg",
> + .args_type = "",
> + .params = "",
> + .help = "Display the table of the fw_cfg entries registered",
> + .cmd = hmp_info_fw_cfg,
> + },
> +
> +STEXI
> +@item info fw_cfg
> +@findex info fw_cfg
> +Show roms.
> ETEXI
>
> STEXI
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 582653f07e..50525ec1dd 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -36,6 +36,7 @@
> #include "qemu/config-file.h"
> #include "qemu/cutils.h"
> #include "qapi/error.h"
> +#include "monitor/monitor.h"
>
> #define FW_CFG_FILE_SLOTS_DFLT 0x20
>
> @@ -1164,3 +1165,117 @@ static void fw_cfg_register_types(void)
> }
>
> type_init(fw_cfg_register_types)
> +
> +static const char *fw_cfg_wellknown_entries[0x20] = {
That magic 0x20 is a shame; we should probably have a #define
for that max, or perhaps it can just be removed.
> + [FW_CFG_SIGNATURE] = "signature",
> + [FW_CFG_ID] = "id",
> + [FW_CFG_UUID] = "uuid",
> + [FW_CFG_RAM_SIZE] = "ram_size",
> + [FW_CFG_NOGRAPHIC] = "nographic",
> + [FW_CFG_NB_CPUS] = "nb_cpus",
> + [FW_CFG_MACHINE_ID] = "machine_id",
> + [FW_CFG_KERNEL_ADDR] = "kernel_addr",
> + [FW_CFG_KERNEL_SIZE] = "kernel_size",
> + [FW_CFG_KERNEL_CMDLINE] = "kernel_cmdline",
> + [FW_CFG_INITRD_ADDR] = "initrd_addr",
> + [FW_CFG_INITRD_SIZE] = "initdr_size",
> + [FW_CFG_BOOT_DEVICE] = "boot_device",
> + [FW_CFG_NUMA] = "numa",
> + [FW_CFG_BOOT_MENU] = "boot_menu",
> + [FW_CFG_MAX_CPUS] = "max_cpus",
> + [FW_CFG_KERNEL_ENTRY] = "kernel_entry",
> + [FW_CFG_KERNEL_DATA] = "kernel_data",
> + [FW_CFG_INITRD_DATA] = "initrd_data",
> + [FW_CFG_CMDLINE_ADDR] = "cmdline_addr",
> + [FW_CFG_CMDLINE_SIZE] = "cmdline_size",
> + [FW_CFG_CMDLINE_DATA] = "cmdline_data",
> + [FW_CFG_SETUP_ADDR] = "setup_addr",
> + [FW_CFG_SETUP_SIZE] = "setup_size",
> + [FW_CFG_SETUP_DATA] = "setup_data",
> + [FW_CFG_FILE_DIR] = "file_dir",
> +};
> +
> +void hmp_info_fw_cfg(Monitor *mon, const QDict *qdict)
> +{
> + FWCfgState *s = fw_cfg_find();
> + int arch, key;
> +
> + monitor_printf(mon, "%12s %5s %7s %9s %6s %-24s\n",
> + "Type", "Perm", "Size", "Specific", "Order", "Info");
> + for (arch = 0; arch < ARRAY_SIZE(s->entries); ++arch) {
> + for (key = 0; key < fw_cfg_max_entry(s); ++key) {
> + FWCfgEntry *e = &s->entries[arch][key];
> + const char *perm = e->allow_write ? "RW" : "RO";
> + const char *arch_spec = arch ? "arch_spec" : "";
> + char *type, *info = NULL;
> +
> + if (!e->len) {
> + continue;
> + }
> + if (key >= FW_CFG_FILE_FIRST) {
> + int id = key - FW_CFG_FILE_FIRST;
> + const char *path = s->files->f[id].name;
> +
> + type = g_strdup_printf("file (id %d)", id);
> + monitor_printf(mon, "%12s %5s %7d %10s %5d %-24s\n",
> + type, perm, e->len, arch_spec,
> + get_fw_cfg_order(s, path), path);
> + g_free(type);
> + continue;
> + }
> + type = g_strdup(fw_cfg_wellknown_entries[key]);
> + if (!type) {
> + type = g_strdup_printf("entry_%d", key);
> + }
> +
> + switch (key) {
> + case FW_CFG_SIGNATURE:
> + info = g_strndup((const gchar *)e->data, e->len);
> + break;
> + case FW_CFG_UUID:
> + info = qemu_uuid_unparse_strdup((const QemuUUID *)e->data);
> + break;
It seems odd to encode the type of each entry in this switch; I guess
it's OK if no one else needs it, else perhaps there should be a type
array.
Other than that, it's fine from the HMP side, but I agree it should
either have a QMP equivalent (in which case the structure is a bit
different) or a good justification of if it's really debug only.
Dave
> + case FW_CFG_ID:
> + case FW_CFG_NOGRAPHIC:
> + case FW_CFG_NB_CPUS:
> + case FW_CFG_BOOT_MENU:
> + case FW_CFG_MAX_CPUS:
> + case FW_CFG_RAM_SIZE:
> + case FW_CFG_KERNEL_ADDR:
> + case FW_CFG_KERNEL_SIZE:
> + case FW_CFG_KERNEL_CMDLINE:
> + case FW_CFG_KERNEL_ENTRY:
> + case FW_CFG_KERNEL_DATA:
> + case FW_CFG_INITRD_ADDR:
> + case FW_CFG_INITRD_SIZE:
> + case FW_CFG_INITRD_DATA:
> + case FW_CFG_CMDLINE_ADDR:
> + case FW_CFG_CMDLINE_SIZE:
> + case FW_CFG_CMDLINE_DATA:
> + case FW_CFG_SETUP_ADDR:
> + case FW_CFG_SETUP_SIZE:
> + case FW_CFG_SETUP_DATA:
> + switch (e->len) {
> + case 2:
> + info = g_strdup_printf("0x%04x", lduw_le_p(e->data));
> + break;
> + case 4:
> + info = g_strdup_printf("0x%08x", ldl_le_p(e->data));
> + break;
> + case 8:
> + info = g_strdup_printf("0x%016" PRIx64, ldq_le_p(e->data));
> + break;
> + default:
> + break;
> + }
> + break;
> + default:
> + break;
> + }
> + monitor_printf(mon, "%12s %5s %7d %10s %5s %-24s\n",
> + type, perm, e->len, arch_spec, "", info ? info : "");
> + g_free(type);
> + g_free(info);
> + }
> + }
> +}
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index f5a6895a74..28630b2f26 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -226,4 +226,6 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
> FWCfgState *fw_cfg_find(void);
> bool fw_cfg_dma_enabled(void *opaque);
>
> +void hmp_info_fw_cfg(Monitor *mon, const QDict *qdict);
> +
> #endif
> --
> 2.17.2
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
I can only muster some random thoughts for this one, presently:
On 12/07/18 18:03, Philippe Mathieu-Daudé wrote:
> $ qemu-system-x86_64 -S -monitor stdio
> (qemu) info fw_cfg
> Type Perm Size Specific Order Info
> signature RO 4 QEMU
> id RO 4 0x00000003
> uuid RO 16 00000000-0000-0000-0000-000000000000
> ram_size RO 8 0x0000000008000000
> nographic RO 2 0x0000
> nb_cpus RO 2 0x0001
> numa RO 16
> boot_menu RO 2 0x0000
> max_cpus RO 2 0x0001
> file_dir RO 2052
> file (id 1) RO 36 160 etc/acpi/rsdp
> file (id 2) RO 131072 130 etc/acpi/tables
> file (id 3) RO 4 15 etc/boot-fail-wait
> file (id 4) RO 20 40 etc/e820
> file (id 5) RO 31 30 etc/smbios/smbios-anchor
> file (id 6) RO 320 20 etc/smbios/smbios-tables
> file (id 7) RO 6 90 etc/system-states
> file (id 8) RO 4096 140 etc/table-loader
> file (id 10) RO 9216 55 genroms/kvmvapic.bin
> uuid RO 4 (arch spec) 01000000-0000-0000-0000-000000000000
> ram_size RO 324 (arch spec)
> nographic RO 121 (arch spec)
> (qemu)
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hmp-commands-info.hx | 14 +++++
> hw/nvram/fw_cfg.c | 115 ++++++++++++++++++++++++++++++++++++++
> include/hw/nvram/fw_cfg.h | 2 +
> 3 files changed, 131 insertions(+)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index cbee8b944d..9e86dceeb4 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -916,6 +916,20 @@ STEXI
> @item info sev
> @findex info sev
> Show SEV information.
> +ETEXI
> +
> + {
> + .name = "fw_cfg",
> + .args_type = "",
> + .params = "",
> + .help = "Display the table of the fw_cfg entries registered",
> + .cmd = hmp_info_fw_cfg,
> + },
> +
> +STEXI
> +@item info fw_cfg
> +@findex info fw_cfg
> +Show roms.
> ETEXI
>
> STEXI
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 582653f07e..50525ec1dd 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -36,6 +36,7 @@
> #include "qemu/config-file.h"
> #include "qemu/cutils.h"
> #include "qapi/error.h"
> +#include "monitor/monitor.h"
>
> #define FW_CFG_FILE_SLOTS_DFLT 0x20
>
> @@ -1164,3 +1165,117 @@ static void fw_cfg_register_types(void)
> }
>
> type_init(fw_cfg_register_types)
> +
> +static const char *fw_cfg_wellknown_entries[0x20] = {
(1) Using 0x20 is not wrong here (it is a well-known magic constant),
but we should refer to it by name: please refer to FW_CFG_FILE_FIRST.
(See "include/standard-headers/linux/qemu_fw_cfg.h")
> + [FW_CFG_SIGNATURE] = "signature",
> + [FW_CFG_ID] = "id",
> + [FW_CFG_UUID] = "uuid",
> + [FW_CFG_RAM_SIZE] = "ram_size",
> + [FW_CFG_NOGRAPHIC] = "nographic",
> + [FW_CFG_NB_CPUS] = "nb_cpus",
> + [FW_CFG_MACHINE_ID] = "machine_id",
> + [FW_CFG_KERNEL_ADDR] = "kernel_addr",
> + [FW_CFG_KERNEL_SIZE] = "kernel_size",
> + [FW_CFG_KERNEL_CMDLINE] = "kernel_cmdline",
> + [FW_CFG_INITRD_ADDR] = "initrd_addr",
> + [FW_CFG_INITRD_SIZE] = "initdr_size",
> + [FW_CFG_BOOT_DEVICE] = "boot_device",
> + [FW_CFG_NUMA] = "numa",
> + [FW_CFG_BOOT_MENU] = "boot_menu",
> + [FW_CFG_MAX_CPUS] = "max_cpus",
> + [FW_CFG_KERNEL_ENTRY] = "kernel_entry",
> + [FW_CFG_KERNEL_DATA] = "kernel_data",
> + [FW_CFG_INITRD_DATA] = "initrd_data",
> + [FW_CFG_CMDLINE_ADDR] = "cmdline_addr",
> + [FW_CFG_CMDLINE_SIZE] = "cmdline_size",
> + [FW_CFG_CMDLINE_DATA] = "cmdline_data",
> + [FW_CFG_SETUP_ADDR] = "setup_addr",
> + [FW_CFG_SETUP_SIZE] = "setup_size",
> + [FW_CFG_SETUP_DATA] = "setup_data",
> + [FW_CFG_FILE_DIR] = "file_dir",
> +};
(2) I suggest introducing a macro that generates the initializer, using
the stringify() macro from "include/qemu/compiler.h". It should go
something like
#define FW_CFG_KEY_STRINGIFY(key) [key] = stringify(key),
Feel free to disagree though; this set of predefined, fixed numeric keys
will never change (nor will it ever be extened), so we might as well
live with the currently proposed verbosity.
> +
> +void hmp_info_fw_cfg(Monitor *mon, const QDict *qdict)
> +{
> + FWCfgState *s = fw_cfg_find();
> + int arch, key;
> +
> + monitor_printf(mon, "%12s %5s %7s %9s %6s %-24s\n",
> + "Type", "Perm", "Size", "Specific", "Order", "Info");
> + for (arch = 0; arch < ARRAY_SIZE(s->entries); ++arch) {
> + for (key = 0; key < fw_cfg_max_entry(s); ++key) {
> + FWCfgEntry *e = &s->entries[arch][key];
> + const char *perm = e->allow_write ? "RW" : "RO";
> + const char *arch_spec = arch ? "arch_spec" : "";
> + char *type, *info = NULL;
> +
> + if (!e->len) {
> + continue;
> + }
> + if (key >= FW_CFG_FILE_FIRST) {
> + int id = key - FW_CFG_FILE_FIRST;
> + const char *path = s->files->f[id].name;
> +
> + type = g_strdup_printf("file (id %d)", id);
> + monitor_printf(mon, "%12s %5s %7d %10s %5d %-24s\n",
> + type, perm, e->len, arch_spec,
> + get_fw_cfg_order(s, path), path);
> + g_free(type);
> + continue;
> + }
> + type = g_strdup(fw_cfg_wellknown_entries[key]);
> + if (!type) {
> + type = g_strdup_printf("entry_%d", key);
> + }
> +
> + switch (key) {
> + case FW_CFG_SIGNATURE:
> + info = g_strndup((const gchar *)e->data, e->len);
> + break;
> + case FW_CFG_UUID:
> + info = qemu_uuid_unparse_strdup((const QemuUUID *)e->data);
> + break;
> + case FW_CFG_ID:
> + case FW_CFG_NOGRAPHIC:
> + case FW_CFG_NB_CPUS:
> + case FW_CFG_BOOT_MENU:
> + case FW_CFG_MAX_CPUS:
> + case FW_CFG_RAM_SIZE:
> + case FW_CFG_KERNEL_ADDR:
> + case FW_CFG_KERNEL_SIZE:
> + case FW_CFG_KERNEL_CMDLINE:
> + case FW_CFG_KERNEL_ENTRY:
> + case FW_CFG_KERNEL_DATA:
> + case FW_CFG_INITRD_ADDR:
> + case FW_CFG_INITRD_SIZE:
> + case FW_CFG_INITRD_DATA:
> + case FW_CFG_CMDLINE_ADDR:
> + case FW_CFG_CMDLINE_SIZE:
> + case FW_CFG_CMDLINE_DATA:
> + case FW_CFG_SETUP_ADDR:
> + case FW_CFG_SETUP_SIZE:
> + case FW_CFG_SETUP_DATA:
> + switch (e->len) {
> + case 2:
> + info = g_strdup_printf("0x%04x", lduw_le_p(e->data));
> + break;
> + case 4:
> + info = g_strdup_printf("0x%08x", ldl_le_p(e->data));
> + break;
> + case 8:
> + info = g_strdup_printf("0x%016" PRIx64, ldq_le_p(e->data));
> + break;
> + default:
> + break;
> + }
> + break;
> + default:
> + break;
> + }
> + monitor_printf(mon, "%12s %5s %7d %10s %5s %-24s\n",
> + type, perm, e->len, arch_spec, "", info ? info : "");
> + g_free(type);
> + g_free(info);
> + }
> + }
> +}
(3) In general I wouldn't try to be smart here, regarding the contents.
I suggest simply dumping arrays of uint8_t values, in hex.
(4) Also I wouldn't reuse the same column for different purposes; I
think it's easier to read if a column always means the same thing, and
if it doesn't apply, it's just left blank. Something like this:
Selector Well-Known Numeric Pathname Perm Size Arch-Spec Order Data
- "Selector" is the uint16 selector key
- "Well-Known Numeric" is the stringified name if the selector refers to
a well-known numerically defined item.
- "Pathname" is the pathname for named files. Mutually exclusive with
"Well-Known Numeric", but that's OK, IMO.
- "Arch-Spec" should be left blank, or marked as "set" with an asterisk.
- "Data" should be a hexdump of uint8_t values (normally limited to 8
bytes, if the array is larger).
(5) I think this interface would benefit from being exposed over QMP /
JSON, and then the tabular presentation would be a separate question in
HMP.
(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.
I don't intend this to spiral into feature creep; take whatever you
like.
Thanks!
Laszlo
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index f5a6895a74..28630b2f26 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -226,4 +226,6 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
> FWCfgState *fw_cfg_find(void);
> bool fw_cfg_dma_enabled(void *opaque);
>
> +void hmp_info_fw_cfg(Monitor *mon, const QDict *qdict);
> +
> #endif
>
© 2016 - 2026 Red Hat, Inc.