Re: [PATCH 00/77] target/microblaze improvements

[Edgar E. Iglesias]

On Thu, Aug 27, 2020 at 04:19:44AM -0700, Richard Henderson wrote:
> On 8/27/20 3:22 AM, Edgar E. Iglesias wrote:
> > Thanks. Here's another issue, it seems some branches are jumping
> > to the wrong address.
> > 
> > This is a disasm from a failing case:
> > 
> > 0x00000000ffd033a0:  brlid      r15, -636       // 0xffffffffffd03124
> > 0x00000000ffd033a4:  or r0, r0, r0
> > 
> > 0x00000000ffa73124:  Address 0xffa73124 is out of bounds.
> 
> That's a weird one.
> 
> My guess is that IMM_FLAG is set in iflags incorrectly.
> Can you verify this with -d in_asm,op,exec?
> 
> When IMM_FLAG is set, you'll see in in iflags: bit 0 will be set in the second
> word of the insn_data.  E.g.:
> 
>  ---- 00000000ffd033a0 0000000000000001
> 
> It would also show up in the tb_flags of the exec lines.  E.g.
> 
> Trace 0: 0x7f38a4000940 [0000000000000000/0000000090000058/0]
> 
> where the format is host_pc [cs_base/pc/tb_flags].
> 
> 
> If so, then we'll need to check where iflags got out of sync.
>

It seems to be getting out of sync when getting a slave-error and the core
is not setup to take exceptions for slave errors. Looks like a pre-existing
bug where we're restoring CPU state without taking the exception.
The following fixes that particular case in my runs.


I'm on a backported QEMU 5.1 so thing may look different in master.

diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
index 831ff2cac1..0cae51c2df 100644
--- a/target/microblaze/op_helper.c
+++ b/target/microblaze/op_helper.c
@@ -432,22 +432,19 @@ void mb_cpu_transaction_failed(CPUState *cs, hwaddr 
physaddr, vaddr addr,
     cpu = MICROBLAZE_CPU(cs);
     env = &cpu->env;
 
-    cpu_restore_state(cs, retaddr, true);
-    if (!(env->msr & MSR_EE)) {
+    if (!cpu->cfg.iopb_bus_exception || !(env->msr & MSR_EE)) {
         return;
     }
 
+    cpu_restore_state(cs, retaddr, true);
+
     env->ear = addr;
     if (access_type == MMU_INST_FETCH) {
-        if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) {
-            env->esr = ESR_EC_INSN_BUS;
-            helper_raise_exception(env, EXCP_HW_EXCP);
-        }
+        env->esr = ESR_EC_INSN_BUS;
+        helper_raise_exception(env, EXCP_HW_EXCP);
     } else {
-        if ((env->pvr.regs[2] & PVR2_DOPB_BUS_EXC_MASK)) {
-            env->esr = ESR_EC_DATA_BUS;
-            helper_raise_exception(env, EXCP_HW_EXCP);
-        }
+        env->esr = ESR_EC_DATA_BUS;
+        helper_raise_exception(env, EXCP_HW_EXCP);
     }
 }
 #endif