* [PATCH v1 0/2] target/microblaze: Improve bus fault handling @ 2020-08-28 11:39 Edgar E. Iglesias 2020-08-28 11:39 ` [PATCH v1 1/2] target/microblaze: Use CPU properties to conditionalize bus exceptions Edgar E. Iglesias 2020-08-28 11:39 ` [PATCH v1 2/2] target/microblaze: Improve transaction failure handling Edgar E. Iglesias 0 siblings, 2 replies; 9+ messages in thread From: Edgar E. Iglesias @ 2020-08-28 11:39 UTC (permalink / raw) To: qemu-devel Cc: figlesia, peter.maydell, sstabellini, edgar.iglesias, sai.pavan.boddu, frasse.iglesias, alistair, richard.henderson, frederic.konrad, qemu-arm, philmd, luc.michel From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Richards recent MicroBlaze patches exposed a bug in the MB ports bus fault handling. If the core is not setup for exceptions, we clobber the CPU state when preparing to raise one. This fixes the bug. Best regards, Edgar Edgar E. Iglesias (2): target/microblaze: Use CPU properties to conditionalize bus exceptions target/microblaze: Improve transaction failure handling target/microblaze/op_helper.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 1/2] target/microblaze: Use CPU properties to conditionalize bus exceptions 2020-08-28 11:39 [PATCH v1 0/2] target/microblaze: Improve bus fault handling Edgar E. Iglesias @ 2020-08-28 11:39 ` Edgar E. Iglesias 2020-08-28 13:03 ` Luc Michel 2020-08-28 18:45 ` Alistair Francis 2020-08-28 11:39 ` [PATCH v1 2/2] target/microblaze: Improve transaction failure handling Edgar E. Iglesias 1 sibling, 2 replies; 9+ messages in thread From: Edgar E. Iglesias @ 2020-08-28 11:39 UTC (permalink / raw) To: qemu-devel Cc: figlesia, peter.maydell, sstabellini, edgar.iglesias, sai.pavan.boddu, frasse.iglesias, alistair, richard.henderson, frederic.konrad, qemu-arm, philmd, luc.michel From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Use CPU properties, instead of PVR fields, to conditionalize bus exceptions. Fixes: 2867a96ffb ("target/microblaze: Add props enabling exceptions on failed bus accesses") Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> --- target/microblaze/op_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c index f3b17a95b3..13ac476199 100644 --- a/target/microblaze/op_helper.c +++ b/target/microblaze/op_helper.c @@ -490,12 +490,12 @@ void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr, env->sregs[SR_EAR] = addr; if (access_type == MMU_INST_FETCH) { - if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) { + if (cpu->cfg.iopb_bus_exception) { env->sregs[SR_ESR] = ESR_EC_INSN_BUS; helper_raise_exception(env, EXCP_HW_EXCP); } } else { - if ((env->pvr.regs[2] & PVR2_DOPB_BUS_EXC_MASK)) { + if (cpu->cfg.dopb_bus_exception) { env->sregs[SR_ESR] = ESR_EC_DATA_BUS; helper_raise_exception(env, EXCP_HW_EXCP); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] target/microblaze: Use CPU properties to conditionalize bus exceptions 2020-08-28 11:39 ` [PATCH v1 1/2] target/microblaze: Use CPU properties to conditionalize bus exceptions Edgar E. Iglesias @ 2020-08-28 13:03 ` Luc Michel 2020-08-28 18:45 ` Alistair Francis 1 sibling, 0 replies; 9+ messages in thread From: Luc Michel @ 2020-08-28 13:03 UTC (permalink / raw) To: Edgar E. Iglesias, qemu-devel Cc: figlesia, peter.maydell, sstabellini, edgar.iglesias, sai.pavan.boddu, frasse.iglesias, alistair, richard.henderson, frederic.konrad, qemu-arm, philmd On 8/28/20 1:39 PM, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Use CPU properties, instead of PVR fields, to conditionalize > bus exceptions. > > Fixes: 2867a96ffb ("target/microblaze: Add props enabling exceptions on failed bus accesses") > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Luc Michel <luc.michel@greensocs.com> > --- > target/microblaze/op_helper.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c > index f3b17a95b3..13ac476199 100644 > --- a/target/microblaze/op_helper.c > +++ b/target/microblaze/op_helper.c > @@ -490,12 +490,12 @@ void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr, > > env->sregs[SR_EAR] = addr; > if (access_type == MMU_INST_FETCH) { > - if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) { > + if (cpu->cfg.iopb_bus_exception) { > env->sregs[SR_ESR] = ESR_EC_INSN_BUS; > helper_raise_exception(env, EXCP_HW_EXCP); > } > } else { > - if ((env->pvr.regs[2] & PVR2_DOPB_BUS_EXC_MASK)) { > + if (cpu->cfg.dopb_bus_exception) { > env->sregs[SR_ESR] = ESR_EC_DATA_BUS; > helper_raise_exception(env, EXCP_HW_EXCP); > } > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] target/microblaze: Use CPU properties to conditionalize bus exceptions 2020-08-28 11:39 ` [PATCH v1 1/2] target/microblaze: Use CPU properties to conditionalize bus exceptions Edgar E. Iglesias 2020-08-28 13:03 ` Luc Michel @ 2020-08-28 18:45 ` Alistair Francis 1 sibling, 0 replies; 9+ messages in thread From: Alistair Francis @ 2020-08-28 18:45 UTC (permalink / raw) To: Edgar E. Iglesias Cc: figlesia, Peter Maydell, Edgar Iglesias, Sai Pavan Boddu, Francisco Iglesias, Alistair Francis, Richard Henderson, qemu-devel@nongnu.org Developers, KONRAD Frederic, Stefano Stabellini, qemu-arm, Philippe Mathieu-Daudé, Luc Michel 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> > > Use CPU properties, instead of PVR fields, to conditionalize > bus exceptions. > > Fixes: 2867a96ffb ("target/microblaze: Add props enabling exceptions on failed bus accesses") > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/microblaze/op_helper.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c > index f3b17a95b3..13ac476199 100644 > --- a/target/microblaze/op_helper.c > +++ b/target/microblaze/op_helper.c > @@ -490,12 +490,12 @@ void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr, > > env->sregs[SR_EAR] = addr; > if (access_type == MMU_INST_FETCH) { > - if ((env->pvr.regs[2] & PVR2_IOPB_BUS_EXC_MASK)) { > + if (cpu->cfg.iopb_bus_exception) { > env->sregs[SR_ESR] = ESR_EC_INSN_BUS; > helper_raise_exception(env, EXCP_HW_EXCP); > } > } else { > - if ((env->pvr.regs[2] & PVR2_DOPB_BUS_EXC_MASK)) { > + if (cpu->cfg.dopb_bus_exception) { > env->sregs[SR_ESR] = ESR_EC_DATA_BUS; > helper_raise_exception(env, EXCP_HW_EXCP); > } > -- > 2.25.1 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 2/2] target/microblaze: Improve transaction failure handling 2020-08-28 11:39 [PATCH v1 0/2] target/microblaze: Improve bus fault handling Edgar E. Iglesias 2020-08-28 11:39 ` [PATCH v1 1/2] target/microblaze: Use CPU properties to conditionalize bus exceptions Edgar E. Iglesias @ 2020-08-28 11:39 ` Edgar E. Iglesias 2020-08-28 13:04 ` Luc Michel ` (2 more replies) 1 sibling, 3 replies; 9+ messages in thread From: Edgar E. Iglesias @ 2020-08-28 11:39 UTC (permalink / raw) To: qemu-devel Cc: figlesia, peter.maydell, sstabellini, edgar.iglesias, sai.pavan.boddu, frasse.iglesias, alistair, richard.henderson, frederic.konrad, qemu-arm, philmd, luc.michel 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] target/microblaze: Improve transaction failure handling 2020-08-28 11:39 ` [PATCH v1 2/2] target/microblaze: Improve transaction failure handling Edgar E. Iglesias @ 2020-08-28 13:04 ` Luc Michel 2020-08-28 18:46 ` Alistair Francis 2020-08-28 20:34 ` Richard Henderson 2 siblings, 0 replies; 9+ messages in thread From: Luc Michel @ 2020-08-28 13:04 UTC (permalink / raw) To: Edgar E. Iglesias, qemu-devel Cc: figlesia, peter.maydell, sstabellini, edgar.iglesias, sai.pavan.boddu, frasse.iglesias, alistair, richard.henderson, frederic.konrad, qemu-arm, philmd 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] target/microblaze: Improve transaction failure handling 2020-08-28 11:39 ` [PATCH v1 2/2] target/microblaze: Improve transaction failure handling Edgar E. Iglesias 2020-08-28 13:04 ` Luc Michel @ 2020-08-28 18:46 ` Alistair Francis 2020-08-28 20:34 ` Richard Henderson 2 siblings, 0 replies; 9+ messages in thread From: Alistair Francis @ 2020-08-28 18:46 UTC (permalink / raw) To: Edgar E. Iglesias Cc: figlesia, Peter Maydell, Edgar Iglesias, Sai Pavan Boddu, Francisco Iglesias, Alistair Francis, Richard Henderson, qemu-devel@nongnu.org Developers, KONRAD Frederic, Stefano Stabellini, qemu-arm, Philippe Mathieu-Daudé, Luc Michel 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 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] target/microblaze: Improve transaction failure handling 2020-08-28 11:39 ` [PATCH v1 2/2] target/microblaze: Improve transaction failure handling Edgar E. Iglesias 2020-08-28 13:04 ` Luc Michel 2020-08-28 18:46 ` Alistair Francis @ 2020-08-28 20:34 ` Richard Henderson 2020-08-31 12:41 ` Edgar E. Iglesias 2 siblings, 1 reply; 9+ messages in thread From: Richard Henderson @ 2020-08-28 20:34 UTC (permalink / raw) To: Edgar E. Iglesias, qemu-devel Cc: figlesia, peter.maydell, sstabellini, edgar.iglesias, sai.pavan.boddu, frasse.iglesias, alistair, frederic.konrad, qemu-arm, philmd, luc.michel 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~ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] target/microblaze: Improve transaction failure handling 2020-08-28 20:34 ` Richard Henderson @ 2020-08-31 12:41 ` Edgar E. Iglesias 0 siblings, 0 replies; 9+ messages in thread From: Edgar E. Iglesias @ 2020-08-31 12:41 UTC (permalink / raw) To: Richard Henderson Cc: figlesia, peter.maydell, sstabellini, sai.pavan.boddu, frasse.iglesias, alistair, qemu-devel, frederic.konrad, qemu-arm, Edgar E. Iglesias, philmd, luc.michel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-08-31 12:42 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-08-28 11:39 [PATCH v1 0/2] target/microblaze: Improve bus fault handling Edgar E. Iglesias 2020-08-28 11:39 ` [PATCH v1 1/2] target/microblaze: Use CPU properties to conditionalize bus exceptions Edgar E. Iglesias 2020-08-28 13:03 ` Luc Michel 2020-08-28 18:45 ` Alistair Francis 2020-08-28 11:39 ` [PATCH v1 2/2] target/microblaze: Improve transaction failure handling Edgar E. Iglesias 2020-08-28 13:04 ` Luc Michel 2020-08-28 18:46 ` Alistair Francis 2020-08-28 20:34 ` Richard Henderson 2020-08-31 12:41 ` Edgar E. Iglesias
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).