[Qemu-devel] [PULL 00/14] pc,pci,virtio fixes and cleanups

[Qemu-devel] [PULL 00/14] pc,pci,virtio fixes and cleanups

[Michael S. Tsirkin]

The following changes since commit 2d1fe1873a984d1c2c89ffa3d12949cafc718551:

  Merge remote-tracking branch 'pmaydell/tags/pull-target-arm-20130910' into staging (2013-09-11 14:46:52 -0500)

are available in the git repository at:


  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony

for you to fetch changes up to e26d3e734650640fabd7d95ace4f3a6f88725e0b:

  smbios: Factor out smbios_maybe_add_str() (2013-09-28 23:49:39 +0300)

----------------------------------------------------------------
pc,pci,virtio fixes and cleanups

This includes pc and pci cleanups and enhancements,
and a virtio-net bugfix related to softmac programming.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Hervé Poussineau (1):
  pci: remove explicit check to 64K ioport size

Markus Armbruster (5):
  smbios: Normalize smbios_entry_add()'s error handling to exit(1)
  smbios: Convert to QemuOpts
  smbios: Improve diagnostics for conflicting entries
  smbios: Make multiple -smbios type= accumulate sanely
  smbios: Factor out smbios_maybe_add_str()

Michael S. Tsirkin (8):
  q35: make pci window address/size match guest cfg
  range: add Range to typedefs
  range: add min/max operations on ranges
  pci: add helper to retrieve the 64-bit range
  q35: use 64 bit window programmed by guest
  piix: use 64 bit window programmed by guest
  piix4: disable io on reset
  virtio-net: fix up HMP NIC info string on reset

 include/hw/i386/smbios.h   |   5 +-
 include/hw/pci/pci.h       |   1 +
 include/qemu/range.h       |  20 ++-
 include/qemu/typedefs.h    |   1 +
 include/sysemu/arch_init.h |   2 +-
 include/sysemu/sysemu.h    |   1 +
 arch_init.c                |   9 +-
 hw/acpi/piix4.c            |   1 +
 hw/i386/smbios.c           | 339 ++++++++++++++++++++++++++++++++-------------
 hw/net/virtio-net.c        |   1 +
 hw/pci-host/piix.c         |  14 +-
 hw/pci-host/q35.c          |  24 +++-
 hw/pci/pci.c               |  56 +++++++-
 vl.c                       |   5 +-
 14 files changed, 359 insertions(+), 120 deletions(-)

-- 
MST

[Qemu-devel] [PULL 01/14] q35: make pci window address/size match guest cfg

[Michael S. Tsirkin]

For Q35, MMCFG address and size are guest configurable.
Update w32 property to make it behave accordingly.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/q35.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 5473504..72f6b72 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -214,6 +214,16 @@ static void mch_update_pciexbar(MCHPCIState *mch)
     }
     addr = pciexbar & addr_mask;
     pcie_host_mmcfg_update(pehb, enable, addr, length);
+    /* Leave enough space for the MCFG BAR */
+    /*
+     * TODO: this matches current bios behaviour, but it's not a power of two,
+     * which means an MTRR can't cover it exactly.
+     */
+    if (enable) {
+        mch->pci_info.w32.begin = addr + length;
+    } else {
+        mch->pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
+    }
 }
 
 /* PAM */
-- 
MST

[Qemu-devel] [PULL 02/14] range: add Range to typedefs

[Michael S. Tsirkin]

will help simplify header dependencies.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/qemu/range.h    | 2 +-
 include/qemu/typedefs.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu/range.h b/include/qemu/range.h
index b76cc0d..4a0780d 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -2,6 +2,7 @@
 #define QEMU_RANGE_H
 
 #include <inttypes.h>
+#include <qemu/typedefs.h>
 
 /*
  * Operations on 64 bit address ranges.
@@ -15,7 +16,6 @@ struct Range {
     uint64_t begin; /* First byte of the range, or 0 if empty. */
     uint64_t end;   /* 1 + the last byte. 0 if range empty or ends at ~0x0LL. */
 };
-typedef struct Range Range;
 
 /* Get last byte of a range from offset + length.
  * Undefined for ranges that wrap around 0. */
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 3205540..a4c1b84 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -68,5 +68,6 @@ typedef struct QEMUSGList QEMUSGList;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct FWCfgState FWCfgState;
 typedef struct PcGuestInfo PcGuestInfo;
+typedef struct Range Range;
 
 #endif /* QEMU_TYPEDEFS_H */
-- 
MST

[Qemu-devel] [PULL 03/14] range: add min/max operations on ranges

[Michael S. Tsirkin]

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/qemu/range.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/qemu/range.h b/include/qemu/range.h
index 4a0780d..aae9720 100644
--- a/include/qemu/range.h
+++ b/include/qemu/range.h
@@ -17,6 +17,24 @@ struct Range {
     uint64_t end;   /* 1 + the last byte. 0 if range empty or ends at ~0x0LL. */
 };
 
+static inline void range_extend(Range *range, Range *extend_by)
+{
+    if (!extend_by->begin && !extend_by->end) {
+        return;
+    }
+    if (!range->begin && !range->end) {
+        *range = *extend_by;
+        return;
+    }
+    if (range->begin > extend_by->begin) {
+        range->begin = extend_by->begin;
+    }
+    /* Compare last byte in case region ends at ~0x0LL */
+    if (range->end - 1 < extend_by->end - 1) {
+        range->end = extend_by->end;
+    }
+}
+
 /* Get last byte of a range from offset + length.
  * Undefined for ranges that wrap around 0. */
 static inline uint64_t range_get_last(uint64_t offset, uint64_t len)
