[Qemu-devel] [PATCH v3 0/5] fw-cfg: docs, cleanup, and user-provided cmdline blobs

[Qemu-devel] [PATCH v3 0/5] fw-cfg: docs, cleanup, and user-provided cmdline blobs

[Gabriel L. Somlo]

Document and clean up fw_cfg; additionally, allow user-provided blobs to
be inserted into fw_cfg via the qemu command line.

Changes since v2:

    - entire series depends on (applies on top of) another fw_cfg patch:
      (http://lists.nongnu.org/archive/html/qemu-devel/2015-03/msg04278.html)
      which is currently queued for 2.3.0-rc.

    - documentation (1/5) is now correct about all architectures using
      fw_cfg revision 0x1 :)

    - patches 2/5, 3/5, and 4/5 are unchanged

    - patch 5/5 (command-line insertion of fw_cfg blob) is now using
      qemu_opts_foreach() after machine_init, and is therefore
      compliant with -writeconfig and -readconfig (thanks again Laszlo,
      Gerd, and Markus for the quick tutorial!).
      Also, ee additional comments below the commit log in the actual
      patch message.

Thanks,
  Gabriel

Gabriel L. Somlo (5):
  fw_cfg: add documentation file (docs/specs/fw_cfg.txt)
  fw_cfg: remove support for guest-side data writes
  fw_cfg: prevent selector key conflict
  fw_cfg: prohibit insertion of duplicate fw_cfg file names
  fw_cfg: insert fw_cfg file blobs via qemu cmdline

 docs/specs/fw_cfg.txt     | 226 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/nvram/fw_cfg.c         |  45 ++-------
 include/hw/nvram/fw_cfg.h |   2 -
 qemu-options.hx           |  11 +++
 trace-events              |   2 -
 vl.c                      |  63 +++++++++++++
 6 files changed, 308 insertions(+), 41 deletions(-)
 create mode 100644 docs/specs/fw_cfg.txt

-- 
2.1.0

[Qemu-devel] [PATCH v3 1/5] fw_cfg: add documentation file (docs/specs/fw_cfg.txt)

[Gabriel L. Somlo]

