[Qemu-devel] [PULL 0/6] fw_cfg patch queue

[Qemu-devel] [PULL 0/6] fw_cfg patch queue

[Gerd Hoffmann]

  Hi,

Here comes the fw_cfg patch queue, bringing a patch series from Gabriel
which drops the (unused) fw_cfg write support and adds a command line
switch to add blobs to fw_cfg.  Also some bugfixes.

please pull,
  Gerd

The following changes since commit 00967f4e0bab246679d0ddc32fd31a7179345baf:

  Merge remote-tracking branch 'remotes/agraf/tags/signed-s390-for-upstream' into staging (2015-06-05 12:04:42 +0100)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-fw_cfg-20150605-1

for you to fetch changes up to 87140d16f51a5dca1d312c7705907d7122413939:

  bios-tables-test: handle false-positive smbios signature matches (2015-06-05 15:49:53 +0200)

----------------------------------------------------------------
fw_cfg: drop write support, qemu cmdline support, bugfixes.
bios-tables-test: fix smbios test.

----------------------------------------------------------------
Gabriel L. Somlo (5):
      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
      bios-tables-test: handle false-positive smbios signature matches

Gerd Hoffmann (1):
      QemuOpts: increase number of vm_config_groups

 docs/specs/fw_cfg.txt     | 21 +++++++++++++
 hw/nvram/fw_cfg.c         | 45 +++++-----------------------
 include/hw/nvram/fw_cfg.h |  2 --
 qemu-options.hx           | 11 +++++++
 tests/bios-tables-test.c  | 76 +++++++++++++++++++++++++++--------------------
 trace-events              |  2 --
 util/qemu-config.c        |  2 +-
 vl.c                      | 63 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 148 insertions(+), 74 deletions(-)

[Qemu-devel] [PULL 1/6] QemuOpts: increase number of vm_config_groups

[Gerd Hoffmann]

Adding the fw_cfg cmd line support patch by
Gabriel L. Somlo hits the limit.

Fix this by making the array larger.

Cc: Gabriel L. Somlo <somlo@cmu.edu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 util/qemu-config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/qemu-config.c b/util/qemu-config.c
index 30d6dcf..4dfde06 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -6,7 +6,7 @@
 #include "qapi/error.h"
 #include "qmp-commands.h"
 
-static QemuOptsList *vm_config_groups[32];
+static QemuOptsList *vm_config_groups[48];
 static QemuOptsList *drive_config_groups[4];
 
 static QemuOptsList *find_list(QemuOptsList **lists, const char *group,
-- 
1.8.3.1

[Qemu-devel] [PULL 2/6] fw_cfg: remove support for guest-side data writes

[Gerd Hoffmann]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

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>
Signed-off-by: Gerd Hoffmann <kraxel@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 a589650..27d4ba7 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"
-- 
1.8.3.1

[Qemu-devel] [PULL 3/6] fw_cfg: prevent selector key conflict

[Gerd Hoffmann]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

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>
Signed-off-by: Gerd Hoffmann <kraxel@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;
-- 
1.8.3.1

[Qemu-devel] [PULL 4/6] fw_cfg: prohibit insertion of duplicate fw_cfg file names

[Gerd Hoffmann]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

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>
Signed-off-by: Gerd Hoffmann <kraxel@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 27d4ba7..9a29df7 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
-- 
1.8.3.1

[Qemu-devel] [PULL 5/6] fw_cfg: insert fw_cfg file blobs via qemu cmdline

[Gerd Hoffmann]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 b3db6cb..7f3d314 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2682,6 +2682,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 cdd81b4..b4aaaeb 100644
--- a/vl.c
+++ b/vl.c
@@ -489,6 +489,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
  *
@@ -2124,6 +2143,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);
@@ -2812,6 +2863,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();
 
@@ -3428,6 +3480,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);
@@ -4252,6 +4310,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)
-- 
1.8.3.1

[Qemu-devel] [PULL 6/6] bios-tables-test: handle false-positive smbios signature matches

[Gerd Hoffmann]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

It has been reported that sometimes the .rodata section of SeaBIOS,
containing the constant string against which the SMBIOS signature
ends up being compared, also falls within the guest f-segment. In
that case, the test obviously fails, unless we continue searching
for the *real* SMBIOS entry point.

Rather than stopping at the first match for the SMBIOS signature
("_SM_") in the f-segment (0xF0000-0xFFFFF), continue scanning
until either a valid entry point table is found, or the f-segment
has been exhausted.

Reported-by: Bruce Rogers <brogers@suse.com>
Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
Tested-by: Bruce Rogers <brogers@suse.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/bios-tables-test.c | 76 ++++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 32 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 7e85dc4..0de1742 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -599,35 +599,15 @@ static void test_acpi_asl(test_data *data)
     free_test_data(&exp_data);
 }
 
-static void test_smbios_ep_address(test_data *data)
-{
-    uint32_t off;
-
-    /* find smbios entry point structure */
-    for (off = 0xf0000; off < 0x100000; off += 0x10) {
-        uint8_t sig[] = "_SM_";
-        int i;
-
-        for (i = 0; i < sizeof sig - 1; ++i) {
-            sig[i] = readb(off + i);
-        }
-
-        if (!memcmp(sig, "_SM_", sizeof sig)) {
-            break;
-        }
-    }
-
-    g_assert_cmphex(off, <, 0x100000);
-    data->smbios_ep_addr = off;
-}
-
-static void test_smbios_ep_table(test_data *data)
+static bool smbios_ep_table_ok(test_data *data)
 {
     struct smbios_entry_point *ep_table = &data->smbios_ep_table;
     uint32_t addr = data->smbios_ep_addr;
 
     ACPI_READ_ARRAY(ep_table->anchor_string, addr);
-    g_assert(!memcmp(ep_table->anchor_string, "_SM_", 4));
+    if (memcmp(ep_table->anchor_string, "_SM_", 4)) {
+        return false;
+    }
     ACPI_READ_FIELD(ep_table->checksum, addr);
     ACPI_READ_FIELD(ep_table->length, addr);
     ACPI_READ_FIELD(ep_table->smbios_major_version, addr);
@@ -636,17 +616,50 @@ static void test_smbios_ep_table(test_data *data)
     ACPI_READ_FIELD(ep_table->entry_point_revision, addr);
     ACPI_READ_ARRAY(ep_table->formatted_area, addr);
     ACPI_READ_ARRAY(ep_table->intermediate_anchor_string, addr);
-    g_assert(!memcmp(ep_table->intermediate_anchor_string, "_DMI_", 5));
+    if (memcmp(ep_table->intermediate_anchor_string, "_DMI_", 5)) {
+        return false;
+    }
     ACPI_READ_FIELD(ep_table->intermediate_checksum, addr);
     ACPI_READ_FIELD(ep_table->structure_table_length, addr);
-    g_assert_cmpuint(ep_table->structure_table_length, >, 0);
+    if (ep_table->structure_table_length == 0) {
+        return false;
+    }
     ACPI_READ_FIELD(ep_table->structure_table_address, addr);
     ACPI_READ_FIELD(ep_table->number_of_structures, addr);
-    g_assert_cmpuint(ep_table->number_of_structures, >, 0);
+    if (ep_table->number_of_structures == 0) {
+        return false;
+    }
     ACPI_READ_FIELD(ep_table->smbios_bcd_revision, addr);
-    g_assert(!acpi_checksum((uint8_t *)ep_table, sizeof *ep_table));
-    g_assert(!acpi_checksum((uint8_t *)ep_table + 0x10,
-                            sizeof *ep_table - 0x10));
+    if (acpi_checksum((uint8_t *)ep_table, sizeof *ep_table) ||
+        acpi_checksum((uint8_t *)ep_table + 0x10, sizeof *ep_table - 0x10)) {
+        return false;
+    }
+    return true;
+}
+
+static void test_smbios_entry_point(test_data *data)
+{
+    uint32_t off;
+
+    /* find smbios entry point structure */
+    for (off = 0xf0000; off < 0x100000; off += 0x10) {
+        uint8_t sig[] = "_SM_";
+        int i;
+
+        for (i = 0; i < sizeof sig - 1; ++i) {
+            sig[i] = readb(off + i);
+        }
+
+        if (!memcmp(sig, "_SM_", sizeof sig)) {
+            /* signature match, but is this a valid entry point? */
+            data->smbios_ep_addr = off;
+            if (smbios_ep_table_ok(data)) {
+                break;
+            }
+        }
+    }
+
+    g_assert_cmphex(off, <, 0x100000);
 }
 
 static inline bool smbios_single_instance(uint8_t type)
@@ -767,8 +780,7 @@ static void test_acpi_one(const char *params, test_data *data)
         }
     }
 
-    test_smbios_ep_address(data);
-    test_smbios_ep_table(data);
+    test_smbios_entry_point(data);
     test_smbios_structs(data);
 
     qtest_quit(global_qtest);
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 2/6] fw_cfg: remove support for guest-side data writes

[Gabriel L. Somlo]

Gerd,

On Fri, Jun 05, 2015 at 04:14:17PM +0200, Gerd Hoffmann wrote:
> From: "Gabriel L. Somlo" <somlo@cmu.edu>
> 

There's a missing line in the commit blurb:

"From this point forward, any guest-side writes to the fw_cfg"

Thanks,
--Gabriel

> 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>
> Signed-off-by: Gerd Hoffmann <kraxel@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 a589650..27d4ba7 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"
> -- 
> 1.8.3.1
> 

Re: [Qemu-devel] [PULL 0/6] fw_cfg patch queue

[Peter Maydell]