-- 
MST

[Qemu-devel] [PULL 04/14] pci: add helper to retrieve the 64-bit range

[Michael S. Tsirkin]

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/pci/pci.h |  1 +
 hw/pci/pci.c         | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 37979aa..4b90e5d 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -397,6 +397,7 @@ const char *pci_root_bus_path(PCIDevice *dev);
 PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn);
 int pci_qdev_find_device(const char *id, PCIDevice **pdev);
 PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, const char *devaddr);
+void pci_bus_get_w64_range(PCIBus *bus, Range *range);
 
 int pci_parse_devaddr(const char *addr, int *domp, int *busp,
                       unsigned int *slotp, unsigned int *funcp);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ad1c1ca..52cbab7 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2257,6 +2257,56 @@ void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque)
     bus->iommu_opaque = opaque;
 }
 
+static void pci_dev_get_w64(PCIBus *b, PCIDevice *dev, void *opaque)
+{
+    Range *range = opaque;
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+    uint16_t cmd = pci_get_word(dev->config + PCI_COMMAND);
+    int r;
+
+    if (!(cmd & PCI_COMMAND_MEMORY)) {
+        return;
+    }
+
+    if (pc->is_bridge) {
+        pcibus_t base = pci_bridge_get_base(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+        pcibus_t limit = pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_MEM_PREFETCH);
+
+        base = MAX(base, 0x1ULL << 32);
+
+        if (limit >= base) {
+            Range pref_range;
+            pref_range.begin = base;
+            pref_range.end = limit + 1;
+            range_extend(range, &pref_range);
+        }
+    }
+    for (r = 0; r < PCI_NUM_REGIONS; ++r) {
+        PCIIORegion *region = &dev->io_regions[r];
+        Range region_range;
+
+        if (!region->size ||
+            (region->type & PCI_BASE_ADDRESS_SPACE_IO) ||
+            !(region->type & PCI_BASE_ADDRESS_MEM_TYPE_64)) {
+            continue;
+        }
+        region_range.begin = pci_get_quad(dev->config + pci_bar(dev, r));
+        region_range.end = region_range.begin + region->size;
+
+        region_range.begin = MAX(region_range.begin, 0x1ULL << 32);
+
+        if (region_range.end - 1 >= region_range.begin) {
+            range_extend(range, &region_range);
+        }
+    }
+}
+
+void pci_bus_get_w64_range(PCIBus *bus, Range *range)
+{
+    range->begin = range->end = 0;
+    pci_for_each_device_under_bus(bus, pci_dev_get_w64, range);
+}
+
 static const TypeInfo pci_device_type_info = {
     .name = TYPE_PCI_DEVICE,
     .parent = TYPE_DEVICE,
-- 
MST

[Qemu-devel] [PULL 05/14] q35: use 64 bit window programmed by guest

[Michael S. Tsirkin]

Detect the 64 bit window programmed by firmware
and configure properties accordingly.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/q35.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 72f6b72..23dbeea 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -89,18 +89,24 @@ static void q35_host_get_pci_hole64_start(Object *obj, Visitor *v,
                                           void *opaque, const char *name,
                                           Error **errp)
 {
-    Q35PCIHost *s = Q35_HOST_DEVICE(obj);
+    PCIHostState *h = PCI_HOST_BRIDGE(obj);
+    Range w64;
+
+    pci_bus_get_w64_range(h->bus, &w64);
 
-    visit_type_uint64(v, &s->mch.pci_info.w64.begin, name, errp);
+    visit_type_uint64(v, &w64.begin, name, errp);
 }
 
 static void q35_host_get_pci_hole64_end(Object *obj, Visitor *v,
                                         void *opaque, const char *name,
                                         Error **errp)
 {
-    Q35PCIHost *s = Q35_HOST_DEVICE(obj);
+    PCIHostState *h = PCI_HOST_BRIDGE(obj);
+    Range w64;
+
+    pci_bus_get_w64_range(h->bus, &w64);
 
-    visit_type_uint64(v, &s->mch.pci_info.w64.end, name, errp);
+    visit_type_uint64(v, &w64.end, name, errp);
 }
 
 static Property mch_props[] = {
-- 
MST

[Qemu-devel] [PULL 06/14] piix: use 64 bit window programmed by guest

[Michael S. Tsirkin]

Detect the 64 bit window programmed by firmware
and configure properties accordingly.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/piix.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 221d82b..c041149 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -235,18 +235,24 @@ static void i440fx_pcihost_get_pci_hole64_start(Object *obj, Visitor *v,
                                                 void *opaque, const char *name,
                                                 Error **errp)
 {
-    I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
+    PCIHostState *h = PCI_HOST_BRIDGE(obj);
+    Range w64;
+
+    pci_bus_get_w64_range(h->bus, &w64);
 
-    visit_type_uint64(v, &s->pci_info.w64.begin, name, errp);
+    visit_type_uint64(v, &w64.begin, name, errp);
 }
 
 static void i440fx_pcihost_get_pci_hole64_end(Object *obj, Visitor *v,
                                               void *opaque, const char *name,
                                               Error **errp)
 {
-    I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
+    PCIHostState *h = PCI_HOST_BRIDGE(obj);
+    Range w64;
+
+    pci_bus_get_w64_range(h->bus, &w64);
 
-    visit_type_uint64(v, &s->pci_info.w64.end, name, errp);
+    visit_type_uint64(v, &w64.end, name, errp);
 }
 
 static void i440fx_pcihost_initfn(Object *obj)
-- 
MST

[Qemu-devel] [PULL 07/14] piix4: disable io on reset

[Michael S. Tsirkin]

io base register at 0x40 is cleared on reset,
but io is not disabled until some other event
happens to call pm_io_space_update.

Invoke pm_io_space_update directly to make this
consistent.

Cc: qemu-stable@nongnu.org
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/piix4.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 0b8d1d9..b46bd5e 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -380,6 +380,7 @@ static void piix4_reset(void *opaque)
         /* Mark SMM as already inited (until KVM supports SMM). */
         pci_conf[0x5B] = 0x02;
     }
+    pm_io_space_update(s);
     piix4_update_hotplug(s);
 }
 
-- 
MST

[Qemu-devel] [PULL 08/14] pci: remove explicit check to 64K ioport size

[Michael S. Tsirkin]

From: Hervé Poussineau <hpoussin@reactos.org>

This check is useless, as bigger addresses will be ignored when
added to 'io' MemoryRegion, which has a size of 64K.

However, some architectures don't use the 'io' MemoryRegion, like
the alpha and versatile platforms. They create a PCI I/O region
bigger than 64K, so let them handle PCI I/O BARs in the higher range.

MST: reinstated work-around for BAR sizing.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 52cbab7..00554a0 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1028,8 +1028,10 @@ static pcibus_t pci_bar_address(PCIDevice *d,
         }
         new_addr = pci_get_long(d->config + bar) & ~(size - 1);
         last_addr = new_addr + size - 1;
-        /* NOTE: we have only 64K ioports on PC */
-        if (last_addr <= new_addr || new_addr == 0 || last_addr > UINT16_MAX) {
+        /* Check if 32 bit BAR wraps around explicitly.
+         * TODO: make priorities correct and remove this work around.
+         */
+        if (last_addr <= new_addr || new_addr == 0 || last_addr >= UINT32_MAX) {
             return PCI_BAR_UNMAPPED;
         }
         return new_addr;
-- 
MST

[Qemu-devel] [PULL 09/14] virtio-net: fix up HMP NIC info string on reset

[Michael S. Tsirkin]

When mac is updated on reset, info string has stale data.
Fix it up.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/virtio-net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index dd41008..22dbd05 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -314,6 +314,7 @@ static void virtio_net_reset(VirtIODevice *vdev)
     n->mac_table.uni_overflow = 0;
     memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
     memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac));
+    qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
     memset(n->vlans, 0, MAX_VLAN >> 3);
 }
 
-- 
MST

[Qemu-devel] [PULL 10/14] smbios: Normalize smbios_entry_add()'s error handling to exit(1)

[Michael S. Tsirkin]

From: Markus Armbruster <armbru@redhat.com>

It exits on all error conditions but one, where it returns -1.
Normalize, and return void.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/i386/smbios.h |  2 +-
 arch_init.c              |  4 +---
 hw/i386/smbios.c         | 10 +++++-----
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 9babeaf..56c6108 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -13,7 +13,7 @@
  *
  */
 
-int smbios_entry_add(const char *t);
+void smbios_entry_add(const char *t);
 void smbios_add_field(int type, int offset, const void *data, size_t len);
 uint8_t *smbios_get_table(size_t *length);
 
diff --git a/arch_init.c b/arch_init.c
index e47e139..6ae8eb6 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1137,9 +1137,7 @@ void do_acpitable_option(const QemuOpts *opts)
 void do_smbios_option(const char *optarg)
 {
 #ifdef TARGET_I386
-    if (smbios_entry_add(optarg) < 0) {
-        exit(1);
-    }
+    smbios_entry_add(optarg);
 #endif
 }
 
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index e708cb8..0608aee 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -183,7 +183,7 @@ static void smbios_build_type_1_fields(const char *t)
                          buf, strlen(buf) + 1);
 }
 
-int smbios_entry_add(const char *t)
+void smbios_entry_add(const char *t)
 {
     char buf[1024];
 
@@ -222,7 +222,7 @@ int smbios_entry_add(const char *t)
         smbios_entries_len += sizeof(*table) + size;
         (*(uint16_t *)smbios_entries) =
                 cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
-        return 0;
+        return;
     }
 
     if (get_param_value(buf, sizeof(buf), "type", t)) {
@@ -230,10 +230,10 @@ int smbios_entry_add(const char *t)
         switch (type) {
         case 0:
             smbios_build_type_0_fields(t);
-            return 0;
+            return;
         case 1:
             smbios_build_type_1_fields(t);
-            return 0;
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
@@ -242,5 +242,5 @@ int smbios_entry_add(const char *t)
     }
 
     error_report("Must specify type= or file=");
-    return -1;
+    exit(1);
 }
-- 
MST

[Qemu-devel] [PULL 11/14] smbios: Convert to QemuOpts

[Michael S. Tsirkin]

From: Markus Armbruster <armbru@redhat.com>

So that it can be set in config file for -readconfig.

