[QEMU v5 PATCH 00/18] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

Quick highlights:

- after 10/18, we're generating tables 100% consistent with SeaBiOS

- after 14/18, we use e820 to generate type 19 (memory area) tables

- after 18/18, we expect SeaBIOS to check fw_cfg for a full set of
    tables and an entry point (e.g., we need a version of SeaBIOS which
    has applied this patch:

http://www.seabios.org/pipermail/seabios/2014-April/007823.html

Please let me know what you all think.

Thanks,
  Gabriel

Gabriel L. Somlo (18):
  SMBIOS: Rename smbios_set_type1_defaults() for more general use
  SMBIOS: Use macro to set smbios defaults
  SMBIOS: Use bitmaps to check for smbios table collisions
  SMBIOS: Add code to build full smbios tables; build type 2 table
  SMBIOS: Build full tables for types 0 and 1
  SMBIOS: Remove unused code for passing individual fields to bios
  SMBIOS: Build full type 3 table
  SMBIOS: Build full type 4 tables
  SMBIOS: Build full smbios memory tables (type 16, 17, 19, and 20)
  SMBIOS: Build full tables for type 32 and 127
  SMBIOS: Update all table definitions to smbios spec v2.3
  SMBIOS: Remove SeaBIOS compatibility quirks
  SMBIOS: Stop including type 20 tables
  SMBIOS: Use e820 memory map to generate type 19 tables
  SMBIOS: Update type 3 definition to smbios spec v2.7
  SMBIOS: Update type 4 definition to smbios spec v2.6
  SMBIOS: Update memory table types (16, 17, and 19) to smbios spec v2.8
  SMBIOS: Generate complete smbios tables, including entry point

hw/i386/pc.c             |  35 ++-
 hw/i386/pc_piix.c        |  14 +-
 hw/i386/pc_q35.c         |  10 +-
 hw/i386/smbios.c         | 762 +++++++++++++++++++++++++++++++++++++++--------
 include/hw/i386/pc.h     |   2 +
 include/hw/i386/smbios.h |  98 ++++--
 6 files changed, 745 insertions(+), 176 deletions(-)

[QEMU v5 PATCH 01/18] SMBIOS: Rename smbios_set_type1_defaults() for more general use

[Gabriel L. Somlo]

Subsequent patches will utilize this function to set defaults for
more smbios types than just type 1, so the function name should
reflect this.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/pc_piix.c        | 12 ++++++------
 hw/i386/pc_q35.c         |  8 ++++----
 hw/i386/smbios.c         |  4 ++--
 include/hw/i386/smbios.h |  4 ++--
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7930a26..8513de0 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);
     }

@@ -264,7 +264,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;
     option_rom_has_mr = true;
     x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
@@ -345,7 +345,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;
     x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
     enable_compat_apic_id_mode();
     pc_init1(args, 1, 0);
@@ -355,7 +355,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 c844dc2..eacec53 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;
     option_rom_has_mr = true;
     x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index e8f41ad..89dc070 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -256,8 +256,8 @@ static void smbios_build_type_1_fields(void)
     }
 }

-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)
 {
     if (!type1.manufacturer) {
         type1.manufacturer = manufacturer;
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 18fb970..e088aae 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);

/*

[QEMU v5 PATCH 02/18] SMBIOS: Use macro to set smbios defaults

[Gabriel L. Somlo]

The function smbios_set_defaults() uses a repeating code pattern
for each field. This patch replaces that pattern with a macro.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 89dc070..f4ee7b4 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -256,18 +256,17 @@ static void smbios_build_type_1_fields(void)
     }
 }

+#define SMBIOS_SET_DEFAULT(field, value)                                  \
+    if (!field) {                                                         \
+        field = value;                                                    \
+    }
+
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
-    if (!type1.manufacturer) {
-        type1.manufacturer = manufacturer;
-    }
-    if (!type1.product) {
-        type1.product = product;
-    }
-    if (!type1.version) {
-        type1.version = version;
-    }
+    SMBIOS_SET_DEFAULT(type1.manufacturer, manufacturer);
+    SMBIOS_SET_DEFAULT(type1.product, product);
+    SMBIOS_SET_DEFAULT(type1.version, version);
 }

uint8_t *smbios_get_table(size_t *length)

[QEMU v5 PATCH 03/18] SMBIOS: Use bitmaps to check for smbios table collisions

[Gabriel L. Somlo]

Replace existing smbios_check_collision() functionality with
a pair of bitmaps: have_binfile_bitmap and have_fields_bitmap.
Bits corresponding to each smbios type are set by smbios_entry_add(),
which also uses the bitmaps to ensure that binary blobs and field
values are never accepted for the same type.

These bitmaps will also be used in the future to decide whether
or not to build a full table for a given smbios type.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c         | 51 ++++++++++++++++++++----------------------------
 include/hw/i386/smbios.h |  2 ++
 2 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index f4ee7b4..6889332 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -49,11 +49,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, SMBIOS_MAX_TYPE+1);
+static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);

static struct {
     const char *vendor, *version, *date;
@@ -164,29 +161,6 @@ 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)
-{
-    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);
-        }
-    }
-}
-
 static void smbios_add_field(int type, int offset, const void *data, size_t len)
 {
     struct smbios_field *field;
@@ -331,7 +305,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++;
         }
@@ -346,7 +327,17 @@ void smbios_entry_add(QemuOpts *opts)
     if (val) {
         unsigned long type = strtoul(val, NULL, 0);

-        smbios_check_collision(type, SMBIOS_FIELD_ENTRY);
+        if (type > SMBIOS_MAX_TYPE) {
+            error_report("smbios type (%ld) out of range!", type);
+            exit(1);
+        }
+
+        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:
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index e088aae..3425d40 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -15,6 +15,8 @@

#include "qemu/option.h"

+#define SMBIOS_MAX_TYPE 127
+
 void smbios_entry_add(QemuOpts *opts);
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version);

[QEMU v5 PATCH 04/18] SMBIOS: Add code to build full smbios tables; build type 2 table

[Gabriel L. Somlo]

This patch adds a set of macros which build full smbios tables
of a given type, including the logic to decide whether a given
table type should be built or not.

To illustrate this new functionality, we introduce and optionally
build a table of type 2 (base board), which is required by some
versions of OS X (10.7 and 10.8).

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c         | 158 +++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/smbios.h |  18 +++++-
 2 files changed, 175 insertions(+), 1 deletion(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 6889332..06f572d 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -48,6 +48,7 @@ static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
+static bool smbios_have_defaults;

static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -63,6 +64,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),
@@ -146,6 +151,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);
@@ -161,6 +199,90 @@ static void smbios_validate_table(void)
     }
 }

+static bool smbios_skip_table(uint8_t type, bool required_table)
+{
+    if (test_bit(type, have_binfile_bitmap)) {
+        return true; /* user provided their own binary blob(s) */
+    }
+    if (test_bit(type, have_fields_bitmap)) {
+        return false; /* user provided fields via command line */
+    }
+    if (smbios_have_defaults && required_table) {
+        return false; /* we're building tables, and this one's required */
+    }
+    return true;
+}
+
+#define SMBIOS_BUILD_TABLE_PRE(tbl_type, tbl_handle, tbl_required)        \
+    struct smbios_table *w;                                               \
+    struct smbios_type_##tbl_type *t;                                     \
+    size_t w_off, t_off; /* wrapper, table offsets into smbios_entries */ \
+    int str_index = 0;                                                    \
+    do {                                                                  \
+        /* should we skip building this table ? */                        \
+        if (smbios_skip_table(tbl_type, tbl_required)) {                  \
+            return;                                                       \
+        }                                                                 \
+                                                                          \
+        /* initialize fw_cfg smbios element count */                      \
+        if (!smbios_entries) {                                            \
+            smbios_entries_len = sizeof(uint16_t);                        \
+            smbios_entries = g_malloc0(smbios_entries_len);               \
+        }                                                                 \
+                                                                          \
+        /* use offsets of wrapper w and table t within smbios_entries  */ \
+        /* (pointers must be updated after each realloc)               */ \
+        w_off = smbios_entries_len;                                       \
+        t_off = w_off + sizeof(*w);                                       \
+        smbios_entries_len = t_off + sizeof(*t);                          \
+        smbios_entries = g_realloc(smbios_entries, smbios_entries_len);   \
+        w = (struct smbios_table *)(smbios_entries + w_off);              \
+        t = (struct smbios_type_##tbl_type *)(smbios_entries + t_off);    \
+                                                                          \
+        w->header.type = SMBIOS_TABLE_ENTRY;                              \
+        w->header.length = sizeof(*w) + sizeof(*t);                       \
+                                                                          \
+        t->header.type = tbl_type;                                        \
+        t->header.length = sizeof(*t);                                    \
+        t->header.handle = tbl_handle;                                    \
+    } while (0)
+
+#define SMBIOS_TABLE_SET_STR(tbl_type, field, value)                      \
+    do {                                                                  \
+        int len = (value != NULL) ? strlen(value) + 1 : 0;                \
+        if (len > 1) {                                                    \
+            smbios_entries = g_realloc(smbios_entries,                    \
+                                       smbios_entries_len + len);         \
+            memcpy(smbios_entries + smbios_entries_len, value, len);      \
+            smbios_entries_len += len;                                    \
+            /* update pointer(s) post-realloc */                          \
+            w = (struct smbios_table *)(smbios_entries + w_off);          \
+            t = (struct smbios_type_##tbl_type *)(smbios_entries + t_off);\
+            t->field = ++str_index;                                       \
+            w->header.length += len;                                      \
+        } else {                                                          \
+            t->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;                                      \
+            /* update pointer(s) post-realloc */                          \
+            w = (struct smbios_table *)(smbios_entries + w_off);          \
+            w->header.length += 1;                                        \
+        }                                                                 \
+                                                                          \
+        /* update fw_cfg smbios element count */                          \
+        *(uint16_t *)smbios_entries += 1;                                 \
+    } while (0)
+
 static void smbios_add_field(int type, int offset, const void *data, size_t len)
 {
     struct smbios_field *field;
@@ -230,6 +352,24 @@ static void smbios_build_type_1_fields(void)
     }
 }

+static void smbios_build_type_2_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(2, 0x200, false); /* optional */
+
+    SMBIOS_TABLE_SET_STR(2, manufacturer_str, type2.manufacturer);
+    SMBIOS_TABLE_SET_STR(2, product_str, type2.product);
+    SMBIOS_TABLE_SET_STR(2, version_str, type2.version);
+    SMBIOS_TABLE_SET_STR(2, serial_number_str, type2.serial);
+    SMBIOS_TABLE_SET_STR(2, asset_tag_number_str, type2.asset);
+    t->feature_flags = 0x01; /* Motherboard */
+    SMBIOS_TABLE_SET_STR(2, location_str, type2.location);
+    t->chassis_handle = 0x300; /* Type 3 (System enclosure) */
+    t->board_type = 0x0A; /* Motherboard */
+    t->contained_element_count = 0;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -238,14 +378,19 @@ static void smbios_build_type_1_fields(void)
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
+    smbios_have_defaults = true;
     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_2_table();
         smbios_build_type_0_fields();
         smbios_build_type_1_fields();
         smbios_validate_table();
@@ -381,6 +526,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 3425d40..2642e1a 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -62,6 +62,22 @@ struct smbios_type_1 {
     uint8_t family_str;
 } QEMU_PACKED;

+/* SMBIOS type 2 - Base Board */
+struct smbios_type_2 {
+    struct smbios_structure_header header;
+    uint8_t manufacturer_str;
+    uint8_t product_str;
+    uint8_t version_str;
+    uint8_t serial_number_str;
+    uint8_t asset_tag_number_str;
+    uint8_t feature_flags;
+    uint8_t location_str;
+    uint16_t chassis_handle;
+    uint8_t board_type;
+    uint8_t contained_element_count;
+    /* contained elements follow */
+} QEMU_PACKED;
+
 /* SMBIOS type 3 - System Enclosure (v2.3) */
 struct smbios_type_3 {
     struct smbios_structure_header header;
@@ -78,7 +94,7 @@ struct smbios_type_3 {
     uint8_t height;
     uint8_t number_of_power_cords;
     uint8_t contained_element_count;
-    // contained elements follow
+    /* contained elements follow */
 } QEMU_PACKED;

/* SMBIOS type 4 - Processor Information (v2.0) */

[QEMU v5 PATCH 05/18] SMBIOS: Build full tables for types 0 and 1

[Gabriel L. Somlo]

Build full tables for types 0 (bios information) and 1 (system
information). Type 0 is optional, and a table will only be built
if requested via the command line; the default is to leave type 0
tables up to the bios itself.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 06f572d..b00a367 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -352,6 +352,62 @@ static void smbios_build_type_1_fields(void)
     }
 }

+static void smbios_build_type_0_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(0, 0x000, false); /* optional, leave up to BIOS */
+
+    SMBIOS_TABLE_SET_STR(0, vendor_str, type0.vendor);
+    SMBIOS_TABLE_SET_STR(0, bios_version_str, type0.version);
+
+    t->bios_starting_address_segment = 0xE800; /* hardcoded in SeaBIOS */
+
+    SMBIOS_TABLE_SET_STR(0, bios_release_date_str, type0.date);
+
+    t->bios_rom_size = 0; /* hardcoded in SeaBIOS with FIXME comment */
+
+    /* BIOS characteristics not supported */
+    memset(t->bios_characteristics, 0, 8);
+    t->bios_characteristics[0] = 0x08;
+
+    /* Enable targeted content distribution (needed for SVVP, per SeaBIOS) */
+    t->bios_characteristics_extension_bytes[0] = 0;
+    t->bios_characteristics_extension_bytes[1] = 4;
+
+    if (type0.have_major_minor) {
+        t->system_bios_major_release = type0.major;
+        t->system_bios_minor_release = type0.minor;
+    } else {
+        t->system_bios_major_release = 0;
+        t->system_bios_minor_release = 0;
+    }
+
+    /* hardcoded in SeaBIOS */
+    t->embedded_controller_major_release = 0xFF;
+    t->embedded_controller_minor_release = 0xFF;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_1_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(1, 0x100, true); /* required */
+
+    SMBIOS_TABLE_SET_STR(1, manufacturer_str, type1.manufacturer);
+    SMBIOS_TABLE_SET_STR(1, product_name_str, type1.product);
+    SMBIOS_TABLE_SET_STR(1, version_str, type1.version);
+    SMBIOS_TABLE_SET_STR(1, serial_number_str, type1.serial);
+    if (qemu_uuid_set) {
+        memcpy(t->uuid, qemu_uuid, 16);
+    } else {
+        memset(t->uuid, 0, 16);
+    }
+    t->wake_up_type = 0x06; /* power switch */
+    SMBIOS_TABLE_SET_STR(1, sku_number_str, type1.sku);
+    SMBIOS_TABLE_SET_STR(1, family_str, type1.family);
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 static void smbios_build_type_2_table(void)
 {
     SMBIOS_BUILD_TABLE_PRE(2, 0x200, false); /* optional */
@@ -379,6 +435,9 @@ void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
     smbios_have_defaults = true;
+    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);
@@ -390,9 +449,13 @@ void smbios_set_defaults(const char *manufacturer,
 uint8_t *smbios_get_table(size_t *length)
 {
     if (!smbios_immutable) {
+        smbios_build_type_0_table();
+        smbios_build_type_1_table();
         smbios_build_type_2_table();
+if (false) { /* shut up gcc until we remove deprecated code */
         smbios_build_type_0_fields();
         smbios_build_type_1_fields();
+}
         smbios_validate_table();
         smbios_immutable = true;
     }

[QEMU v5 PATCH 06/18] SMBIOS: Remove unused code for passing individual fields to bios

[Gabriel L. Somlo]

This patch removes smbios_add_field() and the old code to insert
individual fields for types 0 and 1 into fw_cfg.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c | 80 --------------------------------------------------------
 1 file changed, 80 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index b00a367..4584774 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[];
@@ -283,75 +276,6 @@ static bool smbios_skip_table(uint8_t type, bool required_table)
         *(uint16_t *)smbios_entries += 1;                                 \
     } while (0)

-static void smbios_add_field(int type, int offset, const void *data, size_t len)
-{
-    struct smbios_field *field;
-
-    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);
-}
-
-static void smbios_maybe_add_str(int type, int offset, const char *data)
-{
-    if (data) {
-        smbios_add_field(type, offset, data, strlen(data) + 1);
-    }
-}
-
-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);
-    }
-}
-
-static void smbios_build_type_1_fields(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);
-    if (qemu_uuid_set) {
-        smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
-                         qemu_uuid, 16);
-    }
-}
-
 static void smbios_build_type_0_table(void)
 {
     SMBIOS_BUILD_TABLE_PRE(0, 0x000, false); /* optional, leave up to BIOS */
@@ -452,10 +376,6 @@ uint8_t *smbios_get_table(size_t *length)
         smbios_build_type_0_table();
         smbios_build_type_1_table();
         smbios_build_type_2_table();
-if (false) { /* shut up gcc until we remove deprecated code */
-        smbios_build_type_0_fields();
-        smbios_build_type_1_fields();
-}
         smbios_validate_table();
         smbios_immutable = true;
     }

[QEMU v5 PATCH 07/18] SMBIOS: Build full type 3 table

[Gabriel L. Somlo]

Build smbios type 3 (system enclosure) table, and make it available
to the bios via fw_cfg. For initial compatibility with SeaBIOS, use
"Bochs" as the default manufacturer string, and leave version unset.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 4584774..47f7b0d 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -61,6 +61,10 @@ static struct {
     const char *manufacturer, *product, *version, *serial, *asset, *location;
 } type2;

+static struct {
+    const char *manufacturer, *version, *serial, *asset;
+} type3;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -177,6 +181,31 @@ static const QemuOptDesc qemu_smbios_type2_opts[] = {
     { /* end of list */ }
 };

+static const QemuOptDesc qemu_smbios_type3_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer 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",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -350,6 +379,27 @@ static void smbios_build_type_2_table(void)
     SMBIOS_BUILD_TABLE_POST;
 }

+static void smbios_build_type_3_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(3, 0x300, true); /* required */
+
+    SMBIOS_TABLE_SET_STR(3, manufacturer_str, type3.manufacturer);
+    t->type = 0x01; /* Other */
+    SMBIOS_TABLE_SET_STR(3, version_str, type3.version);
+    SMBIOS_TABLE_SET_STR(3, serial_number_str, type3.serial);
+    SMBIOS_TABLE_SET_STR(3, asset_tag_number_str, type3.asset);
+    t->boot_up_state = 0x03; /* Safe */
+    t->power_supply_state = 0x03; /* Safe */
+    t->thermal_state = 0x03; /* Safe */
+    t->security_status = 0x02; /* Unknown */
+    t->oem_defined = 0;
+    t->height = 0;
+    t->number_of_power_cords = 0;
+    t->contained_element_count = 0;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -358,6 +408,7 @@ static void smbios_build_type_2_table(void)
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
+    const char *manufacturer_compat = "Bochs"; /* SeaBIOS compatibility */
     smbios_have_defaults = true;
     SMBIOS_SET_DEFAULT(type0.vendor, manufacturer);
     SMBIOS_SET_DEFAULT(type0.version, version);
@@ -368,6 +419,10 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type2.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type2.product, product);
     SMBIOS_SET_DEFAULT(type2.version, version);
+    SMBIOS_SET_DEFAULT(type3.manufacturer, manufacturer_compat);
+    /* not set in SeaBIOS
+    SMBIOS_SET_DEFAULT(type3.version, version);
+    */
 }

uint8_t *smbios_get_table(size_t *length)
@@ -376,6 +431,7 @@ uint8_t *smbios_get_table(size_t *length)
         smbios_build_type_0_table();
         smbios_build_type_1_table();
         smbios_build_type_2_table();
+        smbios_build_type_3_table();
         smbios_validate_table();
         smbios_immutable = true;
     }
@@ -522,6 +578,17 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type2.asset, opts, "asset");
             save_opt(&type2.location, opts, "location");
             return;
+        case 3:
+            qemu_opts_validate(opts, qemu_smbios_type3_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type3.manufacturer, opts, "manufacturer");
+            save_opt(&type3.version, opts, "version");
+            save_opt(&type3.serial, opts, "serial");
+            save_opt(&type3.asset, opts, "asset");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);

[QEMU v5 PATCH 08/18] SMBIOS: Build full type 4 tables

[Gabriel L. Somlo]

Build full smbios type 4 (processor information) tables, and make
them available to the bios via fw_cfg. For initial compatibility
with SeaBIOS, use "Bochs" as the default manufacturer string, and
leave version unset.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/pc.c             |  3 ++
 hw/i386/smbios.c         | 96 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/smbios.h |  1 +
 3 files changed, 100 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 14f0d91..6e3962e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1027,6 +1027,9 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
         sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
                                 APIC_DEFAULT_ADDRESS, 0x1000);
     }
+
+    /* tell smbios about cpuid version and features */
+    smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
 }

/* pci-info ROM file. Little endian format */
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 47f7b0d..5b80021 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -42,6 +42,7 @@ static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
 static bool smbios_have_defaults;
+static uint32_t smbios_cpuid_version, smbios_cpuid_features; /* for type 4 */

static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -65,6 +66,10 @@ static struct {
     const char *manufacturer, *version, *serial, *asset;
 } type3;

+static struct {
+    const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
+} type4;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -206,6 +211,39 @@ static const QemuOptDesc qemu_smbios_type3_opts[] = {
     { /* end of list */ }
 };

+static const QemuOptDesc qemu_smbios_type4_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "sock_pfx",
+        .type = QEMU_OPT_STRING,
+        .help = "socket designation string prefix",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer 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 = "part",
+        .type = QEMU_OPT_STRING,
+        .help = "part number",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -400,11 +438,45 @@ static void smbios_build_type_3_table(void)
     SMBIOS_BUILD_TABLE_POST;
 }

+static void smbios_build_type_4_table(unsigned instance)
+{
+    char sock_str[128];
+
+    SMBIOS_BUILD_TABLE_PRE(4, 0x400 + instance, true); /* required */
+
+    snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
+    SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
+    t->processor_type = 0x03; /* CPU */
+    t->processor_family = 0x01; /* Other */
+    SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
+    t->processor_id[0] = smbios_cpuid_version;
+    t->processor_id[1] = smbios_cpuid_features;
+    SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version);
+    t->voltage = 0;
+    t->external_clock = 0; /* Unknown */
+    t->max_speed = 2000; /* hardcoded in SeaBIOS (use 0/Unknown instead ?) */
+    t->current_speed = 2000; /* hardcoded in SeaBIOS (use 0/Unknown ?) */
+    t->status = 0x41; /* Socket populated, CPU enabled */
+    t->processor_upgrade = 0x01; /* Other */
+    t->l1_cache_handle = 0xFFFF; /* N/A */
+    t->l2_cache_handle = 0xFFFF; /* N/A */
+    t->l3_cache_handle = 0xFFFF; /* N/A */
+
+    SMBIOS_BUILD_TABLE_POST;
+    smbios_type4_count++;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
     }

+void smbios_set_cpuid(uint32_t version, uint32_t features)
+{
+    smbios_cpuid_version = version;
+    smbios_cpuid_features = features;
+}
+
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
@@ -423,15 +495,26 @@ void smbios_set_defaults(const char *manufacturer,
     /* not set in SeaBIOS
     SMBIOS_SET_DEFAULT(type3.version, version);
     */
+    SMBIOS_SET_DEFAULT(type4.sock_pfx, "CPU");
+    SMBIOS_SET_DEFAULT(type4.manufacturer, manufacturer_compat);
+    /* not set in SeaBIOS
+    SMBIOS_SET_DEFAULT(type4.version, version);
+    */
 }

