| Message ID | 1460044433-19282-7-git-send-email-sergey.fedorov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Sergey Fedorov <sergey.fedorov@linaro.org> writes: > From: Sergey Fedorov <serge.fdrv@gmail.com> > > Ensure direct jump patching in s390 is atomic by: > * naturally aligning a location of direct jump address; > * using atomic_read()/atomic_set() for code patching. > > Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com> > Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org> > --- > include/exec/exec-all.h | 2 +- > tcg/s390/tcg-target.inc.c | 6 ++++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index 82399175fe80..e18cc24e50f0 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -320,7 +320,7 @@ static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr) > { > /* patch the branch destination */ > intptr_t disp = addr - (jmp_addr - 2); > - stl_be_p((void*)jmp_addr, disp / 2); > + atomic_set((int32_t *)jmp_addr, disp / 2); > /* no need to flush icache explicitly */ > } > #elif defined(__aarch64__) > diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c > index fbf97bb2e15d..84221dfb68c9 100644 > --- a/tcg/s390/tcg-target.inc.c > +++ b/tcg/s390/tcg-target.inc.c > @@ -219,6 +219,8 @@ typedef enum S390Opcode { > RX_ST = 0x50, > RX_STC = 0x42, > RX_STH = 0x40, > + > + NOP = 0x0707, > } S390Opcode; > > #ifndef NDEBUG > @@ -1716,6 +1718,10 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, > > case INDEX_op_goto_tb: > if (s->tb_jmp_offset) { > + /* align branch displacement for atomic pathing */ s/pathing/patching/ > + if (((uintptr_t)s->code_ptr & 3) == 0) { > + tcg_out16(s, NOP); > + } Isn't this the wrong way around? Shouldn't we insert the NOP is code_ptr & 3 == 2 (I assume 1 & 3 are impossible). Or is it that we need to be unaligned when we out the jmp so the offset itself is aligned. > tcg_out16(s, RIL_BRCL | (S390_CC_ALWAYS << 4)); > s->tb_jmp_offset[args[0]] = tcg_current_code_size(s); > s->code_ptr += 2; -- Alex Bennée
On 20/04/16 13:01, Alex Bennée wrote: > Sergey Fedorov <sergey.fedorov@linaro.org> writes: >> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h >> index 82399175fe80..e18cc24e50f0 100644 >> --- a/include/exec/exec-all.h >> +++ b/include/exec/exec-all.h (snip) >> @@ -1716,6 +1718,10 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, >> >> case INDEX_op_goto_tb: >> if (s->tb_jmp_offset) { >> + /* align branch displacement for atomic pathing */ > s/pathing/patching/ > >> + if (((uintptr_t)s->code_ptr & 3) == 0) { >> + tcg_out16(s, NOP); >> + } > Isn't this the wrong way around? Shouldn't we insert the NOP is code_ptr & 3 > == 2 (I assume 1 & 3 are impossible). Or is it that we need to be > unaligned when we out the jmp so the offset itself is aligned. Yes, it is the offset itself should be aligned to patch in atomically. Kind regards, Sergey > >> tcg_out16(s, RIL_BRCL | (S390_CC_ALWAYS << 4)); >> s->tb_jmp_offset[args[0]] = tcg_current_code_size(s); >> s->code_ptr += 2;
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 82399175fe80..e18cc24e50f0 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -320,7 +320,7 @@ static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr) { /* patch the branch destination */ intptr_t disp = addr - (jmp_addr - 2); - stl_be_p((void*)jmp_addr, disp / 2); + atomic_set((int32_t *)jmp_addr, disp / 2); /* no need to flush icache explicitly */ } #elif defined(__aarch64__) diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c index fbf97bb2e15d..84221dfb68c9 100644 --- a/tcg/s390/tcg-target.inc.c +++ b/tcg/s390/tcg-target.inc.c @@ -219,6 +219,8 @@ typedef enum S390Opcode { RX_ST = 0x50, RX_STC = 0x42, RX_STH = 0x40, + + NOP = 0x0707, } S390Opcode; #ifndef NDEBUG @@ -1716,6 +1718,10 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, case INDEX_op_goto_tb: if (s->tb_jmp_offset) { + /* align branch displacement for atomic pathing */ + if (((uintptr_t)s->code_ptr & 3) == 0) { + tcg_out16(s, NOP); + } tcg_out16(s, RIL_BRCL | (S390_CC_ALWAYS << 4)); s->tb_jmp_offset[args[0]] = tcg_current_code_size(s); s->code_ptr += 2;