From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50289) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPCFE-0007Xb-Cl for qemu-devel@nongnu.org; Mon, 18 Jul 2016 13:22:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPCFA-0002zj-4x for qemu-devel@nongnu.org; Mon, 18 Jul 2016 13:22:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51366) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPCF9-0002zf-Uy for qemu-devel@nongnu.org; Mon, 18 Jul 2016 13:22:12 -0400 References: <1468851450-9863-1-git-send-email-pbonzini@redhat.com> <578D0938.2050004@gmail.com> <578D0A60.4020608@gmail.com> <8c52110d-c009-087c-be04-3920d97af323@redhat.com> <578D0CD6.3080005@gmail.com> <4c479cb3-a8ef-caab-4dbc-3bfd014c7a0a@redhat.com> <578D0F42.4040203@gmail.com> From: Paolo Bonzini Message-ID: <84ba6ace-e0bd-515a-5194-2e6704d22805@redhat.com> Date: Mon, 18 Jul 2016 19:22:08 +0200 MIME-Version: 1.0 In-Reply-To: <578D0F42.4040203@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] atomics: add volatile_read/volatile_set List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Fedorov , qemu-devel@nongnu.org Cc: sergey.fedorov@linaro.org, alex.bennee@linaro.org On 18/07/2016 19:17, Sergey Fedorov wrote: > On 18/07/16 20:11, Paolo Bonzini wrote: >> >> On 18/07/2016 19:07, Sergey Fedorov wrote: >>> On 18/07/16 20:00, Paolo Bonzini wrote: >>>> On 18/07/2016 18:57, Sergey Fedorov wrote: >>>>> On 18/07/16 19:53, Paolo Bonzini wrote: >>>>>> On 18/07/2016 18:52, Sergey Fedorov wrote: >>>>>>> So how are we going to use them? >>>>>> Instead of atomic_read/atomic_set when marking invalid TBs. >>>>> But shouldn't they be atomic to avoid reading torn writes? >>>> A torn write would probably fail to match anyway, but even if it doesn't >>>> it is indistinguishable from a race, isn't it? >>> I'm afraid, torn write can happen to be a false match against a wrong >>> TB. In case of a race with atomic access we either get the right TB or >>> an invalid one which couldn't match any valid CPU state. Probably, we >>> have to make sure (and document this) that TB invalidation process >>> cannot make a partially invalidated TB which can match any meaningful >>> CPU state. >> x86 is atomic (because flags are 32-bit); those that have cs_base==0 are >> safe against torn writes too. Only SPARC perhaps could use >> "tb->cs_base|=1" instead in case 0xffffffff........ matches another TB. > > That could really work but needs some comment, of course. Yup. A simpler possibility is this: diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h index e327a35..3278d8a 100644 --- a/target-sparc/cpu.h +++ b/target-sparc/cpu.h @@ -753,14 +753,14 @@ static inline void cpu_get_invalid_tb_cpu_state(target_ulong *pc, target_ulong *cs_base, uint32_t *flags) { - *cs_base = -1; /* npc must be a multible of 4 */ + *flags = TB_FLAG_MMU_MASK; } static inline bool cpu_tb_cpu_state_is_invalidated(target_ulong pc, target_ulong cs_base, uint32_t flags) { - return cs_base == -1; + return flags == TB_FLAG_MMU_MASK; } static inline bool tb_fpu_enabled(int tb_flags) I'll send a fixup patch now, ack it and I'll send another pull request. :) > BTW, what is > the main point of such change? A bit more performance on some 32-bit hosts? No, succeeding to compile on 32-bit hosts. :)