uint8_t *smbios_get_table(size_t *length)
 {
+    unsigned i;
+
     if (!smbios_immutable) {
         smbios_build_type_0_table();
         smbios_build_type_1_table();
         smbios_build_type_2_table();
         smbios_build_type_3_table();
+        for (i = 0; i < smp_cpus; i++) {
+            /* count CPUs starting with 1, to minimize diff vs. SeaBIOS */
+            smbios_build_type_4_table(i + 1);
+        }
         smbios_validate_table();
         smbios_immutable = true;
     }
@@ -589,6 +672,19 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type3.serial, opts, "serial");
             save_opt(&type3.asset, opts, "asset");
             return;
+        case 4:
+            qemu_opts_validate(opts, qemu_smbios_type4_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type4.sock_pfx, opts, "sock_pfx");
+            save_opt(&type4.manufacturer, opts, "manufacturer");
+            save_opt(&type4.version, opts, "version");
+            save_opt(&type4.serial, opts, "serial");
+            save_opt(&type4.asset, opts, "asset");
+            save_opt(&type4.part, opts, "part");
+            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 2642e1a..af5ee01 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -18,6 +18,7 @@
 #define SMBIOS_MAX_TYPE 127

void smbios_entry_add(QemuOpts *opts);
+void smbios_set_cpuid(uint32_t version, uint32_t features);
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version);
 uint8_t *smbios_get_table(size_t *length);

[QEMU v5 PATCH 09/18] SMBIOS: Build full smbios memory tables (type 16, 17, 19, and 20)

[Gabriel L. Somlo]

Build full smbios tables representing the system RAM:
  - type 16 (physical memory array): represents the entire system RAM;
  - type 17 (memory device) tables: one per virtual DIMM;
  - type 19 (memory array mapped address): represent major RAM areas
    (currently one for below-4G memory, and, if applicable, one for
     above-4G memory);
  - type 20 (memory device mapped address): mappings between type 19
    areas and type 17 DIMMs;
These tables will be made available to the bios via fw_cfg.

This patch also thoroughly documents the current memory table layout.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/pc_piix.c        |   3 +-
 hw/i386/pc_q35.c         |   3 +-
 hw/i386/smbios.c         | 256 ++++++++++++++++++++++++++++++++++++++++++++++-
 include/hw/i386/smbios.h |  13 ++-
 4 files changed, 264 insertions(+), 11 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8513de0..db075eb 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -146,7 +146,8 @@ static void pc_init1(QEMUMachineInitArgs *args,
     if (smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
-                                  args->machine->name);
+                             args->machine->name,
+                             below_4g_mem_size, above_4g_mem_size);
     }

/* allocate ram and load rom/bios */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index eacec53..3aaac7a 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -133,7 +133,8 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     if (smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
-                                  args->machine->name);
+                             args->machine->name,
+                             below_4g_mem_size, above_4g_mem_size);
     }

/* allocate ram and load rom/bios */
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 5b80021..6510ff3 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -43,6 +43,7 @@ static int smbios_type4_count = 0;
 static bool smbios_immutable;
 static bool smbios_have_defaults;
 static uint32_t smbios_cpuid_version, smbios_cpuid_features; /* for type 4 */
+static ram_addr_t smbios_below_4g_ram, smbios_above_4g_ram; /* for type 19 */

static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -70,6 +71,10 @@ static struct {
     const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
 } type4;

+static struct {
+    const char *loc_pfx, *bank, *manufacturer, *serial, *asset, *part;
+} type17;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -244,6 +249,39 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
     { /* end of list */ }
 };

+static const QemuOptDesc qemu_smbios_type17_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "loc_pfx",
+        .type = QEMU_OPT_STRING,
+        .help = "device locator string prefix",
+    },{
+        .name = "bank",
+        .type = QEMU_OPT_STRING,
+        .help = "bank locator string",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },{
+        .name = "part",
+        .type = QEMU_OPT_STRING,
+        .help = "part number",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -466,6 +504,117 @@ static void smbios_build_type_4_table(unsigned instance)
     smbios_type4_count++;
 }

+#define ONE_KB ((ram_addr_t)1 << 10)
+#define ONE_MB ((ram_addr_t)1 << 20)
+#define ONE_GB ((ram_addr_t)1 << 30)
+
+static void smbios_build_type_16_table(unsigned dimm_cnt)
+{
+    ram_addr_t  ram_size_kb = ram_size >> 10;
+
+    SMBIOS_BUILD_TABLE_PRE(16, 0x1000, true); /* required */
+
+    t->location = 0x01; /* Other */
+    t->use = 0x03; /* System memory */
+    t->error_correction = 0x06; /* Multi-bit ECC (for Microsoft, per SeaBIOS) */
+    /* if ram_size < 2T, use value in Kilobytes; 0x80000000 == 2T and over */
+    t->maximum_capacity = (ram_size_kb < 0x80000000) ? ram_size_kb : 0x80000000;
+    if (t->maximum_capacity == 0x80000000) {
+        /* TODO: support smbios v2.7 extended capacity */
+        fprintf(stderr, "qemu: warning: SMBIOS v2.7+ required for "
+                        "ram_size >= 2T (%ld)\n", ram_size);
+    }
+    t->memory_error_information_handle = 0xFFFE; /* Not provided */
+    t->number_of_memory_devices = dimm_cnt;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_17_table(unsigned instance, ram_addr_t size)
+{
+    char loc_str[128];
+    ram_addr_t size_mb;
+
+    SMBIOS_BUILD_TABLE_PRE(17, 0x1100 + instance, true); /* required */
+
+    t->physical_memory_array_handle = 0x1000; /* Type 16 (Phys. Mem. Array) */
+    t->memory_error_information_handle = 0; /* SeaBIOS, should be 0xFFFE(N/A) */
+    t->total_width = 64; /* hardcoded in SeaBIOS */
+    t->data_width = 64; /* hardcoded in SeaBIOS */
+    size_mb = QEMU_ALIGN_UP(size, ONE_MB) / ONE_MB;
+    if (size_mb < 0x7FFF) {
+        t->size = size_mb;
+    } else {
+        t->size = 0x7FFF;
+        /* TODO: support smbios v2.7 extended capacity */
+        fprintf(stderr, "qemu: warning: SMBIOS v2.7+ required for "
+                        "DIMM size >= 0x7FFF Mbytes (0x%lx)\n", size_mb);
+    }
+    t->form_factor = 0x09; /* DIMM */
+    t->device_set = 0; /* Not in a set */
+    snprintf(loc_str, sizeof(loc_str), "%s %d", type17.loc_pfx, instance);
+    SMBIOS_TABLE_SET_STR(17, device_locator_str, loc_str);
+    SMBIOS_TABLE_SET_STR(17, bank_locator_str, type17.bank);
+    t->memory_type = 0x07; /* RAM */
+    t->type_detail = 0; /* hardcoded in SeaBIOS */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_19_table(unsigned instance,
+                                       ram_addr_t start, ram_addr_t size)
+{
+    ram_addr_t end, start_kb, end_kb;
+
+    SMBIOS_BUILD_TABLE_PRE(19, 0x1300 + instance, true); /* required */
+
+    end = start + size - 1;
+    assert(end > start);
+    start_kb = start / ONE_KB;
+    end_kb = end / ONE_KB;
+    if (start_kb >= UINT32_MAX || end_kb >= UINT32_MAX) {
+        t->starting_address = t->ending_address = UINT32_MAX;
+        fprintf(stderr, "qemu: warning: SMBIOS v2.7+ required for "
+                        "type19(start=%lx, size=%lx)\n", start, size);
+    } else {
+        t->starting_address = start_kb;
+        t->ending_address = end_kb;
+    }
+    t->memory_array_handle = 0x1000; /* Type 16 (Phys. Mem. Array) */
+    t->partition_width = 1; /* One device per row */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_20_table(unsigned instance,
+                                       unsigned dev_hndl, unsigned array_hndl,
+                                       ram_addr_t start, ram_addr_t size)
+{
+    ram_addr_t end, start_kb, end_kb;
+
+    SMBIOS_BUILD_TABLE_PRE(20, 0x1400 + instance, true); /* required */
+
+    end = start + size - 1;
+    assert(end > start);
+    start_kb = start / ONE_KB;
+    end_kb = end / ONE_KB;
+    if (start_kb >= UINT32_MAX || end_kb >= UINT32_MAX) {
+        t->starting_address = t->ending_address = UINT32_MAX;
+        fprintf(stderr, "qemu: warning: SMBIOS v2.7+ required for "
+                        "type20(start=%lx, size=%lx)\n", start, size);
+    } else {
+        t->starting_address = start_kb;
+        t->ending_address = end_kb;
+    }
+    t->memory_device_handle = 0x1100 + dev_hndl; /* Type 17 (Memory Device) */
+    t->memory_array_mapped_address_handle = 0x1300 + array_hndl; /* Type 19 */
+    t->partition_row_position = 1; /* One device per row, always first pos. */
+    t->interleave_position = 0; /* Not interleaved */
+    t->interleaved_data_depth = 0; /* Not interleaved */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -478,10 +627,17 @@ void smbios_set_cpuid(uint32_t version, uint32_t features)
 }

void smbios_set_defaults(const char *manufacturer,
-                         const char *product, const char *version)
+                         const char *product, const char *version,
+                         ram_addr_t below_4g_mem_size,
+                         ram_addr_t above_4g_mem_size)
 {
     const char *manufacturer_compat = "Bochs"; /* SeaBIOS compatibility */
     smbios_have_defaults = true;
+
+    assert(ram_size == below_4g_mem_size + above_4g_mem_size);
+    smbios_below_4g_ram = below_4g_mem_size;
+    smbios_above_4g_ram = above_4g_mem_size;
+
     SMBIOS_SET_DEFAULT(type0.vendor, manufacturer);
     SMBIOS_SET_DEFAULT(type0.version, version);
     SMBIOS_SET_DEFAULT(type0.date, "01/01/2014");
@@ -500,11 +656,12 @@ void smbios_set_defaults(const char *manufacturer,
     /* not set in SeaBIOS
     SMBIOS_SET_DEFAULT(type4.version, version);
     */
+    SMBIOS_SET_DEFAULT(type17.loc_pfx, "DIMM");
 }

uint8_t *smbios_get_table(size_t *length)
 {
-    unsigned i;
+    unsigned i, dimm_cnt;

if (!smbios_immutable) {
         smbios_build_type_0_table();
@@ -515,6 +672,88 @@ uint8_t *smbios_get_table(size_t *length)
             /* count CPUs starting with 1, to minimize diff vs. SeaBIOS */
             smbios_build_type_4_table(i + 1);
         }
+
+        /* SeaBIOS expects tables compliant to smbios v2.4;
+         * As such, we currently support ram_size up to 2T
+         * (relevant to type 16), and DIMM sizes up to 16G
+         * (for type 17).
+         *
+         * One type 16 (physical memory array) table is created
+         * to represent the entire given ram_size, which is then
+         * split into type 17 (memory device) DMIMMs of 16G, with
+         * the last DIMM covering the sub-16G remainder
+         * (ram_size % 16G).
+         *
+         * Up to two type 19 (memory array mapped address) tables
+         * are created: the first one covers below-4G memory, and
+         * the second, if applicable, covers above-4g memory.
+         *
+         * Tables of type 20 (memory device mapped address) are
+         * created as necessary, to connect type 17 DIMMs to
+         * type 19 memory areas.
+         *
+         * The following figure illustrates how many instances of
+         * each type are generated:
+         *
+         *  -------             -------
+         * |  T17  |           |  T17  |
+         * | <=16G |           | <=16G | ...
+         * | 1100h |<----+     | 1101h |
+         *  -------      |      -------
+         *     ^         |         ^
+         *     |         |         |
+         *  ---+---   ---+---   ---+---
+         * |  T20  | |  T20  | |  T20  |
+         * |  <4G  | |  4G+  | | <=16G | ...
+         * | 1400h | | 1401h | | 1402h |
+         *  ---+---   ---+---   ---+---
+         *     |         |         |
+         *     v         v         v
+         *  -------   -------------------...--
+         * |  T19  | |         T19            |
+         * |  <4G  | |      4G and up         |
+         * | 1300h | |        1301h           |
+         *  -------   -------------------...--
+         *
+         * With under 4G of memory, a single DIMM and a single
+         * below-4G memory area are linked together by a single
+         * type 20 device mapped address.
+         *
+         * With over 4G (but less than 16G) of memory, we still
+         * require only one DIMM, but create two memory areas,
+         * one representing the below_4g_ram, and the other one
+         * for above_4g_ram. Two type 20 device mapped address
+         * tables link our DIMM to the below_4g and above_4g
+         * areas, respectively.
+         *
+         * With over 16G of memory, we create additional DIMMs, and
+         * additional type 20 device mapped address tables to link
+         * each such additional DIMM to the above_4g_ram area.
+         */
+
+#define MAX_DIMM_SZ (16 * ONE_GB)
+#define GET_DIMM_SZ ((i < dimm_cnt - 1) ? MAX_DIMM_SZ : ram_size % MAX_DIMM_SZ)
+
+        dimm_cnt = QEMU_ALIGN_UP(ram_size, MAX_DIMM_SZ) / MAX_DIMM_SZ;
+        smbios_build_type_16_table(dimm_cnt);
+        for (i = 0; i < dimm_cnt; i++) {
+            smbios_build_type_17_table(i, GET_DIMM_SZ);
+        }
+        smbios_build_type_19_table(0, 0, smbios_below_4g_ram);
+        smbios_build_type_20_table(0, 0, 0, 0, smbios_below_4g_ram);
+        if (smbios_above_4g_ram) {
+            ram_addr_t start = 4 * ONE_GB, size;
+            smbios_build_type_19_table(1, start, smbios_above_4g_ram);
+            for (i = 0; i < dimm_cnt; i++) {
+                size = GET_DIMM_SZ;
+                if (i == 0) { /* below-4G portion of DIMM 0 already mapped */
+                    size -= smbios_below_4g_ram;
+                }
+                smbios_build_type_20_table(i + 1, i, 1, start, size);
+                start += size;
+            }
+        }
+
         smbios_validate_table();
         smbios_immutable = true;
     }
@@ -685,6 +924,19 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type4.asset, opts, "asset");
             save_opt(&type4.part, opts, "part");
             return;
+        case 17:
+            qemu_opts_validate(opts, qemu_smbios_type17_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type17.loc_pfx, opts, "loc_pfx");
+            save_opt(&type17.bank, opts, "bank");
+            save_opt(&type17.manufacturer, opts, "manufacturer");
+            save_opt(&type17.serial, opts, "serial");
+            save_opt(&type17.asset, opts, "asset");
+            save_opt(&type17.part, opts, "part");
+            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 af5ee01..2a0d384 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -20,7 +20,9 @@
 void smbios_entry_add(QemuOpts *opts);
 void smbios_set_cpuid(uint32_t version, uint32_t features);
 void smbios_set_defaults(const char *manufacturer,
-                         const char *product, const char *version);
+                         const char *product, const char *version,
+                         ram_addr_t below_4g_mem_size,
+                         ram_addr_t above_4g_mem_size);
 uint8_t *smbios_get_table(size_t *length);

/*
@@ -118,9 +120,7 @@ struct smbios_type_4 {
     uint16_t l3_cache_handle;
 } QEMU_PACKED;

-/* SMBIOS type 16 - Physical Memory Array
- *   Associated with one type 17 (Memory Device).
- */
+/* SMBIOS type 16 - Physical Memory Array */
 struct smbios_type_16 {
     struct smbios_structure_header header;
     uint8_t location;
@@ -130,9 +130,8 @@ struct smbios_type_16 {
     uint16_t memory_error_information_handle;
     uint16_t number_of_memory_devices;
 } QEMU_PACKED;
-/* SMBIOS type 17 - Memory Device
- *   Associated with one type 19
- */
+
+/* SMBIOS type 17 - Memory Device */
 struct smbios_type_17 {
     struct smbios_structure_header header;
     uint16_t physical_memory_array_handle;

[QEMU v5 PATCH 10/18] SMBIOS: Build full tables for type 32 and 127

[Gabriel L. Somlo]

Build full smbios type 32 (system boot info) and 127 (end-of-table)
tables, and make them available via fw_cfg.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 6510ff3..b1f1d46 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -615,6 +615,22 @@ static void smbios_build_type_20_table(unsigned instance,
     SMBIOS_BUILD_TABLE_POST;
 }

+static void smbios_build_type_32_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(32, 0x2000, true); /* required */
+
+    memset(t->reserved, 0, 6);
+    t->boot_status = 0; /* No errors detected */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_127_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(127, 0x7F00, true); /* required */
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -754,6 +770,9 @@ uint8_t *smbios_get_table(size_t *length)
             }
         }

+        smbios_build_type_32_table();
+        smbios_build_type_127_table();
+
         smbios_validate_table();
         smbios_immutable = true;
     }

[QEMU v5 PATCH 11/18] SMBIOS: Update all table definitions to smbios spec v2.3

[Gabriel L. Somlo]

Table definitions for types 4 and 17 are only up to v2.0,
so add fields specified in smbios v2.3, as expected (and
advertised) by the SeaBIOS smbios entry point structure.

In particular, OS X guests insist on type 17 being v2.3
compliant, to avoid GUI crashes when "about this mac" is
chosen in the Finder menu.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c         |  9 +++++++++
 include/hw/i386/smbios.h | 13 +++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index b1f1d46..f7a9a92 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -499,6 +499,9 @@ static void smbios_build_type_4_table(unsigned instance)
     t->l1_cache_handle = 0xFFFF; /* N/A */
     t->l2_cache_handle = 0xFFFF; /* N/A */
     t->l3_cache_handle = 0xFFFF; /* N/A */
+    SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
+    SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
+    SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);

SMBIOS_BUILD_TABLE_POST;
     smbios_type4_count++;
@@ -557,6 +560,11 @@ static void smbios_build_type_17_table(unsigned instance, ram_addr_t size)
     SMBIOS_TABLE_SET_STR(17, bank_locator_str, type17.bank);
     t->memory_type = 0x07; /* RAM */
     t->type_detail = 0; /* hardcoded in SeaBIOS */
+    t->speed = 0; /* Unknown */
+    SMBIOS_TABLE_SET_STR(17, manufacturer_str, type17.manufacturer);
+    SMBIOS_TABLE_SET_STR(17, serial_number_str, type17.serial);
+    SMBIOS_TABLE_SET_STR(17, asset_tag_number_str, type17.asset);
+    SMBIOS_TABLE_SET_STR(17, part_number_str, type17.part);

SMBIOS_BUILD_TABLE_POST;
 }
@@ -673,6 +681,7 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type4.version, version);
     */
     SMBIOS_SET_DEFAULT(type17.loc_pfx, "DIMM");
+    SMBIOS_SET_DEFAULT(type17.manufacturer, manufacturer);
 }

uint8_t *smbios_get_table(size_t *length)
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 2a0d384..6bde151 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -100,7 +100,7 @@ struct smbios_type_3 {
     /* contained elements follow */
 } QEMU_PACKED;

-/* SMBIOS type 4 - Processor Information (v2.0) */
+/* SMBIOS type 4 - Processor Information (v2.3) */
 struct smbios_type_4 {
     struct smbios_structure_header header;
     uint8_t socket_designation_str;
@@ -118,6 +118,10 @@ struct smbios_type_4 {
     uint16_t l1_cache_handle;
     uint16_t l2_cache_handle;
     uint16_t l3_cache_handle;
+    uint8_t serial_number_str;
+    uint8_t asset_tag_number_str;
+    uint8_t part_number_str;
+
 } QEMU_PACKED;

/* SMBIOS type 16 - Physical Memory Array */
@@ -131,7 +135,7 @@ struct smbios_type_16 {
     uint16_t number_of_memory_devices;
 } QEMU_PACKED;

-/* SMBIOS type 17 - Memory Device */
+/* SMBIOS type 17 - Memory Device (v2.3) */
 struct smbios_type_17 {
     struct smbios_structure_header header;
     uint16_t physical_memory_array_handle;
@@ -145,6 +149,11 @@ struct smbios_type_17 {
     uint8_t bank_locator_str;
     uint8_t memory_type;
     uint16_t type_detail;
+    uint16_t speed;
+    uint8_t manufacturer_str;
+    uint8_t serial_number_str;
+    uint8_t asset_tag_number_str;
+    uint8_t part_number_str;
 } QEMU_PACKED;

/* SMBIOS type 19 - Memory Array Mapped Address */

[QEMU v5 PATCH 12/18] SMBIOS: Remove SeaBIOS compatibility quirks

[Gabriel L. Somlo]

- Replace some arbitrarily hardcoded fields with proper
    "n/a" or "unknown" values;
  - Use QEMU-supplied default manufacturer and version strings;
  - Count CPUs starting with 0 instead of 1, to maintain uniformity
    with other multiple-instance items.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index f7a9a92..999c400 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -492,8 +492,8 @@ static void smbios_build_type_4_table(unsigned instance)
     SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version);
     t->voltage = 0;
     t->external_clock = 0; /* Unknown */
-    t->max_speed = 2000; /* hardcoded in SeaBIOS (use 0/Unknown instead ?) */
-    t->current_speed = 2000; /* hardcoded in SeaBIOS (use 0/Unknown ?) */
+    t->max_speed = 0; /* Unknown */
+    t->current_speed = 0; /* Unknown */
     t->status = 0x41; /* Socket populated, CPU enabled */
     t->processor_upgrade = 0x01; /* Other */
     t->l1_cache_handle = 0xFFFF; /* N/A */
@@ -541,9 +541,9 @@ static void smbios_build_type_17_table(unsigned instance, ram_addr_t size)
     SMBIOS_BUILD_TABLE_PRE(17, 0x1100 + instance, true); /* required */

t->physical_memory_array_handle = 0x1000; /* Type 16 (Phys. Mem. Array) */
-    t->memory_error_information_handle = 0; /* SeaBIOS, should be 0xFFFE(N/A) */
-    t->total_width = 64; /* hardcoded in SeaBIOS */
-    t->data_width = 64; /* hardcoded in SeaBIOS */
+    t->memory_error_information_handle = 0xFFFE; /* Not provided */
+    t->total_width = 0xFFFF; /* Unknown */
+    t->data_width = 0xFFFF; /* Unknown */
     size_mb = QEMU_ALIGN_UP(size, ONE_MB) / ONE_MB;
     if (size_mb < 0x7FFF) {
         t->size = size_mb;
@@ -559,7 +559,7 @@ static void smbios_build_type_17_table(unsigned instance, ram_addr_t size)
     SMBIOS_TABLE_SET_STR(17, device_locator_str, loc_str);
     SMBIOS_TABLE_SET_STR(17, bank_locator_str, type17.bank);
     t->memory_type = 0x07; /* RAM */
-    t->type_detail = 0; /* hardcoded in SeaBIOS */
+    t->type_detail = 0x02; /* Other */
     t->speed = 0; /* Unknown */
     SMBIOS_TABLE_SET_STR(17, manufacturer_str, type17.manufacturer);
     SMBIOS_TABLE_SET_STR(17, serial_number_str, type17.serial);
@@ -655,7 +655,6 @@ void smbios_set_defaults(const char *manufacturer,
                          ram_addr_t below_4g_mem_size,
                          ram_addr_t above_4g_mem_size)
 {
-    const char *manufacturer_compat = "Bochs"; /* SeaBIOS compatibility */
     smbios_have_defaults = true;

assert(ram_size == below_4g_mem_size + above_4g_mem_size);
@@ -671,15 +670,11 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type2.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type2.product, product);
     SMBIOS_SET_DEFAULT(type2.version, version);
-    SMBIOS_SET_DEFAULT(type3.manufacturer, manufacturer_compat);
-    /* not set in SeaBIOS
+    SMBIOS_SET_DEFAULT(type3.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type3.version, version);
-    */
     SMBIOS_SET_DEFAULT(type4.sock_pfx, "CPU");
-    SMBIOS_SET_DEFAULT(type4.manufacturer, manufacturer_compat);
-    /* not set in SeaBIOS
+    SMBIOS_SET_DEFAULT(type4.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type4.version, version);
-    */
     SMBIOS_SET_DEFAULT(type17.loc_pfx, "DIMM");
     SMBIOS_SET_DEFAULT(type17.manufacturer, manufacturer);
 }
@@ -694,8 +689,7 @@ uint8_t *smbios_get_table(size_t *length)
         smbios_build_type_2_table();
         smbios_build_type_3_table();
         for (i = 0; i < smp_cpus; i++) {
-            /* count CPUs starting with 1, to minimize diff vs. SeaBIOS */
-            smbios_build_type_4_table(i + 1);
+            smbios_build_type_4_table(i);
         }

/* SeaBIOS expects tables compliant to smbios v2.4;

[QEMU v5 PATCH 13/18] SMBIOS: Stop including type 20 tables

[Gabriel L. Somlo]

Table type 20 (memory device mapped address) is no longer required
as of smbios spec v2.5. Leaving it out completely saves us from having
to figure out how to connect type 17 devices to type 19 memory areas.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c         | 99 +-----------------------------------------------
 include/hw/i386/smbios.h | 12 ------
 2 files changed, 1 insertion(+), 110 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 999c400..04a7f36 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -594,35 +594,6 @@ static void smbios_build_type_19_table(unsigned instance,
     SMBIOS_BUILD_TABLE_POST;
 }

-static void smbios_build_type_20_table(unsigned instance,
-                                       unsigned dev_hndl, unsigned array_hndl,
-                                       ram_addr_t start, ram_addr_t size)
-{
-    ram_addr_t end, start_kb, end_kb;
-
-    SMBIOS_BUILD_TABLE_PRE(20, 0x1400 + instance, true); /* required */
-
-    end = start + size - 1;
-    assert(end > start);
-    start_kb = start / ONE_KB;
-    end_kb = end / ONE_KB;
-    if (start_kb >= UINT32_MAX || end_kb >= UINT32_MAX) {
-        t->starting_address = t->ending_address = UINT32_MAX;
-        fprintf(stderr, "qemu: warning: SMBIOS v2.7+ required for "
-                        "type20(start=%lx, size=%lx)\n", start, size);
-    } else {
-        t->starting_address = start_kb;
-        t->ending_address = end_kb;
-    }
-    t->memory_device_handle = 0x1100 + dev_hndl; /* Type 17 (Memory Device) */
-    t->memory_array_mapped_address_handle = 0x1300 + array_hndl; /* Type 19 */
-    t->partition_row_position = 1; /* One device per row, always first pos. */
-    t->interleave_position = 0; /* Not interleaved */
-    t->interleaved_data_depth = 0; /* Not interleaved */
-
-    SMBIOS_BUILD_TABLE_POST;
-}
-
 static void smbios_build_type_32_table(void)
 {
     SMBIOS_BUILD_TABLE_PRE(32, 0x2000, true); /* required */
@@ -692,64 +663,6 @@ uint8_t *smbios_get_table(size_t *length)
             smbios_build_type_4_table(i);
         }

