[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/6] hw/nvram/fw_cfg: Add HMP 'info fw_cfg' comm
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 5/6] hw/nvram/fw_cfg: Add HMP 'info fw_cfg' command |
Date: |
Mon, 10 Dec 2018 11:42:54 +0000 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
* Philippe Mathieu-Daudé (address@hidden) 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é <address@hidden>
> ---
> 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
> address@hidden info fw_cfg
> address@hidden 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 / address@hidden / Manchester, UK