| Message ID | 20200817140144.373403-4-edgar.iglesias@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/microblaze: Enable MTTCG | expand |
On 8/17/20 7:01 AM, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Add support for data-access barriers. > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@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.henderson@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); (2) mbar_imm & 16 (sleep) should check for user-mode and generate an illegal instruction. r~
On Mon, Aug 17, 2020 at 7:02 AM Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Add support for data-access barriers. > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > 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); > -- > 2.25.1 > >
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.iglesias@xilinx.com> > > > > Add support for data-access barriers. > > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@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.henderson@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
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);