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
+
 typedef struct CPUArchState CPUMBState;
 #if !defined(CONFIG_USER_ONLY)
 #include "mmu.h"
-- 
2.34.1

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
>
>

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 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>