[Qemu-devel] [PATCH 2/2 (RFC)] QEMU: SMBIOS: Build full smbios tables

[Gabriel L. Somlo]

   - deprecate passing individual smbios fields (SMBIOS_FIELD_ENTRY)
   - added functionality to build full tables instead, and pass them
     to the bios via fw_cfg as SMBIOS_TABLE_ENTRY blobs
   - added code to optionally build type 2 smbios table expected by OS X

Signed-off-by: Gabriel Somlo <address@hidden>
---

First off, Kevin, Gerd, Laszlo: thanks much for helping me understand
the current code. With your explanations, and after staring at it
patiently for a few hours, it started coming into focus :)

So, currently, QEMU sends tables acquired from binary files via the
command line as SMBIOS_TABLE_ENTRY fw_cfg blobs, and individual fields
via smaller SMBIOS_FIELD_ENTRY blobs. SeaBIOS looks for the whole-table
blobs first, and if none are found, attempts to build the table using
field blobs as default values if available.

With this patch, I'm changing QEMU to only ever send whole-table blobs
(SMBIOS_TABLE_ENTRY) to SeaBIOS. This applies currently to Type 0 and
Type 1 tables, and from what I could test, there's no user-visible change
in functionality.

I've also added code to (optionally) generate a Type 2 table.

Tables are generated only if full binary blobs were NOT given via the
command line. Optional tables are only generated if fields for them are
mentioned on the command line.

Kevin and Gerd: I'd like to get your acked-by for this patch (or any
future revision): you're the two people who've been editing src/fw/smbios.c
in SeaBIOS, and while I didn't directly cut'n'paste anything from there,
I did in fact stare at it very intently, and I'd like to get your OK with
the lgplv3 -> gplv2+ difference in licensing between SeaBIOS and,
respectively, QEMU.

We're still passing individual structure types one per blob, with SeaBIOS
doing the final assembly. In the future we might consider putting it all
together in QEMU, and submitting it as a third blob type, but that would
require coordinating with SeaBIOS, and I'd like to at least figure out how
to build all required table types first.

But even before that, I'd like to get feedback as early as possible, before
I get too attached to any code changes I'm making :)

Thanks much,
  Gabriel

 hw/i386/pc_piix.c        |  12 +-
 hw/i386/pc_q35.c         |   8 +-
 hw/i386/smbios.c         | 312 +++++++++++++++++++++++++++++++----------------
 include/hw/i386/smbios.h |   4 +-
 4 files changed, 220 insertions(+), 116 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d5dc1ef..36b1b9c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,7 +60,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 
 static bool has_pci_info;
 static bool has_acpi_build = true;
-static bool smbios_type1_defaults = true;
+static bool smbios_defaults = true;
 /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
  * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
  * pages in the host.
