All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type
@ 2020-12-14 20:50 Eduardo Habkost
  2020-12-14 20:50 ` [PATCH v2 1/3] smbios: Rename SMBIOS_ENTRY_POINT_* enums Eduardo Habkost
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Eduardo Habkost @ 2020-12-14 20:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Daniel P. Berrange, Eduardo Habkost,
	Michael S. Tsirkin, Philippe Mathieu-Daudé,
	Richard Henderson, Michael Roth, Markus Armbruster, qemu-arm,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

This includes code previously submitted[1] by Daniel P. Berrangé
to add a "smbios-ep" machine property on PC.

SMBIOS 3.0 is necessary to support more than ~720 VCPUs, as a
large number of VCPUs can easily hit the table size limit of
SMBIOS 2.1 entry points.

[1] https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berrange@redhat.com
    https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berrange@redhat.com

Daniel P. Berrangé (1):
  hw/i386: expose a "smbios-ep" PC machine property

Eduardo Habkost (2):
  smbios: Rename SMBIOS_ENTRY_POINT_* enums
  hw/smbios: Use qapi for SmbiosEntryPointType

 qapi/qapi-schema.json        |  1 +
 qapi/smbios.json             | 11 +++++++++++
 include/hw/firmware/smbios.h | 10 ++--------
 include/hw/i386/pc.h         |  3 +++
 hw/arm/virt.c                |  2 +-
 hw/i386/pc.c                 | 26 ++++++++++++++++++++++++++
 hw/i386/pc_piix.c            |  2 +-
 hw/i386/pc_q35.c             |  2 +-
 hw/smbios/smbios.c           |  8 ++++----
 qapi/meson.build             |  1 +
 10 files changed, 51 insertions(+), 15 deletions(-)
 create mode 100644 qapi/smbios.json

-- 
2.28.0




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/3] smbios: Rename SMBIOS_ENTRY_POINT_* enums
  2020-12-14 20:50 [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type Eduardo Habkost
@ 2020-12-14 20:50 ` Eduardo Habkost
  2020-12-29 13:20   ` Igor Mammedov
  2020-12-14 20:50 ` [PATCH v2 2/3] hw/smbios: Use qapi for SmbiosEntryPointType Eduardo Habkost
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Eduardo Habkost @ 2020-12-14 20:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Daniel P. Berrange, Eduardo Habkost,
	Michael S. Tsirkin, Philippe Mathieu-Daudé,
	Richard Henderson, Michael Roth, Markus Armbruster, qemu-arm,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

Rename the enums to match the naming style used by QAPI.  This
will allow us to more easily move the enum to the QAPI schema
later.

Based on portions of a patch submitted by Daniel P. Berrangé.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
First version of this code was submitted at:
https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berrange@redhat.com

