[Qemu-devel] [ARM SMBIOS V4 PATCH 0/2] SMBIOS Support for ARM

[Qemu-devel] [ARM SMBIOS V4 PATCH 0/2] SMBIOS Support for ARM

[Wei Huang]

SMBIOS tables present userful system hardware info to management
applications, such as DMI tools. Even though SMBIOS was originally
developed for Intel x86, it has been extended to both Itanium and 
ARM (32bit & 64bit). More and more ARM server releases, such as 
RHEL Server for ARM, start to integrate support for SMBIOS.

This patchset is intendted to provid SMBIOS tables for ARM mach-virt
machine. The SMBIOS tables are created and stored in fw_cfg, relying on
OVMF (AAVMF) to parse/present SMBIOS entry.

Given that refractoring patches have been accepted by mst. This new
version (V4) integrates SMBIOS 3.0 support for ARM mach-virt. I have
tested this version using a customized AAVMF created by Laszlo, who
has submitted his patches to OVMF mailing list.

V3->V4:
 * Patch 1 - 3 accepted by mst tree. So start from Patch 4 & 5
 * Remove ep_length; Instead infer anchor_length from anchor string (mst)

V2->V3:
 * Removed unncessary ram_size paramemter (patch 3 in V2, Laszlo)
 * Fixed UUID encode (Laszlo)
 * Added -smbios option (Leif)
 * Fixed misc variable defintion (Laszlo)
 * V2 regression tested on x86 (Gabriel and Leif)

V1->V2:
 * Add NULL checking for fw_cfg (Shannon Zhao)
 * Init 3.0 entry point table max size to smbios_tables_len (Laszlo)
 * Minor re-arrangement of smbios.h layout with function headers to the bottom
 * Validated SMBIOS 3.0 tables with a customized AAVMF created by Laszlo

RFC->V1:
 * Add SMBIOS 3.0 support for buidling SMBIOS
 * Switch from SMBIOS 2.1 to 3.0 for ARM mach-virt
 * RFC version Tested-by Laszlo Ersek and Acked-by Gabriel Somlo

Thanks,
-Wei

Wei Huang (2):
  smbios: add smbios 3.0 support
  smbios: implement smbios support for mach-virt

 default-configs/arm-softmmu.mak |  1 +
 hw/arm/virt.c                   | 25 ++++++++++++
 hw/i386/pc_piix.c               |  3 +-
 hw/i386/pc_q35.c                |  3 +-
 hw/smbios/smbios.c              | 84 ++++++++++++++++++++++++++++++-----------
 include/hw/smbios/smbios.h      | 51 ++++++++++++++++++-------
 qemu-options.hx                 |  2 +-
 7 files changed, 129 insertions(+), 40 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [ARM SMBIOS V4 PATCH 1/2] smbios: add smbios 3.0 support

[Wei Huang]

This patch adds support for SMBIOS 3.0 entry point. When caller invokes
smbios_set_defaults(), it can specify entry point as 2.1 or 3.0. Then
smbios_get_tables() will return the entry point table in right format.

Acked-by: Gabriel Somlo <somlo@cmu.edu>
Tested-by: Gabriel Somlo <somlo@cmu.edu>
Tested-by: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Wei Huang <wei@redhat.com>
---
 hw/i386/pc_piix.c          |  3 +-
 hw/i386/pc_q35.c           |  3 +-
 hw/smbios/smbios.c         | 84 +++++++++++++++++++++++++++++++++-------------
 include/hw/smbios/smbios.h | 51 ++++++++++++++++++++--------
 4 files changed, 102 insertions(+), 39 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9558467..b82921d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -173,7 +173,8 @@ static void pc_init1(MachineState *machine)
         MachineClass *mc = MACHINE_GET_CLASS(machine);
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
-                            mc->name, smbios_legacy_mode, smbios_uuid_encoded);
+                            mc->name, smbios_legacy_mode, smbios_uuid_encoded,
+                            SMBIOS_ENTRY_POINT_21);
     }
 
     /* allocate ram and load rom/bios */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c07d65b..7217cbf 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -165,7 +165,8 @@ static void pc_q35_init(MachineState *machine)
     if (smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
-                            mc->name, smbios_legacy_mode, smbios_uuid_encoded);
+                            mc->name, smbios_legacy_mode, smbios_uuid_encoded,
+                            SMBIOS_ENTRY_POINT_21);
     }
 
     /* allocate ram and load rom/bios */
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index efdbb5d..b81a1d3 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -55,7 +55,9 @@ static uint8_t *smbios_tables;
 static size_t smbios_tables_len;
 static unsigned smbios_table_max;
 static unsigned smbios_table_cnt;
