Re: [Qemu-devel] [RFC 13/38] cputlb: add physical address to CPUTLBEntry

[Alex Bennée]

Emilio G. Cota <address@hidden> writes:

> Having the physical address in the TLB entry will allow us
> to portably obtain the physical address of a memory access,
> which will prove useful when implementing a scalable emulation
> of atomic instructions.
>
> Signed-off-by: Emilio G. Cota <address@hidden>
> ---
>  cputlb.c                | 1 +
>  include/exec/cpu-defs.h | 7 ++++---
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/cputlb.c b/cputlb.c
> index d1ad8e8..1b3673e 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -409,6 +409,7 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong 
> vaddr,
>      } else {
>          te->addr_write = -1;
>      }
> +    te->addr_phys = paddr;
>  }
>  
>  /* Add a new TLB entry, but without specifying the memory
> diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
> index 5093be2..ca9c85c 100644
> --- a/include/exec/cpu-defs.h
> +++ b/include/exec/cpu-defs.h
> @@ -60,10 +60,10 @@ typedef uint64_t target_ulong;
>  /* use a fully associative victim tlb of 8 entries */
>  #define CPU_VTLB_SIZE 8
>  
> -#if HOST_LONG_BITS == 32 && TARGET_LONG_BITS == 32
> -#define CPU_TLB_ENTRY_BITS 4
> -#else
> +#if TARGET_LONG_BITS == 32
>  #define CPU_TLB_ENTRY_BITS 5
> +#else
> +#define CPU_TLB_ENTRY_BITS 6
>  #endif
>  
>  /* TCG_TARGET_TLB_DISPLACEMENT_BITS is used in CPU_TLB_BITS to ensure that
> @@ -110,6 +110,7 @@ typedef struct CPUTLBEntry {
>              target_ulong addr_read;
>              target_ulong addr_write;
>              target_ulong addr_code;
> +            target_ulong addr_phys;
>              /* Addend to virtual address to get host address.  IO accesses
>                 use the corresponding iotlb value.  */
>              uintptr_t addend;

So this ends up expanding the TLB entry size and either pushing the
overall TLB up in size or reducing the number of entries per-TLB so I
think we would need some numbers on the impact on performance this has.

As far as I can see you never use this value in this patch series so
maybe this is worth deferring for now?

-- 
Alex Bennée