This tightens parsing of -smbios, and makes it more consistent with
other options: unknown parameters are rejected, numbers with trailing
junk are rejected, when a parameter is given multiple times, last
rather than first wins, ...

MST: drop one chunk to fix build errors

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/i386/smbios.h   |   4 +-
 include/sysemu/arch_init.h |   2 +-
 arch_init.c                |   4 +-
 hw/i386/smbios.c           | 209 ++++++++++++++++++++++++++++++++++++---------
 vl.c                       |   3 +-
 5 files changed, 179 insertions(+), 43 deletions(-)

diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 56c6108..d9f43b7 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -13,7 +13,9 @@
  *
  */
 
-void smbios_entry_add(const char *t);
+#include "qemu/option.h"
+
+void smbios_entry_add(QemuOpts *opts);
 void smbios_add_field(int type, int offset, const void *data, size_t len);
 uint8_t *smbios_get_table(size_t *length);
 
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index dece913..be71bca 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -28,7 +28,7 @@ extern const uint32_t arch_type;
 
 void select_soundhw(const char *optarg);
 void do_acpitable_option(const QemuOpts *opts);
-void do_smbios_option(const char *optarg);
+void do_smbios_option(QemuOpts *opts);
 void cpudef_init(void);
 void audio_init(void);
 int tcg_available(void);
diff --git a/arch_init.c b/arch_init.c
index 6ae8eb6..62f1118 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1134,10 +1134,10 @@ void do_acpitable_option(const QemuOpts *opts)
 #endif
 }
 
-void do_smbios_option(const char *optarg)
+void do_smbios_option(QemuOpts *opts)
 {
 #ifdef TARGET_I386
-    smbios_entry_add(optarg);
+    smbios_entry_add(opts);
 #endif
 }
 
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 0608aee..abfd6f7 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -2,9 +2,11 @@
  * SMBIOS Support
  *
  * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
+ * Copyright (C) 2013 Red Hat, Inc.
  *
  * Authors:
  *  Alex Williamson <alex.williamson@hp.com>
+ *  Markus Armbruster <armbru@redhat.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2.  See
  * the COPYING file in the top-level directory.
@@ -13,6 +15,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
+#include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "hw/i386/smbios.h"
@@ -41,11 +44,100 @@ struct smbios_table {
 #define SMBIOS_FIELD_ENTRY 0
 #define SMBIOS_TABLE_ENTRY 1
 
-
 static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 
+static QemuOptsList qemu_smbios_opts = {
+    .name = "smbios",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
+    .desc = {
+        /*
+         * no elements => accept any params
+         * validation will happen later
+         */
+        { /* end of list */ }
+    }
+};
+
+static const QemuOptDesc qemu_smbios_file_opts[] = {
+    {
+        .name = "file",
+        .type = QEMU_OPT_STRING,
+        .help = "binary file containing an SMBIOS element",
+    },
+    { /* end of list */ }
+};
+
+static const QemuOptDesc qemu_smbios_type0_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "vendor",
+        .type = QEMU_OPT_STRING,
+        .help = "vendor name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "date",
+        .type = QEMU_OPT_STRING,
+        .help = "release date",
+    },{
+        .name = "release",
+        .type = QEMU_OPT_STRING,
+        .help = "revision number",
+    },
+    { /* end of list */ }
+};
+
+static const QemuOptDesc qemu_smbios_type1_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "product",
+        .type = QEMU_OPT_STRING,
+        .help = "product name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "uuid",
+        .type = QEMU_OPT_STRING,
+        .help = "UUID",
+    },{
+        .name = "sku",
+        .type = QEMU_OPT_STRING,
+        .help = "SKU number",
+    },{
+        .name = "family",
+        .type = QEMU_OPT_STRING,
+        .help = "family name",
+    },
+    { /* end of list */ }
+};
+
+static void smbios_register_config(void)
+{
+    qemu_add_opts(&qemu_smbios_opts);
+}
+
+machine_init(smbios_register_config);
+
 static void smbios_validate_table(void)
 {
     if (smbios_type4_count && smbios_type4_count != smp_cpus) {
@@ -124,23 +216,30 @@ void smbios_add_field(int type, int offset, const void *data, size_t len)
             cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
 }
 
-static void smbios_build_type_0_fields(const char *t)
+static void smbios_build_type_0_fields(QemuOpts *opts)
 {
-    char buf[1024];
+    const char *val;
     unsigned char major, minor;
 
-    if (get_param_value(buf, sizeof(buf), "vendor", t))
+    val = qemu_opt_get(opts, "vendor");
+    if (val) {
         smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "version", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "version");
+    if (val) {
         smbios_add_field(0, offsetof(struct smbios_type_0, bios_version_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "date", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "date");
+    if (val) {
         smbios_add_field(0, offsetof(struct smbios_type_0,
                                      bios_release_date_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "release", t)) {
-        if (sscanf(buf, "%hhu.%hhu", &major, &minor) != 2) {
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "release");
+    if (val) {
+        if (sscanf(val, "%hhu.%hhu", &major, &minor) != 2) {
             error_report("Invalid release");
             exit(1);
         }
@@ -153,47 +252,69 @@ static void smbios_build_type_0_fields(const char *t)
     }
 }
 
-static void smbios_build_type_1_fields(const char *t)
+static void smbios_build_type_1_fields(QemuOpts *opts)
 {
-    char buf[1024];
+    const char *val;
 
-    if (get_param_value(buf, sizeof(buf), "manufacturer", t))
+    val = qemu_opt_get(opts, "manufacturer");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, manufacturer_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "product", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "product");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, product_name_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "version", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "version");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, version_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "serial", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "serial");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, serial_number_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "uuid", t)) {
-        if (qemu_uuid_parse(buf, qemu_uuid) != 0) {
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "uuid");
+    if (val) {
+        if (qemu_uuid_parse(val, qemu_uuid) != 0) {
             error_report("Invalid UUID");
             exit(1);
         }
     }
-    if (get_param_value(buf, sizeof(buf), "sku", t))
+    val = qemu_opt_get(opts, "sku");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, sku_number_str),
-                         buf, strlen(buf) + 1);
-    if (get_param_value(buf, sizeof(buf), "family", t))
+                         val, strlen(val) + 1);
+    }
+    val = qemu_opt_get(opts, "family");
+    if (val) {
         smbios_add_field(1, offsetof(struct smbios_type_1, family_str),
-                         buf, strlen(buf) + 1);
+                         val, strlen(val) + 1);
+    }
 }
 
