All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/2] hw/i386/pc: Update max_cpus and default to SMBIOS
@ 2023-06-07 20:57 Suravee Suthikulpanit
  2023-06-07 20:57 ` [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models Suravee Suthikulpanit
  2023-06-07 20:57 ` [PATCH v6 2/2] pc: q35: Bump max_cpus to 1024 Suravee Suthikulpanit
  0 siblings, 2 replies; 9+ messages in thread
From: Suravee Suthikulpanit @ 2023-06-07 20:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: pbonzini, richard.henderson, eduardo, mst, marcel.apfelbaum,
	imammedo, berrange, jusual, dfaggioli, joao.m.martins, jon.grimm,
	santosh.Shukla, Suravee Suthikulpanit

In order to support large number of vcpus, a newer 64-bit SMBIOS
entry point type is needed. Therefore, upgrade the default SMBIOS version
for PC machines to SMBIOS 3.0 for newer systems. Then increase the maximum
number of vCPUs for Q35 models to 1024, which is the limit for KVM.

Changes from V5:
(https://lore.kernel.org/qemu-devel/20230607024939.703991-1-suravee.suthikulpanit@amd.com/T/#m5a9f0d0e2355aebf81501355a1bf349a9929f4bb)
 * Patch 1: Get rid of pc_machine_init_smbios() and simplify the logic
   per Igor's suggestion.
 * Patch 2: Added reviewed-by tag.

Thank you,
Suravee

Suravee Suthikulpanit (2):
  hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models
  pc: q35: Bump max_cpus to 1024

 hw/i386/pc.c         | 4 +++-
 hw/i386/pc_piix.c    | 5 +++++
 hw/i386/pc_q35.c     | 8 +++++++-
 include/hw/i386/pc.h | 1 +
 4 files changed, 16 insertions(+), 2 deletions(-)

-- 
2.34.1



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

* [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models
  2023-06-07 20:57 [PATCH v6 0/2] hw/i386/pc: Update max_cpus and default to SMBIOS Suravee Suthikulpanit
@ 2023-06-07 20:57 ` Suravee Suthikulpanit
  2023-06-08  7:52   ` Daniel P. Berrangé
                     ` (2 more replies)
  2023-06-07 20:57 ` [PATCH v6 2/2] pc: q35: Bump max_cpus to 1024 Suravee Suthikulpanit
  1 sibling, 3 replies; 9+ messages in thread
From: Suravee Suthikulpanit @ 2023-06-07 20:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: pbonzini, richard.henderson, eduardo, mst, marcel.apfelbaum,
	imammedo, berrange, jusual, dfaggioli, joao.m.martins, jon.grimm,
	santosh.Shukla, Suravee Suthikulpanit

Currently, pc-q35 and pc-i44fx machine models are default to use SMBIOS 2.8
(32-bit entry point). Since SMBIOS 3.0 (64-bit entry point) is now fully
supported since QEMU 7.0, default to use SMBIOS 3.0 for newer machine
models. This is necessary to avoid the following message when launching
a VM with large number of vcpus.

   "SMBIOS 2.1 table length 66822 exceeds 65535"

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 hw/i386/pc.c         | 4 +++-
 hw/i386/pc_piix.c    | 5 +++++
 hw/i386/pc_q35.c     | 5 +++++
 include/hw/i386/pc.h | 1 +
 4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index bb62c994fa..33ffb03a32 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1853,6 +1853,7 @@ static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
 static void pc_machine_initfn(Object *obj)
 {
     PCMachineState *pcms = PC_MACHINE(obj);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
 
 #ifdef CONFIG_VMPORT
     pcms->vmport = ON_OFF_AUTO_AUTO;
@@ -1860,7 +1861,7 @@ 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_entry_point_type = SMBIOS_ENTRY_POINT_TYPE_32;
+    pcms->smbios_entry_point_type = pcmc->default_smbios_ep_type;
 
     /* acpi build is enabled by default if machine supports it */
     pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
@@ -1980,6 +1981,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     mc->nvdimm_supported = true;
     mc->smp_props.dies_supported = true;
     mc->default_ram_id = "pc.ram";
+    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_64;
 
     object_class_property_add(oc, PC_MACHINE_MAX_RAM_BELOW_4G, "size",
         pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d5b0dcd1fe..49462b0e29 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -476,11 +476,16 @@ DEFINE_I440FX_MACHINE(v8_1, "pc-i440fx-8.1", NULL,
 
 static void pc_i440fx_8_0_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_8_1_machine_options(m);
     m->alias = NULL;
     m->is_default = false;
     compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
     compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
+
+    /* For pc-i44fx-8.0 and older, use SMBIOS 2.8 by default */
+    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_32;
 }
 
 DEFINE_I440FX_MACHINE(v8_0, "pc-i440fx-8.0", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 6155427e48..6b9fd4d537 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -387,10 +387,15 @@ DEFINE_Q35_MACHINE(v8_1, "pc-q35-8.1", NULL,
 
 static void pc_q35_8_0_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_q35_8_1_machine_options(m);
     m->alias = NULL;
     compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
     compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
+
+    /* For pc-q35-8.0 and older, use SMBIOS 2.8 by default */
+    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_32;
 }
 
 DEFINE_Q35_MACHINE(v8_0, "pc-q35-8.0", NULL,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index c661e9cc80..6eec0fc51d 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -110,6 +110,7 @@ struct PCMachineClass {
     bool smbios_defaults;
     bool smbios_legacy_mode;
     bool smbios_uuid_encoded;
+    SmbiosEntryPointType default_smbios_ep_type;
 
     /* RAM / address space compat: */
     bool gigabyte_align;
-- 
2.34.1



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

* [PATCH v6 2/2] pc: q35: Bump max_cpus to 1024
  2023-06-07 20:57 [PATCH v6 0/2] hw/i386/pc: Update max_cpus and default to SMBIOS Suravee Suthikulpanit
  2023-06-07 20:57 ` [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models Suravee Suthikulpanit
@ 2023-06-07 20:57 ` Suravee Suthikulpanit
  2023-06-08  7:52   ` Daniel P. Berrangé
  1 sibling, 1 reply; 9+ messages in thread
From: Suravee Suthikulpanit @ 2023-06-07 20:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: pbonzini, richard.henderson, eduardo, mst, marcel.apfelbaum,
	imammedo, berrange, jusual, dfaggioli, joao.m.martins, jon.grimm,
	santosh.Shukla, Suravee Suthikulpanit

Since KVM_MAX_VCPUS is currently defined to 1024 for x86 as shown in
arch/x86/include/asm/kvm_host.h, update QEMU limits to the same number.

In case KVM could not support the specified number of vcpus, QEMU would
return the following error message:

  qemu-system-x86_64: kvm_init_vcpu: kvm_get_vcpu failed (xxx): Invalid argument

Also, keep max_cpus at 288 for machine version 8.0 and older.

Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Julia Suvorova <jusual@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 hw/i386/pc_q35.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 6b9fd4d537..b26fd9bbaf 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -368,12 +368,12 @@ static void pc_q35_machine_options(MachineClass *m)
     m->default_nic = "e1000e";
     m->default_kernel_irqchip_split = false;
     m->no_floppy = 1;
+    m->max_cpus = 1024;
     m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
-    m->max_cpus = 288;
 }
 
 static void pc_q35_8_1_machine_options(MachineClass *m)
@@ -396,6 +396,7 @@ static void pc_q35_8_0_machine_options(MachineClass *m)
 
     /* For pc-q35-8.0 and older, use SMBIOS 2.8 by default */
     pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_32;
+    m->max_cpus = 288;
 }
 
 DEFINE_Q35_MACHINE(v8_0, "pc-q35-8.0", NULL,
-- 
2.34.1



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

* Re: [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models
  2023-06-07 20:57 ` [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models Suravee Suthikulpanit
@ 2023-06-08  7:52   ` Daniel P. Berrangé
  2023-06-08  8:40   ` Igor Mammedov
  2023-06-23 10:05   ` Michael S. Tsirkin
  2 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrangé @ 2023-06-08  7:52 UTC (permalink / raw)
  To: Suravee Suthikulpanit
  Cc: qemu-devel, pbonzini, richard.henderson, eduardo, mst,
	marcel.apfelbaum, imammedo, jusual, dfaggioli, joao.m.martins,
	jon.grimm, santosh.Shukla

On Wed, Jun 07, 2023 at 03:57:16PM -0500, Suravee Suthikulpanit wrote:
> Currently, pc-q35 and pc-i44fx machine models are default to use SMBIOS 2.8
> (32-bit entry point). Since SMBIOS 3.0 (64-bit entry point) is now fully
> supported since QEMU 7.0, default to use SMBIOS 3.0 for newer machine
> models. This is necessary to avoid the following message when launching
> a VM with large number of vcpus.
> 
>    "SMBIOS 2.1 table length 66822 exceeds 65535"
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  hw/i386/pc.c         | 4 +++-
>  hw/i386/pc_piix.c    | 5 +++++
>  hw/i386/pc_q35.c     | 5 +++++
>  include/hw/i386/pc.h | 1 +
>  4 files changed, 14 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

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



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

* Re: [PATCH v6 2/2] pc: q35: Bump max_cpus to 1024
  2023-06-07 20:57 ` [PATCH v6 2/2] pc: q35: Bump max_cpus to 1024 Suravee Suthikulpanit
@ 2023-06-08  7:52   ` Daniel P. Berrangé
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrangé @ 2023-06-08  7:52 UTC (permalink / raw)
  To: Suravee Suthikulpanit
  Cc: qemu-devel, pbonzini, richard.henderson, eduardo, mst,
	marcel.apfelbaum, imammedo, jusual, dfaggioli, joao.m.martins,
	jon.grimm, santosh.Shukla

On Wed, Jun 07, 2023 at 03:57:17PM -0500, Suravee Suthikulpanit wrote:
> Since KVM_MAX_VCPUS is currently defined to 1024 for x86 as shown in
> arch/x86/include/asm/kvm_host.h, update QEMU limits to the same number.
> 
> In case KVM could not support the specified number of vcpus, QEMU would
> return the following error message:
> 
>   qemu-system-x86_64: kvm_init_vcpu: kvm_get_vcpu failed (xxx): Invalid argument
> 
> Also, keep max_cpus at 288 for machine version 8.0 and older.
> 
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Julia Suvorova <jusual@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  hw/i386/pc_q35.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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



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

* Re: [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models
  2023-06-07 20:57 ` [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models Suravee Suthikulpanit
  2023-06-08  7:52   ` Daniel P. Berrangé
@ 2023-06-08  8:40   ` Igor Mammedov
  2023-06-09 16:42     ` Suthikulpanit, Suravee
  2023-06-23 10:05   ` Michael S. Tsirkin
  2 siblings, 1 reply; 9+ messages in thread
From: Igor Mammedov @ 2023-06-08  8:40 UTC (permalink / raw)
  To: Suravee Suthikulpanit
  Cc: qemu-devel, pbonzini, richard.henderson, eduardo, mst,
	marcel.apfelbaum, berrange, jusual, dfaggioli, joao.m.martins,
	jon.grimm, santosh.Shukla

On Wed, 7 Jun 2023 15:57:16 -0500
Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> wrote:

> Currently, pc-q35 and pc-i44fx machine models are default to use SMBIOS 2.8
> (32-bit entry point). Since SMBIOS 3.0 (64-bit entry point) is now fully
> supported since QEMU 7.0, default to use SMBIOS 3.0 for newer machine
> models. This is necessary to avoid the following message when launching
> a VM with large number of vcpus.
> 
>    "SMBIOS 2.1 table length 66822 exceeds 65535"
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

Looks good to me (see comment below for extra cleanup possibility):

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

> ---
>  hw/i386/pc.c         | 4 +++-
>  hw/i386/pc_piix.c    | 5 +++++
>  hw/i386/pc_q35.c     | 5 +++++
>  include/hw/i386/pc.h | 1 +
>  4 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index bb62c994fa..33ffb03a32 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1853,6 +1853,7 @@ static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
>  static void pc_machine_initfn(Object *obj)
>  {
>      PCMachineState *pcms = PC_MACHINE(obj);
> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);

since you introduce this local,
I suggest you to post an extra clean up patch on top of this series

here is a line to cleanup with 'pcmc'

    /* acpi build is enabled by default if machine supports it */                
    pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;  

>  
>  #ifdef CONFIG_VMPORT
>      pcms->vmport = ON_OFF_AUTO_AUTO;
> @@ -1860,7 +1861,7 @@ 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_entry_point_type = SMBIOS_ENTRY_POINT_TYPE_32;
> +    pcms->smbios_entry_point_type = pcmc->default_smbios_ep_type;
>  
>      /* acpi build is enabled by default if machine supports it */
>      pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
> @@ -1980,6 +1981,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>      mc->nvdimm_supported = true;
>      mc->smp_props.dies_supported = true;
>      mc->default_ram_id = "pc.ram";
> +    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_64;
>  
>      object_class_property_add(oc, PC_MACHINE_MAX_RAM_BELOW_4G, "size",
>          pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g,
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d5b0dcd1fe..49462b0e29 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -476,11 +476,16 @@ DEFINE_I440FX_MACHINE(v8_1, "pc-i440fx-8.1", NULL,
>  
>  static void pc_i440fx_8_0_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_8_1_machine_options(m);
>      m->alias = NULL;
>      m->is_default = false;
>      compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
>      compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
> +
> +    /* For pc-i44fx-8.0 and older, use SMBIOS 2.8 by default */
> +    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_32;
>  }
>  
>  DEFINE_I440FX_MACHINE(v8_0, "pc-i440fx-8.0", NULL,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 6155427e48..6b9fd4d537 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -387,10 +387,15 @@ DEFINE_Q35_MACHINE(v8_1, "pc-q35-8.1", NULL,
>  
>  static void pc_q35_8_0_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_q35_8_1_machine_options(m);
>      m->alias = NULL;
>      compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
>      compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
> +
> +    /* For pc-q35-8.0 and older, use SMBIOS 2.8 by default */
> +    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_32;
>  }
>  
>  DEFINE_Q35_MACHINE(v8_0, "pc-q35-8.0", NULL,
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index c661e9cc80..6eec0fc51d 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -110,6 +110,7 @@ struct PCMachineClass {
>      bool smbios_defaults;
>      bool smbios_legacy_mode;
>      bool smbios_uuid_encoded;
> +    SmbiosEntryPointType default_smbios_ep_type;
>  
>      /* RAM / address space compat: */
>      bool gigabyte_align;



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

* Re: [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models
  2023-06-08  8:40   ` Igor Mammedov
@ 2023-06-09 16:42     ` Suthikulpanit, Suravee
  0 siblings, 0 replies; 9+ messages in thread
From: Suthikulpanit, Suravee @ 2023-06-09 16:42 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: qemu-devel, pbonzini, richard.henderson, eduardo, mst,
	marcel.apfelbaum, berrange, jusual, dfaggioli, joao.m.martins,
	jon.grimm, santosh.Shukla



On 6/8/2023 3:40 PM, Igor Mammedov wrote:
> On Wed, 7 Jun 2023 15:57:16 -0500
> Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>  wrote:
> 
>> Currently, pc-q35 and pc-i44fx machine models are default to use SMBIOS 2.8
>> (32-bit entry point). Since SMBIOS 3.0 (64-bit entry point) is now fully
>> supported since QEMU 7.0, default to use SMBIOS 3.0 for newer machine
>> models. This is necessary to avoid the following message when launching
>> a VM with large number of vcpus.
>>
>>     "SMBIOS 2.1 table length 66822 exceeds 65535"
>>
>> Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>
> Looks good to me (see comment below for extra cleanup possibility):
> 
> Reviewed-by: Igor Mammedov<imammedo@redhat.com>
> 
>> ---
>>   hw/i386/pc.c         | 4 +++-
>>   hw/i386/pc_piix.c    | 5 +++++
>>   hw/i386/pc_q35.c     | 5 +++++
>>   include/hw/i386/pc.h | 1 +
>>   4 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index bb62c994fa..33ffb03a32 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1853,6 +1853,7 @@ static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
>>   static void pc_machine_initfn(Object *obj)
>>   {
>>       PCMachineState *pcms = PC_MACHINE(obj);
>> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> since you introduce this local,
> I suggest you to post an extra clean up patch on top of this series
> 
> here is a line to cleanup with 'pcmc'
> 
>      /* acpi build is enabled by default if machine supports it */
>      pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;

Sure just sent out the "[PATCH] hw/i386/pc: Clean up pc_machine_initfn".

Thanks,
Suravee


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

* Re: [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models
  2023-06-07 20:57 ` [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models Suravee Suthikulpanit
  2023-06-08  7:52   ` Daniel P. Berrangé
  2023-06-08  8:40   ` Igor Mammedov
@ 2023-06-23 10:05   ` Michael S. Tsirkin
  2023-06-23 14:40     ` Igor Mammedov
  2 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2023-06-23 10:05 UTC (permalink / raw)
  To: Suravee Suthikulpanit
  Cc: qemu-devel, pbonzini, richard.henderson, eduardo,
	marcel.apfelbaum, imammedo, berrange, jusual, dfaggioli,
	joao.m.martins, jon.grimm, santosh.Shukla

On Wed, Jun 07, 2023 at 03:57:16PM -0500, Suravee Suthikulpanit wrote:
> Currently, pc-q35 and pc-i44fx machine models are default to use SMBIOS 2.8
> (32-bit entry point). Since SMBIOS 3.0 (64-bit entry point) is now fully
> supported since QEMU 7.0, default to use SMBIOS 3.0 for newer machine
> models. This is necessary to avoid the following message when launching
> a VM with large number of vcpus.
> 
>    "SMBIOS 2.1 table length 66822 exceeds 65535"
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

For some reason this causes a diff in ACPI:

$ diff -u /tmp/asl-WQRZ61.dsl /tmp/asl-S25X61.dsl 
--- /tmp/asl-WQRZ61.dsl 2023-06-23 05:37:54.067525946 -0400
+++ /tmp/asl-S25X61.dsl 2023-06-23 05:37:54.073525987 -0400
@@ -5,13 +5,13 @@
  * 
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of /tmp/aml-IROZ61, Fri Jun 23 05:37:54 2023
+ * Disassembly of tests/data/acpi/q35/SSDT.dimmpxm, Fri Jun 23 05:37:54 2023
  *
  * Original Table Header:
  *     Signature        "SSDT"
  *     Length           0x00000717 (1815)
  *     Revision         0x01
- *     Checksum         0xBC
+ *     Checksum         0xAC
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "NVDIMM"
  *     OEM Revision     0x00000001 (1)
@@ -389,6 +389,6 @@
         }
     }
 
-    Name (MEMA, 0x07FFE000)
+    Name (MEMA, 0x07FFF000)
 }
 

Igor, any idea why?



> ---
>  hw/i386/pc.c         | 4 +++-
>  hw/i386/pc_piix.c    | 5 +++++
>  hw/i386/pc_q35.c     | 5 +++++
>  include/hw/i386/pc.h | 1 +
>  4 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index bb62c994fa..33ffb03a32 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1853,6 +1853,7 @@ static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
>  static void pc_machine_initfn(Object *obj)
>  {
>      PCMachineState *pcms = PC_MACHINE(obj);
> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>  
>  #ifdef CONFIG_VMPORT
>      pcms->vmport = ON_OFF_AUTO_AUTO;
> @@ -1860,7 +1861,7 @@ 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_entry_point_type = SMBIOS_ENTRY_POINT_TYPE_32;
> +    pcms->smbios_entry_point_type = pcmc->default_smbios_ep_type;
>  
>      /* acpi build is enabled by default if machine supports it */
>      pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
> @@ -1980,6 +1981,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>      mc->nvdimm_supported = true;
>      mc->smp_props.dies_supported = true;
>      mc->default_ram_id = "pc.ram";
> +    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_64;
>  
>      object_class_property_add(oc, PC_MACHINE_MAX_RAM_BELOW_4G, "size",
>          pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g,
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d5b0dcd1fe..49462b0e29 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -476,11 +476,16 @@ DEFINE_I440FX_MACHINE(v8_1, "pc-i440fx-8.1", NULL,
>  
>  static void pc_i440fx_8_0_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_8_1_machine_options(m);
>      m->alias = NULL;
>      m->is_default = false;
>      compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
>      compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
> +
> +    /* For pc-i44fx-8.0 and older, use SMBIOS 2.8 by default */
> +    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_32;
>  }
>  
>  DEFINE_I440FX_MACHINE(v8_0, "pc-i440fx-8.0", NULL,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 6155427e48..6b9fd4d537 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -387,10 +387,15 @@ DEFINE_Q35_MACHINE(v8_1, "pc-q35-8.1", NULL,
>  
>  static void pc_q35_8_0_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_q35_8_1_machine_options(m);
>      m->alias = NULL;
>      compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
>      compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
> +
> +    /* For pc-q35-8.0 and older, use SMBIOS 2.8 by default */
> +    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_32;
>  }
>  
>  DEFINE_Q35_MACHINE(v8_0, "pc-q35-8.0", NULL,
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index c661e9cc80..6eec0fc51d 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -110,6 +110,7 @@ struct PCMachineClass {
>      bool smbios_defaults;
>      bool smbios_legacy_mode;
>      bool smbios_uuid_encoded;
> +    SmbiosEntryPointType default_smbios_ep_type;
>  
>      /* RAM / address space compat: */
>      bool gigabyte_align;
> -- 
> 2.34.1



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

* Re: [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models
  2023-06-23 10:05   ` Michael S. Tsirkin
@ 2023-06-23 14:40     ` Igor Mammedov
  0 siblings, 0 replies; 9+ messages in thread
From: Igor Mammedov @ 2023-06-23 14:40 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Suravee Suthikulpanit, qemu-devel, pbonzini, richard.henderson,
	eduardo, marcel.apfelbaum, berrange, jusual, dfaggioli,
	joao.m.martins, jon.grimm, santosh.Shukla

On Fri, 23 Jun 2023 06:05:28 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Jun 07, 2023 at 03:57:16PM -0500, Suravee Suthikulpanit wrote:
> > Currently, pc-q35 and pc-i44fx machine models are default to use SMBIOS 2.8
> > (32-bit entry point). Since SMBIOS 3.0 (64-bit entry point) is now fully
> > supported since QEMU 7.0, default to use SMBIOS 3.0 for newer machine
> > models. This is necessary to avoid the following message when launching
> > a VM with large number of vcpus.
> > 
> >    "SMBIOS 2.1 table length 66822 exceeds 65535"
> > 
> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>  
> 
> For some reason this causes a diff in ACPI:
> 
> $ diff -u /tmp/asl-WQRZ61.dsl /tmp/asl-S25X61.dsl 
> --- /tmp/asl-WQRZ61.dsl 2023-06-23 05:37:54.067525946 -0400
> +++ /tmp/asl-S25X61.dsl 2023-06-23 05:37:54.073525987 -0400
> @@ -5,13 +5,13 @@
>   * 
>   * Disassembling to symbolic ASL+ operators
>   *
> - * Disassembly of /tmp/aml-IROZ61, Fri Jun 23 05:37:54 2023
> + * Disassembly of tests/data/acpi/q35/SSDT.dimmpxm, Fri Jun 23 05:37:54 2023
>   *
>   * Original Table Header:
>   *     Signature        "SSDT"
>   *     Length           0x00000717 (1815)
>   *     Revision         0x01
> - *     Checksum         0xBC
> + *     Checksum         0xAC
>   *     OEM ID           "BOCHS "
>   *     OEM Table ID     "NVDIMM"
>   *     OEM Revision     0x00000001 (1)
> @@ -389,6 +389,6 @@
>          }
>      }
>  
> -    Name (MEMA, 0x07FFE000)
> +    Name (MEMA, 0x07FFF000)
>  }
>  
> 
> Igor, any idea why?

It should be due to smbios tables size difference, which causes ACPI blob
allocated differently => different patched pointer. 

one can verify by adding '-machine  smbios-entry-point-type=64'
to test_acpi_tcg_dimm_pxm()

> 
> 
> > ---
> >  hw/i386/pc.c         | 4 +++-
> >  hw/i386/pc_piix.c    | 5 +++++
> >  hw/i386/pc_q35.c     | 5 +++++
> >  include/hw/i386/pc.h | 1 +
> >  4 files changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index bb62c994fa..33ffb03a32 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1853,6 +1853,7 @@ static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
> >  static void pc_machine_initfn(Object *obj)
> >  {
> >      PCMachineState *pcms = PC_MACHINE(obj);
> > +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> >  
> >  #ifdef CONFIG_VMPORT
> >      pcms->vmport = ON_OFF_AUTO_AUTO;
> > @@ -1860,7 +1861,7 @@ 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_entry_point_type = SMBIOS_ENTRY_POINT_TYPE_32;
> > +    pcms->smbios_entry_point_type = pcmc->default_smbios_ep_type;
> >  
> >      /* acpi build is enabled by default if machine supports it */
> >      pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
> > @@ -1980,6 +1981,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
> >      mc->nvdimm_supported = true;
> >      mc->smp_props.dies_supported = true;
> >      mc->default_ram_id = "pc.ram";
> > +    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_64;
> >  
> >      object_class_property_add(oc, PC_MACHINE_MAX_RAM_BELOW_4G, "size",
> >          pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g,
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index d5b0dcd1fe..49462b0e29 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -476,11 +476,16 @@ DEFINE_I440FX_MACHINE(v8_1, "pc-i440fx-8.1", NULL,
> >  
> >  static void pc_i440fx_8_0_machine_options(MachineClass *m)
> >  {
> > +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> > +
> >      pc_i440fx_8_1_machine_options(m);
> >      m->alias = NULL;
> >      m->is_default = false;
> >      compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
> >      compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
> > +
> > +    /* For pc-i44fx-8.0 and older, use SMBIOS 2.8 by default */
> > +    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_32;
> >  }
> >  
> >  DEFINE_I440FX_MACHINE(v8_0, "pc-i440fx-8.0", NULL,
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 6155427e48..6b9fd4d537 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -387,10 +387,15 @@ DEFINE_Q35_MACHINE(v8_1, "pc-q35-8.1", NULL,
> >  
> >  static void pc_q35_8_0_machine_options(MachineClass *m)
> >  {
> > +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> > +
> >      pc_q35_8_1_machine_options(m);
> >      m->alias = NULL;
> >      compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
> >      compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
> > +
> > +    /* For pc-q35-8.0 and older, use SMBIOS 2.8 by default */
> > +    pcmc->default_smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_32;
> >  }
> >  
> >  DEFINE_Q35_MACHINE(v8_0, "pc-q35-8.0", NULL,
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index c661e9cc80..6eec0fc51d 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -110,6 +110,7 @@ struct PCMachineClass {
> >      bool smbios_defaults;
> >      bool smbios_legacy_mode;
> >      bool smbios_uuid_encoded;
> > +    SmbiosEntryPointType default_smbios_ep_type;
> >  
> >      /* RAM / address space compat: */
> >      bool gigabyte_align;
> > -- 
> > 2.34.1  
> 



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

end of thread, other threads:[~2023-06-23 14:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-07 20:57 [PATCH v6 0/2] hw/i386/pc: Update max_cpus and default to SMBIOS Suravee Suthikulpanit
2023-06-07 20:57 ` [PATCH v6 1/2] hw/i386/pc: Default to use SMBIOS 3.0 for newer machine models Suravee Suthikulpanit
2023-06-08  7:52   ` Daniel P. Berrangé
2023-06-08  8:40   ` Igor Mammedov
2023-06-09 16:42     ` Suthikulpanit, Suravee
2023-06-23 10:05   ` Michael S. Tsirkin
2023-06-23 14:40     ` Igor Mammedov
2023-06-07 20:57 ` [PATCH v6 2/2] pc: q35: Bump max_cpus to 1024 Suravee Suthikulpanit
2023-06-08  7:52   ` Daniel P. Berrangé

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.