[Qemu-devel] pause_all_vcpus() TCG bug?

[Qemu-devel] pause_all_vcpus() TCG bug?

[Andreas Färber]

Hello,

This is today's function, with annotations and question inline:

void pause_all_vcpus(void)
{
    CPUArchState *penv = first_cpu;

    qemu_clock_enable(vm_clock, false);
    while (penv) {
        CPUState *pcpu = ENV_GET_CPU(penv);
        pcpu->stop = true;
        qemu_cpu_kick(pcpu);
        penv = penv->next_cpu;
    }

/* So, at this point penv == NULL. */

    if (qemu_in_vcpu_thread()) {
        cpu_stop_current();
        if (!kvm_enabled()) {
            while (penv) {

/* Looks like this can never be true then? */
/* Is penv = first_cpu; missing? */

                CPUState *pcpu = ENV_GET_CPU(penv);
                pcpu->stop = 0;

/* 0 instead of false may hint at a mismerge... */

                pcpu->stopped = true;
                penv = penv->next_cpu;
            }
            return;
        }
    }

    while (!all_vcpus_paused()) {
        qemu_cond_wait(&qemu_pause_cond, &qemu_global_mutex);
        penv = first_cpu;
        while (penv) {
            qemu_cpu_kick(ENV_GET_CPU(penv));
            penv = penv->next_cpu;
        }
    }
}

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] pause_all_vcpus() TCG bug?

[Peter Maydell]

On 1 May 2013 14:33, Andreas Färber <afaerber@suse.de> wrote:
> Hello,
>
> This is today's function, with annotations and question inline:
>
> void pause_all_vcpus(void)
> {
>     CPUArchState *penv = first_cpu;
>
>     qemu_clock_enable(vm_clock, false);
>     while (penv) {
>         CPUState *pcpu = ENV_GET_CPU(penv);
>         pcpu->stop = true;
>         qemu_cpu_kick(pcpu);
>         penv = penv->next_cpu;
>     }
>
> /* So, at this point penv == NULL. */
>
>     if (qemu_in_vcpu_thread()) {
>         cpu_stop_current();
>         if (!kvm_enabled()) {
>             while (penv) {
>
> /* Looks like this can never be true then? */
> /* Is penv = first_cpu; missing? */
>
>                 CPUState *pcpu = ENV_GET_CPU(penv);
>                 pcpu->stop = 0;
>
> /* 0 instead of false may hint at a mismerge... */
>
>                 pcpu->stopped = true;
>                 penv = penv->next_cpu;
>             }
>             return;
>         }
>     }

This certainly looks odd. This bug seems to have
been present since this patch was first committed
(d798e9745, January last year, by Jan.)

-- PMM

Re: [Qemu-devel] pause_all_vcpus() TCG bug?

[Jan Kiszka]

On 2013-05-01 15:47, Peter Maydell wrote:
> On 1 May 2013 14:33, Andreas Färber <afaerber@suse.de> wrote:
>> Hello,
>>
>> This is today's function, with annotations and question inline:
>>
>> void pause_all_vcpus(void)
>> {
>>     CPUArchState *penv = first_cpu;
>>
>>     qemu_clock_enable(vm_clock, false);
>>     while (penv) {
>>         CPUState *pcpu = ENV_GET_CPU(penv);
>>         pcpu->stop = true;
>>         qemu_cpu_kick(pcpu);
>>         penv = penv->next_cpu;
>>     }
>>
>> /* So, at this point penv == NULL. */
>>
>>     if (qemu_in_vcpu_thread()) {
>>         cpu_stop_current();
>>         if (!kvm_enabled()) {
>>             while (penv) {
>>
>> /* Looks like this can never be true then? */
>> /* Is penv = first_cpu; missing? */

Yeah, obviously. It practically didn't matter so far for the use case of
pause/resume_all_vcpus over a vcpu itself (kvmvapic). Feel free to fix.

>>
>>                 CPUState *pcpu = ENV_GET_CPU(penv);
>>                 pcpu->stop = 0;
>>
>> /* 0 instead of false may hint at a mismerge... */

Nope, stop was uint32_t at that time.

Jan

>>
>>                 pcpu->stopped = true;
>>                 penv = penv->next_cpu;
>>             }
>>             return;
>>         }
>>     }
> 
> This certainly looks odd. This bug seems to have
> been present since this patch was first committed
> (d798e9745, January last year, by Jan.)
> 
> -- PMM
> 

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux