[QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

New in version 6 of the patch set:

- down to 17 patches (squashed adding spec v2.4 fields
    in together with adding v2.8 fields further down).

- switching to monolithic aggregate tables plus entry point
    in patch 11/17, right after accomplishing full SeaBIOS
    compatibility (in 10/17).

- Type 0 (bios info) structure continues to be optional. The BIOS
    is expected to supply its own Type 0 structure unless we force
    one to be provided via the command line.

On Mon, Apr 14, 2014 at 11:14:08AM +0200, Gerd Hoffmann wrote:
> 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.

Is *this* a hard requirement for merging ? I'm currently brainstorming
for ways to do this that won't be horrifyingly ugly, but so far everything
I managed to come up with makes me want to scratch my eyes out :)

Thanks,
  Gabriel

Gabriel L. Somlo (17):
  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: Generate aggregate smbios tables, including entry point
  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

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 v6 PATCH 01/17] 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 v6 PATCH 02/17] 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 v6 PATCH 03/17] 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 v6 PATCH 04/17] 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 v6 PATCH 05/17] 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 v6 PATCH 06/17] 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 v6 PATCH 07/17] 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 v6 PATCH 08/17] 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 v6 PATCH 09/17] 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 v6 PATCH 10/17] 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 v6 PATCH 11/17] SMBIOS: Generate aggregate smbios tables, including entry point

[Gabriel L. Somlo]

Build a complete 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<insert-seabios-commit-no-here>XXXXX. An earlier
version will work, but without the ability to retrieve any
smbios data passed from QEMU, effectively resulting in any
command-line smbios options being ignored.

Signed-off-by: Gabriel Somlo 
---
 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 6e3962e..b3c7cc2 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)
@@ -627,8 +627,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);
@@ -655,10 +655,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 b1f1d46..12cd06a 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -21,24 +21,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;
@@ -312,9 +300,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 ? */                        \
@@ -322,24 +309,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;                                    \
@@ -353,11 +329,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;                                                 \
         }                                                                 \
@@ -365,20 +339,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)
@@ -675,7 +652,32 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type17.loc_pfx, "DIMM");
 }

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

@@ -774,10 +776,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)
@@ -798,7 +805,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);
@@ -813,24 +819,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);
@@ -842,9 +840,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 2a0d384..8a4e3b1 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -23,12 +23,33 @@ 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);
-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 v6 PATCH 12/17] 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 12cd06a..1b9465a 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -469,8 +469,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 */
@@ -515,9 +515,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;
@@ -533,7 +533,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 */

SMBIOS_BUILD_TABLE_POST;
 }
@@ -624,7 +624,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);
@@ -640,15 +639,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");
 }

@@ -687,8 +682,7 @@ void smbios_get_tables(uint8_t **tables, size_t *tables_len,
         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 v6 PATCH 13/17] 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 1b9465a..f6fbb69 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -563,35 +563,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 */
@@ -685,64 +656,6 @@ void smbios_get_tables(uint8_t **tables, size_t *tables_len,
             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)

@@ -752,18 +665,8 @@ void smbios_get_tables(uint8_t **tables, size_t *tables_len,
             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 8a4e3b1..83a3604 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -177,18 +177,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 v6 PATCH 14/17] 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 b3c7cc2..8d54489 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 f6fbb69..eb11095 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"

