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

[Mihai Carabas]

La 18.11.2022 21:11, Peter Maydell a scris:
> 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://urldefense.com/v3/__https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.5.0.pdf__;!!ACWV5N9M2RV99hQ!KF2xmQw9nxPvqvNCgDleyVHv4MoZseoZFHmR1veww7O2BmRxSH1spOCNWX-c-FvzcaR_o8PunXSWWH2ECvFqlR4E7vw$
> >
> > 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.)


This is required by ARM SystemReady Virtual Environment [1]. As 
described in the Arm SystemReady Requirements Specification v2.0
 [2] page 9, 2.5.1 SystemReady Virtual Environment (VE) v1.0 
requirements,: "FirmwareTestSuite (FWTS) must still be used" -> fwts 
checks for the presence of SMBIOS type 19 table and fails the test in 
this case.


[1] 
https://www.arm.com/architecture/system-architectures/systemready-certification-program/ve

[2] https://developer.arm.com/documentation/den0109/latest

> > ---
> >   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).
In ./tests/qtest/bios-tables-test.c we have test_smbios_structs which is 
checking for all structs present. Also it is looking for mandatory types 
and 19 is one of them (base_required_struct_types -> 19). I think we 
cover everything. I will re-run make check to see.
> 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?)
Here you are refering to the fact that when we hot plug a memory dim, to 
automatically update smbios type 19 entry/entries?
> thanks
> -- PMM