This document covers the guest-side hardware interface, as
well as the host-side programming API of QEMU's firmware
configuration (fw_cfg) device.

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 docs/specs/fw_cfg.txt | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 205 insertions(+)
 create mode 100644 docs/specs/fw_cfg.txt

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
new file mode 100644
index 0000000..6accd92
--- /dev/null
+++ b/docs/specs/fw_cfg.txt
@@ -0,0 +1,205 @@
+QEMU Firmware Configuration (fw_cfg) Device
+===========================================
+
+= Guest-side Hardware Interface =
+
+This hardware interface allows the guest to retrieve various data items
+(blobs) that can influence how the firmware configures itself, or may
+contain tables to be installed for the guest OS. Examples include device
+boot order, ACPI and SMBIOS tables, virtual machine UUID, SMP and NUMA
+information, kernel/initrd images for direct (Linux) kernel booting, etc.
+
+== Selector (Control) Register ==
+
+* Write only
+* Location: platform dependent (IOport or MMIO)
+* Width: 16-bit
+* Endianness: little-endian (if IOport), or big-endian (if MMIO)
+
+A write to this register sets the index of a firmware configuration
+item which can subsequently be accessed via the data register.
+
+Setting the selector register will cause the data offset to be set
+to zero. The data offset impacts which data is accessed via the data
+register, and is explained below.
+
+Bit14 of the selector register indicates whether the configuration
+setting is being written. A value of 0 means the item is only being
+read, and all write access to the data port will be ignored. A value
+of 1 means the item's data can be overwritten by writes to the data
+register. In other words, configuration write mode is enabled when
+the selector value is between 0x4000-0x7fff or 0xc000-0xffff.
+
+NOTE: As of QEMU v2.4, writes to the fw_cfg data register are no
+      longer supported, and will be ignored (treated as no-ops)!
+
+Bit15 of the selector register indicates whether the configuration
+setting is architecture specific. A value of 0 means the item is a
+generic configuration item. A value of 1 means the item is specific
+to a particular architecture. In other words, generic configuration
+items are accessed with a selector value between 0x0000-0x7fff, and
+architecture specific configuration items are accessed with a selector
+value between 0x8000-0xffff.
+
+== Data Register ==
+
+* Read/Write (writes ignored as of QEMU v2.4)
+* Location: platform dependent (IOport [*] or MMIO)
+* Width: 8-bit (if IOport), 8/16/32/64-bit (if MMIO)
+* Endianness: string-preserving
+
+[*] On platforms where the data register is exposed as an IOport, its
+port number will always be one greater than the port number of the
+selector register. In other words, the two ports overlap, and can not
+be mapped separately.
+
+The data register allows access to an array of bytes for each firmware
+configuration data item. The specific item is selected by writing to
+the selector register, as described above.
+
+Initially following a write to the selector register, the data offset
+will be set to zero. Each successful access to the data register will
+increment the data offset by the appropriate access width.
+
+Each firmware configuration item has a maximum length of data
+associated with the item. After the data offset has passed the
+end of this maximum data length, then any reads will return a data
+value of 0x00, and all writes will be ignored.
+
+An N-byte wide read of the data register will return the next available
+N bytes of the selected firmware configuration item, as a substring, in
+increasing address order, similar to memcpy().
+
+== Register Locations ==
+
+=== x86, x86_64 Register Locations ===
+
+Selector Register IOport: 0x510
+Data Register IOport:     0x511
+
+== Firmware Configuration Items ==
+
+=== Signature (Key 0x0000, FW_CFG_SIGNATURE) ===
+
+The presence of the fw_cfg selector and data registers can be verified
+by selecting the "signature" item using key 0x0000 (FW_CFG_SIGNATURE),
+and reading four bytes from the data register. If the fw_cfg device is
+present, the four bytes read will contain the characters "QEMU".
+
+=== Revision (Key 0x0001, FW_CFG_ID) ===
+
+A 32-bit little-endian unsigned int, this item is used as an interface
+revision number, and is currently set to 1 by QEMU when fw_cfg is
+initialized.
+
+=== File Directory (Key 0x0019, FW_CFG_FILE_DIR) ===
+
+Firmware configuration items stored at selector keys 0x0020 or higher
+(FW_CFG_FILE_FIRST or higher) have an associated entry in a directory
+structure, which makes it easier for guest-side firmware to identify
+and retrieve them. The format of this file directory (from fw_cfg.h in
+the QEMU source tree) is shown here, slightly annotated for clarity:
+
+struct FWCfgFiles {		/* the entire file directory fw_cfg item */
+    uint32_t count;		/* number of entries, in big-endian format */
+    struct FWCfgFile f[];	/* array of file entries, see below */
+};
+
+struct FWCfgFile {		/* an individual file entry, 64 bytes total */
+    uint32_t size;		/* size of referenced fw_cfg item, big-endian */
+    uint16_t select;		/* selector key of fw_cfg item, big-endian */
+    uint16_t reserved;
+    char name[56];		/* fw_cfg item name, NUL-terminated ascii */
+};
+
+=== All Other Data Items ===
+
+Please consult the QEMU source for the most up-to-date and authoritative
+list of selector keys and their respective items' purpose and format.
+
+=== Ranges ===
+
+Theoretically, there may be up to 0x4000 generic firmware configuration
+items, and up to 0x4000 architecturally specific ones.
+
+Selector Reg.    Range Usage
+---------------  -----------
+0x0000 - 0x3fff  Generic (0x0000 - 0x3fff, RO)
+0x4000 - 0x7fff  Generic (0x0000 - 0x3fff, RW, ignored in QEMU v2.4+)
+0x8000 - 0xbfff  Arch. Specific (0x0000 - 0x3fff, RO)
+0xc000 - 0xffff  Arch. Specific (0x0000 - 0x3fff, RW, ignored in v2.4+)
+
+In practice, the number of allowed firmware configuration items is given
+by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h).
+
+= Host-side API =
+
+The following functions are available to the QEMU programmer for adding
+data to a fw_cfg device during guest initialization (see fw_cfg.h for
+each function's complete prototype):
+
+== fw_cfg_add_bytes() ==
+
+Given a selector key value, starting pointer, and size, create an item
+as a raw "blob" of the given size, available by selecting the given key.
+The data referenced by the starting pointer is only linked, NOT copied,
+into the data structure of the fw_cfg device.
+
+== fw_cfg_add_string() ==
+
+Instead of a starting pointer and size, this function accepts a pointer
+to a NUL-terminated ascii string, and inserts a newly allocated copy of
+the string (including the NUL terminator) into the fw_cfg device data
+structure.
+
+== fw_cfg_add_iXX() ==
+
+Insert an XX-bit item, where XX may be 16, 32, or 64. These functions
+will convert a 16-, 32-, or 64-bit integer to little-endian, then add
+a dynamically allocated copy of the appropriately sized item to fw_cfg
+under the given selector key value.
+
+== fw_cfg_add_file() ==
+
+Given a filename (i.e., fw_cfg item name), starting pointer, and size,
+create an item as a raw "blob" of the given size. Unlike fw_cfg_add_bytes()
+above, the next available selector key (above 0x0020, FW_CFG_FILE_FIRST)
+will be used, and a new entry will be added to the file directory structure
+(at key 0x0019), containing the item name, blob size, and automatically
+assigned selector key value. The data referenced by the starting pointer
+is only linked, NOT copied, into the fw_cfg data structure.
+
+== fw_cfg_add_file_callback() ==
+
+Like fw_cfg_add_file(), but additionally sets pointers to a callback
+function (and opaque argument), which will be executed host-side by
+QEMU each time a byte is read by the guest from this particular item.
+
+NOTE: The callback function is given the opaque argument set by
+fw_cfg_add_file_callback(), but also the current data offset,
+allowing it the option of only acting upon specific offset values
+(e.g., 0, before the first data byte of the selected item is
+returned to the guest).
+
+== fw_cfg_modify_file() ==
+
+Given a filename (i.e., fw_cfg item name), starting pointer, and size,
+completely replace the configuration item referenced by the given item
+name with the new given blob. If an existing blob is found, its
+callback information is removed, and a pointer to the old data is
+returned to allow the caller to free it, helping avoid memory leaks.
+If a configuration item does not already exist under the given item
+name, a new item will be created as with fw_cfg_add_file(), and NULL
+is returned to the caller. In any case, the data referenced by the
+starting pointer is only linked, NOT copied, into the fw_cfg data
+structure.
+
+== fw_cfg_add_callback() ==
+
+Like fw_cfg_add_bytes(), but additionally sets pointers to a callback
+function (and opaque argument), which will be executed host-side by
+QEMU each time a guest-side write operation to this particular item
+completes fully overwriting the item's data.
+
+NOTE: This function is deprecated, and will be completely removed
+starting with QEMU v2.4.
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3 1/5] fw_cfg: add documentation file (docs/specs/fw_cfg.txt)

[Laszlo Ersek]

On 03/21/15 21:23, Gabriel L. Somlo wrote:
> This document covers the guest-side hardware interface, as
> well as the host-side programming API of QEMU's firmware
> configuration (fw_cfg) device.
> 
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> ---
>  docs/specs/fw_cfg.txt | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 205 insertions(+)
>  create mode 100644 docs/specs/fw_cfg.txt

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> new file mode 100644
> index 0000000..6accd92
> --- /dev/null
> +++ b/docs/specs/fw_cfg.txt
> @@ -0,0 +1,205 @@
> +QEMU Firmware Configuration (fw_cfg) Device
> +===========================================
> +
> += Guest-side Hardware Interface =
> +
> +This hardware interface allows the guest to retrieve various data items
> +(blobs) that can influence how the firmware configures itself, or may
> +contain tables to be installed for the guest OS. Examples include device
> +boot order, ACPI and SMBIOS tables, virtual machine UUID, SMP and NUMA
> +information, kernel/initrd images for direct (Linux) kernel booting, etc.
> +
> +== Selector (Control) Register ==
> +
> +* Write only
> +* Location: platform dependent (IOport or MMIO)
> +* Width: 16-bit
> +* Endianness: little-endian (if IOport), or big-endian (if MMIO)
> +
> +A write to this register sets the index of a firmware configuration
> +item which can subsequently be accessed via the data register.
> +
> +Setting the selector register will cause the data offset to be set
> +to zero. The data offset impacts which data is accessed via the data
> +register, and is explained below.
> +
> +Bit14 of the selector register indicates whether the configuration
> +setting is being written. A value of 0 means the item is only being
> +read, and all write access to the data port will be ignored. A value
> +of 1 means the item's data can be overwritten by writes to the data
> +register. In other words, configuration write mode is enabled when
> +the selector value is between 0x4000-0x7fff or 0xc000-0xffff.
> +
> +NOTE: As of QEMU v2.4, writes to the fw_cfg data register are no
> +      longer supported, and will be ignored (treated as no-ops)!
> +
> +Bit15 of the selector register indicates whether the configuration
> +setting is architecture specific. A value of 0 means the item is a
> +generic configuration item. A value of 1 means the item is specific
> +to a particular architecture. In other words, generic configuration
> +items are accessed with a selector value between 0x0000-0x7fff, and
> +architecture specific configuration items are accessed with a selector
> +value between 0x8000-0xffff.
> +
> +== Data Register ==
> +
> +* Read/Write (writes ignored as of QEMU v2.4)
> +* Location: platform dependent (IOport [*] or MMIO)
> +* Width: 8-bit (if IOport), 8/16/32/64-bit (if MMIO)
> +* Endianness: string-preserving
> +
> +[*] On platforms where the data register is exposed as an IOport, its
> +port number will always be one greater than the port number of the
> +selector register. In other words, the two ports overlap, and can not
> +be mapped separately.
> +
> +The data register allows access to an array of bytes for each firmware
> +configuration data item. The specific item is selected by writing to
> +the selector register, as described above.
> +
> +Initially following a write to the selector register, the data offset
> +will be set to zero. Each successful access to the data register will
> +increment the data offset by the appropriate access width.
> +
> +Each firmware configuration item has a maximum length of data
> +associated with the item. After the data offset has passed the
> +end of this maximum data length, then any reads will return a data
> +value of 0x00, and all writes will be ignored.
> +
> +An N-byte wide read of the data register will return the next available
> +N bytes of the selected firmware configuration item, as a substring, in
> +increasing address order, similar to memcpy().
> +
> +== Register Locations ==
> +
> +=== x86, x86_64 Register Locations ===
> +
> +Selector Register IOport: 0x510
> +Data Register IOport:     0x511
> +
> +== Firmware Configuration Items ==
> +
> +=== Signature (Key 0x0000, FW_CFG_SIGNATURE) ===
> +
> +The presence of the fw_cfg selector and data registers can be verified
> +by selecting the "signature" item using key 0x0000 (FW_CFG_SIGNATURE),
> +and reading four bytes from the data register. If the fw_cfg device is
> +present, the four bytes read will contain the characters "QEMU".
> +
> +=== Revision (Key 0x0001, FW_CFG_ID) ===
> +
> +A 32-bit little-endian unsigned int, this item is used as an interface
> +revision number, and is currently set to 1 by QEMU when fw_cfg is
> +initialized.
> +
> +=== File Directory (Key 0x0019, FW_CFG_FILE_DIR) ===
> +
> +Firmware configuration items stored at selector keys 0x0020 or higher
> +(FW_CFG_FILE_FIRST or higher) have an associated entry in a directory
> +structure, which makes it easier for guest-side firmware to identify
> +and retrieve them. The format of this file directory (from fw_cfg.h in
> +the QEMU source tree) is shown here, slightly annotated for clarity:
> +
> +struct FWCfgFiles {		/* the entire file directory fw_cfg item */
> +    uint32_t count;		/* number of entries, in big-endian format */
> +    struct FWCfgFile f[];	/* array of file entries, see below */
> +};
> +
> +struct FWCfgFile {		/* an individual file entry, 64 bytes total */
> +    uint32_t size;		/* size of referenced fw_cfg item, big-endian */
> +    uint16_t select;		/* selector key of fw_cfg item, big-endian */
> +    uint16_t reserved;
> +    char name[56];		/* fw_cfg item name, NUL-terminated ascii */
> +};
> +
> +=== All Other Data Items ===
> +
> +Please consult the QEMU source for the most up-to-date and authoritative
> +list of selector keys and their respective items' purpose and format.
> +
> +=== Ranges ===
> +
> +Theoretically, there may be up to 0x4000 generic firmware configuration
> +items, and up to 0x4000 architecturally specific ones.
> +
> +Selector Reg.    Range Usage
> +---------------  -----------
> +0x0000 - 0x3fff  Generic (0x0000 - 0x3fff, RO)
> +0x4000 - 0x7fff  Generic (0x0000 - 0x3fff, RW, ignored in QEMU v2.4+)
> +0x8000 - 0xbfff  Arch. Specific (0x0000 - 0x3fff, RO)
> +0xc000 - 0xffff  Arch. Specific (0x0000 - 0x3fff, RW, ignored in v2.4+)
> +
> +In practice, the number of allowed firmware configuration items is given
> +by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h).
> +
> += Host-side API =
> +
> +The following functions are available to the QEMU programmer for adding
> +data to a fw_cfg device during guest initialization (see fw_cfg.h for
> +each function's complete prototype):
> +
> +== fw_cfg_add_bytes() ==
> +
> +Given a selector key value, starting pointer, and size, create an item
> +as a raw "blob" of the given size, available by selecting the given key.
> +The data referenced by the starting pointer is only linked, NOT copied,
> +into the data structure of the fw_cfg device.
> +
> +== fw_cfg_add_string() ==
> +
> +Instead of a starting pointer and size, this function accepts a pointer
> +to a NUL-terminated ascii string, and inserts a newly allocated copy of
> +the string (including the NUL terminator) into the fw_cfg device data
> +structure.
> +
> +== fw_cfg_add_iXX() ==
> +
> +Insert an XX-bit item, where XX may be 16, 32, or 64. These functions
> +will convert a 16-, 32-, or 64-bit integer to little-endian, then add
> +a dynamically allocated copy of the appropriately sized item to fw_cfg
> +under the given selector key value.
> +
> +== fw_cfg_add_file() ==
> +
> +Given a filename (i.e., fw_cfg item name), starting pointer, and size,
> +create an item as a raw "blob" of the given size. Unlike fw_cfg_add_bytes()
> +above, the next available selector key (above 0x0020, FW_CFG_FILE_FIRST)
> +will be used, and a new entry will be added to the file directory structure
> +(at key 0x0019), containing the item name, blob size, and automatically
> +assigned selector key value. The data referenced by the starting pointer
> +is only linked, NOT copied, into the fw_cfg data structure.
> +
> +== fw_cfg_add_file_callback() ==
> +
> +Like fw_cfg_add_file(), but additionally sets pointers to a callback
> +function (and opaque argument), which will be executed host-side by
> +QEMU each time a byte is read by the guest from this particular item.
> +
> +NOTE: The callback function is given the opaque argument set by
> +fw_cfg_add_file_callback(), but also the current data offset,
> +allowing it the option of only acting upon specific offset values
> +(e.g., 0, before the first data byte of the selected item is
> +returned to the guest).
> +
> +== fw_cfg_modify_file() ==
> +
> +Given a filename (i.e., fw_cfg item name), starting pointer, and size,
> +completely replace the configuration item referenced by the given item
> +name with the new given blob. If an existing blob is found, its
> +callback information is removed, and a pointer to the old data is
> +returned to allow the caller to free it, helping avoid memory leaks.
> +If a configuration item does not already exist under the given item
> +name, a new item will be created as with fw_cfg_add_file(), and NULL
> +is returned to the caller. In any case, the data referenced by the
> +starting pointer is only linked, NOT copied, into the fw_cfg data
> +structure.
> +
> +== fw_cfg_add_callback() ==
> +
> +Like fw_cfg_add_bytes(), but additionally sets pointers to a callback
> +function (and opaque argument), which will be executed host-side by
> +QEMU each time a guest-side write operation to this particular item
> +completes fully overwriting the item's data.
> +
> +NOTE: This function is deprecated, and will be completely removed
> +starting with QEMU v2.4.
> 

[Qemu-devel] [PATCH v3 2/5] fw_cfg: remove support for guest-side data writes

[Gabriel L. Somlo]

>From this point forward, any guest-side writes to the fw_cfg
data register will be treated as no-ops. This patch also removes
the unused host-side API function fw_cfg_add_callback(), which
allowed the registration of a callback to be executed each time
the guest completed a full overwrite of a given fw_cfg data item.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/nvram/fw_cfg.c         | 33 +--------------------------------
 include/hw/nvram/fw_cfg.h |  2 --
 trace-events              |  1 -
 3 files changed, 1 insertion(+), 35 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 68eff77..ed70798 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -46,7 +46,6 @@ typedef struct FWCfgEntry {
     uint32_t len;
     uint8_t *data;
     void *callback_opaque;
-    FWCfgCallback callback;
     FWCfgReadCallback read_callback;
 } FWCfgEntry;
 
@@ -232,19 +231,7 @@ static void fw_cfg_reboot(FWCfgState *s)
 
 static void fw_cfg_write(FWCfgState *s, uint8_t value)
 {
-    int arch = !!(s->cur_entry & FW_CFG_ARCH_LOCAL);
-    FWCfgEntry *e = &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MASK];
-
-    trace_fw_cfg_write(s, value);
-
-    if (s->cur_entry & FW_CFG_WRITE_CHANNEL && e->callback &&
-        s->cur_offset < e->len) {
-        e->data[s->cur_offset++] = value;
-        if (s->cur_offset == e->len) {
-            e->callback(e->callback_opaque, e->data);
-            s->cur_offset = 0;
-        }
-    }
+    /* nothing, write support removed in QEMU v2.4+ */
 }
 
 static int fw_cfg_select(FWCfgState *s, uint16_t key)
