Re: [PATCH v2 02/16] accel/tcg: Add probe_access_flags

[Richard Henderson]

On 4/16/20 4:54 AM, Peter Maydell wrote:
>> -    target_ulong tlb_addr;
>> -    size_t elt_ofs;
>> -    int wp_access;
>> -
>> -    g_assert(-(addr | TARGET_PAGE_MASK) >= size);
>> -
>> -    switch (access_type) {
>> -    case MMU_DATA_LOAD:
>> -        elt_ofs = offsetof(CPUTLBEntry, addr_read);
>> -        wp_access = BP_MEM_READ;
>> -        break;
>> -    case MMU_DATA_STORE:
>> -        elt_ofs = offsetof(CPUTLBEntry, addr_write);
>> -        wp_access = BP_MEM_WRITE;
>> -        break;
>> -    case MMU_INST_FETCH:
>> -        elt_ofs = offsetof(CPUTLBEntry, addr_code);
>> -        wp_access = BP_MEM_READ;
>> -        break;
>> -    default:
>> -        g_assert_not_reached();
>> -    }
>> -    tlb_addr = tlb_read_ofs(entry, elt_ofs);
>> -
>> -    if (unlikely(!tlb_hit(tlb_addr, addr))) {
>> -        if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs,
>> -                            addr & TARGET_PAGE_MASK)) {
>> -            tlb_fill(env_cpu(env), addr, size, access_type, mmu_idx, 
>> retaddr);
>> -            /* TLB resize via tlb_fill may have moved the entry. */
>> -            index = tlb_index(env, mmu_idx, addr);
>> -            entry = tlb_entry(env, mmu_idx, addr);
>> -        }
>> -        tlb_addr = tlb_read_ofs(entry, elt_ofs);
>> -    }
> 
> All of the code above seems to have disappeared in this
> refactoring -- it's not in probe_access_internal()
> but it hasn't moved to the new probe_access().

It's certainly still there.  I think this is diff being a bit daft.  There were
2 copies of this code before -- in probe_access and tlb_vaddr_to_host.


>> +void *probe_access(CPUArchState *env, target_ulong addr, int size,
>> +                   MMUAccessType access_type, int mmu_idx, uintptr_t 
>> retaddr)
>> +{
>> +    void *host;
>> +
>> +    g_assert(-(addr | TARGET_PAGE_MASK) >= size);
>> +    probe_access_flags(env, addr, access_type, mmu_idx, false, &host, 
>> retaddr);
>> +    return host;
>>  }
> 
> probe_access() used to pass the 'size' argument through to
> page_check_range() and cc->tlb_fill(); after this refactoring
> it no longer does that.

Seems I've fixed this in my tree since posting v2 -- probe_access now uses
probe_access_internal directly.  Anyway, definitely fixed for the next round.


r~