-static struct smbios_entry_point ep;
+static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21;
+
+static SmbiosEntryPoint ep;
 
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
@@ -771,11 +773,12 @@ void smbios_set_cpuid(uint32_t version, uint32_t features)
 
 void smbios_set_defaults(const char *manufacturer, const char *product,
                          const char *version, bool legacy_mode,
-                         bool uuid_encoded)
+                         bool uuid_encoded, SmbiosEntryPointType ep_type)
 {
     smbios_have_defaults = true;
     smbios_legacy = legacy_mode;
     smbios_uuid_encoded = uuid_encoded;
+    smbios_ep_type = ep_type;
 
     /* drop unwanted version of command-line file blob(s) */
     if (smbios_legacy) {
@@ -808,26 +811,53 @@ void smbios_set_defaults(const char *manufacturer, const char *product,
 
 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 = cpu_to_le16(smbios_tables_len);
-    ep.max_structure_size = cpu_to_le16(smbios_table_max);
-    ep.number_of_structures = cpu_to_le16(smbios_table_cnt);
-
-    /* BIOS must recalculate: */
-    ep.checksum = 0;
-    ep.intermediate_checksum = 0;
-    ep.structure_table_address = cpu_to_le32(0);
+    switch (smbios_ep_type) {
+    case SMBIOS_ENTRY_POINT_21:
+        memcpy(ep.ep21.anchor_string, "_SM_", 4);
+        memcpy(ep.ep21.intermediate_anchor_string, "_DMI_", 5);
+        ep.ep21.length = sizeof(struct smbios_21_entry_point);
+        ep.ep21.entry_point_revision = 0; /* formatted_area reserved */
+        memset(ep.ep21.formatted_area, 0, 5);
+
+        /* compliant with smbios spec v2.8 */
+        ep.ep21.smbios_major_version = 2;
+        ep.ep21.smbios_minor_version = 8;
+        ep.ep21.smbios_bcd_revision = 0x28;
+
+        /* set during table construction, but BIOS may override: */
+        ep.ep21.structure_table_length = cpu_to_le16(smbios_tables_len);
+        ep.ep21.max_structure_size = cpu_to_le16(smbios_table_max);
+        ep.ep21.number_of_structures = cpu_to_le16(smbios_table_cnt);
+
+        /* BIOS must recalculate */
+        ep.ep21.checksum = 0;
+        ep.ep21.intermediate_checksum = 0;
+        ep.ep21.structure_table_address = cpu_to_le32(0);
+
+        break;
+    case SMBIOS_ENTRY_POINT_30:
+        memcpy(ep.ep30.anchor_string, "_SM3_", 5);
+        ep.ep30.length = sizeof(struct smbios_30_entry_point);
+        ep.ep30.entry_point_revision = 1;
+        ep.ep30.reserved = 0;
+
+        /* compliant with smbios spec 3.0 */
+        ep.ep30.smbios_major_version = 3;
+        ep.ep30.smbios_minor_version = 0;
+        ep.ep30.smbios_doc_rev = 0;
+
+        /* set during table construct, but BIOS might override */
+        ep.ep30.structure_table_max_size = cpu_to_le32(smbios_tables_len);
+
+        /* BIOS must recalculate */
+        ep.ep30.checksum = 0;
+        ep.ep30.structure_table_address = cpu_to_le64(0);
+
+        break;
+    default:
+        abort();
+        break;
+    }
 }
 
 void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
@@ -885,7 +915,15 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
     *tables = smbios_tables;
     *tables_len = smbios_tables_len;
     *anchor = (uint8_t *)&ep;
-    *anchor_len = sizeof(struct smbios_entry_point);
+
+    /* calculate length based on anchor string */
+    if (!strncmp((char *)&ep, "_SM_", 4)) {
+        *anchor_len = sizeof(struct smbios_21_entry_point);
+    } else if (!strncmp((char *)&ep, "_SM3_", 5)) {
+        *anchor_len = sizeof(struct smbios_30_entry_point);
+    } else {
+        abort();
+    }
 }
 
 static void save_opt(const char **dest, QemuOpts *opts, const char *name)
diff --git a/include/hw/smbios/smbios.h b/include/hw/smbios/smbios.h
index 4269aab..7d999cd 100644
--- a/include/hw/smbios/smbios.h
+++ b/include/hw/smbios/smbios.h
@@ -23,25 +23,19 @@ struct smbios_phys_mem_area {
     uint64_t length;
 };
 
-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, bool legacy_mode,
-                         bool uuid_encoded);
-uint8_t *smbios_get_table_legacy(size_t *length);
-void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
-                       const unsigned int mem_array_size,
-                       uint8_t **tables, size_t *tables_len,
-                       uint8_t **anchor, size_t *anchor_len);
-
 /*
  * SMBIOS spec defined tables
  */
+typedef enum SmbiosEntryPointType {
+    SMBIOS_ENTRY_POINT_21,
+    SMBIOS_ENTRY_POINT_30,
+} SmbiosEntryPointType;
 
-/* SMBIOS entry point (anchor).
- * BIOS must place this at a 16-bit-aligned address between 0xf0000 and 0xfffff.
+/* SMBIOS entry point
+ * BIOS must place this at a 16-bit-aligned address between 0xf0000
+ * and 0xfffff.
  */
