[PATCH v2 0/4] target/riscv: SMBIOS support for RISC-V virt machine

[PATCH v2 0/4] target/riscv: SMBIOS support for RISC-V virt machine

[Heinrich Schuchardt]

Generate SMBIOS tables for the RISC-V mach-virt.
Add CONFIG_SMBIOS=y to the RISC-V default config.

With the series the following firmware tables are provided:

    etc/smbios/smbios-anchor
    etc/smbios/smbios-tables

Add processor-family to the '-smbios type=4' command line options.

Heinrich Schuchardt (4):
  smbios: add processor-family option
  smbios: function to set default processor family
  target/riscv: SMBIOS support for RISC-V virt machine
  qemu-options: enable -smbios option on RISC-V

 hw/riscv/Kconfig             |  1 +
 hw/riscv/virt.c              | 42 ++++++++++++++++++++++++++++++++++++
 hw/smbios/smbios.c           | 20 ++++++++++++++--
 include/hw/firmware/smbios.h |  1 +
 qemu-options.hx              |  6 +++---
 5 files changed, 65 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH v2 1/4] smbios: add processor-family option

[Heinrich Schuchardt]

For RISC-V the SMBIOS standard requires specific values of the processor
family value depending on the bitness of the CPU.

Add a processor-family option for SMBIOS table 4.

The value of processor-family may exceed 255 and therefore must be provided
in the Processor Family 2 field. Set the Processor Family field to 0xFE
which signals that the Processor Family 2 is used.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	new patch
---
 hw/smbios/smbios.c | 13 +++++++++++--
 qemu-options.hx    |  4 ++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 2a90601ac5..647bc6d603 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -102,6 +102,7 @@ static struct {
 #define DEFAULT_CPU_SPEED 2000
 
 static struct {
+    uint16_t processor_family;
     const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
     uint64_t max_speed;
     uint64_t current_speed;
@@ -110,6 +111,7 @@ static struct {
     .max_speed = DEFAULT_CPU_SPEED,
     .current_speed = DEFAULT_CPU_SPEED,
     .processor_id = 0,
+    .processor_family = 0x01, /* Other */
 };
 
 struct type8_instance {
@@ -337,6 +339,10 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
         .name = "part",
         .type = QEMU_OPT_STRING,
         .help = "part number",
+    }, {
+        .name = "processor-family",
+        .type = QEMU_OPT_NUMBER,
+        .help = "processor family",
     }, {
         .name = "processor-id",
         .type = QEMU_OPT_NUMBER,
@@ -726,7 +732,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
     snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
     SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
     t->processor_type = 0x03; /* CPU */
-    t->processor_family = 0x01; /* Other */
+    t->processor_family = 0xfe; /* use Processor Family 2 field */
     SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
     if (type4.processor_id == 0) {
         t->processor_id[0] = cpu_to_le32(smbios_cpuid_version);
@@ -758,7 +764,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
     t->thread_count = (threads_per_socket > 255) ? 0xFF : threads_per_socket;
 
     t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
-    t->processor_family2 = cpu_to_le16(0x01); /* Other */
+    t->processor_family2 = cpu_to_le16(type4.processor_family);
 
     if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
         t->core_count2 = t->core_enabled2 = cpu_to_le16(cores_per_socket);
@@ -1402,6 +1408,9 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
                 return;
             }
             save_opt(&type4.sock_pfx, opts, "sock_pfx");
+            type4.processor_family = qemu_opt_get_number(opts,
+                                                         "processor-family",
+                                                         0x01 /* Other */);
             save_opt(&type4.manufacturer, opts, "manufacturer");
             save_opt(&type4.version, opts, "version");
             save_opt(&type4.serial, opts, "serial");
diff --git a/qemu-options.hx b/qemu-options.hx
index b66570ae00..7bdb414345 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2694,7 +2694,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
     "                specify SMBIOS type 3 fields\n"
     "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n"
     "              [,asset=str][,part=str][,max-speed=%d][,current-speed=%d]\n"
-    "              [,processor-id=%d]\n"
+    "              [,processor-family=%d,processor-id=%d]\n"
     "                specify SMBIOS type 4 fields\n"
     "-smbios type=8[,external_reference=str][,internal_reference=str][,connector_type=%d][,port_type=%d]\n"
     "                specify SMBIOS type 8 fields\n"
@@ -2722,7 +2722,7 @@ SRST
 ``-smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str][,sku=str]``
     Specify SMBIOS type 3 fields
 
-``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-id=%d]``
+``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-family=%d][,processor-id=%d]``
     Specify SMBIOS type 4 fields
 
 ``-smbios type=11[,value=str][,path=filename]``
-- 
2.43.0

[PATCH v2 2/4] smbios: function to set default processor family

[Heinrich Schuchardt]

Provide a function to set the default processor family.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	new patch
---
 hw/smbios/smbios.c           | 7 +++++++
 include/hw/firmware/smbios.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 647bc6d603..03fe736565 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -989,6 +989,13 @@ void smbios_set_cpuid(uint32_t version, uint32_t features)
         field = value;                                                    \
     }
 
+void smbios_set_default_processor_family(uint16_t processor_family)
+{
+    if (type4.processor_family <= 0x01) {
+        type4.processor_family = processor_family;
+    }
+}
+
 void smbios_set_defaults(const char *manufacturer, const char *product,
                          const char *version, bool legacy_mode,
                          bool uuid_encoded, SmbiosEntryPointType ep_type)
diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
index 7f3259a630..6e514982d4 100644
--- a/include/hw/firmware/smbios.h
+++ b/include/hw/firmware/smbios.h
@@ -295,6 +295,7 @@ 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);
+void smbios_set_default_processor_family(uint16_t processor_family);
 uint8_t *smbios_get_table_legacy(MachineState *ms, size_t *length);
 void smbios_get_tables(MachineState *ms,
                        const struct smbios_phys_mem_area *mem_array,
-- 
2.43.0

[PATCH v2 3/4] target/riscv: SMBIOS support for RISC-V virt machine

[Heinrich Schuchardt]

Generate SMBIOS tables for the RISC-V mach-virt.
Add CONFIG_SMBIOS=y to the RISC-V default config.
Set the default processor family in the type 4 table.

The implementation is based on the corresponding ARM and Loongson code.

With the patch the following firmware tables are provided:

    etc/smbios/smbios-anchor
    etc/smbios/smbios-tables

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	set processor family
---
 hw/riscv/Kconfig |  1 +
 hw/riscv/virt.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index b6a5eb4452..1e11ac9432 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -41,6 +41,7 @@ config RISCV_VIRT
     select RISCV_IMSIC
     select SIFIVE_PLIC
     select SIFIVE_TEST
+    select SMBIOS
     select VIRTIO_MMIO
     select FW_CFG_DMA
     select PLATFORM_BUS
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index d2eac24156..a876dd8f34 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -36,6 +36,7 @@
 #include "hw/riscv/boot.h"
 #include "hw/riscv/numa.h"
 #include "kvm/kvm_riscv.h"
+#include "hw/firmware/smbios.h"
 #include "hw/intc/riscv_aclint.h"
 #include "hw/intc/riscv_aplic.h"
 #include "hw/intc/riscv_imsic.h"
@@ -1249,6 +1250,45 @@ static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
                                 sysbus_mmio_get_region(sysbus, 0));
 }
 
+static void virt_build_smbios(RISCVVirtState *s)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(s);
+    MachineState *ms = MACHINE(s);
+    uint8_t *smbios_tables, *smbios_anchor;
+    size_t smbios_tables_len, smbios_anchor_len;
+    struct smbios_phys_mem_area mem_array;
+    const char *product = "QEMU Virtual Machine";
+
+    if (kvm_enabled()) {
+        product = "KVM Virtual Machine";
+    }
+
+    smbios_set_defaults("QEMU", product, mc->name, false,
+                        true, SMBIOS_ENTRY_POINT_TYPE_64);
+
+#if defined(TARGET_RISCV32)
+    smbios_set_default_processor_family(0x200);
+#elif defined(TARGET_RISCV64)
+    smbios_set_default_processor_family(0x201);
+#endif
+
+    /* build the array of physical mem area from base_memmap */
+    mem_array.address = s->memmap[VIRT_DRAM].base;
+    mem_array.length = ms->ram_size;
+
+    smbios_get_tables(ms, &mem_array, 1,
+                      &smbios_tables, &smbios_tables_len,
+                      &smbios_anchor, &smbios_anchor_len,
+                      &error_fatal);
+
+    if (smbios_anchor) {
+        fw_cfg_add_file(s->fw_cfg, "etc/smbios/smbios-tables",
+                        smbios_tables, smbios_tables_len);
+        fw_cfg_add_file(s->fw_cfg, "etc/smbios/smbios-anchor",
+                        smbios_anchor, smbios_anchor_len);
+    }
+}
+
 static void virt_machine_done(Notifier *notifier, void *data)
 {
     RISCVVirtState *s = container_of(notifier, RISCVVirtState,
@@ -1337,6 +1377,8 @@ static void virt_machine_done(Notifier *notifier, void *data)
         riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
     }
 
+    virt_build_smbios(s);
+
     if (virt_is_acpi_enabled(s)) {
         virt_acpi_setup(s);
     }
-- 
2.43.0

[PATCH v2 4/4] qemu-options: enable -smbios option on RISC-V

[Heinrich Schuchardt]

With SMBIOS support added for RISC-V we also should enable the command line
option.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	new patch
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 7bdb414345..5ed82df11f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2705,7 +2705,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
     "                specify SMBIOS type 17 fields\n"
     "-smbios type=41[,designation=str][,kind=str][,instance=%d][,pcidev=str]\n"
     "                specify SMBIOS type 41 fields\n",
-    QEMU_ARCH_I386 | QEMU_ARCH_ARM | QEMU_ARCH_LOONGARCH)
+    QEMU_ARCH_I386 | QEMU_ARCH_ARM | QEMU_ARCH_LOONGARCH | QEMU_ARCH_RISCV)
 SRST
 ``-smbios file=binary``
     Load SMBIOS entry from binary file.
-- 
2.43.0

Re: [PATCH v2 3/4] target/riscv: SMBIOS support for RISC-V virt machine

[Daniel Henrique Barboza]

On 12/29/23 09:07, Heinrich Schuchardt wrote:
> Generate SMBIOS tables for the RISC-V mach-virt.
> Add CONFIG_SMBIOS=y to the RISC-V default config.
> Set the default processor family in the type 4 table.
> 
> The implementation is based on the corresponding ARM and Loongson code.
> 
> With the patch the following firmware tables are provided:
> 
>      etc/smbios/smbios-anchor
>      etc/smbios/smbios-tables
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	set processor family
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>


>   hw/riscv/Kconfig |  1 +
>   hw/riscv/virt.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 43 insertions(+)
> 
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index b6a5eb4452..1e11ac9432 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -41,6 +41,7 @@ config RISCV_VIRT
>       select RISCV_IMSIC
>       select SIFIVE_PLIC
>       select SIFIVE_TEST
> +    select SMBIOS
>       select VIRTIO_MMIO
>       select FW_CFG_DMA
>       select PLATFORM_BUS
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d2eac24156..a876dd8f34 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -36,6 +36,7 @@
>   #include "hw/riscv/boot.h"
>   #include "hw/riscv/numa.h"
>   #include "kvm/kvm_riscv.h"
> +#include "hw/firmware/smbios.h"
>   #include "hw/intc/riscv_aclint.h"
>   #include "hw/intc/riscv_aplic.h"
>   #include "hw/intc/riscv_imsic.h"
> @@ -1249,6 +1250,45 @@ static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
>                                   sysbus_mmio_get_region(sysbus, 0));
>   }
>   
> +static void virt_build_smbios(RISCVVirtState *s)
> +{
> +    MachineClass *mc = MACHINE_GET_CLASS(s);
> +    MachineState *ms = MACHINE(s);
> +    uint8_t *smbios_tables, *smbios_anchor;
> +    size_t smbios_tables_len, smbios_anchor_len;
> +    struct smbios_phys_mem_area mem_array;
> +    const char *product = "QEMU Virtual Machine";
> +
> +    if (kvm_enabled()) {
> +        product = "KVM Virtual Machine";
> +    }
> +
> +    smbios_set_defaults("QEMU", product, mc->name, false,
> +                        true, SMBIOS_ENTRY_POINT_TYPE_64);
> +
> +#if defined(TARGET_RISCV32)
> +    smbios_set_default_processor_family(0x200);
> +#elif defined(TARGET_RISCV64)
> +    smbios_set_default_processor_family(0x201);
> +#endif
> +
> +    /* build the array of physical mem area from base_memmap */
> +    mem_array.address = s->memmap[VIRT_DRAM].base;
> +    mem_array.length = ms->ram_size;
> +
> +    smbios_get_tables(ms, &mem_array, 1,
> +                      &smbios_tables, &smbios_tables_len,
> +                      &smbios_anchor, &smbios_anchor_len,
> +                      &error_fatal);
> +
> +    if (smbios_anchor) {
> +        fw_cfg_add_file(s->fw_cfg, "etc/smbios/smbios-tables",
> +                        smbios_tables, smbios_tables_len);
> +        fw_cfg_add_file(s->fw_cfg, "etc/smbios/smbios-anchor",
> +                        smbios_anchor, smbios_anchor_len);
> +    }
> +}
> +
>   static void virt_machine_done(Notifier *notifier, void *data)
>   {
>       RISCVVirtState *s = container_of(notifier, RISCVVirtState,
> @@ -1337,6 +1377,8 @@ static void virt_machine_done(Notifier *notifier, void *data)
>           riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
>       }
>   
> +    virt_build_smbios(s);
> +
>       if (virt_is_acpi_enabled(s)) {
>           virt_acpi_setup(s);
>       }

Re: [PATCH v2 4/4] qemu-options: enable -smbios option on RISC-V

[Daniel Henrique Barboza]

On 12/29/23 09:07, Heinrich Schuchardt wrote:
> With SMBIOS support added for RISC-V we also should enable the command line
> option.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	new patch
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   qemu-options.hx | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 7bdb414345..5ed82df11f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2705,7 +2705,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
>       "                specify SMBIOS type 17 fields\n"
>       "-smbios type=41[,designation=str][,kind=str][,instance=%d][,pcidev=str]\n"
>       "                specify SMBIOS type 41 fields\n",
> -    QEMU_ARCH_I386 | QEMU_ARCH_ARM | QEMU_ARCH_LOONGARCH)
> +    QEMU_ARCH_I386 | QEMU_ARCH_ARM | QEMU_ARCH_LOONGARCH | QEMU_ARCH_RISCV)
>   SRST
>   ``-smbios file=binary``
>       Load SMBIOS entry from binary file.

