Re: [PATCH] xen: arm: enable perf counters

[Julien Grall <julien.grall@linaro.org>]

On 05/15/2014 04:30 PM, Ian Campbell wrote:
>> 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.

I agree it's a starting point and I took the opportunity to give some
feedback on what kind of perf counter it would be nice to have on Xen.

IHMO, p2m_lookup and flush_tlb_domain should have the own perf counter
because they are used in hot patch. I'm fine to create a follow-up but
as you were working on it...

> 
>>>      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.

Oh ok. Thanks.

Regards,

-- 
Julien Grall