Re: [PATCH 4/5] hw/smbios: use qapi for SMBIOS entry point type enum

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Sep 08, 2020 at 08:29:43PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/8/20 6:54 PM, Daniel P. Berrangé wrote:
> > This refactoring prepares for exposing the SMBIOS entry point type as a
> > machine property on x86.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  hw/arm/virt.c                |  2 +-
> >  hw/i386/pc_piix.c            |  2 +-
> >  hw/i386/pc_q35.c             |  2 +-
> >  hw/smbios/smbios.c           |  9 +++++----
> >  include/hw/firmware/smbios.h |  9 ++-------
> >  qapi/machine.json            | 12 ++++++++++++
> >  6 files changed, 22 insertions(+), 14 deletions(-)
> > 
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index acf9bfbece..fd32b10f75 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1450,7 +1450,7 @@ static void virt_build_smbios(VirtMachineState *vms)
> >  
> >      smbios_set_defaults("QEMU", product,
> >                          vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
> > -                        true, SMBIOS_ENTRY_POINT_30);
> > +                        true, SMBIOS_ENTRY_POINT_TYPE_3_0);
> >  
> >      smbios_get_tables(MACHINE(vms), NULL, 0, &smbios_tables, &smbios_tables_len,
> >                        &smbios_anchor, &smbios_anchor_len);
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 32b1453e6a..1c5bc6ae6e 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -179,7 +179,7 @@ static void pc_init1(MachineState *machine,
> >          smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
> >                              mc->name, pcmc->smbios_legacy_mode,
> >                              pcmc->smbios_uuid_encoded,
> > -                            SMBIOS_ENTRY_POINT_21);
> > +                            SMBIOS_ENTRY_POINT_TYPE_2_1);
> >      }
> >  
> >      /* allocate ram and load rom/bios */
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 0cb9c18cd4..cc202407c7 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -204,7 +204,7 @@ static void pc_q35_init(MachineState *machine)
> >          smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
> >                              mc->name, pcmc->smbios_legacy_mode,
> >                              pcmc->smbios_uuid_encoded,
> > -                            SMBIOS_ENTRY_POINT_21);
> > +                            SMBIOS_ENTRY_POINT_TYPE_2_1);
> >      }
> >  
> >      /* allocate ram and load rom/bios */
> > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> > index 3c87be6c91..c99c9b01ae 100644
> > --- a/hw/smbios/smbios.c
> > +++ b/hw/smbios/smbios.c
> > @@ -61,7 +61,7 @@ uint8_t *smbios_tables;
> >  size_t smbios_tables_len;
> >  unsigned smbios_table_max;
> >  unsigned smbios_table_cnt;
> > -static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21;
> > +static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_2_1;
> >  
> >  static SmbiosEntryPoint ep;
> >  
> > @@ -383,7 +383,7 @@ static void smbios_validate_table(MachineState *ms)
> >          exit(1);
> >      }
> >  
> > -    if (smbios_ep_type == SMBIOS_ENTRY_POINT_21 &&
> > +    if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_2_1 &&
> >          smbios_tables_len > SMBIOS_21_MAX_TABLES_LEN) {
> >          error_report("SMBIOS 2.1 table length %zu exceeds %d",
> >                       smbios_tables_len, SMBIOS_21_MAX_TABLES_LEN);
> > @@ -831,7 +831,7 @@ void smbios_set_defaults(const char *manufacturer, const char *product,
> >  static void smbios_entry_point_setup(void)
> >  {
> >      switch (smbios_ep_type) {
> > -    case SMBIOS_ENTRY_POINT_21:
> > +    case SMBIOS_ENTRY_POINT_TYPE_2_1:
> >          memcpy(ep.ep21.anchor_string, "_SM_", 4);
> >          memcpy(ep.ep21.intermediate_anchor_string, "_DMI_", 5);
> >          ep.ep21.length = sizeof(struct smbios_21_entry_point);
> > @@ -854,7 +854,7 @@ static void smbios_entry_point_setup(void)
> >          ep.ep21.structure_table_address = cpu_to_le32(0);
> >  
> >          break;
> > -    case SMBIOS_ENTRY_POINT_30:
> > +    case SMBIOS_ENTRY_POINT_TYPE_3_0:
> >          memcpy(ep.ep30.anchor_string, "_SM3_", 5);
> >          ep.ep30.length = sizeof(struct smbios_30_entry_point);
> >          ep.ep30.entry_point_revision = 1;
> > @@ -939,6 +939,7 @@ void smbios_get_tables(MachineState *ms,
> >      *tables = smbios_tables;
> >      *tables_len = smbios_tables_len;
> >      *anchor = (uint8_t *)&ep;
> > +    g_printerr("Total len %zu\n", smbios_tables_len);
> 
> This seems to belong to patch 2 of this series:
> "hw/smbios: report error if table size is too large"

Actually it doens't belong anywhere. This is debug junk from
investigating EDK2/SeaBIOS flaws

> 
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|