-struct smbios_entry_point {
+struct smbios_21_entry_point {
     uint8_t anchor_string[4];
     uint8_t checksum;
     uint8_t length;
@@ -58,6 +52,25 @@ struct smbios_entry_point {
     uint8_t smbios_bcd_revision;
 } QEMU_PACKED;
 
+/* SMBIOS 3.0 entry point */
+struct smbios_30_entry_point {
+    uint8_t anchor_string[5];
+    uint8_t checksum;
+    uint8_t length;
+    uint8_t smbios_major_version;
+    uint8_t smbios_minor_version;
+    uint8_t smbios_doc_rev;
+    uint8_t entry_point_revision;
+    uint8_t reserved;
+    uint32_t structure_table_max_size;
+    uint64_t structure_table_address;
+} QEMU_PACKED;
+
+typedef union {
+    struct smbios_21_entry_point ep21;
+    struct smbios_30_entry_point ep30;
+} QEMU_PACKED SmbiosEntryPoint;
+
 /* This goes at the beginning of every SMBIOS structure. */
 struct smbios_structure_header {
     uint8_t type;
@@ -232,4 +245,14 @@ struct smbios_type_127 {
     struct smbios_structure_header header;
 } QEMU_PACKED;
 
+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, bool legacy_mode,
+                         bool uuid_encoded, SmbiosEntryPointType ep_type);
+uint8_t *smbios_get_table_legacy(size_t *length);
+void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
+                       const unsigned int mem_array_size,
+                       uint8_t **tables, size_t *tables_len,
+                       uint8_t **anchor, size_t *anchor_len);
 #endif /*QEMU_SMBIOS_H */
-- 
1.8.3.1

Re: [Qemu-devel] [ARM SMBIOS V4 PATCH 1/2] smbios: add smbios 3.0 support

[Laszlo Ersek]

On 08/13/15 19:09, Wei Huang wrote:
> This patch adds support for SMBIOS 3.0 entry point. When caller invokes
> smbios_set_defaults(), it can specify entry point as 2.1 or 3.0. Then
> smbios_get_tables() will return the entry point table in right format.
> 
> Acked-by: Gabriel Somlo <somlo@cmu.edu>
> Tested-by: Gabriel Somlo <somlo@cmu.edu>
> Tested-by: Leif Lindholm <leif.lindholm@linaro.org>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  hw/i386/pc_piix.c          |  3 +-
>  hw/i386/pc_q35.c           |  3 +-
>  hw/smbios/smbios.c         | 84 +++++++++++++++++++++++++++++++++-------------
>  include/hw/smbios/smbios.h | 51 ++++++++++++++++++++--------
>  4 files changed, 102 insertions(+), 39 deletions(-)

Diffed it against the corresponding v3 patch; I'm OK with this version too.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [Qemu-devel] [ARM SMBIOS V4 PATCH 1/2] smbios: add smbios 3.0 support

[Peter Maydell]

On 13 August 2015 at 18:09, Wei Huang <wei@redhat.com> wrote:
> This patch adds support for SMBIOS 3.0 entry point. When caller invokes
> smbios_set_defaults(), it can specify entry point as 2.1 or 3.0. Then
> smbios_get_tables() will return the entry point table in right format.


> -/* SMBIOS entry point (anchor).
> - * BIOS must place this at a 16-bit-aligned address between 0xf0000 and 0xfffff.
> +/* SMBIOS entry point
> + * BIOS must place this at a 16-bit-aligned address between 0xf0000
> + * and 0xfffff.
>   */
> -struct smbios_entry_point {
> +struct smbios_21_entry_point {
>      uint8_t anchor_string[4];
>      uint8_t checksum;
>      uint8_t length;
> @@ -58,6 +52,25 @@ struct smbios_entry_point {
>      uint8_t smbios_bcd_revision;
>  } QEMU_PACKED;

This breaks 'make check' for x86, because tests/bios-tables-test.c
still uses 'struct smbios_entry_point' and no longer compiles
if this patch is applied.

I'm removing these two patches from my target-arm queue.

thanks
-- PMM

Re: [Qemu-devel] [ARM SMBIOS V4 PATCH 1/2] smbios: add smbios 3.0 support

[Leif Lindholm]

On Tue, Aug 25, 2015 at 04:17:42PM +0100, Peter Maydell wrote:
> On 13 August 2015 at 18:09, Wei Huang <wei@redhat.com> wrote:
> > This patch adds support for SMBIOS 3.0 entry point. When caller invokes
> > smbios_set_defaults(), it can specify entry point as 2.1 or 3.0. Then
> > smbios_get_tables() will return the entry point table in right format.
> 
> 
> > -/* SMBIOS entry point (anchor).
> > - * BIOS must place this at a 16-bit-aligned address between 0xf0000 and 0xfffff.
> > +/* SMBIOS entry point
> > + * BIOS must place this at a 16-bit-aligned address between 0xf0000
> > + * and 0xfffff.
> >   */
> > -struct smbios_entry_point {
> > +struct smbios_21_entry_point {
> >      uint8_t anchor_string[4];
> >      uint8_t checksum;
> >      uint8_t length;
> > @@ -58,6 +52,25 @@ struct smbios_entry_point {
> >      uint8_t smbios_bcd_revision;
> >  } QEMU_PACKED;
> 
> This breaks 'make check' for x86, because tests/bios-tables-test.c
> still uses 'struct smbios_entry_point' and no longer compiles
> if this patch is applied.

Urgh.

> I'm removing these two patches from my target-arm queue.

Fair enough.

Wei - is there actually any particular point in renaming this
structure? In all versions of the specification before 3.0, this was
only known as the "smbios entry point". Only with the introduction of
SMBIOS 3.0 this was retrospectively renamed.

(And personally, I find that renaming a bit counterintuitive, since it
is still a valid 32-bit entry point in SMBIOS3, and was the only entry
point up until and including SMBIOS 2.8.)

/
    Leif

Re: [Qemu-devel] [ARM SMBIOS V4 PATCH 1/2] smbios: add smbios 3.0 support

[Wei Huang]

On 08/25/2015 10:29 AM, Leif Lindholm wrote:
> On Tue, Aug 25, 2015 at 04:17:42PM +0100, Peter Maydell wrote:
>> On 13 August 2015 at 18:09, Wei Huang <wei@redhat.com> wrote:
>>> This patch adds support for SMBIOS 3.0 entry point. When caller invokes
>>> smbios_set_defaults(), it can specify entry point as 2.1 or 3.0. Then
>>> smbios_get_tables() will return the entry point table in right format.
>>
>>
>>> -/* SMBIOS entry point (anchor).
>>> - * BIOS must place this at a 16-bit-aligned address between 0xf0000 and 0xfffff.
>>> +/* SMBIOS entry point
>>> + * BIOS must place this at a 16-bit-aligned address between 0xf0000
>>> + * and 0xfffff.
>>>   */
>>> -struct smbios_entry_point {
>>> +struct smbios_21_entry_point {
>>>      uint8_t anchor_string[4];
>>>      uint8_t checksum;
>>>      uint8_t length;
>>> @@ -58,6 +52,25 @@ struct smbios_entry_point {
>>>      uint8_t smbios_bcd_revision;
>>>  } QEMU_PACKED;
>>
>> This breaks 'make check' for x86, because tests/bios-tables-test.c
>> still uses 'struct smbios_entry_point' and no longer compiles
>> if this patch is applied.
> 
> Urgh.
> 
>> I'm removing these two patches from my target-arm queue.
> 
> Fair enough.
> 
> Wei - is there actually any particular point in renaming this
> structure? In all versions of the specification before 3.0, this was
> only known as the "smbios entry point". Only with the introduction of
> SMBIOS 3.0 this was retrospectively renamed.

I can take this suggestion, with clear comment in header file so nobody
will get confused. Peter, please let me know if you object.

The new patches should be out in a short while.

Thanks,
-Wei

> 
> (And personally, I find that renaming a bit counterintuitive, since it
> is still a valid 32-bit entry point in SMBIOS3, and was the only entry
> point up until and including SMBIOS 2.8.)


> 
> /
>     Leif
> 

Re: [Qemu-devel] [ARM SMBIOS V4 PATCH 1/2] smbios: add smbios 3.0 support

[Peter Maydell]

On 25 August 2015 at 16:59, Wei Huang <wei@redhat.com> wrote:
> On 08/25/2015 10:29 AM, Leif Lindholm wrote:
>> Wei - is there actually any particular point in renaming this
>> structure? In all versions of the specification before 3.0, this was
>> only known as the "smbios entry point". Only with the introduction of
>> SMBIOS 3.0 this was retrospectively renamed.
>
> I can take this suggestion, with clear comment in header file so nobody
> will get confused. Peter, please let me know if you object.

I don't object (though the opinion of the qemu smbios/acpi
folk is probably more important than mine).

Please make sure you test the x86 platform has not been broken by
this change (preferably more thoroughly than just running
'make check'...).

thanks
-- PMM

Re: [Qemu-devel] [ARM SMBIOS V4 PATCH 1/2] smbios: add smbios 3.0 support

[Wei Huang]

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

On 08/25/2015 11:01 AM, Peter Maydell wrote:
> On 25 August 2015 at 16:59, Wei Huang <wei@redhat.com> wrote:
>> On 08/25/2015 10:29 AM, Leif Lindholm wrote:
>>> Wei - is there actually any particular point in renaming this
>>> structure? In all versions of the specification before 3.0, this was
>>> only known as the "smbios entry point". Only with the introduction of
>>> SMBIOS 3.0 this was retrospectively renamed.

Hi Leif,

While fixing up the patches, I started to lean towards keeping -21 in
smbios-entry-point naming. Here are my points:

1) It is very easy to get confused the "union SmbiosEntryPoint" with
"struct smbios_entry_point" if we remove _21 from the original "struct
smbios_21_entry_point". A symmetric naming of "struct
smbios_21_entry_point" & "struct smbios_30_entry_point" actually is
easier to read;
2) SMBIOS 3.0 specification clearly separate 2.1 and 3.0 definitions. I
understand that people who started from old spec might be a bit confused
to read the code; But for those who pick up SMBIOS 3.0 (and beyond)
fresh, they will know the difference while reading the code.
3) The issues Peter found is x86 specific and we know that QEMU only
generates SMBIOS 2.1 tables for PC model. So we can fix the test code to
only use _21 struct. In the future, when we started to create test code
for ARM, it can be further merged.

Anyway, I attached the patch for your review. It addresses some of your
concerns regarding SMBIOS 2.1 usage cases by detailed comments (see
smbios.h).

>>
>> I can take this suggestion, with clear comment in header file so nobody
>> will get confused. Peter, please let me know if you object.
> 
> I don't object (though the opinion of the qemu smbios/acpi
> folk is probably more important than mine).
> 
> Please make sure you test the x86 platform has not been broken by
> this change (preferably more thoroughly than just running
> 'make check'...).

(I have tested the code with the following methods:

* dmidecode insider an ARM guest VM
* "make check" on both ARM and x86 hosts
* dmidecode insider a x86-64 Windows 7 VM
)


> 
> thanks
> -- PMM
> 


[-- Attachment #2: smbios_v5_patch1.patch --]
[-- Type: text/x-patch, Size: 10914 bytes --]

commit 4abc639a34a43acccd81725f2176760058bb9849
Author: Wei Huang <wei@redhat.com>
Date:   Tue Aug 25 16:34:47 2015 -0400

    smbios: add smbios 3.0 support
    
    This patch adds support for SMBIOS 3.0 entry point. When caller invokes
    smbios_set_defaults(), it can specify entry point as 2.1 or 3.0. Then
    smbios_get_tables() will return the entry point table in right format.
    
    Acked-by: Gabriel Somlo <somlo@cmu.edu>
    Tested-by: Gabriel Somlo <somlo@cmu.edu>
    Tested-by: Leif Lindholm <leif.lindholm@linaro.org>
    Signed-off-by: Wei Huang <wei@redhat.com>

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9558467..b82921d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -173,7 +173,8 @@ static void pc_init1(MachineState *machine)
         MachineClass *mc = MACHINE_GET_CLASS(machine);
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
-                            mc->name, smbios_legacy_mode, smbios_uuid_encoded);
+                            mc->name, smbios_legacy_mode, smbios_uuid_encoded,
+                            SMBIOS_ENTRY_POINT_21);
     }
 
     /* allocate ram and load rom/bios */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c07d65b..7217cbf 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -165,7 +165,8 @@ static void pc_q35_init(MachineState *machine)
     if (smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
-                            mc->name, smbios_legacy_mode, smbios_uuid_encoded);
+                            mc->name, smbios_legacy_mode, smbios_uuid_encoded,
+                            SMBIOS_ENTRY_POINT_21);
     }
 
     /* allocate ram and load rom/bios */
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index efdbb5d..b81a1d3 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -55,7 +55,9 @@ static uint8_t *smbios_tables;
 static size_t smbios_tables_len;
 static unsigned smbios_table_max;
 static unsigned smbios_table_cnt;
