Re: [Qemu-devel] [RFC v2 1/5] tcg: Add tlb_index and tlb_entry helpers

2018-10-09 Thread Alex Bennée 


Emilio G. Cota  writes:

> From: Richard Henderson 
>
> Isolate the computation of an index from an address into a
> helper before we change that function.
>
> Signed-off-by: Richard Henderson 
> [ cota: convert tlb_vaddr_to_host; use atomic_read on addr_write ]
> Signed-off-by: Emilio G. Cota 
> ---
>  accel/tcg/softmmu_template.h | 72 
>  include/exec/cpu_ldst.h  | 19 +++--
>  include/exec/cpu_ldst_template.h | 25 +--
>  accel/tcg/cputlb.c   | 61 ---
>  4 files changed, 92 insertions(+), 85 deletions(-)
>
> diff --git a/accel/tcg/softmmu_template.h b/accel/tcg/softmmu_template.h
> index 1e50263871..aa34c692ee 100644
> --- a/accel/tcg/softmmu_template.h
> +++ b/accel/tcg/softmmu_template.h
> @@ -111,9 +111,10 @@ static inline DATA_TYPE glue(io_read, 
> SUFFIX)(CPUArchState *env,
>  WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong addr,
>  TCGMemOpIdx oi, uintptr_t retaddr)
>  {
> -unsigned mmu_idx = get_mmuidx(oi);
> -int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> -target_ulong tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
> +uintptr_t mmu_idx = get_mmuidx(oi);
> +uintptr_t index = tlb_index(env, mmu_idx, addr);
> +CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
> +target_ulong tlb_addr = entry->ADDR_READ;
>  unsigned a_bits = get_alignment_bits(get_memop(oi));
>  uintptr_t haddr;
>  DATA_TYPE res;
> @@ -129,7 +130,7 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, 
> target_ulong addr,
>  tlb_fill(ENV_GET_CPU(env), addr, DATA_SIZE, READ_ACCESS_TYPE,
>   mmu_idx, retaddr);
>  }
> -tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
> +tlb_addr = entry->ADDR_READ;
>  }

Hmm I can see this clashing with the de-macro clean-up

>
>  /* Handle an IO access.  */
> @@ -166,7 +167,7 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, 
> target_ulong addr,
>  return res;
>  }
>
> -haddr = addr + env->tlb_table[mmu_idx][index].addend;
> +haddr = addr + entry->addend;
>  #if DATA_SIZE == 1
>  res = glue(glue(ld, LSUFFIX), _p)((uint8_t *)haddr);
>  #else
> @@ -179,9 +180,10 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, 
> target_ulong addr,
>  WORD_TYPE helper_be_ld_name(CPUArchState *env, target_ulong addr,
>  TCGMemOpIdx oi, uintptr_t retaddr)
>  {
> -unsigned mmu_idx = get_mmuidx(oi);
> -int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> -target_ulong tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
> +uintptr_t mmu_idx = get_mmuidx(oi);
> +uintptr_t index = tlb_index(env, mmu_idx, addr);
> +CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
> +target_ulong tlb_addr = entry->ADDR_READ;
>  unsigned a_bits = get_alignment_bits(get_memop(oi));
>  uintptr_t haddr;
>  DATA_TYPE res;
> @@ -197,7 +199,7 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, 
> target_ulong addr,
>  tlb_fill(ENV_GET_CPU(env), addr, DATA_SIZE, READ_ACCESS_TYPE,
>   mmu_idx, retaddr);
>  }
> -tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
> +tlb_addr = entry->ADDR_READ;
>  }
>
>  /* Handle an IO access.  */
> @@ -234,7 +236,7 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, 
> target_ulong addr,
>  return res;
>  }
>
> -haddr = addr + env->tlb_table[mmu_idx][index].addend;
> +haddr = addr + entry->addend;
>  res = glue(glue(ld, LSUFFIX), _be_p)((uint8_t *)haddr);
>  return res;
>  }
> @@ -275,10 +277,10 @@ static inline void glue(io_write, SUFFIX)(CPUArchState 
> *env,
>  void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
> TCGMemOpIdx oi, uintptr_t retaddr)
>  {
> -unsigned mmu_idx = get_mmuidx(oi);
> -int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> -target_ulong tlb_addr =
> -atomic_read(&env->tlb_table[mmu_idx][index].addr_write);
> +uintptr_t mmu_idx = get_mmuidx(oi);
> +uintptr_t index = tlb_index(env, mmu_idx, addr);
> +CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
> +target_ulong tlb_addr = atomic_read(&entry->addr_write);
>  unsigned a_bits = get_alignment_bits(get_memop(oi));
>  uintptr_t haddr;
>
> @@ -293,8 +295,7 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
> addr, DATA_TYPE val,
>  tlb_fill(ENV_GET_CPU(env), addr, DATA_SIZE, MMU_DATA_STORE,
>   mmu_idx, retaddr);
>  }
> -tlb_addr = atomic_read(&env->tlb_table[mmu_idx][index].addr_write) &
> -~TLB_INVALID_MASK;
> +tlb_addr = atomic_read(&entry->addr_write) & ~TLB_INVALID_MASK;
>  }
>
>  /* Handle an IO access.  */
> @@ -315,16 +316,16 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
> addr, DATA_TYPE val,
>

