public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Gleb Natapov <gleb@redhat.com>
Cc: Vasilis Liaskovitis <vliaskov@gmail.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH] cpu hotplug issue
Date: Mon, 25 Jul 2011 15:26:17 +0200	[thread overview]
Message-ID: <4E2D6EF9.8000609@siemens.com> (raw)
In-Reply-To: <20110725132141.GE4404@redhat.com>

On 2011-07-25 15:21, Gleb Natapov wrote:
> On Mon, Jul 25, 2011 at 03:18:19PM +0200, Jan Kiszka wrote:
>> On 2011-07-24 18:11, Jan Kiszka wrote:
>>>>> I had a closer look and identified two further issues, one generic, one
>>>>> CPU-hotplug-specific:
>>>>>  - (qdev) devices that are hotplugged do not receive any reset. That
>>>>>    does not only apply to the APIC in case of CPU hotplugging, it is
>>>>>    also broken for NICs, storage controllers, etc. when doing PCI
>>>>>    hot-add as I just checked via gdb.
>>>>>  - CPU hotplugging was always (or at least for a fairly long time),
>>>>>    well, fragile as it failed to make CPU thread creation and CPU
>>>>>    initialization atomic against APIC addition and other initialization
>>>>>    steps. IOW, we need to create CPUs stopped, finish all init work,
>>>>>    sync their states completely to the kernel
>>>>>    (cpu_synchronize_post_init), and then kick them of. Actually I'm
>>>> Syncing the state to the kernel should be done by vcpu thread, so I it
>>>> cannot be stopped while the sync is done. May be I misunderstood what
>>>> you mean here.
>>>
>>> Stopped first of all means not entering kvm_cpu_exec before the whole
>>> setup and the initial sync are done.
>>>
>>> Syncing the initial state may also happen over the creating context as
>>> long as the vcpus are stopped (analogously to
>>> kvm_cpu_synchronize_post_init).
>>
>> OK, hacks below plus the following three patches make CPU hotplug work
>> again - with some exceptions. Here are the patches:
>>
>> 1. http://thread.gmane.org/gmane.comp.emulators.kvm.devel/76484
>> 2. http://thread.gmane.org/gmane.comp.emulators.qemu/110272
>> 3. http://thread.gmane.org/gmane.comp.emulators.qemu/110426
>>
>> And here are the hacks (well, the first hunk is clearly a fix, the last
>> one clearly a hack, /me still undecided about the rest):
>>
>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>> index c30a050..f650250 100644
>> --- a/hw/acpi_piix4.c
>> +++ b/hw/acpi_piix4.c
>> @@ -92,7 +92,8 @@ static void pm_update_sci(PIIX4PMState *s)
>>                     ACPI_BITMASK_POWER_BUTTON_ENABLE |
>>                     ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
>>                     ACPI_BITMASK_TIMER_ENABLE)) != 0) ||
>> -        (((s->gpe.sts[0] & s->gpe.en[0]) & PIIX4_PCI_HOTPLUG_STATUS) != 0);
>> +        (((s->gpe.sts[0] & s->gpe.en[0]) &
>> +          (PIIX4_PCI_HOTPLUG_STATUS | PIIX4_CPU_HOTPLUG_STATUS)) != 0);
>>  
>>      qemu_set_irq(s->irq, sci_level);
>>      /* schedule a timer interruption if needed */
>> diff --git a/hw/pc.c b/hw/pc.c
>> index c0a88e1..e5371be 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -42,6 +42,7 @@
>>  #include "kvm.h"
>>  #include "blockdev.h"
>>  #include "ui/qemu-spice.h"
>> +#include "cpus.h"
>>  
>>  /* output Bochs bios info messages */
>>  //#define DEBUG_BIOS
>> @@ -936,6 +937,10 @@ CPUState *pc_new_cpu(const char *cpu_model)
>>  #endif
>>      }
>>  
>> +    if (vm_running) {
>> +        pause_all_vcpus();
>> +    }
>> +
>>      env = cpu_init(cpu_model);
>>      if (!env) {
>>          fprintf(stderr, "Unable to find x86 CPU definition\n");
>> @@ -947,6 +952,11 @@ CPUState *pc_new_cpu(const char *cpu_model)
>>      }
>>      qemu_register_reset(pc_cpu_reset, env);
>>      pc_cpu_reset(env);
>> +
>> +    cpu_synchronize_post_init(env);
>> +    if (vm_running) {
>> +        resume_all_vcpus();
>> +    }
>>      return env;
>>  }
>>  
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index 1626131..b91e2c2 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -330,6 +330,7 @@ BusState *sysbus_get_default(void)
>>      if (!main_system_bus) {
>>          main_system_bus = qbus_create(&system_bus_info, NULL,
>>                                        "main-system-bus");
>> +        main_system_bus->allow_hotplug = 1;
>>      }
>>      return main_system_bus;
>>  }
>>
>> I see two remaining problems:
>>  - kvmclock is somehow broken, either in my guest kernel (OpenSUSE HEAD
>>    3.0.0-2) or the host, -cpu host,-kvmclock works around sporadic
>>    guest lockups on echo 1 > /sys...
>>  - Seabios tends to lock up once every few system_reset after some
>>    CPU has been hot-added - also in TCG mode. It seems to dislike any
>>    setup of #CPUs > smp_cpus (whatever that implies in details).
>>
> Have you specified maxcpus? Something like -smp 1,maxcpus=4.

Yes, for sure.

BTW, cpu_set completely lacks any parameter sanity checks. That
interface looks like "for gurus only". Hope we can do better via qdev
and properties.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

  reply	other threads:[~2011-07-25 13:26 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-19 17:40 cpu hotplug issue Vasilis Liaskovitis
2011-07-20  8:35 ` Gleb Natapov
2011-07-21 11:06   ` [PATCH] " Vasilis Liaskovitis
2011-07-21 11:33     ` Gleb Natapov
2011-07-21 11:42       ` Jan Kiszka
2011-07-21 11:51         ` Gleb Natapov
2011-07-21 11:55           ` Jan Kiszka
2011-07-21 12:00             ` Gleb Natapov
2011-07-21 12:18             ` Avi Kivity
2011-07-21 12:22               ` Gleb Natapov
2011-07-21 12:39               ` Jan Kiszka
2011-07-21 13:27               ` Lucas Meneghel Rodrigues
2011-07-21 12:45           ` Gleb Natapov
2011-07-22 10:56             ` Jan Kiszka
2011-07-24 11:56               ` Gleb Natapov
2011-07-24 16:11                 ` Jan Kiszka
2011-07-25 13:18                   ` Jan Kiszka
2011-07-25 13:21                     ` Gleb Natapov
2011-07-25 13:26                       ` Jan Kiszka [this message]
2011-07-27 16:35                     ` Vasilis Liaskovitis
2011-07-28 16:52                       ` Jan Kiszka
2011-08-02  9:46                         ` Vasilis Liaskovitis
2011-08-02 10:24                           ` Jan Kiszka
2011-08-02 13:41                             ` Vasilis Liaskovitis
2011-08-03 10:07                               ` Vasilis Liaskovitis
2011-08-03 10:37                                 ` Jan Kiszka
2011-08-03 10:38                                   ` Gleb Natapov
2011-08-03 10:42                                     ` Jan Kiszka
2011-08-03 16:25                                       ` Vasilis Liaskovitis
2011-08-04  8:01                                         ` Gleb Natapov
2011-08-04  8:40                                           ` Jan Kiszka
2011-07-21 13:08       ` Vasilis Liaskovitis
2011-07-21 13:11         ` Gleb Natapov
2011-07-21 13:12           ` Vasilis Liaskovitis
2011-07-21 13:13             ` Gleb Natapov
2011-07-21 13:15         ` Avi Kivity
2011-07-21 13:15           ` Avi Kivity
2011-07-21 11:36     ` Jan Kiszka
2011-07-21 12:22     ` Jan Kiszka
2011-07-21 12:25       ` Gleb Natapov
2011-07-21 12:35         ` Jan Kiszka
2011-07-21 12:40           ` Gleb Natapov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E2D6EF9.8000609@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=armbru@redhat.com \
    --cc=gleb@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=vliaskov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox