[Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] 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.

RFC version have been tested by Laszlo using his customized version of
AAVMF. We were able to detect SMBIOS 2.8 tables using dmidecode inside
an AArch64 guest VM. Moving forward, it is better to support SMBIOS 3.0
for ARM guest VM. This new version (V1) integrates SMBIOS 3.0 support
for ARM mach-virt. I have tested this version by forcing SMBIOS 2.1
format (i.e. passing SMBIOS_21_ENTRY_POINT to smbios_set_defaults()).
SMBIOS 3.0 hasn't been tested yet as it requires AAVMF to install 3.0 entry.

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 (6):
  smbios: extract x86 smbios building code into a function
  smbios: remove dependency on x86 e820 tables
  smbios: pass ram size as a parameter to build smbios tables
  smbios: move smbios code into a common folder
  smbios: add smbios 3.0 support
  smbios: implement smbios support for mach-virt

 arch_init.c                          |  2 +-
 default-configs/arm-softmmu.mak      |  1 +
 default-configs/i386-softmmu.mak     |  1 +
 default-configs/x86_64-softmmu.mak   |  1 +
 hw/Makefile.objs                     |  1 +
 hw/arm/virt.c                        | 24 +++++++++
 hw/i386/Makefile.objs                |  2 +-
 hw/i386/pc.c                         | 56 ++++++++++++++-------
 hw/i386/pc_piix.c                    |  5 +-
 hw/i386/pc_q35.c                     |  5 +-
 hw/smbios/Makefile.objs              |  1 +
 hw/{i386 => smbios}/smbios.c         | 96 +++++++++++++++++++++++-------------
 include/hw/arm/virt-acpi-build.h     |  1 +
 include/hw/{i386 => smbios}/smbios.h | 42 ++++++++++++++--
 tests/bios-tables-test.c             |  2 +-
 vl.c                                 |  2 +-
 16 files changed, 179 insertions(+), 63 deletions(-)
 create mode 100644 hw/smbios/Makefile.objs
 rename hw/{i386 => smbios}/smbios.c (93%)
 rename include/hw/{i386 => smbios}/smbios.h (84%)

-- 
1.8.3.1

[Qemu-devel] [ARM SMBIOS V1 PATCH 1/6] smbios: extract x86 smbios building code into a function

[Wei Huang]

This patch extracts out the procedure of buidling x86 SMBIOS tables
into a dedicated function.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 hw/i386/pc.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7661ea9..00e45f3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -718,11 +718,30 @@ static unsigned int pc_apic_id_limit(unsigned int max_cpus)
     return x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
 }
 
-static FWCfgState *bochs_bios_init(void)
+static void pc_build_smbios(FWCfgState *fw_cfg)
 {
-    FWCfgState *fw_cfg;
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
+
+    smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
+    if (smbios_tables) {
+        fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
+                         smbios_tables, smbios_tables_len);
+    }
+
+    smbios_get_tables(&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 FWCfgState *bochs_bios_init(void)
+{
+    FWCfgState *fw_cfg;
     uint64_t *numa_fw_cfg;
     int i, j;
     unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
@@ -748,20 +767,7 @@ 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_tables = smbios_get_table_legacy(&smbios_tables_len);
-    if (smbios_tables) {
-        fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
-                         smbios_tables, smbios_tables_len);
-    }
-
-    smbios_get_tables(&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);
-    }
+    pc_build_smbios(fw_cfg);
 
     fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
                      &e820_reserve, sizeof(e820_reserve));
-- 
1.8.3.1

[Qemu-devel] [ARM SMBIOS V1 PATCH 2/6] smbios: remove dependency on x86 e820 tables

[Wei Huang]

Current smbios builds type 19 table from e820, which is x86 specific.
This patch removes smbios' dependency on e820 by passing an array
of memory area to smbios_get_tables().

Signed-off-by: Wei Huang <wei@redhat.com>
---
 hw/i386/pc.c             | 18 +++++++++++++++++-
 hw/i386/smbios.c         | 14 +++++++-------
 include/hw/i386/smbios.h | 10 +++++++++-
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 00e45f3..34e9133 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -722,6 +722,8 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
 {
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
+    struct smbios_phys_mem_area *mem_array;
+    unsigned i, array_count;
 
     smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
     if (smbios_tables) {
@@ -729,8 +731,22 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
                          smbios_tables, smbios_tables_len);
     }
 
-    smbios_get_tables(&smbios_tables, &smbios_tables_len,
+    /* build the array of physical mem area from e820 table */
+    mem_array = g_malloc0(sizeof(*mem_array) * e820_get_num_entries());
+    for (i = 0, array_count = 0; i < e820_get_num_entries(); i++) {
+        uint64_t addr, len;
+
+        if (e820_get_entry(i, E820_RAM, &addr, &len)) {
+            mem_array[array_count].address = addr;
+            mem_array[array_count].length = len;
+            array_count++;
+        }
+    }
+    smbios_get_tables(mem_array, array_count,
+                      &smbios_tables, &smbios_tables_len,
                       &smbios_anchor, &smbios_anchor_len);
+    g_free(mem_array);
+
     if (smbios_anchor) {
         fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
                         smbios_tables, smbios_tables_len);
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 1341e02..6f715c6 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -831,10 +831,12 @@ static void smbios_entry_point_setup(void)
     ep.structure_table_address = cpu_to_le32(0);
 }
 