@@ -31,7 +32,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);
@@ -591,16 +591,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");
@@ -645,7 +639,7 @@ static void smbios_entry_point_setup(void)
 void smbios_get_tables(uint8_t **tables, size_t *tables_len,
                        uint8_t **anchor, size_t *anchor_len)
 {
-    unsigned i, dimm_cnt;
+    unsigned i, dimm_cnt, instance;

if (!smbios_immutable) {
         smbios_build_type_0_table();
@@ -660,13 +654,18 @@ void smbios_get_tables(uint8_t **tables, size_t *tables_len,
 #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 83a3604..6be0445 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);
 void smbios_get_tables(uint8_t **tables, size_t *tables_len,
                        uint8_t **anchor, size_t *anchor_len);

[QEMU v6 PATCH 15/17] 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 eb11095..25d2aa3 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -52,7 +52,7 @@ static struct {
 } type2;

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

static struct {
@@ -200,6 +200,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 */ }
 };
@@ -449,6 +453,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;
 }
@@ -826,6 +831,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 6be0445..7d5eac8 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -100,7 +100,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;
@@ -116,6 +116,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 v6 PATCH 16/17] 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         | 22 +++++++++++++++++-----
 include/hw/i386/smbios.h | 10 +++++++++-
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 25d2aa3..8b524a9 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"
@@ -31,7 +32,7 @@ static struct smbios_entry_point ep;
 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);
@@ -283,8 +284,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);
     }
 }
@@ -467,7 +469,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;
@@ -481,6 +482,13 @@ 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);
+    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++;
@@ -651,7 +659,11 @@ void smbios_get_tables(uint8_t **tables, size_t *tables_len,
         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 7d5eac8..649c80c 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -120,7 +120,7 @@ struct smbios_type_3 {
     /* contained elements follow */
 } QEMU_PACKED;

-/* SMBIOS type 4 - Processor Information (v2.0) */
+/* SMBIOS type 4 - Processor Information (v2.6) */
 struct smbios_type_4 {
     struct smbios_structure_header header;
     uint8_t socket_designation_str;
@@ -138,6 +138,14 @@ 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;
+    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 v6 PATCH 17/17] 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         | 52 ++++++++++++++++++++++++++++++++----------------
 include/hw/i386/smbios.h | 20 ++++++++++++++++---
 2 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 8b524a9..ebb25b1 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -498,21 +498,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;
@@ -520,6 +523,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];
@@ -532,13 +538,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 */
@@ -547,6 +553,16 @@ 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 = 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);
+    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;
 }
@@ -562,13 +578,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 */
@@ -623,6 +640,7 @@ void smbios_set_defaults(const char *manufacturer,
     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);
 }

static void smbios_entry_point_setup(void)
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 649c80c..096d12b 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -148,7 +148,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;
@@ -157,9 +157,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 */
+/* SMBIOS type 17 - Memory Device (v2.8) */
 struct smbios_type_17 {
     struct smbios_structure_header header;
     uint16_t physical_memory_array_handle;
@@ -173,15 +174,28 @@ 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;
+    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 v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gerd Hoffmann]

On Mo, 2014-04-14 at 16:54 -0400, Gabriel L. Somlo wrote:
> New in version 6 of the patch set:
>
> down to 17 patches (squashed adding spec v2.4 fields
> in together with adding v2.8 fields further down).
>
> switching to monolithic aggregate tables plus entry point
> in patch 11/17, right after accomplishing full SeaBIOS
> compatibility (in 10/17).
>
> Type 0 (bios info) structure continues to be optional. The BIOS
> is expected to supply its own Type 0 structure unless we force
> one to be provided via the command line.
>
> On Mon, Apr 14, 2014 at 11:14:08AM +0200, Gerd Hoffmann wrote:
> > 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.
>
> Is *this* a hard requirement for merging ? I'm currently brainstorming
> for ways to do this that won't be horrifyingly ugly, but so far everything
> I managed to come up with makes me want to scratch my eyes out :)

Yep, that one is, together with some machine compatibility stuff.  The
series is in pretty good shape, and the compatibility stuff is the only
major thing left to sort out.

Now that we have the complete smbios blob generated in qemu, it doesn't
make much sense to pass the tables using the old interface.

So, how about this plan:

Leave the old interface code basically as-is.  type0 and type1
individual fields are passed like they are passed today.  We don't
change to to pass full tables, and we don't extend that to new table
types.  Continue to provide these in parallel to the new interface, for
compatibility with old firmware (and old machine types).

The code to generate complete tables will only be used for
"etc/smbios/smbios-tables".  Only machine types for 2.1 + newer will
provide them, so with older machine types seabios will continue to
generate the smbios tables and guest wouldn't notice a difference.