-static struct smbios_entry_point ep;
+static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21;
+
+static SmbiosEntryPoint ep;
 
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
@@ -771,11 +773,12 @@ void smbios_set_cpuid(uint32_t version, uint32_t features)
 
 void smbios_set_defaults(const char *manufacturer, const char *product,
                          const char *version, bool legacy_mode,
-                         bool uuid_encoded)
+                         bool uuid_encoded, SmbiosEntryPointType ep_type)
 {
     smbios_have_defaults = true;
     smbios_legacy = legacy_mode;
     smbios_uuid_encoded = uuid_encoded;
+    smbios_ep_type = ep_type;
 
     /* drop unwanted version of command-line file blob(s) */
     if (smbios_legacy) {
@@ -808,26 +811,53 @@ void smbios_set_defaults(const char *manufacturer, const char *product,
 
 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 = cpu_to_le16(smbios_tables_len);
-    ep.max_structure_size = cpu_to_le16(smbios_table_max);
-    ep.number_of_structures = cpu_to_le16(smbios_table_cnt);
-
-    /* BIOS must recalculate: */
-    ep.checksum = 0;
-    ep.intermediate_checksum = 0;
-    ep.structure_table_address = cpu_to_le32(0);
+    switch (smbios_ep_type) {
+    case SMBIOS_ENTRY_POINT_21:
+        memcpy(ep.ep21.anchor_string, "_SM_", 4);
+        memcpy(ep.ep21.intermediate_anchor_string, "_DMI_", 5);
+        ep.ep21.length = sizeof(struct smbios_21_entry_point);
+        ep.ep21.entry_point_revision = 0; /* formatted_area reserved */
+        memset(ep.ep21.formatted_area, 0, 5);
+
+        /* compliant with smbios spec v2.8 */
+        ep.ep21.smbios_major_version = 2;
+        ep.ep21.smbios_minor_version = 8;
+        ep.ep21.smbios_bcd_revision = 0x28;
+
+        /* set during table construction, but BIOS may override: */
+        ep.ep21.structure_table_length = cpu_to_le16(smbios_tables_len);
+        ep.ep21.max_structure_size = cpu_to_le16(smbios_table_max);
+        ep.ep21.number_of_structures = cpu_to_le16(smbios_table_cnt);
+
+        /* BIOS must recalculate */
+        ep.ep21.checksum = 0;
+        ep.ep21.intermediate_checksum = 0;
+        ep.ep21.structure_table_address = cpu_to_le32(0);
+
+        break;
+    case SMBIOS_ENTRY_POINT_30:
+        memcpy(ep.ep30.anchor_string, "_SM3_", 5);
+        ep.ep30.length = sizeof(struct smbios_30_entry_point);
+        ep.ep30.entry_point_revision = 1;
+        ep.ep30.reserved = 0;
+
+        /* compliant with smbios spec 3.0 */
+        ep.ep30.smbios_major_version = 3;
+        ep.ep30.smbios_minor_version = 0;
+        ep.ep30.smbios_doc_rev = 0;
+
+        /* set during table construct, but BIOS might override */
+        ep.ep30.structure_table_max_size = cpu_to_le32(smbios_tables_len);
+
+        /* BIOS must recalculate */
+        ep.ep30.checksum = 0;
+        ep.ep30.structure_table_address = cpu_to_le64(0);
+
+        break;
+    default:
+        abort();
+        break;
+    }
 }
 
 void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
@@ -885,7 +915,15 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
     *tables = smbios_tables;
     *tables_len = smbios_tables_len;
     *anchor = (uint8_t *)&ep;
-    *anchor_len = sizeof(struct smbios_entry_point);
+
+    /* calculate length based on anchor string */
+    if (!strncmp((char *)&ep, "_SM_", 4)) {
+        *anchor_len = sizeof(struct smbios_21_entry_point);
+    } else if (!strncmp((char *)&ep, "_SM3_", 5)) {
+        *anchor_len = sizeof(struct smbios_30_entry_point);
+    } else {
+        abort();
+    }
 }
 
 static void save_opt(const char **dest, QemuOpts *opts, const char *name)
diff --git a/include/hw/smbios/smbios.h b/include/hw/smbios/smbios.h
index 4269aab..76ccf70 100644
--- a/include/hw/smbios/smbios.h
+++ b/include/hw/smbios/smbios.h
@@ -23,25 +23,27 @@ struct smbios_phys_mem_area {
     uint64_t length;
 };
 
-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, bool legacy_mode,
-                         bool uuid_encoded);
-uint8_t *smbios_get_table_legacy(size_t *length);
-void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
-                       const unsigned int mem_array_size,
-                       uint8_t **tables, size_t *tables_len,
-                       uint8_t **anchor, size_t *anchor_len);
-
 /*
  * SMBIOS spec defined tables
  */
