From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:58558)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1UXp8K-0005Kr-OG
	for qemu-devel@nongnu.org; Thu, 02 May 2013 04:44:58 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1UXp8J-0005oJ-7U
	for qemu-devel@nongnu.org; Thu, 02 May 2013 04:44:56 -0400
Received: from thoth.sbs.de ([192.35.17.2]:28938)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1UXp8I-0005nT-U8
	for qemu-devel@nongnu.org; Thu, 02 May 2013 04:44:55 -0400
Message-ID: <51822773.6070905@siemens.com>
Date: Thu, 02 May 2013 10:44:35 +0200
From: Jan Kiszka <jan.kiszka@siemens.com>
MIME-Version: 1.0
References: <51811995.1010409@suse.de>
	<CAFEAcA_NtSre2wA6hu+RGPLBq-HRreE9EcT-AyOGz_ddbspC6A@mail.gmail.com>
In-Reply-To: <CAFEAcA_NtSre2wA6hu+RGPLBq-HRreE9EcT-AyOGz_ddbspC6A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] pause_all_vcpus() TCG bug?
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= <afaerber@suse.de>, =?UTF-8?B?6Zmz6Z+L5Lu7?= <chenwj@iis.sinica.edu.tw>, qemu-devel <qemu-devel@nongnu.org>

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

Nope, stop was uint32_t at that time.

Jan

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

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