From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47012) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eFOeS-0005Yu-RR for qemu-devel@nongnu.org; Thu, 16 Nov 2017 13:12:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eFOeP-0003Ak-NI for qemu-devel@nongnu.org; Thu, 16 Nov 2017 13:12:36 -0500 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:42843) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eFOeP-0003A5-HN for qemu-devel@nongnu.org; Thu, 16 Nov 2017 13:12:33 -0500 Received: by mail-wm0-x244.google.com with SMTP id 5so755232wmk.1 for ; Thu, 16 Nov 2017 10:12:33 -0800 (PST) References: <20171116170526.12643-1-david@redhat.com> <20171116170526.12643-3-david@redhat.com> <87375e15vx.fsf@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: Date: Thu, 16 Nov 2017 18:12:30 +0000 Message-ID: <871sky14a9.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v1 for-2.11 2/3] s390x/tcg: fix DIAG 308 with > 1 VCPU (single threaded TCG) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Hildenbrand Cc: qemu-s390x@nongnu.org, qemu-devel@nongnu.org, Christian Borntraeger , Cornelia Huck , Alexander Graf , Richard Henderson David Hildenbrand writes: > On 16.11.2017 18:37, Alex Benn=C3=A9e wrote: >> >> David Hildenbrand writes: >> >>> Currently, single threaded TCG with > 1 VCPU gets stuck during IPL, when >>> the bios tries to switch to the loaded kernel via DIAG 308. >>> >>> pause_all_vcpus()/resume_all_vcpus() should not be called from a VCPU. >>> >>> And there is also no need for it. run_on_cpu() will make sure that the >>> CPUs exit KVM/TCG, where they get stopped. Once stopped, they will no >>> longer run. >>> >>> Signed-off-by: David Hildenbrand >>> --- >>> target/s390x/diag.c | 4 ---- >>> 1 file changed, 4 deletions(-) >>> >>> diff --git a/target/s390x/diag.c b/target/s390x/diag.c >>> index dbbb9e886f..52bc348808 100644 >>> --- a/target/s390x/diag.c >>> +++ b/target/s390x/diag.c >>> @@ -27,7 +27,6 @@ static int modified_clear_reset(S390CPU *cpu) >>> S390CPUClass *scc =3D S390_CPU_GET_CLASS(cpu); >>> CPUState *t; >>> >>> - pause_all_vcpus(); >>> cpu_synchronize_all_states(); >>> CPU_FOREACH(t) { >>> run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL); >> >> I think you also need to fix the run_on_cpu to be a async_run_on_cpu as >> you would otherwise hang waiting for run_on_cpu to finish on a >> single-threaded TCG run (as you are in the only vCPU context). > > No, it works just fine for single threaded TCG. run_on_cpu() can deal > with single threaded TCG just fine. (otherwise e.g. SIGP code also > wouldn't work) > > In do_run_on_cpu, the following code always directly triggers for single > threaded tcg: > > if (qemu_cpu_is_self(cpu)) { > > func(cpu, data); > > return; > > } For -smp 1 it's fine, but have you tested --accel thread=3Dsingle -smp 2? > >> >> If it is important that the source vCPU doesn't continue you can >> schedule it's work afterwards with a async_safe_run_on_cpu which will >> complete after all other vCPUs have executed their work. >> > > It is important. Introducing async helpers at a point where sync is > needed sounds strange. Well the helper that schedulules the final async helper also needs to exit the run loop at that point, otherwise you are right it would attempt to execute a few more instructions in the block. > > Thanks! > >>> >>> -- >>> 2.13.6 >> >> >> -- >> Alex Benn=C3=A9e >> > > > -- > > Thanks, > > David / dhildenb -- Alex Benn=C3=A9e