From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60581) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cjliU-0005ng-SN for qemu-devel@nongnu.org; Fri, 03 Mar 2017 06:49:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cjliR-0001WZ-Nv for qemu-devel@nongnu.org; Fri, 03 Mar 2017 06:49:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36008) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cjliR-0001WO-IN for qemu-devel@nongnu.org; Fri, 03 Mar 2017 06:49:43 -0500 References: <20170302195337.31558-1-alex.bennee@linaro.org> <20170302195337.31558-5-alex.bennee@linaro.org> From: Paolo Bonzini Message-ID: <776ee8db-a894-e17d-2cd9-234882daee0c@redhat.com> Date: Fri, 3 Mar 2017 12:49:36 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 04/11] translate: downgrade IRQ BQL asserts to tcg_debug_assert List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , =?UTF-8?Q?Alex_Benn=c3=a9e?= Cc: Richard Henderson , QEMU Developers , MTTCG Devel , =?UTF-8?B?S09OUkFEIEZyw6lkw6lyaWM=?= , Alvise Rigo , "Emilio G. Cota" , Pranith Kumar , Nikunj A Dadhania , Peter Crosthwaite On 03/03/2017 11:08, Peter Maydell wrote: > On 2 March 2017 at 19:53, Alex Benn=C3=A9e wro= te: >> While on MTTCG hosts it is very important that updates to >> cpu->interrupt_request are protected by the BQL not all guests have >> been converted to the correct locking yet. As a result we are seeing >> breaking on non-MTTCG enabled guests in production builds. >> >> The locking in the guests needs to be fixed but while running single >> threaded they will continue to work. By moving the asserts to >> tcg_debug_asserts() they will still be useful during conversion >> work (much like the existing assert_memory_lock/assert_tb_lock >> asserts). >> >> Signed-off-by: Alex Benn=C3=A9e >> --- >> translate-all.c | 2 +- >> translate-common.c | 3 ++- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/translate-all.c b/translate-all.c >> index 9bac061c9b..7ee273410d 100644 >> --- a/translate-all.c >> +++ b/translate-all.c >> @@ -1928,7 +1928,7 @@ void dump_opcount_info(FILE *f, fprintf_function= cpu_fprintf) >> >> void cpu_interrupt(CPUState *cpu, int mask) >> { >> - g_assert(qemu_mutex_iothread_locked()); >> + tcg_debug_assert(qemu_mutex_iothread_locked()); >=20 > If CONFIG_DEBUG_TCG isn't enabled then tcg_debug_assert() > turns into "if (!(X)) { __builtin_unreachable(); }", which > means that instead of asserting we now run straight > into compiler undefined behaviour, don't we? > If what we want is "don't actually check this condition in > the non-tcg-debug config" then we should do something > that means we don't actually check the condition... I think we should fix it for good and leave this as a hard assertion. Paolo