From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59408) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9qMf-0006iP-Pf for qemu-devel@nongnu.org; Mon, 06 Jun 2016 04:58:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b9qMb-0005aN-Iw for qemu-devel@nongnu.org; Mon, 06 Jun 2016 04:58:28 -0400 Received: from mail-wm0-x234.google.com ([2a00:1450:400c:c09::234]:36798) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9qMb-0005a8-C6 for qemu-devel@nongnu.org; Mon, 06 Jun 2016 04:58:25 -0400 Received: by mail-wm0-x234.google.com with SMTP id n184so81375514wmn.1 for ; Mon, 06 Jun 2016 01:58:24 -0700 (PDT) From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <57545D88.5010209@gmail.com> Date: Mon, 06 Jun 2016 09:58:37 +0100 Message-ID: <87eg8aithe.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC v1 12/12] cpus: default MTTCG to on for 32 bit ARM on x86 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Fedorov Cc: mttcg@listserver.greensocs.com, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, cota@braap.org, qemu-devel@nongnu.org, mark.burton@greensocs.com, pbonzini@redhat.com, jan.kiszka@siemens.com, rth@twiddle.net, peter.maydell@linaro.org, claudio.fontana@huawei.com, Peter Crosthwaite Sergey Fedorov writes: > On 15/04/16 17:23, Alex Bennée wrote: >> This makes multi-threading the default for 32 bit ARM on x86. It has >> been tested with Debian Jessie as well as my extended KVM unit tests >> which stress the SMC and TB invalidation code. Those tests can be found >> at: >> >> https://github.com/stsquad/kvm-unit-tests/tree/mttcg/current-tests-v5 >> >> Signed-off-by: Alex Bennée >> --- >> cpus.c | 20 ++++++++++++++++---- >> 1 file changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/cpus.c b/cpus.c >> index 860e2a9..daa92c7 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -171,12 +171,24 @@ opts_init(tcg_register_config); >> >> static bool default_mttcg_enabled(void) >> { >> - /* >> - * TODO: Check if we have a chance to have MTTCG working on this guest/host. >> - * Basically is the atomic instruction implemented? Is there any >> - * memory ordering issue? >> + /* Checklist for enabling MTTCG on a given frontend/backend combination >> + * >> + * - Are atomics correctly modelled for an MTTCG environment > > - Are cross-CPU manipulations safe (e.g. TLB invalidation/flush from > target helper) > - Are TCG context manipulations safe (e.g. TB invalidation from target > helper) OK > >> + * - If the backend is weakly ordered >> + * - has the front-end implemented explicit memory ordering ops >> + * - does the back-end generate code to ensure memory ordering >> */ >> +#if defined(__i386__) || defined(__x86_64__) >> + /* x86 backend is strongly ordered which helps a lot */ >> + #if defined(TARGET_ARM) >> + return true; >> + #else >> + return false; >> + #endif > > Is it okay to indent preprocessor lines this way? I think preprocessor > lines are better to stand out from regular code and could be indented > like this: > > #if defined(__foo__) > # if defined(BAR) > /* ... */ > # else > /* ... */ > # endif > #else > /* ... */ > #endif To be honest I was expecting more push-back on this because it is such an ugly way of solving the problem and expressing what a default on means. > > Kind regards, > Sergey > >> +#else >> + /* Until memory ordering implemented things will likely break */ >> return false; >> +#endif >> } >> >> void qemu_tcg_configure(QemuOpts *opts) -- Alex Bennée