From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36233) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFIEv-0005rr-Jj for qemu-devel@nongnu.org; Wed, 15 Jul 2015 04:40:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFIEo-0008Tb-LI for qemu-devel@nongnu.org; Wed, 15 Jul 2015 04:40:29 -0400 Received: from greensocs.com ([193.104.36.180]:34104) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFIEo-0008TB-9l for qemu-devel@nongnu.org; Wed, 15 Jul 2015 04:40:22 -0400 Message-ID: <55A61C6F.9080407@greensocs.com> Date: Wed, 15 Jul 2015 10:40:15 +0200 From: Frederic Konrad MIME-Version: 1.0 References: <1436544486-31169-1-git-send-email-fred.konrad@greensocs.com> <1436544486-31169-3-git-send-email-fred.konrad@greensocs.com> <87si8sjbk9.fsf@linaro.org> In-Reply-To: <87si8sjbk9.fsf@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH V2 2/3] cpus: add a tcg_executing flag. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QWxleCBCZW5uw6ll?= Cc: mttcg@listserver.greensocs.com, mark.burton@greensocs.com, qemu-devel@nongnu.org, a.rigo@virtualopensystems.com, guillaume.delbergue@greensocs.com, pbonzini@redhat.com On 13/07/2015 17:56, Alex Benn=C3=A9e wrote: > fred.konrad@greensocs.com writes: > >> From: KONRAD Frederic >> >> This flag indicates if the VCPU is currently executing TCG code. >> >> Signed-off-by: KONRAD Frederic >> >> Changes V1 -> V2: >> * do both tcg_executing =3D 0 or 1 in cpu_exec(). >> --- >> cpu-exec.c | 2 ++ >> include/qom/cpu.h | 3 +++ >> qom/cpu.c | 1 + >> 3 files changed, 6 insertions(+) >> >> diff --git a/cpu-exec.c b/cpu-exec.c >> index 75694f3..2fdf89d 100644 >> --- a/cpu-exec.c >> +++ b/cpu-exec.c >> @@ -371,6 +371,7 @@ int cpu_exec(CPUState *cpu) >> cpu->halted =3D 0; >> } >> =20 >> + cpu->tcg_executing =3D 1; >> current_cpu =3D cpu; >> =20 >> /* As long as current_cpu is null, up to the assignment just abo= ve, >> @@ -583,5 +584,6 @@ int cpu_exec(CPUState *cpu) >> =20 >> /* fail safe : never use current_cpu outside cpu_exec() */ >> current_cpu =3D NULL; >> + cpu->tcg_executing =3D 0; >> return ret; >> } >> diff --git a/include/qom/cpu.h b/include/qom/cpu.h >> index efa9624..a2de536 100644 >> --- a/include/qom/cpu.h >> +++ b/include/qom/cpu.h >> @@ -226,6 +226,7 @@ struct kvm_run; >> * @stopped: Indicates the CPU has been artificially stopped. >> * @tcg_exit_req: Set to force TCG to stop executing linked TBs for = this >> * CPU and return to its top level loop. >> + * @tcg_executing: This TCG thread is in cpu_exec(). >> * @singlestep_enabled: Flags for single-stepping. >> * @icount_extra: Instructions until next timer event. >> * @icount_decr: Number of cycles left, with interrupt flag in high = bit. >> @@ -322,6 +323,8 @@ struct CPUState { >> (absolute value) offset as small as possible. This reduces c= ode >> size, especially for hosts without large memory offsets. */ >> volatile sig_atomic_t tcg_exit_req; >> + >> + volatile int tcg_executing; > My concern is on weakly ordered backends is volatile enough for this > flag or do we need some sort of memory barrier when we update it? Does > it just introduce an inefficiency that other threads may spin a few > times waiting to find out the vCPU has halted? I think it will just spin (see in flush_queued_safe_work in patch 3). > > If other threads are waiting for it to halt is there a mechanism that > ensures we'll never start-up again until everything is done? This flag is not supposed to do that, it's in the third patch as well. It will check async_safe_work_pending before starting the execution. We might have a race here, if the flush is triggered between async_safe_work_pending and the tcg_executing flag set in cpu-exec. if (async_safe_work_pending()) { cpu->exit_request =3D 1; return 0; } if (cpu->halted) { if (!cpu_has_work(cpu)) { return EXCP_HALTED; } cpu->halted =3D 0; } cpu->tcg_executing =3D 1; I need to check and fix that. Fred > > >> }; >> =20 >> QTAILQ_HEAD(CPUTailQ, CPUState); >> diff --git a/qom/cpu.c b/qom/cpu.c >> index 4e12598..62663e5 100644 >> --- a/qom/cpu.c >> +++ b/qom/cpu.c >> @@ -249,6 +249,7 @@ static void cpu_common_reset(CPUState *cpu) >> cpu->icount_decr.u32 =3D 0; >> cpu->can_do_io =3D 0; >> cpu->exception_index =3D -1; >> + cpu->tcg_executing =3D 0; >> memset(cpu->tb_jmp_cache, 0, TB_JMP_CACHE_SIZE * sizeof(void *))= ; >> }