* [Qemu-devel] target/arm: Fix abort on exception return from AArch64 to AArch32 [not found] <CGME20170712113922eucas1p1c6588b2e29439254cafd3d8dde5c505c@eucas1p1.samsung.com> @ 2017-07-12 11:39 ` Jaroslaw Pelczar 2017-07-12 12:29 ` Peter Maydell 0 siblings, 1 reply; 3+ messages in thread From: Jaroslaw Pelczar @ 2017-07-12 11:39 UTC (permalink / raw) To: qemu-devel Problem manifests itself when we handle the following sequence: 1. 64-bit Secure EL3 returns to 32-bit Secure EL1 2. 32-bit Secure EL1 performs SMC call to 64-bit Secure EL3 3. 64-bit Secure EL3 performs return ERET to 32-bit Secure EL1] 4. 32-bit Secure EL1 receives prefetch abort If CPU's env->pc is not set to the same value as env->regs[15], during ERET the simulator will try to fetch instruction from EL3's virtual address inside 32-bit Secure EL1 virtual address space. This will cause Prefetch Abort in 32-bit Secure EL1. Problem occurs because of the following code generation scheme: 1. disas_uncond_b_reg will decode ERET 2. gen_helper_exception_return(cpu_env) will generate thunk to helper_exception_return 3. s->is_jmp is set to DISAS_JUMP 4. gen_intermediate_code_a64 will see dc->is_jmp == DISAS_JUMP 5. tcg_gen_lookup_and_goto_ptr(cpu_pc) will be called which trigger access cpu->pc to fetch next opcode At this point cpu->pc will have EL3's PC value. Signed-off-by: Jaroslaw Pelczar <j.pelczar@samsung.com> --- target/arm/op_helper.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index 2a85666..8222c19 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -1027,6 +1027,9 @@ void HELPER(exception_return)(CPUARMState *env) } else { env->regs[15] = env->elr_el[cur_el] & ~0x3; } + + env->pc = env->regs[15]; + qemu_log_mask(CPU_LOG_INT, "Exception return from AArch64 EL%d to " "AArch32 EL%d PC 0x%" PRIx32 "\n", cur_el, new_el, env->regs[15]); -- 2.7.4 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] target/arm: Fix abort on exception return from AArch64 to AArch32 2017-07-12 11:39 ` [Qemu-devel] target/arm: Fix abort on exception return from AArch64 to AArch32 Jaroslaw Pelczar @ 2017-07-12 12:29 ` Peter Maydell 2017-07-12 15:36 ` Alex Bennée 0 siblings, 1 reply; 3+ messages in thread From: Peter Maydell @ 2017-07-12 12:29 UTC (permalink / raw) To: Jaroslaw Pelczar; +Cc: QEMU Developers, Alex Bennée On 12 July 2017 at 12:39, Jaroslaw Pelczar <j.pelczar@samsung.com> wrote: > Problem manifests itself when we handle the following sequence: > > 1. 64-bit Secure EL3 returns to 32-bit Secure EL1 > 2. 32-bit Secure EL1 performs SMC call to 64-bit Secure EL3 > 3. 64-bit Secure EL3 performs return ERET to 32-bit Secure EL1] > 4. 32-bit Secure EL1 receives prefetch abort > > If CPU's env->pc is not set to the same value as env->regs[15], > during ERET the simulator will try to fetch instruction from EL3's > virtual address inside 32-bit Secure EL1 virtual address space. > This will cause Prefetch Abort in 32-bit Secure EL1. > > Problem occurs because of the following code generation scheme: > > 1. disas_uncond_b_reg will decode ERET > 2. gen_helper_exception_return(cpu_env) will generate thunk to > helper_exception_return > 3. s->is_jmp is set to DISAS_JUMP > 4. gen_intermediate_code_a64 will see dc->is_jmp == DISAS_JUMP > 5. tcg_gen_lookup_and_goto_ptr(cpu_pc) will be called which trigger > access cpu->pc to fetch next opcode > > At this point cpu->pc will have EL3's PC value. > > Signed-off-by: Jaroslaw Pelczar <j.pelczar@samsung.com> > --- > target/arm/op_helper.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c > index 2a85666..8222c19 100644 > --- a/target/arm/op_helper.c > +++ b/target/arm/op_helper.c > @@ -1027,6 +1027,9 @@ void HELPER(exception_return)(CPUARMState *env) > } else { > env->regs[15] = env->elr_el[cur_el] & ~0x3; > } > + > + env->pc = env->regs[15]; > + > qemu_log_mask(CPU_LOG_INT, "Exception return from AArch64 EL%d to " > "AArch32 EL%d PC 0x%" PRIx32 "\n", > cur_el, new_el, env->regs[15]); Thanks for the bug report. I don't think this is the best fix, though. What should happen is that for ERET we do not attempt to do a lookup_and_goto_ptr(), we should just go back out to the main loop and get the next TB from there. I think this may be fixed as one of the things Alex's recent patchset tackles? thanks -- PMM ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] target/arm: Fix abort on exception return from AArch64 to AArch32 2017-07-12 12:29 ` Peter Maydell @ 2017-07-12 15:36 ` Alex Bennée 0 siblings, 0 replies; 3+ messages in thread From: Alex Bennée @ 2017-07-12 15:36 UTC (permalink / raw) To: Peter Maydell; +Cc: Jaroslaw Pelczar, QEMU Developers Peter Maydell <peter.maydell@linaro.org> writes: > On 12 July 2017 at 12:39, Jaroslaw Pelczar <j.pelczar@samsung.com> wrote: >> Problem manifests itself when we handle the following sequence: >> >> 1. 64-bit Secure EL3 returns to 32-bit Secure EL1 >> 2. 32-bit Secure EL1 performs SMC call to 64-bit Secure EL3 >> 3. 64-bit Secure EL3 performs return ERET to 32-bit Secure EL1] >> 4. 32-bit Secure EL1 receives prefetch abort >> >> If CPU's env->pc is not set to the same value as env->regs[15], >> during ERET the simulator will try to fetch instruction from EL3's >> virtual address inside 32-bit Secure EL1 virtual address space. >> This will cause Prefetch Abort in 32-bit Secure EL1. >> >> Problem occurs because of the following code generation scheme: >> >> 1. disas_uncond_b_reg will decode ERET >> 2. gen_helper_exception_return(cpu_env) will generate thunk to >> helper_exception_return >> 3. s->is_jmp is set to DISAS_JUMP >> 4. gen_intermediate_code_a64 will see dc->is_jmp == DISAS_JUMP >> 5. tcg_gen_lookup_and_goto_ptr(cpu_pc) will be called which trigger >> access cpu->pc to fetch next opcode >> >> At this point cpu->pc will have EL3's PC value. >> >> Signed-off-by: Jaroslaw Pelczar <j.pelczar@samsung.com> >> --- >> target/arm/op_helper.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c >> index 2a85666..8222c19 100644 >> --- a/target/arm/op_helper.c >> +++ b/target/arm/op_helper.c >> @@ -1027,6 +1027,9 @@ void HELPER(exception_return)(CPUARMState *env) >> } else { >> env->regs[15] = env->elr_el[cur_el] & ~0x3; >> } >> + >> + env->pc = env->regs[15]; >> + >> qemu_log_mask(CPU_LOG_INT, "Exception return from AArch64 EL%d to " >> "AArch32 EL%d PC 0x%" PRIx32 "\n", >> cur_el, new_el, env->regs[15]); > > Thanks for the bug report. I don't think this is the best > fix, though. What should happen is that for ERET we do > not attempt to do a lookup_and_goto_ptr(), we should just > go back out to the main loop and get the next TB from there. > > I think this may be fixed as one of the things Alex's > recent patchset tackles? Yes, see: https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg02963.html I'll probably get v4 out tomorrow at which point you can merge it ;-) > > thanks > -- PMM -- Alex Bennée ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-07-12 15:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20170712113922eucas1p1c6588b2e29439254cafd3d8dde5c505c@eucas1p1.samsung.com>
2017-07-12 11:39 ` [Qemu-devel] target/arm: Fix abort on exception return from AArch64 to AArch32 Jaroslaw Pelczar
2017-07-12 12:29 ` Peter Maydell
2017-07-12 15:36 ` Alex Bennée
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).