Re: [PATCH] target/arm: build smbios 19 table

[Peter Maydell]

On Fri, 18 Nov 2022 at 17:37, Mihai Carabas <mihai.carabas@oracle.com> wrote:
>
> Use the base_memmap to build the SMBIOS 19 table which provides the address
> mapping for a Physical Memory Array (from spec [1] chapter 7.20).
>
> This was present on i386 from commit c97294ec1b9e36887e119589d456557d72ab37b5
> ("SMBIOS: Build aggregate smbios tables and entry point").
>
> [1] 
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.5.0.pdf
>
> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>

Is this a bug fix, or a new feature? What are the consequences
of it being missing? Is this important enough to go into the 7.2
release? (My default position would be "no", given this has been
like this on the virt board for a very long time.)

> ---
>  hw/arm/virt.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index cda9defe8f09..855b6982f363 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1607,9 +1607,11 @@ static void *machvirt_dtb(const struct arm_boot_info 
> *binfo, int *fdt_size)
>  static void virt_build_smbios(VirtMachineState *vms)
>  {
>      MachineClass *mc = MACHINE_GET_CLASS(vms);
> +    MachineState *ms = MACHINE(vms);
>      VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>      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()) {
> @@ -1620,7 +1622,11 @@ static void virt_build_smbios(VirtMachineState *vms)
>                          vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
>                          true, SMBIOS_ENTRY_POINT_TYPE_64);
>
> -    smbios_get_tables(MACHINE(vms), NULL, 0,
> +    /* build the array of physical mem area from base_memmap */
> +    mem_array.address = vms->memmap[VIRT_MEM].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);

Does this show up as a difference in the ACPI tables? If so then
the bios-tables-tests will need updating (and this would probably
show up as "make check" failing).

Do we need to care here about pluggable memory devices?
(We seem to do something with them in the ACPI tables
via build_srat_memory(), so maybe not?)

thanks
-- PMM