-void smbios_entry_add(const char *t)
+void smbios_entry_add(QemuOpts *opts)
 {
-    char buf[1024];
+    Error *local_err = NULL;
+    const char *val;
 
-    if (get_param_value(buf, sizeof(buf), "file", t)) {
+    val = qemu_opt_get(opts, "file");
+    if (val) {
         struct smbios_structure_header *header;
         struct smbios_table *table;
-        int size = get_image_size(buf);
+        int size;
+
+        qemu_opts_validate(opts, qemu_smbios_file_opts, &local_err);
+        if (local_err) {
+            error_report("%s", error_get_pretty(local_err));
+            exit(1);
+        }
 
+        size = get_image_size(val);
         if (size == -1 || size < sizeof(struct smbios_structure_header)) {
-            error_report("Cannot read SMBIOS file %s", buf);
+            error_report("Cannot read SMBIOS file %s", val);
             exit(1);
         }
 
@@ -208,8 +329,8 @@ void smbios_entry_add(const char *t)
         table->header.type = SMBIOS_TABLE_ENTRY;
         table->header.length = cpu_to_le16(sizeof(*table) + size);
 
-        if (load_image(buf, table->data) != size) {
-            error_report("Failed to load SMBIOS file %s", buf);
+        if (load_image(val, table->data) != size) {
+            error_report("Failed to load SMBIOS file %s", val);
             exit(1);
         }
 
@@ -225,14 +346,26 @@ void smbios_entry_add(const char *t)
         return;
     }
 
-    if (get_param_value(buf, sizeof(buf), "type", t)) {
-        unsigned long type = strtoul(buf, NULL, 0);
+    val = qemu_opt_get(opts, "type");
+    if (val) {
+        unsigned long type = strtoul(val, NULL, 0);
+
         switch (type) {
         case 0:
-            smbios_build_type_0_fields(t);
+            qemu_opts_validate(opts, qemu_smbios_type0_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            smbios_build_type_0_fields(opts);
             return;
         case 1:
-            smbios_build_type_1_fields(t);
+            qemu_opts_validate(opts, qemu_smbios_type1_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            smbios_build_type_1_fields(opts);
             return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
diff --git a/vl.c b/vl.c
index 4e709d5..503f903 100644
--- a/vl.c
+++ b/vl.c
@@ -3491,7 +3491,8 @@ int main(int argc, char **argv, char **envp)
                 do_acpitable_option(opts);
                 break;
             case QEMU_OPTION_smbios:
-                do_smbios_option(optarg);
+                opts = qemu_opts_parse(qemu_find_opts("smbios"), optarg, 0);
+                do_smbios_option(opts);
                 break;
             case QEMU_OPTION_enable_kvm:
                 olist = qemu_find_opts("machine");
-- 
MST

[Qemu-devel] [PULL 12/14] smbios: Improve diagnostics for conflicting entries

[Michael S. Tsirkin]

From: Markus Armbruster <armbru@redhat.com>

We allow either tables or fields for the same type.  Makes sense,
because SeaBIOS uses fields only when no tables are present.

We do this by searching the SMBIOS blob for a previously added table
or field.  Error messages look like this:

    qemu-system-x86_64: -smbios type=1,serial=42: SMBIOS type 1 table already defined, cannot add field

User needs to know that "table" is defined by -smbios file=..., and
"field" by -smbios type=...

Instead of searching the blob, record additions of interest, and check
that.  Simpler, and makes better error messages possible:

    qemu-system-x86_64: -smbios file=smbios_type_1.bin: Can't mix file= and type= for same type
    qemu-system-x86_64: -smbios type=1,serial=42,serial=99: This is the conflicting setting

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/smbios.c | 43 +++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index abfd6f7..4263551 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -48,6 +48,12 @@ static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 
+static struct {
+    bool seen;
+    int headertype;
+    Location loc;
+} first_opt[2];
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -159,35 +165,20 @@ uint8_t *smbios_get_table(size_t *length)
  */
 static void smbios_check_collision(int type, int entry)
 {
-    uint16_t *num_entries = (uint16_t *)smbios_entries;
-    struct smbios_header *header;
-    char *p;
-    int i;
-
-    if (!num_entries)
-        return;
-
-    p = (char *)(num_entries + 1);
-
-    for (i = 0; i < *num_entries; i++) {
-        header = (struct smbios_header *)p;
-        if (entry == SMBIOS_TABLE_ENTRY && header->type == SMBIOS_FIELD_ENTRY) {
-            struct smbios_field *field = (void *)header;
-            if (type == field->type) {
-                error_report("SMBIOS type %d field already defined, "
-                             "cannot add table", type);
-                exit(1);
-            }
-        } else if (entry == SMBIOS_FIELD_ENTRY &&
-                   header->type == SMBIOS_TABLE_ENTRY) {
-            struct smbios_structure_header *table = (void *)(header + 1);
-            if (type == table->type) {
-                error_report("SMBIOS type %d table already defined, "
-                             "cannot add field", type);
+    if (type < ARRAY_SIZE(first_opt)) {
+        if (first_opt[type].seen) {
+            if (first_opt[type].headertype != entry) {
+                error_report("Can't mix file= and type= for same type");
+                loc_push_restore(&first_opt[type].loc);
+                error_report("This is the conflicting setting");
+                loc_pop(&first_opt[type].loc);
                 exit(1);
             }
+        } else {
+            first_opt[type].seen = true;
+            first_opt[type].headertype = entry;
+            loc_save(&first_opt[type].loc);
         }
-        p += le16_to_cpu(header->length);
     }
 }
 
-- 
MST

[Qemu-devel] [PULL 13/14] smbios: Make multiple -smbios type= accumulate sanely

[Michael S. Tsirkin]

From: Markus Armbruster <armbru@redhat.com>

Currently, -smbios type=T,NAME=VAL,... adds one field (T,NAME) with
value VAL to fw_cfg for each unique NAME.  If NAME occurs multiple
times, the last one's VAL is used (before the QemuOpts conversion, the
first one was used).

Multiple -smbios can add multiple fields with the same (T, NAME).
SeaBIOS reads all of them from fw_cfg, but uses only the first field
(T, NAME).  The others are ignored.

"First one wins, subsequent ones get ignored silently" isn't nice.  We
commonly let the last option win.  Useful, because it lets you
-readconfig first, then selectively override with command line
options.

Clean up -smbios to work the common way.  Accumulate the settings,
with later ones overwriting earlier ones.  Put the result into fw_cfg
(no more useless duplicates).

Bonus cleanup: qemu_uuid_parse() no longer sets SMBIOS system uuid by
side effect.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/i386/smbios.h |   1 -
 include/sysemu/sysemu.h  |   1 +
 arch_init.c              |   3 -
 hw/i386/smbios.c         | 152 ++++++++++++++++++++++++++++-------------------
 vl.c                     |   2 +
 5 files changed, 94 insertions(+), 65 deletions(-)

diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index d9f43b7..b08ec71 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -16,7 +16,6 @@
 #include "qemu/option.h"
 
 void smbios_entry_add(QemuOpts *opts);
-void smbios_add_field(int type, int offset, const void *data, size_t len);
 uint8_t *smbios_get_table(size_t *length);
 
 /*
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b1aa059..4257736 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -16,6 +16,7 @@ extern const char *bios_name;
 
 extern const char *qemu_name;
 extern uint8_t qemu_uuid[];
+extern bool qemu_uuid_set;
 int qemu_uuid_parse(const char *str, uint8_t *uuid);
 #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
 
diff --git a/arch_init.c b/arch_init.c
index 62f1118..150647b 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1113,9 +1113,6 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid)
     if (ret != 16) {
         return -1;
     }
-#ifdef TARGET_I386
-    smbios_add_field(1, offsetof(struct smbios_type_1, uuid), uuid, 16);
-#endif
     return 0;
 }
 
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 4263551..d2dba6c 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -47,6 +47,7 @@ struct smbios_table {
 static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
+static bool smbios_immutable;
 
 static struct {
     bool seen;
@@ -54,6 +55,17 @@ static struct {
     Location loc;
 } first_opt[2];
 
+static struct {
+    const char *vendor, *version, *date;
+    bool have_major_minor;
+    uint8_t major, minor;
+} type0;
+
+static struct {
+    const char *manufacturer, *product, *version, *serial, *sku, *family;
+    /* uuid is in qemu_uuid[] */
+} type1;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -152,13 +164,6 @@ static void smbios_validate_table(void)
     }
 }
 
-uint8_t *smbios_get_table(size_t *length)
-{
-    smbios_validate_table();
-    *length = smbios_entries_len;
-    return smbios_entries;
-}
-
 /*
  * To avoid unresolvable overlaps in data, don't allow both
  * tables and fields for the same smbios type.
@@ -182,12 +187,10 @@ static void smbios_check_collision(int type, int entry)
     }
 }
 
-void smbios_add_field(int type, int offset, const void *data, size_t len)
+static void smbios_add_field(int type, int offset, const void *data, size_t len)
 {
     struct smbios_field *field;
 
-    smbios_check_collision(type, SMBIOS_FIELD_ENTRY);
-
     if (!smbios_entries) {
         smbios_entries_len = sizeof(uint16_t);
         smbios_entries = g_malloc0(smbios_entries_len);
@@ -207,82 +210,81 @@ void smbios_add_field(int type, int offset, const void *data, size_t len)
             cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
 }
 
-static void smbios_build_type_0_fields(QemuOpts *opts)
+static void smbios_build_type_0_fields(void)
 {
-    const char *val;
-    unsigned char major, minor;
-
-    val = qemu_opt_get(opts, "vendor");
-    if (val) {
+    if (type0.vendor) {
         smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
-                         val, strlen(val) + 1);
+                         type0.vendor, strlen(type0.vendor) + 1);
     }
-    val = qemu_opt_get(opts, "version");
-    if (val) {
+    if (type0.version) {
         smbios_add_field(0, offsetof(struct smbios_type_0, bios_version_str),
-                         val, strlen(val) + 1);
+                         type0.version, strlen(type0.version) + 1);
     }
-    val = qemu_opt_get(opts, "date");
-    if (val) {
+    if (type0.date) {
         smbios_add_field(0, offsetof(struct smbios_type_0,
                                      bios_release_date_str),
-                         val, strlen(val) + 1);
+                         type0.date, strlen(type0.date) + 1);
     }
-    val = qemu_opt_get(opts, "release");
-    if (val) {
-        if (sscanf(val, "%hhu.%hhu", &major, &minor) != 2) {
-            error_report("Invalid release");
-            exit(1);
-        }
+    if (type0.have_major_minor) {
         smbios_add_field(0, offsetof(struct smbios_type_0,
                                      system_bios_major_release),
-                         &major, 1);
+                         &type0.major, 1);
         smbios_add_field(0, offsetof(struct smbios_type_0,
                                      system_bios_minor_release),
-                         &minor, 1);
+                         &type0.minor, 1);
     }
 }
 
-static void smbios_build_type_1_fields(QemuOpts *opts)
+static void smbios_build_type_1_fields(void)
 {
-    const char *val;
-
-    val = qemu_opt_get(opts, "manufacturer");
-    if (val) {
+    if (type1.manufacturer) {
         smbios_add_field(1, offsetof(struct smbios_type_1, manufacturer_str),
-                         val, strlen(val) + 1);
+                         type1.manufacturer, strlen(type1.manufacturer) + 1);
     }
-    val = qemu_opt_get(opts, "product");
-    if (val) {
+    if (type1.product) {
         smbios_add_field(1, offsetof(struct smbios_type_1, product_name_str),
-                         val, strlen(val) + 1);
+                         type1.product, strlen(type1.product) + 1);
     }
-    val = qemu_opt_get(opts, "version");
-    if (val) {
+    if (type1.version) {
         smbios_add_field(1, offsetof(struct smbios_type_1, version_str),
-                         val, strlen(val) + 1);
+                         type1.version, strlen(type1.version) + 1);
     }
-    val = qemu_opt_get(opts, "serial");
-    if (val) {
+    if (type1.serial) {
         smbios_add_field(1, offsetof(struct smbios_type_1, serial_number_str),
-                         val, strlen(val) + 1);
-    }
-    val = qemu_opt_get(opts, "uuid");
-    if (val) {
-        if (qemu_uuid_parse(val, qemu_uuid) != 0) {
-            error_report("Invalid UUID");
-            exit(1);
-        }
+                         type1.serial, strlen(type1.serial) + 1);
     }
-    val = qemu_opt_get(opts, "sku");
-    if (val) {
+    if (type1.sku) {
         smbios_add_field(1, offsetof(struct smbios_type_1, sku_number_str),
-                         val, strlen(val) + 1);
+                         type1.sku, strlen(type1.sku) + 1);
     }
-    val = qemu_opt_get(opts, "family");
-    if (val) {
+    if (type1.family) {
         smbios_add_field(1, offsetof(struct smbios_type_1, family_str),
-                         val, strlen(val) + 1);
+                         type1.family, strlen(type1.family) + 1);
+    }
+    if (qemu_uuid_set) {
+        smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
+                         qemu_uuid, 16);
+    }
+}
+
+uint8_t *smbios_get_table(size_t *length)
+{
+    if (!smbios_immutable) {
+        smbios_build_type_0_fields();
+        smbios_build_type_1_fields();
+        smbios_validate_table();
+        smbios_immutable = true;
+    }
+    *length = smbios_entries_len;
+    return smbios_entries;
+}
+
+static void save_opt(const char **dest, QemuOpts *opts, const char *name)
+{
+    const char *val = qemu_opt_get(opts, name);
+
+    if (val) {
+        *dest = val;
     }
 }
 
@@ -291,6 +293,7 @@ void smbios_entry_add(QemuOpts *opts)
     Error *local_err = NULL;
     const char *val;
 
+    assert(!smbios_immutable);
     val = qemu_opt_get(opts, "file");
     if (val) {
         struct smbios_structure_header *header;
@@ -341,6 +344,8 @@ void smbios_entry_add(QemuOpts *opts)
     if (val) {
         unsigned long type = strtoul(val, NULL, 0);
 
+        smbios_check_collision(type, SMBIOS_FIELD_ENTRY);
+
         switch (type) {
         case 0:
             qemu_opts_validate(opts, qemu_smbios_type0_opts, &local_err);
@@ -348,7 +353,18 @@ void smbios_entry_add(QemuOpts *opts)
                 error_report("%s", error_get_pretty(local_err));
                 exit(1);
             }
-            smbios_build_type_0_fields(opts);
+            save_opt(&type0.vendor, opts, "vendor");
+            save_opt(&type0.version, opts, "version");
+            save_opt(&type0.date, opts, "date");
+
+            val = qemu_opt_get(opts, "release");
+            if (val) {
+                if (sscanf(val, "%hhu.%hhu", &type0.major, &type0.minor) != 2) {
+                    error_report("Invalid release");
+                    exit(1);
+                }
+                type0.have_major_minor = true;
+            }
             return;
         case 1:
             qemu_opts_validate(opts, qemu_smbios_type1_opts, &local_err);
@@ -356,7 +372,21 @@ void smbios_entry_add(QemuOpts *opts)
                 error_report("%s", error_get_pretty(local_err));
                 exit(1);
             }
-            smbios_build_type_1_fields(opts);
+            save_opt(&type1.manufacturer, opts, "manufacturer");
+            save_opt(&type1.product, opts, "product");
+            save_opt(&type1.version, opts, "version");
+            save_opt(&type1.serial, opts, "serial");
+            save_opt(&type1.sku, opts, "sku");
+            save_opt(&type1.family, opts, "family");
+
+            val = qemu_opt_get(opts, "uuid");
+            if (val) {
+                if (qemu_uuid_parse(val, qemu_uuid) != 0) {
+                    error_report("Invalid UUID");
+                    exit(1);
+                }
+                qemu_uuid_set = true;
+            }
             return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
diff --git a/vl.c b/vl.c
index 503f903..fb8006e 100644
--- a/vl.c
+++ b/vl.c
@@ -254,6 +254,7 @@ uint64_t node_mem[MAX_NODES];
 unsigned long *node_cpumask[MAX_NODES];
 
 uint8_t qemu_uuid[16];
+bool qemu_uuid_set;
 
 static QEMUBootSetHandler *boot_set_handler;
 static void *boot_set_opaque;
@@ -3588,6 +3589,7 @@ int main(int argc, char **argv, char **envp)
                             " Wrong format.\n");
                     exit(1);
                 }
+                qemu_uuid_set = true;
                 break;
 	    case QEMU_OPTION_option_rom:
 		if (nb_option_roms >= MAX_OPTION_ROMS) {
-- 
MST

[Qemu-devel] [PULL 14/14] smbios: Factor out smbios_maybe_add_str()

[Michael S. Tsirkin]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/smbios.c | 61 +++++++++++++++++++++++---------------------------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index d2dba6c..d3f1ee6 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -210,21 +210,22 @@ static void smbios_add_field(int type, int offset, const void *data, size_t len)
             cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
 }
 
-static void smbios_build_type_0_fields(void)
+static void smbios_maybe_add_str(int type, int offset, const char *data)
 {
-    if (type0.vendor) {
-        smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
-                         type0.vendor, strlen(type0.vendor) + 1);
-    }
-    if (type0.version) {
-        smbios_add_field(0, offsetof(struct smbios_type_0, bios_version_str),
-                         type0.version, strlen(type0.version) + 1);
+    if (data) {
+        smbios_add_field(type, offset, data, strlen(data) + 1);
     }
-    if (type0.date) {
-        smbios_add_field(0, offsetof(struct smbios_type_0,
+}
+
+static void smbios_build_type_0_fields(void)
+{
+    smbios_maybe_add_str(0, offsetof(struct smbios_type_0, vendor_str),
+                         type0.vendor);
+    smbios_maybe_add_str(0, offsetof(struct smbios_type_0, bios_version_str),
+                         type0.version);
+    smbios_maybe_add_str(0, offsetof(struct smbios_type_0,
                                      bios_release_date_str),
-                         type0.date, strlen(type0.date) + 1);
-    }
+                         type0.date);
     if (type0.have_major_minor) {
         smbios_add_field(0, offsetof(struct smbios_type_0,
                                      system_bios_major_release),
@@ -237,30 +238,18 @@ static void smbios_build_type_0_fields(void)
 
 static void smbios_build_type_1_fields(void)
 {
-    if (type1.manufacturer) {
-        smbios_add_field(1, offsetof(struct smbios_type_1, manufacturer_str),
-                         type1.manufacturer, strlen(type1.manufacturer) + 1);
-    }
-    if (type1.product) {
-        smbios_add_field(1, offsetof(struct smbios_type_1, product_name_str),
-                         type1.product, strlen(type1.product) + 1);
-    }
-    if (type1.version) {
-        smbios_add_field(1, offsetof(struct smbios_type_1, version_str),
-                         type1.version, strlen(type1.version) + 1);
-    }
-    if (type1.serial) {
-        smbios_add_field(1, offsetof(struct smbios_type_1, serial_number_str),
-                         type1.serial, strlen(type1.serial) + 1);
-    }
-    if (type1.sku) {
-        smbios_add_field(1, offsetof(struct smbios_type_1, sku_number_str),
-                         type1.sku, strlen(type1.sku) + 1);
-    }
-    if (type1.family) {
-        smbios_add_field(1, offsetof(struct smbios_type_1, family_str),
-                         type1.family, strlen(type1.family) + 1);
-    }
+    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, manufacturer_str),
+                         type1.manufacturer);
+    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, product_name_str),
+                         type1.product);
+    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, version_str),
+                         type1.version);
+    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, serial_number_str),
+                         type1.serial);
+    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, sku_number_str),
+                         type1.sku);
+    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, family_str),
+                         type1.family);
     if (qemu_uuid_set) {
         smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
                          qemu_uuid, 16);
-- 
MST