-        /* SeaBIOS expects tables compliant to smbios v2.4;
-         * As such, we currently support ram_size up to 2T
-         * (relevant to type 16), and DIMM sizes up to 16G
-         * (for type 17).
-         *
-         * One type 16 (physical memory array) table is created
-         * to represent the entire given ram_size, which is then
-         * split into type 17 (memory device) DMIMMs of 16G, with
-         * the last DIMM covering the sub-16G remainder
-         * (ram_size % 16G).
-         *
-         * Up to two type 19 (memory array mapped address) tables
-         * are created: the first one covers below-4G memory, and
-         * the second, if applicable, covers above-4g memory.
-         *
-         * Tables of type 20 (memory device mapped address) are
-         * created as necessary, to connect type 17 DIMMs to
-         * type 19 memory areas.
-         *
-         * The following figure illustrates how many instances of
-         * each type are generated:
-         *
-         *  -------             -------
-         * |  T17  |           |  T17  |
-         * | <=16G |           | <=16G | ...
-         * | 1100h |<----+     | 1101h |
-         *  -------      |      -------
-         *     ^         |         ^
-         *     |         |         |
-         *  ---+---   ---+---   ---+---
-         * |  T20  | |  T20  | |  T20  |
-         * |  <4G  | |  4G+  | | <=16G | ...
-         * | 1400h | | 1401h | | 1402h |
-         *  ---+---   ---+---   ---+---
-         *     |         |         |
-         *     v         v         v
-         *  -------   -------------------...--
-         * |  T19  | |         T19            |
-         * |  <4G  | |      4G and up         |
-         * | 1300h | |        1301h           |
-         *  -------   -------------------...--
-         *
-         * With under 4G of memory, a single DIMM and a single
-         * below-4G memory area are linked together by a single
-         * type 20 device mapped address.
-         *
-         * With over 4G (but less than 16G) of memory, we still
-         * require only one DIMM, but create two memory areas,
-         * one representing the below_4g_ram, and the other one
-         * for above_4g_ram. Two type 20 device mapped address
-         * tables link our DIMM to the below_4g and above_4g
-         * areas, respectively.
-         *
-         * With over 16G of memory, we create additional DIMMs, and
-         * additional type 20 device mapped address tables to link
-         * each such additional DIMM to the above_4g_ram area.
-         */
-
 #define MAX_DIMM_SZ (16 * ONE_GB)
 #define GET_DIMM_SZ ((i < dimm_cnt - 1) ? MAX_DIMM_SZ : ram_size % MAX_DIMM_SZ)

@@ -759,18 +672,8 @@ uint8_t *smbios_get_table(size_t *length)
             smbios_build_type_17_table(i, GET_DIMM_SZ);
         }
         smbios_build_type_19_table(0, 0, smbios_below_4g_ram);
-        smbios_build_type_20_table(0, 0, 0, 0, smbios_below_4g_ram);
         if (smbios_above_4g_ram) {
-            ram_addr_t start = 4 * ONE_GB, size;
-            smbios_build_type_19_table(1, start, smbios_above_4g_ram);
-            for (i = 0; i < dimm_cnt; i++) {
-                size = GET_DIMM_SZ;
-                if (i == 0) { /* below-4G portion of DIMM 0 already mapped */
-                    size -= smbios_below_4g_ram;
-                }
-                smbios_build_type_20_table(i + 1, i, 1, start, size);
-                start += size;
-            }
+            smbios_build_type_19_table(1, 4 * ONE_GB, smbios_above_4g_ram);
         }

smbios_build_type_32_table();
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 6bde151..6e59ed0 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -165,18 +165,6 @@ struct smbios_type_19 {
     uint8_t partition_width;
 } QEMU_PACKED;

-/* SMBIOS type 20 - Memory Device Mapped Address */
-struct smbios_type_20 {
-    struct smbios_structure_header header;
-    uint32_t starting_address;
-    uint32_t ending_address;
-    uint16_t memory_device_handle;
-    uint16_t memory_array_mapped_address_handle;
-    uint8_t partition_row_position;
-    uint8_t interleave_position;
-    uint8_t interleaved_data_depth;
-} QEMU_PACKED;
-
 /* SMBIOS type 32 - System Boot Information */
 struct smbios_type_32 {
     struct smbios_structure_header header;

[QEMU v5 PATCH 14/18] SMBIOS: Use e820 memory map to generate type 19 tables

[Gabriel L. Somlo]

Build type 19 (memory array mapped address, a.k.a. memory area)
tables by scanning the e820 map for E820_RAM entries. Since this
supercedes below_4g and above_4g ram amounts, we no longer need
them as arguments to smbios_set_defaults().

Signed-off-by: Gabriel Somlo 
---
 hw/i386/pc.c             | 15 +++++++++++++++
 hw/i386/pc_piix.c        |  3 +--
 hw/i386/pc_q35.c         |  3 +--
 hw/i386/smbios.c         | 23 +++++++++++------------
 include/hw/i386/pc.h     |  2 ++
 include/hw/i386/smbios.h |  4 +---
 6 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6e3962e..ff353cf 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -612,6 +612,21 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
     return e820_entries;
 }

+int e820_get_num_entries(void)
+{
+    return e820_entries;
+}
+
+bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t *length)
+{
+    if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
+        *address = le64_to_cpu(e820_table[idx].address);
+        *length = le64_to_cpu(e820_table[idx].length);
+        return true;
+    }
+    return false;
+}
+
 /* Calculates the limit to CPU APIC ID values
  *
  * This function returns the limit for the APIC ID value, so that all
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index db075eb..ba854eb 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -146,8 +146,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
     if (smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
-                             args->machine->name,
-                             below_4g_mem_size, above_4g_mem_size);
+                             args->machine->name);
     }

/* allocate ram and load rom/bios */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 3aaac7a..6d503ac 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -133,8 +133,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     if (smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
-                             args->machine->name,
-                             below_4g_mem_size, above_4g_mem_size);
+                             args->machine->name);
     }

/* allocate ram and load rom/bios */
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 04a7f36..b98d7ba 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -18,6 +18,7 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
+#include "hw/i386/pc.h"
 #include "hw/i386/smbios.h"
 #include "hw/loader.h"

@@ -43,7 +44,6 @@ static int smbios_type4_count = 0;
 static bool smbios_immutable;
 static bool smbios_have_defaults;
 static uint32_t smbios_cpuid_version, smbios_cpuid_features; /* for type 4 */
-static ram_addr_t smbios_below_4g_ram, smbios_above_4g_ram; /* for type 19 */

static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -622,16 +622,10 @@ void smbios_set_cpuid(uint32_t version, uint32_t features)
 }

void smbios_set_defaults(const char *manufacturer,
-                         const char *product, const char *version,
-                         ram_addr_t below_4g_mem_size,
-                         ram_addr_t above_4g_mem_size)
+                         const char *product, const char *version)
 {
     smbios_have_defaults = true;

-    assert(ram_size == below_4g_mem_size + above_4g_mem_size);
-    smbios_below_4g_ram = below_4g_mem_size;
-    smbios_above_4g_ram = above_4g_mem_size;
-
     SMBIOS_SET_DEFAULT(type0.vendor, manufacturer);
     SMBIOS_SET_DEFAULT(type0.version, version);
     SMBIOS_SET_DEFAULT(type0.date, "01/01/2014");
@@ -652,7 +646,7 @@ void smbios_set_defaults(const char *manufacturer,

uint8_t *smbios_get_table(size_t *length)
 {
-    unsigned i, dimm_cnt;
+    unsigned i, dimm_cnt, instance;

if (!smbios_immutable) {
         smbios_build_type_0_table();
@@ -667,13 +661,18 @@ uint8_t *smbios_get_table(size_t *length)
 #define GET_DIMM_SZ ((i < dimm_cnt - 1) ? MAX_DIMM_SZ : ram_size % MAX_DIMM_SZ)

dimm_cnt = QEMU_ALIGN_UP(ram_size, MAX_DIMM_SZ) / MAX_DIMM_SZ;
+
         smbios_build_type_16_table(dimm_cnt);
+
         for (i = 0; i < dimm_cnt; i++) {
             smbios_build_type_17_table(i, GET_DIMM_SZ);
         }
-        smbios_build_type_19_table(0, 0, smbios_below_4g_ram);
-        if (smbios_above_4g_ram) {
-            smbios_build_type_19_table(1, 4 * ONE_GB, smbios_above_4g_ram);
+
+        for (i = 0, instance = 0; i < e820_get_num_entries(); i++) {
+            ram_addr_t address, length;
+            if (e820_get_entry(i, E820_RAM, &address, &length)) {
+                smbios_build_type_19_table(instance++, address, length);
+            }
         }

smbios_build_type_32_table();
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9010246..9f26e14 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -239,6 +239,8 @@ uint16_t pvpanic_port(void);
 #define E820_UNUSABLE   5

int e820_add_entry(uint64_t, uint64_t, uint32_t);
+int e820_get_num_entries(void);
+bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);

#define PC_Q35_COMPAT_1_7 \
         PC_COMPAT_1_7, \
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 6e59ed0..80e91b4 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -20,9 +20,7 @@
 void smbios_entry_add(QemuOpts *opts);
 void smbios_set_cpuid(uint32_t version, uint32_t features);
 void smbios_set_defaults(const char *manufacturer,
-                         const char *product, const char *version,
-                         ram_addr_t below_4g_mem_size,
-                         ram_addr_t above_4g_mem_size);
+                         const char *product, const char *version);
 uint8_t *smbios_get_table(size_t *length);

/*

[QEMU v5 PATCH 15/18] SMBIOS: Update type 3 definition to smbios spec v2.7

[Gabriel L. Somlo]

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c         | 8 +++++++-
 include/hw/i386/smbios.h | 3 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index b98d7ba..f5507cb 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -64,7 +64,7 @@ static struct {
 } type2;

static struct {
-    const char *manufacturer, *version, *serial, *asset;
+    const char *manufacturer, *version, *serial, *asset, *sku;
 } type3;

static struct {
@@ -212,6 +212,10 @@ static const QemuOptDesc qemu_smbios_type3_opts[] = {
         .name = "asset",
         .type = QEMU_OPT_STRING,
         .help = "asset tag number",
+    },{
+        .name = "sku",
+        .type = QEMU_OPT_STRING,
+        .help = "SKU number",
     },
     { /* end of list */ }
 };
@@ -472,6 +476,7 @@ static void smbios_build_type_3_table(void)
     t->height = 0;
     t->number_of_power_cords = 0;
     t->contained_element_count = 0;
+    SMBIOS_TABLE_SET_STR(3, sku_number_str, type3.sku);

SMBIOS_BUILD_TABLE_POST;
 }
@@ -834,6 +839,7 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type3.version, opts, "version");
             save_opt(&type3.serial, opts, "serial");
             save_opt(&type3.asset, opts, "asset");
+            save_opt(&type3.sku, opts, "sku");
             return;
         case 4:
             qemu_opts_validate(opts, qemu_smbios_type4_opts, &local_err);
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 80e91b4..ce2f0f2 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -79,7 +79,7 @@ struct smbios_type_2 {
     /* contained elements follow */
 } QEMU_PACKED;

-/* SMBIOS type 3 - System Enclosure (v2.3) */
+/* SMBIOS type 3 - System Enclosure (v2.7) */
 struct smbios_type_3 {
     struct smbios_structure_header header;
     uint8_t manufacturer_str;
@@ -95,6 +95,7 @@ struct smbios_type_3 {
     uint8_t height;
     uint8_t number_of_power_cords;
     uint8_t contained_element_count;
+    uint8_t sku_number_str;
     /* contained elements follow */
 } QEMU_PACKED;

[QEMU v5 PATCH 16/18] SMBIOS: Update type 4 definition to smbios spec v2.6

[Gabriel L. Somlo]

With this update, we generate one type 4 (processor information) table
per socket, calculated as "smp_cpus / (smp_cores * smp_threads)", which
is in line with what happens on modern hardware.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c         | 19 ++++++++++++++-----
 include/hw/i386/smbios.h |  8 ++++++--
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index f5507cb..382b75d 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -18,6 +18,7 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/cpus.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/smbios.h"
 #include "hw/loader.h"
@@ -43,7 +44,7 @@ static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
 static bool smbios_have_defaults;
-static uint32_t smbios_cpuid_version, smbios_cpuid_features; /* for type 4 */
+static uint32_t smbios_cpuid_version, smbios_cpuid_features, smbios_smp_sockets;

static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -295,8 +296,9 @@ machine_init(smbios_register_config);

static void smbios_validate_table(void)
 {
-    if (smbios_type4_count && smbios_type4_count != smp_cpus) {
-        error_report("Number of SMBIOS Type 4 tables must match cpu count");
+    if (smbios_type4_count && smbios_type4_count != smbios_smp_sockets) {
+        error_report("Number of SMBIOS Type 4 tables "
+                     "must match socket count (%d)", smbios_smp_sockets);
         exit(1);
     }
 }
@@ -490,7 +492,6 @@ static void smbios_build_type_4_table(unsigned instance)
     snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
     SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
     t->processor_type = 0x03; /* CPU */
-    t->processor_family = 0x01; /* Other */
     SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
     t->processor_id[0] = smbios_cpuid_version;
     t->processor_id[1] = smbios_cpuid_features;
@@ -507,6 +508,10 @@ static void smbios_build_type_4_table(unsigned instance)
     SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
     SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
     SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
+    t->core_count = t->core_enabled = smp_cores;
+    t->thread_count = smp_threads;
+    t->processor_characteristics = 0x02; /* Unknown */
+    t->processor_family = t->processor_family2 = 0x01; /* Other */

SMBIOS_BUILD_TABLE_POST;
     smbios_type4_count++;
@@ -658,7 +663,11 @@ uint8_t *smbios_get_table(size_t *length)
         smbios_build_type_1_table();
         smbios_build_type_2_table();
         smbios_build_type_3_table();
-        for (i = 0; i < smp_cpus; i++) {
+
+        smbios_smp_sockets = smp_cpus / (smp_cores * smp_threads);
+        assert (smbios_smp_sockets >= 1);
+
+        for (i = 0; i < smbios_smp_sockets; i++) {
             smbios_build_type_4_table(i);
         }

diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index ce2f0f2..9067139 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -99,7 +99,7 @@ struct smbios_type_3 {
     /* contained elements follow */
 } QEMU_PACKED;

-/* SMBIOS type 4 - Processor Information (v2.3) */
+/* SMBIOS type 4 - Processor Information (v2.6) */
 struct smbios_type_4 {
     struct smbios_structure_header header;
     uint8_t socket_designation_str;
@@ -120,7 +120,11 @@ struct smbios_type_4 {
     uint8_t serial_number_str;
     uint8_t asset_tag_number_str;
     uint8_t part_number_str;
-
+    uint8_t core_count;
+    uint8_t core_enabled;
+    uint8_t thread_count;
+    uint16_t processor_characteristics;
+    uint16_t processor_family2;
 } QEMU_PACKED;

/* SMBIOS type 16 - Physical Memory Array */

[QEMU v5 PATCH 17/18] SMBIOS: Update memory table types (16, 17, and 19) to smbios spec v2.8

[Gabriel L. Somlo]

This patch adds extended start/end address and extended size fields
to each memory table type.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/smbios.c         | 46 +++++++++++++++++++++++++++++-----------------
 include/hw/i386/smbios.h | 15 ++++++++++++---
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 382b75d..2f0755f 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -521,21 +521,24 @@ static void smbios_build_type_4_table(unsigned instance)
 #define ONE_MB ((ram_addr_t)1 << 20)
 #define ONE_GB ((ram_addr_t)1 << 30)

+#define MAX_T16_STD_SZ 0x80000000 /* 2T in Kilobytes */
+
 static void smbios_build_type_16_table(unsigned dimm_cnt)
 {
-    ram_addr_t  ram_size_kb = ram_size >> 10;
+    ram_addr_t size_kb;

SMBIOS_BUILD_TABLE_PRE(16, 0x1000, true); /* required */

t->location = 0x01; /* Other */
     t->use = 0x03; /* System memory */
     t->error_correction = 0x06; /* Multi-bit ECC (for Microsoft, per SeaBIOS) */
-    /* if ram_size < 2T, use value in Kilobytes; 0x80000000 == 2T and over */
-    t->maximum_capacity = (ram_size_kb < 0x80000000) ? ram_size_kb : 0x80000000;
-    if (t->maximum_capacity == 0x80000000) {
-        /* TODO: support smbios v2.7 extended capacity */
-        fprintf(stderr, "qemu: warning: SMBIOS v2.7+ required for "
-                        "ram_size >= 2T (%ld)\n", ram_size);
+    size_kb = QEMU_ALIGN_UP(ram_size, ONE_KB) / ONE_KB;
+    if (size_kb < MAX_T16_STD_SZ) {
+        t->maximum_capacity = size_kb;
+        t->extended_maximum_capacity = 0;
+    } else {
+        t->maximum_capacity = MAX_T16_STD_SZ;
+        t->extended_maximum_capacity = ram_size;
     }
     t->memory_error_information_handle = 0xFFFE; /* Not provided */
     t->number_of_memory_devices = dimm_cnt;
@@ -543,6 +546,9 @@ static void smbios_build_type_16_table(unsigned dimm_cnt)
     SMBIOS_BUILD_TABLE_POST;
 }

+#define MAX_T17_STD_SZ 0x7FFF /* (32G - 1M), in Megabytes */
+#define MAX_T18_EXT_SZ 0x80000000 /* in Megabytes */
+
 static void smbios_build_type_17_table(unsigned instance, ram_addr_t size)
 {
     char loc_str[128];
@@ -555,13 +561,13 @@ static void smbios_build_type_17_table(unsigned instance, ram_addr_t size)
     t->total_width = 0xFFFF; /* Unknown */
     t->data_width = 0xFFFF; /* Unknown */
     size_mb = QEMU_ALIGN_UP(size, ONE_MB) / ONE_MB;
-    if (size_mb < 0x7FFF) {
+    if (size_mb < MAX_T17_STD_SZ) {
         t->size = size_mb;
+        t->extended_size = 0;
     } else {
-        t->size = 0x7FFF;
-        /* TODO: support smbios v2.7 extended capacity */
-        fprintf(stderr, "qemu: warning: SMBIOS v2.7+ required for "
-                        "DIMM size >= 0x7FFF Mbytes (0x%lx)\n", size_mb);
+        assert(size_mb < MAX_T18_EXT_SZ);
+        t->size = MAX_T17_STD_SZ;
+        t->extended_size = size_mb;
     }
     t->form_factor = 0x09; /* DIMM */
     t->device_set = 0; /* Not in a set */
@@ -575,6 +581,11 @@ static void smbios_build_type_17_table(unsigned instance, ram_addr_t size)
     SMBIOS_TABLE_SET_STR(17, serial_number_str, type17.serial);
     SMBIOS_TABLE_SET_STR(17, asset_tag_number_str, type17.asset);
     SMBIOS_TABLE_SET_STR(17, part_number_str, type17.part);
+    t->attributes = 0; /* Unknown */
+    t->configured_clock_speed = 0; /* Unknown */
+    t->minimum_voltage = 0; /* Unknown */
+    t->maximum_voltage = 0; /* Unknown */
+    t->configured_voltage = 0; /* Unknown */

SMBIOS_BUILD_TABLE_POST;
 }
@@ -590,13 +601,14 @@ static void smbios_build_type_19_table(unsigned instance,
     assert(end > start);
     start_kb = start / ONE_KB;
     end_kb = end / ONE_KB;
-    if (start_kb >= UINT32_MAX || end_kb >= UINT32_MAX) {
-        t->starting_address = t->ending_address = UINT32_MAX;
-        fprintf(stderr, "qemu: warning: SMBIOS v2.7+ required for "
-                        "type19(start=%lx, size=%lx)\n", start, size);
-    } else {
+    if (start_kb < UINT32_MAX && end_kb < UINT32_MAX) {
         t->starting_address = start_kb;
         t->ending_address = end_kb;
+        t->extended_starting_address = t->extended_ending_address = 0;
+    } else {
+        t->starting_address = t->ending_address = UINT32_MAX;
+        t->extended_starting_address = start;
+        t->extended_ending_address = end;
     }
     t->memory_array_handle = 0x1000; /* Type 16 (Phys. Mem. Array) */
     t->partition_width = 1; /* One device per row */
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 9067139..4525c9b 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -127,7 +127,7 @@ struct smbios_type_4 {
     uint16_t processor_family2;
 } QEMU_PACKED;

-/* SMBIOS type 16 - Physical Memory Array */
+/* SMBIOS type 16 - Physical Memory Array (v2.7) */
 struct smbios_type_16 {
     struct smbios_structure_header header;
     uint8_t location;
@@ -136,9 +136,10 @@ struct smbios_type_16 {
     uint32_t maximum_capacity;
     uint16_t memory_error_information_handle;
     uint16_t number_of_memory_devices;
+    uint64_t extended_maximum_capacity;
 } QEMU_PACKED;

-/* SMBIOS type 17 - Memory Device (v2.3) */
+/* SMBIOS type 17 - Memory Device (v2.8) */
 struct smbios_type_17 {
     struct smbios_structure_header header;
     uint16_t physical_memory_array_handle;
@@ -157,15 +158,23 @@ struct smbios_type_17 {
     uint8_t serial_number_str;
     uint8_t asset_tag_number_str;
     uint8_t part_number_str;
+    uint8_t attributes;
+    uint32_t extended_size;
+    uint32_t configured_clock_speed;
+    uint32_t minimum_voltage;
+    uint32_t maximum_voltage;
+    uint32_t configured_voltage;
 } QEMU_PACKED;

-/* SMBIOS type 19 - Memory Array Mapped Address */
+/* SMBIOS type 19 - Memory Array Mapped Address (v2.7) */
 struct smbios_type_19 {
     struct smbios_structure_header header;
     uint32_t starting_address;
     uint32_t ending_address;
     uint16_t memory_array_handle;
     uint8_t partition_width;
+    uint64_t extended_starting_address;
+    uint64_t extended_ending_address;
 } QEMU_PACKED;

/* SMBIOS type 32 - System Boot Information */

[QEMU v5 PATCH 18/18] SMBIOS: Generate complete smbios tables, including entry point

[Gabriel L. Somlo]

Build a full set of smbios tables as a monolithic blob; Also,
build an entry point structure, and insert both the set
of tables and the entry point into distinct fw_cfg files.

This patch expects a SeaBIOS version equal or later than
commit XXXX<to-be-filled-out-once-applied>XXXXX. An earlier
version will work, but will not be able to retrieve any
smbios data passed from QEMU, effectively resulting in any
command-line smbios options being ignored.

Signed-off-by: Gabriel Somlo 
---

Before applying this patch, QEMU should switch to a version of SeaBIOS
which includes this (or a similar) commit:

http://www.seabios.org/pipermail/seabios/2014-April/007823.html

While older SeaBIOS versions will still work, any command-line "-smbios"
options would get ignored.

Thanks,
   Gabriel

hw/i386/pc.c             |  17 +++---
 hw/i386/smbios.c         | 135 ++++++++++++++++++++++++-----------------------
 include/hw/i386/smbios.h |  23 +++++++-
 3 files changed, 100 insertions(+), 75 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ff353cf..8d54489 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -73,7 +73,7 @@
 #define ACPI_DATA_SIZE       0x10000
 #define BIOS_CFG_IOPORT 0x510
 #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
-#define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
+#define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1) /* deprecated */
 #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
 #define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
 #define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
@@ -642,8 +642,8 @@ static unsigned int pc_apic_id_limit(unsigned int max_cpus)
 static FWCfgState *bochs_bios_init(void)
 {
     FWCfgState *fw_cfg;
-    uint8_t *smbios_table;
-    size_t smbios_len;
+    uint8_t *smbios_tables, *smbios_anchor;
+    size_t smbios_tables_len, smbios_anchor_len;
     uint64_t *numa_fw_cfg;
     int i, j;
     unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
@@ -670,10 +670,13 @@ static FWCfgState *bochs_bios_init(void)
                      acpi_tables, acpi_tables_len);
     fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());

-    smbios_table = smbios_get_table(&smbios_len);
-    if (smbios_table)
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
-                         smbios_table, smbios_len);
+    smbios_get_tables(&smbios_tables, &smbios_tables_len,
+                      &smbios_anchor, &smbios_anchor_len);
+    fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
+                    smbios_tables, smbios_tables_len);
+    fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
+                    smbios_anchor, smbios_anchor_len);
+
     fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
                      &e820_reserve, sizeof(e820_reserve));
     fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 2f0755f..20f6b7c 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -23,24 +23,12 @@
 #include "hw/i386/smbios.h"
 #include "hw/loader.h"

-/*
- * Structures shared with the BIOS
- */
-struct smbios_header {
-    uint16_t length;
-    uint8_t type;
-} QEMU_PACKED;
-
-struct smbios_table {
-    struct smbios_header header;
-    uint8_t data[];
-} QEMU_PACKED;
-
-#define SMBIOS_FIELD_ENTRY 0
-#define SMBIOS_TABLE_ENTRY 1
-
 static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
+static unsigned smbios_table_max;
+static unsigned smbios_table_cnt;
+static struct smbios_entry_point ep;
+
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
 static bool smbios_have_defaults;
@@ -318,9 +306,8 @@ static bool smbios_skip_table(uint8_t type, bool required_table)
 }

#define SMBIOS_BUILD_TABLE_PRE(tbl_type, tbl_handle, tbl_required)        \
-    struct smbios_table *w;                                               \
     struct smbios_type_##tbl_type *t;                                     \
-    size_t w_off, t_off; /* wrapper, table offsets into smbios_entries */ \
+    size_t t_off; /* table offset into smbios_entries */                  \
     int str_index = 0;                                                    \
     do {                                                                  \
         /* should we skip building this table ? */                        \
@@ -328,24 +315,13 @@ static bool smbios_skip_table(uint8_t type, bool required_table)
             return;                                                       \
         }                                                                 \
                                                                           \
-        /* initialize fw_cfg smbios element count */                      \
-        if (!smbios_entries) {                                            \
-            smbios_entries_len = sizeof(uint16_t);                        \
-            smbios_entries = g_malloc0(smbios_entries_len);               \
-        }                                                                 \
-                                                                          \
-        /* use offsets of wrapper w and table t within smbios_entries  */ \
-        /* (pointers must be updated after each realloc)               */ \
-        w_off = smbios_entries_len;                                       \
-        t_off = w_off + sizeof(*w);                                       \
-        smbios_entries_len = t_off + sizeof(*t);                          \
+        /* use offset of table t within smbios_entries */                 \
+        /* (pointer must be updated after each realloc) */                \
+        t_off = smbios_entries_len;                                       \
+        smbios_entries_len += sizeof(*t);                                 \
         smbios_entries = g_realloc(smbios_entries, smbios_entries_len);   \
-        w = (struct smbios_table *)(smbios_entries + w_off);              \
         t = (struct smbios_type_##tbl_type *)(smbios_entries + t_off);    \
                                                                           \
-        w->header.type = SMBIOS_TABLE_ENTRY;                              \
-        w->header.length = sizeof(*w) + sizeof(*t);                       \
-                                                                          \
         t->header.type = tbl_type;                                        \
         t->header.length = sizeof(*t);                                    \
         t->header.handle = tbl_handle;                                    \
@@ -359,11 +335,9 @@ static bool smbios_skip_table(uint8_t type, bool required_table)
                                        smbios_entries_len + len);         \
             memcpy(smbios_entries + smbios_entries_len, value, len);      \
             smbios_entries_len += len;                                    \
-            /* update pointer(s) post-realloc */                          \
-            w = (struct smbios_table *)(smbios_entries + w_off);          \
+            /* update pointer post-realloc */                             \
             t = (struct smbios_type_##tbl_type *)(smbios_entries + t_off);\
             t->field = ++str_index;                                       \
-            w->header.length += len;                                      \
         } else {                                                          \
             t->field = 0;                                                 \
         }                                                                 \
@@ -371,20 +345,23 @@ static bool smbios_skip_table(uint8_t type, bool required_table)

#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;                                      \
-            /* update pointer(s) post-realloc */                          \
-            w = (struct smbios_table *)(smbios_entries + w_off);          \
-            w->header.length += 1;                                        \
+        size_t term_cnt, t_size;                                          \
+                                                                          \
+        /* add '\0' terminator (add two if no strings defined) */         \
+        term_cnt = (str_index == 0) ? 2 : 1;                              \
+        smbios_entries = g_realloc(smbios_entries,                        \
+                                   smbios_entries_len + term_cnt);        \
+        memset(smbios_entries + smbios_entries_len, 0, term_cnt);         \
+        smbios_entries_len += term_cnt;                                   \
+                                                                          \
+        /* update smbios max. element size */                             \
+        t_size = smbios_entries_len - t_off;                              \
+        if (t_size > smbios_table_max) {                                  \
+            smbios_table_max = t_size;                                    \
         }                                                                 \
                                                                           \
-        /* update fw_cfg smbios element count */                          \
-        *(uint16_t *)smbios_entries += 1;                                 \
+        /* update smbios element count */                                 \
+        smbios_table_cnt++;                                               \
     } while (0)

static void smbios_build_type_0_table(void)
@@ -666,7 +643,32 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type17.manufacturer, manufacturer);
 }

-uint8_t *smbios_get_table(size_t *length)
+static void smbios_entry_point_setup(void)
+{
+    memcpy(ep.anchor_string, "_SM_", 4);
+    memcpy(ep.intermediate_anchor_string, "_DMI_", 5);
+    ep.length = sizeof(struct smbios_entry_point);
+    ep.entry_point_revision = 0; /* formatted_area reserved, per spec v2.1+ */
+    memset(ep.formatted_area, 0, 5);
+
+    /* compliant with smbios spec v2.8 */
+    ep.smbios_major_version = 2;
+    ep.smbios_minor_version = 8;
+    ep.smbios_bcd_revision = 0x28;
+
+    /* set during table construction, but BIOS may override: */
+    ep.structure_table_length = smbios_entries_len;
+    ep.max_structure_size = smbios_table_max;
+    ep.number_of_structures = smbios_table_cnt;
+
+    /* BIOS must recalculate: */
+    ep.checksum = 0;
+    ep.intermediate_checksum = 0;
+    ep.structure_table_address = 0; /* where BIOS has copied smbios_entries */
+}
+
+void smbios_get_tables(uint8_t **tables, size_t *tables_len,
+                       uint8_t **anchor, size_t *anchor_len)
 {
     unsigned i, dimm_cnt, instance;

@@ -705,10 +707,15 @@ uint8_t *smbios_get_table(size_t *length)
         smbios_build_type_127_table();

smbios_validate_table();
+        smbios_entry_point_setup();
         smbios_immutable = true;
     }
-    *length = smbios_entries_len;
-    return smbios_entries;
+
+    /* return tables blob and entry point (anchor), and their sizes */
+    *tables = smbios_entries;
+    *tables_len = smbios_entries_len;
+    *anchor = (uint8_t *)&ep;
+    *anchor_len = sizeof(struct smbios_entry_point);
 }