-void smbios_get_tables(uint8_t **tables, size_t *tables_len,
+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)
 {
-    unsigned i, dimm_cnt, instance;
+    unsigned i, dimm_cnt;
 
     if (smbios_legacy) {
         *tables = *anchor = NULL;
@@ -867,11 +869,9 @@ void smbios_get_tables(uint8_t **tables, size_t *tables_len,
             smbios_build_type_17_table(i, GET_DIMM_SZ);
         }
 
-        for (i = 0, instance = 0; i < e820_get_num_entries(); i++) {
-            uint64_t address, length;
-            if (e820_get_entry(i, E820_RAM, &address, &length)) {
-                smbios_build_type_19_table(instance++, address, length);
-            }
+        for (i = 0; i < mem_array_size; i++) {
+            smbios_build_type_19_table(i, mem_array[i].address,
+                                       mem_array[i].length);
         }
 
         smbios_build_type_32_table();
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index d2850be..4269aab 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -17,13 +17,21 @@
 
 #define SMBIOS_MAX_TYPE 127
 
+/* memory area description, used by type 19 table */
+struct smbios_phys_mem_area {
+    uint64_t address;
+    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(uint8_t **tables, size_t *tables_len,
+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);
 
 /*
-- 
1.8.3.1

[Qemu-devel] [ARM SMBIOS V1 PATCH 3/6] smbios: pass ram size as a parameter to build smbios tables

[Wei Huang]

This patch adds a new parameter, mem_size, to smbios_get_tables()
function. This step is required to make smbios code architecture
independent.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 hw/i386/pc.c             | 2 +-
 hw/i386/smbios.c         | 8 ++++----
 include/hw/i386/smbios.h | 2 ++
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 34e9133..944d5b1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -742,7 +742,7 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
             array_count++;
         }
     }
-    smbios_get_tables(mem_array, array_count,
+    smbios_get_tables(mem_array, array_count, ram_size,
                       &smbios_tables, &smbios_tables_len,
                       &smbios_anchor, &smbios_anchor_len);
     g_free(mem_array);
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 6f715c6..12aee90 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -19,10 +19,9 @@
 #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"
-
+#include "exec/cpu-common.h"
 
 /* legacy structures and constants for <= 2.0 machines */
 struct smbios_header {
@@ -649,7 +648,7 @@ static void smbios_build_type_4_table(unsigned instance)
 
 #define MAX_T16_STD_SZ 0x80000000 /* 2T in Kilobytes */
 
-static void smbios_build_type_16_table(unsigned dimm_cnt)
+static void smbios_build_type_16_table(unsigned dimm_cnt, ram_addr_t ram_size)
 {
     uint64_t size_kb;
 
@@ -833,6 +832,7 @@ static void smbios_entry_point_setup(void)
 
 void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
                        const unsigned int mem_array_size,
+                       const ram_addr_t ram_size,
                        uint8_t **tables, size_t *tables_len,
                        uint8_t **anchor, size_t *anchor_len)
 {
@@ -863,7 +863,7 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
 
         dimm_cnt = QEMU_ALIGN_UP(ram_size, MAX_DIMM_SZ) / MAX_DIMM_SZ;
 
-        smbios_build_type_16_table(dimm_cnt);
+        smbios_build_type_16_table(dimm_cnt, ram_size);
 
         for (i = 0; i < dimm_cnt; i++) {
             smbios_build_type_17_table(i, GET_DIMM_SZ);
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 4269aab..e727233 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -14,6 +14,7 @@
  */
 
 #include "qemu/option.h"
+#include "exec/cpu-common.h"
 
 #define SMBIOS_MAX_TYPE 127
 
@@ -31,6 +32,7 @@ void smbios_set_defaults(const char *manufacturer, const char *product,
 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,
+                       const ram_addr_t ram_size,
                        uint8_t **tables, size_t *tables_len,
                        uint8_t **anchor, size_t *anchor_len);
 
-- 
1.8.3.1

[Qemu-devel] [ARM SMBIOS V1 PATCH 4/6] smbios: move smbios code into a common folder

[Wei Huang]

To share smbios among different architectures, this patch moves SMBIOS
code (smbios.c and smbios.h) from x86 specific folders into new
hw/smbios directories. As a result, CONFIG_SMBIOS=y is defined in
x86 default config files.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 arch_init.c                          | 2 +-
 default-configs/i386-softmmu.mak     | 1 +
 default-configs/x86_64-softmmu.mak   | 1 +
 hw/Makefile.objs                     | 1 +
 hw/i386/Makefile.objs                | 2 +-
 hw/i386/pc.c                         | 2 +-
 hw/i386/pc_piix.c                    | 2 +-
 hw/i386/pc_q35.c                     | 2 +-
 hw/smbios/Makefile.objs              | 1 +
 hw/{i386 => smbios}/smbios.c         | 2 +-
 include/hw/{i386 => smbios}/smbios.h | 0
 tests/bios-tables-test.c             | 2 +-
 vl.c                                 | 2 +-
 13 files changed, 12 insertions(+), 8 deletions(-)
 create mode 100644 hw/smbios/Makefile.objs
 rename hw/{i386 => smbios}/smbios.c (99%)
 rename include/hw/{i386 => smbios}/smbios.h (100%)

diff --git a/arch_init.c b/arch_init.c
index 725c638..38f5fb9 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -26,7 +26,7 @@
 #include "sysemu/arch_init.h"
 #include "hw/pci/pci.h"
 #include "hw/audio/audio.h"
-#include "hw/i386/smbios.h"
+#include "hw/smbios/smbios.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "qmp-commands.h"
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 48b5762..5eaafa1 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -49,3 +49,4 @@ CONFIG_MEM_HOTPLUG=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
+CONFIG_SMBIOS=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 4962ed7..28e2099 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -50,3 +50,4 @@ CONFIG_MEM_HOTPLUG=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
+CONFIG_SMBIOS=y
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 73afa41..7e7c241 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -31,6 +31,7 @@ devices-dirs-$(CONFIG_VIRTIO) += virtio/
 devices-dirs-$(CONFIG_SOFTMMU) += watchdog/
 devices-dirs-$(CONFIG_SOFTMMU) += xen/
 devices-dirs-$(CONFIG_MEM_HOTPLUG) += mem/
+devices-dirs-$(CONFIG_SMBIOS) += smbios/
 devices-dirs-y += core/
 common-obj-y += $(devices-dirs-y)
 obj-y += $(devices-dirs-y)
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index bd4f147..723a4d8 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -1,5 +1,5 @@
 obj-$(CONFIG_KVM) += kvm/
-obj-y += multiboot.o smbios.o
+obj-y += multiboot.o
 obj-y += pc.o pc_piix.o pc_q35.o
 obj-y += pc_sysfw.o
 obj-y += intel_iommu.o
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 944d5b1..80291fe 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -33,7 +33,7 @@
 #include "hw/pci/pci_bus.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/timer/hpet.h"
-#include "hw/i386/smbios.h"
+#include "hw/smbios/smbios.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "multiboot.h"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a896624..653c710 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -28,7 +28,7 @@
 #include "hw/loader.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/apic.h"
-#include "hw/i386/smbios.h"
+#include "hw/smbios/smbios.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_ids.h"
 #include "hw/usb.h"
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 974aead..d83df14 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -39,7 +39,7 @@
 #include "hw/pci-host/q35.h"
 #include "exec/address-spaces.h"
 #include "hw/i386/ich9.h"
-#include "hw/i386/smbios.h"
+#include "hw/smbios/smbios.h"
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
 #include "hw/usb.h"
diff --git a/hw/smbios/Makefile.objs b/hw/smbios/Makefile.objs
new file mode 100644
index 0000000..f69a92f
--- /dev/null
+++ b/hw/smbios/Makefile.objs
@@ -0,0 +1 @@
+common-obj-$(CONFIG_SMBIOS) += smbios.o
diff --git a/hw/i386/smbios.c b/hw/smbios/smbios.c
similarity index 99%
rename from hw/i386/smbios.c
rename to hw/smbios/smbios.c
index 12aee90..08ba62a 100644
--- a/hw/i386/smbios.c
+++ b/hw/smbios/smbios.c
@@ -19,7 +19,7 @@
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/cpus.h"
-#include "hw/i386/smbios.h"
+#include "hw/smbios/smbios.h"
 #include "hw/loader.h"
 #include "exec/cpu-common.h"
 
diff --git a/include/hw/i386/smbios.h b/include/hw/smbios/smbios.h
similarity index 100%
rename from include/hw/i386/smbios.h
rename to include/hw/smbios/smbios.h
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 0de1742..613867a 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -18,7 +18,7 @@
 #include "libqtest.h"
 #include "qemu/compiler.h"
 #include "hw/acpi/acpi-defs.h"
-#include "hw/i386/smbios.h"
+#include "hw/smbios/smbios.h"
 #include "qemu/bitmap.h"
 
 #define MACHINE_PC "pc"
diff --git a/vl.c b/vl.c
index 0adbbd6..584ca88 100644
--- a/vl.c
+++ b/vl.c
@@ -68,7 +68,7 @@ int main(int argc, char **argv)
 #include "hw/isa/isa.h"
 #include "hw/bt.h"
 #include "sysemu/watchdog.h"
-#include "hw/i386/smbios.h"
+#include "hw/smbios/smbios.h"
 #include "hw/xen/xen.h"
 #include "hw/qdev.h"
 #include "hw/loader.h"
-- 
1.8.3.1

[Qemu-devel] [ARM SMBIOS V1 PATCH 5/6] 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.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 hw/i386/pc_piix.c          |  3 +-
 hw/i386/pc_q35.c           |  3 +-
 hw/smbios/smbios.c         | 72 +++++++++++++++++++++++++++++++---------------
 include/hw/smbios/smbios.h | 30 +++++++++++++++++--
 4 files changed, 81 insertions(+), 27 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 653c710..04636b1 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 d83df14..061507d 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 08ba62a..70ecd87 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -48,6 +48,7 @@ static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
 static bool smbios_legacy = true;
 static bool smbios_uuid_encoded = true;
+static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21;
 /* end: legacy structures & constants for <= 2.0 machines */
 
 
@@ -55,7 +56,8 @@ 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 smbios_entry_point ep;
+static size_t ep_length;
 
 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,49 @@ 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);
+    if (smbios_ep_type == 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);
+
+        /* setup the anchor point length */
+        ep_length = sizeof (struct smbios_21_entry_point);
+    } else if (smbios_ep_type == 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;
+
+        /* 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_table_max);
+
+        /* BIOS must recalculate */
+        ep.ep30.checksum = 0;
+        ep.ep30.structure_table_address = cpu_to_le64(0);
+
+        ep_length = sizeof (struct smbios_30_entry_point);
+    }
 }
 
 void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
@@ -886,7 +912,7 @@ 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);
+    *anchor_len = ep_length;
 }
 
 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 e727233..3dd8a41 100644
--- a/include/hw/smbios/smbios.h
+++ b/include/hw/smbios/smbios.h
@@ -18,6 +18,12 @@
 
 #define SMBIOS_MAX_TYPE 127
 
+typedef enum SmbiosEntryPointType {
+    SMBIOS_ENTRY_POINT_21,
+    SMBIOS_ENTRY_POINT_30,
+} SmbiosEntryPointType;
+
+
 /* memory area description, used by type 19 table */
 struct smbios_phys_mem_area {
     uint64_t address;
@@ -28,7 +34,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, bool legacy_mode,
-                         bool uuid_encoded);
+                         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,
@@ -43,7 +49,8 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
 /* SMBIOS entry point (anchor).
  * BIOS must place this at a 16-bit-aligned address between 0xf0000 and 0xfffff.
  */
-struct smbios_entry_point {
+/* SMBIOS 2.1 entry point */
+struct smbios_21_entry_point {
     uint8_t anchor_string[4];
     uint8_t checksum;
     uint8_t length;
@@ -60,6 +67,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 QEMU_PACKED {
+    struct smbios_21_entry_point ep21;
+    struct smbios_30_entry_point ep30;
+} smbios_entry_point;
+
 /* This goes at the beginning of every SMBIOS structure. */
 struct smbios_structure_header {
     uint8_t type;
-- 
1.8.3.1

[Qemu-devel] [ARM SMBIOS V1 PATCH 6/6] 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.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 default-configs/arm-softmmu.mak  |  1 +
 hw/arm/virt.c                    | 24 ++++++++++++++++++++++++
 include/hw/arm/virt-acpi-build.h |  1 +
 3 files changed, 26 insertions(+)

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..04e5da4 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,34 @@ 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;
+
+    smbios_set_defaults("QEMU", "QEMU Virtual Machine",
+                        "1.0", false, false, SMBIOS_ENTRY_POINT_30);
+
+    smbios_get_tables(NULL, 0, guest_info->ram_size,
+                      &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)
@@ -894,6 +917,7 @@ static void machvirt_init(MachineState *machine)
 
     guest_info->smp_cpus = smp_cpus;
     guest_info->fw_cfg = fw_cfg_find();
+    guest_info->ram_size = machine->ram_size;
     guest_info->memmap = vbi->memmap;
     guest_info->irqmap = vbi->irqmap;
     guest_info_state->machine_done.notify = virt_guest_info_machine_done;
diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
index 04f174d..ef274c6 100644
--- a/include/hw/arm/virt-acpi-build.h
+++ b/include/hw/arm/virt-acpi-build.h
@@ -29,6 +29,7 @@
 typedef struct VirtGuestInfo {
     int smp_cpus;
     FWCfgState *fw_cfg;
+    ram_addr_t ram_size;
     const MemMapEntry *memmap;
     const int *irqmap;
 } VirtGuestInfo;
-- 
1.8.3.1

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 6/6] smbios: implement smbios support for mach-virt

[Shannon Zhao]

On 2015/7/28 14:00, Wei Huang wrote:
> This patch generates smbios tables for ARM mach-virt. Also add
> CONFIG_SMBIOS=y for ARM default config.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  default-configs/arm-softmmu.mak  |  1 +
>  hw/arm/virt.c                    | 24 ++++++++++++++++++++++++
>  include/hw/arm/virt-acpi-build.h |  1 +
>  3 files changed, 26 insertions(+)
> 
> 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..04e5da4 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,34 @@ 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;

Would be better to check whether guest_info->fw_cfg is NULL?

> +    uint8_t *smbios_tables, *smbios_anchor;
> +    size_t smbios_tables_len, smbios_anchor_len;
> +
> +    smbios_set_defaults("QEMU", "QEMU Virtual Machine",
> +                        "1.0", false, false, SMBIOS_ENTRY_POINT_30);
> +
> +    smbios_get_tables(NULL, 0, guest_info->ram_size,
> +                      &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)
> @@ -894,6 +917,7 @@ static void machvirt_init(MachineState *machine)
>  
>      guest_info->smp_cpus = smp_cpus;
>      guest_info->fw_cfg = fw_cfg_find();
> +    guest_info->ram_size = machine->ram_size;
>      guest_info->memmap = vbi->memmap;
>      guest_info->irqmap = vbi->irqmap;
>      guest_info_state->machine_done.notify = virt_guest_info_machine_done;
> diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
> index 04f174d..ef274c6 100644
> --- a/include/hw/arm/virt-acpi-build.h
> +++ b/include/hw/arm/virt-acpi-build.h
> @@ -29,6 +29,7 @@
>  typedef struct VirtGuestInfo {
>      int smp_cpus;
>      FWCfgState *fw_cfg;
> +    ram_addr_t ram_size;
>      const MemMapEntry *memmap;
>      const int *irqmap;
>  } VirtGuestInfo;
> 

-- 
Shannon

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 6/6] smbios: implement smbios support for mach-virt

[Wei Huang]

On 07/30/2015 09:11 PM, Shannon Zhao wrote:
> 
> 
> On 2015/7/28 14:00, Wei Huang wrote:
>> This patch generates smbios tables for ARM mach-virt. Also add
>> CONFIG_SMBIOS=y for ARM default config.
>>
>> Signed-off-by: Wei Huang <wei@redhat.com>
>> ---
>>  default-configs/arm-softmmu.mak  |  1 +
>>  hw/arm/virt.c                    | 24 ++++++++++++++++++++++++
>>  include/hw/arm/virt-acpi-build.h |  1 +
>>  3 files changed, 26 insertions(+)
>>
>> 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..04e5da4 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,34 @@ 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;
> 
> Would be better to check whether guest_info->fw_cfg is NULL?
Good point. I will address it in next spin.

Thanks,
-Wei
> 
>> +    uint8_t *smbios_tables, *smbios_anchor;
>> +    size_t smbios_tables_len, smbios_anchor_len;
>> +
>> +    smbios_set_defaults("QEMU", "QEMU Virtual Machine",
>> +                        "1.0", false, false, SMBIOS_ENTRY_POINT_30);
>> +
>> +    smbios_get_tables(NULL, 0, guest_info->ram_size,
>> +                      &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)
>> @@ -894,6 +917,7 @@ static void machvirt_init(MachineState *machine)
>>  
>>      guest_info->smp_cpus = smp_cpus;
>>      guest_info->fw_cfg = fw_cfg_find();
>> +    guest_info->ram_size = machine->ram_size;
>>      guest_info->memmap = vbi->memmap;
>>      guest_info->irqmap = vbi->irqmap;
>>      guest_info_state->machine_done.notify = virt_guest_info_machine_done;
>> diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
>> index 04f174d..ef274c6 100644
>> --- a/include/hw/arm/virt-acpi-build.h
>> +++ b/include/hw/arm/virt-acpi-build.h
>> @@ -29,6 +29,7 @@
>>  typedef struct VirtGuestInfo {
>>      int smp_cpus;
>>      FWCfgState *fw_cfg;
>> +    ram_addr_t ram_size;
>>      const MemMapEntry *memmap;
>>      const int *irqmap;
>>  } VirtGuestInfo;
>>
> 

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 5/6] smbios: add smbios 3.0 support

[Laszlo Ersek]

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

Hello Wei,

there's a bug in one of the SMBIOS 3.0 entry point struct fields. It
relates to the use of "smbios_table_max":

On 07/28/15 08:00, 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.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  hw/i386/pc_piix.c          |  3 +-
>  hw/i386/pc_q35.c           |  3 +-
>  hw/smbios/smbios.c         | 72 +++++++++++++++++++++++++++++++---------------
>  include/hw/smbios/smbios.h | 30 +++++++++++++++++--
>  4 files changed, 81 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 653c710..04636b1 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 d83df14..061507d 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 08ba62a..70ecd87 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -48,6 +48,7 @@ static uint8_t *smbios_entries;
>  static size_t smbios_entries_len;
>  static bool smbios_legacy = true;
>  static bool smbios_uuid_encoded = true;
> +static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21;
>  /* end: legacy structures & constants for <= 2.0 machines */
>  
>  
> @@ -55,7 +56,8 @@ 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 smbios_entry_point ep;
> +static size_t ep_length;
>  
>  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,49 @@ 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);
> +    if (smbios_ep_type == 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);

This is correct. In SMBIOS 2.x, the "max_structure_size" field reports:

  Size of the largest SMBIOS structure, in bytes, and encompasses the
  structure’s formatted area and text strings

And the "smbios_table_max" variable actually tracks that, see the
SMBIOS_BUILD_TABLE_POST macro.

        /* update smbios max. element size */

However,

> +        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);
> +
> +        /* setup the anchor point length */
> +        ep_length = sizeof (struct smbios_21_entry_point);
> +    } else if (smbios_ep_type == 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;
> +
> +        /* 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_table_max);

This is incorrect. In SMBIOS 3.0, the "structure_table_max_size" field
does not track the largest individual table (there is no such field any
longer). Instead, this field reports

  Maximum size of SMBIOS Structure Table, pointed to by the Structure
  Table Address, in bytes. The actual size is guaranteed to be less or
  equal to the maximum size.

In other words, you should fill this field in from "smbios_tables_len"
(similarly to "ep.ep21.structure_table_length").

Now you might ask what sense it makes for the 3.0 spec to require an
"upper limit" on the full blob length, and not an *exact* length (as it
was with 2.x). I don't know. I asked Jon Masters on IRC (CC'd), and if I
understood his reply correctly, this is simply an error in the 3.0 spec,
to be fixed in 3.1.

So, please just squash in the attached patch, which should fix this bug
and satisfy the requirements of both the 3.0 and the (expected) 3.1 specs.

With that patch applied, the firmware code I'm about to post (round 2)
seems to accept and install the stuff in the guest. I haven't verified
the tables in a Linux guest (with the dmidecode utility) yet, because I
don't have ready access to a Mustang right now, and with TCG I won't
boot Linux :)

But, I ran the SMBIOSVIEW command in the UEFI shell of AAVMF, and things
look okay there. I'm attaching an excerpt of the output. (Note that the
Type=0 table comes from the firmware itself, as a fallback, same as on
x86 / OVMF, if QEMU doesn't provide one.)

Thanks!
Laszlo

> +
> +        /* BIOS must recalculate */
> +        ep.ep30.checksum = 0;
> +        ep.ep30.structure_table_address = cpu_to_le64(0);
> +
> +        ep_length = sizeof (struct smbios_30_entry_point);
> +    }
>  }
>  
>  void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
> @@ -886,7 +912,7 @@ 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);
> +    *anchor_len = ep_length;
>  }
>  
>  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 e727233..3dd8a41 100644
> --- a/include/hw/smbios/smbios.h
> +++ b/include/hw/smbios/smbios.h
> @@ -18,6 +18,12 @@
>  
>  #define SMBIOS_MAX_TYPE 127
>  
> +typedef enum SmbiosEntryPointType {
> +    SMBIOS_ENTRY_POINT_21,
> +    SMBIOS_ENTRY_POINT_30,
> +} SmbiosEntryPointType;
> +
> +
>  /* memory area description, used by type 19 table */
>  struct smbios_phys_mem_area {
>      uint64_t address;
> @@ -28,7 +34,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, bool legacy_mode,
> -                         bool uuid_encoded);
> +                         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,
> @@ -43,7 +49,8 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
>  /* SMBIOS entry point (anchor).
>   * BIOS must place this at a 16-bit-aligned address between 0xf0000 and 0xfffff.
>   */
> -struct smbios_entry_point {
> +/* SMBIOS 2.1 entry point */
> +struct smbios_21_entry_point {
>      uint8_t anchor_string[4];
>      uint8_t checksum;
>      uint8_t length;
> @@ -60,6 +67,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 QEMU_PACKED {
> +    struct smbios_21_entry_point ep21;
> +    struct smbios_30_entry_point ep30;
> +} smbios_entry_point;
> +
>  /* This goes at the beginning of every SMBIOS structure. */
>  struct smbios_structure_header {
>      uint8_t type;
> 


[-- Attachment #2: 0001-hw-smbios-fix-Structure-table-maximum-size-field-in-.patch --]
[-- Type: text/x-patch, Size: 1051 bytes --]

>From 4bac0aa296bd2b3b7dc63895c3d7e19e39c5b13c Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Fri, 31 Jul 2015 18:54:21 +0200
Subject: [PATCH] hw/smbios: fix "Structure table maximum size" field in the
 3.0 entry point

This field carries an inclusive upper bound on the size of the blob that
collects the tables, and not the size of the largest individual table.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 hw/smbios/smbios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 70ecd87..6f58709 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -846,7 +846,7 @@ static void smbios_entry_point_setup(void)
         ep.ep30.smbios_doc_rev = 0;
 
         /* set during table construct, but BIOS might override */
-        ep.ep30.structure_table_max_size = cpu_to_le32(smbios_table_max);
+        ep.ep30.structure_table_max_size = cpu_to_le32(smbios_tables_len);
 
         /* BIOS must recalculate */
         ep.ep30.checksum = 0;
-- 
1.8.3.1


[-- Attachment #3: smbiosview.txt --]
[-- Type: text/plain, Size: 6632 bytes --]

SMBIOS 3.0 (64-bit) Entry Point Structure:
Anchor String:        _SM3_
EPS Checksum:         0x71
Entry Point Len:      24
Version:              3.0
SMBIOS Docrev:        0x0
Table Max Size:       316
Table Address:        0xB6EF0000
Entry Point revision: 0x1

=========================================================
Query Structure, conditions are:
QueryType   = Random
QueryHandle = Random
ShowType    = SHOW_DETAIL


=========================================================
Type=1, Handle=0x100
Dump Structure as:
Index=0,Length=0x3A,Addr=0xB6EF0000
00000000: 01 1B 00 01 01 02 03 00-00 00 00 00 00 00 00 00  *................*
00000010: 00 00 00 00 00 00 00 00-06 00 00 51 45 4D 55 00  *...........QEMU.*
00000020: 51 45 4D 55 20 56 69 72-74 75 61 6C 20 4D 61 63  *QEMU Virtual Mac*
00000030: 68 69 6E 65 00 31 2E 30-00 00                    *hine.1.0..*
Structure Type: System Information
Format part Len : 27
Structure Handle: 256
Manufacturer: QEMU
ProductName: QEMU Virtual Machine
Version: 1.0
SerialNumber:
Dump Uuid
size=16:
00000000: 00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00  *................*
System Wakeup Type: Power Switch
SKUNumber:
Family:


=========================================================
Type=3, Handle=0x300
Dump Structure as:
Index=1,Length=0x1F,Addr=0xB6EF003A
00000000: 03 15 00 03 01 01 02 00-00 03 03 03 02 00 00 00  *................*
00000010: 00 00 00 00 00 51 45 4D-55 00 31 2E 30 00 00     *.....QEMU.1.0..*
Structure Type: System Enclosure
Format part Len : 21
Structure Handle: 768
Manufacturer: QEMU
Type: 1
System Enclosure or Chassis Types:  Other
Version: 1.0
SerialNumber:
AssetTag:
Bootup state System Enclosure or Chassis Status:  Safe
Power Supply State System Enclosure or Chassis Status:  Safe
Thermal state System Enclosure or Chassis Status:  Safe
Security Status System Enclosure or Chassis Security Status:  Unknown
Dump OemDefined
size=4:
00000000: 00 00 00 00                                      *....*


=========================================================
Type=4, Handle=0x400
Dump Structure as:
Index=2,Length=0x3A,Addr=0xB6EF0059
00000000: 04 2A 00 04 01 03 01 02-00 00 00 00 00 00 00 00  *.*..............*
00000010: 03 00 00 00 D0 07 D0 07-41 01 FF FF FF FF FF FF  *........A.......*
00000020: 00 00 00 01 01 01 02 00-01 00 43 50 55 20 30 00  *..........CPU 0.*
00000030: 51 45 4D 55 00 31 2E 30-00 00                    *QEMU.1.0..*
Structure Type: Processor Information
Format part Len : 42
Structure Handle: 1024
Socket: CPU 0
Processor Type:  Central Processor
Processor Family: Other
ProcessorManufacture: QEMU
Dump ProcessorId
size=8:
00000000: 00 00 00 00 00 00 00 00-                         *........*
ProcessorVersion: 1.0
Processor Information -  Voltage:
ExternalClock: 0
MaxSpeed: 2000
CurrentSpeed: 2000
Processor Status:
CPU Socket Populated
CPU Enabled
Processor Upgrade: Other
L1CacheHandle: 0xFFFF
L2CacheHandle: 0xFFFF
L3CacheHandle: 0xFFFF
SerialNumber:
AssetTag:
PartNumber:
CoreCount: 1
EnabledCoreCount: 1
ThreadCount: 1
Processor Characteristics:  Unknown |


=========================================================
Type=16, Handle=0x1000
Dump Structure as:
Index=3,Length=0x19,Addr=0xB6EF0093
00000000: 10 17 00 10 01 03 06 00-00 20 00 FE FF 01 00 00  *......... ......*
00000010: 00 00 00 00 00 00 00 00-00                       *.........*
Structure Type: Physical Memory Array
Format part Len : 23
Structure Handle: 4096
Physical Memory Array Location:   Other
Physical Memory Array Use:  System memory
Physical Memory Array Error Correction Types:   Multi-bit ECC
MaximumCapacity: 0x200000
MemoryErrorInformationHandle: 0xFFFE
NumberOfMemoryDevices: 0x1
ExtendedMaximumCapacity: 0x0


=========================================================
Type=17, Handle=0x1100
Dump Structure as:
Index=4,Length=0x35,Addr=0xB6EF00AC
00000000: 11 28 00 11 00 10 FE FF-FF FF FF FF 00 08 09 00  *.(..............*
00000010: 01 00 07 02 00 00 00 02-00 00 00 00 00 00 00 00  *................*
00000020: 00 00 00 00 00 00 00 00-44 49 4D 4D 20 30 00 51  *........DIMM 0.Q*
00000030: 45 4D 55 00 00                                   *EMU..*
Structure Type: Memory Device
Format part Len : 40
Structure Handle: 4352
MemoryArrayHandle: 0x1000
MemoryErrorInformationHandle: 0xFFFE
TotalWidth: 65535
DataWidth: 65535
Size: 2048
Memory Device - Form Factor:   DIMM
DeviceSet: 0x0
DeviceLocator: DIMM 0
BankLocator:
Memory Device - Type:   RAM
Memory Device - Type Detail:  Other |
Speed: 0x0
Manufacturer: QEMU
SerialNumber:
AssetTag:
PartNumber:
Attributes: 0x0
ExtendedSize: 0
ConfiguredMemoryClockSpeed: 0


=========================================================
Type=32, Handle=0x2000
Dump Structure as:
Index=5,Length=0xD,Addr=0xB6EF00E1
00000000: 20 0B 00 20 00 00 00 00-00 00 00 00 00           * .. .........*
Structure Type: System Boot Information
Format part Len : 11
Structure Handle: 8192
Dump Reserved
size=6:
00000000: 00 00 00 00 00 00                                *......*
System Boot Status:  No errors detected

=========================================================
Type=0, Handle=0x0
Dump Structure as:
Index=6,Length=0x48,Addr=0xB6EF00EE
00000000: 00 18 00 00 01 02 00 E8-03 00 08 00 00 00 00 00  *................*
00000010: 00 00 00 1C 00 00 FF FF-45 46 49 20 44 65 76 65  *........EFI Deve*
00000020: 6C 6F 70 6D 65 6E 74 20-4B 69 74 20 49 49 20 2F  *lopment Kit II /*
00000030: 20 4F 56 4D 46 00 30 2E-30 2E 30 00 30 32 2F 30  * OVMF.0.0.0.02/0*
00000040: 36 2F 32 30 31 35 00 00-                         *6/2015..*
Structure Type: BIOS Information
Format part Len : 24
Structure Handle: 0
Vendor: EFI Development Kit II / OVMF
BiosVersion: 0.0.0
BiosSegment: 59392
BiosReleaseDate: 02/06/2015
BiosSize:  64 KB
BIOS Characteristics:
BIOS Characteristics Not Supported
 Bits 32:47 are reserved for BIOS Vendor
 Bits 48:64 are reserved for System Vendor
BIOS Characteristics Extension Byte1:
BIOS Characteristics Extension Byte2:
Enable Targeted Content Distribution
UEFI Specification is supported
The SMBIOS table describes a virtual machine
 Bits 5:7 are reserved for future assignment
SystemBiosMajorRelease: 0
SystemBiosMinorRelease: 0
EmbeddedControllerFirmwareMajorRelease: 255
EmbeddedControllerFirmwareMinorRelease: 255


=========================================================
Type=127, Handle=0xFEFF
Dump Structure as:
Index=7,Length=0x6,Addr=0xB6EF0136
00000000: 7F 04 FF FE 00 00                                *......*
Structure Type: End-of-Table
Format part Len : 4
Structure Handle: 65279
This structure indicates the End-of-table!


=========================================================

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Laszlo Ersek]

On 07/28/15 08:00, Wei Huang 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.
> 
> RFC version have been tested by Laszlo using his customized version of
> AAVMF. We were able to detect SMBIOS 2.8 tables using dmidecode inside
> an AArch64 guest VM. Moving forward, it is better to support SMBIOS 3.0
> for ARM guest VM. This new version (V1) integrates SMBIOS 3.0 support
> for ARM mach-virt. I have tested this version by forcing SMBIOS 2.1
> format (i.e. passing SMBIOS_21_ENTRY_POINT to smbios_set_defaults()).
> SMBIOS 3.0 hasn't been tested yet as it requires AAVMF to install 3.0 entry.
> 
> 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 (6):
>   smbios: extract x86 smbios building code into a function
>   smbios: remove dependency on x86 e820 tables
>   smbios: pass ram size as a parameter to build smbios tables
>   smbios: move smbios code into a common folder
>   smbios: add smbios 3.0 support
>   smbios: implement smbios support for mach-virt
> 
>  arch_init.c                          |  2 +-
>  default-configs/arm-softmmu.mak      |  1 +
>  default-configs/i386-softmmu.mak     |  1 +
>  default-configs/x86_64-softmmu.mak   |  1 +
>  hw/Makefile.objs                     |  1 +
>  hw/arm/virt.c                        | 24 +++++++++
>  hw/i386/Makefile.objs                |  2 +-
>  hw/i386/pc.c                         | 56 ++++++++++++++-------
>  hw/i386/pc_piix.c                    |  5 +-
>  hw/i386/pc_q35.c                     |  5 +-
>  hw/smbios/Makefile.objs              |  1 +
>  hw/{i386 => smbios}/smbios.c         | 96 +++++++++++++++++++++++-------------
>  include/hw/arm/virt-acpi-build.h     |  1 +
>  include/hw/{i386 => smbios}/smbios.h | 42 ++++++++++++++--
>  tests/bios-tables-test.c             |  2 +-
>  vl.c                                 |  2 +-
>  16 files changed, 179 insertions(+), 63 deletions(-)
>  create mode 100644 hw/smbios/Makefile.objs
>  rename hw/{i386 => smbios}/smbios.c (93%)
>  rename include/hw/{i386 => smbios}/smbios.h (84%)
> 

I was hoping there would be a focused review from the subsystem
maintainers / feature owners for this patchset. Thus far only Shannon
commented on the series, plus I tested it and reported a small bug (with
a fix).

Peter: if I review this series (and version 2 that Wei is already
planning to post, in order to address the notes above, plus anything
that further review might turn up), will my review suffice for you to
apply this series (after 2.4 is out)?

If not, then everyone with jurisdiction (and those people are on the Cc
list I think), please consider this a ping. (A week has passed so it
shouldn't be too early.)

Thanks!
Laszlo

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Peter Maydell]

On 5 August 2015 at 18:16, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/28/15 08:00, Wei Huang 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.
>>
>> RFC version have been tested by Laszlo using his customized version of
>> AAVMF. We were able to detect SMBIOS 2.8 tables using dmidecode inside
>> an AArch64 guest VM. Moving forward, it is better to support SMBIOS 3.0
>> for ARM guest VM. This new version (V1) integrates SMBIOS 3.0 support
>> for ARM mach-virt. I have tested this version by forcing SMBIOS 2.1
>> format (i.e. passing SMBIOS_21_ENTRY_POINT to smbios_set_defaults()).
>> SMBIOS 3.0 hasn't been tested yet as it requires AAVMF to install 3.0 entry.
>>
>> 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 (6):
>>   smbios: extract x86 smbios building code into a function
>>   smbios: remove dependency on x86 e820 tables
>>   smbios: pass ram size as a parameter to build smbios tables
>>   smbios: move smbios code into a common folder
>>   smbios: add smbios 3.0 support
>>   smbios: implement smbios support for mach-virt
>>
>>  arch_init.c                          |  2 +-
>>  default-configs/arm-softmmu.mak      |  1 +
>>  default-configs/i386-softmmu.mak     |  1 +
>>  default-configs/x86_64-softmmu.mak   |  1 +
>>  hw/Makefile.objs                     |  1 +
>>  hw/arm/virt.c                        | 24 +++++++++
>>  hw/i386/Makefile.objs                |  2 +-
>>  hw/i386/pc.c                         | 56 ++++++++++++++-------
>>  hw/i386/pc_piix.c                    |  5 +-
>>  hw/i386/pc_q35.c                     |  5 +-
>>  hw/smbios/Makefile.objs              |  1 +
>>  hw/{i386 => smbios}/smbios.c         | 96 +++++++++++++++++++++++-------------
>>  include/hw/arm/virt-acpi-build.h     |  1 +
>>  include/hw/{i386 => smbios}/smbios.h | 42 ++++++++++++++--
>>  tests/bios-tables-test.c             |  2 +-
>>  vl.c                                 |  2 +-
>>  16 files changed, 179 insertions(+), 63 deletions(-)
>>  create mode 100644 hw/smbios/Makefile.objs
>>  rename hw/{i386 => smbios}/smbios.c (93%)
>>  rename include/hw/{i386 => smbios}/smbios.h (84%)
>>
>
> I was hoping there would be a focused review from the subsystem
> maintainers / feature owners for this patchset. Thus far only Shannon
> commented on the series, plus I tested it and reported a small bug (with
> a fix).
>
> Peter: if I review this series (and version 2 that Wei is already
> planning to post, in order to address the notes above, plus anything
> that further review might turn up), will my review suffice for you to
> apply this series (after 2.4 is out)?

Maybe. I haven't looked at the series at all, because it fell
under "not for 2.4 and not something I know enough about to
easily and quickly review" (and besides 5 out of 6 patches are
not ARM-related but just about refactoring the x86 code).

What is SMBIOS supposed to provide for ARM virt anyway?
I would have expected all the information a guest needs
to be in the dtb or ACPI tables...

Is support for this all in the mainline kernel yet?

thanks
-- PMM

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Laszlo Ersek]

On 08/05/15 19:35, Peter Maydell wrote:
> On 5 August 2015 at 18:16, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 07/28/15 08:00, Wei Huang 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.
>>>
>>> RFC version have been tested by Laszlo using his customized version of
>>> AAVMF. We were able to detect SMBIOS 2.8 tables using dmidecode inside
>>> an AArch64 guest VM. Moving forward, it is better to support SMBIOS 3.0
>>> for ARM guest VM. This new version (V1) integrates SMBIOS 3.0 support
>>> for ARM mach-virt. I have tested this version by forcing SMBIOS 2.1
>>> format (i.e. passing SMBIOS_21_ENTRY_POINT to smbios_set_defaults()).
>>> SMBIOS 3.0 hasn't been tested yet as it requires AAVMF to install 3.0 entry.
>>>
>>> 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 (6):
>>>   smbios: extract x86 smbios building code into a function
>>>   smbios: remove dependency on x86 e820 tables
>>>   smbios: pass ram size as a parameter to build smbios tables
>>>   smbios: move smbios code into a common folder
>>>   smbios: add smbios 3.0 support
>>>   smbios: implement smbios support for mach-virt
>>>
>>>  arch_init.c                          |  2 +-
>>>  default-configs/arm-softmmu.mak      |  1 +
>>>  default-configs/i386-softmmu.mak     |  1 +
>>>  default-configs/x86_64-softmmu.mak   |  1 +
>>>  hw/Makefile.objs                     |  1 +
>>>  hw/arm/virt.c                        | 24 +++++++++
>>>  hw/i386/Makefile.objs                |  2 +-
>>>  hw/i386/pc.c                         | 56 ++++++++++++++-------
>>>  hw/i386/pc_piix.c                    |  5 +-
>>>  hw/i386/pc_q35.c                     |  5 +-
>>>  hw/smbios/Makefile.objs              |  1 +
>>>  hw/{i386 => smbios}/smbios.c         | 96 +++++++++++++++++++++++-------------
>>>  include/hw/arm/virt-acpi-build.h     |  1 +
>>>  include/hw/{i386 => smbios}/smbios.h | 42 ++++++++++++++--
>>>  tests/bios-tables-test.c             |  2 +-
>>>  vl.c                                 |  2 +-
>>>  16 files changed, 179 insertions(+), 63 deletions(-)
>>>  create mode 100644 hw/smbios/Makefile.objs
>>>  rename hw/{i386 => smbios}/smbios.c (93%)
>>>  rename include/hw/{i386 => smbios}/smbios.h (84%)
>>>
>>
>> I was hoping there would be a focused review from the subsystem
>> maintainers / feature owners for this patchset. Thus far only Shannon
>> commented on the series, plus I tested it and reported a small bug (with
>> a fix).
>>
>> Peter: if I review this series (and version 2 that Wei is already
>> planning to post, in order to address the notes above, plus anything
>> that further review might turn up), will my review suffice for you to
>> apply this series (after 2.4 is out)?
> 
> Maybe. I haven't looked at the series at all, because it fell
> under "not for 2.4 and not something I know enough about to
> easily and quickly review" (and besides 5 out of 6 patches are
> not ARM-related but just about refactoring the x86 code).
> 
> What is SMBIOS supposed to provide for ARM virt anyway?

No clue. It is dictated by the most recent SMBIOS (3.0) spec. The actual
contents is dictated by asset management needs ("run dmidecode, look at
this and that in the output"). So this series should not be reviewed
primarily for SMBIOS payload, but for correctness of refactoring, and
well-formedness of the new tables. At least the latter point seems to be
confirmed by testing already.

> I would have expected all the information a guest needs
> to be in the dtb or ACPI tables...

SMBIOS is usually not needed by the guest OS or the firmware. The spec says,

  [...] the System Management BIOS Reference Specification addresses
  how motherboard and system vendors present management information
  about their products in a standard format by extending the BIOS
  interface on Intel architecture systems. The information is intended
  to allow generic instrumentation to deliver this data to management
  applications that use CIM (the WBEM data model) or direct access and
  eliminates the need for error prone operations such as probing system
  hardware for presence detection. [...]

(The Intel-specificity in the Introduction that I quoted is certainly a
bug in the spec, because under 1 Scope | 1.1 Supported processor
architectures, Aarch32 and Aarch64 too are listed.)

As far as I know (although I have no hard evidence), SMBIOS 3.0 was
brought about by ARM platform needs. Aarch64 hardware that I have access
to seems to expose SMBIOS 3.0 indeed, therefore we should do the same in
qemu.

> Is support for this all in the mainline kernel yet?

Yes, it is. See (minimally)

$ git log --reverse fc43026278^.. -- drivers/firmware/dmi*

There are patches for arch/arm64/ and drivers/firmware/efi/ too (Ard
will know better, he wrote at least a few of them).

The userspace tools are in a more messy state AFAICT. For example I'm
unable to locate a *git* repository for upstream dmidecode. But, Ivan
and Roy should know better (Cc'd).

See also CARD-1779.

Also, guest userspace is not QEMU's problem :)

Thanks
Laszlo

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Ivan Khoronzhuk]

Hi Laszlo

On 05.08.15 21:35, Laszlo Ersek wrote:
> On 08/05/15 19:35, Peter Maydell wrote:
>> On 5 August 2015 at 18:16, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 07/28/15 08:00, Wei Huang 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.
>>>>
>>>> RFC version have been tested by Laszlo using his customized version of
>>>> AAVMF. We were able to detect SMBIOS 2.8 tables using dmidecode inside
>>>> an AArch64 guest VM. Moving forward, it is better to support SMBIOS 3.0
>>>> for ARM guest VM. This new version (V1) integrates SMBIOS 3.0 support
>>>> for ARM mach-virt. I have tested this version by forcing SMBIOS 2.1
>>>> format (i.e. passing SMBIOS_21_ENTRY_POINT to smbios_set_defaults()).
>>>> SMBIOS 3.0 hasn't been tested yet as it requires AAVMF to install 3.0 entry.
>>>>
>>>> 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 (6):
>>>>    smbios: extract x86 smbios building code into a function
>>>>    smbios: remove dependency on x86 e820 tables
>>>>    smbios: pass ram size as a parameter to build smbios tables
>>>>    smbios: move smbios code into a common folder
>>>>    smbios: add smbios 3.0 support
>>>>    smbios: implement smbios support for mach-virt
>>>>
>>>>   arch_init.c                          |  2 +-
>>>>   default-configs/arm-softmmu.mak      |  1 +
>>>>   default-configs/i386-softmmu.mak     |  1 +
>>>>   default-configs/x86_64-softmmu.mak   |  1 +
>>>>   hw/Makefile.objs                     |  1 +
>>>>   hw/arm/virt.c                        | 24 +++++++++
>>>>   hw/i386/Makefile.objs                |  2 +-
>>>>   hw/i386/pc.c                         | 56 ++++++++++++++-------
>>>>   hw/i386/pc_piix.c                    |  5 +-
>>>>   hw/i386/pc_q35.c                     |  5 +-
>>>>   hw/smbios/Makefile.objs              |  1 +
>>>>   hw/{i386 => smbios}/smbios.c         | 96 +++++++++++++++++++++++-------------
>>>>   include/hw/arm/virt-acpi-build.h     |  1 +
>>>>   include/hw/{i386 => smbios}/smbios.h | 42 ++++++++++++++--
>>>>   tests/bios-tables-test.c             |  2 +-
>>>>   vl.c                                 |  2 +-
>>>>   16 files changed, 179 insertions(+), 63 deletions(-)
>>>>   create mode 100644 hw/smbios/Makefile.objs
>>>>   rename hw/{i386 => smbios}/smbios.c (93%)
>>>>   rename include/hw/{i386 => smbios}/smbios.h (84%)
>>>>
>>>
>>> I was hoping there would be a focused review from the subsystem
>>> maintainers / feature owners for this patchset. Thus far only Shannon
>>> commented on the series, plus I tested it and reported a small bug (with
>>> a fix).
>>>
>>> Peter: if I review this series (and version 2 that Wei is already
>>> planning to post, in order to address the notes above, plus anything
>>> that further review might turn up), will my review suffice for you to
>>> apply this series (after 2.4 is out)?
>>
>> Maybe. I haven't looked at the series at all, because it fell
>> under "not for 2.4 and not something I know enough about to
>> easily and quickly review" (and besides 5 out of 6 patches are
>> not ARM-related but just about refactoring the x86 code).
>>
>> What is SMBIOS supposed to provide for ARM virt anyway?
>
> No clue. It is dictated by the most recent SMBIOS (3.0) spec. The actual
> contents is dictated by asset management needs ("run dmidecode, look at
> this and that in the output"). So this series should not be reviewed
> primarily for SMBIOS payload, but for correctness of refactoring, and
> well-formedness of the new tables. At least the latter point seems to be
> confirmed by testing already.
>
>> I would have expected all the information a guest needs
>> to be in the dtb or ACPI tables...
>
> SMBIOS is usually not needed by the guest OS or the firmware. The spec says,
>
>    [...] the System Management BIOS Reference Specification addresses
>    how motherboard and system vendors present management information
>    about their products in a standard format by extending the BIOS
>    interface on Intel architecture systems. The information is intended
>    to allow generic instrumentation to deliver this data to management
>    applications that use CIM (the WBEM data model) or direct access and
>    eliminates the need for error prone operations such as probing system
>    hardware for presence detection. [...]
>
> (The Intel-specificity in the Introduction that I quoted is certainly a
> bug in the spec, because under 1 Scope | 1.1 Supported processor
> architectures, Aarch32 and Aarch64 too are listed.)
>
> As far as I know (although I have no hard evidence), SMBIOS 3.0 was
> brought about by ARM platform needs. Aarch64 hardware that I have access
> to seems to expose SMBIOS 3.0 indeed, therefore we should do the same in
> qemu.
>
>> Is support for this all in the mainline kernel yet?
>
> Yes, it is. See (minimally)
>
> $ git log --reverse fc43026278^.. -- drivers/firmware/dmi*
>
> There are patches for arch/arm64/ and drivers/firmware/efi/ too (Ard
> will know better, he wrote at least a few of them).
>
> The userspace tools are in a more messy state AFAICT. For example I'm
> unable to locate a *git* repository for upstream dmidecode. But, Ivan
> and Roy should know better (Cc'd).

There is no git repo for dmidecode.
Only CVS: http://savannah.nongnu.org/cvs/?group=dmidecode
AFAIK, It supports SMBIOSv3 already.

+ Jean Delvare, who is creator of dmidecode, as I know.

>
> See also CARD-1779.
>
> Also, guest userspace is not QEMU's problem :)
>
> Thanks
> Laszlo
>

-- 
Regards,
Ivan Khoronzhuk

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Jean Delvare]

On Wed, 05 Aug 2015 22:39:57 +0300, Ivan Khoronzhuk wrote:
> On 05.08.15 21:35, Laszlo Ersek wrote:
> > The userspace tools are in a more messy state AFAICT. For example I'm
> > unable to locate a *git* repository for upstream dmidecode. But, Ivan
> > and Roy should know better (Cc'd).
> 
> There is no git repo for dmidecode.
> Only CVS: http://savannah.nongnu.org/cvs/?group=dmidecode

Correct. Savannah support git now so it should be possible to convert
the CVS repository to git, and I'm all for it if it makes users and
potential contributors happy. Just I don't know how this is done and
did not have the time to look into it so far.

> AFAIK, It supports SMBIOSv3 already.

Yes it does in CVS (not in 2.12 which is the latest release.) I plan to
release dmidecode 3.0 "soon" (but that is in a few months given my
current workload.)

-- 
Jean Delvare
SUSE L3 Support

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Laszlo Ersek]

On 08/06/15 00:03, Jean Delvare wrote:
> On Wed, 05 Aug 2015 22:39:57 +0300, Ivan Khoronzhuk wrote:
>> On 05.08.15 21:35, Laszlo Ersek wrote:
>>> The userspace tools are in a more messy state AFAICT. For example I'm
>>> unable to locate a *git* repository for upstream dmidecode. But, Ivan
>>> and Roy should know better (Cc'd).
>>
>> There is no git repo for dmidecode.
>> Only CVS: http://savannah.nongnu.org/cvs/?group=dmidecode
> 
> Correct. Savannah support git now so it should be possible to convert
> the CVS repository to git, and I'm all for it if it makes users and
> potential contributors happy.

Yes, please do that, if you can find the time.

> Just I don't know how this is done and
> did not have the time to look into it so far.

I've never done it myself, so the only thing I could do to help is
google it for you, which would be useless. :)

> 
>> AFAIK, It supports SMBIOSv3 already.
> 
> Yes it does in CVS (not in 2.12 which is the latest release.) I plan to
> release dmidecode 3.0 "soon" (but that is in a few months given my
> current workload.)
> 

Thank you!
Laszlo

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Ivan Khoronzhuk]

On 06.08.15 11:07, Laszlo Ersek wrote:
> On 08/06/15 00:03, Jean Delvare wrote:
>> On Wed, 05 Aug 2015 22:39:57 +0300, Ivan Khoronzhuk wrote:
>>> On 05.08.15 21:35, Laszlo Ersek wrote:
>>>> The userspace tools are in a more messy state AFAICT. For example I'm
>>>> unable to locate a *git* repository for upstream dmidecode. But, Ivan
>>>> and Roy should know better (Cc'd).
>>>
>>> There is no git repo for dmidecode.
>>> Only CVS: http://savannah.nongnu.org/cvs/?group=dmidecode
>>
>> Correct. Savannah support git now so it should be possible to convert
>> the CVS repository to git, and I'm all for it if it makes users and
>> potential contributors happy.
>
> Yes, please do that, if you can find the time.
>
>> Just I don't know how this is done and
>> did not have the time to look into it so far.
>
> I've never done it myself, so the only thing I could do to help is
> google it for you, which would be useless. :)

FYI
git-cvsimport
Сonversion itself may take not so much time

http://www.gromacs.org/Developer_Zone/Git/Migrating_a_CVS_repository_to_GIT

http://stackoverflow.com/questions/11362676/how-to-import-a-cvs-repository-in-git-and-use-it-locally

http://git-scm.com/docs/git-cvsimport

>
>>
>>> AFAIK, It supports SMBIOSv3 already.
>>
>> Yes it does in CVS (not in 2.12 which is the latest release.) I plan to
>> release dmidecode 3.0 "soon" (but that is in a few months given my
>> current workload.)
>>
>
> Thank you!
> Laszlo
>

-- 
Regards,
Ivan Khoronzhuk

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Jean Delvare]

On Thu, 06 Aug 2015 11:16:10 +0300, Ivan Khoronzhuk wrote:
> On 06.08.15 11:07, Laszlo Ersek wrote:
> > On 08/06/15 00:03, Jean Delvare wrote:
> >> On Wed, 05 Aug 2015 22:39:57 +0300, Ivan Khoronzhuk wrote:
> >>> On 05.08.15 21:35, Laszlo Ersek wrote:
> >>>> The userspace tools are in a more messy state AFAICT. For example I'm
> >>>> unable to locate a *git* repository for upstream dmidecode. But, Ivan
> >>>> and Roy should know better (Cc'd).
> >>>
> >>> There is no git repo for dmidecode.
> >>> Only CVS: http://savannah.nongnu.org/cvs/?group=dmidecode
> >>
> >> Correct. Savannah support git now so it should be possible to convert
> >> the CVS repository to git, and I'm all for it if it makes users and
> >> potential contributors happy.
> >
> > Yes, please do that, if you can find the time.
> >
> >> Just I don't know how this is done and
> >> did not have the time to look into it so far.
> >
> > I've never done it myself, so the only thing I could do to help is
> > google it for you, which would be useless. :)
> 
> FYI
> git-cvsimport
> Сonversion itself may take not so much time
> 
> http://www.gromacs.org/Developer_Zone/Git/Migrating_a_CVS_repository_to_GIT
> 
> http://stackoverflow.com/questions/11362676/how-to-import-a-cvs-repository-in-git-and-use-it-locally
> 
> http://git-scm.com/docs/git-cvsimport

I know the command, I'm more interested in how to do it (properly) in
Savannah. There are many options and I want the conversion to be as
good as possible. Once the initial git repository is pushed to
Savannah, there is no way back. This is where I would welcome hints and
suggestions, if others have gone through the same process for other
Savannah projects already.

I tried following the instructions from:

http://savannah.gnu.org/maintenance/UsingGit/

(section Importing from CVS) and it looks promising locally. Now I must
figure out how to feed the result back into Savannah. And then ideally
turn the CVS repository read-only, but I can't seem to find any option
in the Savannah administration page to do that.

-- 
Jean Delvare
SUSE L3 Support

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Andrew Jones]

On Wed, Aug 05, 2015 at 08:35:16PM +0200, Laszlo Ersek wrote:
> On 08/05/15 19:35, Peter Maydell wrote:
> > On 5 August 2015 at 18:16, Laszlo Ersek <lersek@redhat.com> wrote:
> >> On 07/28/15 08:00, Wei Huang wrote:
[...]
> SMBIOS is usually not needed by the guest OS or the firmware. The spec says,

I think SMBIOS is the only way we can expose to an ACPI-booted ARM
guest (without heuristics) that it's a QEMU/KVM guest. While that
information isn't useful in most cases, and we should do our best to
avoid its need in almost all cases, there are times it's nice to have
(or necessary for distros...)

drew

Re: [Qemu-devel] [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM

[Ard Biesheuvel]

On 5 August 2015 at 20:35, Laszlo Ersek <lersek@redhat.com> wrote:
> On 08/05/15 19:35, Peter Maydell wrote:
[...]
>> Is support for [SMBIOS 3.0] in the mainline kernel yet?
>
> Yes, it is. See (minimally)
>
> $ git log --reverse fc43026278^.. -- drivers/firmware/dmi*
>
> There are patches for arch/arm64/ and drivers/firmware/efi/ too (Ard
> will know better, he wrote at least a few of them).
>

The primary reason for supporting SMBIOS 3.0 is to allow the SMBIOS
tables to reside at an offset > 4 GB. The SMBIOS 3.0 entry point has a
64-bit offset field, where the pre-3.0 entry point has only 32-bits.
SMBIOS 3.0 introduces other changes as well, but none of those are
essential for AArch64 support. Unlike PCs, whose RAM always starts at
0x0, AArch64 systems may not have any memory below 4 GB, so it would
be impossible to support SMBIOS on those systems.

-- 
Ard.

Re: [Qemu-devel] dmidecode repository (Was: [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM)

[Jean Delvare]

On Thu, 6 Aug 2015 10:07:40 +0200, Laszlo Ersek wrote:
> On 08/06/15 00:03, Jean Delvare wrote:
> > On Wed, 05 Aug 2015 22:39:57 +0300, Ivan Khoronzhuk wrote:
> >> There is no git repo for dmidecode.
> >> Only CVS: http://savannah.nongnu.org/cvs/?group=dmidecode
> > 
> > Correct. Savannah support git now so it should be possible to convert
> > the CVS repository to git, and I'm all for it if it makes users and
> > potential contributors happy.
> 
> Yes, please do that, if you can find the time.
> 
> > Just I don't know how this is done and
> > did not have the time to look into it so far.
> 
> I've never done it myself, so the only thing I could do to help is
> google it for you, which would be useless. :)

OK, I think I came up with something that looks reasonably good:

http://git.savannah.gnu.org/cgit/dmidecode.git

Can anyone please check it out and verify that it looks sane and can be
worked with?

If it's OK then I'll tag the CVS repository as deprecated.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [Qemu-devel] dmidecode repository (Was: [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM)

[Laszlo Ersek]

On 08/10/15 09:43, Jean Delvare wrote:
> On Thu, 6 Aug 2015 10:07:40 +0200, Laszlo Ersek wrote:
>> On 08/06/15 00:03, Jean Delvare wrote:
>>> On Wed, 05 Aug 2015 22:39:57 +0300, Ivan Khoronzhuk wrote:
>>>> There is no git repo for dmidecode.
>>>> Only CVS: http://savannah.nongnu.org/cvs/?group=dmidecode
>>>
>>> Correct. Savannah support git now so it should be possible to convert
>>> the CVS repository to git, and I'm all for it if it makes users and
>>> potential contributors happy.
>>
>> Yes, please do that, if you can find the time.
>>
>>> Just I don't know how this is done and
>>> did not have the time to look into it so far.
>>
>> I've never done it myself, so the only thing I could do to help is
>> google it for you, which would be useless. :)
> 
> OK, I think I came up with something that looks reasonably good:
> 
> http://git.savannah.gnu.org/cgit/dmidecode.git
> 
> Can anyone please check it out and verify that it looks sane and can be
> worked with?

I cloned it and built it with "make". (That's all the "testing" I did. :))

Ideas:
- please consider tagging commits that correspond to releases

- probably useful to tag the git commit somehow that marks the switch
  from CVS to git (eg. "last_patch_from_cvs").

- after building, "git status" lists the *.o files and the built
  binaries as untracked files. For the former, please add a .gitignore
  file. For the latter, please list them individually in .gitignore too,
  or else build things in a separate directory, and ignore everything
  inside that directory.

> If it's OK then I'll tag the CVS repository as deprecated.

If you can ascertain that the latest tree in git (at
"last_patch_from_cvs") matches the latest tree in CVS (with a recursive
diff excluding the SCM meta-dirs), there's no reason to delay switching
to git. If you realize later that something's "wrong", you can format
the new patches from git and reapply them to CVS. (But I don't expect
anything to go wrong.)

Thanks
Laszlo

Re: [Qemu-devel] dmidecode repository (Was: [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM)

[Jean Delvare]

Hi Laszlo,

On Mon, 10 Aug 2015 13:58:31 +0200, Laszlo Ersek wrote:
> On 08/10/15 09:43, Jean Delvare wrote:
> > OK, I think I came up with something that looks reasonably good:
> > 
> > http://git.savannah.gnu.org/cgit/dmidecode.git
> > 
> > Can anyone please check it out and verify that it looks sane and can be
> > worked with?
> 
> I cloned it and built it with "make". (That's all the "testing" I did. :))

Thanks for testing and reporting.

> Ideas:
> - please consider tagging commits that correspond to releases

The conversion already did exactly that as far as I can see:

dmidecode$ git tag
dmidecode-1-8
dmidecode-2-0
dmidecode-2-1
dmidecode-2-10
dmidecode-2-11
dmidecode-2-12
dmidecode-2-2
dmidecode-2-3
dmidecode-2-4
dmidecode-2-5
dmidecode-2-6
dmidecode-2-7
dmidecode-2-8
dmidecode-2-9

And the tags appear in the web frontend too so my attempt to push them
there must have worked.

> - probably useful to tag the git commit somehow that marks the switch
>   from CVS to git (eg. "last_patch_from_cvs").

The conversion guide suggested tagging the cvs repository and I intend
to do so. But tagging the git repository seems like adding noise to me,
I can't see why anybody should care about the migration point.

> - after building, "git status" lists the *.o files and the built
>   binaries as untracked files. For the former, please add a .gitignore
>   file. For the latter, please list them individually in .gitignore too,
>   or else build things in a separate directory, and ignore everything
>   inside that directory.

I had noticed too and that was on my to-do list. Now this is done,
thanks for the reminder. Please pull again and "git status" should be
quiet now.

> > If it's OK then I'll tag the CVS repository as deprecated.
> 
> If you can ascertain that the latest tree in git (at
> "last_patch_from_cvs") matches the latest tree in CVS (with a recursive
> diff excluding the SCM meta-dirs), there's no reason to delay switching

I already did that comparison and the result is positive.

> to git. If you realize later that something's "wrong", you can format
> the new patches from git and reapply them to CVS. (But I don't expect
> anything to go wrong.)

I am more worried about the history being incorrect, due to incorrect
or missing options during the conversion. That being said, the history
of dmidecode is very simple (which is why I did not bother switching to
git so far) so hopefully the basic settings were good enough.

-- 
Jean Delvare
SUSE L3 Support

Re: [Qemu-devel] dmidecode repository (Was: [ARM SMBIOS V1 PATCH 0/6] SMBIOS Support for ARM)

[Laszlo Ersek]

On 08/10/15 14:26, Jean Delvare wrote:
> Hi Laszlo,
> 
> On Mon, 10 Aug 2015 13:58:31 +0200, Laszlo Ersek wrote:
>> On 08/10/15 09:43, Jean Delvare wrote:
>>> OK, I think I came up with something that looks reasonably good:
>>>
>>> http://git.savannah.gnu.org/cgit/dmidecode.git
>>>
>>> Can anyone please check it out and verify that it looks sane and can be
>>> worked with?
>>
>> I cloned it and built it with "make". (That's all the "testing" I did. :))
> 
> Thanks for testing and reporting.
> 
>> Ideas:
>> - please consider tagging commits that correspond to releases
> 
> The conversion already did exactly that as far as I can see:
> 
> dmidecode$ git tag
> dmidecode-1-8
> dmidecode-2-0
> dmidecode-2-1
> dmidecode-2-10
> dmidecode-2-11
> dmidecode-2-12
> dmidecode-2-2
> dmidecode-2-3
> dmidecode-2-4
> dmidecode-2-5
> dmidecode-2-6
> dmidecode-2-7
> dmidecode-2-8
> dmidecode-2-9
> 
> And the tags appear in the web frontend too so my attempt to push them
> there must have worked.

Interesting. I didn't specify --no-tags (and it also wasn't in effect in
my global ~/.gitconfig). "git tag" doesn't print anything in my clone.

> 
>> - probably useful to tag the git commit somehow that marks the switch
>>   from CVS to git (eg. "last_patch_from_cvs").
> 
> The conversion guide suggested tagging the cvs repository and I intend
> to do so. But tagging the git repository seems like adding noise to me,
> I can't see why anybody should care about the migration point.

I have no experience here, it was just an idea. Tagging the CVS repo is
probably a good idea (could work as a "stop sign"). Tagging the git repo
would be nice because one could easily list "patches from before, and
in, the git era". Maybe it's not useful for end users (and you could
just make a note about the git hash somewhere else).

> 
>> - after building, "git status" lists the *.o files and the built
>>   binaries as untracked files. For the former, please add a .gitignore
>>   file. For the latter, please list them individually in .gitignore too,
>>   or else build things in a separate directory, and ignore everything
>>   inside that directory.
> 
> I had noticed too and that was on my to-do list. Now this is done,
> thanks for the reminder. Please pull again and "git status" should be
> quiet now.

Looks good, thanks!

> 
>>> If it's OK then I'll tag the CVS repository as deprecated.
>>
>> If you can ascertain that the latest tree in git (at
>> "last_patch_from_cvs") matches the latest tree in CVS (with a recursive
>> diff excluding the SCM meta-dirs), there's no reason to delay switching
> 
> I already did that comparison and the result is positive.
> 
>> to git. If you realize later that something's "wrong", you can format
>> the new patches from git and reapply them to CVS. (But I don't expect
>> anything to go wrong.)
> 
> I am more worried about the history being incorrect, due to incorrect
> or missing options during the conversion. That being said, the history
> of dmidecode is very simple (which is why I did not bother switching to
> git so far) so hopefully the basic settings were good enough.

Thank you for migrating to git!
Laszlo