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?
---
 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",
     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.
 ERST
 
 DEFHEADING()
-- 
2.28.0

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

On Wed, Dec 09, 2020 at 09:42:25AM +0000, Daniel P. Berrangé wrote:
> 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

Thanks!

> 
[...]
> > 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.

Good point.  It would also make it easier to change the machine
type default in the future.

I will submit something based on your patches, instead.

-- 
Eduardo

On 12/09/20 19:16, Eduardo Habkost wrote:
> On Wed, Dec 09, 2020 at 09:42:25AM +0000, Daniel P. Berrangé wrote:
>> 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
> 
> Thanks!
> 
>>
> [...]
>>> 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.
> 
> Good point.  It would also make it easier to change the machine
> type default in the future.
> 
> I will submit something based on your patches, instead.
> 

Thank you, Daniel, for finding that write-up; I've got it completely
paged-out by now :) I don't have anything to add here, just confirming
that I've read this thread quickly. Eduardo, if you have questions wrt.
OVMF, please let me know; I'll try to find the time.

Thanks!
Laszlo