@@ -458,7 +445,6 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key,
     s->entries[arch][key].data = data;
     s->entries[arch][key].len = len;
     s->entries[arch][key].callback_opaque = NULL;
-    s->entries[arch][key].callback = NULL;
 
     return ptr;
 }
@@ -502,23 +488,6 @@ void fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value)
     fw_cfg_add_bytes(s, key, copy, sizeof(value));
 }
 
-void fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
-                         void *callback_opaque, void *data, size_t len)
-{
-    int arch = !!(key & FW_CFG_ARCH_LOCAL);
-
-    assert(key & FW_CFG_WRITE_CHANNEL);
-
-    key &= FW_CFG_ENTRY_MASK;
-
-    assert(key < FW_CFG_MAX_ENTRY && len <= UINT32_MAX);
-
-    s->entries[arch][key].data = data;
-    s->entries[arch][key].len = (uint32_t)len;
-    s->entries[arch][key].callback_opaque = callback_opaque;
-    s->entries[arch][key].callback = callback;
-}
-
 void fw_cfg_add_file_callback(FWCfgState *s,  const char *filename,
                               FWCfgReadCallback callback, void *callback_opaque,
                               void *data, size_t len)
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 6d8a8ac..b2e10c2 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -69,8 +69,6 @@ void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value);
 void fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value);
 void fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value);
 void fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value);
