From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50207) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYOAf-0003bT-IQ for qemu-devel@nongnu.org; Wed, 18 Mar 2015 20:18:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YYOAX-0006av-CS for qemu-devel@nongnu.org; Wed, 18 Mar 2015 20:18:45 -0400 Received: from relay-06.andrew.cmu.edu ([128.2.157.21]:48114 helo=relay.andrew.cmu.edu) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYOAX-0006Zx-7e for qemu-devel@nongnu.org; Wed, 18 Mar 2015 20:18:37 -0400 From: "Gabriel L. Somlo" Date: Wed, 18 Mar 2015 20:18:31 -0400 Message-Id: <1426724311-9343-6-git-send-email-somlo@cmu.edu> In-Reply-To: <1426724311-9343-1-git-send-email-somlo@cmu.edu> References: <1426724311-9343-1-git-send-email-somlo@cmu.edu> Subject: [Qemu-devel] [PATCH v2 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: matt.fleming@intel.com, rjones@redhat.com, jordan.l.justen@intel.com, gleb@cloudius-systems.com, mdroth@linux.vnet.ibm.com, gsomlo@gmail.com, kraxel@redhat.com, pbonzini@redhat.com, lersek@redhat.com, armbru@redhat.com Allow user supplied files to be inserted into the fw_cfg device before starting the guest. Since fw_cfg_add_file() already disallows duplicate fw_cfg file names, qemu will exit with an error message if the user supplies multiple blobs with the same fw_cfg file name, or if a blob name collides with a fw_cfg name programmatically added from within the QEMU source code. Signed-off-by: Gabriel Somlo --- I belive at this point I'm addressing all of Laszlo's feedback. fw_cfg_option_add() operates from within the "Location" set by qemu_opts_parse(qemu_find_opts("fw_cfg"), optarg, 1), and, as such, does not require a "location switch" to print out the proper context around the error message. I'm allocating blobs (and reading host-side file contents) all from within fw_cfg_option_add(), so those error messages are also printed with the right context. Simply traversing the command-line blob list and inserting it into fw_cfg now happens directly from fw_cfg_init1(), immediately after the device is initialized and ready to accept fw_cfg_add_* calls. I'm not sure I'm addressing Gerd's concerns re. init order, but since each blob key and each file name can only be inserted at most once, we will exit with an error whether the user-specified blob from the command line or the programmatically-inserted one from the arch specific init routine gets inserted first. And, since that's the case, I took full advantage of not having to determine with certainty for each architecture where the "last place any fw_cfg blobs are inserted" is, so I could add the command line blobs *last*. Since *first* will work just as well, or exit with the same error message, I don't have to worry about handling each arch separately. The one other thing I may not have comprehended was the part about > Which reminds me: Going for QemuOpts would be very useful (gives > -readconfig support), and it would solve the init order issue too. > Instead of having your custom storage you just let QemuOpts parse and > store the command line switch, then process them after fw_cfg device > initialization. If the code below is still in serious need of cleaning up, please help me figure out what I'm missing, and maybe point me toward an example. Thanks much, Gabriel hw/nvram/fw_cfg.c | 40 ++++++++++++++++++++++++++++++++++++++++ include/hw/nvram/fw_cfg.h | 4 ++++ qemu-options.hx | 11 +++++++++++ vl.c | 27 +++++++++++++++++++++++++++ 4 files changed, 82 insertions(+) diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index a5fd512..7036fde 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -29,6 +29,7 @@ #include "trace.h" #include "qemu/error-report.h" #include "qemu/config-file.h" +#include "hw/loader.h" #define FW_CFG_SIZE 2 #define FW_CFG_NAME "fw_cfg" @@ -573,9 +574,42 @@ static void fw_cfg_machine_ready(struct Notifier *n, void *data) } +static struct FWCfgOption { + const char *name; + gchar *data; + size_t size; +} *fw_cfg_options; +static size_t fw_cfg_num_options; + +void fw_cfg_option_add(QemuOpts *opts) +{ + const char *name = qemu_opt_get(opts, "name"); + const char *file = qemu_opt_get(opts, "file"); + + if (name == NULL || *name == '\0' || file == NULL || *file == '\0') { + error_report("invalid argument value"); + exit(1); + } + if (strlen(name) > FW_CFG_MAX_FILE_PATH - 1) { + error_report("name too long (max. %d char)", FW_CFG_MAX_FILE_PATH - 1); + exit(1); + } + + fw_cfg_options = g_realloc(fw_cfg_options, sizeof(struct FWCfgOption) * + (fw_cfg_num_options + 1)); + fw_cfg_options[fw_cfg_num_options].name = name; + if (!g_file_get_contents(file, &fw_cfg_options[fw_cfg_num_options].data, + &fw_cfg_options[fw_cfg_num_options].size, NULL)) { + error_report("can't load %s", file); + exit(1); + } + fw_cfg_num_options++; +} + static void fw_cfg_init1(DeviceState *dev) { + size_t i; FWCfgState *s = FW_CFG(dev); assert(!object_resolve_path(FW_CFG_PATH, NULL)); @@ -584,6 +618,12 @@ static void fw_cfg_init1(DeviceState *dev) qdev_init_nofail(dev); + /* insert file(s) from command line */ + for (i = 0; i < fw_cfg_num_options; i++) { + fw_cfg_add_file(s, fw_cfg_options[i].name, + fw_cfg_options[i].data, fw_cfg_options[i].size); + } + fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4); fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16); fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC)); diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index b2e10c2..e6b0eeb 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -7,6 +7,7 @@ #include "exec/hwaddr.h" #include "qemu/typedefs.h" +#include "qemu/option.h" #endif #define FW_CFG_SIGNATURE 0x00 @@ -76,6 +77,9 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, void *data, size_t len); void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data, size_t len); + +void fw_cfg_option_add(QemuOpts *opts); + FWCfgState *fw_cfg_init_io(uint32_t iobase); FWCfgState *fw_cfg_init_mem(hwaddr ctl_addr, hwaddr data_addr); FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, hwaddr data_addr, diff --git a/qemu-options.hx b/qemu-options.hx index 16ff72c..3879c89 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2669,6 +2669,17 @@ STEXI @table @option ETEXI +DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg, + "-fw_cfg name=,file=\n" + " add named fw_cfg entry from file\n", + QEMU_ARCH_ALL) +STEXI +@item -fw_cfg name=@var{name},file=@var{file} +@findex -fw_cfg +Add named fw_cfg entry from file. @var{name} determines the name of +the entry in the fw_cfg file directory exposed to the guest. +ETEXI + DEF("serial", HAS_ARG, QEMU_OPTION_serial, \ "-serial dev redirect the serial port to char device 'dev'\n", QEMU_ARCH_ALL) diff --git a/vl.c b/vl.c index 694deb4..88a0b6e 100644 --- a/vl.c +++ b/vl.c @@ -490,6 +490,25 @@ static QemuOptsList qemu_semihosting_config_opts = { }, }; +static QemuOptsList qemu_fw_cfg_opts = { + .name = "fw_cfg", + .implied_opt_name = "name", + .head = QTAILQ_HEAD_INITIALIZER(qemu_fw_cfg_opts.head), + .desc = { + { + .name = "name", + .type = QEMU_OPT_STRING, + .help = "Sets the fw_cfg name of the blob to be inserted", + }, { + .name = "file", + .type = QEMU_OPT_STRING, + .help = "Sets the name of the file from which\n" + "the fw_cfg blob will be loaded", + }, + { /* end of list */ } + }, +}; + /** * Get machine options * @@ -2804,6 +2823,7 @@ int main(int argc, char **argv, char **envp) qemu_add_opts(&qemu_numa_opts); qemu_add_opts(&qemu_icount_opts); qemu_add_opts(&qemu_semihosting_config_opts); + qemu_add_opts(&qemu_fw_cfg_opts); runstate_init(); @@ -3420,6 +3440,13 @@ int main(int argc, char **argv, char **envp) } do_smbios_option(opts); break; + case QEMU_OPTION_fwcfg: + opts = qemu_opts_parse(qemu_find_opts("fw_cfg"), optarg, 1); + if (opts == NULL) { + exit(1); + } + fw_cfg_option_add(opts); + break; case QEMU_OPTION_enable_kvm: olist = qemu_find_opts("machine"); qemu_opts_parse(olist, "accel=kvm", 0); -- 2.1.0