* [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
* [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 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 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 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
* 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).