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

[Richard Henderson]

On 4/30/19 10:32 AM, Max Filippov wrote:
> On Tue, Apr 30, 2019 at 3:11 AM Peter Maydell <address@hidden> wrote:
>>
>> 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 ?)
> 
> I guess it doesn't matter, because linux-user userspace never handles
> exceptions. PS, PC and similar must be fixed up by the linux-user
> exception handler. But I haven't tested it.

It does handle exceptions, in linux-user/xtensa/cpu_loop.c.
And Peter's right that I should have kept EXC_USER.

> Richard, is there a branch with this series available somewhere?

https://github.com/rth7680/qemu/tree/tcg-tlb-fill


r~