From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34861) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1asoeX-0005QC-Ga for qemu-devel@nongnu.org; Wed, 20 Apr 2016 05:42:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1asoeT-00037A-5T for qemu-devel@nongnu.org; Wed, 20 Apr 2016 05:42:33 -0400 Received: from mail-wm0-x22a.google.com ([2a00:1450:400c:c09::22a]:35653) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1asoeS-000373-W5 for qemu-devel@nongnu.org; Wed, 20 Apr 2016 05:42:29 -0400 Received: by mail-wm0-x22a.google.com with SMTP id e201so43533951wme.0 for ; Wed, 20 Apr 2016 02:42:28 -0700 (PDT) References: <1460044433-19282-1-git-send-email-sergey.fedorov@linaro.org> <1460044433-19282-4-git-send-email-sergey.fedorov@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1460044433-19282-4-git-send-email-sergey.fedorov@linaro.org> Date: Wed, 20 Apr 2016 10:42:26 +0100 Message-ID: <87shygvcot.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 03/11] tci: Make direct jump patching thread-safe List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Fedorov Cc: qemu-devel@nongnu.org, Sergey Fedorov , Paolo Bonzini , Peter Crosthwaite , Richard Henderson , Stefan Weil Sergey Fedorov writes: > From: Sergey Fedorov > > Ensure direct jump patching in TCI is atomic by: > * naturally aligning a location of direct jump address; > * using atomic_read()/atomic_set() to load/store the address. > > Signed-off-by: Sergey Fedorov > Signed-off-by: Sergey Fedorov > --- > include/exec/exec-all.h | 2 +- > tcg/tci/tcg-target.inc.c | 2 ++ > tci.c | 5 ++++- > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index 736209505a68..59709c9dd5c9 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -302,7 +302,7 @@ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr); > static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr) > { > /* patch the branch destination */ > - *(uint32_t *)jmp_addr = addr - (jmp_addr + 4); > + atomic_set((int32_t *)jmp_addr, addr - (jmp_addr + 4)); > /* no need to flush icache explicitly */ > } > #elif defined(_ARCH_PPC) > diff --git a/tcg/tci/tcg-target.inc.c b/tcg/tci/tcg-target.inc.c > index 4afe4d7a8d59..7e6180e62898 100644 > --- a/tcg/tci/tcg-target.inc.c > +++ b/tcg/tci/tcg-target.inc.c > @@ -556,6 +556,8 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args, > if (s->tb_jmp_offset) { > /* Direct jump method. */ > assert(args[0] < ARRAY_SIZE(s->tb_jmp_offset)); > + /* Align for atomic patching and thread safety */ > + s->code_ptr = (uint8_t *)(((uintptr_t)s->code_ptr + 3) & > ~3); Seeing this pattern is being used over and over again I wonder if we should have some utility helper functions for this? Perhaps we should steal the kernels ALIGN macros? > s->tb_jmp_offset[args[0]] = tcg_current_code_size(s); > tcg_out32(s, 0); > } else { > diff --git a/tci.c b/tci.c > index 82705fe77295..531f5ebf2c2e 100644 > --- a/tci.c > +++ b/tci.c > @@ -1089,7 +1089,10 @@ uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t *tb_ptr) > goto exit; > break; > case INDEX_op_goto_tb: > - t0 = tci_read_i32(&tb_ptr); > + /* Jump address is aligned */ > + tb_ptr = (uint8_t *)(((uintptr_t)tb_ptr + 3) & ~3); > + t0 = atomic_read((int32_t *)tb_ptr); > + tb_ptr += sizeof(int32_t); > tci_assert(tb_ptr == old_code_ptr + op_size); > tb_ptr += (int32_t)t0; > continue; -- Alex Bennée