diff mbox series

[v2] pc: q35: Bump max_cpus to 1024

Message ID 20230531225127.331998-1-suravee.suthikulpanit@amd.com (mailing list archive)
State New, archived
Headers show
Series [v2] pc: q35: Bump max_cpus to 1024 | expand

Commit Message

Suthikulpanit, Suravee May 31, 2023, 10:51 p.m. UTC
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

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---

Changes from V1:
(https://lore.kernel.org/all/YnkDGsIi1vFvXmiP@redhat.com/T/)
 * Bump from 512 to KVM_MAX_VCPUS (per Igor's suggestion)

Note:
 From the last discussion, Daniel mentioned that SMBIO 2.1 tables might
 cause overflow at approx 720 CPUs, and it might require using the
 SMBIO 3.0 entry point. Also, we might need to change the default for
 the x86 machine type to SMBIO 3.0. However, I do not know the status
 of this.

Thank you,
Suravee

 hw/i386/pc_q35.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Joao Martins June 1, 2023, 10:09 a.m. UTC | #1
On 31/05/2023 23:51, 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
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
> 
> Changes from V1:
> (https://lore.kernel.org/all/YnkDGsIi1vFvXmiP@redhat.com/T/)
>  * Bump from 512 to KVM_MAX_VCPUS (per Igor's suggestion)
> 
> Note:
>  From the last discussion, Daniel mentioned that SMBIO 2.1 tables might
>  cause overflow at approx 720 CPUs, and it might require using the
>  SMBIO 3.0 entry point. Also, we might need to change the default for
>  the x86 machine type to SMBIO 3.0. However, I do not know the status
>  of this.
> 
I suspect smbios 3.0 (64-bit entry point) is already supported.

With current qemu and all the smbios fixes in the last cycle, perhaps this is
mainly just setting smbios_entry_point_type to SMBIOS_ENTRY_POINT_TYPE_64 if
MachineState::smp::max_cpus is bigger than 720 (e.g. in pc_q35_init()?)

> Thank you,
> Suravee
> 
>  hw/i386/pc_q35.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index f02919d92c..e7dc226bd5 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -370,7 +370,7 @@ static void pc_q35_machine_options(MachineClass *m)
>      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;
> +    m->max_cpus = 1024;
>  }
>  
>  static void pc_q35_8_1_machine_options(MachineClass *m)
Daniel P. Berrangé June 1, 2023, 10:17 a.m. UTC | #2
On Thu, Jun 01, 2023 at 11:09:45AM +0100, Joao Martins wrote:
> 
> 
> On 31/05/2023 23:51, 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
> > 
> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > ---
> > 
> > Changes from V1:
> > (https://lore.kernel.org/all/YnkDGsIi1vFvXmiP@redhat.com/T/)
> >  * Bump from 512 to KVM_MAX_VCPUS (per Igor's suggestion)
> > 
> > Note:
> >  From the last discussion, Daniel mentioned that SMBIO 2.1 tables might
> >  cause overflow at approx 720 CPUs, and it might require using the
> >  SMBIO 3.0 entry point. Also, we might need to change the default for
> >  the x86 machine type to SMBIO 3.0. However, I do not know the status
> >  of this.
> > 
> I suspect smbios 3.0 (64-bit entry point) is already supported.
> 
> With current qemu and all the smbios fixes in the last cycle, perhaps this is
> mainly just setting smbios_entry_point_type to SMBIOS_ENTRY_POINT_TYPE_64 if
> MachineState::smp::max_cpus is bigger than 720 (e.g. in pc_q35_init()?)

The need for the 64-bit entry point depends on a combination of RAM config
and CPU count. IMHO we need to unconditionally switch the latest machine
types to use the 64-bit entry point by default, rather than trying to infer
some special condition to dynamically change on the fly.

Before doing that we need confirmation that our firmware builds in qemu.git
(SeaBIOS and EDK2) will be happy with the 64-bit EP. I suspect they should
be fine by now, but I've CC'd Gerd to confirm.

With regards,
Daniel
Gerd Hoffmann June 1, 2023, 11:03 a.m. UTC | #3
Hi,

> > I suspect smbios 3.0 (64-bit entry point) is already supported.
> > 
> > With current qemu and all the smbios fixes in the last cycle, perhaps this is
> > mainly just setting smbios_entry_point_type to SMBIOS_ENTRY_POINT_TYPE_64 if
> > MachineState::smp::max_cpus is bigger than 720 (e.g. in pc_q35_init()?)
> 
> The need for the 64-bit entry point depends on a combination of RAM config
> and CPU count. IMHO we need to unconditionally switch the latest machine
> types to use the 64-bit entry point by default, rather than trying to infer
> some special condition to dynamically change on the fly.
> 
> Before doing that we need confirmation that our firmware builds in qemu.git
> (SeaBIOS and EDK2) will be happy with the 64-bit EP. I suspect they should
> be fine by now, but I've CC'd Gerd to confirm.

edk2 is ready for quite a while (years).

seabios was lagging behind for a long time, but smbios 3.0 support
finally landed early last year, in release 1.16.0 (qemu 7.0).

So, yes, firmware-wise we are ready to flip the default now.

take care,
  Gerd
Michael S. Tsirkin June 1, 2023, 11:09 a.m. UTC | #4
On Thu, Jun 01, 2023 at 11:17:30AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 01, 2023 at 11:09:45AM +0100, Joao Martins wrote:
> > 
> > 
> > On 31/05/2023 23:51, 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
> > > 
> > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > > ---
> > > 
> > > Changes from V1:
> > > (https://lore.kernel.org/all/YnkDGsIi1vFvXmiP@redhat.com/T/)
> > >  * Bump from 512 to KVM_MAX_VCPUS (per Igor's suggestion)
> > > 
> > > Note:
> > >  From the last discussion, Daniel mentioned that SMBIO 2.1 tables might
> > >  cause overflow at approx 720 CPUs, and it might require using the
> > >  SMBIO 3.0 entry point. Also, we might need to change the default for
> > >  the x86 machine type to SMBIO 3.0. However, I do not know the status
> > >  of this.
> > > 
> > I suspect smbios 3.0 (64-bit entry point) is already supported.
> > 
> > With current qemu and all the smbios fixes in the last cycle, perhaps this is
> > mainly just setting smbios_entry_point_type to SMBIOS_ENTRY_POINT_TYPE_64 if
> > MachineState::smp::max_cpus is bigger than 720 (e.g. in pc_q35_init()?)
> 
> The need for the 64-bit entry point depends on a combination of RAM config
> and CPU count. IMHO we need to unconditionally switch the latest machine
> types to use the 64-bit entry point by default, rather than trying to infer
> some special condition to dynamically change on the fly.

Yes, makes sense.

> Before doing that we need confirmation that our firmware builds in qemu.git
> (SeaBIOS and EDK2) will be happy with the 64-bit EP. I suspect they should
> be fine by now, but I've CC'd Gerd to confirm.
> 
> With regards,
> Daniel
Suthikulpanit, Suravee June 1, 2023, 9:54 p.m. UTC | #5
On 6/1/2023 6:09 PM, Michael S. Tsirkin wrote:
> On Thu, Jun 01, 2023 at 11:17:30AM +0100, Daniel P. Berrangé wrote:
>> On Thu, Jun 01, 2023 at 11:09:45AM +0100, Joao Martins wrote:
>>>
>>> On 31/05/2023 23:51, 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
>>>>
>>>> Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>
>>>> ---
>>>>
>>>> Changes from V1:
>>>> (https://lore.kernel.org/all/YnkDGsIi1vFvXmiP@redhat.com/T/)
>>>>   * Bump from 512 to KVM_MAX_VCPUS (per Igor's suggestion)
>>>>
>>>> Note:
>>>>   From the last discussion, Daniel mentioned that SMBIO 2.1 tables might
>>>>   cause overflow at approx 720 CPUs, and it might require using the
>>>>   SMBIO 3.0 entry point. Also, we might need to change the default for
>>>>   the x86 machine type to SMBIO 3.0. However, I do not know the status
>>>>   of this.
>>>>
>>> I suspect smbios 3.0 (64-bit entry point) is already supported.
>>>
>>> With current qemu and all the smbios fixes in the last cycle, perhaps this is
>>> mainly just setting smbios_entry_point_type to SMBIOS_ENTRY_POINT_TYPE_64 if
>>> MachineState::smp::max_cpus is bigger than 720 (e.g. in pc_q35_init()?)
>> >> The need for the 64-bit entry point depends on a combination of RAM 
config
>> and CPU count. IMHO we need to unconditionally switch the latest machine
>> types to use the 64-bit entry point by default, rather than trying to infer
>> some special condition to dynamically change on the fly.
> Yes, makes sense.

Thanks all for the feedback. So, IIUC, here is how the SMBIOS entry 
point types would be affected by the QEMU options:

- pc-q35-8.1 and later, default to SMBIOS EP type 64.
- pc-q35-8.0 and older, default to SMBIOS EP type 32.
- User can override the type w/ QEMU option "-M .., 
smbios-entry-point-type=[32|64]"

Please let me know if I am missing anything. If this is accurate, I'll 
send out v3 with this change.

Thanks,
Suravee
Daniel P. Berrangé June 2, 2023, 8:43 a.m. UTC | #6
On Thu, Jun 01, 2023 at 02:54:54PM -0700, Suthikulpanit, Suravee wrote:
> 
> 
> On 6/1/2023 6:09 PM, Michael S. Tsirkin wrote:
> > On Thu, Jun 01, 2023 at 11:17:30AM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jun 01, 2023 at 11:09:45AM +0100, Joao Martins wrote:
> > > > 
> > > > On 31/05/2023 23:51, 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
> > > > > 
> > > > > Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>
> > > > > ---
> > > > > 
> > > > > Changes from V1:
> > > > > (https://lore.kernel.org/all/YnkDGsIi1vFvXmiP@redhat.com/T/)
> > > > >   * Bump from 512 to KVM_MAX_VCPUS (per Igor's suggestion)
> > > > > 
> > > > > Note:
> > > > >   From the last discussion, Daniel mentioned that SMBIO 2.1 tables might
> > > > >   cause overflow at approx 720 CPUs, and it might require using the
> > > > >   SMBIO 3.0 entry point. Also, we might need to change the default for
> > > > >   the x86 machine type to SMBIO 3.0. However, I do not know the status
> > > > >   of this.
> > > > > 
> > > > I suspect smbios 3.0 (64-bit entry point) is already supported.
> > > > 
> > > > With current qemu and all the smbios fixes in the last cycle, perhaps this is
> > > > mainly just setting smbios_entry_point_type to SMBIOS_ENTRY_POINT_TYPE_64 if
> > > > MachineState::smp::max_cpus is bigger than 720 (e.g. in pc_q35_init()?)
> > > >> The need for the 64-bit entry point depends on a combination of
> > > RAM
> config
> > > and CPU count. IMHO we need to unconditionally switch the latest machine
> > > types to use the 64-bit entry point by default, rather than trying to infer
> > > some special condition to dynamically change on the fly.
> > Yes, makes sense.
> 
> Thanks all for the feedback. So, IIUC, here is how the SMBIOS entry point
> types would be affected by the QEMU options:
> 
> - pc-q35-8.1 and later, default to SMBIOS EP type 64.
> - pc-q35-8.0 and older, default to SMBIOS EP type 32.
> - User can override the type w/ QEMU option "-M ..,
> smbios-entry-point-type=[32|64]"
> 
> Please let me know if I am missing anything. If this is accurate, I'll send
> out v3 with this change.

Yes, and for pc-i440fx* too


With regards,
Daniel
diff mbox series

Patch

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f02919d92c..e7dc226bd5 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -370,7 +370,7 @@  static void pc_q35_machine_options(MachineClass *m)
     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;
+    m->max_cpus = 1024;
 }
 
 static void pc_q35_8_1_machine_options(MachineClass *m)