Re: [PATCH] smbios: entry-point-type option

[Daniel P . Berrangé]

On Tue, Dec 08, 2020 at 04:20:23PM -0500, Eduardo Habkost wrote:
> Add command-line option that lets the SMBIOS entry point type to
> be configured.
> 
> SMBIOS 3.0 support is necessary to allow us to support more
> than 720 VCPUs in x86_64, due to SMBIOS 2.1 table size limits.
> 
> Note that it's still up to firmware to decide whether to generate
> SMBIOS 2.1 and/or 3.0 entry points for the guest, using the
> information contained in etc/smbios/smbios-anchor.  OVMF, for
> example, is able to generate both entry points, depending on the
> value of PcdSmbiosEntryPointProvideMethod.
> 
> The SMBIOS 3.0 entry point won't be enabled by default because it
> is not supported yet by Seabios.  This may be changed once
> Seabios starts supporting SMBIOS 3.0 entry points.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Laszlo, Philippe: do you know how exactly the value of
> PcdSmbiosEntryPointProvideMethod is chosen when running OVMF?

Laszlo proides alot of detail in my original proposal for
selecting SMBIOS entry point here:

https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg03347.html

> ---
>  hw/smbios/smbios.c | 39 +++++++++++++++++++++++++++++++++++++--
>  qemu-options.hx    | 16 +++++++++++++++-
>  2 files changed, 52 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 6a3d39793b..ed5cf7fa9a 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -62,6 +62,7 @@ size_t smbios_tables_len;
>  unsigned smbios_table_max;
>  unsigned smbios_table_cnt;
>  static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21;
> +static bool smbios_ep_type_set;
>  
>  static SmbiosEntryPoint ep;
>  
> @@ -139,6 +140,15 @@ static const QemuOptDesc qemu_smbios_file_opts[] = {
>      { /* end of list */ }
>  };
>  
> +static const QemuOptDesc qemu_smbios_entry_point_opts[] = {
> +    {
> +        .name = "entry-point-type",
> +        .type = QEMU_OPT_STRING,
> +        .help = "SMBIOS Entry Point type (2.1 or 3.0)",
> +    },
> +    { /* end of list */ }
> +};
> +
>  static const QemuOptDesc qemu_smbios_type0_opts[] = {
>      {
>          .name = "type",
> @@ -797,7 +807,9 @@ void smbios_set_defaults(const char *manufacturer, const 
> char *product,
>      smbios_have_defaults = true;
>      smbios_legacy = legacy_mode;
>      smbios_uuid_encoded = uuid_encoded;
> -    smbios_ep_type = ep_type;
> +    if (!smbios_ep_type_set) {
> +        smbios_ep_type = ep_type;
> +    }
>  
>      /* drop unwanted version of command-line file blob(s) */
>      if (smbios_legacy) {
> @@ -1232,5 +1244,28 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
>          }
>      }
>  
> -    error_setg(errp, "Must specify type= or file=");
> +    val = qemu_opt_get(opts, "entry-point-type");
> +    if (val) {
> +        if (!qemu_opts_validate(opts, qemu_smbios_entry_point_opts, errp)) {
> +            return;
> +        }
> +
> +        if (smbios_ep_type_set) {
> +            error_setg(errp, "entry-point-type can't be set twice");
> +            return;
> +        }
> +
> +        if (!strcmp(val, "2.1")) {
> +            smbios_ep_type = SMBIOS_ENTRY_POINT_21;
> +            smbios_ep_type_set = true;
> +        } else if (!strcmp(val, "3.0")) {
> +            smbios_ep_type = SMBIOS_ENTRY_POINT_30;
> +            smbios_ep_type_set = true;
> +        } else {
> +            error_setg(errp, "Invalid entry point type: %s", val);
> +        }
> +        return;
> +    }
> +
> +    error_setg(errp, "Must specify type=, file=, or entry-point-type=");
>  }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 104632ea34..d2a973f8a7 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2294,7 +2294,9 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
>      "                specify SMBIOS type 11 fields\n"
>      "-smbios 
> type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str]\n"
>      "               [,asset=str][,part=str][,speed=%d]\n"
> -    "                specify SMBIOS type 17 fields\n",
> +    "                specify SMBIOS type 17 fields\n"
> +    "-smbios entry-point-type=2.1|3.0\n"
> +    "                specify SMBIOS entry point type\n",

My previous patch:

  https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg03027.html

exposed the entry point version as a property on the PC machine
rather than the -smbios arg, principally because it is the machin
setup code that currently defines what version is used via the calls
to smbios_set_defaults().

IIUC from Laszlo's reply,  SMBIOS 2.1 is not valid for AArch64
at all - they only support 3.0.  So there's a small benefit from
configuring this against the PC machine types, because it prevents
ability to select 2.1 for ARM SMBIOS which would be invalid.



>      QEMU_ARCH_I386 | QEMU_ARCH_ARM)
>  SRST
>  ``-smbios file=binary``
> @@ -2356,6 +2358,18 @@ SRST
>  
>  ``-smbios 
> type=17[,loc_pfx=str][,bank=str][,manufacturer=str][,serial=str][,asset=str][,part=str][,speed=%d]``
>      Specify SMBIOS type 17 fields
> +
> +``-smbios entry-point-type=2.1|3.0``
> +    Specify SMBIOS entry point type
> +
> +    SMBIOS 3.0 allows the total size of SMBIOS tables to be much
> +    larger, but note that Seabios for x86 does not support SMBIOS
> +    3.0 (as of Seabios v1.14.0).
> +
> +    Note that this option only affects the SMBIOS data exposed
> +    through fw_cfg.  It's still up to the firmware to generate
> +    the actual SMBIOS entry point(s) seen by the guest operating
> +    system.

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 :|