[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] return default values for apic probe functions.
From: |
Glauber Costa |
Subject: |
[Qemu-devel] Re: [PATCH] return default values for apic probe functions. |
Date: |
Fri, 17 Apr 2009 10:59:11 -0300 |
On Fri, Apr 17, 2009 at 10:53 AM, Marcelo Tosatti <address@hidden> wrote:
> Hi Glauber,
>
> On Fri, Apr 17, 2009 at 01:15:21AM -0400, Glauber Costa wrote:
>> As KVM cpus runs on threads, it is possible that
>> we call kvm_load_registers() from a cpu thread, while the
>> apic has not yet fully initialized. kvm_load_registers() is called
>> from ap_main_loop.
>>
>> This is not a problem when we're starting the whole machine together,
>> but is a problem for hotplug, since we don't have the protection
>> of the locks that protect machine initialization. Currently, some executions
>> of cpu hotplug on rainy sundays fail with a segfault.
>
> /* and wait for machine initialization */
> while (!qemu_system_ready)
> qemu_cond_wait(&qemu_system_cond);
> pthread_mutex_unlock(&qemu_mutex);
>
> Shouldnt this cover the cpu hotplug case too? Perhaps have:
>
> /* wait for machine initialization */
> while (!qemu_system_ready)
> qemu_cond_wait(&qemu_system_cond);
> /* wait for vcpu initialization */
> while (!env->initialized)
> qemu_cond_wait(&qemu_system_cond);
> pthread_mutex_unlock(&qemu_mutex);
>
> And then set env->initialized when the cpu is good to go.
>From my understanding, all this is only useful when the whole machine
is starting, since they are global locks that wait for a system wide condition.
This is not the case with cpu hotplug, since the box is already on.
>
> Because there could be other dependencies other than APIC
> initialization, for eg in pc_new_cpu
>
> if (cpu != 0)
> env->halted = 1;
it is okay for the cpu to be halted. Btw, I believe this should be
moved inside cpu init.