From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51558) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPCIv-0002Qk-G9 for qemu-devel@nongnu.org; Mon, 18 Jul 2016 13:26:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPCIr-0004B0-8y for qemu-devel@nongnu.org; Mon, 18 Jul 2016 13:26:04 -0400 Received: from mail-lf0-x242.google.com ([2a00:1450:4010:c07::242]:33244) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPCIr-0004Ar-0S for qemu-devel@nongnu.org; Mon, 18 Jul 2016 13:26:01 -0400 Received: by mail-lf0-x242.google.com with SMTP id f93so11692799lfi.0 for ; Mon, 18 Jul 2016 10:26:00 -0700 (PDT) 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> <84ba6ace-e0bd-515a-5194-2e6704d22805@redhat.com> From: Sergey Fedorov Message-ID: <578D1126.4060803@gmail.com> Date: Mon, 18 Jul 2016 20:25:58 +0300 MIME-Version: 1.0 In-Reply-To: <84ba6ace-e0bd-515a-5194-2e6704d22805@redhat.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: Paolo Bonzini , qemu-devel@nongnu.org Cc: sergey.fedorov@linaro.org, alex.bennee@linaro.org On 18/07/16 20:22, Paolo Bonzini wrote: > > 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; > } Hmm, not sure if it is really simpler to follow. Maybe " |= 1;" anyway? > > 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. :)