Re: [PATCH v2 1/4] smbios: add processor-family option

[Alistair Francis]

On Fri, Dec 29, 2023 at 10:48 PM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> For RISC-V the SMBIOS standard requires specific values of the processor
> family value depending on the bitness of the CPU.

Can you provide some details of where this is described? I can't seem to find it

Alistair

>
> Add a processor-family option for SMBIOS table 4.
>
> The value of processor-family may exceed 255 and therefore must be provided
> in the Processor Family 2 field. Set the Processor Family field to 0xFE
> which signals that the Processor Family 2 is used.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
>         new patch
> ---
>  hw/smbios/smbios.c | 13 +++++++++++--
>  qemu-options.hx    |  4 ++--
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 2a90601ac5..647bc6d603 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -102,6 +102,7 @@ static struct {
>  #define DEFAULT_CPU_SPEED 2000
>
>  static struct {
> +    uint16_t processor_family;
>      const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
>      uint64_t max_speed;
>      uint64_t current_speed;
> @@ -110,6 +111,7 @@ static struct {
>      .max_speed = DEFAULT_CPU_SPEED,
>      .current_speed = DEFAULT_CPU_SPEED,
>      .processor_id = 0,
> +    .processor_family = 0x01, /* Other */
>  };
>
>  struct type8_instance {
> @@ -337,6 +339,10 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
>          .name = "part",
>          .type = QEMU_OPT_STRING,
>          .help = "part number",
> +    }, {
> +        .name = "processor-family",
> +        .type = QEMU_OPT_NUMBER,
> +        .help = "processor family",
>      }, {
>          .name = "processor-id",
>          .type = QEMU_OPT_NUMBER,
> @@ -726,7 +732,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>      snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
>      SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
>      t->processor_type = 0x03; /* CPU */
> -    t->processor_family = 0x01; /* Other */
> +    t->processor_family = 0xfe; /* use Processor Family 2 field */
>      SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
>      if (type4.processor_id == 0) {
>          t->processor_id[0] = cpu_to_le32(smbios_cpuid_version);
> @@ -758,7 +764,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>      t->thread_count = (threads_per_socket > 255) ? 0xFF : threads_per_socket;
>
>      t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> -    t->processor_family2 = cpu_to_le16(0x01); /* Other */
> +    t->processor_family2 = cpu_to_le16(type4.processor_family);
>
>      if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
>          t->core_count2 = t->core_enabled2 = cpu_to_le16(cores_per_socket);
> @@ -1402,6 +1408,9 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
>                  return;
>              }
>              save_opt(&type4.sock_pfx, opts, "sock_pfx");
> +            type4.processor_family = qemu_opt_get_number(opts,
> +                                                         "processor-family",
> +                                                         0x01 /* Other */);
>              save_opt(&type4.manufacturer, opts, "manufacturer");
>              save_opt(&type4.version, opts, "version");
>              save_opt(&type4.serial, opts, "serial");
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b66570ae00..7bdb414345 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2694,7 +2694,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
>      "                specify SMBIOS type 3 fields\n"
>      "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n"
>      "              [,asset=str][,part=str][,max-speed=%d][,current-speed=%d]\n"
> -    "              [,processor-id=%d]\n"
> +    "              [,processor-family=%d,processor-id=%d]\n"
>      "                specify SMBIOS type 4 fields\n"
>      "-smbios type=8[,external_reference=str][,internal_reference=str][,connector_type=%d][,port_type=%d]\n"
>      "                specify SMBIOS type 8 fields\n"
> @@ -2722,7 +2722,7 @@ SRST
>  ``-smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str][,sku=str]``
>      Specify SMBIOS type 3 fields
>
> -``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-id=%d]``
> +``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-family=%d][,processor-id=%d]``
>      Specify SMBIOS type 4 fields
>
>  ``-smbios type=11[,value=str][,path=filename]``
> --
> 2.43.0
>
>

Re: [PATCH v2 4/4] qemu-options: enable -smbios option on RISC-V

[Alistair Francis]

On Fri, Dec 29, 2023 at 10:08 PM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> With SMBIOS support added for RISC-V we also should enable the command line
> option.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> v2:
>         new patch
> ---
>  qemu-options.hx | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 7bdb414345..5ed82df11f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2705,7 +2705,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
>      "                specify SMBIOS type 17 fields\n"
>      "-smbios type=41[,designation=str][,kind=str][,instance=%d][,pcidev=str]\n"
>      "                specify SMBIOS type 41 fields\n",
> -    QEMU_ARCH_I386 | QEMU_ARCH_ARM | QEMU_ARCH_LOONGARCH)
> +    QEMU_ARCH_I386 | QEMU_ARCH_ARM | QEMU_ARCH_LOONGARCH | QEMU_ARCH_RISCV)
>  SRST
>  ``-smbios file=binary``
>      Load SMBIOS entry from binary file.
> --
> 2.43.0
>
>

Re: [PATCH v2 1/4] smbios: add processor-family option

[Heinrich Schuchardt]

On 1/5/24 06:24, Alistair Francis wrote:
> On Fri, Dec 29, 2023 at 10:48 PM Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> For RISC-V the SMBIOS standard requires specific values of the processor
>> family value depending on the bitness of the CPU.
> 
> Can you provide some details of where this is described? I can't seem to find it
> 
> Alistair

System Management BIOS (SMBIOS) Reference Specification 3.7.0 (DSP0134)
https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.7.0.pdf
7.5.2 Processor Information — Processor Family
Table 23 – Processor Information: Processor Family field

200h 512 RISC-V RV32
201h 513 RISC-V RV64
202h 514 RISC-V RV128

While for other architectures values for different CPU generations have 
been defined the values for RISC-V only depend on the bitness.

Best regards

Heinrich

> 
>>
>> Add a processor-family option for SMBIOS table 4.
>>
>> The value of processor-family may exceed 255 and therefore must be provided
>> in the Processor Family 2 field. Set the Processor Family field to 0xFE
>> which signals that the Processor Family 2 is used.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>> v2:
>>          new patch
>> ---
>>   hw/smbios/smbios.c | 13 +++++++++++--
>>   qemu-options.hx    |  4 ++--
>>   2 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
>> index 2a90601ac5..647bc6d603 100644
>> --- a/hw/smbios/smbios.c
>> +++ b/hw/smbios/smbios.c
>> @@ -102,6 +102,7 @@ static struct {
>>   #define DEFAULT_CPU_SPEED 2000
>>
>>   static struct {
>> +    uint16_t processor_family;
>>       const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
>>       uint64_t max_speed;
>>       uint64_t current_speed;
>> @@ -110,6 +111,7 @@ static struct {
>>       .max_speed = DEFAULT_CPU_SPEED,
>>       .current_speed = DEFAULT_CPU_SPEED,
>>       .processor_id = 0,
>> +    .processor_family = 0x01, /* Other */
>>   };
>>
>>   struct type8_instance {
>> @@ -337,6 +339,10 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
>>           .name = "part",
>>           .type = QEMU_OPT_STRING,
>>           .help = "part number",
>> +    }, {
>> +        .name = "processor-family",
>> +        .type = QEMU_OPT_NUMBER,
>> +        .help = "processor family",
>>       }, {
>>           .name = "processor-id",
>>           .type = QEMU_OPT_NUMBER,
>> @@ -726,7 +732,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>>       snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
>>       SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
>>       t->processor_type = 0x03; /* CPU */
>> -    t->processor_family = 0x01; /* Other */
>> +    t->processor_family = 0xfe; /* use Processor Family 2 field */
>>       SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
>>       if (type4.processor_id == 0) {
>>           t->processor_id[0] = cpu_to_le32(smbios_cpuid_version);
>> @@ -758,7 +764,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>>       t->thread_count = (threads_per_socket > 255) ? 0xFF : threads_per_socket;
>>
>>       t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
>> -    t->processor_family2 = cpu_to_le16(0x01); /* Other */
>> +    t->processor_family2 = cpu_to_le16(type4.processor_family);
>>
>>       if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
>>           t->core_count2 = t->core_enabled2 = cpu_to_le16(cores_per_socket);
>> @@ -1402,6 +1408,9 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
>>                   return;
>>               }
>>               save_opt(&type4.sock_pfx, opts, "sock_pfx");
>> +            type4.processor_family = qemu_opt_get_number(opts,
>> +                                                         "processor-family",
>> +                                                         0x01 /* Other */);
>>               save_opt(&type4.manufacturer, opts, "manufacturer");
>>               save_opt(&type4.version, opts, "version");
>>               save_opt(&type4.serial, opts, "serial");
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index b66570ae00..7bdb414345 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -2694,7 +2694,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
>>       "                specify SMBIOS type 3 fields\n"
>>       "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n"
>>       "              [,asset=str][,part=str][,max-speed=%d][,current-speed=%d]\n"
>> -    "              [,processor-id=%d]\n"
>> +    "              [,processor-family=%d,processor-id=%d]\n"
>>       "                specify SMBIOS type 4 fields\n"
>>       "-smbios type=8[,external_reference=str][,internal_reference=str][,connector_type=%d][,port_type=%d]\n"
>>       "                specify SMBIOS type 8 fields\n"
>> @@ -2722,7 +2722,7 @@ SRST
>>   ``-smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str][,sku=str]``
>>       Specify SMBIOS type 3 fields
>>
>> -``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-id=%d]``
>> +``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-family=%d][,processor-id=%d]``
>>       Specify SMBIOS type 4 fields
>>
>>   ``-smbios type=11[,value=str][,path=filename]``
>> --
>> 2.43.0
>>
>>

Re: [PATCH v2 1/4] smbios: add processor-family option

[Alistair Francis]

On Fri, Dec 29, 2023 at 10:48 PM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> For RISC-V the SMBIOS standard requires specific values of the processor
> family value depending on the bitness of the CPU.
>
> Add a processor-family option for SMBIOS table 4.
>
> The value of processor-family may exceed 255 and therefore must be provided
> in the Processor Family 2 field. Set the Processor Family field to 0xFE
> which signals that the Processor Family 2 is used.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> v2:
>         new patch
> ---
>  hw/smbios/smbios.c | 13 +++++++++++--
>  qemu-options.hx    |  4 ++--
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 2a90601ac5..647bc6d603 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -102,6 +102,7 @@ static struct {
>  #define DEFAULT_CPU_SPEED 2000
>
>  static struct {
> +    uint16_t processor_family;
>      const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
>      uint64_t max_speed;
>      uint64_t current_speed;
> @@ -110,6 +111,7 @@ static struct {
>      .max_speed = DEFAULT_CPU_SPEED,
>      .current_speed = DEFAULT_CPU_SPEED,
>      .processor_id = 0,
> +    .processor_family = 0x01, /* Other */
>  };
>
>  struct type8_instance {
> @@ -337,6 +339,10 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
>          .name = "part",
>          .type = QEMU_OPT_STRING,
>          .help = "part number",
> +    }, {
> +        .name = "processor-family",
> +        .type = QEMU_OPT_NUMBER,
> +        .help = "processor family",
>      }, {
>          .name = "processor-id",
>          .type = QEMU_OPT_NUMBER,
> @@ -726,7 +732,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>      snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
>      SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
>      t->processor_type = 0x03; /* CPU */
> -    t->processor_family = 0x01; /* Other */
> +    t->processor_family = 0xfe; /* use Processor Family 2 field */
>      SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
>      if (type4.processor_id == 0) {
>          t->processor_id[0] = cpu_to_le32(smbios_cpuid_version);
> @@ -758,7 +764,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>      t->thread_count = (threads_per_socket > 255) ? 0xFF : threads_per_socket;
>
>      t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> -    t->processor_family2 = cpu_to_le16(0x01); /* Other */
> +    t->processor_family2 = cpu_to_le16(type4.processor_family);
>
>      if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
>          t->core_count2 = t->core_enabled2 = cpu_to_le16(cores_per_socket);
> @@ -1402,6 +1408,9 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
>                  return;
>              }
>              save_opt(&type4.sock_pfx, opts, "sock_pfx");
> +            type4.processor_family = qemu_opt_get_number(opts,
> +                                                         "processor-family",
> +                                                         0x01 /* Other */);
>              save_opt(&type4.manufacturer, opts, "manufacturer");
>              save_opt(&type4.version, opts, "version");
>              save_opt(&type4.serial, opts, "serial");
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b66570ae00..7bdb414345 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2694,7 +2694,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
>      "                specify SMBIOS type 3 fields\n"
>      "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n"
>      "              [,asset=str][,part=str][,max-speed=%d][,current-speed=%d]\n"
> -    "              [,processor-id=%d]\n"
> +    "              [,processor-family=%d,processor-id=%d]\n"
>      "                specify SMBIOS type 4 fields\n"
>      "-smbios type=8[,external_reference=str][,internal_reference=str][,connector_type=%d][,port_type=%d]\n"
>      "                specify SMBIOS type 8 fields\n"
> @@ -2722,7 +2722,7 @@ SRST
>  ``-smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str][,sku=str]``
>      Specify SMBIOS type 3 fields
>
> -``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-id=%d]``
> +``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-family=%d][,processor-id=%d]``
>      Specify SMBIOS type 4 fields
>
>  ``-smbios type=11[,value=str][,path=filename]``
> --
> 2.43.0
>
>

Re: [PATCH v2 1/4] smbios: add processor-family option

[Alistair Francis]

On Fri, Jan 5, 2024 at 3:44 PM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 1/5/24 06:24, Alistair Francis wrote:
> > On Fri, Dec 29, 2023 at 10:48 PM Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> For RISC-V the SMBIOS standard requires specific values of the processor
> >> family value depending on the bitness of the CPU.
> >
> > Can you provide some details of where this is described? I can't seem to find it
> >
> > Alistair
>
> System Management BIOS (SMBIOS) Reference Specification 3.7.0 (DSP0134)
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.7.0.pdf
> 7.5.2 Processor Information — Processor Family
> Table 23 – Processor Information: Processor Family field
>
> 200h 512 RISC-V RV32
> 201h 513 RISC-V RV64
> 202h 514 RISC-V RV128
>
> While for other architectures values for different CPU generations have
> been defined the values for RISC-V only depend on the bitness.

Ah, I first read the commit message as saying this is RISC-V specific,
which I couldn't find in the spec.

I now realise you just mean it's important for RISC-V

Alistair

>
> Best regards
>
> Heinrich
>
> >
> >>
> >> Add a processor-family option for SMBIOS table 4.
> >>
> >> The value of processor-family may exceed 255 and therefore must be provided
> >> in the Processor Family 2 field. Set the Processor Family field to 0xFE
> >> which signals that the Processor Family 2 is used.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> ---
> >> v2:
> >>          new patch
> >> ---
> >>   hw/smbios/smbios.c | 13 +++++++++++--
> >>   qemu-options.hx    |  4 ++--
> >>   2 files changed, 13 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> >> index 2a90601ac5..647bc6d603 100644
> >> --- a/hw/smbios/smbios.c
> >> +++ b/hw/smbios/smbios.c
> >> @@ -102,6 +102,7 @@ static struct {
> >>   #define DEFAULT_CPU_SPEED 2000
> >>
> >>   static struct {
> >> +    uint16_t processor_family;
> >>       const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
> >>       uint64_t max_speed;
> >>       uint64_t current_speed;
> >> @@ -110,6 +111,7 @@ static struct {
> >>       .max_speed = DEFAULT_CPU_SPEED,
> >>       .current_speed = DEFAULT_CPU_SPEED,
> >>       .processor_id = 0,
> >> +    .processor_family = 0x01, /* Other */
> >>   };
> >>
> >>   struct type8_instance {
> >> @@ -337,6 +339,10 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
> >>           .name = "part",
> >>           .type = QEMU_OPT_STRING,
> >>           .help = "part number",
> >> +    }, {
> >> +        .name = "processor-family",
> >> +        .type = QEMU_OPT_NUMBER,
> >> +        .help = "processor family",
> >>       }, {
> >>           .name = "processor-id",
> >>           .type = QEMU_OPT_NUMBER,
> >> @@ -726,7 +732,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> >>       snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
> >>       SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
> >>       t->processor_type = 0x03; /* CPU */
> >> -    t->processor_family = 0x01; /* Other */
> >> +    t->processor_family = 0xfe; /* use Processor Family 2 field */
> >>       SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
> >>       if (type4.processor_id == 0) {
> >>           t->processor_id[0] = cpu_to_le32(smbios_cpuid_version);
> >> @@ -758,7 +764,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> >>       t->thread_count = (threads_per_socket > 255) ? 0xFF : threads_per_socket;
> >>
> >>       t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> >> -    t->processor_family2 = cpu_to_le16(0x01); /* Other */
> >> +    t->processor_family2 = cpu_to_le16(type4.processor_family);
> >>
> >>       if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
> >>           t->core_count2 = t->core_enabled2 = cpu_to_le16(cores_per_socket);
> >> @@ -1402,6 +1408,9 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
> >>                   return;
> >>               }
> >>               save_opt(&type4.sock_pfx, opts, "sock_pfx");
> >> +            type4.processor_family = qemu_opt_get_number(opts,
> >> +                                                         "processor-family",
> >> +                                                         0x01 /* Other */);
> >>               save_opt(&type4.manufacturer, opts, "manufacturer");
> >>               save_opt(&type4.version, opts, "version");
> >>               save_opt(&type4.serial, opts, "serial");
> >> diff --git a/qemu-options.hx b/qemu-options.hx
> >> index b66570ae00..7bdb414345 100644
> >> --- a/qemu-options.hx
> >> +++ b/qemu-options.hx
> >> @@ -2694,7 +2694,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
> >>       "                specify SMBIOS type 3 fields\n"
> >>       "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n"
> >>       "              [,asset=str][,part=str][,max-speed=%d][,current-speed=%d]\n"
> >> -    "              [,processor-id=%d]\n"
> >> +    "              [,processor-family=%d,processor-id=%d]\n"
> >>       "                specify SMBIOS type 4 fields\n"
> >>       "-smbios type=8[,external_reference=str][,internal_reference=str][,connector_type=%d][,port_type=%d]\n"
> >>       "                specify SMBIOS type 8 fields\n"
> >> @@ -2722,7 +2722,7 @@ SRST
> >>   ``-smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str][,sku=str]``
> >>       Specify SMBIOS type 3 fields
> >>
> >> -``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-id=%d]``
> >> +``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-family=%d][,processor-id=%d]``
> >>       Specify SMBIOS type 4 fields
> >>
> >>   ``-smbios type=11[,value=str][,path=filename]``
> >> --
> >> 2.43.0
> >>
> >>
>

Re: [PATCH v2 3/4] target/riscv: SMBIOS support for RISC-V virt machine

[Andrew Jones]

