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

[Andrew Jones <ajones@ventanamicro.com>]

On Mon, Jan 22, 2024 at 02:07:57PM +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>
> ---
> v3:
> 	use misa_mxl_max to determine bitness
> v2:
> 	set processor family
> ---
>  hw/riscv/Kconfig |  1 +
>  hw/riscv/virt.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index a50717be87..5d644eb7b1 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 f9fd1341fc..1b333af4f0 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/sifive_plic.h"
> @@ -1263,6 +1264,47 @@ 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 (riscv_is_32bit(&s->soc[0])) {

I just saw [1], but I think riscv_is_32bit() is still appropriate,
since, at the time SMBIOS tables are built, the effective mxl will
be set to the max mxl.

[1] 20240122145610.413836-2-alex.bennee@linaro.org

> +        smbios_set_default_processor_family(0x200);
> +#if defined(TARGET_RISCV64)
> +    } else {
> +        smbios_set_default_processor_family(0x201);
> +#endif

Despite the #ifdef being in riscv_cpu_validate_misa_mxl(), I'd drop it
here. I didn't see any riscv_is_32bit() call sites which do this.

Thanks,
drew

> +    }
> +
> +    /* 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,
> @@ -1351,6 +1393,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
>