@@ -143,9 +143,9 @@ static void pc_init1(QEMUMachineInitArgs *args,
     guest_info->has_pci_info = has_pci_info;
     guest_info->isapc_ram_fw = !pci_enabled;
 
-    if (smbios_type1_defaults) {
+    if (smbios_defaults) {
         /* These values are guest ABI, do not change */
-        smbios_set_type1_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
+        smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
                                   args->machine->name);
     }
 
@@ -258,7 +258,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
 
 static void pc_compat_1_7(QEMUMachineInitArgs *args)
 {
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     gigabyte_align = false;
 }
 
@@ -337,7 +337,7 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs 
*args)
 {
     has_pci_info = false;
     has_acpi_build = false;
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
     pc_init1(args, 1, 0);
@@ -347,7 +347,7 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
 {
     has_pci_info = false;
     has_acpi_build = false;
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     if (!args->cpu_model) {
         args->cpu_model = "486";
     }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a7f6260..dfcc252 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -50,7 +50,7 @@
 
 static bool has_pci_info;
 static bool has_acpi_build = true;
-static bool smbios_type1_defaults = true;
+static bool smbios_defaults = true;
 /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
  * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
  * pages in the host.
@@ -130,9 +130,9 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     guest_info->isapc_ram_fw = false;
     guest_info->has_acpi_build = has_acpi_build;
 
-    if (smbios_type1_defaults) {
+    if (smbios_defaults) {
         /* These values are guest ABI, do not change */
-        smbios_set_type1_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
+        smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
                                   args->machine->name);
     }
 
@@ -242,7 +242,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
 static void pc_compat_1_7(QEMUMachineInitArgs *args)
 {
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     gigabyte_align = false;
 }
 
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index e8f41ad..39051b1 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -29,13 +29,6 @@ struct smbios_header {
     uint8_t type;
 } QEMU_PACKED;
 
-struct smbios_field {
-    struct smbios_header header;
-    uint8_t type;
-    uint16_t offset;
-    uint8_t data[];
-} QEMU_PACKED;
-
 struct smbios_table {
     struct smbios_header header;
     uint8_t data[];
@@ -49,11 +42,8 @@ static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
 
-static struct {
-    bool seen;
-    int headertype;
-    Location loc;
-} first_opt[2];
+static DECLARE_BITMAP(have_binfile_bitmap, 128);
+static DECLARE_BITMAP(have_fields_bitmap, 128);
 
 static struct {
     const char *vendor, *version, *date;
@@ -66,6 +56,10 @@ static struct {
     /* uuid is in qemu_uuid[] */
 } type1;
 
+static struct {
+    const char *manufacturer, *product, *version, *serial, *asset, *location;
+} type2;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -149,6 +143,39 @@ static const QemuOptDesc qemu_smbios_type1_opts[] = {
     { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type2_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 = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },{
+        .name = "location",
+        .type = QEMU_OPT_STRING,
+        .help = "location in chassis",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -164,117 +191,169 @@ static void smbios_validate_table(void)
     }
 }
 
-/*
- * To avoid unresolvable overlaps in data, don't allow both
- * tables and fields for the same smbios type.
- */
-static void smbios_check_collision(int type, int entry)
+#define SMBIOS_BUILD_TABLE_PRE(tbl_type, tbl_handle, tbl_required)        \
+    struct smbios_table *t;                                               \
+    struct smbios_type_##tbl_type *p;                                     \
+    int str_index = 0;                                                    \
+    do {                                                                  \
+        /* skip if we got (a) binary file(s) for this type, or      */    \
+        /* if this type is optional and wasn't requested explicitly */    \
+        if (test_bit(tbl_type, have_binfile_bitmap) ||                    \
+            !(tbl_required || test_bit(tbl_type, have_fields_bitmap))) {  \
+            return;                                                       \
+        }                                                                 \
+                                                                          \
+        /* initialize fw_cfg smbios element count */                      \
+        if (!smbios_entries) {                                            \
+            smbios_entries_len = sizeof(uint16_t);                        \
+            smbios_entries = g_malloc0(smbios_entries_len);               \
+        }                                                                 \
+                                                                          \
+        smbios_entries = g_realloc(smbios_entries, smbios_entries_len +   \
+                                                sizeof(*t) + sizeof(*p)); \
+                                                                          \
+        t = (struct smbios_table *)(smbios_entries + smbios_entries_len); \
+        p = (struct smbios_type_##tbl_type *)(t + 1);                     \
+                                                                          \
+        smbios_entries_len += sizeof(*t) + sizeof(*p);                    \
+                                                                          \
+        p->header.type = tbl_type;                                        \
+        p->header.length = sizeof(*p);                                    \
+        p->header.handle = tbl_handle;                                    \
+    } while (0)
+
+#define SMBIOS_TABLE_SET_STR(field, value)                                \
+    do {                                                                  \
+        int len;                                                          \
+        if (value != NULL && (len = strlen(value) + 1) > 1) {             \
+            smbios_entries = g_realloc(smbios_entries,                    \
+                                       smbios_entries_len + len);         \
+            memcpy(smbios_entries + smbios_entries_len, value, len);      \
+            smbios_entries_len += len;                                    \
+            p->field = ++str_index;                                       \
+        } else {                                                          \
+            p->field = 0;                                                 \
+        }                                                                 \
+    } while (0)
+
+#define SMBIOS_BUILD_TABLE_POST                                           \
+    do {                                                                  \
+        /* add empty string if no strings defined in table */             \
+        /* (NOTE: terminating \0 currently handled by fw_cfg/seabios) */  \
+        if (str_index == 0) {                                             \
+            smbios_entries = g_realloc(smbios_entries,                    \
+                                       smbios_entries_len + 1);           \
+            *(smbios_entries + smbios_entries_len) = 0;                   \
+            smbios_entries_len += 1;                                      \
+        }                                                                 \
+                                                                          \
+        t->header.type = SMBIOS_TABLE_ENTRY;                              \
+        t->header.length = smbios_entries_len -                           \
+                           ((uint8_t *)t - smbios_entries);               \
+                                                                          \
+        /* update fw_cfg smbios element count */                          \
+        *(uint16_t *)smbios_entries += 1;                                 \
+    } while (0)
+
+static void smbios_build_type_0_table(void)
 {
-    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);
-        }
-    }
-}
+    SMBIOS_BUILD_TABLE_PRE(0, 0x000, 1); /* required */
 
-static void smbios_add_field(int type, int offset, const void *data, size_t 
len)
-{
-    struct smbios_field *field;
+    SMBIOS_TABLE_SET_STR(vendor_str, type0.vendor);
+    SMBIOS_TABLE_SET_STR(bios_version_str, type0.version);
 
-    if (!smbios_entries) {
-        smbios_entries_len = sizeof(uint16_t);
-        smbios_entries = g_malloc0(smbios_entries_len);
-    }
-    smbios_entries = g_realloc(smbios_entries, smbios_entries_len +
-                                                  sizeof(*field) + len);
-    field = (struct smbios_field *)(smbios_entries + smbios_entries_len);
-    field->header.type = SMBIOS_FIELD_ENTRY;
-    field->header.length = cpu_to_le16(sizeof(*field) + len);
-
-    field->type = type;
-    field->offset = cpu_to_le16(offset);
-    memcpy(field->data, data, len);
-
-    smbios_entries_len += sizeof(*field) + len;
-    (*(uint16_t *)smbios_entries) =
-            cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
-}
+    p->bios_starting_address_segment = 0xE800; /* hardcoded in SeaBIOS */
 
-static void smbios_maybe_add_str(int type, int offset, const char *data)
-{
-    if (data) {
-        smbios_add_field(type, offset, data, strlen(data) + 1);
-    }
-}
+    SMBIOS_TABLE_SET_STR(bios_release_date_str, type0.date);
+
+    p->bios_rom_size = 0; /* hardcoded in SeaBIOS with FIXME comment */
+
+    /* BIOS characteristics not supported */
+    memset(p->bios_characteristics, 0, 8);
+    p->bios_characteristics[0] = 0x08;
+
+    /* Enable targeted content distribution (needed for SVVP, per SeaBIOS) */
+    p->bios_characteristics_extension_bytes[0] = 0;
+    p->bios_characteristics_extension_bytes[1] = 4;
 
-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);
     if (type0.have_major_minor) {
-        smbios_add_field(0, offsetof(struct smbios_type_0,
-                                     system_bios_major_release),
-                         &type0.major, 1);
-        smbios_add_field(0, offsetof(struct smbios_type_0,
-                                     system_bios_minor_release),
-                         &type0.minor, 1);
+        p->system_bios_major_release = type0.major;
+        p->system_bios_minor_release = type0.minor;
+    } else {
+        p->system_bios_major_release = 0;
+        p->system_bios_minor_release = 0;
     }
+
+    /* hardcoded in SeaBIOS */
+    p->embedded_controller_major_release = 0xFF;
+    p->embedded_controller_minor_release = 0xFF;
+
+    SMBIOS_BUILD_TABLE_POST;
 }
 
