Merge remote-tracking branch 'remotes/kraxel/tags/pull-fw_cfg-2015061…

…0-1' into staging

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

# gpg: Signature made Wed Jun 10 07:29:53 2015 BST using RSA key ID D3E87138
# gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>"
# gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>"
# gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>"

* remotes/kraxel/tags/pull-fw_cfg-20150610-1:
  bios-tables-test: handle false-positive smbios signature matches
  fw_cfg: insert fw_cfg file blobs via qemu cmdline
  fw_cfg: prohibit insertion of duplicate fw_cfg file names
  fw_cfg: prevent selector key conflict
  fw_cfg: remove support for guest-side data writes
  fw_cfg: fix FW_CFG_BOOT_DEVICE update on ppc and sparc
  fw_cfg: add fw_cfg_modify_i16 (update) method
  QemuOpts: increase number of vm_config_groups

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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().

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)
@@ -436,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;
@@ -458,7 +446,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;
 }
@@ -484,6 +471,16 @@ void fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value)
     fw_cfg_add_bytes(s, key, copy, sizeof(value));
 }
 
+void fw_cfg_modify_i16(FWCfgState *s, uint16_t key, uint16_t value)
+{
+    uint16_t *copy, *old;
+
+    copy = g_malloc(sizeof(value));
+    *copy = cpu_to_le16(value);
+    old = fw_cfg_modify_bytes_read(s, key, copy, sizeof(value));
+    g_free(old);
+}
+
 void fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value)
 {
     uint32_t *copy;
@@ -502,23 +499,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)
@@ -535,18 +515,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);

hw/ppc/mac_newworld.c

@@ -119,7 +119,7 @@ static const MemoryRegionOps unin_ops = {
 static void fw_cfg_boot_set(void *opaque, const char *boot_device,
                             Error **errp)
 {
-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
+    fw_cfg_modify_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
 }
 
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)

hw/ppc/mac_oldworld.c

@@ -52,7 +52,7 @@
 static void fw_cfg_boot_set(void *opaque, const char *boot_device,
                             Error **errp)
 {
-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
+    fw_cfg_modify_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
 }
 
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)

hw/sparc/sun4m.c

@@ -124,7 +124,7 @@ void DMA_register_channel (int nchan,
 static void fw_cfg_boot_set(void *opaque, const char *boot_device,
                             Error **errp)
 {
-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
+    fw_cfg_modify_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
 }
 
 static void nvram_init(Nvram *nvram, uint8_t *macaddr,

hw/sparc64/sun4u.c

@@ -127,7 +127,7 @@ void DMA_register_channel (int nchan,
 static void fw_cfg_boot_set(void *opaque, const char *boot_device,
                             Error **errp)
 {
-    fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
+    fw_cfg_modify_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
 }
 
 static int sun4u_NVRAM_set_params(Nvram *nvram, uint16_t NVRAM_size,

include/hw/nvram/fw_cfg.h

@@ -67,10 +67,9 @@ typedef void (*FWCfgReadCallback)(void *opaque, uint32_t offset);
 void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len);
 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_modify_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,

qemu-options.hx

@@ -2686,6 +2686,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)

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);

trace-events

@@ -193,10 +193,8 @@ 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"
 fw_cfg_add_file(void *s, int index, char *name, size_t len) "%p #%d: %s (%zd bytes)"
 
 # hw/block/hd-geometry.c

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,