On Mon, Aug 17, 2020 at 08:42:04AM -0700, Richard Henderson wrote: > On 8/17/20 7:01 AM, Edgar E. Iglesias wrote: > > From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com> > > > > Add support for data-access barriers. > > > > Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com> > > --- > > target/microblaze/translate.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c > > index c1be76d4c8..c58f334a0f 100644 > > --- a/target/microblaze/translate.c > > +++ b/target/microblaze/translate.c > > @@ -1233,6 +1233,11 @@ static void dec_br(DisasContext *dc) > > > > LOG_DIS("mbar %d\n", mbar_imm); > > > > + /* Data access memory barrier. */ > > + if ((mbar_imm & 2) == 0) { > > + tcg_gen_mb(TCG_BAR_SC | TCG_MO_ALL); > > + } > > + > > /* mbar IMM & 16 decodes to sleep. */ > > if (mbar_imm & 16) { > > TCGv_i32 tmp_hlt = tcg_const_i32(EXCP_HLT); > > > > The patch as written is fine, so > Reviewed-by: Richard Henderson <richard.hender...@linaro.org> > > However, a couple of other notes for mbar: > > (1) mbar_imm & 1 is insn memory barrier. For ARM, we do: > > /* > * We need to break the TB after this insn to execute > * self-modifying code correctly and also to take > * any pending interrupts immediately. > */ > gen_goto_tb(s, 0, s->base.pc_next);
Actually, we're already breaking the TB at the end of all mbars. I thought of perhaps not breaking it for data-only barriers but IIRC, we have some SW that depends on the current behavior (taking interrupts after raised due to previous load/stores) so I left it as is. > > (2) mbar_imm & 16 (sleep) should check for user-mode and generate > an illegal instruction. Yes, I'll fix that in a follow-up patch! Thanks, Edgar