Re: [Xen-devel] [PATCH] xen: arm: enable perf counters

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-05-15 at 15:53 +0100, Julien Grall wrote:
> On 05/15/2014 03:17 PM, Ian Campbell wrote:
> > As well as the existing common perf counters add a bunch of ARM specifics,
> > including the various trap types, vuart/vgic/vtimer accesses and different
> > types of interrupt.
> 
> Performance counters in vgic don't make sense for me as we need to trap
> it in any case.

??? The point of these is so you can see how frequently something is
trapping, so you can measure and optimise as appropriate, the fact that
we have to trap a particular thing doesn't make it pointless to measure.

In this case the perfc lets you see that the majority of the traps are
to send SGIs and that the vast majority are sent to a specific list of
processors, which is something I was interested in.

> But we might want perf counter in p2m_lookup because this function is
> costly.
> 
> I would also add one in flush_tlb_* functions, such as flush_tlb_domain.
> It will help us optimizing TLBs.

Please do add more if you think they will be useful, this is just a
starting point. I think this applies to most of your comments, if you
are doing some debugging or performance measurement and you find that
you want an extra perfc or a more granular one or whatever then please
add it and send a patch. Otherwise than that I don't think there is much
need to bikeshed what exactly is being added here.

> >      case HSR_EC_CP15_64:
> >          if ( !is_32bit_domain(current->domain) )
> >              goto bad_trap;
> > +        perfc_incr(trap_cp15_32);
> 
> Did you mean trap_cp15_64?

Yes.

> > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> > index ef291ff..0de6f7e 100644
> > --- a/xen/include/asm-arm/config.h
> > +++ b/xen/include/asm-arm/config.h
> > @@ -178,6 +178,8 @@
> >  #define PAGE_MASK           (~(PAGE_SIZE-1))
> >  #define PAGE_FLAG_MASK      (~0)
> >  
> > +#define NR_hypercalls 64
> > +
> 
> Should not it be define in common code?

Could be, but it's not. Since different architectures can implement
different subsets of hypercalls I'm not too bothered about moving this.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel