| Message ID | 20180820023110.6583-1-jcmvbkbc@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/xtensa: convert to do_transaction_failed | expand |
On 20 August 2018 at 03:31, Max Filippov <jcmvbkbc@gmail.com> wrote: > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> > --- > target/xtensa/cpu.c | 2 +- > target/xtensa/cpu.h | 7 ++++--- > target/xtensa/op_helper.c | 12 +++++++----- > 3 files changed, 12 insertions(+), 9 deletions(-) Hi -- thanks for doing this conversion. This change means that you'll no longer get guest exceptions for bus errors that happen during get_pte()'s page table entry fetch. Don't you need at least a TODO comment there indicating that there's missing behaviour? (Obviously the ideal would be to switch to address_space_ldl_* and handle the case when they don't return MEMTX_OK.) thanks -- PMM
Hi Peter, On Fri, Aug 24, 2018 at 10:56 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 20 August 2018 at 03:31, Max Filippov <jcmvbkbc@gmail.com> wrote: >> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> >> --- >> target/xtensa/cpu.c | 2 +- >> target/xtensa/cpu.h | 7 ++++--- >> target/xtensa/op_helper.c | 12 +++++++----- >> 3 files changed, 12 insertions(+), 9 deletions(-) > > Hi -- thanks for doing this conversion. This change means that > you'll no longer get guest exceptions for bus errors that happen > during get_pte()'s page table entry fetch. Don't you need at > least a TODO comment there indicating that there's missing > behaviour? Thanks for taking a look and letting me know, I've missed that entirely. > (Obviously the ideal would be to switch to address_space_ldl_* > and handle the case when they don't return MEMTX_OK.) Yeah, let me do something about it.
On 24 August 2018 at 19:10, Max Filippov <jcmvbkbc@gmail.com> wrote: > Hi Peter, > > On Fri, Aug 24, 2018 at 10:56 AM, Peter Maydell > <peter.maydell@linaro.org> wrote: >> On 20 August 2018 at 03:31, Max Filippov <jcmvbkbc@gmail.com> wrote: >>> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> >>> --- >>> target/xtensa/cpu.c | 2 +- >>> target/xtensa/cpu.h | 7 ++++--- >>> target/xtensa/op_helper.c | 12 +++++++----- >>> 3 files changed, 12 insertions(+), 9 deletions(-) >> >> Hi -- thanks for doing this conversion. This change means that >> you'll no longer get guest exceptions for bus errors that happen >> during get_pte()'s page table entry fetch. Don't you need at >> least a TODO comment there indicating that there's missing >> behaviour? > > Thanks for taking a look and letting me know, I've missed that entirely. Basically the new hook only handles failures for code paths that go via the MMU; anything where the target code does a direct physical address lookup has to handle the failure itself. I think for xtensa that's just the page table reads. (This is why it's a bit of a pain to convert a target, but the end result makes much more sense, because almost always the target code doing physical address accesses doesn't want to handle bus errors by randomly longjumping out.) thanks -- PMM
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c index 590813d4f7b9..a54dbe42602d 100644 --- a/target/xtensa/cpu.c +++ b/target/xtensa/cpu.c @@ -186,7 +186,7 @@ static void xtensa_cpu_class_init(ObjectClass *oc, void *data) #else cc->do_unaligned_access = xtensa_cpu_do_unaligned_access; cc->get_phys_page_debug = xtensa_cpu_get_phys_page_debug; - cc->do_unassigned_access = xtensa_cpu_do_unassigned_access; + cc->do_transaction_failed = xtensa_cpu_do_transaction_failed; #endif cc->debug_excp_handler = xtensa_breakpoint_handler; cc->disas_set_info = xtensa_cpu_disas_set_info; diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h index 7472cf3ca32a..1362772617ea 100644 --- a/target/xtensa/cpu.h +++ b/target/xtensa/cpu.h @@ -497,9 +497,10 @@ int xtensa_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw, int size, int mmu_idx); void xtensa_cpu_do_interrupt(CPUState *cpu); bool xtensa_cpu_exec_interrupt(CPUState *cpu, int interrupt_request); -void xtensa_cpu_do_unassigned_access(CPUState *cpu, hwaddr addr, - bool is_write, bool is_exec, int opaque, - unsigned size); +void xtensa_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr, + unsigned size, MMUAccessType access_type, + int mmu_idx, MemTxAttrs attrs, + MemTxResult response, uintptr_t retaddr); void xtensa_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf, int flags); hwaddr xtensa_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr); diff --git a/target/xtensa/op_helper.c b/target/xtensa/op_helper.c index d4c942d87980..06fe346f02ff 100644 --- a/target/xtensa/op_helper.c +++ b/target/xtensa/op_helper.c @@ -78,18 +78,20 @@ void tlb_fill(CPUState *cs, target_ulong vaddr, int size, } } -void xtensa_cpu_do_unassigned_access(CPUState *cs, hwaddr addr, - bool is_write, bool is_exec, int opaque, - unsigned size) +void xtensa_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr, + unsigned size, MMUAccessType access_type, + int mmu_idx, MemTxAttrs attrs, + MemTxResult response, uintptr_t retaddr) { XtensaCPU *cpu = XTENSA_CPU(cs); CPUXtensaState *env = &cpu->env; + cpu_restore_state(cs, retaddr, true); HELPER(exception_cause_vaddr)(env, env->pc, - is_exec ? + access_type == MMU_INST_FETCH ? INSTR_PIF_ADDR_ERROR_CAUSE : LOAD_STORE_PIF_ADDR_ERROR_CAUSE, - is_exec ? addr : cs->mem_io_vaddr); + addr); } static void tb_invalidate_virtual_addr(CPUXtensaState *env, uint32_t vaddr)
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> --- target/xtensa/cpu.c | 2 +- target/xtensa/cpu.h | 7 ++++--- target/xtensa/op_helper.c | 12 +++++++----- 3 files changed, 12 insertions(+), 9 deletions(-)