[Qemu-devel] [RFC v2 1/5] tcg: Add tlb_index and tlb_entry helpers

2018-10-08 Thread Emilio G. Cota 
From: Richard Henderson 

Isolate the computation of an index from an address into a
helper before we change that function.

Signed-off-by: Richard Henderson 
[ cota: convert tlb_vaddr_to_host; use atomic_read on addr_write ]
Signed-off-by: Emilio G. Cota 
---
 accel/tcg/softmmu_template.h | 72 
 include/exec/cpu_ldst.h  | 19 +++--
 include/exec/cpu_ldst_template.h | 25 +--
 accel/tcg/cputlb.c   | 61 ---
 4 files changed, 92 insertions(+), 85 deletions(-)

diff --git a/accel/tcg/softmmu_template.h b/accel/tcg/softmmu_template.h
index 1e50263871..aa34c692ee 100644
--- a/accel/tcg/softmmu_template.h
+++ b/accel/tcg/softmmu_template.h
@@ -111,9 +111,10 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState 
*env,
 WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong addr,
 TCGMemOpIdx oi, uintptr_t retaddr)
 {
-unsigned mmu_idx = get_mmuidx(oi);
-int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-target_ulong tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
+uintptr_t mmu_idx = get_mmuidx(oi);
+uintptr_t index = tlb_index(env, mmu_idx, addr);
+CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
+target_ulong tlb_addr = entry->ADDR_READ;
 unsigned a_bits = get_alignment_bits(get_memop(oi));
 uintptr_t haddr;
 DATA_TYPE res;
@@ -129,7 +130,7 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong 
addr,
 tlb_fill(ENV_GET_CPU(env), addr, DATA_SIZE, READ_ACCESS_TYPE,
  mmu_idx, retaddr);
 }
-tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
+tlb_addr = entry->ADDR_READ;
 }
 
 /* Handle an IO access.  */
@@ -166,7 +167,7 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong 
addr,
 return res;
 }
 
-haddr = addr + env->tlb_table[mmu_idx][index].addend;
+haddr = addr + entry->addend;
 #if DATA_SIZE == 1
 res = glue(glue(ld, LSUFFIX), _p)((uint8_t *)haddr);
 #else
@@ -179,9 +180,10 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, 
target_ulong addr,
 WORD_TYPE helper_be_ld_name(CPUArchState *env, target_ulong addr,
 TCGMemOpIdx oi, uintptr_t retaddr)
 {
-unsigned mmu_idx = get_mmuidx(oi);
-int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-target_ulong tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
+uintptr_t mmu_idx = get_mmuidx(oi);
+uintptr_t index = tlb_index(env, mmu_idx, addr);
+CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
+target_ulong tlb_addr = entry->ADDR_READ;
 unsigned a_bits = get_alignment_bits(get_memop(oi));
 uintptr_t haddr;
 DATA_TYPE res;
@@ -197,7 +199,7 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, target_ulong 
addr,
 tlb_fill(ENV_GET_CPU(env), addr, DATA_SIZE, READ_ACCESS_TYPE,
  mmu_idx, retaddr);
 }
-tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
+tlb_addr = entry->ADDR_READ;
 }
 
 /* Handle an IO access.  */
@@ -234,7 +236,7 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, target_ulong 
addr,
 return res;
 }
 
-haddr = addr + env->tlb_table[mmu_idx][index].addend;
+haddr = addr + entry->addend;
 res = glue(glue(ld, LSUFFIX), _be_p)((uint8_t *)haddr);
 return res;
 }
@@ -275,10 +277,10 @@ static inline void glue(io_write, SUFFIX)(CPUArchState 
*env,
 void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
 {
-unsigned mmu_idx = get_mmuidx(oi);
-int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-target_ulong tlb_addr =
-atomic_read(&env->tlb_table[mmu_idx][index].addr_write);
+uintptr_t mmu_idx = get_mmuidx(oi);
+uintptr_t index = tlb_index(env, mmu_idx, addr);
+CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
+target_ulong tlb_addr = atomic_read(&entry->addr_write);
 unsigned a_bits = get_alignment_bits(get_memop(oi));
 uintptr_t haddr;
 
@@ -293,8 +295,7 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
addr, DATA_TYPE val,
 tlb_fill(ENV_GET_CPU(env), addr, DATA_SIZE, MMU_DATA_STORE,
  mmu_idx, retaddr);
 }
-tlb_addr = atomic_read(&env->tlb_table[mmu_idx][index].addr_write) &
-~TLB_INVALID_MASK;
+tlb_addr = atomic_read(&entry->addr_write) & ~TLB_INVALID_MASK;
 }
 
 /* Handle an IO access.  */
@@ -315,16 +316,16 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
addr, DATA_TYPE val,
 if (DATA_SIZE > 1
 && unlikely((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1
  >= TARGET_PAGE_SIZE)) {
-int i, index2;
-target_ulong page2, tlb_addr2;
+int i;
+target_ulong page2;
+CPUTLBEntry *entry2;
 do_unaligned_access: