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

[Emilio G. Cota]

On Thu, Sep 10, 2015 at 14:49:07 +0100, Alex Bennée wrote:
> 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.

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