[Qemu-devel] [RFC] target/microblaze: Switch to transaction_failed hook

Peter Maydell posted 1 patch 5 years, 4 months ago
Test checkpatch passed
Test docker-quick@centos7 passed
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181210175630.30643-1-peter.maydell@linaro.org
target/microblaze/cpu.h       |  7 ++++---
target/microblaze/cpu.c       |  2 +-
target/microblaze/op_helper.c | 20 ++++++++++----------
3 files changed, 15 insertions(+), 14 deletions(-)
[Qemu-devel] [RFC] target/microblaze: Switch to transaction_failed hook
Posted by Peter Maydell 5 years, 4 months ago
Switch the microblaze target from the old unassigned_access hook
to the transaction_failed hook.

The notable difference is that rather than it being called
for all physical memory accesses which fail (including
those made by DMA devices or by the gdbstub), it is only
called for those made by the CPU via its MMU. For
microblaze this makes no difference because none of the
target CPU code needs to make loads or stores by physical
address.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
A straightforward conversion, but tagged RFC because I don't have
any microblaze test images and have tested only with "make check".

 target/microblaze/cpu.h       |  7 ++++---
 target/microblaze/cpu.c       |  2 +-
 target/microblaze/op_helper.c | 20 ++++++++++----------
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index 3c4e0ba80ac..03ca91007d5 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -388,9 +388,10 @@ static inline void cpu_get_tb_cpu_state(CPUMBState *env, target_ulong *pc,
 }
 
 #if !defined(CONFIG_USER_ONLY)
-void mb_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
-                              bool is_write, bool is_exec, int is_asi,
-                              unsigned size);
+void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
+                               unsigned size, MMUAccessType access_type,
+                               int mmu_idx, MemTxAttrs attrs,
+                               MemTxResult response, uintptr_t retaddr);
 #endif
 
 #endif
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 9b546a2c18e..49876b19b38 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -297,7 +297,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
 #ifdef CONFIG_USER_ONLY
     cc->handle_mmu_fault = mb_cpu_handle_mmu_fault;
 #else
-    cc->do_unassigned_access = mb_cpu_unassigned_access;
+    cc->do_transaction_failed = mb_cpu_transaction_failed;
     cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
 #endif
     dc->vmsd = &vmstate_mb_cpu;
diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
index 7cdbbcccaef..d25d3b626c8 100644
--- a/target/microblaze/op_helper.c
+++ b/target/microblaze/op_helper.c
@@ -486,18 +486,18 @@ void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
     mmu_write(env, ext, rn, v);
 }
 