On Fri, Dec 29, 2023 at 01:07:23PM +0100, Heinrich Schuchardt wrote:
> Generate SMBIOS tables for the RISC-V mach-virt.
> Add CONFIG_SMBIOS=y to the RISC-V default config.
> Set the default processor family in the type 4 table.
> 
> The implementation is based on the corresponding ARM and Loongson code.
> 
> With the patch the following firmware tables are provided:
> 
>     etc/smbios/smbios-anchor
>     etc/smbios/smbios-tables
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	set processor family
> ---
>  hw/riscv/Kconfig |  1 +
>  hw/riscv/virt.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index b6a5eb4452..1e11ac9432 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -41,6 +41,7 @@ config RISCV_VIRT
>      select RISCV_IMSIC
>      select SIFIVE_PLIC
>      select SIFIVE_TEST
> +    select SMBIOS
>      select VIRTIO_MMIO
>      select FW_CFG_DMA
>      select PLATFORM_BUS
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index d2eac24156..a876dd8f34 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -36,6 +36,7 @@
>  #include "hw/riscv/boot.h"
>  #include "hw/riscv/numa.h"
>  #include "kvm/kvm_riscv.h"
> +#include "hw/firmware/smbios.h"
>  #include "hw/intc/riscv_aclint.h"
>  #include "hw/intc/riscv_aplic.h"
>  #include "hw/intc/riscv_imsic.h"
> @@ -1249,6 +1250,45 @@ static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
>                                  sysbus_mmio_get_region(sysbus, 0));
>  }
>  
> +static void virt_build_smbios(RISCVVirtState *s)
> +{
> +    MachineClass *mc = MACHINE_GET_CLASS(s);
> +    MachineState *ms = MACHINE(s);
> +    uint8_t *smbios_tables, *smbios_anchor;
> +    size_t smbios_tables_len, smbios_anchor_len;
> +    struct smbios_phys_mem_area mem_array;
> +    const char *product = "QEMU Virtual Machine";
> +
> +    if (kvm_enabled()) {
> +        product = "KVM Virtual Machine";
> +    }
> +
> +    smbios_set_defaults("QEMU", product, mc->name, false,
> +                        true, SMBIOS_ENTRY_POINT_TYPE_64);
> +
> +#if defined(TARGET_RISCV32)
> +    smbios_set_default_processor_family(0x200);
> +#elif defined(TARGET_RISCV64)
> +    smbios_set_default_processor_family(0x201);
> +#endif

I think we should use misa_mxl_max to determine the family, rather than
TARGET_*, because, iirc, we're slowly working our ways towards allowing
rv32 cpus to be instantiated with qemu-system-riscv64.

> +
> +    /* build the array of physical mem area from base_memmap */
> +    mem_array.address = s->memmap[VIRT_DRAM].base;
> +    mem_array.length = ms->ram_size;
> +
> +    smbios_get_tables(ms, &mem_array, 1,
> +                      &smbios_tables, &smbios_tables_len,
> +                      &smbios_anchor, &smbios_anchor_len,
> +                      &error_fatal);
> +
> +    if (smbios_anchor) {
> +        fw_cfg_add_file(s->fw_cfg, "etc/smbios/smbios-tables",
> +                        smbios_tables, smbios_tables_len);
> +        fw_cfg_add_file(s->fw_cfg, "etc/smbios/smbios-anchor",
> +                        smbios_anchor, smbios_anchor_len);
> +    }
> +}
> +
>  static void virt_machine_done(Notifier *notifier, void *data)
>  {
>      RISCVVirtState *s = container_of(notifier, RISCVVirtState,
> @@ -1337,6 +1377,8 @@ static void virt_machine_done(Notifier *notifier, void *data)
>          riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
>      }
>  
> +    virt_build_smbios(s);
> +
>      if (virt_is_acpi_enabled(s)) {
>          virt_acpi_setup(s);
>      }
> -- 
> 2.43.0
> 
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

Re: [PATCH v2 1/4] smbios: add processor-family option

[Andrew Jones]

On Fri, Dec 29, 2023 at 01:07:21PM +0100, Heinrich Schuchardt wrote:
> For RISC-V the SMBIOS standard requires specific values of the processor
> family value depending on the bitness of the CPU.
> 
> Add a processor-family option for SMBIOS table 4.
> 
> The value of processor-family may exceed 255 and therefore must be provided
> in the Processor Family 2 field. Set the Processor Family field to 0xFE
> which signals that the Processor Family 2 is used.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	new patch
> ---
>  hw/smbios/smbios.c | 13 +++++++++++--
>  qemu-options.hx    |  4 ++--
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 2a90601ac5..647bc6d603 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -102,6 +102,7 @@ static struct {
>  #define DEFAULT_CPU_SPEED 2000
>  
>  static struct {
> +    uint16_t processor_family;
>      const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
>      uint64_t max_speed;
>      uint64_t current_speed;
> @@ -110,6 +111,7 @@ static struct {
>      .max_speed = DEFAULT_CPU_SPEED,
>      .current_speed = DEFAULT_CPU_SPEED,
>      .processor_id = 0,
> +    .processor_family = 0x01, /* Other */
>  };
>  
>  struct type8_instance {
> @@ -337,6 +339,10 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
>          .name = "part",
>          .type = QEMU_OPT_STRING,
>          .help = "part number",
> +    }, {
> +        .name = "processor-family",
> +        .type = QEMU_OPT_NUMBER,
> +        .help = "processor family",
>      }, {
>          .name = "processor-id",
>          .type = QEMU_OPT_NUMBER,
> @@ -726,7 +732,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>      snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
>      SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
>      t->processor_type = 0x03; /* CPU */
> -    t->processor_family = 0x01; /* Other */
> +    t->processor_family = 0xfe; /* use Processor Family 2 field */
>      SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
>      if (type4.processor_id == 0) {
>          t->processor_id[0] = cpu_to_le32(smbios_cpuid_version);
> @@ -758,7 +764,7 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>      t->thread_count = (threads_per_socket > 255) ? 0xFF : threads_per_socket;
>  
>      t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> -    t->processor_family2 = cpu_to_le16(0x01); /* Other */
> +    t->processor_family2 = cpu_to_le16(type4.processor_family);
>  
>      if (tbl_len == SMBIOS_TYPE_4_LEN_V30) {
>          t->core_count2 = t->core_enabled2 = cpu_to_le16(cores_per_socket);
> @@ -1402,6 +1408,9 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
>                  return;
>              }
>              save_opt(&type4.sock_pfx, opts, "sock_pfx");
> +            type4.processor_family = qemu_opt_get_number(opts,
> +                                                         "processor-family",
> +                                                         0x01 /* Other */);
>              save_opt(&type4.manufacturer, opts, "manufacturer");
>              save_opt(&type4.version, opts, "version");
>              save_opt(&type4.serial, opts, "serial");
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b66570ae00..7bdb414345 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2694,7 +2694,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
>      "                specify SMBIOS type 3 fields\n"
>      "-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str]\n"
>      "              [,asset=str][,part=str][,max-speed=%d][,current-speed=%d]\n"
> -    "              [,processor-id=%d]\n"
> +    "              [,processor-family=%d,processor-id=%d]\n"
>      "                specify SMBIOS type 4 fields\n"
>      "-smbios type=8[,external_reference=str][,internal_reference=str][,connector_type=%d][,port_type=%d]\n"
>      "                specify SMBIOS type 8 fields\n"
> @@ -2722,7 +2722,7 @@ SRST
>  ``-smbios type=3[,manufacturer=str][,version=str][,serial=str][,asset=str][,sku=str]``
>      Specify SMBIOS type 3 fields
>  
> -``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-id=%d]``
> +``-smbios type=4[,sock_pfx=str][,manufacturer=str][,version=str][,serial=str][,asset=str][,part=str][,processor-family=%d][,processor-id=%d]``
>      Specify SMBIOS type 4 fields
>  
>  ``-smbios type=11[,value=str][,path=filename]``
> -- 
> 2.43.0
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [PATCH v2 2/4] smbios: function to set default processor family

[Andrew Jones]

On Fri, Dec 29, 2023 at 01:07:22PM +0100, Heinrich Schuchardt wrote:
> Provide a function to set the default processor family.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	new patch
> ---
>  hw/smbios/smbios.c           | 7 +++++++
>  include/hw/firmware/smbios.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 647bc6d603..03fe736565 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -989,6 +989,13 @@ void smbios_set_cpuid(uint32_t version, uint32_t features)
>          field = value;                                                    \
>      }
>  
> +void smbios_set_default_processor_family(uint16_t processor_family)
> +{
> +    if (type4.processor_family <= 0x01) {
> +        type4.processor_family = processor_family;
> +    }
> +}
> +
>  void smbios_set_defaults(const char *manufacturer, const char *product,
>                           const char *version, bool legacy_mode,
>                           bool uuid_encoded, SmbiosEntryPointType ep_type)
> diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> index 7f3259a630..6e514982d4 100644
> --- a/include/hw/firmware/smbios.h
> +++ b/include/hw/firmware/smbios.h
> @@ -295,6 +295,7 @@ 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);
> +void smbios_set_default_processor_family(uint16_t processor_family);
>  uint8_t *smbios_get_table_legacy(MachineState *ms, size_t *length);
>  void smbios_get_tables(MachineState *ms,
>                         const struct smbios_phys_mem_area *mem_array,
> -- 
> 2.43.0
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [PATCH v2 4/4] qemu-options: enable -smbios option on RISC-V

[Andrew Jones]

On Fri, Dec 29, 2023 at 01:07:24PM +0100, Heinrich Schuchardt wrote:
> With SMBIOS support added for RISC-V we also should enable the command line
> option.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	new patch
> ---
>  qemu-options.hx | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 7bdb414345..5ed82df11f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2705,7 +2705,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
>      "                specify SMBIOS type 17 fields\n"
>      "-smbios type=41[,designation=str][,kind=str][,instance=%d][,pcidev=str]\n"
>      "                specify SMBIOS type 41 fields\n",
> -    QEMU_ARCH_I386 | QEMU_ARCH_ARM | QEMU_ARCH_LOONGARCH)
> +    QEMU_ARCH_I386 | QEMU_ARCH_ARM | QEMU_ARCH_LOONGARCH | QEMU_ARCH_RISCV)
>  SRST
>  ``-smbios file=binary``
>      Load SMBIOS entry from binary file.
> -- 
> 2.43.0
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [PATCH v2 3/4] target/riscv: SMBIOS support for RISC-V virt machine

[Heinrich Schuchardt]

On 22.01.24 10:57, Andrew Jones wrote:
> On Fri, Dec 29, 2023 at 01:07:23PM +0100, Heinrich Schuchardt wrote:
>> Generate SMBIOS tables for the RISC-V mach-virt.
>> Add CONFIG_SMBIOS=y to the RISC-V default config.
>> Set the default processor family in the type 4 table.
>>
>> The implementation is based on the corresponding ARM and Loongson code.
>>
>> With the patch the following firmware tables are provided:
>>
>>      etc/smbios/smbios-anchor
>>      etc/smbios/smbios-tables
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>> v2:
>> 	set processor family
>> ---
>>   hw/riscv/Kconfig |  1 +
>>   hw/riscv/virt.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 43 insertions(+)
>>
>> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
>> index b6a5eb4452..1e11ac9432 100644
>> --- a/hw/riscv/Kconfig
>> +++ b/hw/riscv/Kconfig
>> @@ -41,6 +41,7 @@ config RISCV_VIRT
>>       select RISCV_IMSIC
>>       select SIFIVE_PLIC
>>       select SIFIVE_TEST
>> +    select SMBIOS
>>       select VIRTIO_MMIO
>>       select FW_CFG_DMA
>>       select PLATFORM_BUS
>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>> index d2eac24156..a876dd8f34 100644
>> --- a/hw/riscv/virt.c
>> +++ b/hw/riscv/virt.c
>> @@ -36,6 +36,7 @@
>>   #include "hw/riscv/boot.h"
>>   #include "hw/riscv/numa.h"
>>   #include "kvm/kvm_riscv.h"
>> +#include "hw/firmware/smbios.h"
>>   #include "hw/intc/riscv_aclint.h"
>>   #include "hw/intc/riscv_aplic.h"
>>   #include "hw/intc/riscv_imsic.h"
>> @@ -1249,6 +1250,45 @@ static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
>>                                   sysbus_mmio_get_region(sysbus, 0));
>>   }
>>   
>> +static void virt_build_smbios(RISCVVirtState *s)
>> +{
>> +    MachineClass *mc = MACHINE_GET_CLASS(s);
>> +    MachineState *ms = MACHINE(s);
>> +    uint8_t *smbios_tables, *smbios_anchor;
>> +    size_t smbios_tables_len, smbios_anchor_len;
>> +    struct smbios_phys_mem_area mem_array;
>> +    const char *product = "QEMU Virtual Machine";
>> +
>> +    if (kvm_enabled()) {
>> +        product = "KVM Virtual Machine";
>> +    }
>> +
>> +    smbios_set_defaults("QEMU", product, mc->name, false,
>> +                        true, SMBIOS_ENTRY_POINT_TYPE_64);
>> +
>> +#if defined(TARGET_RISCV32)
>> +    smbios_set_default_processor_family(0x200);
>> +#elif defined(TARGET_RISCV64)
>> +    smbios_set_default_processor_family(0x201);
>> +#endif
> 
> I think we should use misa_mxl_max to determine the family, rather than
> TARGET_*, because, iirc, we're slowly working our ways towards allowing
> rv32 cpus to be instantiated with qemu-system-riscv64.

Hello Andrew,

thank you for reviewing. I guess you mean something like:

     if (riscv_is_32bit(&s->soc[0])) {
         smbios_set_default_processor_family(0x200);
#if defined(TARGET_RISCV64)
     } else {
         smbios_set_default_processor_family(0x201);
#endif
     }

riscv_is_32bit returns harts->harts[0].env.misa_mxl_max == MXL_RV32.

Some real hardware has a 32bit hart and multiple 64bit harts. Will QEMU 
support mixing harts with different bitness on the virt machine in 
future? In that case we would have to revisit the code using 
misa_mxl_max in multiple places.

Best regards

Heinrich

> 
>> +
>> +    /* build the array of physical mem area from base_memmap */
>> +    mem_array.address = s->memmap[VIRT_DRAM].base;
>> +    mem_array.length = ms->ram_size;
>> +
>> +    smbios_get_tables(ms, &mem_array, 1,
>> +                      &smbios_tables, &smbios_tables_len,
>> +                      &smbios_anchor, &smbios_anchor_len,
>> +                      &error_fatal);
>> +
>> +    if (smbios_anchor) {
>> +        fw_cfg_add_file(s->fw_cfg, "etc/smbios/smbios-tables",
>> +                        smbios_tables, smbios_tables_len);
>> +        fw_cfg_add_file(s->fw_cfg, "etc/smbios/smbios-anchor",
>> +                        smbios_anchor, smbios_anchor_len);
>> +    }
>> +}
>> +
>>   static void virt_machine_done(Notifier *notifier, void *data)
>>   {
>>       RISCVVirtState *s = container_of(notifier, RISCVVirtState,
>> @@ -1337,6 +1377,8 @@ static void virt_machine_done(Notifier *notifier, void *data)
>>           riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
>>       }
>>   
>> +    virt_build_smbios(s);
>> +
>>       if (virt_is_acpi_enabled(s)) {
>>           virt_acpi_setup(s);
>>       }
>> -- 
>> 2.43.0
>>
>>
> 
> Otherwise,
> 
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> 
> Thanks,
> drew

Re: Re: [PATCH v2 3/4] target/riscv: SMBIOS support for RISC-V virt machine

[Andrew Jones]

On Mon, Jan 22, 2024 at 01:28:18PM +0100, Heinrich Schuchardt wrote:
> On 22.01.24 10:57, Andrew Jones wrote:
> > On Fri, Dec 29, 2023 at 01:07:23PM +0100, Heinrich Schuchardt wrote:
...
> > > +#if defined(TARGET_RISCV32)
> > > +    smbios_set_default_processor_family(0x200);
> > > +#elif defined(TARGET_RISCV64)
> > > +    smbios_set_default_processor_family(0x201);
> > > +#endif
> > 
> > I think we should use misa_mxl_max to determine the family, rather than
> > TARGET_*, because, iirc, we're slowly working our ways towards allowing
> > rv32 cpus to be instantiated with qemu-system-riscv64.
> 
> Hello Andrew,
> 
> thank you for reviewing. I guess you mean something like:
> 
>     if (riscv_is_32bit(&s->soc[0])) {
>         smbios_set_default_processor_family(0x200);
> #if defined(TARGET_RISCV64)
>     } else {
>         smbios_set_default_processor_family(0x201);
> #endif
>     }

Yes, but I'm not sure we need the #ifdef around the 64-bit part.

> 
> riscv_is_32bit returns harts->harts[0].env.misa_mxl_max == MXL_RV32.
> 
> Some real hardware has a 32bit hart and multiple 64bit harts. Will QEMU
> support mixing harts with different bitness on the virt machine in future?
> In that case we would have to revisit the code using misa_mxl_max in
> multiple places.
> 

Never say never, but I don't think there has been much effort to support
these types of configurations with a single QEMU binary. My googling is
failing me right now, but I seem to recall that there may have been
efforts to implement Arm big.LITTLE with multiprocess QEMU [1]. IOW, I
think we're safe to use misa_mxl_max, since we'll have one for each QEMU
instance and we'll use a different QEMU instance for each hart bitness.

[1] docs/system/multi-process.rst

Thanks,
drew

Re: [PATCH v2 3/4] target/riscv: SMBIOS support for RISC-V virt machine

[Heinrich Schuchardt]

On 22.01.24 13:59, Andrew Jones wrote:
> On Mon, Jan 22, 2024 at 01:28:18PM +0100, Heinrich Schuchardt wrote:
>> On 22.01.24 10:57, Andrew Jones wrote:
>>> On Fri, Dec 29, 2023 at 01:07:23PM +0100, Heinrich Schuchardt wrote:
> ...
>>>> +#if defined(TARGET_RISCV32)
>>>> +    smbios_set_default_processor_family(0x200);
>>>> +#elif defined(TARGET_RISCV64)
>>>> +    smbios_set_default_processor_family(0x201);
>>>> +#endif
>>>
>>> I think we should use misa_mxl_max to determine the family, rather than
>>> TARGET_*, because, iirc, we're slowly working our ways towards allowing
>>> rv32 cpus to be instantiated with qemu-system-riscv64.
>>
>> Hello Andrew,
>>
>> thank you for reviewing. I guess you mean something like:
>>
>>      if (riscv_is_32bit(&s->soc[0])) {
>>          smbios_set_default_processor_family(0x200);
>> #if defined(TARGET_RISCV64)
>>      } else {
>>          smbios_set_default_processor_family(0x201);
>> #endif
>>      }
> 
> Yes, but I'm not sure we need the #ifdef around the 64-bit part.

I just followed the style in riscv_cpu_validate_misa_mxl().

Best regards

Heinrich

> 
>>
>> riscv_is_32bit returns harts->harts[0].env.misa_mxl_max == MXL_RV32.
>>
>> Some real hardware has a 32bit hart and multiple 64bit harts. Will QEMU
>> support mixing harts with different bitness on the virt machine in future?
>> In that case we would have to revisit the code using misa_mxl_max in
>> multiple places.
>>
> 
> Never say never, but I don't think there has been much effort to support
> these types of configurations with a single QEMU binary. My googling is
> failing me right now, but I seem to recall that there may have been
> efforts to implement Arm big.LITTLE with multiprocess QEMU [1]. IOW, I
> think we're safe to use misa_mxl_max, since we'll have one for each QEMU
> instance and we'll use a different QEMU instance for each hart bitness.
> 
> [1] docs/system/multi-process.rst
> 
> Thanks,
> drew

Re: Re: [PATCH v2 3/4] target/riscv: SMBIOS support for RISC-V virt machine

[Alistair Francis]

On Tue, Jan 23, 2024 at 12:19 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Mon, Jan 22, 2024 at 01:28:18PM +0100, Heinrich Schuchardt wrote:
> > On 22.01.24 10:57, Andrew Jones wrote:
> > > On Fri, Dec 29, 2023 at 01:07:23PM +0100, Heinrich Schuchardt wrote:
> ...
> > > > +#if defined(TARGET_RISCV32)
> > > > +    smbios_set_default_processor_family(0x200);
> > > > +#elif defined(TARGET_RISCV64)
> > > > +    smbios_set_default_processor_family(0x201);
> > > > +#endif
> > >
> > > I think we should use misa_mxl_max to determine the family, rather than
> > > TARGET_*, because, iirc, we're slowly working our ways towards allowing
> > > rv32 cpus to be instantiated with qemu-system-riscv64.
> >
> > Hello Andrew,
> >
> > thank you for reviewing. I guess you mean something like:
> >
> >     if (riscv_is_32bit(&s->soc[0])) {
> >         smbios_set_default_processor_family(0x200);
> > #if defined(TARGET_RISCV64)
> >     } else {
> >         smbios_set_default_processor_family(0x201);
> > #endif
> >     }
>
> Yes, but I'm not sure we need the #ifdef around the 64-bit part.
>
> >
> > riscv_is_32bit returns harts->harts[0].env.misa_mxl_max == MXL_RV32.
> >
> > Some real hardware has a 32bit hart and multiple 64bit harts. Will QEMU
> > support mixing harts with different bitness on the virt machine in future?

That is the hope. Although no one is directly working on it at the
moment. We want to try and keep the RISC-V target setup to do this.

The first goal is to just create and use a 32-bit CPU on the 64-bit
build, which currently doesn't work but it's something that should
work.

Alistair

> > In that case we would have to revisit the code using misa_mxl_max in
> > multiple places.
> >
>
> Never say never, but I don't think there has been much effort to support
> these types of configurations with a single QEMU binary. My googling is
> failing me right now, but I seem to recall that there may have been
> efforts to implement Arm big.LITTLE with multiprocess QEMU [1]. IOW, I
> think we're safe to use misa_mxl_max, since we'll have one for each QEMU
> instance and we'll use a different QEMU instance for each hart bitness.
>
> [1] docs/system/multi-process.rst
>
> Thanks,
> drew
>