That means to drop patch #6.  Patches 4+5+7+8+9+10+11 could be squashed
together (although it is probably better to leave them separate, even
though 4..10 will not have any effect until 11 is applied) and
simplified a bit (don't provide complete tables via old interface).

Comments?

cheers,
  Gerd

[QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

On Tue, Apr 15, 2014 at 10:29:26AM +0200, Gerd Hoffmann wrote:
> So, how about this plan:
>
> Leave the old interface code basically as-is.  type0 and type1
> individual fields are passed like they are passed today.  We don't
> change to to pass full tables, and we don't extend that to new table
> types.  Continue to provide these in parallel to the new interface, for
> compatibility with old firmware (and old machine types).
>
> The code to generate complete tables will only be used for
> "etc/smbios/smbios-tables".  Only machine types for 2.1 + newer will
> provide them, so with older machine types seabios will continue to
> generate the smbios tables and guest wouldn't notice a difference.
>
> That means to drop patch #6.  Patches 4+5+7+8+9+10+11 could be squashed
> together (although it is probably better to leave them separate, even
> though 4..10 will not have any effect until 11 is applied) and
> simplified a bit (don't provide complete tables via old interface).
>
> Comments?

So, basically, leave "uint8_t *smbios_get_table()" unchanged, i.e.
have it generate a blob of field overrides as it does today.

Add "void smbios_get_tables()" as a separate function to generate the
entry point and aggregate smbios table, and then:

if "machine_type < 2.1" then

insert smbios_get_table() into fw_cfg (compat mode)

else

insert smbios_get_tables() blob + entry point (new hotness)

fi

I guess both compat and new smbios_get_table[s] can use the same
default fields set via the command line.

The remaining issue is how to handle "-smbios file=<foo>" blobs; maybe
smbios_set_defaults() can be used to set the version, and then the
command line parser (smbios_entry_add()) would know which kind of table
to insert the blob into, compat or aggregate ?

That being said, how do I even tell the difference between pre- and
post- 2.1 machine types ? Would that be in pc.c, or pc_[q35|piix].c ?

Thanks,
--Gabriel

PS. This should make the patch set a whole lot more compact, as the
main reason in my book for splitting it up into a million pieces was
to avoid confusion related to mixing diff deletions and insertions in
a non-human-readable form; Since now we're talking about mostly
insertions and (almost) no deletions, that should make things quite a
bit easier to follow... :)

[QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gerd Hoffmann]

Hi,
> Add "void smbios_get_tables()" as a separate function to generate the
> entry point and aggregate smbios table, and then:
>
> if "machine_type < 2.1" then
>   insert smbios_get_table() into fw_cfg (compat mode)
>
> else
>   insert smbios_get_tables() blob + entry point (new hotness)
>
> fi

I mean this:

insert smbios_get_table() into fw_cfg (compat mode)
      if (machine_type >= 2.1)
          insert smbios_get_tables() blob + entry point (new hotness)

But maybe your variant is better as it encourages the move to the new
interfaces on the firmware side.  And if we merge it early in the 2.1
cycle there is quite some time for the firmware to sort things.
> I guess both compat and new smbios_get_table[s] can use the same
> default fields set via the command line.

Yes.
> The remaining issue is how to handle "-smbios file=<foo>" blobs; maybe
> smbios_set_defaults() can be used to set the version, and then the
> command line parser (smbios_entry_add()) would know which kind of table
> to insert the blob into, compat or aggregate ?
>
> That being said, how do I even tell the difference between pre- and
> post- 2.1 machine types ? Would that be in pc.c, or pc_[q35|piix].c ?

Global variable, simliar to smbios_type1_defaults.  machine type init
code will set it accordingly.  smbios code just does "if
(smbios_generate_tables) { ... }" (or however we name this).

cheers,
  Gerd

[QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

OK, so I have the "legacy" (field-by-field, types 0 and 1 only) code
back in, right next to the new aggregate-smbios-table-plus-entrypoint
code, tested and apparently working fine.

Before I get carried away with "git rebase", do we still want to go
through the whole patch sequence of generating aggregate tables
identical to what SeaBIOS would have (including e.g. type 20), then
drop type 20 and upgrade to smbios spec v2.8, etc ?

Since we're keeping the legacy code, I can add in the new code (only
for machine types >= 2.1) directly as smbios spec v2.8 compliant
(mainly without adding in generation for type 20, then ripping it
right back out again).

This would result in a smaller, cleaner patch set. Any objections ?

On Wed, Apr 16, 2014 at 08:41:51AM +0200, Gerd Hoffmann wrote:
> Global variable, simliar to smbios_type1_defaults.  machine type init
> code will set it accordingly.  smbios code just does "if
> (smbios_generate_tables) { ... }" (or however we name this).

OK, so right now I'm parsing the "version" argument to what is
currently smbios_set_type1_defaults() (and will become
smbios_set_defaults() after patching).

This gets passed in by pc_piix.c or pc_q35.c from args->machine->name,
and comes in turn from the various machine definitions in
pc_[q35|piix].c.

I noticed some of the older QEMUMachine structures have a hw_version
member set, but the latest I could find being set explicitly is 1.0,
and only in pc_piix.c.

On current (2.0) piix and q35 machines, the hw_version field is NULL,
is that a bug or a feature ?

If the latter (feature), is there a better way to find out which side
of "2.1" I'm currently on than by scraping it out of args->machine->name ?

Thanks,
--Gabriel

[QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gerd Hoffmann]

On Mi, 2014-04-16 at 17:02 -0400, Gabriel L. Somlo wrote:
> OK, so I have the "legacy" (field-by-field, types 0 and 1 only) code
> back in, right next to the new aggregate-smbios-table-plus-entrypoint
> code, tested and apparently working fine.
>
> Before I get carried away with "git rebase", do we still want to go
> through the whole patch sequence of generating aggregate tables
> identical to what SeaBIOS would have (including e.g. type 20), then
> drop type 20 and upgrade to smbios spec v2.8, etc ?
>
> Since we're keeping the legacy code, I can add in the new code (only
> for machine types >= 2.1) directly as smbios spec v2.8 compliant
> (mainly without adding in generation for type 20, then ripping it
> right back out again).
>
> This would result in a smaller, cleaner patch set. Any objections ?

IMHO that is fine.
> On Wed, Apr 16, 2014 at 08:41:51AM +0200, Gerd Hoffmann wrote:
> > Global variable, simliar to smbios_type1_defaults.  machine type init
> > code will set it accordingly.  smbios code just does "if
> > (smbios_generate_tables) { ... }" (or however we name this).
>
> OK, so right now I'm parsing the "version" argument to what is
> currently smbios_set_type1_defaults() (and will become
> smbios_set_defaults() after patching).

No, don't parse stuff please.  Have a look at pc_piix.c, how
smbios_type1_defaults is handled there.  It's true by default, and
pc_compat_1_7 (called for 1.7+older machine types) flips it to false.

We'll add a new variable here, say smbios_generate_table_blob, default
it to true, then flip to false in the (to be added with the 2.1 machine
type) pc_compat_2_0 function.  Same in pc_q35.c.

Then you can simply use the new global variable.

cheers,
  Gerd

[QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

On Thu, Apr 17, 2014 at 10:31:15AM +0200, Gerd Hoffmann wrote:
> > OK, so right now I'm parsing the "version" argument to what is
> > currently smbios_set_type1_defaults() (and will become
> > smbios_set_defaults() after patching).
>
> No, don't parse stuff please.  Have a look at pc_piix.c, how
> smbios_type1_defaults is handled there.  It's true by default, and
> pc_compat_1_7 (called for 1.7+older machine types) flips it to false.
>
> We'll add a new variable here, say smbios_generate_table_blob, default
> it to true, then flip to false in the (to be added with the 2.1 machine
> type) pc_compat_2_0 function.  Same in pc_q35.c.
>
> Then you can simply use the new global variable.

There's one problem I don't see a way around:

Command line options are processed before machine types are
initialized.

This means smbios_entry_add() runs *before* anyone has had a chance to
properly set any global variable regarding whether we're in smbios
legacy mode, or whether we're doing aggregate tables plus entry point.

This means smbios_entry_add() can't know whether a binary blob
received on the command line needs to be added with a table wrapper
for legacy mode, or without one to the aggregate blob for types 2.1
and later...

Guess I could add binary blobs from the command line to *both* legacy
"smbios_entries", and to a new "smbios_aggregate_table", with and
without a SMBIOS_TABLE_ENTRY wrapper, respectively. Then, once
smbios_set_defaults() runs, I can free the one I don't need...

Can anyone think of a less repugnant way to work around this, maybe
something not quite as far "beyond the environment" ?   ;)

Thanks,
--Gabriel

[QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gerd Hoffmann]

Hi,
> Command line options are processed before machine types are
> initialized.

acpi is pretty much in the same boat ...

/me looks ...

Ah, there is a notifier where you (hopefully) can hook in easily:
pc_guest_info_machine_done (see hw/i386/pc.c).

cheers,
  Gerd

[QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

On Tue, Apr 22, 2014 at 08:42:29AM +0200, Gerd Hoffmann wrote:
> acpi is pretty much in the same boat ...
>
> /me looks ...
>
> Ah, there is a notifier where you (hopefully) can hook in easily:
> pc_guest_info_machine_done (see hw/i386/pc.c).

Not sure this can help me though. The order in which things happen is:

1. smbios_entry_add() gets called for every "-smbios file=..." and
     "-smbios type=..." command line argument

a. "type=..." is OK, we just remember the values for later

b. "file=..." less so, because we have to load the blobs into
           *something*. In my v7 patch set I decided to load the blobs
           into *both* the legacy and new-fangled aggregate tables.

2. smbios_set_defaults() is called

- by now we *already* know the machine version we have, so
          we get to free the table we now know we won't need
          (aggregate if we're on <= v2.0, legacy if 2.1 or newer,
           see 1.b. above)

3. smbios_get_table* functions are called and the various resulting
     blobs are inserted into fw_cfg.

4. pc_guest_info_machine_done() gets called after all of the above.

- placing a callback here is unlikely to be helpful, unless
          I'm missing something.

Please let me know what you think of the v7 patches:

http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg03195.html

Thanks,
--Gabriel

[Qemu-devel] [QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gerd Hoffmann]

On Di, 2014-04-22 at 09:01 -0400, Gabriel L. Somlo wrote:
> On Tue, Apr 22, 2014 at 08:42:29AM +0200, Gerd Hoffmann wrote:
> > acpi is pretty much in the same boat ...
> >
> > /me looks ...
> >
> > Ah, there is a notifier where you (hopefully) can hook in easily:
> > pc_guest_info_machine_done (see hw/i386/pc.c).
>
> Not sure this can help me though. The order in which things happen is:
>
> smbios_entry_add() gets called for every "-smbios file=..." and
> "-smbios type=..." command line argument
>
> a. "type=..." is OK, we just remember the values for later
>
> b. "file=..." less so, because we have to load the blobs into
>       *something*.

Remember the filenames for later ...
> In my v7 patch set I decided to load the blobs
>            into *both* the legacy and new-fangled aggregate tables.
>
> smbios_set_defaults() is called
> by now we *already* know the machine version we have, so
> we get to free the table we now know we won't need
> (aggregate if we're on <= v2.0, legacy if 2.1 or newer,
>  see 1.b. above)

... and load the blobs here?
> smbios_get_table* functions are called and the various resulting
> blobs are inserted into fw_cfg.

... or here?
> pc_guest_info_machine_done() gets called after all of the above.
> placing a callback here is unlikely to be helpful, unless
> I'm missing something.

Indeed.  /me was looking for a place surely late enough in init order.

I think acpi needs this because there are also dependencies on devices
added via -device, so doing acpi_setup in QEMUMachine->init() doesn't
work.  For smbios it doesn't make a difference whenever fw_cfg is filled
in that notifier or in QEMUMachine->init().

cheers,
  Gerd

[Qemu-devel] [QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Markus Armbruster]

Gerd Hoffmann  writes:
> On Di, 2014-04-22 at 09:01 -0400, Gabriel L. Somlo wrote:
> > On Tue, Apr 22, 2014 at 08:42:29AM +0200, Gerd Hoffmann wrote:
> > > acpi is pretty much in the same boat ...
> > >
> > > /me looks ...
> > >
> > > Ah, there is a notifier where you (hopefully) can hook in easily:
> > > pc_guest_info_machine_done (see hw/i386/pc.c).
> >
> > Not sure this can help me though. The order in which things happen is:
> >
> > smbios_entry_add() gets called for every "-smbios file=..." and
> > "-smbios type=..." command line argument
> >
> > a. "type=..." is OK, we just remember the values for later
> >
> > b. "file=..." less so, because we have to load the blobs into
> >       *something*.
>
> Remember the filenames for later ...
> > In my v7 patch set I decided to load the blobs
> >            into *both* the legacy and new-fangled aggregate tables.
> >
> > smbios_set_defaults() is called
> > by now we *already* know the machine version we have, so
> > we get to free the table we now know we won't need
> > (aggregate if we're on <= v2.0, legacy if 2.1 or newer,
> >  see 1.b. above)
>
> ... and load the blobs here?

I don't like this technique, because it tends to degrade error messages.
When you're processing the option, your error messages come out with
nice location information automatically.  If you just store filenames,
that's lost.  To preserve it, you need to do extra work.

Note that smbios.c already stores the -smbios type=... for later use by
smbios_get_table()'s table building.  Could you store the blob, too?

[...]

[Qemu-devel] [QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

On Tue, Apr 22, 2014 at 06:16:53PM +0200, Markus Armbruster wrote:
> > > smbios_entry_add() gets called for every "-smbios file=..." and
> > > "-smbios type=..." command line argument
> > >
> > > a. "type=..." is OK, we just remember the values for later
> > >
> > > b. "file=..." less so, because we have to load the blobs into
> > >       *something*.
> >
> > Remember the filenames for later ...
> > > In my v7 patch set I decided to load the blobs
> > >            into *both* the legacy and new-fangled aggregate tables.
> > >
> > > smbios_set_defaults() is called
> > > by now we *already* know the machine version we have, so
> > > we get to free the table we now know we won't need
> > > (aggregate if we're on <= v2.0, legacy if 2.1 or newer,
> > >  see 1.b. above)
> >
> > ... and load the blobs here?
>
> I don't like this technique, because it tends to degrade error messages.
> When you're processing the option, your error messages come out with
> nice location information automatically.  If you just store filenames,
> that's lost.  To preserve it, you need to do extra work.
>
> Note that smbios.c already stores the -smbios type=... for later use by
> smbios_get_table()'s table building.  Could you store the blob, too?

Can you all please check out v7 of the patch set at

http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg03195.html

In particular, patch 7/7 (toward the bottom, where smbios_entry_add()
is being patched, here:

http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg03196.html

The existing code already stores the blobs, wrapped in a
SMBIOS_TABLE_ENTRY wrapper, in "smbios_entries".

The new code needs the blobs *without* any wrappers, just concatenated
together, in the new aggregate table I named "smbios_tables".

So I'm only reading the file blobs *once* (in the new code), and then
malloc room for a wrapper+blob in the legacy smbios_entries table, and
memcpy from where I already read them into "smbios_tables".

I think this is the cleanest way to have both legacy and new/aggregate
logic live side by side. Intrinsically, the new code wouldn't need to
"reuse" any of the old code, so trying to factor out any "common bits"
would actually be less clean, IMHO...

Once you've had a chance to look at the latest patch, please let me
know if you have any strong objections to how it's handling the
problem, or advice on how to do it more cleanly...

Thanks much,
--Gabriel

[QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Kevin O'Connor]

On Tue, Apr 15, 2014 at 10:29:26AM +0200, Gerd Hoffmann wrote:
> Leave the old interface code basically as-is.  type0 and type1
> individual fields are passed like they are passed today.  We don't
> change to to pass full tables, and we don't extend that to new table
> types.  Continue to provide these in parallel to the new interface, for
> compatibility with old firmware (and old machine types).
>
> The code to generate complete tables will only be used for
> "etc/smbios/smbios-tables".  Only machine types for 2.1 + newer will
> provide them, so with older machine types seabios will continue to
> generate the smbios tables and guest wouldn't notice a difference.

This does mean that SeaBIOS would need to continue to support
generating of smbios tables for the foreseeable future as it would
need that support whenever one wanted to run a v2.0 or earlier machine
type.  It would be nice if a future version of SeaBIOS could drop the
smbios generation code.

However, I'm okay with either direction.

-Kevin

[Qemu-devel] [QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Markus Armbruster]

"Kevin O'Connor"  writes:
> On Tue, Apr 15, 2014 at 10:29:26AM +0200, Gerd Hoffmann wrote:
> > Leave the old interface code basically as-is.  type0 and type1
> > individual fields are passed like they are passed today.  We don't
> > change to to pass full tables, and we don't extend that to new table
> > types.  Continue to provide these in parallel to the new interface, for
> > compatibility with old firmware (and old machine types).
> >
> > The code to generate complete tables will only be used for
> > "etc/smbios/smbios-tables".  Only machine types for 2.1 + newer will
> > provide them, so with older machine types seabios will continue to
> > generate the smbios tables and guest wouldn't notice a difference.
>
> This does mean that SeaBIOS would need to continue to support
> generating of smbios tables for the foreseeable future as it would
> need that support whenever one wanted to run a v2.0 or earlier machine
> type.  It would be nice if a future version of SeaBIOS could drop the
> smbios generation code.
>
> However, I'm okay with either direction.

If we want to retire the SMBIOS tables generation from SeaBIOS entirely,
we first need to stop use of the old interface in QEMU, then let enough
time pass to avoid awkward upgrade dependencies.

Gerd's proposal is compatible with that.

[QEMU v6 PATCH 00/17] SMBIOS: build full tables in QEMU

[Gerd Hoffmann]

On Di, 2014-04-15 at 20:47 -0400, Kevin O'Connor wrote:
> On Tue, Apr 15, 2014 at 10:29:26AM +0200, Gerd Hoffmann wrote:
> > Leave the old interface code basically as-is.  type0 and type1
> > individual fields are passed like they are passed today.  We don't
> > change to to pass full tables, and we don't extend that to new table
> > types.  Continue to provide these in parallel to the new interface, for
> > compatibility with old firmware (and old machine types).
> >
> > The code to generate complete tables will only be used for
> > "etc/smbios/smbios-tables".  Only machine types for 2.1 + newer will
> > provide them, so with older machine types seabios will continue to
> > generate the smbios tables and guest wouldn't notice a difference.
>
> This does mean that SeaBIOS would need to continue to support
> generating of smbios tables for the foreseeable future as it would
> need that support whenever one wanted to run a v2.0 or earlier machine
> type.  It would be nice if a future version of SeaBIOS could drop the
> smbios generation code.
>
> However, I'm okay with either direction.

Same situation as with acpi, where qemu generating the tables is limited
to new machine types too.  So we'll continue to need the acpi/smbios
generation code.  Maintenance burden should be low as there is little
reason to ever touch that code as any new development happens in qemu.
We can add config options to disable it, so it is possible to reduce the
memory footprint in case you know you don't need it.

cheers,
  Gerd