[Qemu-devel] Re: [PATCH] return default values for apic probe functions.

[Glauber Costa]

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.