Re: [Qemu-devel] [PATCH RFC 3/5] softmmu: add a tlb_vaddr_to_host_fill function

[Richard Henderson]

On 06/02/2015 04:26 AM, Aurelien Jarno wrote:
>      int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> -    CPUTLBEntry *tlbentry = &env->tlb_table[mmu_idx][index];
> +    CPUTLBEntry *tlbentry;
>      target_ulong tlb_addr;
>      uintptr_t haddr;
>  
> +again:
> +    tlbentry = &env->tlb_table[mmu_idx][index];
> +
>      switch (access_type) {
> -    case 0:
> +    case MMU_DATA_LOAD:
>          tlb_addr = tlbentry->addr_read;
>          break;
> -    case 1:
> +    case MMU_DATA_STORE:
>          tlb_addr = tlbentry->addr_write;
>          break;
> -    case 2:
> +    case MMU_INST_FETCH:
>          tlb_addr = tlbentry->addr_code;
>          break;
>      default:
> @@ -347,10 +350,14 @@ static inline void *tlb_vaddr_to_host(CPUArchState 
> *env, target_ulong addr,
>      if ((addr & TARGET_PAGE_MASK)
>          != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) {
>          /* TLB entry is for a different page */
> +        if (fill) {
> +            tlb_fill(ENV_GET_CPU(env), addr, access_type, mmu_idx, retaddr);
> +            goto again;
> +        }
>          return NULL;
>      }

To properly perform a fill, you also ought to check the victim cache.
There's a macro to do that in softmmu_template.h, which is why I
placed probe_write there.  It's not so convenient to use with a
variable type though.

In addition, the address of tlbentry cannot change, so there's no
point in recomputing that.  Indeed, you'd probably be better off
saving &addr_foo so that you only have to go through the switch once.

  switch (access_type) {
  case N:
    tlb_addr_ptr = &tlbentry->addr_foo;
    break;
  }
  tlb_addr = *tlb_addr_ptr;
  if (...) {
    if (!VICTIM_TLB_HIT(...)) {
      if (!fill) {
        return NULL;
      }
      tlb_fill(...);
    }
    tlb_addr = *tlb_addr_ptr;
  }

and thus there's no loop to be mis-predicted.


r~