Changes from v1:
* Patch was split in two
* Hunks included this patch are not changed from v1
---
 include/hw/firmware/smbios.h | 4 ++--
 hw/arm/virt.c                | 2 +-
 hw/i386/pc_piix.c            | 2 +-
 hw/i386/pc_q35.c             | 2 +-
 hw/smbios/smbios.c           | 8 ++++----
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
index 02a0ced0a0..5467ecec78 100644
--- a/include/hw/firmware/smbios.h
+++ b/include/hw/firmware/smbios.h
@@ -27,8 +27,8 @@ struct smbios_phys_mem_area {
  * SMBIOS spec defined tables
  */
 typedef enum SmbiosEntryPointType {
-    SMBIOS_ENTRY_POINT_21,
-    SMBIOS_ENTRY_POINT_30,
+    SMBIOS_ENTRY_POINT_TYPE_2_1,
+    SMBIOS_ENTRY_POINT_TYPE_3_0,
 } SmbiosEntryPointType;
 
 /* SMBIOS Entry Point
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 556592012e..af53e09d1e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1445,7 +1445,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 6188c3e97e..08b82df4d1 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 0a212443aa..f71b1e2dcf 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -198,7 +198,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 f22c4f5b73..930cf52c6b 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;
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/3] hw/smbios: Use qapi for SmbiosEntryPointType
  2020-12-14 20:50 [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type Eduardo Habkost
  2020-12-14 20:50 ` [PATCH v2 1/3] smbios: Rename SMBIOS_ENTRY_POINT_* enums Eduardo Habkost
@ 2020-12-14 20:50 ` Eduardo Habkost
  2020-12-29 13:21   ` Igor Mammedov
  2020-12-14 20:50 ` [PATCH v2 3/3] hw/i386: expose a "smbios-ep" PC machine property Eduardo Habkost
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Eduardo Habkost @ 2020-12-14 20:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Daniel P. Berrange, Eduardo Habkost,
	Michael S. Tsirkin, Philippe Mathieu-Daudé,
	Richard Henderson, Michael Roth, Markus Armbruster, qemu-arm,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

This prepares for exposing the SMBIOS entry point type as a
machine property on x86.

Based on a patch from Daniel P. Berrangé.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
First version of this code was submitted at:
https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berrange@redhat.com

Changes from v1:
* Patch was split in two
* Declarations were moved to qapi/smbios.json
* Documentation was updated to use the same terminology used in
  SMBIOS documentation
* Documentation was updated to "Since: 6.0"
---
 qapi/qapi-schema.json        |  1 +
 qapi/smbios.json             | 11 +++++++++++
 include/hw/firmware/smbios.h | 10 ++--------
 qapi/meson.build             |  1 +
 4 files changed, 15 insertions(+), 8 deletions(-)
 create mode 100644 qapi/smbios.json

diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 0b444b76d2..87a183fb13 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -86,6 +86,7 @@
 { 'include': 'machine.json' }
 { 'include': 'machine-target.json' }
 { 'include': 'replay.json' }
+{ 'include': 'smbios.json' }
 { 'include': 'misc.json' }
 { 'include': 'misc-target.json' }
 { 'include': 'audio.json' }
diff --git a/qapi/smbios.json b/qapi/smbios.json
new file mode 100644
index 0000000000..55b3bd2e83
--- /dev/null
+++ b/qapi/smbios.json
@@ -0,0 +1,11 @@
+##
+# @SmbiosEntryPointType:
+#
+# @2_1: SMBIOS version 2.1 (32-bit) Entry Point
+#
+# @3_0: SMBIOS version 3.0 (64-bit) Entry Point
+#
+# Since: 6.0
+##
+{ 'enum': 'SmbiosEntryPointType',
+  'data': [ '2_1', '3_0' ] }
diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
index 5467ecec78..b3beef1606 100644
--- a/include/hw/firmware/smbios.h
+++ b/include/hw/firmware/smbios.h
@@ -1,6 +1,8 @@
 #ifndef QEMU_SMBIOS_H
 #define QEMU_SMBIOS_H
 
+#include "qapi/qapi-types-smbios.h"
+
 /*
  * SMBIOS Support
  *
@@ -23,14 +25,6 @@ struct smbios_phys_mem_area {
     uint64_t length;
 };
 
-/*
- * SMBIOS spec defined tables
- */
-typedef enum SmbiosEntryPointType {
-    SMBIOS_ENTRY_POINT_TYPE_2_1,
-    SMBIOS_ENTRY_POINT_TYPE_3_0,
-} SmbiosEntryPointType;
-
 /* SMBIOS Entry Point
  * There are two types of entry points defined in the SMBIOS specification
  * (see below). BIOS must place the entry point(s) at a 16-byte-aligned
diff --git a/qapi/meson.build b/qapi/meson.build
index 0e98146f1f..f7fb73d41b 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -42,6 +42,7 @@ qapi_all_modules = [
   'replay',
   'rocker',
   'run-state',
+  'smbios',
   'sockets',
   'tpm',
   'trace',
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/3] hw/i386: expose a "smbios-ep" PC machine property
  2020-12-14 20:50 [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type Eduardo Habkost
  2020-12-14 20:50 ` [PATCH v2 1/3] smbios: Rename SMBIOS_ENTRY_POINT_* enums Eduardo Habkost
  2020-12-14 20:50 ` [PATCH v2 2/3] hw/smbios: Use qapi for SmbiosEntryPointType Eduardo Habkost
@ 2020-12-14 20:50 ` Eduardo Habkost
  2021-01-04 22:44   ` Igor Mammedov
  2021-01-05 11:02   ` Michael S. Tsirkin
  2020-12-29 13:20 ` [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type Igor Mammedov
  2020-12-29 15:10 ` Philippe Mathieu-Daudé
  4 siblings, 2 replies; 11+ messages in thread
From: Eduardo Habkost @ 2020-12-14 20:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Daniel P. Berrange, Eduardo Habkost,
	Michael S. Tsirkin, Philippe Mathieu-Daudé,
	Richard Henderson, Michael Roth, Markus Armbruster, qemu-arm,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

From: Daniel P. Berrangé <berrange@redhat.com>

The i440fx and Q35 machine types are both hardcoded to use the legacy
SMBIOS 2.1 entry point. This is a sensible conservative choice because
SeaBIOS only supports SMBIOS 2.1

EDK2, however, can also support SMBIOS 3.0 and QEMU already uses this on
the ARM virt machine type.

This adds a property to allow the choice of SMBIOS entry point versions
For example to opt in to version 3.0

   $QEMU -machine q35,smbios-ep=3_0

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is patch was previously submitted at:
https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berrange@redhat.com

Changes from v1:
* Include qapi-visit-smbios.h instead of qapi-visit-machine.h
* Commit message fix: s/smbios_ep/smbios-ep/
---
 include/hw/i386/pc.h |  3 +++
 hw/i386/pc.c         | 26 ++++++++++++++++++++++++++
 hw/i386/pc_piix.c    |  2 +-
 hw/i386/pc_q35.c     |  2 +-
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 2aa8797c6e..2075093b32 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -11,6 +11,7 @@
 #include "hw/acpi/acpi_dev_interface.h"
 #include "hw/hotplug.h"
 #include "qom/object.h"
+#include "hw/firmware/smbios.h"
 
 #define HPET_INTCAP "hpet-intcap"
 
@@ -38,6 +39,7 @@ typedef struct PCMachineState {
     /* Configuration options: */
     uint64_t max_ram_below_4g;
     OnOffAuto vmport;
+    SmbiosEntryPointType smbios_ep;
 
     bool acpi_build_enabled;
     bool smbus_enabled;
@@ -62,6 +64,7 @@ typedef struct PCMachineState {
 #define PC_MACHINE_SATA             "sata"
 #define PC_MACHINE_PIT              "pit"
 #define PC_MACHINE_MAX_FW_SIZE      "max-fw-size"
+#define PC_MACHINE_SMBIOS_EP        "smbios-ep"
 
 /**
  * PCMachineClass:
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 640fb5b0b7..3cc559e0d9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -81,6 +81,7 @@
 #include "hw/mem/nvdimm.h"
 #include "qapi/error.h"
 #include "qapi/qapi-visit-common.h"
+#include "qapi/qapi-visit-smbios.h"
 #include "qapi/visitor.h"
 #include "hw/core/cpu.h"
 #include "hw/usb.h"
@@ -1532,6 +1533,23 @@ static void pc_machine_set_hpet(Object *obj, bool value, Error **errp)
     pcms->hpet_enabled = value;
 }
 
+static void pc_machine_get_smbios_ep(Object *obj, Visitor *v, const char *name,
+                                     void *opaque, Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(obj);
+    SmbiosEntryPointType smbios_ep = pcms->smbios_ep;
+
+    visit_type_SmbiosEntryPointType(v, name, &smbios_ep, errp);
+}
+
+static void pc_machine_set_smbios_ep(Object *obj, Visitor *v, const char *name,
+                                     void *opaque, Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(obj);
+
+    visit_type_SmbiosEntryPointType(v, name, &pcms->smbios_ep, errp);
+}
+
 static void pc_machine_get_max_ram_below_4g(Object *obj, Visitor *v,
                                             const char *name, void *opaque,
                                             Error **errp)
@@ -1621,6 +1639,8 @@ static void pc_machine_initfn(Object *obj)
     pcms->vmport = ON_OFF_AUTO_OFF;
 #endif /* CONFIG_VMPORT */
     pcms->max_ram_below_4g = 0; /* use default */
+    pcms->smbios_ep = SMBIOS_ENTRY_POINT_TYPE_2_1;
+
     /* acpi build is enabled by default if machine supports it */
     pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
     pcms->smbus_enabled = true;
@@ -1759,6 +1779,12 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
         NULL, NULL);
     object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE,
         "Maximum combined firmware size");
+
+    object_class_property_add(oc, PC_MACHINE_SMBIOS_EP, "str",
+        pc_machine_get_smbios_ep, pc_machine_set_smbios_ep,
+        NULL, NULL);
+    object_class_property_set_description(oc, PC_MACHINE_SMBIOS_EP,
+        "SMBIOS Entry Point version [v2_1, v3_0]");
 }
 
 static const TypeInfo pc_machine_info = {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 08b82df4d1..30ae7f27af 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_TYPE_2_1);
+                            pcms->smbios_ep);
     }
 
     /* allocate ram and load rom/bios */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f71b1e2dcf..9974426806 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -198,7 +198,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_TYPE_2_1);
+                            pcms->smbios_ep);
     }
 
     /* allocate ram and load rom/bios */
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type
  2020-12-14 20:50 [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type Eduardo Habkost
                   ` (2 preceding siblings ...)
  2020-12-14 20:50 ` [PATCH v2 3/3] hw/i386: expose a "smbios-ep" PC machine property Eduardo Habkost
@ 2020-12-29 13:20 ` Igor Mammedov
  2021-01-04 22:10   ` Eduardo Habkost
  2020-12-29 15:10 ` Philippe Mathieu-Daudé
  4 siblings, 1 reply; 11+ messages in thread
From: Igor Mammedov @ 2020-12-29 13:20 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Daniel P. Berrange, Laszlo Ersek,
	Michael S. Tsirkin, Markus Armbruster, Richard Henderson,
	qemu-devel, Michael Roth, qemu-arm, Paolo Bonzini,
	Philippe Mathieu-Daudé

On Mon, 14 Dec 2020 15:50:26 -0500
Eduardo Habkost <ehabkost@redhat.com> wrote:

> This includes code previously submitted[1] by Daniel P. Berrangé
> to add a "smbios-ep" machine property on PC.
> 
> SMBIOS 3.0 is necessary to support more than ~720 VCPUs, as a
> large number of VCPUs can easily hit the table size limit of
> SMBIOS 2.1 entry points.

Eduardo,
do you plan to submit Seabios patches for SMBIOS 3.0?
will OVMF pick up new tables automatically?

> 
> [1] https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berrange@redhat.com
>     https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berrange@redhat.com
> 
> Daniel P. Berrangé (1):
>   hw/i386: expose a "smbios-ep" PC machine property
> 
> Eduardo Habkost (2):
>   smbios: Rename SMBIOS_ENTRY_POINT_* enums
>   hw/smbios: Use qapi for SmbiosEntryPointType
> 
>  qapi/qapi-schema.json        |  1 +
>  qapi/smbios.json             | 11 +++++++++++
>  include/hw/firmware/smbios.h | 10 ++--------
>  include/hw/i386/pc.h         |  3 +++
>  hw/arm/virt.c                |  2 +-
>  hw/i386/pc.c                 | 26 ++++++++++++++++++++++++++
>  hw/i386/pc_piix.c            |  2 +-
>  hw/i386/pc_q35.c             |  2 +-
>  hw/smbios/smbios.c           |  8 ++++----
>  qapi/meson.build             |  1 +
>  10 files changed, 51 insertions(+), 15 deletions(-)
>  create mode 100644 qapi/smbios.json
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] smbios: Rename SMBIOS_ENTRY_POINT_* enums
  2020-12-14 20:50 ` [PATCH v2 1/3] smbios: Rename SMBIOS_ENTRY_POINT_* enums Eduardo Habkost
@ 2020-12-29 13:20   ` Igor Mammedov
  0 siblings, 0 replies; 11+ messages in thread
From: Igor Mammedov @ 2020-12-29 13:20 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Daniel P. Berrange, Michael Roth,
	Michael S. Tsirkin, Philippe Mathieu-Daudé,
	Richard Henderson, Markus Armbruster, qemu-devel, qemu-arm,
	Paolo Bonzini, Laszlo Ersek

On Mon, 14 Dec 2020 15:50:27 -0500
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Rename the enums to match the naming style used by QAPI.  This
> will allow us to more easily move the enum to the QAPI schema
> later.
> 
> Based on portions of a patch submitted by Daniel P. Berrangé.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> First version of this code was submitted at:
> https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berrange@redhat.com
> 
> Changes from v1:
> * Patch was split in two
> * Hunks included this patch are not changed from v1
> ---
>  include/hw/firmware/smbios.h | 4 ++--
>  hw/arm/virt.c                | 2 +-
>  hw/i386/pc_piix.c            | 2 +-
>  hw/i386/pc_q35.c             | 2 +-
>  hw/smbios/smbios.c           | 8 ++++----
>  5 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> index 02a0ced0a0..5467ecec78 100644
> --- a/include/hw/firmware/smbios.h
> +++ b/include/hw/firmware/smbios.h
> @@ -27,8 +27,8 @@ struct smbios_phys_mem_area {
>   * SMBIOS spec defined tables
>   */
>  typedef enum SmbiosEntryPointType {
> -    SMBIOS_ENTRY_POINT_21,
> -    SMBIOS_ENTRY_POINT_30,
> +    SMBIOS_ENTRY_POINT_TYPE_2_1,
> +    SMBIOS_ENTRY_POINT_TYPE_3_0,
>  } SmbiosEntryPointType;
>  
>  /* SMBIOS Entry Point
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 556592012e..af53e09d1e 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1445,7 +1445,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 6188c3e97e..08b82df4d1 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 0a212443aa..f71b1e2dcf 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -198,7 +198,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 f22c4f5b73..930cf52c6b 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;



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] hw/smbios: Use qapi for SmbiosEntryPointType
  2020-12-14 20:50 ` [PATCH v2 2/3] hw/smbios: Use qapi for SmbiosEntryPointType Eduardo Habkost
@ 2020-12-29 13:21   ` Igor Mammedov
  0 siblings, 0 replies; 11+ messages in thread
From: Igor Mammedov @ 2020-12-29 13:21 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Daniel P. Berrange, Michael Roth,
	Michael S. Tsirkin, Philippe Mathieu-Daudé,
	Richard Henderson, Markus Armbruster, qemu-devel, qemu-arm,
	Paolo Bonzini, Laszlo Ersek

On Mon, 14 Dec 2020 15:50:28 -0500
Eduardo Habkost <ehabkost@redhat.com> wrote:

> This prepares for exposing the SMBIOS entry point type as a
> machine property on x86.
> 
> Based on a patch from Daniel P. Berrangé.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> First version of this code was submitted at:
> https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berrange@redhat.com
> 
> Changes from v1:
> * Patch was split in two
> * Declarations were moved to qapi/smbios.json
> * Documentation was updated to use the same terminology used in
>   SMBIOS documentation
> * Documentation was updated to "Since: 6.0"
> ---
>  qapi/qapi-schema.json        |  1 +
>  qapi/smbios.json             | 11 +++++++++++
>  include/hw/firmware/smbios.h | 10 ++--------
>  qapi/meson.build             |  1 +
>  4 files changed, 15 insertions(+), 8 deletions(-)
>  create mode 100644 qapi/smbios.json
> 
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index 0b444b76d2..87a183fb13 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -86,6 +86,7 @@
>  { 'include': 'machine.json' }
>  { 'include': 'machine-target.json' }
>  { 'include': 'replay.json' }
> +{ 'include': 'smbios.json' }
>  { 'include': 'misc.json' }
>  { 'include': 'misc-target.json' }
>  { 'include': 'audio.json' }
> diff --git a/qapi/smbios.json b/qapi/smbios.json
> new file mode 100644
> index 0000000000..55b3bd2e83
> --- /dev/null
> +++ b/qapi/smbios.json
> @@ -0,0 +1,11 @@
> +##
> +# @SmbiosEntryPointType:
> +#
> +# @2_1: SMBIOS version 2.1 (32-bit) Entry Point
> +#
> +# @3_0: SMBIOS version 3.0 (64-bit) Entry Point
> +#
> +# Since: 6.0
> +##
> +{ 'enum': 'SmbiosEntryPointType',
> +  'data': [ '2_1', '3_0' ] }
> diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> index 5467ecec78..b3beef1606 100644
> --- a/include/hw/firmware/smbios.h
> +++ b/include/hw/firmware/smbios.h
> @@ -1,6 +1,8 @@
>  #ifndef QEMU_SMBIOS_H
>  #define QEMU_SMBIOS_H
>  
> +#include "qapi/qapi-types-smbios.h"
> +
>  /*
>   * SMBIOS Support
>   *
> @@ -23,14 +25,6 @@ struct smbios_phys_mem_area {
>      uint64_t length;
>  };
>  
> -/*
> - * SMBIOS spec defined tables
> - */
> -typedef enum SmbiosEntryPointType {
> -    SMBIOS_ENTRY_POINT_TYPE_2_1,
> -    SMBIOS_ENTRY_POINT_TYPE_3_0,
> -} SmbiosEntryPointType;
> -
>  /* SMBIOS Entry Point
>   * There are two types of entry points defined in the SMBIOS specification
>   * (see below). BIOS must place the entry point(s) at a 16-byte-aligned
> diff --git a/qapi/meson.build b/qapi/meson.build
> index 0e98146f1f..f7fb73d41b 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -42,6 +42,7 @@ qapi_all_modules = [
>    'replay',
>    'rocker',
>    'run-state',
> +  'smbios',
>    'sockets',
>    'tpm',
>    'trace',



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type
  2020-12-14 20:50 [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type Eduardo Habkost
                   ` (3 preceding siblings ...)
  2020-12-29 13:20 ` [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type Igor Mammedov
@ 2020-12-29 15:10 ` Philippe Mathieu-Daudé
  4 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-29 15:10 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel
  Cc: Peter Maydell, Daniel P. Berrange, Michael S. Tsirkin,
	Richard Henderson, Michael Roth, Markus Armbruster, qemu-arm,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

On 12/14/20 9:50 PM, Eduardo Habkost wrote:
> This includes code previously submitted[1] by Daniel P. Berrangé
> to add a "smbios-ep" machine property on PC.
> 
> SMBIOS 3.0 is necessary to support more than ~720 VCPUs, as a
> large number of VCPUs can easily hit the table size limit of
> SMBIOS 2.1 entry points.
> 
> [1] https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berrange@redhat.com
>     https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berrange@redhat.com
> 
> Daniel P. Berrangé (1):
>   hw/i386: expose a "smbios-ep" PC machine property
> 
> Eduardo Habkost (2):
>   smbios: Rename SMBIOS_ENTRY_POINT_* enums
>   hw/smbios: Use qapi for SmbiosEntryPointType
> 
>  qapi/qapi-schema.json        |  1 +
>  qapi/smbios.json             | 11 +++++++++++
>  include/hw/firmware/smbios.h | 10 ++--------
>  include/hw/i386/pc.h         |  3 +++
>  hw/arm/virt.c                |  2 +-
>  hw/i386/pc.c                 | 26 ++++++++++++++++++++++++++
>  hw/i386/pc_piix.c            |  2 +-
>  hw/i386/pc_q35.c             |  2 +-
>  hw/smbios/smbios.c           |  8 ++++----
>  qapi/meson.build             |  1 +
>  10 files changed, 51 insertions(+), 15 deletions(-)
>  create mode 100644 qapi/smbios.json

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type
  2020-12-29 13:20 ` [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type Igor Mammedov
@ 2021-01-04 22:10   ` Eduardo Habkost
  0 siblings, 0 replies; 11+ messages in thread
From: Eduardo Habkost @ 2021-01-04 22:10 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Peter Maydell, Daniel P. Berrange, Laszlo Ersek,
	Michael S. Tsirkin, Markus Armbruster, Richard Henderson,
	qemu-devel, Michael Roth, qemu-arm, Paolo Bonzini,
	Philippe Mathieu-Daudé

On Tue, Dec 29, 2020 at 02:20:01PM +0100, Igor Mammedov wrote:
> On Mon, 14 Dec 2020 15:50:26 -0500
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > This includes code previously submitted[1] by Daniel P. Berrangé
> > to add a "smbios-ep" machine property on PC.
> > 
> > SMBIOS 3.0 is necessary to support more than ~720 VCPUs, as a
> > large number of VCPUs can easily hit the table size limit of
> > SMBIOS 2.1 entry points.
> 
> Eduardo,
> do you plan to submit Seabios patches for SMBIOS 3.0?
> will OVMF pick up new tables automatically?

OVMF will pick the new tables automatically.

SeaBIOS patches are at:
https://www.mail-archive.com/search?l=mid&q=20201210212640.2023885-1-ehabkost@redhat.com

-- 
Eduardo



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] hw/i386: expose a "smbios-ep" PC machine property
  2020-12-14 20:50 ` [PATCH v2 3/3] hw/i386: expose a "smbios-ep" PC machine property Eduardo Habkost
@ 2021-01-04 22:44   ` Igor Mammedov
  2021-01-05 11:02   ` Michael S. Tsirkin
  1 sibling, 0 replies; 11+ messages in thread
From: Igor Mammedov @ 2021-01-04 22:44 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Daniel P. Berrange, Laszlo Ersek,
	Michael S. Tsirkin, Markus Armbruster, Richard Henderson,
	qemu-devel, Michael Roth, qemu-arm, Paolo Bonzini,
	Philippe Mathieu-Daudé

On Mon, 14 Dec 2020 15:50:29 -0500
Eduardo Habkost <ehabkost@redhat.com> wrote:

> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> The i440fx and Q35 machine types are both hardcoded to use the legacy
> SMBIOS 2.1 entry point. This is a sensible conservative choice because
> SeaBIOS only supports SMBIOS 2.1
> 
> EDK2, however, can also support SMBIOS 3.0 and QEMU already uses this on
> the ARM virt machine type.
> 
> This adds a property to allow the choice of SMBIOS entry point versions
> For example to opt in to version 3.0
> 
>    $QEMU -machine q35,smbios-ep=3_0
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> This is patch was previously submitted at:
> https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berrange@redhat.com
> 
> Changes from v1:
> * Include qapi-visit-smbios.h instead of qapi-visit-machine.h
> * Commit message fix: s/smbios_ep/smbios-ep/
> ---
>  include/hw/i386/pc.h |  3 +++
>  hw/i386/pc.c         | 26 ++++++++++++++++++++++++++
>  hw/i386/pc_piix.c    |  2 +-
>  hw/i386/pc_q35.c     |  2 +-
>  4 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 2aa8797c6e..2075093b32 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -11,6 +11,7 @@
>  #include "hw/acpi/acpi_dev_interface.h"
>  #include "hw/hotplug.h"
>  #include "qom/object.h"
> +#include "hw/firmware/smbios.h"
>  
>  #define HPET_INTCAP "hpet-intcap"
>  
> @@ -38,6 +39,7 @@ typedef struct PCMachineState {
>      /* Configuration options: */
>      uint64_t max_ram_below_4g;
>      OnOffAuto vmport;
> +    SmbiosEntryPointType smbios_ep;
>  
>      bool acpi_build_enabled;
>      bool smbus_enabled;
> @@ -62,6 +64,7 @@ typedef struct PCMachineState {
>  #define PC_MACHINE_SATA             "sata"
>  #define PC_MACHINE_PIT              "pit"
>  #define PC_MACHINE_MAX_FW_SIZE      "max-fw-size"
> +#define PC_MACHINE_SMBIOS_EP        "smbios-ep"
>  
>  /**
>   * PCMachineClass:
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 640fb5b0b7..3cc559e0d9 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -81,6 +81,7 @@
>  #include "hw/mem/nvdimm.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-visit-common.h"
> +#include "qapi/qapi-visit-smbios.h"
>  #include "qapi/visitor.h"
>  #include "hw/core/cpu.h"
>  #include "hw/usb.h"
> @@ -1532,6 +1533,23 @@ static void pc_machine_set_hpet(Object *obj, bool value, Error **errp)
>      pcms->hpet_enabled = value;
>  }
>  
> +static void pc_machine_get_smbios_ep(Object *obj, Visitor *v, const char *name,
> +                                     void *opaque, Error **errp)
> +{
> +    PCMachineState *pcms = PC_MACHINE(obj);
> +    SmbiosEntryPointType smbios_ep = pcms->smbios_ep;
> +
> +    visit_type_SmbiosEntryPointType(v, name, &smbios_ep, errp);
> +}
> +
> +static void pc_machine_set_smbios_ep(Object *obj, Visitor *v, const char *name,
> +                                     void *opaque, Error **errp)
> +{
> +    PCMachineState *pcms = PC_MACHINE(obj);
> +
> +    visit_type_SmbiosEntryPointType(v, name, &pcms->smbios_ep, errp);
> +}
> +
>  static void pc_machine_get_max_ram_below_4g(Object *obj, Visitor *v,
>                                              const char *name, void *opaque,
>                                              Error **errp)
> @@ -1621,6 +1639,8 @@ static void pc_machine_initfn(Object *obj)
>      pcms->vmport = ON_OFF_AUTO_OFF;
>  #endif /* CONFIG_VMPORT */
>      pcms->max_ram_below_4g = 0; /* use default */
> +    pcms->smbios_ep = SMBIOS_ENTRY_POINT_TYPE_2_1;
> +
>      /* acpi build is enabled by default if machine supports it */
>      pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
>      pcms->smbus_enabled = true;
> @@ -1759,6 +1779,12 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>          NULL, NULL);
>      object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE,
>          "Maximum combined firmware size");
> +
> +    object_class_property_add(oc, PC_MACHINE_SMBIOS_EP, "str",
> +        pc_machine_get_smbios_ep, pc_machine_set_smbios_ep,
> +        NULL, NULL);
> +    object_class_property_set_description(oc, PC_MACHINE_SMBIOS_EP,
> +        "SMBIOS Entry Point version [v2_1, v3_0]");
>  }
>  
>  static const TypeInfo pc_machine_info = {
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 08b82df4d1..30ae7f27af 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_TYPE_2_1);
> +                            pcms->smbios_ep);
>      }
>  
>      /* allocate ram and load rom/bios */
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index f71b1e2dcf..9974426806 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -198,7 +198,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_TYPE_2_1);
> +                            pcms->smbios_ep);
>      }
>  
>      /* allocate ram and load rom/bios */



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] hw/i386: expose a "smbios-ep" PC machine property
  2020-12-14 20:50 ` [PATCH v2 3/3] hw/i386: expose a "smbios-ep" PC machine property Eduardo Habkost
  2021-01-04 22:44   ` Igor Mammedov
@ 2021-01-05 11:02   ` Michael S. Tsirkin
  1 sibling, 0 replies; 11+ messages in thread
From: Michael S. Tsirkin @ 2021-01-05 11:02 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Peter Maydell, Daniel P. Berrange, Michael Roth,
	Philippe Mathieu-Daudé,
	Richard Henderson, qemu-devel, Markus Armbruster, qemu-arm,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

On Mon, Dec 14, 2020 at 03:50:29PM -0500, Eduardo Habkost wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> The i440fx and Q35 machine types are both hardcoded to use the legacy
> SMBIOS 2.1 entry point. This is a sensible conservative choice because
> SeaBIOS only supports SMBIOS 2.1
> 
> EDK2, however, can also support SMBIOS 3.0 and QEMU already uses this on
> the ARM virt machine type.
> 
> This adds a property to allow the choice of SMBIOS entry point versions
> For example to opt in to version 3.0
> 
>    $QEMU -machine q35,smbios-ep=3_0
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Just one small point: wouldn't it be a good idea to eschew
abbreviation here, and call the property smbios-entry-point
or even smbios-entry-point-type or smbios-entry-point-version?

> ---
> This is patch was previously submitted at:
> https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berrange@redhat.com
> 
> Changes from v1:
> * Include qapi-visit-smbios.h instead of qapi-visit-machine.h
> * Commit message fix: s/smbios_ep/smbios-ep/
> ---
>  include/hw/i386/pc.h |  3 +++
>  hw/i386/pc.c         | 26 ++++++++++++++++++++++++++
>  hw/i386/pc_piix.c    |  2 +-
>  hw/i386/pc_q35.c     |  2 +-
>  4 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 2aa8797c6e..2075093b32 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -11,6 +11,7 @@
>  #include "hw/acpi/acpi_dev_interface.h"
>  #include "hw/hotplug.h"
>  #include "qom/object.h"
> +#include "hw/firmware/smbios.h"
>  
>  #define HPET_INTCAP "hpet-intcap"
>  
> @@ -38,6 +39,7 @@ typedef struct PCMachineState {
>      /* Configuration options: */
>      uint64_t max_ram_below_4g;
>      OnOffAuto vmport;
> +    SmbiosEntryPointType smbios_ep;
>  
>      bool acpi_build_enabled;
>      bool smbus_enabled;
> @@ -62,6 +64,7 @@ typedef struct PCMachineState {
>  #define PC_MACHINE_SATA             "sata"
>  #define PC_MACHINE_PIT              "pit"
>  #define PC_MACHINE_MAX_FW_SIZE      "max-fw-size"
> +#define PC_MACHINE_SMBIOS_EP        "smbios-ep"
>  
>  /**
>   * PCMachineClass:
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 640fb5b0b7..3cc559e0d9 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -81,6 +81,7 @@
>  #include "hw/mem/nvdimm.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-visit-common.h"
> +#include "qapi/qapi-visit-smbios.h"
>  #include "qapi/visitor.h"
>  #include "hw/core/cpu.h"
>  #include "hw/usb.h"
> @@ -1532,6 +1533,23 @@ static void pc_machine_set_hpet(Object *obj, bool value, Error **errp)
>      pcms->hpet_enabled = value;
>  }
>  
> +static void pc_machine_get_smbios_ep(Object *obj, Visitor *v, const char *name,
> +                                     void *opaque, Error **errp)
> +{
> +    PCMachineState *pcms = PC_MACHINE(obj);
> +    SmbiosEntryPointType smbios_ep = pcms->smbios_ep;
> +
> +    visit_type_SmbiosEntryPointType(v, name, &smbios_ep, errp);
> +}
> +
> +static void pc_machine_set_smbios_ep(Object *obj, Visitor *v, const char *name,
> +                                     void *opaque, Error **errp)
> +{
> +    PCMachineState *pcms = PC_MACHINE(obj);
> +
> +    visit_type_SmbiosEntryPointType(v, name, &pcms->smbios_ep, errp);
> +}
> +
>  static void pc_machine_get_max_ram_below_4g(Object *obj, Visitor *v,
>                                              const char *name, void *opaque,
>                                              Error **errp)
> @@ -1621,6 +1639,8 @@ static void pc_machine_initfn(Object *obj)
>      pcms->vmport = ON_OFF_AUTO_OFF;
>  #endif /* CONFIG_VMPORT */
>      pcms->max_ram_below_4g = 0; /* use default */
> +    pcms->smbios_ep = SMBIOS_ENTRY_POINT_TYPE_2_1;
> +
>      /* acpi build is enabled by default if machine supports it */
>      pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
>      pcms->smbus_enabled = true;
> @@ -1759,6 +1779,12 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>          NULL, NULL);
>      object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE,
>          "Maximum combined firmware size");
> +
> +    object_class_property_add(oc, PC_MACHINE_SMBIOS_EP, "str",
> +        pc_machine_get_smbios_ep, pc_machine_set_smbios_ep,
> +        NULL, NULL);
> +    object_class_property_set_description(oc, PC_MACHINE_SMBIOS_EP,
> +        "SMBIOS Entry Point version [v2_1, v3_0]");