-void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
-                              bool is_write, bool is_exec, int is_asi,
-                              unsigned size)
+void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
+                               unsigned size, MMUAccessType access_type,
+                               int mmu_idx, MemTxAttrs attrs,
+                               MemTxResult response, uintptr_t retaddr)
 {
     MicroBlazeCPU *cpu;
     CPUMBState *env;
-
-    qemu_log_mask(CPU_LOG_INT, "Unassigned " TARGET_FMT_plx " wr=%d exe=%d\n",
-             addr, is_write ? 1 : 0, is_exec ? 1 : 0);
-    if (cs == NULL) {
-        return;
-    }
+    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
+                  " physaddr 0x" TARGET_FMT_plx "size %d access type %s\n",
+                  addr, physaddr, size,
+                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
+                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
     cpu = MICROBLAZE_CPU(cs);
     env = &cpu->env;
     if (!(env->sregs[SR_MSR] & MSR_EE)) {
@@ -505,7 +505,7 @@ void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
     }
 
     env->sregs[SR_EAR] = addr;
-    if (is_exec) {
+    if (access_type == MMU_INST_FETCH) {
         if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) {
             env->sregs[SR_ESR] = ESR_EC_INSN_BUS;
             helper_raise_exception(env, EXCP_HW_EXCP);
-- 
2.19.2


Re: [Qemu-devel] [RFC] target/microblaze: Switch to transaction_failed hook
Posted by Peter Maydell 5 years, 4 months ago
On Mon, 10 Dec 2018 at 17:57, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Switch the microblaze target from the old unassigned_access hook
> to the transaction_failed hook.
>
> The notable difference is that rather than it being called
> for all physical memory accesses which fail (including
> those made by DMA devices or by the gdbstub), it is only
> called for those made by the CPU via its MMU. For
> microblaze this makes no difference because none of the
> target CPU code needs to make loads or stores by physical
> address.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> A straightforward conversion, but tagged RFC because I don't have
> any microblaze test images and have tested only with "make check".
>
>  target/microblaze/cpu.h       |  7 ++++---
>  target/microblaze/cpu.c       |  2 +-
>  target/microblaze/op_helper.c | 20 ++++++++++----------
>  3 files changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index 3c4e0ba80ac..03ca91007d5 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -388,9 +388,10 @@ static inline void cpu_get_tb_cpu_state(CPUMBState *env, target_ulong *pc,
>  }
>
>  #if !defined(CONFIG_USER_ONLY)
> -void mb_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
> -                              bool is_write, bool is_exec, int is_asi,
> -                              unsigned size);
> +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> +                               unsigned size, MMUAccessType access_type,
> +                               int mmu_idx, MemTxAttrs attrs,
> +                               MemTxResult response, uintptr_t retaddr);
>  #endif
>
>  #endif
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 9b546a2c18e..49876b19b38 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -297,7 +297,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
>  #ifdef CONFIG_USER_ONLY
>      cc->handle_mmu_fault = mb_cpu_handle_mmu_fault;
>  #else
> -    cc->do_unassigned_access = mb_cpu_unassigned_access;
> +    cc->do_transaction_failed = mb_cpu_transaction_failed;
>      cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
>  #endif
>      dc->vmsd = &vmstate_mb_cpu;
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index 7cdbbcccaef..d25d3b626c8 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -486,18 +486,18 @@ void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
>      mmu_write(env, ext, rn, v);
>  }
>
> -void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> -                              bool is_write, bool is_exec, int is_asi,
> -                              unsigned size)
> +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> +                               unsigned size, MMUAccessType access_type,
> +                               int mmu_idx, MemTxAttrs attrs,
> +                               MemTxResult response, uintptr_t retaddr)
>  {
>      MicroBlazeCPU *cpu;
>      CPUMBState *env;
> -
> -    qemu_log_mask(CPU_LOG_INT, "Unassigned " TARGET_FMT_plx " wr=%d exe=%d\n",
> -             addr, is_write ? 1 : 0, is_exec ? 1 : 0);
> -    if (cs == NULL) {
> -        return;
> -    }
> +    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
> +                  " physaddr 0x" TARGET_FMT_plx "size %d access type %s\n",
> +                  addr, physaddr, size,
> +                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
> +                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
>      cpu = MICROBLAZE_CPU(cs);
>      env = &cpu->env;
>      if (!(env->sregs[SR_MSR] & MSR_EE)) {
> @@ -505,7 +505,7 @@ void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
>      }
>
>      env->sregs[SR_EAR] = addr;
> -    if (is_exec) {
> +    if (access_type == MMU_INST_FETCH) {
>          if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) {
>              env->sregs[SR_ESR] = ESR_EC_INSN_BUS;
>              helper_raise_exception(env, EXCP_HW_EXCP);
> --
> 2.19.2
>
>


-- 
12345678901234567890123456789012345678901234567890123456789012345678901234567890
         1         2         3         4         5         6         7         8

Re: [Qemu-devel] [RFC] target/microblaze: Switch to transaction_failed hook
Posted by Peter Maydell 5 years, 4 months ago
On Mon, 10 Dec 2018 at 18:31, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 10 Dec 2018 at 17:57, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > Switch the microblaze target from the old unassigned_access hook
> > to the transaction_failed hook.
> >
> > The notable difference is that rather than it being called
> > for all physical memory accesses which fail (including
> > those made by DMA devices or by the gdbstub), it is only
> > called for those made by the CPU via its MMU. For
> > microblaze this makes no difference because none of the
> > target CPU code needs to make loads or stores by physical
> > address.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > A straightforward conversion, but tagged RFC because I don't have
> > any microblaze test images and have tested only with "make check".
> >
> >  target/microblaze/cpu.h       |  7 ++++---
> >  target/microblaze/cpu.c       |  2 +-
> >  target/microblaze/op_helper.c | 20 ++++++++++----------
> >  3 files changed, 15 insertions(+), 14 deletions(-)
> >
> > diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> > index 3c4e0ba80ac..03ca91007d5 100644
> > --- a/target/microblaze/cpu.h
> > +++ b/target/microblaze/cpu.h
> > @@ -388,9 +388,10 @@ static inline void cpu_get_tb_cpu_state(CPUMBState *env, target_ulong *pc,
> >  }
> >
> >  #if !defined(CONFIG_USER_ONLY)
> > -void mb_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
> > -                              bool is_write, bool is_exec, int is_asi,
> > -                              unsigned size);
> > +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> > +                               unsigned size, MMUAccessType access_type,
> > +                               int mmu_idx, MemTxAttrs attrs,
> > +                               MemTxResult response, uintptr_t retaddr);
> >  #endif
> >
> >  #endif
> > diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> > index 9b546a2c18e..49876b19b38 100644
> > --- a/target/microblaze/cpu.c
> > +++ b/target/microblaze/cpu.c
> > @@ -297,7 +297,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
> >  #ifdef CONFIG_USER_ONLY
> >      cc->handle_mmu_fault = mb_cpu_handle_mmu_fault;
> >  #else
> > -    cc->do_unassigned_access = mb_cpu_unassigned_access;
> > +    cc->do_transaction_failed = mb_cpu_transaction_failed;
> >      cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
> >  #endif
> >      dc->vmsd = &vmstate_mb_cpu;
> > diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> > index 7cdbbcccaef..d25d3b626c8 100644
> > --- a/target/microblaze/op_helper.c
> > +++ b/target/microblaze/op_helper.c
> > @@ -486,18 +486,18 @@ void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
> >      mmu_write(env, ext, rn, v);
> >  }
> >
> > -void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> > -                              bool is_write, bool is_exec, int is_asi,
> > -                              unsigned size)
> > +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> > +                               unsigned size, MMUAccessType access_type,
> > +                               int mmu_idx, MemTxAttrs attrs,
> > +                               MemTxResult response, uintptr_t retaddr)
> >  {
> >      MicroBlazeCPU *cpu;
> >      CPUMBState *env;
> > -
> > -    qemu_log_mask(CPU_LOG_INT, "Unassigned " TARGET_FMT_plx " wr=%d exe=%d\n",
> > -             addr, is_write ? 1 : 0, is_exec ? 1 : 0);
> > -    if (cs == NULL) {
> > -        return;
> > -    }
> > +    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
> > +                  " physaddr 0x" TARGET_FMT_plx "size %d access type %s\n",
> > +                  addr, physaddr, size,
> > +                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
> > +                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
> >      cpu = MICROBLAZE_CPU(cs);
> >      env = &cpu->env;

Oops. Here there should be

    cpu_restore_state(cs, retaddr, true);

> >      if (!(env->sregs[SR_MSR] & MSR_EE)) {
> > @@ -505,7 +505,7 @@ void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> >      }
> >
> >      env->sregs[SR_EAR] = addr;
> > -    if (is_exec) {
> > +    if (access_type == MMU_INST_FETCH) {
> >          if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) {
> >              env->sregs[SR_ESR] = ESR_EC_INSN_BUS;
> >              helper_raise_exception(env, EXCP_HW_EXCP);
> > --
> > 2.19.2

thanks
-- PMM

Re: [Qemu-devel] [RFC] target/microblaze: Switch to transaction_failed hook
Posted by Edgar E. Iglesias 5 years, 3 months ago
On Mon, Dec 10, 2018 at 06:32:49PM +0000, Peter Maydell wrote:
> On Mon, 10 Dec 2018 at 18:31, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 10 Dec 2018 at 17:57, Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > Switch the microblaze target from the old unassigned_access hook
> > > to the transaction_failed hook.
> > >
> > > The notable difference is that rather than it being called
> > > for all physical memory accesses which fail (including
> > > those made by DMA devices or by the gdbstub), it is only
> > > called for those made by the CPU via its MMU. For
> > > microblaze this makes no difference because none of the
> > > target CPU code needs to make loads or stores by physical
> > > address.

Hi again,

I just noticed a small typo



> > >
> > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > > ---
> > > A straightforward conversion, but tagged RFC because I don't have
> > > any microblaze test images and have tested only with "make check".
> > >
> > >  target/microblaze/cpu.h       |  7 ++++---
> > >  target/microblaze/cpu.c       |  2 +-
> > >  target/microblaze/op_helper.c | 20 ++++++++++----------
> > >  3 files changed, 15 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> > > index 3c4e0ba80ac..03ca91007d5 100644
> > > --- a/target/microblaze/cpu.h
> > > +++ b/target/microblaze/cpu.h
> > > @@ -388,9 +388,10 @@ static inline void cpu_get_tb_cpu_state(CPUMBState *env, target_ulong *pc,
> > >  }
> > >
> > >  #if !defined(CONFIG_USER_ONLY)
> > > -void mb_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
> > > -                              bool is_write, bool is_exec, int is_asi,
> > > -                              unsigned size);
> > > +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> > > +                               unsigned size, MMUAccessType access_type,
> > > +                               int mmu_idx, MemTxAttrs attrs,
> > > +                               MemTxResult response, uintptr_t retaddr);
> > >  #endif
> > >
> > >  #endif
> > > diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> > > index 9b546a2c18e..49876b19b38 100644
> > > --- a/target/microblaze/cpu.c
> > > +++ b/target/microblaze/cpu.c
> > > @@ -297,7 +297,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
> > >  #ifdef CONFIG_USER_ONLY
> > >      cc->handle_mmu_fault = mb_cpu_handle_mmu_fault;
> > >  #else
> > > -    cc->do_unassigned_access = mb_cpu_unassigned_access;
> > > +    cc->do_transaction_failed = mb_cpu_transaction_failed;
> > >      cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
> > >  #endif
> > >      dc->vmsd = &vmstate_mb_cpu;
> > > diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> > > index 7cdbbcccaef..d25d3b626c8 100644
> > > --- a/target/microblaze/op_helper.c
> > > +++ b/target/microblaze/op_helper.c
> > > @@ -486,18 +486,18 @@ void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
> > >      mmu_write(env, ext, rn, v);
> > >  }
> > >
> > > -void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> > > -                              bool is_write, bool is_exec, int is_asi,
> > > -                              unsigned size)
> > > +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> > > +                               unsigned size, MMUAccessType access_type,
> > > +                               int mmu_idx, MemTxAttrs attrs,
> > > +                               MemTxResult response, uintptr_t retaddr)
> > >  {
> > >      MicroBlazeCPU *cpu;
> > >      CPUMBState *env;
> > > -
> > > -    qemu_log_mask(CPU_LOG_INT, "Unassigned " TARGET_FMT_plx " wr=%d exe=%d\n",
> > > -             addr, is_write ? 1 : 0, is_exec ? 1 : 0);
> > > -    if (cs == NULL) {
> > > -        return;
> > > -    }
> > > +    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
> > > +                  " physaddr 0x" TARGET_FMT_plx "size %d access type %s\n",
                                                       ^^^

We'd want a space before size here.

Cheers,
Edgar



> > > +                  addr, physaddr, size,
> > > +                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
> > > +                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
> > >      cpu = MICROBLAZE_CPU(cs);
> > >      env = &cpu->env;
> 
> Oops. Here there should be
> 
>     cpu_restore_state(cs, retaddr, true);
> 
> > >      if (!(env->sregs[SR_MSR] & MSR_EE)) {
> > > @@ -505,7 +505,7 @@ void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> > >      }
> > >
> > >      env->sregs[SR_EAR] = addr;
> > > -    if (is_exec) {
> > > +    if (access_type == MMU_INST_FETCH) {
> > >          if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) {
> > >              env->sregs[SR_ESR] = ESR_EC_INSN_BUS;
> > >              helper_raise_exception(env, EXCP_HW_EXCP);
> > > --
> > > 2.19.2
> 
> thanks
> -- PMM

Re: [Qemu-devel] [RFC] target/microblaze: Switch to transaction_failed hook
Posted by Edgar E. Iglesias 5 years, 3 months ago
On Mon, Dec 10, 2018 at 06:32:49PM +0000, Peter Maydell wrote:
> On Mon, 10 Dec 2018 at 18:31, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 10 Dec 2018 at 17:57, Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > Switch the microblaze target from the old unassigned_access hook
> > > to the transaction_failed hook.
> > >
> > > The notable difference is that rather than it being called
> > > for all physical memory accesses which fail (including
> > > those made by DMA devices or by the gdbstub), it is only
> > > called for those made by the CPU via its MMU. For
> > > microblaze this makes no difference because none of the
> > > target CPU code needs to make loads or stores by physical
> > > address.


Thanks Peter, this looks good.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


Actually, upstream QEMU is lacking the necessary CPU cfg properties
to enable CPU exceptions for transaction errors. I'll post patches
to add those. On boards without those props enabled, the logging
happens but the error gets ignored by the core.

Cheers,
Edgar

> > >
> > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > > ---
> > > A straightforward conversion, but tagged RFC because I don't have
> > > any microblaze test images and have tested only with "make check".
> > >
> > >  target/microblaze/cpu.h       |  7 ++++---
> > >  target/microblaze/cpu.c       |  2 +-
> > >  target/microblaze/op_helper.c | 20 ++++++++++----------
> > >  3 files changed, 15 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> > > index 3c4e0ba80ac..03ca91007d5 100644
> > > --- a/target/microblaze/cpu.h
> > > +++ b/target/microblaze/cpu.h
> > > @@ -388,9 +388,10 @@ static inline void cpu_get_tb_cpu_state(CPUMBState *env, target_ulong *pc,
> > >  }
> > >
> > >  #if !defined(CONFIG_USER_ONLY)
> > > -void mb_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
> > > -                              bool is_write, bool is_exec, int is_asi,
> > > -                              unsigned size);
> > > +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> > > +                               unsigned size, MMUAccessType access_type,
> > > +                               int mmu_idx, MemTxAttrs attrs,
> > > +                               MemTxResult response, uintptr_t retaddr);
> > >  #endif
> > >
> > >  #endif
> > > diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> > > index 9b546a2c18e..49876b19b38 100644
> > > --- a/target/microblaze/cpu.c
> > > +++ b/target/microblaze/cpu.c
> > > @@ -297,7 +297,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
> > >  #ifdef CONFIG_USER_ONLY
> > >      cc->handle_mmu_fault = mb_cpu_handle_mmu_fault;
> > >  #else
> > > -    cc->do_unassigned_access = mb_cpu_unassigned_access;
> > > +    cc->do_transaction_failed = mb_cpu_transaction_failed;
> > >      cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
> > >  #endif
> > >      dc->vmsd = &vmstate_mb_cpu;
> > > diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> > > index 7cdbbcccaef..d25d3b626c8 100644
> > > --- a/target/microblaze/op_helper.c
> > > +++ b/target/microblaze/op_helper.c
> > > @@ -486,18 +486,18 @@ void helper_mmu_write(CPUMBState *env, uint32_t ext, uint32_t rn, uint32_t v)
> > >      mmu_write(env, ext, rn, v);
> > >  }
> > >
> > > -void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> > > -                              bool is_write, bool is_exec, int is_asi,
> > > -                              unsigned size)
> > > +void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> > > +                               unsigned size, MMUAccessType access_type,
> > > +                               int mmu_idx, MemTxAttrs attrs,
> > > +                               MemTxResult response, uintptr_t retaddr)
> > >  {
> > >      MicroBlazeCPU *cpu;
> > >      CPUMBState *env;
> > > -
> > > -    qemu_log_mask(CPU_LOG_INT, "Unassigned " TARGET_FMT_plx " wr=%d exe=%d\n",
> > > -             addr, is_write ? 1 : 0, is_exec ? 1 : 0);
> > > -    if (cs == NULL) {
> > > -        return;
> > > -    }
> > > +    qemu_log_mask(CPU_LOG_INT, "Transaction failed: vaddr 0x%" VADDR_PRIx
> > > +                  " physaddr 0x" TARGET_FMT_plx "size %d access type %s\n",
> > > +                  addr, physaddr, size,
> > > +                  access_type == MMU_INST_FETCH ? "INST_FETCH" :
> > > +                  (access_type == MMU_DATA_LOAD ? "DATA_LOAD" : "DATA_STORE"));
> > >      cpu = MICROBLAZE_CPU(cs);
> > >      env = &cpu->env;
> 
> Oops. Here there should be
> 
>     cpu_restore_state(cs, retaddr, true);
> 
> > >      if (!(env->sregs[SR_MSR] & MSR_EE)) {
> > > @@ -505,7 +505,7 @@ void mb_cpu_unassigned_access(CPUState *cs, hwaddr addr,
> > >      }
> > >
> > >      env->sregs[SR_EAR] = addr;
> > > -    if (is_exec) {
> > > +    if (access_type == MMU_INST_FETCH) {
> > >          if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) {
> > >              env->sregs[SR_ESR] = ESR_EC_INSN_BUS;
> > >              helper_raise_exception(env, EXCP_HW_EXCP);
> > > --
> > > 2.19.2
> 
> thanks
> -- PMM