Message ID | 20231229120724.41383-4-heinrich.schuchardt@canonical.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
Series | target/riscv: SMBIOS support for RISC-V virt machine | expand |
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); > }
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
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
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
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
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 >
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); }
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(+)