To me this reads like the legal values are v2_1 and v3_0, in fact
they are 2_1 and 3_0.


>  }
>  
>  static const TypeInfo pc_machine_info = {
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 08b82df4d1..30ae7f27af 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_TYPE_2_1);
> +                            pcms->smbios_ep);
>      }
>  
>      /* allocate ram and load rom/bios */
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index f71b1e2dcf..9974426806 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -198,7 +198,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_TYPE_2_1);
> +                            pcms->smbios_ep);
>      }
>  
>      /* allocate ram and load rom/bios */
> -- 
> 2.28.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-01-05 11:04 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14 20:50 [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type Eduardo Habkost
2020-12-14 20:50 ` [PATCH v2 1/3] smbios: Rename SMBIOS_ENTRY_POINT_* enums Eduardo Habkost
2020-12-29 13:20   ` Igor Mammedov
2020-12-14 20:50 ` [PATCH v2 2/3] hw/smbios: Use qapi for SmbiosEntryPointType Eduardo Habkost
2020-12-29 13:21   ` Igor Mammedov
2020-12-14 20:50 ` [PATCH v2 3/3] hw/i386: expose a "smbios-ep" PC machine property Eduardo Habkost
2021-01-04 22:44   ` Igor Mammedov
2021-01-05 11:02   ` Michael S. Tsirkin
2020-12-29 13:20 ` [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type Igor Mammedov
2021-01-04 22:10   ` Eduardo Habkost
2020-12-29 15:10 ` Philippe Mathieu-Daudé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.