Re: [Qemu-devel] [PATCH 23/26] target/xtensa: Convert to CPUClass::tlb_fill

[Peter Maydell]

On Wed, 3 Apr 2019 at 05:00, Richard Henderson
<address@hidden> wrote:
>
> Cc: Max Filippov <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/xtensa/cpu.h    |  5 +--
>  target/xtensa/cpu.c    |  5 ++-
>  target/xtensa/helper.c | 74 +++++++++++++++++++++---------------------
>  3 files changed, 42 insertions(+), 42 deletions(-)

> -#ifdef CONFIG_USER_ONLY
> -
> -int xtensa_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int 
> rw,
> -                                int mmu_idx)
> +bool xtensa_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> +                         MMUAccessType access_type, int mmu_idx,
> +                         bool probe, uintptr_t retaddr)
>  {
>      XtensaCPU *cpu = XTENSA_CPU(cs);
>      CPUXtensaState *env = &cpu->env;
> +    target_ulong vaddr = address;
> +    int ret;
>
> -    qemu_log_mask(CPU_LOG_INT,
> -                  "%s: rw = %d, address = 0x%08" VADDR_PRIx ", size = %d\n",
> -                  __func__, rw, address, size);
> -    env->sregs[EXCVADDR] = address;
> -    env->sregs[EXCCAUSE] = rw ? STORE_PROHIBITED_CAUSE : 
> LOAD_PROHIBITED_CAUSE;
> -    cs->exception_index = EXC_USER;
> -    return 1;

Previously we set exception_index to EXC_USER...

> +#ifdef CONFIG_USER_ONLY
> +    ret = (access_type == MMU_DATA_STORE ?
> +           STORE_PROHIBITED_CAUSE : LOAD_PROHIBITED_CAUSE);
> +#else
> +    uint32_t paddr;
> +    uint32_t page_size;
> +    unsigned access;
> +
> +    ret = xtensa_get_physical_addr(env, true, vaddr, access_type, mmu_idx,
> +                                   &paddr, &page_size, &access);
> +
> +    qemu_log_mask(CPU_LOG_MMU, "%s(%08x, %d, %d) -> %08x, ret = %d\n",
> +                  __func__, vaddr, access_type, mmu_idx, paddr, ret);
> +
> +    if (ret == 0) {
> +        tlb_set_page(cs, vaddr & TARGET_PAGE_MASK, paddr & TARGET_PAGE_MASK,
> +                     access, mmu_idx, page_size);
> +        return true;
> +    }
> +    if (probe) {
> +        return false;
> +    }
> +#endif
> +
> +    cpu_restore_state(cs, retaddr, true);
> +    HELPER(exception_cause_vaddr)(env, env->pc, ret, vaddr);

...but now we'll set it to whatever exception_cause_vaddr does,
which is something more complicated based on the state of
env->sregs[PS].

We'll also end up setting env->sregs[PS] bits and env->pc, which
the old code did not. (In particular since we set the PS_EXCM bit,
the second time we take an exception won't we then end up
setting exception_index to EXC_DOUBLE, not EXC_USER ?)

thanks
-- PMM