-static void smbios_build_type_1_fields(void)
+static void smbios_build_type_1_table(void)
 {
-    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);
+    SMBIOS_BUILD_TABLE_PRE(1, 0x100, 1); /* required */
+
+    SMBIOS_TABLE_SET_STR(manufacturer_str, type1.manufacturer);
+    SMBIOS_TABLE_SET_STR(product_name_str, type1.product);
+    SMBIOS_TABLE_SET_STR(version_str, type1.version);
+    SMBIOS_TABLE_SET_STR(serial_number_str, type1.serial);
     if (qemu_uuid_set) {
-        smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
-                         qemu_uuid, 16);
+        memcpy(p->uuid, qemu_uuid, 16);
+    } else {
+        memset(p->uuid, 0, 16);
     }
+    p->wake_up_type = 0x06; /* power switch */
+    SMBIOS_TABLE_SET_STR(sku_number_str, type1.sku);
+    SMBIOS_TABLE_SET_STR(family_str, type1.family);
+
+    SMBIOS_BUILD_TABLE_POST;
 }
 
-void smbios_set_type1_defaults(const char *manufacturer,
-                               const char *product, const char *version)
+static void smbios_build_type_2_table(void)
 {
-    if (!type1.manufacturer) {
-        type1.manufacturer = manufacturer;
-    }
-    if (!type1.product) {
-        type1.product = product;
-    }
-    if (!type1.version) {
-        type1.version = version;
+    SMBIOS_BUILD_TABLE_PRE(2, 0x200, 0); /* optional */
+
+    SMBIOS_TABLE_SET_STR(manufacturer_str, type2.manufacturer);
+    SMBIOS_TABLE_SET_STR(product_str, type2.product);
+    SMBIOS_TABLE_SET_STR(version_str, type2.version);
+    SMBIOS_TABLE_SET_STR(serial_number_str, type2.serial);
+    SMBIOS_TABLE_SET_STR(asset_tag_number_str, type2.asset);
+    p->feature_flags = 0x01; /* Motherboard */
+    SMBIOS_TABLE_SET_STR(location_str, type2.location);
+    p->chassis_handle = 0x300; /* Type 3 (System enclosure) */
+    p->board_type = 0x0A; /* Motherboard */
+    p->contained_element_count = 0;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+#define SMBIOS_SET_DEFAULT(field, value)                                  \
+    if (!field) {                                                         \
+        field = value;                                                    \
     }
+
+void smbios_set_defaults(const char *manufacturer,
+                         const char *product, const char *version)
+{
+    SMBIOS_SET_DEFAULT(type0.vendor, manufacturer);
+    SMBIOS_SET_DEFAULT(type0.version, version);
+    SMBIOS_SET_DEFAULT(type0.date, "01/01/2014");
+    SMBIOS_SET_DEFAULT(type1.manufacturer, manufacturer);
+    SMBIOS_SET_DEFAULT(type1.product, product);
+    SMBIOS_SET_DEFAULT(type1.version, version);
+    SMBIOS_SET_DEFAULT(type2.manufacturer, manufacturer);
+    SMBIOS_SET_DEFAULT(type2.product, product);
+    SMBIOS_SET_DEFAULT(type2.version, version);
 }
 
 uint8_t *smbios_get_table(size_t *length)
 {
     if (!smbios_immutable) {
-        smbios_build_type_0_fields();
-        smbios_build_type_1_fields();
+        smbios_build_type_0_table();
+        smbios_build_type_1_table();
+        smbios_build_type_2_table();
         smbios_validate_table();
         smbios_immutable = true;
     }
@@ -332,7 +411,14 @@ void smbios_entry_add(QemuOpts *opts)
         }
 
         header = (struct smbios_structure_header *)(table->data);
-        smbios_check_collision(header->type, SMBIOS_TABLE_ENTRY);
+
+        if (test_bit(header->type, have_fields_bitmap)) {
+            error_report("Can't add binary type %d table! "
+                         "(fields already specified)", header->type);
+            exit(1);
+        }
+        set_bit(header->type, have_binfile_bitmap);
+
         if (header->type == 4) {
             smbios_type4_count++;
         }
@@ -347,7 +433,12 @@ void smbios_entry_add(QemuOpts *opts)
     if (val) {
         unsigned long type = strtoul(val, NULL, 0);
 
-        smbios_check_collision(type, SMBIOS_FIELD_ENTRY);
+        if (test_bit(type, have_binfile_bitmap)) {
+            error_report("Can't add fields for type %ld table! "
+                         "(binary file already loaded)", type);
+            exit(1);
+        }
+        set_bit(type, have_fields_bitmap);
 
         switch (type) {
         case 0:
@@ -391,6 +482,19 @@ void smbios_entry_add(QemuOpts *opts)
                 qemu_uuid_set = true;
             }
             return;
+        case 2:
+            qemu_opts_validate(opts, qemu_smbios_type2_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type2.manufacturer, opts, "manufacturer");
+            save_opt(&type2.product, opts, "product");
+            save_opt(&type2.version, opts, "version");
+            save_opt(&type2.serial, opts, "serial");
+            save_opt(&type2.asset, opts, "asset");
+            save_opt(&type2.location, opts, "location");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 4e0d9c9..5d8c657 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -16,8 +16,8 @@
 #include "qemu/option.h"
 
 void smbios_entry_add(QemuOpts *opts);
-void smbios_set_type1_defaults(const char *manufacturer,
-                               const char *product, const char *version);
+void smbios_set_defaults(const char *manufacturer,
+                         const char *product, const char *version);
 uint8_t *smbios_get_table(size_t *length);
 
 /*
-- 
1.8.1.4