+typedef enum SmbiosEntryPointType {
+    SMBIOS_ENTRY_POINT_21,
+    SMBIOS_ENTRY_POINT_30,
+} SmbiosEntryPointType;
+
+/* SMBIOS Entry Point
+ * There are two types of entry points defined in the SMBIOS specification
+ * (see below). BIOS must place the entry point(s) at a 16-bit-aligned
+ * address between 0xf0000 and 0xfffff. Note that either entry point type
+ * can be used in a 64-bit target system, except that SMBIOS 2.1 entry point
+ * only allows the SMBIOS struct table to reside below 4GB address space.
+ */
 
-/* SMBIOS entry point (anchor).
- * BIOS must place this at a 16-bit-aligned address between 0xf0000 and 0xfffff.
+/* SMBIOS 2.1 (32-bit) Entry Point
+ *  - introduced since SMBIOS 2.1
+ *  - supports structure table below 4GB only
  */
-struct smbios_entry_point {
+struct smbios_21_entry_point {
     uint8_t anchor_string[4];
     uint8_t checksum;
     uint8_t length;
@@ -58,6 +60,28 @@ struct smbios_entry_point {
     uint8_t smbios_bcd_revision;
 } QEMU_PACKED;
 
+/* SMBIOS 3.0 (64-bit) Entry Point
+ *  - introduced since SMBIOS 3.0
+ *  - supports structure table at 64-bit address space
+ */
+struct smbios_30_entry_point {
+    uint8_t anchor_string[5];
+    uint8_t checksum;
+    uint8_t length;
+    uint8_t smbios_major_version;
+    uint8_t smbios_minor_version;
+    uint8_t smbios_doc_rev;
+    uint8_t entry_point_revision;
+    uint8_t reserved;
+    uint32_t structure_table_max_size;
+    uint64_t structure_table_address;
+} QEMU_PACKED;
+
+typedef union {
+    struct smbios_21_entry_point ep21;
+    struct smbios_30_entry_point ep30;
+} QEMU_PACKED SmbiosEntryPoint;
+
 /* This goes at the beginning of every SMBIOS structure. */
 struct smbios_structure_header {
     uint8_t type;
@@ -232,4 +256,14 @@ struct smbios_type_127 {
     struct smbios_structure_header header;
 } QEMU_PACKED;
 
+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, bool legacy_mode,
+                         bool uuid_encoded, SmbiosEntryPointType ep_type);
+uint8_t *smbios_get_table_legacy(size_t *length);
+void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
+                       const unsigned int mem_array_size,
+                       uint8_t **tables, size_t *tables_len,
+                       uint8_t **anchor, size_t *anchor_len);
 #endif /*QEMU_SMBIOS_H */
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 613867a..9686328 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -50,7 +50,7 @@ typedef struct {
     int rsdt_tables_nr;
     GArray *tables;
     uint32_t smbios_ep_addr;
-    struct smbios_entry_point smbios_ep_table;
+    struct smbios_21_entry_point smbios_ep_table;
 } test_data;
 
 #define LOW(x) ((x) & 0xff)
@@ -601,7 +601,7 @@ static void test_acpi_asl(test_data *data)
 
 static bool smbios_ep_table_ok(test_data *data)
 {
-    struct smbios_entry_point *ep_table = &data->smbios_ep_table;
+    struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
     uint32_t addr = data->smbios_ep_addr;
 
     ACPI_READ_ARRAY(ep_table->anchor_string, addr);
@@ -681,7 +681,7 @@ static inline bool smbios_single_instance(uint8_t type)
 static void test_smbios_structs(test_data *data)
 {
     DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
-    struct smbios_entry_point *ep_table = &data->smbios_ep_table;
+    struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
     uint32_t addr = ep_table->structure_table_address;
     int i, len, max_len = 0;
     uint8_t type, prv, crt;

Re: [Qemu-devel] [ARM SMBIOS V4 PATCH 1/2] smbios: add smbios 3.0 support

[Leif Lindholm]

Hi Wei,

On Wed, Aug 26, 2015 at 11:41:26AM -0500, Wei Huang wrote:
> While fixing up the patches, I started to lean towards keeping -21 in
> smbios-entry-point naming. Here are my points:
> 
> 1) It is very easy to get confused the "union SmbiosEntryPoint" with
> "struct smbios_entry_point" if we remove _21 from the original "struct
> smbios_21_entry_point". A symmetric naming of "struct
> smbios_21_entry_point" & "struct smbios_30_entry_point" actually is
> easier to read;
> 2) SMBIOS 3.0 specification clearly separate 2.1 and 3.0 definitions. I
> understand that people who started from old spec might be a bit confused
> to read the code; But for those who pick up SMBIOS 3.0 (and beyond)
> fresh, they will know the difference while reading the code.
> 3) The issues Peter found is x86 specific and we know that QEMU only
> generates SMBIOS 2.1 tables for PC model. So we can fix the test code to
> only use _21 struct. In the future, when we started to create test code
> for ARM, it can be further merged.
> 
> Anyway, I attached the patch for your review. It addresses some of your
> concerns regarding SMBIOS 2.1 usage cases by detailed comments (see
> smbios.h).