On 5 June 2015 at 15:14, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Here comes the fw_cfg patch queue, bringing a patch series from Gabriel
> which drops the (unused) fw_cfg write support and adds a command line
> switch to add blobs to fw_cfg.  Also some bugfixes.
>
> please pull,
>   Gerd
>
> The following changes since commit 00967f4e0bab246679d0ddc32fd31a7179345baf:
>
>   Merge remote-tracking branch 'remotes/agraf/tags/signed-s390-for-upstream' into staging (2015-06-05 12:04:42 +0100)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-fw_cfg-20150605-1
>
> for you to fetch changes up to 87140d16f51a5dca1d312c7705907d7122413939:
>
>   bios-tables-test: handle false-positive smbios signature matches (2015-06-05 15:49:53 +0200)
>
> ----------------------------------------------------------------
> fw_cfg: drop write support, qemu cmdline support, bugfixes.
> bios-tables-test: fix smbios test.

Hi. I'm afraid this fails make check for me:

GTESTER check-qtest-ppc
qemu-system-ppc:
/home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:426:
fw_cfg_add_bytes_read_callback: Assertion `s->entries[arch][key].data
== ((void *)0)' failed.
Broken pipe
GTester: last random seed: R02Sd9273ba718a6b28ff4381eed659a0cb4
qemu-system-ppc:
/home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:426:
fw_cfg_add_bytes_read_callback: Assertion `s->entries[arch][key].data
== ((void *)0)' failed.
Broken pipe
GTester: last random seed: R02S59d67a130b823e6e6d04a41eb4b168c7

-- PMM

Re: [Qemu-devel] [PULL 0/6] fw_cfg patch queue

[Gerd Hoffmann]

> Hi. I'm afraid this fails make check for me:
> 
> GTESTER check-qtest-ppc
> qemu-system-ppc:
> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:426:
> fw_cfg_add_bytes_read_callback: Assertion `s->entries[arch][key].data
> == ((void *)0)' failed.
> Broken pipe
> GTester: last random seed: R02Sd9273ba718a6b28ff4381eed659a0cb4
> qemu-system-ppc:
> /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:426:
> fw_cfg_add_bytes_read_callback: Assertion `s->entries[arch][key].data
> == ((void *)0)' failed.
> Broken pipe
> GTester: last random seed: R02S59d67a130b823e6e6d04a41eb4b168c7

Reproducer: "qemu-system-ppc -M g3beige -boot once=d,order=c".
Culprit: Patch #3 (fw_cfg: prevent selector key conflict).
Looks like FW_CFG_BOOT_DEVICE is set twice for macs.

Gabriel?  To me this looks like the assert found an actual bug ...

cheers,
  Gerd

Re: [Qemu-devel] [PULL 2/6] fw_cfg: remove support for guest-side data writes

[Gerd Hoffmann]

On Fr, 2015-06-05 at 10:20 -0400, Gabriel L. Somlo wrote:
> Gerd,
> 
> On Fri, Jun 05, 2015 at 04:14:17PM +0200, Gerd Hoffmann wrote:
> > From: "Gabriel L. Somlo" <somlo@cmu.edu>
> > 
> 
> There's a missing line in the commit blurb:
> 
> "From this point forward, any guest-side writes to the fw_cfg"

Hmm, looks like git killed it, possibly because it started with ">From".
I have to re-do the poll anyway (see other mail), so I've added the line
back now, will be there in pull v2.

cheers,
  Gerd

Re: [Qemu-devel] [PULL 0/6] fw_cfg patch queue

[Gabriel L. Somlo]

On Mon, Jun 08, 2015 at 02:26:03PM +0200, Gerd Hoffmann wrote:
> > Hi. I'm afraid this fails make check for me:
> > 
> > GTESTER check-qtest-ppc
> > qemu-system-ppc:
> > /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:426:
> > fw_cfg_add_bytes_read_callback: Assertion `s->entries[arch][key].data
> > == ((void *)0)' failed.
> > Broken pipe
> > GTester: last random seed: R02Sd9273ba718a6b28ff4381eed659a0cb4
> > qemu-system-ppc:
> > /home/petmay01/linaro/qemu-for-merges/hw/nvram/fw_cfg.c:426:
> > fw_cfg_add_bytes_read_callback: Assertion `s->entries[arch][key].data
> > == ((void *)0)' failed.
> > Broken pipe
> > GTester: last random seed: R02S59d67a130b823e6e6d04a41eb4b168c7
> 
> Reproducer: "qemu-system-ppc -M g3beige -boot once=d,order=c".
> Culprit: Patch #3 (fw_cfg: prevent selector key conflict).
> Looks like FW_CFG_BOOT_DEVICE is set twice for macs.
> 
> Gabriel?  To me this looks like the assert found an actual bug ...

I think it did -- a (small) memory leak in the fw_cfg_boot_set()
callback. I'm sending out two patches which should go in before
the current set in this pull request (to maintain "bisectability").

Thanks,
--Gabriel