-void fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
-                         void *callback_opaque, void *data, size_t len);
 void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
                      size_t len);
 void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
diff --git a/trace-events b/trace-events
index 30eba92..1275b70 100644
--- a/trace-events
+++ b/trace-events
@@ -193,7 +193,6 @@ ecc_diag_mem_writeb(uint64_t addr, uint32_t val) "Write diagnostic %"PRId64" = %
 ecc_diag_mem_readb(uint64_t addr, uint32_t ret) "Read diagnostic %"PRId64"= %02x"
 
 # hw/nvram/fw_cfg.c
-fw_cfg_write(void *s, uint8_t value) "%p %d"
 fw_cfg_select(void *s, uint16_t key, int ret) "%p key %d = %d"
 fw_cfg_read(void *s, uint8_t ret) "%p = %d"
 fw_cfg_add_file_dupe(void *s, char *name) "%p %s"
-- 
2.1.0

[Qemu-devel] [PATCH v3 3/5] fw_cfg: prevent selector key conflict

[Gabriel L. Somlo]

Enforce a single assignment of data for each distinct selector key.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/nvram/fw_cfg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index ed70798..227beaf 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -423,6 +423,7 @@ static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key,
     key &= FW_CFG_ENTRY_MASK;
 
     assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX);
+    assert(s->entries[arch][key].data == NULL); /* avoid key conflict */
 
     s->entries[arch][key].data = data;
     s->entries[arch][key].len = (uint32_t)len;
-- 
2.1.0

[Qemu-devel] [PATCH v3 4/5] fw_cfg: prohibit insertion of duplicate fw_cfg file names

[Gabriel L. Somlo]

Exit with an error (instead of simply logging a trace event)
whenever the same fw_cfg file name is added multiple times via
one of the fw_cfg_add_file[_callback]() host-side API calls.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/nvram/fw_cfg.c | 11 ++++++-----
 trace-events      |  1 -
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 227beaf..8d4ea25 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -505,18 +505,19 @@ void fw_cfg_add_file_callback(FWCfgState *s,  const char *filename,
     index = be32_to_cpu(s->files->count);
     assert(index < FW_CFG_FILE_SLOTS);
 
-    fw_cfg_add_bytes_read_callback(s, FW_CFG_FILE_FIRST + index,
-                                   callback, callback_opaque, data, len);
-
     pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name),
             filename);
     for (i = 0; i < index; i++) {
         if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
-            trace_fw_cfg_add_file_dupe(s, s->files->f[index].name);
-            return;
+            error_report("duplicate fw_cfg file name: %s",
+                         s->files->f[index].name);
+            exit(1);
         }
     }
 
+    fw_cfg_add_bytes_read_callback(s, FW_CFG_FILE_FIRST + index,
+                                   callback, callback_opaque, data, len);
+
     s->files->f[index].size   = cpu_to_be32(len);
     s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index);
     trace_fw_cfg_add_file(s, index, s->files->f[index].name, len);
diff --git a/trace-events b/trace-events
index 1275b70..a340c5a 100644
--- a/trace-events
+++ b/trace-events
@@ -195,7 +195,6 @@ ecc_diag_mem_readb(uint64_t addr, uint32_t ret) "Read diagnostic %"PRId64"= %02x
 # hw/nvram/fw_cfg.c
 fw_cfg_select(void *s, uint16_t key, int ret) "%p key %d = %d"
 fw_cfg_read(void *s, uint8_t ret) "%p = %d"
