[RFC,13/38] cputlb: add physical address to CPUTLBEntry

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 <cota@braap.org>
---
 cputlb.c                | 1 +
 include/exec/cpu-defs.h | 7 ++++---
 2 files changed, 5 insertions(+), 3 deletions(-)

Emilio G. Cota <cota@braap.org> 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 <cota@braap.org>
> ---
>  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?

On Thu, Sep 10, 2015 at 14:49:07 +0100, Alex Bennée wrote:
> Emilio G. Cota <cota@braap.org> 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 <cota@braap.org>
> > ---
> >  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.

My tests show little to no perf impact, but note that I work on fairly big
machines that have large caches. I could run some benchmarks, although
this will take a while--will be on vacation for a couple of weeks.

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

This is used for atomic instruction emulation in full-system mode; see
aie-helper.c which has calls to helpers added to softmmu_template.h.

		Emilio

On 24/08/2015 02:23, Emilio G. Cota wrote:
> 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.

It came to my mind that addr_read and addr_code only differ in the
bottom TARGET_PAGE_BITS bits, and they are always zero in addr_phys.

So we could store addr_code as

   addr_read ^ (addr_phys & (TARGET_PAGE_SIZE - 1))

and discard the bottom bits of addr_phys.  This would make it possible
to include addr_phys without growing the size of the TLB entry.

Paolo