In general, I care a lot more about getting this functionality than
about the finer details of the inner workings of the code. The
comments are an improvement, and using -21 aligns the code closer with
current spec (but further from Linux).

Thanks,

Leif

> >> I can take this suggestion, with clear comment in header file so nobody
> >> will get confused. Peter, please let me know if you object.
> > 
> > I don't object (though the opinion of the qemu smbios/acpi
> > folk is probably more important than mine).
> > 
> > Please make sure you test the x86 platform has not been broken by
> > this change (preferably more thoroughly than just running
> > 'make check'...).
> 
> (I have tested the code with the following methods:
> 
> * dmidecode insider an ARM guest VM
> * "make check" on both ARM and x86 hosts
> * dmidecode insider a x86-64 Windows 7 VM
> )
> 
> 
> > 
> > thanks
> > -- PMM
> > 
> 
> commit 4abc639a34a43acccd81725f2176760058bb9849
> Author: Wei Huang <wei@redhat.com>
> Date:   Tue Aug 25 16:34:47 2015 -0400
> 
>     smbios: add smbios 3.0 support
>     
>     This patch adds support for SMBIOS 3.0 entry point. When caller invokes
>     smbios_set_defaults(), it can specify entry point as 2.1 or 3.0. Then
>     smbios_get_tables() will return the entry point table in right format.
>     
>     Acked-by: Gabriel Somlo <somlo@cmu.edu>
>     Tested-by: Gabriel Somlo <somlo@cmu.edu>
>     Tested-by: Leif Lindholm <leif.lindholm@linaro.org>
>     Signed-off-by: Wei Huang <wei@redhat.com>
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 9558467..b82921d 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -173,7 +173,8 @@ static void pc_init1(MachineState *machine)
>          MachineClass *mc = MACHINE_GET_CLASS(machine);
>          /* These values are guest ABI, do not change */
>          smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
> -                            mc->name, smbios_legacy_mode, smbios_uuid_encoded);
> +                            mc->name, smbios_legacy_mode, smbios_uuid_encoded,
> +                            SMBIOS_ENTRY_POINT_21);
>      }
>  
>      /* allocate ram and load rom/bios */
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index c07d65b..7217cbf 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -165,7 +165,8 @@ static void pc_q35_init(MachineState *machine)
>      if (smbios_defaults) {
>          /* These values are guest ABI, do not change */
>          smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
> -                            mc->name, smbios_legacy_mode, smbios_uuid_encoded);
> +                            mc->name, smbios_legacy_mode, smbios_uuid_encoded,
> +                            SMBIOS_ENTRY_POINT_21);
>      }
>  
>      /* allocate ram and load rom/bios */
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index efdbb5d..b81a1d3 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -55,7 +55,9 @@ static uint8_t *smbios_tables;
>  static size_t smbios_tables_len;
>  static unsigned smbios_table_max;
>  static unsigned smbios_table_cnt;
> -static struct smbios_entry_point ep;
> +static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21;
> +
> +static SmbiosEntryPoint ep;
>  
>  static int smbios_type4_count = 0;
>  static bool smbios_immutable;
> @@ -771,11 +773,12 @@ void smbios_set_cpuid(uint32_t version, uint32_t features)
>  
>  void smbios_set_defaults(const char *manufacturer, const char *product,
>                           const char *version, bool legacy_mode,
> -                         bool uuid_encoded)
> +                         bool uuid_encoded, SmbiosEntryPointType ep_type)
>  {
>      smbios_have_defaults = true;
>      smbios_legacy = legacy_mode;
>      smbios_uuid_encoded = uuid_encoded;
> +    smbios_ep_type = ep_type;
>  
>      /* drop unwanted version of command-line file blob(s) */
>      if (smbios_legacy) {
> @@ -808,26 +811,53 @@ void smbios_set_defaults(const char *manufacturer, const char *product,
>  
>  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 = cpu_to_le16(smbios_tables_len);
> -    ep.max_structure_size = cpu_to_le16(smbios_table_max);
> -    ep.number_of_structures = cpu_to_le16(smbios_table_cnt);
> -
> -    /* BIOS must recalculate: */
> -    ep.checksum = 0;
> -    ep.intermediate_checksum = 0;
> -    ep.structure_table_address = cpu_to_le32(0);
> +    switch (smbios_ep_type) {
> +    case SMBIOS_ENTRY_POINT_21:
> +        memcpy(ep.ep21.anchor_string, "_SM_", 4);
> +        memcpy(ep.ep21.intermediate_anchor_string, "_DMI_", 5);
> +        ep.ep21.length = sizeof(struct smbios_21_entry_point);
> +        ep.ep21.entry_point_revision = 0; /* formatted_area reserved */
> +        memset(ep.ep21.formatted_area, 0, 5);
> +
> +        /* compliant with smbios spec v2.8 */
> +        ep.ep21.smbios_major_version = 2;
> +        ep.ep21.smbios_minor_version = 8;
> +        ep.ep21.smbios_bcd_revision = 0x28;
> +
> +        /* set during table construction, but BIOS may override: */
> +        ep.ep21.structure_table_length = cpu_to_le16(smbios_tables_len);
> +        ep.ep21.max_structure_size = cpu_to_le16(smbios_table_max);
> +        ep.ep21.number_of_structures = cpu_to_le16(smbios_table_cnt);
> +
> +        /* BIOS must recalculate */
> +        ep.ep21.checksum = 0;
> +        ep.ep21.intermediate_checksum = 0;
> +        ep.ep21.structure_table_address = cpu_to_le32(0);
> +
> +        break;
> +    case SMBIOS_ENTRY_POINT_30:
> +        memcpy(ep.ep30.anchor_string, "_SM3_", 5);
> +        ep.ep30.length = sizeof(struct smbios_30_entry_point);
> +        ep.ep30.entry_point_revision = 1;
> +        ep.ep30.reserved = 0;
> +
> +        /* compliant with smbios spec 3.0 */
> +        ep.ep30.smbios_major_version = 3;
> +        ep.ep30.smbios_minor_version = 0;
> +        ep.ep30.smbios_doc_rev = 0;
> +
> +        /* set during table construct, but BIOS might override */
> +        ep.ep30.structure_table_max_size = cpu_to_le32(smbios_tables_len);
> +
> +        /* BIOS must recalculate */
> +        ep.ep30.checksum = 0;
> +        ep.ep30.structure_table_address = cpu_to_le64(0);
> +
> +        break;
> +    default:
> +        abort();
> +        break;
> +    }
>  }
>  
>  void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
> @@ -885,7 +915,15 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
>      *tables = smbios_tables;
>      *tables_len = smbios_tables_len;
>      *anchor = (uint8_t *)&ep;
> -    *anchor_len = sizeof(struct smbios_entry_point);
> +
> +    /* calculate length based on anchor string */
> +    if (!strncmp((char *)&ep, "_SM_", 4)) {
> +        *anchor_len = sizeof(struct smbios_21_entry_point);
> +    } else if (!strncmp((char *)&ep, "_SM3_", 5)) {
> +        *anchor_len = sizeof(struct smbios_30_entry_point);
> +    } else {
> +        abort();
> +    }
>  }
>  
>  static void save_opt(const char **dest, QemuOpts *opts, const char *name)
> diff --git a/include/hw/smbios/smbios.h b/include/hw/smbios/smbios.h
> index 4269aab..76ccf70 100644
> --- a/include/hw/smbios/smbios.h
> +++ b/include/hw/smbios/smbios.h
> @@ -23,25 +23,27 @@ struct smbios_phys_mem_area {
>      uint64_t length;
>  };
>  
> -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, bool legacy_mode,
> -                         bool uuid_encoded);
> -uint8_t *smbios_get_table_legacy(size_t *length);
> -void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
> -                       const unsigned int mem_array_size,
> -                       uint8_t **tables, size_t *tables_len,
> -                       uint8_t **anchor, size_t *anchor_len);
> -
>  /*
>   * SMBIOS spec defined tables
>   */
> +typedef enum SmbiosEntryPointType {
> +    SMBIOS_ENTRY_POINT_21,
> +    SMBIOS_ENTRY_POINT_30,
> +} SmbiosEntryPointType;
> +
> +/* SMBIOS Entry Point
> + * There are two types of entry points defined in the SMBIOS specification
> + * (see below). BIOS must place the entry point(s) at a 16-bit-aligned
> + * address between 0xf0000 and 0xfffff. Note that either entry point type
> + * can be used in a 64-bit target system, except that SMBIOS 2.1 entry point
> + * only allows the SMBIOS struct table to reside below 4GB address space.
> + */
>  
> -/* SMBIOS entry point (anchor).
> - * BIOS must place this at a 16-bit-aligned address between 0xf0000 and 0xfffff.
> +/* SMBIOS 2.1 (32-bit) Entry Point
> + *  - introduced since SMBIOS 2.1
> + *  - supports structure table below 4GB only
>   */
> -struct smbios_entry_point {
> +struct smbios_21_entry_point {
>      uint8_t anchor_string[4];
>      uint8_t checksum;
>      uint8_t length;
> @@ -58,6 +60,28 @@ struct smbios_entry_point {
>      uint8_t smbios_bcd_revision;
>  } QEMU_PACKED;
>  
> +/* SMBIOS 3.0 (64-bit) Entry Point
> + *  - introduced since SMBIOS 3.0
> + *  - supports structure table at 64-bit address space
> + */
> +struct smbios_30_entry_point {
> +    uint8_t anchor_string[5];
> +    uint8_t checksum;
> +    uint8_t length;
> +    uint8_t smbios_major_version;
> +    uint8_t smbios_minor_version;
> +    uint8_t smbios_doc_rev;
> +    uint8_t entry_point_revision;
> +    uint8_t reserved;
> +    uint32_t structure_table_max_size;
> +    uint64_t structure_table_address;
> +} QEMU_PACKED;
> +
> +typedef union {
> +    struct smbios_21_entry_point ep21;
> +    struct smbios_30_entry_point ep30;
> +} QEMU_PACKED SmbiosEntryPoint;
> +
>  /* This goes at the beginning of every SMBIOS structure. */
>  struct smbios_structure_header {
>      uint8_t type;
> @@ -232,4 +256,14 @@ struct smbios_type_127 {
>      struct smbios_structure_header header;
>  } QEMU_PACKED;
>  
> +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, bool legacy_mode,
> +                         bool uuid_encoded, SmbiosEntryPointType ep_type);
> +uint8_t *smbios_get_table_legacy(size_t *length);
> +void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
> +                       const unsigned int mem_array_size,
> +                       uint8_t **tables, size_t *tables_len,
> +                       uint8_t **anchor, size_t *anchor_len);
>  #endif /*QEMU_SMBIOS_H */
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 613867a..9686328 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -50,7 +50,7 @@ typedef struct {
>      int rsdt_tables_nr;
>      GArray *tables;
>      uint32_t smbios_ep_addr;
> -    struct smbios_entry_point smbios_ep_table;
> +    struct smbios_21_entry_point smbios_ep_table;
>  } test_data;
>  
>  #define LOW(x) ((x) & 0xff)
> @@ -601,7 +601,7 @@ static void test_acpi_asl(test_data *data)
>  
>  static bool smbios_ep_table_ok(test_data *data)
>  {
> -    struct smbios_entry_point *ep_table = &data->smbios_ep_table;
> +    struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
>      uint32_t addr = data->smbios_ep_addr;
>  
>      ACPI_READ_ARRAY(ep_table->anchor_string, addr);
> @@ -681,7 +681,7 @@ static inline bool smbios_single_instance(uint8_t type)
>  static void test_smbios_structs(test_data *data)
>  {
>      DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
> -    struct smbios_entry_point *ep_table = &data->smbios_ep_table;
> +    struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
>      uint32_t addr = ep_table->structure_table_address;
>      int i, len, max_len = 0;
>      uint8_t type, prv, crt;

[Qemu-devel] [ARM SMBIOS V4 PATCH 2/2] smbios: implement smbios support for mach-virt

[Wei Huang]

This patch generates smbios tables for ARM mach-virt. Also add
CONFIG_SMBIOS=y for ARM default config.

Acked-by: Gabriel Somlo <somlo@cmu.edu>
Tested-by: Gabriel Somlo <somlo@cmu.edu>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org>
Tested-by: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Wei Huang <wei@redhat.com>
---
 default-configs/arm-softmmu.mak |  1 +
 hw/arm/virt.c                   | 25 +++++++++++++++++++++++++
 qemu-options.hx                 |  2 +-
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 74f1db3..99b41e9 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -102,3 +102,4 @@ CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
 CONFIG_ACPI=y
+CONFIG_SMBIOS=y
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 4846892..b7c1822 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -48,6 +48,7 @@
 #include "hw/arm/sysbus-fdt.h"
 #include "hw/platform-bus.h"
 #include "hw/arm/fdt.h"
+#include "hw/smbios/smbios.h"
 
 /* Number of external interrupt lines to configure the GIC with */
 #define NUM_IRQS 256
@@ -780,12 +781,36 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
     return board->fdt;
 }
 
+static void virt_build_smbios(VirtGuestInfo *guest_info)
+{
+    FWCfgState *fw_cfg = guest_info->fw_cfg;
+    uint8_t *smbios_tables, *smbios_anchor;
+    size_t smbios_tables_len, smbios_anchor_len;
+
+    if (!fw_cfg)
+        return;
+
+    smbios_set_defaults("QEMU", "QEMU Virtual Machine",
+                        "1.0", false, true, SMBIOS_ENTRY_POINT_30);
+
+    smbios_get_tables(NULL, 0, &smbios_tables, &smbios_tables_len,
+                      &smbios_anchor, &smbios_anchor_len);
+
+    if (smbios_anchor) {
+        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);
+    }
+}
+
 static
 void virt_guest_info_machine_done(Notifier *notifier, void *data)
 {
     VirtGuestInfoState *guest_info_state = container_of(notifier,
                                               VirtGuestInfoState, machine_done);
     virt_acpi_setup(&guest_info_state->info);
+    virt_build_smbios(&guest_info_state->info);
 }
 
 static void machvirt_init(MachineState *machine)
diff --git a/qemu-options.hx b/qemu-options.hx
index 77f5853..efce775 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1412,7 +1412,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
     "-smbios type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n"
     "               [,asset=str][,part=str][,speed=%d]\n"
     "                specify SMBIOS type 17 fields\n",
-    QEMU_ARCH_I386)
+    QEMU_ARCH_I386 | QEMU_ARCH_ARM)
 STEXI
 @item -smbios file=@var{binary}
 @findex -smbios
-- 
1.8.3.1

Re: [Qemu-devel] [ARM SMBIOS V4 PATCH 0/2] SMBIOS Support for ARM

[Peter Maydell]

On 13 August 2015 at 18:09, Wei Huang <wei@redhat.com> wrote:
> SMBIOS tables present userful system hardware info to management
> applications, such as DMI tools. Even though SMBIOS was originally
> developed for Intel x86, it has been extended to both Itanium and
> ARM (32bit & 64bit). More and more ARM server releases, such as
> RHEL Server for ARM, start to integrate support for SMBIOS.
>
> This patchset is intendted to provid SMBIOS tables for ARM mach-virt
> machine. The SMBIOS tables are created and stored in fw_cfg, relying on
> OVMF (AAVMF) to parse/present SMBIOS entry.
>
> Given that refractoring patches have been accepted by mst. This new
> version (V4) integrates SMBIOS 3.0 support for ARM mach-virt. I have
> tested this version using a customized AAVMF created by Laszlo, who
> has submitted his patches to OVMF mailing list.

Applied to target-arm.next, thanks.

(It seemed to me like this should go through the ARM tree since
it's adding ARM support; let me know if anybody would rather
it go through a different tree or has a reason to hold off
applying it just now.)

-- PMM