| Message ID | 20230619142333.429028-2-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg: Issue memory barriers for guest memory model | expand |
On 19/6/23 16:23, Richard Henderson wrote: > The microblaze architecture does not reorder instructions. > While there is an MBAR wait-for-data-access instruction, > this concerns synchronizing with DMA. > > This should have been defined when enabling MTTCG. > > Cc: Alistair Francis <alistair.francis@wdc.com> > Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> > Fixes: d449561b130 ("configure: microblaze: Enable mttcg") > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/microblaze/cpu.h | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 19/6/23 16:23, Richard Henderson wrote: > The microblaze architecture does not reorder instructions. > While there is an MBAR wait-for-data-access instruction, > this concerns synchronizing with DMA. > > This should have been defined when enabling MTTCG. > > Cc: Alistair Francis <alistair.francis@wdc.com> > Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> > Fixes: d449561b130 ("configure: microblaze: Enable mttcg") > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/microblaze/cpu.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h > index 88324d0bc1..b474abcc2a 100644 > --- a/target/microblaze/cpu.h > +++ b/target/microblaze/cpu.h > @@ -24,6 +24,9 @@ > #include "exec/cpu-defs.h" > #include "qemu/cpu-float.h" > > +/* MicroBlaze is always in-order. */ > +#define TCG_GUEST_DEFAULT_MO TCG_MO_ALL Targets missing such definition: - cris - m68k - nios2 - rx - sh4 - sparc/64 (!) - tricore I expect targets designed for embedded systems to be in-order for power efficiency. What about having each target being explicit about that, having a build failure if TCG_GUEST_DEFAULT_MO is not defined, instead of the '#ifdef TCG_GUEST_DEFAULT_MO' in accel/tcg/?
On 6/20/23 17:41, Philippe Mathieu-Daudé wrote: > On 19/6/23 16:23, Richard Henderson wrote: >> The microblaze architecture does not reorder instructions. >> While there is an MBAR wait-for-data-access instruction, >> this concerns synchronizing with DMA. >> >> This should have been defined when enabling MTTCG. >> >> Cc: Alistair Francis <alistair.francis@wdc.com> >> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> >> Fixes: d449561b130 ("configure: microblaze: Enable mttcg") >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> target/microblaze/cpu.h | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h >> index 88324d0bc1..b474abcc2a 100644 >> --- a/target/microblaze/cpu.h >> +++ b/target/microblaze/cpu.h >> @@ -24,6 +24,9 @@ >> #include "exec/cpu-defs.h" >> #include "qemu/cpu-float.h" >> +/* MicroBlaze is always in-order. */ >> +#define TCG_GUEST_DEFAULT_MO TCG_MO_ALL > > Targets missing such definition: > > - cris > - m68k > - nios2 > - rx > - sh4 > - sparc/64 (!) > - tricore > > I expect targets designed for embedded systems > to be in-order for power efficiency. > > What about having each target being explicit about that, > having a build failure if TCG_GUEST_DEFAULT_MO is not defined, > instead of the '#ifdef TCG_GUEST_DEFAULT_MO' in accel/tcg/? I'd be ok with that. r~
On Mon, Jun 19, 2023 at 4:23 PM Richard Henderson < richard.henderson@linaro.org> wrote: > The microblaze architecture does not reorder instructions. > While there is an MBAR wait-for-data-access instruction, > this concerns synchronizing with DMA. > > This should have been defined when enabling MTTCG. Reviewed-by: Edgar E. Iglesias <edgar@zeroasic.com> There might be MicroBlaze systems that allow reordering of load vs store streams but it doesn't seem to be documented and I'm not 100% certain so this change LGTM! Thanks, Edgar > > Cc: Alistair Francis <alistair.francis@wdc.com> > Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> > Fixes: d449561b130 ("configure: microblaze: Enable mttcg") > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/microblaze/cpu.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h > index 88324d0bc1..b474abcc2a 100644 > --- a/target/microblaze/cpu.h > +++ b/target/microblaze/cpu.h > @@ -24,6 +24,9 @@ > #include "exec/cpu-defs.h" > #include "qemu/cpu-float.h" > > +/* MicroBlaze is always in-order. */ > +#define TCG_GUEST_DEFAULT_MO TCG_MO_ALL > + > typedef struct CPUArchState CPUMBState; > #if !defined(CONFIG_USER_ONLY) > #include "mmu.h" > -- > 2.34.1 > >
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h index 88324d0bc1..b474abcc2a 100644 --- a/target/microblaze/cpu.h +++ b/target/microblaze/cpu.h @@ -24,6 +24,9 @@ #include "exec/cpu-defs.h" #include "qemu/cpu-float.h" +/* MicroBlaze is always in-order. */ +#define TCG_GUEST_DEFAULT_MO TCG_MO_ALL + typedef struct CPUArchState CPUMBState; #if !defined(CONFIG_USER_ONLY) #include "mmu.h"
The microblaze architecture does not reorder instructions. While there is an MBAR wait-for-data-access instruction, this concerns synchronizing with DMA. This should have been defined when enabling MTTCG. Cc: Alistair Francis <alistair.francis@wdc.com> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> Fixes: d449561b130 ("configure: microblaze: Enable mttcg") Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/microblaze/cpu.h | 3 +++ 1 file changed, 3 insertions(+)