-fw_cfg_add_file_dupe(void *s, char *name) "%p %s"
 fw_cfg_add_file(void *s, int index, char *name, size_t len) "%p #%d: %s (%zd bytes)"
 
 # hw/block/hd-geometry.c
-- 
2.1.0

[Qemu-devel] [PATCH v3 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline

[Gabriel L. Somlo]

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. A warning message will be
printed if the fw_cfg item name does not begin with the
prefix "opt/", which is recommended for external, user
provided blobs.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---

I tested that object_resolve_path() always returns NULL when it can't find
a match for the given argument (i.e., rather than triggering an assert);
this means on architectures which do not create/initialize a fw_cfg object,
the call to fw_cfg_find() returns NULL.

I spent a while trying to decide whether to wrap the vl.c bits
in #ifdef CONFIG_SOFTMMU or not. Making fw_cfg_find() an always-available
static inline doesn't actually help, since I'm also calling fw_cfg_add_file()
from parse_fw_cfg(), so it's either #ifdef or no #ifdef, no point in turning
fw_cfg_find() into "static inline".

This version of the patch (without #ifdef CONFIG_SOFTMMU) actually builds
correctly, without linking errors due to missing fw_cfg symbols, on ALL
current qemu architectures (there's only *-softmmu and *-linux-user, BTW,
and the latter group doesn't actually appear to be using vl.c).

The "common-obj-y += vl.o" line in the toplevel Makefile.objs is wrapped
inside an "ifeq ($(CONFIG_SOFTMMU),y) ... endif", which would make checking
for CONFIG_SOFTMMU inside vl.c redundant anyway, so we should be good on
that front as well.

If I'm still missing anything, or something pops up that we didn't talk
about, I'd be happy to throw out a v4, just let me know.

Thanks,
   Gabriel

 docs/specs/fw_cfg.txt | 21 +++++++++++++++++
 qemu-options.hx       | 11 +++++++++
 vl.c                  | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 6accd92..74351dd 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -203,3 +203,24 @@ completes fully overwriting the item's data.
 
 NOTE: This function is deprecated, and will be completely removed
 starting with QEMU v2.4.
+
+== Externally Provided Items ==
+
+As of v2.4, "file" fw_cfg items (i.e., items with selector keys above
+FW_CFG_FILE_FIRST, and with a corresponding entry in the fw_cfg file
+directory structure) may be inserted via the QEMU command line, using
+the following syntax:
+
+    -fw_cfg [name=]<item_name>,file=<path>
+
+where <item_name> is the fw_cfg item name, and <path> is the location
+on the host file system of a file containing the data to be inserted.
+
+NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/"
+when using the "-fw_cfg" command line option, to avoid conflicting with
+item names used internally by QEMU. For instance:
+
+    -fw_cfg name=opt/my_item_name,file=./my_blob.bin
+
+Similarly, QEMU developers *SHOULD NOT* use item names prefixed with
+"opt/" when inserting items programmatically, e.g. via fw_cfg_add_file().
diff --git a/qemu-options.hx b/qemu-options.hx
index 319d971..138b9cd 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2668,6 +2668,17 @@ STEXI
 @table @option
 ETEXI
 
+DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg,
+    "-fw_cfg name=<name>,file=<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 75ec292..1fc047d 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
  *
@@ -2118,6 +2137,38 @@ char *qemu_find_file(int type, const char *name)
     return NULL;
 }
 
+static int parse_fw_cfg(QemuOpts *opts, void *opaque)
+{
+    gchar *buf;
+    size_t size;
+    const char *name, *file;
+
+    if (opaque == NULL) {
+        error_report("fw_cfg device not available");
+        return -1;
+    }
+    name = qemu_opt_get(opts, "name");
+    file = qemu_opt_get(opts, "file");
+    if (name == NULL || *name == '\0' || file == NULL || *file == '\0') {
+        error_report("invalid argument value");
+        return -1;
+    }
+    if (strlen(name) > FW_CFG_MAX_FILE_PATH - 1) {
+        error_report("name too long (max. %d char)", FW_CFG_MAX_FILE_PATH - 1);
+        return -1;
+    }
+    if (strncmp(name, "opt/", 4) != 0) {
+        error_report("WARNING: externally provided fw_cfg item names "
+                     "should be prefixed with \"opt/\"!");
+    }
+    if (!g_file_get_contents(file, &buf, &size, NULL)) {
+        error_report("can't load %s", file);
+        return -1;
+    }
+    fw_cfg_add_file((FWCfgState *)opaque, name, buf, size);
+    return 0;
+}
+
 static int device_help_func(QemuOpts *opts, void *opaque)
 {
     return qdev_device_help(opts);
@@ -2806,6 +2857,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();
 
@@ -3422,6 +3474,12 @@ 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);
+                }
+                break;
             case QEMU_OPTION_enable_kvm:
                 olist = qemu_find_opts("machine");
                 qemu_opts_parse(olist, "accel=kvm", 0);
@@ -4231,6 +4289,11 @@ int main(int argc, char **argv, char **envp)
 
     numa_post_machine_init();
 
+    if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
+                          parse_fw_cfg, fw_cfg_find(), 1) != 0) {
+        exit(1);
+    }
+
     /* init USB devices */
     if (usb_enabled()) {
         if (foreach_device_config(DEV_USB, usb_parse) < 0)
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline

[Laszlo Ersek]

comments below

On 03/21/15 21:23, Gabriel L. Somlo wrote:
> 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. A warning message will be
> printed if the fw_cfg item name does not begin with the
> prefix "opt/", which is recommended for external, user
> provided blobs.
> 
> Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> ---
> 
> I tested that object_resolve_path() always returns NULL when it can't find
> a match for the given argument (i.e., rather than triggering an assert);
> this means on architectures which do not create/initialize a fw_cfg object,
> the call to fw_cfg_find() returns NULL.

Great!

> I spent a while trying to decide whether to wrap the vl.c bits
> in #ifdef CONFIG_SOFTMMU or not. Making fw_cfg_find() an always-available
> static inline doesn't actually help, since I'm also calling fw_cfg_add_file()
> from parse_fw_cfg(), so it's either #ifdef or no #ifdef, no point in turning
> fw_cfg_find() into "static inline".
> 
> This version of the patch (without #ifdef CONFIG_SOFTMMU) actually builds
> correctly, without linking errors due to missing fw_cfg symbols, on ALL
> current qemu architectures (there's only *-softmmu and *-linux-user, BTW,
> and the latter group doesn't actually appear to be using vl.c).
> 
> The "common-obj-y += vl.o" line in the toplevel Makefile.objs is wrapped
> inside an "ifeq ($(CONFIG_SOFTMMU),y) ... endif", which would make checking
> for CONFIG_SOFTMMU inside vl.c redundant anyway, so we should be good on
> that front as well.

Cool.

> If I'm still missing anything, or something pops up that we didn't talk
> about, I'd be happy to throw out a v4, just let me know.
> 
> Thanks,
>    Gabriel
> 
>  docs/specs/fw_cfg.txt | 21 +++++++++++++++++
>  qemu-options.hx       | 11 +++++++++
>  vl.c                  | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 95 insertions(+)
> 
> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> index 6accd92..74351dd 100644
> --- a/docs/specs/fw_cfg.txt
> +++ b/docs/specs/fw_cfg.txt
> @@ -203,3 +203,24 @@ completes fully overwriting the item's data.
>  
>  NOTE: This function is deprecated, and will be completely removed
>  starting with QEMU v2.4.
> +
> +== Externally Provided Items ==
> +
> +As of v2.4, "file" fw_cfg items (i.e., items with selector keys above
> +FW_CFG_FILE_FIRST, and with a corresponding entry in the fw_cfg file
> +directory structure) may be inserted via the QEMU command line, using
> +the following syntax:
> +
> +    -fw_cfg [name=]<item_name>,file=<path>
> +
> +where <item_name> is the fw_cfg item name, and <path> is the location
> +on the host file system of a file containing the data to be inserted.
> +
> +NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/"
> +when using the "-fw_cfg" command line option, to avoid conflicting with
> +item names used internally by QEMU. For instance:
> +
> +    -fw_cfg name=opt/my_item_name,file=./my_blob.bin
> +
> +Similarly, QEMU developers *SHOULD NOT* use item names prefixed with
> +"opt/" when inserting items programmatically, e.g. via fw_cfg_add_file().

Nice. Thanks.

> diff --git a/qemu-options.hx b/qemu-options.hx
> index 319d971..138b9cd 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2668,6 +2668,17 @@ STEXI
>  @table @option
>  ETEXI
>  
> +DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg,
> +    "-fw_cfg name=<name>,file=<file>\n"

I guess I should have pointed this out earlier -- you could have
bracketed the "name=" part, to communicate that "name" is
"implied_opt_name".

Anyway, don't respin just because of this.

> +    "                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 75ec292..1fc047d 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
>   *
> @@ -2118,6 +2137,38 @@ char *qemu_find_file(int type, const char *name)
>      return NULL;
>  }
>  
> +static int parse_fw_cfg(QemuOpts *opts, void *opaque)
> +{
> +    gchar *buf;
> +    size_t size;
> +    const char *name, *file;
> +
> +    if (opaque == NULL) {
> +        error_report("fw_cfg device not available");
> +        return -1;
> +    }
> +    name = qemu_opt_get(opts, "name");
> +    file = qemu_opt_get(opts, "file");
> +    if (name == NULL || *name == '\0' || file == NULL || *file == '\0') {
> +        error_report("invalid argument value");
> +        return -1;
> +    }
> +    if (strlen(name) > FW_CFG_MAX_FILE_PATH - 1) {
> +        error_report("name too long (max. %d char)", FW_CFG_MAX_FILE_PATH - 1);
> +        return -1;
> +    }
> +    if (strncmp(name, "opt/", 4) != 0) {
> +        error_report("WARNING: externally provided fw_cfg item names "
> +                     "should be prefixed with \"opt/\"!");
> +    }
> +    if (!g_file_get_contents(file, &buf, &size, NULL)) {
> +        error_report("can't load %s", file);
> +        return -1;
> +    }
> +    fw_cfg_add_file((FWCfgState *)opaque, name, buf, size);
> +    return 0;
> +}
> +
>  static int device_help_func(QemuOpts *opts, void *opaque)
>  {
>      return qdev_device_help(opts);
> @@ -2806,6 +2857,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();
>  
> @@ -3422,6 +3474,12 @@ 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);
> +                }
> +                break;
>              case QEMU_OPTION_enable_kvm:
>                  olist = qemu_find_opts("machine");
>                  qemu_opts_parse(olist, "accel=kvm", 0);
> @@ -4231,6 +4289,11 @@ int main(int argc, char **argv, char **envp)
>  
>      numa_post_machine_init();
>  
> +    if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
> +                          parse_fw_cfg, fw_cfg_find(), 1) != 0) {
> +        exit(1);
> +    }
> +
>      /* init USB devices */
>      if (usb_enabled()) {
>          if (foreach_device_config(DEV_USB, usb_parse) < 0)
> 

Nice.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [Qemu-devel] [PATCH v3 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline

[Gabriel L. Somlo]

On Mon, Mar 23, 2015 at 09:48:52AM +0100, Laszlo Ersek wrote:
> 
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 319d971..138b9cd 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -2668,6 +2668,17 @@ STEXI
> >  @table @option
> >  ETEXI
> >  
> > +DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg,
> > +    "-fw_cfg name=<name>,file=<file>\n"
> 
> I guess I should have pointed this out earlier -- you could have
> bracketed the "name=" part, to communicate that "name" is
> "implied_opt_name".
> 
> Anyway, don't respin just because of this.

Well, this *should* go in, aesthetics are important to me too :)
If I get more feedback, I'll work this into a v4 of the series.

However, if this is it, and everything is OK otherwise, what's the
standard procedure for fixing just this one minor item ? Do I still
send out a v4, or is this something the maintainer ultimately applying
the patch would rather do themselves ?

Thanks again,
--Gabriel

Re: [Qemu-devel] [PATCH v3 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline

[Laszlo Ersek]

On 03/23/15 14:19, Gabriel L. Somlo wrote:
> On Mon, Mar 23, 2015 at 09:48:52AM +0100, Laszlo Ersek wrote:
>>
>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>> index 319d971..138b9cd 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -2668,6 +2668,17 @@ STEXI
>>>  @table @option
>>>  ETEXI
>>>  
>>> +DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg,
>>> +    "-fw_cfg name=<name>,file=<file>\n"
>>
>> I guess I should have pointed this out earlier -- you could have
>> bracketed the "name=" part, to communicate that "name" is
>> "implied_opt_name".
>>
>> Anyway, don't respin just because of this.
> 
> Well, this *should* go in, aesthetics are important to me too :)
> If I get more feedback, I'll work this into a v4 of the series.
> 
> However, if this is it, and everything is OK otherwise, what's the
> standard procedure for fixing just this one minor item ? Do I still
> send out a v4, or is this something the maintainer ultimately applying
> the patch would rather do themselves ?

Depends on the exact maintainer picking up your series.

... Now good luck figuring out who that's going to be :)

(I ran "scripts/get_maintainer.pl" on a squashed diff for this series.
It named only Paolo, but for the wrong reason: "vl.c" matched "Main
loop" from the MAINTAINERS file. fw_cfg has no dedicated owner, apparently.)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v3 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline

[Paolo Bonzini]

On 23/03/2015 15:11, Laszlo Ersek wrote:
> Depends on the exact maintainer picking up your series.
> 
> ... Now good luck figuring out who that's going to be :)
> 
> (I ran "scripts/get_maintainer.pl" on a squashed diff for this series.
> It named only Paolo, but for the wrong reason: "vl.c" matched "Main
> loop" from the MAINTAINERS file. fw_cfg has no dedicated owner, apparently.)

Gerd replied with "Looks good to me", so I think it should go through
him or through mst.

Paolo

Re: [Qemu-devel] [PATCH v3 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline

[Gabriel L. Somlo]

On Mon, Mar 23, 2015 at 04:52:44PM +0100, Paolo Bonzini wrote:
> On 23/03/2015 15:11, Laszlo Ersek wrote:
> > Depends on the exact maintainer picking up your series.
> > 
> > ... Now good luck figuring out who that's going to be :)
> > 
> > (I ran "scripts/get_maintainer.pl" on a squashed diff for this series.
> > It named only Paolo, but for the wrong reason: "vl.c" matched "Main
> > loop" from the MAINTAINERS file. fw_cfg has no dedicated owner, apparently.)
> 
> Gerd replied with "Looks good to me", so I think it should go through
> him or through mst.

I'm guessing I'll send out a v4 of this series (for context, see
http://lists.nongnu.org/archive/html/qemu-devel/2015-03/msg04586.html)
once 2.3 is out and the code freeze is over.

In the mean time, the commit log for 3a5c76ba mentions "the fw_cfg
documentation".

Would it make sense for me to re-send just 1/5
("fw_cfg: add documentation file") to actually provide the
above-mentioned documentation file with 2.3, leaving only
patches 2..5 (the actual code changes, subject to the freeze)
for 2.4 resubmission ?

Thanks,
--Gabriel

Re: [Qemu-devel] [PATCH v3 5/5] fw_cfg: insert fw_cfg file blobs via qemu cmdline

[Gerd Hoffmann]

  Hi,

> Would it make sense for me to re-send just 1/5
> ("fw_cfg: add documentation file") to actually provide the
> above-mentioned documentation file with 2.3, leaving only
> patches 2..5 (the actual code changes, subject to the freeze)
> for 2.4 resubmission ?

Yes, doc updates are fine even in freeze.

cheers,
  Gerd