1. Patchew
  2. QEMU
  3. [PATCH v1 0/2] target/microblaze: Improve bus fault handling
  4. View patch

[PATCH v1 2/2] target/microblaze: Improve transaction failure handling

Edgar E. Iglesias posted 2 patches 5 years, 2 months ago
Maintainers: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
[PATCH v1 2/2] target/microblaze: Improve transaction failure handling
Posted by Edgar E. Iglesias 5 years, 2 months ago 
From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

When the CPU has exceptions disabled, avoid unwinding CPU
state and clobbering registers if we're not going to raise
any exception.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target/microblaze/op_helper.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
index 13ac476199..190cd96c6a 100644
--- a/target/microblaze/op_helper.c
+++ b/target/microblaze/op_helper.c
@@ -483,22 +483,17 @@ 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->sregs[SR_MSR] & MSR_EE)) {
         return;
     }
 
-    env->sregs[SR_EAR] = addr;
-    if (access_type == MMU_INST_FETCH) {
-        if (cpu->cfg.iopb_bus_exception) {
-            env->sregs[SR_ESR] = ESR_EC_INSN_BUS;
-            helper_raise_exception(env, EXCP_HW_EXCP);
-        }
-    } else {
-        if (cpu->cfg.dopb_bus_exception) {
-            env->sregs[SR_ESR] = ESR_EC_DATA_BUS;
-            helper_raise_exception(env, EXCP_HW_EXCP);
-        }
+    if ((access_type == MMU_INST_FETCH && cpu->cfg.iopb_bus_exception) ||
+        (access_type != MMU_INST_FETCH && cpu->cfg.dopb_bus_exception)) {
+        cpu_restore_state(cs, retaddr, true);
+        env->sregs[SR_ESR] = access_type == MMU_INST_FETCH ?
+                             ESR_EC_INSN_BUS : ESR_EC_DATA_BUS;
+        env->sregs[SR_EAR] = addr;
+        helper_raise_exception(env, EXCP_HW_EXCP);
     }
 }
 #endif
-- 
2.25.1
Re: [PATCH v1 2/2] target/microblaze: Improve transaction failure handling
Posted by Luc Michel 5 years, 2 months ago 
On 8/28/20 1:39 PM, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> When the CPU has exceptions disabled, avoid unwinding CPU
> state and clobbering registers if we're not going to raise
> any exception.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Luc Michel <luc.michel@greensocs.com>

> ---
>   target/microblaze/op_helper.c | 19 +++++++------------
>   1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index 13ac476199..190cd96c6a 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -483,22 +483,17 @@ 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->sregs[SR_MSR] & MSR_EE)) {
>           return;
>       }
>   
> -    env->sregs[SR_EAR] = addr;
> -    if (access_type == MMU_INST_FETCH) {
> -        if (cpu->cfg.iopb_bus_exception) {
> -            env->sregs[SR_ESR] = ESR_EC_INSN_BUS;
> -            helper_raise_exception(env, EXCP_HW_EXCP);
> -        }
> -    } else {
> -        if (cpu->cfg.dopb_bus_exception) {
> -            env->sregs[SR_ESR] = ESR_EC_DATA_BUS;
> -            helper_raise_exception(env, EXCP_HW_EXCP);
> -        }
> +    if ((access_type == MMU_INST_FETCH && cpu->cfg.iopb_bus_exception) ||
> +        (access_type != MMU_INST_FETCH && cpu->cfg.dopb_bus_exception)) {
> +        cpu_restore_state(cs, retaddr, true);
> +        env->sregs[SR_ESR] = access_type == MMU_INST_FETCH ?
> +                             ESR_EC_INSN_BUS : ESR_EC_DATA_BUS;
> +        env->sregs[SR_EAR] = addr;
> +        helper_raise_exception(env, EXCP_HW_EXCP);
>       }
>   }
>   #endif
>
Re: [PATCH v1 2/2] target/microblaze: Improve transaction failure handling
Posted by Alistair Francis 5 years, 2 months ago 
On Fri, Aug 28, 2020 at 4:41 AM Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
>
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> When the CPU has exceptions disabled, avoid unwinding CPU
> state and clobbering registers if we're not going to raise
> any exception.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/microblaze/op_helper.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index 13ac476199..190cd96c6a 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -483,22 +483,17 @@ 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->sregs[SR_MSR] & MSR_EE)) {
>          return;
>      }
>
> -    env->sregs[SR_EAR] = addr;
> -    if (access_type == MMU_INST_FETCH) {
> -        if (cpu->cfg.iopb_bus_exception) {
> -            env->sregs[SR_ESR] = ESR_EC_INSN_BUS;
> -            helper_raise_exception(env, EXCP_HW_EXCP);
> -        }
> -    } else {
> -        if (cpu->cfg.dopb_bus_exception) {
> -            env->sregs[SR_ESR] = ESR_EC_DATA_BUS;
> -            helper_raise_exception(env, EXCP_HW_EXCP);
> -        }
> +    if ((access_type == MMU_INST_FETCH && cpu->cfg.iopb_bus_exception) ||
> +        (access_type != MMU_INST_FETCH && cpu->cfg.dopb_bus_exception)) {
> +        cpu_restore_state(cs, retaddr, true);
> +        env->sregs[SR_ESR] = access_type == MMU_INST_FETCH ?
> +                             ESR_EC_INSN_BUS : ESR_EC_DATA_BUS;
> +        env->sregs[SR_EAR] = addr;
> +        helper_raise_exception(env, EXCP_HW_EXCP);
>      }
>  }
>  #endif
> --
> 2.25.1
>
>
Re: [PATCH v1 2/2] target/microblaze: Improve transaction failure handling
Posted by Richard Henderson 5 years, 2 months ago 
On 8/28/20 4:39 AM, Edgar E. Iglesias wrote:
> +    if ((access_type == MMU_INST_FETCH && cpu->cfg.iopb_bus_exception) ||
> +        (access_type != MMU_INST_FETCH && cpu->cfg.dopb_bus_exception)) {
> +        cpu_restore_state(cs, retaddr, true);
> +        env->sregs[SR_ESR] = access_type == MMU_INST_FETCH ?
> +                             ESR_EC_INSN_BUS : ESR_EC_DATA_BUS;
> +        env->sregs[SR_EAR] = addr;
> +        helper_raise_exception(env, EXCP_HW_EXCP);

I think it's better to use cpu_loop_exit_restore, adding the one line for
cs->exception_index from helper_raise_exception.


r~
Re: [PATCH v1 2/2] target/microblaze: Improve transaction failure handling
Posted by Edgar E. Iglesias 5 years, 2 months ago 
On Fri, Aug 28, 2020 at 01:34:08PM -0700, Richard Henderson wrote:
> On 8/28/20 4:39 AM, Edgar E. Iglesias wrote:
> > +    if ((access_type == MMU_INST_FETCH && cpu->cfg.iopb_bus_exception) ||
> > +        (access_type != MMU_INST_FETCH && cpu->cfg.dopb_bus_exception)) {
> > +        cpu_restore_state(cs, retaddr, true);
> > +        env->sregs[SR_ESR] = access_type == MMU_INST_FETCH ?
> > +                             ESR_EC_INSN_BUS : ESR_EC_DATA_BUS;
> > +        env->sregs[SR_EAR] = addr;
> > +        helper_raise_exception(env, EXCP_HW_EXCP);
> 
> I think it's better to use cpu_loop_exit_restore, adding the one line for
> cs->exception_index from helper_raise_exception.
> 

OK, let's use the patch you posted:

https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg07630.html

Thanks,
Edgar

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.