static void save_opt(const char **dest, QemuOpts *opts, const char *name)
@@ -729,7 +736,6 @@ void smbios_entry_add(QemuOpts *opts)
     val = qemu_opt_get(opts, "file");
     if (val) {
         struct smbios_structure_header *header;
-        struct smbios_table *table;
         int size;

qemu_opts_validate(opts, qemu_smbios_file_opts, &local_err);
@@ -744,24 +750,16 @@ void smbios_entry_add(QemuOpts *opts)
             exit(1);
         }

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

-        smbios_entries = g_realloc(smbios_entries, smbios_entries_len +
-                                                      sizeof(*table) + size);
-        table = (struct smbios_table *)(smbios_entries + smbios_entries_len);
-        table->header.type = SMBIOS_TABLE_ENTRY;
-        table->header.length = cpu_to_le16(sizeof(*table) + size);
+        header = (struct smbios_structure_header *)
+                 (smbios_entries + smbios_entries_len);

-        if (load_image(val, table->data) != size) {
+        if (load_image(val, (uint8_t *)header) != size) {
             error_report("Failed to load SMBIOS file %s", val);
             exit(1);
         }

-        header = (struct smbios_structure_header *)(table->data);
-
         if (test_bit(header->type, have_fields_bitmap)) {
             error_report("Can't add binary type %d table! "
                          "(fields already specified)", header->type);
@@ -773,9 +771,12 @@ void smbios_entry_add(QemuOpts *opts)
             smbios_type4_count++;
         }

-        smbios_entries_len += sizeof(*table) + size;
-        (*(uint16_t *)smbios_entries) =
-                cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
+        smbios_entries_len += size;
+        if (size > smbios_table_max) {
+            smbios_table_max = size;
+        }
+        smbios_table_cnt++;
+
         return;
     }

diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 4525c9b..096d12b 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -21,12 +21,33 @@ void smbios_entry_add(QemuOpts *opts);
 void smbios_set_cpuid(uint32_t version, uint32_t features);
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version);
-uint8_t *smbios_get_table(size_t *length);
+void smbios_get_tables(uint8_t **tables, size_t *tables_len,
+                       uint8_t **anchor, size_t *anchor_len);

/*
  * SMBIOS spec defined tables
  */

+/* SMBIOS entry point (anchor).
+ * BIOS must place this at a 16-bit-aligned address between 0xf0000 and 0xfffff.
+ */
+struct smbios_entry_point {
+    uint8_t anchor_string[4];
+    uint8_t checksum;
+    uint8_t length;
+    uint8_t smbios_major_version;
+    uint8_t smbios_minor_version;
+    uint16_t max_structure_size;
+    uint8_t entry_point_revision;
+    uint8_t formatted_area[5];
+    uint8_t intermediate_anchor_string[5];
+    uint8_t intermediate_checksum;
+    uint16_t structure_table_length;
+    uint32_t structure_table_address;
+    uint16_t number_of_structures;
+    uint8_t smbios_bcd_revision;
+} QEMU_PACKED;
+
 /* This goes at the beginning of every SMBIOS structure. */
 struct smbios_structure_header {
     uint8_t type;

[QEMU v5 PATCH 00/18] SMBIOS: build full tables in QEMU

[Kevin O'Connor]

On Fri, Apr 11, 2014 at 12:11:40PM -0400, Gabriel L. Somlo wrote:
> Quick highlights:
>
> after 10/18, we're generating tables 100% consistent with SeaBiOS
>
> after 14/18, we use e820 to generate type 19 (memory area) tables
>
> after 18/18, we expect SeaBIOS to check fw_cfg for a full set of
> tables and an entry point (e.g., we need a version of SeaBIOS which
> has applied this patch:
>
> http://www.seabios.org/pipermail/seabios/2014-April/007823.html
>
> Please let me know what you all think.

Thanks.  I'm not a QEMU expert, but I did review your patches.  The
patch series looks good to me and I think this is the right direction.

As a minor quibble, I think patch 18 should make type0 required
instead of optional (once there are the new fw_cfg entries there is no
harm in always producing type0).  Also, it would be nice to move up
patch 18 to after patch 10 - that way an end-to-end test between
old/new smbios with the new interface could be done.  I defer to
regular qemu developers on these comments though.

-Kevin

[QEMU v5 PATCH 00/18] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

Kevin,

Thanks for the comments. I'll work your feedback (and any other
feedback I get by early next week) into another iteration of smbios
patches for both SeaBIOS and QEMU.

In the mean time, there's one remaining "big picture" design question:

On Sat, Apr 12, 2014 at 11:56:08AM -0400, Kevin O'Connor wrote:
> QEMU currently has command-line options that can modify the fields of
> the type0 tables (-smbios type=0,vendor='foo').  To continue to
> support that, I think QEMU should be able to build the type0 table as
> it feels fit to, and SeaBIOS should be able to pass it through.  Of
> course, if there's no specific request from the end user, then I think
> QEMU can tell SeaBIOS that it may replace the type0 content with its
> own data (eg, via "etc/update-smbios-type0").
>
> [...]
>
> As a minor quibble, I think patch 18 should make type0 required
> instead of optional (once there are the new fw_cfg entries there is no
> harm in always producing type0).  Also, it would be nice to move up
> patch 18 to after patch 10 - that way an end-to-end test between
> old/new smbios with the new interface could be done.  I defer to
> regular qemu developers on these comments though.

There's three options I can think of:

1. QEMU always generates its own type 0 table. In this case, SeaBIOS
can probably just use that, along with the rest of the tables, as
provided. QEMU would have to "impersonate" or "channel" SeaBIOS when
generating the type 0 table (or "channel" TianoCore, depending on which
bios is being used).

2. QEMU only generates type 0 if explicitly told to do so on the
command line (i.e., *not* by default). In this case, SeaBIOS (or OVMF,
or any other BIOS) would have to scan the tables and insert its own
default type 0 if one was not purposely supplied by QEMU. (I know my
current SeaBIOS patch always overrides type 0, and agree that's
inconsistent with this option, and plan on fixing it :)

3. QEMU never generates a type 0 structure (i.e. we remove that
command line option), and the BIOS is *always* responsible for
generating it ("allowing type 0 on the qemu command line was a bad
idea, nobody uses it, we shouldn't have done it in the first place",
to paraphrase from an earlier thread).

I personally like #2 as it appears simple and flexible, and doesn't
require any further coordination (beyond qemu providing an entry
point and a set of tables).

However, I'm not religious about it -- I'm only really after type 2
and 17, for OS X's sake, as you all may remember... :)

Gerd, Laszlo, what do you guys think ?

Thanks,
--Gabriel

[QEMU v5 PATCH 00/18] SMBIOS: build full tables in QEMU

[Laszlo Ersek]

On 04/13/14 02:55, Gabriel L. Somlo wrote:
> Kevin,
>
> Thanks for the comments. I'll work your feedback (and any other
> feedback I get by early next week) into another iteration of smbios
> patches for both SeaBIOS and QEMU.
>
> In the mean time, there's one remaining "big picture" design question:
>
> On Sat, Apr 12, 2014 at 11:56:08AM -0400, Kevin O'Connor wrote:
> > QEMU currently has command-line options that can modify the fields of
> > the type0 tables (-smbios type=0,vendor='foo').  To continue to
> > support that, I think QEMU should be able to build the type0 table as
> > it feels fit to, and SeaBIOS should be able to pass it through.  Of
> > course, if there's no specific request from the end user, then I think
> > QEMU can tell SeaBIOS that it may replace the type0 content with its
> > own data (eg, via "etc/update-smbios-type0").
> >
> > [...]
> >
> > As a minor quibble, I think patch 18 should make type0 required
> > instead of optional (once there are the new fw_cfg entries there is no
> > harm in always producing type0).  Also, it would be nice to move up
> > patch 18 to after patch 10 - that way an end-to-end test between
> > old/new smbios with the new interface could be done.  I defer to
> > regular qemu developers on these comments though.
>
> There's three options I can think of:
> QEMU always generates its own type 0 table. In this case, SeaBIOS
>
> can probably just use that, along with the rest of the tables, as
> provided. QEMU would have to "impersonate" or "channel" SeaBIOS when
> generating the type 0 table (or "channel" TianoCore, depending on which
> bios is being used).
> QEMU only generates type 0 if explicitly told to do so on the
>
> command line (i.e., *not* by default). In this case, SeaBIOS (or OVMF,
> or any other BIOS) would have to scan the tables and insert its own
> default type 0 if one was not purposely supplied by QEMU. (I know my
> current SeaBIOS patch always overrides type 0, and agree that's
> inconsistent with this option, and plan on fixing it :)
> QEMU never generates a type 0 structure (i.e. we remove that
>
> command line option), and the BIOS is *always* responsible for
> generating it ("allowing type 0 on the qemu command line was a bad
> idea, nobody uses it, we shouldn't have done it in the first place",
> to paraphrase from an earlier thread).
>
> I personally like #2 as it appears simple and flexible, and doesn't
> require any further coordination (beyond qemu providing an entry
> point and a set of tables).
>
> However, I'm not religious about it -- I'm only really after type 2
> and 17, for OS X's sake, as you all may remember... :)
>
> Gerd, Laszlo, what do you guys think ?

You're moving too fast for me. I'm swamped. By the time I'd come around
reading and *maybe* answering your email, you make so much progress that
you send three others :)

So, I'll defer to y'all's good judgement.

(I don't let my sense of taste get in my way any longer (and actually I
don't want *others'* taste get in my way either, for that matter, when
I'm the one posting patches...), so just make something simple and
bug-free, document the format, and I'll adapt.)

Thanks!
Laszlo

[Qemu-devel] [QEMU v5 PATCH 00/18] SMBIOS: build full tables in QEMU

[Markus Armbruster]

"Gabriel L. Somlo"  writes:
> Kevin,
>
> Thanks for the comments. I'll work your feedback (and any other
> feedback I get by early next week) into another iteration of smbios
> patches for both SeaBIOS and QEMU.
>
> In the mean time, there's one remaining "big picture" design question:
>
> On Sat, Apr 12, 2014 at 11:56:08AM -0400, Kevin O'Connor wrote:
> > QEMU currently has command-line options that can modify the fields of
> > the type0 tables (-smbios type=0,vendor='foo').  To continue to
> > support that, I think QEMU should be able to build the type0 table as
> > it feels fit to, and SeaBIOS should be able to pass it through.  Of
> > course, if there's no specific request from the end user, then I think
> > QEMU can tell SeaBIOS that it may replace the type0 content with its
> > own data (eg, via "etc/update-smbios-type0").
> >
> > [...]
> >
> > As a minor quibble, I think patch 18 should make type0 required
> > instead of optional (once there are the new fw_cfg entries there is no
> > harm in always producing type0).  Also, it would be nice to move up
> > patch 18 to after patch 10 - that way an end-to-end test between
> > old/new smbios with the new interface could be done.  I defer to
> > regular qemu developers on these comments though.
>
> There's three options I can think of:
> QEMU always generates its own type 0 table. In this case, SeaBIOS
>
> can probably just use that, along with the rest of the tables, as
> provided. QEMU would have to "impersonate" or "channel" SeaBIOS when
> generating the type 0 table (or "channel" TianoCore, depending on which
> bios is being used).

Begs the question how QEMU would figure out what exactly the BIOS wants
it to put into the type 0 table.  Unless somebody can come up with a
practical answer, we can ignore this option :)
> QEMU only generates type 0 if explicitly told to do so on the
>
> command line (i.e., *not* by default). In this case, SeaBIOS (or OVMF,
> or any other BIOS) would have to scan the tables and insert its own
> default type 0 if one was not purposely supplied by QEMU. (I know my
> current SeaBIOS patch always overrides type 0, and agree that's
> inconsistent with this option, and plan on fixing it :)
> QEMU never generates a type 0 structure (i.e. we remove that
>
> command line option), and the BIOS is *always* responsible for
> generating it ("allowing type 0 on the qemu command line was a bad
> idea, nobody uses it, we shouldn't have done it in the first place",
> to paraphrase from an earlier thread).

This is a simplification of 2: the BIOS doesn't have to check whether
QEMU provided type 0 information.  Paid for with an incompatible change
of a somewhat obscure QEMU feature.
> I personally like #2 as it appears simple and flexible, and doesn't
> require any further coordination (beyond qemu providing an entry
> point and a set of tables).
>
> However, I'm not religious about it -- I'm only really after type 2
> and 17, for OS X's sake, as you all may remember... :)
>
> Gerd, Laszlo, what do you guys think ?

My unsolicited advice: if 2. is easy for you, go for it.

[QEMU v5 PATCH 00/18] SMBIOS: build full tables in QEMU

[Gerd Hoffmann]

Hi,
> QEMU only generates type 0 if explicitly told to do so on the
>
> command line (i.e., *not* by default). In this case, SeaBIOS (or OVMF,
> or any other BIOS) would have to scan the tables and insert its own
> default type 0 if one was not purposely supplied by QEMU. (I know my
> current SeaBIOS patch always overrides type 0, and agree that's
> inconsistent with this option, and plan on fixing it :)

This is the best option IMO.

cheers,
  Gerd

[QEMU v5 PATCH 00/18] SMBIOS: build full tables in QEMU

[Gerd Hoffmann]

Hi,
> harm in always producing type0).  Also, it would be nice to move up
> patch 18 to after patch 10 - that way an end-to-end test between
> old/new smbios with the new interface could be done.

Agree, that would be nice to have (although I wouldn't make that a hard
requirement for merging).

I also think we should continue providing the tables one-by-one using
the old interface, at least for a transition period, so older seabios
versions continue to work.

cheers,
  Gerd