[Qemu-devel] [patch 0/2] QEMU maxcpus support v2

[Qemu-devel] [patch 0/2] QEMU maxcpus support v2

[Jes Sorensen]

Hi,

Here is an updated version of the ,maxcpus patch, now adding it as a
sub-option to -smp, ie, -smp <n>,maxcpus=<y>. In addition it has the
QEMU BOCHS patches to make the BIOS use the value passed.

Specifying maxcpus= allows the user to cap the limit of possibly cpus
of the system, as opposed to -smp which specifies the number of
running cpus at startup. This allows us to build BIOS tables
accordingly when trying to scale up in size, and it also allows for
the BIOS to build smaller tables when not needed.

I am going to submit a set of patches directly for BOCHS as well.

Cheers,
Jes

[Qemu-devel] [patch 1/2] Introduce -smp , maxcpus= flag to specify maximum number of CPUS.

[Jes Sorensen]

[-- Attachment #1: 0001-qemu-cfg-maxcpus.patch --]
[-- Type: text/plain, Size: 3781 bytes --]

Follow on patch will use it to determine the size of the MADT and
other BIOS tables.

Signed-off-by: Jes Sorensen <jes@sgi.com>

---
 hw/fw_cfg.c     |    1 +
 hw/fw_cfg.h     |    1 +
 qemu-options.hx |    5 ++++-
 sysemu.h        |    1 +
 vl.c            |   20 +++++++++++++++++++-
 5 files changed, 26 insertions(+), 2 deletions(-)

Index: qemu/hw/fw_cfg.c
===================================================================
--- qemu.orig/hw/fw_cfg.c
+++ qemu/hw/fw_cfg.c
@@ -279,6 +279,7 @@ void *fw_cfg_init(uint32_t ctl_port, uin
     fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
     fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
     fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
+    fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
 
     register_savevm("fw_cfg", -1, 1, fw_cfg_save, fw_cfg_load, s);
     qemu_register_reset(fw_cfg_reset, 0, s);
Index: qemu/hw/fw_cfg.h
===================================================================
--- qemu.orig/hw/fw_cfg.h
+++ qemu/hw/fw_cfg.h
@@ -15,6 +15,7 @@
 #define FW_CFG_INITRD_SIZE      0x0b
 #define FW_CFG_BOOT_DEVICE      0x0c
 #define FW_CFG_NUMA             0x0d
+#define FW_CFG_MAX_CPUS         0x0e
 #define FW_CFG_MAX_ENTRY        0x10
 
 #define FW_CFG_WRITE_CHANNEL    0x4000
Index: qemu/qemu-options.hx
===================================================================
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -39,7 +39,10 @@ Select CPU model (-cpu ? for list and ad
 ETEXI
 
 DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-    "-smp n          set the number of CPUs to 'n' [default=1]\n")
+    "-smp n[,maxcpus=cpus]\n"
+    "                set the number of CPUs to 'n' [default=1]\n"
+    "                maxcpus= maximum number of total cpus, including\n"
+    "                offline CPUs for hotplug etc.\n")
 STEXI
 @item -smp @var{n}
 Simulate an SMP system with @var{n} CPUs. On the PC target, up to 255
Index: qemu/sysemu.h
===================================================================
--- qemu.orig/sysemu.h
+++ qemu/sysemu.h
@@ -118,6 +118,7 @@ extern int alt_grab;
 extern int usb_enabled;
 extern int no_virtio_balloon;
 extern int smp_cpus;
+extern int max_cpus;
 extern int cursor_hide;
 extern int graphic_rotate;
 extern int no_quit;
Index: qemu/vl.c
===================================================================
--- qemu.orig/vl.c
+++ qemu/vl.c
@@ -241,6 +241,7 @@ int rtc_td_hack = 0;
 int usb_enabled = 0;
 int singlestep = 0;
 int smp_cpus = 1;
+int max_cpus = 16;
 const char *vnc_display;
 int acpi_enabled = 1;
 int no_hpet = 0;
@@ -5550,12 +5551,29 @@ int main(int argc, char **argv, char **e
                 usb_devices_index++;
                 break;
             case QEMU_OPTION_smp:
-                smp_cpus = atoi(optarg);
+            {
+                char *p;
+                char option[128];
+                smp_cpus = strtol(optarg, &p, 10);
                 if (smp_cpus < 1) {
                     fprintf(stderr, "Invalid number of CPUs\n");
                     exit(1);
                 }
+                if (*p++ != ',')
+                    break;
+                if (get_param_value(option, 128, "maxcpus", p))
+                    max_cpus = strtol(option, NULL, 0);
+                if (max_cpus < smp_cpus) {
+                    fprintf(stderr, "maxcpus must be equal to or greater than "
+                            "smp\n");
+                    exit(1);
+                }
+                if (max_cpus > 255) {
+                    fprintf(stderr, "Unsupported number of maxcpus\n");
+                    exit(1);
+                }
                 break;
+            }
 	    case QEMU_OPTION_vnc:
                 display_type = DT_VNC;
 		vnc_display = optarg;

[Qemu-devel] Re: [patch 1/2] Introduce -smp , maxcpus= flag to specify maximum number of CPUS.

[Avi Kivity]

On 06/24/2009 11:35 AM, Jes Sorensen wrote:
> Follow on patch will use it to determine the size of the MADT and
> other BIOS tables.
>
> +int max_cpus = 16;
>    

Again I should have noticed this before, but this and the other 
constants should come from the QEMUMachine we're targeting, since each 
target has different limits.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [patch 1/2] Introduce -smp , maxcpus= flag to specify maximum number of CPUS.

[Jes Sorensen]

On 06/24/2009 11:02 AM, Avi Kivity wrote:
> On 06/24/2009 11:35 AM, Jes Sorensen wrote:
>> Follow on patch will use it to determine the size of the MADT and
>> other BIOS tables.
>>
>> +int max_cpus = 16;
>
> Again I should have noticed this before, but this and the other
> constants should come from the QEMUMachine we're targeting, since each
> target has different limits.

Actually this is on purpose. The PC machine has a max limit of 255, if
we use that as the max per default, you are going to get a BIOS
declaring 255 possible CPUs and certain broken operating systems are not
going to boot.

So I will argue the code is correct as is.

Cheers,
Jes

[Qemu-devel] Re: [patch 1/2] Introduce -smp , maxcpus= flag to specify maximum number of CPUS.

[Avi Kivity]

On 06/24/2009 12:02 PM, Jes Sorensen wrote:
> On 06/24/2009 11:02 AM, Avi Kivity wrote:
>> On 06/24/2009 11:35 AM, Jes Sorensen wrote:
>>> Follow on patch will use it to determine the size of the MADT and
>>> other BIOS tables.
>>>
>>> +int max_cpus = 16;
>>
>> Again I should have noticed this before, but this and the other
>> constants should come from the QEMUMachine we're targeting, since each
>> target has different limits.
>
> Actually this is on purpose. The PC machine has a max limit of 255, if
> we use that as the max per default, you are going to get a BIOS
> declaring 255 possible CPUs and certain broken operating systems are not
> going to boot.
>
> So I will argue the code is correct as is.
>

What about the various ARMs, powerpcs, and sparcs that we support?

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [patch 1/2] Introduce -smp , maxcpus= flag to specify maximum number of CPUS.

[Jes Sorensen]

On 06/24/2009 11:15 AM, Avi Kivity wrote:
> On 06/24/2009 12:02 PM, Jes Sorensen wrote:
>> Actually this is on purpose. The PC machine has a max limit of 255, if
>> we use that as the max per default, you are going to get a BIOS
>> declaring 255 possible CPUs and certain broken operating systems are not
>> going to boot.
>>
>> So I will argue the code is correct as is.
>
> What about the various ARMs, powerpcs, and sparcs that we support?

None of them are affected by the maxcpus patch. They still have their
check making sure that the -smp argument doesn't exceed whats specified
in their machine description.

Jes

[Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Jes Sorensen]

[-- Attachment #1: 0002-qemu-bios-patches.patch --]
[-- Type: text/plain, Size: 6708 bytes --]

Signed-off-by: Jes Sorensen <jes@sgi.com>
---
 pc-bios/bios-pq/0017-qemu-kvm-cfg-maxcpus.patch |   57 +++++++++++++
 pc-bios/bios-pq/0018-qemu-madt-maxcpus.patch    |  105 ++++++++++++++++++++++++
 pc-bios/bios-pq/series                          |    2 
 3 files changed, 164 insertions(+)

Index: qemu/pc-bios/bios-pq/0017-qemu-kvm-cfg-maxcpus.patch
===================================================================
--- /dev/null
+++ qemu/pc-bios/bios-pq/0017-qemu-kvm-cfg-maxcpus.patch
@@ -0,0 +1,57 @@
+Read max_cpus variable from QEMU_CFG. If not provided, use value of
+smp_cpus.
+
+Signed-off-by: Jes Sorensen <jes@sgi.com>
+
+diff --git a/bios/rombios32.c b/bios/rombios32.c
+index f861f81..f4a6300 100644
+--- a/bios/rombios32.c
++++ b/bios/rombios32.c
+@@ -436,6 +436,7 @@ void delay_ms(int n)
+ }
+ 
+ uint16_t smp_cpus;
++uint16_t max_cpus;
+ uint32_t cpuid_signature;
+ uint32_t cpuid_features;
+ uint32_t cpuid_ext_features;
+@@ -474,6 +475,7 @@ void wrmsr_smp(uint32_t index, uint64_t val)
+ #define QEMU_CFG_ID         0x01
+ #define QEMU_CFG_UUID       0x02
+ #define QEMU_CFG_NUMA       0x0D
++#define QEMU_CFG_MAX_CPUS   0x0E
+ #define QEMU_CFG_ARCH_LOCAL     0x8000
+ #define QEMU_CFG_ACPI_TABLES  (QEMU_CFG_ARCH_LOCAL + 0)
+ #define QEMU_CFG_SMBIOS_ENTRIES  (QEMU_CFG_ARCH_LOCAL + 1)
+@@ -536,6 +538,19 @@ static uint16_t smbios_entries(void)
+     return cnt;
+ }
+ 
++static uint16_t get_max_cpus(void)
++{
++    uint16_t cnt;
++
++    qemu_cfg_select(QEMU_CFG_MAX_CPUS);
++    qemu_cfg_read((uint8_t*)&cnt, sizeof(cnt));
++
++    if (!cnt)
++        cnt = smp_cpus;
++
++    return cnt;
++}
++
+ uint64_t qemu_cfg_get64 (void)
+ {
+     uint64_t ret;
+@@ -1655,6 +1670,11 @@ void acpi_bios_init(void)
+     uint16_t i, external_tables;
+     int nb_numa_nodes;
+ 
++#ifdef BX_QEMU
++    max_cpus = get_max_cpus();
++#else
++    max_cpus = smp_cpus;
++#endif
+     /* reserve memory space for tables */
+ #ifdef BX_USE_EBDA_TABLES
+     ebda_cur_addr = align(ebda_cur_addr, 16);
Index: qemu/pc-bios/bios-pq/0018-qemu-madt-maxcpus.patch
===================================================================
--- /dev/null
+++ qemu/pc-bios/bios-pq/0018-qemu-madt-maxcpus.patch
@@ -0,0 +1,105 @@
+Use max_cpus when building bios tables.
+
+Signed-off-by: Jes Sorensen <jes@sgi.com>
+
+diff --git a/bios/rombios32.c b/bios/rombios32.c
+index f4a6300..fb4d663 100644
+--- a/bios/rombios32.c
++++ b/bios/rombios32.c
+@@ -1155,13 +1155,13 @@ static void mptable_init(void)
+     putstr(&q, "0.1         "); /* vendor id */
+     putle32(&q, 0); /* OEM table ptr */
+     putle16(&q, 0); /* OEM table size */
+-    putle16(&q, smp_cpus + 18); /* entry count */
++    putle16(&q, max_cpus + 18); /* entry count */
+     putle32(&q, 0xfee00000); /* local APIC addr */
+     putle16(&q, 0); /* ext table length */
+     putb(&q, 0); /* ext table checksum */
+     putb(&q, 0); /* reserved */
+ 
+-    for(i = 0; i < smp_cpus; i++) {
++    for(i = 0; i < max_cpus; i++) {
+         putb(&q, 0); /* entry type = processor */
+         putb(&q, i); /* APIC id */
+         putb(&q, 0x11); /* local APIC version number */
+@@ -1188,7 +1188,7 @@ static void mptable_init(void)
+     putstr(&q, "ISA   ");
+ 
+     /* ioapic */
+-    ioapic_id = smp_cpus;
++    ioapic_id = max_cpus;
+     putb(&q, 2); /* entry type = I/O APIC */
+     putb(&q, ioapic_id); /* apic ID */
+     putb(&q, 0x11); /* I/O APIC version number */
+@@ -1588,7 +1588,7 @@ int acpi_build_processor_ssdt(uint8_t *ssdt)
+ {
+     uint8_t *ssdt_ptr = ssdt;
+     int i, length;
+-    int acpi_cpus = smp_cpus > 0xff ? 0xff : smp_cpus;
++    int acpi_cpus = max_cpus > 0xff ? 0xff : max_cpus;
+ 
+     ssdt_ptr[9] = 0; // checksum;
+     ssdt_ptr += sizeof(struct acpi_table_header);
+@@ -1725,7 +1725,7 @@ void acpi_bios_init(void)
+         addr = (addr + 7) & ~7;
+         srat_addr = addr;
+         srat_size = sizeof(*srat) +
+-            sizeof(struct srat_processor_affinity) * smp_cpus +
++            sizeof(struct srat_processor_affinity) * max_cpus +
+             sizeof(struct srat_memory_affinity) * (nb_numa_nodes + 2);
+         srat = (void *)(addr);
+         addr += srat_size;
+@@ -1738,7 +1738,7 @@ void acpi_bios_init(void)
+     addr = (addr + 7) & ~7;
+     madt_addr = addr;
+     madt_size = sizeof(*madt) +
+-        sizeof(struct madt_processor_apic) * smp_cpus +
++        sizeof(struct madt_processor_apic) * max_cpus +
+ #ifdef BX_QEMU
+         sizeof(struct madt_io_apic) + sizeof(struct madt_int_override);
+ #else
+@@ -1811,7 +1811,7 @@ void acpi_bios_init(void)
+         madt->local_apic_address = cpu_to_le32(0xfee00000);
+         madt->flags = cpu_to_le32(1);
+         apic = (void *)(madt + 1);
+-        for(i=0;i<smp_cpus;i++) {
++        for(i = 0;i < max_cpus; i++) {
+             apic->type = APIC_PROCESSOR;
+             apic->length = sizeof(*apic);
+             apic->processor_id = i;
+@@ -1822,7 +1822,7 @@ void acpi_bios_init(void)
+         io_apic = (void *)apic;
+         io_apic->type = APIC_IO;
+         io_apic->length = sizeof(*io_apic);
+-        io_apic->io_apic_id = smp_cpus;
++        io_apic->io_apic_id = max_cpus;
+         io_apic->address = cpu_to_le32(0xfec00000);
+         io_apic->interrupt = cpu_to_le32(0);
+ #ifdef BX_QEMU
+@@ -1856,7 +1856,7 @@ void acpi_bios_init(void)
+         srat->reserved1=1;
+  
+         core = (void*)(srat + 1);
+-        for (i = 0; i < smp_cpus; ++i) {
++        for (i = 0; i < max_cpus; ++i) {
+              core->type = SRAT_PROCESSOR;
+              core->length = sizeof(*core);
+              core->local_apic_id = i;
+@@ -1864,7 +1864,7 @@ void acpi_bios_init(void)
+              core->proximity_lo = curnode;
+              memset (core->proximity_hi, 0, 3);
+              core->local_sapic_eid = 0;
+-             if (i < smp_cpus)
++             if (i < max_cpus)
+                  core->flags = cpu_to_le32(1);
+              else
+                  core->flags = 0;
+@@ -2614,7 +2614,7 @@ void smbios_init(void)
+     add_struct(0, p);
+     add_struct(1, p);
+     add_struct(3, p);
+-    for (cpu_num = 1; cpu_num <= smp_cpus; cpu_num++)
++    for (cpu_num = 1; cpu_num <= max_cpus; cpu_num++)
+         add_struct(4, p, cpu_num);
+ 
+     /* Each 'memory device' covers up to 16GB of address space. */
Index: qemu/pc-bios/bios-pq/series
===================================================================
--- qemu.orig/pc-bios/bios-pq/series
+++ qemu/pc-bios/bios-pq/series
@@ -14,3 +14,5 @@
 0014_add-srat-acpi-table-support.patch
 0015_enable-power-button-even-generation.patch
 0016-use-correct-mask-to-size-pci-option-rom-bar.patch
+0017-qemu-kvm-cfg-maxcpus.patch
+0018-qemu-madt-maxcpus.patch

Re: [Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Anthony Liguori]

These changes make my Ubuntu server guest very unhappy.  I get a bunch 
of messages about "Not responding." on startup.

If nothing else, maxcpus ==smp_cpus under QEMU because we don't do CPU 
hotplug (and I don't think we should).

Regards,

Anthony Liguori

Jes Sorensen wrote:

Re: [Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Avi Kivity]

On 07/10/2009 12:57 AM, Anthony Liguori wrote:
> These changes make my Ubuntu server guest very unhappy.  I get a bunch 
> of messages about "Not responding." on startup.
>
> If nothing else, maxcpus ==smp_cpus under QEMU because we don't do CPU 
> hotplug (and I don't think we should).
>

Why shouldn't we do cpu hotplug?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Anthony Liguori]

Avi Kivity wrote:
> On 07/10/2009 12:57 AM, Anthony Liguori wrote:
>> These changes make my Ubuntu server guest very unhappy.  I get a 
>> bunch of messages about "Not responding." on startup.
>>
>> If nothing else, maxcpus ==smp_cpus under QEMU because we don't do 
>> CPU hotplug (and I don't think we should).
>>
>
> Why shouldn't we do cpu hotplug?

I don't think we should do cpu hotplug via ACPI.  I don't think ACPI 
actually models CPU hotplug and the fact that this works with Linux in 
KVM is a happy accident.  VMware only supports CPU hotplug for Windows 
7/2k8 guests so I'm assuming their using Viridian PV extensions to do it.

I think we should go the PV route for Linux too.  I'd rather see us 
create all vcpu threads at once and then let the guest offline each vcpu 
via a PV notification.  I don't see a lot of value in 
spawning/terminating vcpu threads dynamically and it adds an awful lot 
of complexity.  There's very little overhead to an idle thread.

Regards,

Anthony Liguori

Re: [Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Avi Kivity]

On 07/12/2009 04:23 PM, Anthony Liguori wrote:
>>> If nothing else, maxcpus ==smp_cpus under QEMU because we don't do 
>>> CPU hotplug (and I don't think we should).
>>>
>>
>> Why shouldn't we do cpu hotplug?
>
>
> I don't think we should do cpu hotplug via ACPI. 

Well, that's a different story from "we shouldn't do cou hotplug".

> I don't think ACPI actually models CPU hotplug and the fact that this 
> works with Linux in KVM is a happy accident. 

I think it's based on the Unisys machines and thus no accident.

> VMware only supports CPU hotplug for Windows 7/2k8 guests so I'm 
> assuming their using Viridian PV extensions to do it.

I don't recall seeing hotplug support in Viridian.  Further, Windows 
2008 appears to support cpu hotplug on bare metal.  My guess is that it 
uses ACPI, perhaps with an additional vendor driver.

> I think we should go the PV route for Linux too. 

Seems like needless churn, plus disabling that functionality for older 
kernels.

> I'd rather see us create all vcpu threads at once and then let the 
> guest offline each vcpu via a PV notification. 

That doesn't work, for example, when the guest reboots into an older 
version of itself.  It also gives control of resource usage to the guest 
instead of the host.  The latter issue can be fixed using control 
groups, but I'd rather not break it in the first place.

> I don't see a lot of value in spawning/terminating vcpu threads 
> dynamically and it adds an awful lot of complexity.  There's very 
> little overhead to an idle thread.

Can you elaborate on the complexity you think dynamic vcpu threads add?  
IIRC there's some synchronization to be done at startup, but nothing 
that merits the label "awful".

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Jes Sorensen]

On 07/09/2009 11:57 PM, Anthony Liguori wrote:
> These changes make my Ubuntu server guest very unhappy.  I get a bunch
> of messages about "Not responding." on startup.
>
> If nothing else, maxcpus ==smp_cpus under QEMU because we don't do CPU
> hotplug (and I don't think we should).

Anthony,

Sorry haven't gotten back to you earlier as I was on vacation. Are you
saying the Ubuntu kernel doesn't like having more CPU entries in the
ACPI table than it actually boots on?

Does the same guest boot using an older KVM setup? Curious since it does
have the larger CPU table in the DSDT.

Cheers,
Jes

Re: [Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Filip Navara]

On Tue, Jul 14, 2009 at 10:38 AM, Jes Sorensen<jes@sgi.com> wrote:
> On 07/09/2009 11:57 PM, Anthony Liguori wrote:
>>
>> These changes make my Ubuntu server guest very unhappy.  I get a bunch
>> of messages about "Not responding." on startup.
>>
>> If nothing else, maxcpus ==smp_cpus under QEMU because we don't do CPU
>> hotplug (and I don't think we should).
>
> Anthony,
>
> Sorry haven't gotten back to you earlier as I was on vacation. Are you
> saying the Ubuntu kernel doesn't like having more CPU entries in the
> ACPI table than it actually boots on?
>
> Does the same guest boot using an older KVM setup? Curious since it does
> have the larger CPU table in the DSDT.
>
> Cheers,
> Jes
>

BTW, many other guests complain when ACPI describes more processors
than actually present in machine. That's why I implemented the dynamic
DSDT generation in Bochs BIOS in the first place. One that comes to
mind is MacOS X, or the Darwin kernel respectively.

Best regards,
Filip Navara

Re: [Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Gleb Natapov]

On Tue, Jul 14, 2009 at 11:21:53AM +0200, Filip Navara wrote:
> On Tue, Jul 14, 2009 at 10:38 AM, Jes Sorensen<jes@sgi.com> wrote:
> > On 07/09/2009 11:57 PM, Anthony Liguori wrote:
> >>
> >> These changes make my Ubuntu server guest very unhappy.  I get a bunch
> >> of messages about "Not responding." on startup.
> >>
> >> If nothing else, maxcpus ==smp_cpus under QEMU because we don't do CPU
> >> hotplug (and I don't think we should).
> >
> > Anthony,
> >
> > Sorry haven't gotten back to you earlier as I was on vacation. Are you
> > saying the Ubuntu kernel doesn't like having more CPU entries in the
> > ACPI table than it actually boots on?
> >
> > Does the same guest boot using an older KVM setup? Curious since it does
> > have the larger CPU table in the DSDT.
> >
> > Cheers,
> > Jes
> >
> 
> BTW, many other guests complain when ACPI describes more processors
> than actually present in machine. That's why I implemented the dynamic
> DSDT generation in Bochs BIOS in the first place. One that comes to
> mind is MacOS X, or the Darwin kernel respectively.
> 
There is nothing wrong in describing more processors than actually
present. The disable flag is defined by ACPI for a reason. My real HW
IBM server does this.

--
			Gleb.

Re: [Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Alexander Graf]

On 14.07.2009, at 11:32, Gleb Natapov wrote:

> On Tue, Jul 14, 2009 at 11:21:53AM +0200, Filip Navara wrote:
>> On Tue, Jul 14, 2009 at 10:38 AM, Jes Sorensen<jes@sgi.com> wrote:
>>> On 07/09/2009 11:57 PM, Anthony Liguori wrote:
>>>>
>>>> These changes make my Ubuntu server guest very unhappy.  I get a  
>>>> bunch
>>>> of messages about "Not responding." on startup.
>>>>
>>>> If nothing else, maxcpus ==smp_cpus under QEMU because we don't  
>>>> do CPU
>>>> hotplug (and I don't think we should).
>>>
>>> Anthony,
>>>
>>> Sorry haven't gotten back to you earlier as I was on vacation. Are  
>>> you
>>> saying the Ubuntu kernel doesn't like having more CPU entries in the
>>> ACPI table than it actually boots on?
>>>
>>> Does the same guest boot using an older KVM setup? Curious since  
>>> it does
>>> have the larger CPU table in the DSDT.
>>>
>>> Cheers,
>>> Jes
>>>
>>
>> BTW, many other guests complain when ACPI describes more processors
>> than actually present in machine. That's why I implemented the  
>> dynamic
>> DSDT generation in Bochs BIOS in the first place. One that comes to
>> mind is MacOS X, or the Darwin kernel respectively.
>>
> There is nothing wrong in describing more processors than actually
> present. The disable flag is defined by ACPI for a reason. My real HW
> IBM server does this.

Yeah, last time I tried MacOS X was happy with more CPU descriptions  
than actual CPUs too as long as they were in disabled state. Has  
anything changed there I should know about?

Alex

Re: [Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Filip Navara]

On Tue, Jul 14, 2009 at 12:16 PM, Alexander Graf<agraf@suse.de> wrote:
>
> On 14.07.2009, at 11:32, Gleb Natapov wrote:
>
>> On Tue, Jul 14, 2009 at 11:21:53AM +0200, Filip Navara wrote:
>>>
>>> On Tue, Jul 14, 2009 at 10:38 AM, Jes Sorensen<jes@sgi.com> wrote:
>>>>
>>>> On 07/09/2009 11:57 PM, Anthony Liguori wrote:
>>>>>
>>>>> These changes make my Ubuntu server guest very unhappy.  I get a bunch
>>>>> of messages about "Not responding." on startup.
>>>>>
>>>>> If nothing else, maxcpus ==smp_cpus under QEMU because we don't do CPU
>>>>> hotplug (and I don't think we should).
>>>>
>>>> Anthony,
>>>>
>>>> Sorry haven't gotten back to you earlier as I was on vacation. Are you
>>>> saying the Ubuntu kernel doesn't like having more CPU entries in the
>>>> ACPI table than it actually boots on?
>>>>
>>>> Does the same guest boot using an older KVM setup? Curious since it does
>>>> have the larger CPU table in the DSDT.
>>>>
>>>> Cheers,
>>>> Jes
>>>>
>>>
>>> BTW, many other guests complain when ACPI describes more processors
>>> than actually present in machine. That's why I implemented the dynamic
>>> DSDT generation in Bochs BIOS in the first place. One that comes to
>>> mind is MacOS X, or the Darwin kernel respectively.
>>>
>> There is nothing wrong in describing more processors than actually
>> present. The disable flag is defined by ACPI for a reason. My real HW
>> IBM server does this.
>
> Yeah, last time I tried MacOS X was happy with more CPU descriptions than
> actual CPUs too as long as they were in disabled state. Has anything changed
> there I should know about?
>
> Alex

Not really... it just spits warnings that are not seen unless you
explicitly ask for complete debug/io logs (not sure which one it is
in). In any case the disabled flag was not set in QEMU when I was
debugging the code.

Best regards,
Filip Navara

Re: [Qemu-devel] [patch 2/2] QEMU BOCHS bios patches to use maxcpus value.

[Jes Sorensen]

On 07/14/2009 11:21 AM, Filip Navara wrote:
> BTW, many other guests complain when ACPI describes more processors
> than actually present in machine. That's why I implemented the dynamic
> DSDT generation in Bochs BIOS in the first place. One that comes to
> mind is MacOS X, or the Darwin kernel respectively.

Hi Filip,

As Gleb mentioned, this is pretty common behavior on real hardware. Most
of the boards I have here, declare the maximum number of possible CPUs,
even when they are not all active (like if you disable hyper threading).

I like the idea of dynamic declerations, but it's problematic as the
ACPI code gets more complicated.

I'd like to try and find out what broke in Anthony's case.

Cheers,
Jes

[Qemu-devel] [PATCH 0/2] QEMU maxcpus support v2

[Jes Sorensen]

Hi,

Another update to the maxcpus patch. Gleb pointed out that I was
enabling all processors in the MADT, which clearly wasn't my
intention. My brain had obviously already left for vacation when
I wrote it.

Anthony, any chance you could try this and see if it fixes your
Ubuntu guest problem?

Specifying maxcpus= allows the user to cap the limit of possibly cpus
of the system, as opposed to -smp which specifies the number of
running cpus at startup. This allows us to build BIOS tables
accordingly when trying to scale up in size, and it also allows for
the BIOS to build smaller tables when not needed.

Cheers,
Jes

[Qemu-devel] [PATCH 1/2] Introduce -smp , maxcpus= flag to specify maximum number of CPUS.

[Jes Sorensen]

[-- Attachment #1: 0001-qemu-cfg-maxcpus.patch --]
[-- Type: text/plain, Size: 3781 bytes --]

Follow on patch will use it to determine the size of the MADT and
other BIOS tables.

Signed-off-by: Jes Sorensen <jes@sgi.com>

---
 hw/fw_cfg.c     |    1 +
 hw/fw_cfg.h     |    1 +
 qemu-options.hx |    5 ++++-
 sysemu.h        |    1 +
 vl.c            |   20 +++++++++++++++++++-
 5 files changed, 26 insertions(+), 2 deletions(-)

Index: qemu/hw/fw_cfg.c
===================================================================
--- qemu.orig/hw/fw_cfg.c
+++ qemu/hw/fw_cfg.c
@@ -279,6 +279,7 @@ void *fw_cfg_init(uint32_t ctl_port, uin
     fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
     fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
     fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
+    fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
 
     register_savevm("fw_cfg", -1, 1, fw_cfg_save, fw_cfg_load, s);
     qemu_register_reset(fw_cfg_reset, 0, s);
Index: qemu/hw/fw_cfg.h
===================================================================
--- qemu.orig/hw/fw_cfg.h
+++ qemu/hw/fw_cfg.h
@@ -15,6 +15,7 @@
 #define FW_CFG_INITRD_SIZE      0x0b
 #define FW_CFG_BOOT_DEVICE      0x0c
 #define FW_CFG_NUMA             0x0d
+#define FW_CFG_MAX_CPUS         0x0e
 #define FW_CFG_MAX_ENTRY        0x10
 
 #define FW_CFG_WRITE_CHANNEL    0x4000
Index: qemu/qemu-options.hx
===================================================================
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -39,7 +39,10 @@ Select CPU model (-cpu ? for list and ad
 ETEXI
 
 DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-    "-smp n          set the number of CPUs to 'n' [default=1]\n")
+    "-smp n[,maxcpus=cpus]\n"
+    "                set the number of CPUs to 'n' [default=1]\n"
+    "                maxcpus= maximum number of total cpus, including\n"
+    "                offline CPUs for hotplug etc.\n")
 STEXI
 @item -smp @var{n}
 Simulate an SMP system with @var{n} CPUs. On the PC target, up to 255
Index: qemu/sysemu.h
===================================================================
--- qemu.orig/sysemu.h
+++ qemu/sysemu.h
@@ -118,6 +118,7 @@ extern int alt_grab;
 extern int usb_enabled;
 extern int no_virtio_balloon;
 extern int smp_cpus;
+extern int max_cpus;
 extern int cursor_hide;
 extern int graphic_rotate;
 extern int no_quit;
Index: qemu/vl.c
===================================================================
--- qemu.orig/vl.c
+++ qemu/vl.c
@@ -241,6 +241,7 @@ int rtc_td_hack = 0;
 int usb_enabled = 0;
 int singlestep = 0;
 int smp_cpus = 1;
+int max_cpus = 16;
 const char *vnc_display;
 int acpi_enabled = 1;
 int no_hpet = 0;
@@ -5550,12 +5551,29 @@ int main(int argc, char **argv, char **e
                 usb_devices_index++;
                 break;
             case QEMU_OPTION_smp:
-                smp_cpus = atoi(optarg);
+            {
+                char *p;
+                char option[128];
+                smp_cpus = strtol(optarg, &p, 10);
                 if (smp_cpus < 1) {
                     fprintf(stderr, "Invalid number of CPUs\n");
                     exit(1);
                 }
+                if (*p++ != ',')
+                    break;
+                if (get_param_value(option, 128, "maxcpus", p))
+                    max_cpus = strtol(option, NULL, 0);
+                if (max_cpus < smp_cpus) {
+                    fprintf(stderr, "maxcpus must be equal to or greater than "
+                            "smp\n");
+                    exit(1);
+                }
+                if (max_cpus > 255) {
+                    fprintf(stderr, "Unsupported number of maxcpus\n");
+                    exit(1);
+                }
                 break;
+            }
 	    case QEMU_OPTION_vnc:
                 display_type = DT_VNC;
 		vnc_display = optarg;

[Qemu-devel] [PATCH 0/2] QEMU maxcpus support v3

[Jes Sorensen]

Anthony,

Here's an updated version of the maxcpus patch, which goes on top of
the most recent QEMU. My older version was against a 2 week old
version and I see you have applied some boot menu stuff since then
which did clash with the FW_CFG magic number I used. I suspect that
could have caused the problems.

Just did a full fresh rebuild and this one works fine for me with the
Ubuntu test image.

Jes

[Qemu-devel] [PATCH 1/2] Introduce -smp , maxcpus= flag to specify maximum number of CPUS.

[Jes Sorensen]

[-- Attachment #1: 0001-qemu-cfg-maxcpus.patch --]
[-- Type: text/plain, Size: 4262 bytes --]

Follow on patch will use it to determine the size of the MADT and
other BIOS tables.

Signed-off-by: Jes Sorensen <jes@sgi.com>

---
 hw/fw_cfg.c     |    1 +
 hw/fw_cfg.h     |    1 +
 qemu-options.hx |    5 ++++-
 sysemu.h        |    1 +
 vl.c            |   27 ++++++++++++++++++++++++++-
 5 files changed, 33 insertions(+), 2 deletions(-)

Index: qemu/hw/fw_cfg.c
===================================================================
--- qemu.orig/hw/fw_cfg.c
+++ qemu/hw/fw_cfg.c
@@ -279,6 +279,7 @@ void *fw_cfg_init(uint32_t ctl_port, uin
     fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
     fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
     fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
+    fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
     fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
 
     register_savevm("fw_cfg", -1, 1, fw_cfg_save, fw_cfg_load, s);
Index: qemu/hw/fw_cfg.h
===================================================================
--- qemu.orig/hw/fw_cfg.h
+++ qemu/hw/fw_cfg.h
@@ -16,6 +16,7 @@
 #define FW_CFG_BOOT_DEVICE      0x0c
 #define FW_CFG_NUMA             0x0d
 #define FW_CFG_BOOT_MENU        0x0e
+#define FW_CFG_MAX_CPUS         0x0f
 #define FW_CFG_MAX_ENTRY        0x10
 
 #define FW_CFG_WRITE_CHANNEL    0x4000
Index: qemu/qemu-options.hx
===================================================================
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -39,7 +39,10 @@ Select CPU model (-cpu ? for list and ad
 ETEXI
 
 DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-    "-smp n          set the number of CPUs to 'n' [default=1]\n")
+    "-smp n[,maxcpus=cpus]\n"
+    "                set the number of CPUs to 'n' [default=1]\n"
+    "                maxcpus= maximum number of total cpus, including\n"
+    "                offline CPUs for hotplug etc.\n")
 STEXI
 @item -smp @var{n}
 Simulate an SMP system with @var{n} CPUs. On the PC target, up to 255
Index: qemu/sysemu.h
===================================================================
--- qemu.orig/sysemu.h
+++ qemu/sysemu.h
@@ -119,6 +119,7 @@ extern int usb_enabled;
 extern int virtio_balloon;
 extern const char *virtio_balloon_devaddr;
 extern int smp_cpus;
+extern int max_cpus;
 extern int cursor_hide;
 extern int graphic_rotate;
 extern int no_quit;
Index: qemu/vl.c
===================================================================
--- qemu.orig/vl.c
+++ qemu/vl.c
@@ -227,6 +227,7 @@ int rtc_td_hack = 0;
 int usb_enabled = 0;
 int singlestep = 0;
 int smp_cpus = 1;
+int max_cpus = 0;
 const char *vnc_display;
 int acpi_enabled = 1;
 int no_hpet = 0;
@@ -5458,12 +5459,29 @@ int main(int argc, char **argv, char **e
                 usb_devices_index++;
                 break;
             case QEMU_OPTION_smp:
-                smp_cpus = atoi(optarg);
+            {
+                char *p;
+                char option[128];
+                smp_cpus = strtol(optarg, &p, 10);
                 if (smp_cpus < 1) {
                     fprintf(stderr, "Invalid number of CPUs\n");
                     exit(1);
                 }
+                if (*p++ != ',')
+                    break;
+                if (get_param_value(option, 128, "maxcpus", p))
+                    max_cpus = strtol(option, NULL, 0);
+                if (max_cpus < smp_cpus) {
+                    fprintf(stderr, "maxcpus must be equal to or greater than "
+                            "smp\n");
+                    exit(1);
+                }
+                if (max_cpus > 255) {
+                    fprintf(stderr, "Unsupported number of maxcpus\n");
+                    exit(1);
+                }
                 break;
+            }
 	    case QEMU_OPTION_vnc:
                 display_type = DT_VNC;
 		vnc_display = optarg;
@@ -5644,6 +5662,13 @@ int main(int argc, char **argv, char **e
     }
 #endif
 
+    /*
+     * Default to max_cpus = smp_cpus, in case the user doesn't
+     * specify a max_cpus value.
+     */
+    if (!max_cpus)
+        max_cpus = smp_cpus;
+
     machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
     if (smp_cpus > machine->max_cpus) {
         fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "

[Qemu-devel] [PATCH 0/2] QEMU maxcpus support v4

[Jes Sorensen]

Hi,

Ok so this time I think i nailed the problem. Using Anthony's tree I
was able to reproduce the failure. Why it worked with my testing
previously I have no idea :-(

The problem was that the local processor APICs were not marked
disabled for the processors not being active. I also changed the code
to mark unused processors as disabled in the MP table, though that
seemed to have no impact on this particular failure.

With this version I can boot Ubuntu all the way to X using Anthony's
tree.

Sorry for the noise, I am still mystified :-(

Cheers,
Jes

[Qemu-devel] [PATCH 1/2] Introduce -smp , maxcpus= flag to specify maximum number of CPUS.

[Jes Sorensen]

[-- Attachment #1: 0001-qemu-cfg-maxcpus.patch --]
[-- Type: text/plain, Size: 4262 bytes --]

Follow on patch will use it to determine the size of the MADT and
other BIOS tables.

Signed-off-by: Jes Sorensen <jes@sgi.com>

---
 hw/fw_cfg.c     |    1 +
 hw/fw_cfg.h     |    1 +
 qemu-options.hx |    5 ++++-
 sysemu.h        |    1 +
 vl.c            |   27 ++++++++++++++++++++++++++-
 5 files changed, 33 insertions(+), 2 deletions(-)

Index: qemu/hw/fw_cfg.c
===================================================================
--- qemu.orig/hw/fw_cfg.c
+++ qemu/hw/fw_cfg.c
@@ -279,6 +279,7 @@ void *fw_cfg_init(uint32_t ctl_port, uin
     fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
     fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
     fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
+    fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
     fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
 
     register_savevm("fw_cfg", -1, 1, fw_cfg_save, fw_cfg_load, s);
Index: qemu/hw/fw_cfg.h
===================================================================
--- qemu.orig/hw/fw_cfg.h
+++ qemu/hw/fw_cfg.h
@@ -16,6 +16,7 @@
 #define FW_CFG_BOOT_DEVICE      0x0c
 #define FW_CFG_NUMA             0x0d
 #define FW_CFG_BOOT_MENU        0x0e
+#define FW_CFG_MAX_CPUS         0x0f
 #define FW_CFG_MAX_ENTRY        0x10
 
 #define FW_CFG_WRITE_CHANNEL    0x4000
Index: qemu/qemu-options.hx
===================================================================
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -39,7 +39,10 @@ Select CPU model (-cpu ? for list and ad
 ETEXI
 
 DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-    "-smp n          set the number of CPUs to 'n' [default=1]\n")
+    "-smp n[,maxcpus=cpus]\n"
+    "                set the number of CPUs to 'n' [default=1]\n"
+    "                maxcpus= maximum number of total cpus, including\n"
+    "                offline CPUs for hotplug etc.\n")
 STEXI
 @item -smp @var{n}
 Simulate an SMP system with @var{n} CPUs. On the PC target, up to 255
Index: qemu/sysemu.h
===================================================================
--- qemu.orig/sysemu.h
+++ qemu/sysemu.h
@@ -119,6 +119,7 @@ extern int usb_enabled;
 extern int virtio_balloon;
 extern const char *virtio_balloon_devaddr;
 extern int smp_cpus;
+extern int max_cpus;
 extern int cursor_hide;
 extern int graphic_rotate;
 extern int no_quit;
Index: qemu/vl.c
===================================================================
--- qemu.orig/vl.c
+++ qemu/vl.c
@@ -227,6 +227,7 @@ int rtc_td_hack = 0;
 int usb_enabled = 0;
 int singlestep = 0;
 int smp_cpus = 1;
+int max_cpus = 0;
 const char *vnc_display;
 int acpi_enabled = 1;
 int no_hpet = 0;
@@ -5458,12 +5459,29 @@ int main(int argc, char **argv, char **e
                 usb_devices_index++;
                 break;
             case QEMU_OPTION_smp:
-                smp_cpus = atoi(optarg);
+            {
+                char *p;
+                char option[128];
+                smp_cpus = strtol(optarg, &p, 10);
                 if (smp_cpus < 1) {
                     fprintf(stderr, "Invalid number of CPUs\n");
                     exit(1);
                 }
+                if (*p++ != ',')
+                    break;
+                if (get_param_value(option, 128, "maxcpus", p))
+                    max_cpus = strtol(option, NULL, 0);
+                if (max_cpus < smp_cpus) {
+                    fprintf(stderr, "maxcpus must be equal to or greater than "
+                            "smp\n");
+                    exit(1);
+                }
+                if (max_cpus > 255) {
+                    fprintf(stderr, "Unsupported number of maxcpus\n");
+                    exit(1);
+                }
                 break;
+            }
 	    case QEMU_OPTION_vnc:
                 display_type = DT_VNC;
 		vnc_display = optarg;
@@ -5644,6 +5662,13 @@ int main(int argc, char **argv, char **e
     }
 #endif
 
+    /*
+     * Default to max_cpus = smp_cpus, in case the user doesn't
+     * specify a max_cpus value.
+     */
+    if (!max_cpus)
+        max_cpus = smp_cpus;
+
     machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
     if (smp_cpus > machine->max_cpus) {
         fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "