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
