Re: [PATCH v5 RESEND 13/17] x86/VPMU: Add privileged PMU mode

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 04/26/2014 04:39 AM, Tian, Kevin wrote:
>> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
>> Sent: Wednesday, April 23, 2014 8:51 PM
>>
>> Add support for privileged PMU mode which allows privileged domain (dom0)
>> profile both itself (and the hypervisor) and the guests. While this mode is on
>> profiling in guests is disabled.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> ---
>>   xen/arch/x86/hvm/vpmu.c  | 97
>> +++++++++++++++++++++++++++++++++---------------
>>   xen/arch/x86/traps.c     |  6 ++-
>>   xen/include/public/pmu.h |  3 ++
>>   3 files changed, 76 insertions(+), 30 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
>> index abc4c1f..dc416f9 100644
>> --- a/xen/arch/x86/hvm/vpmu.c
>> +++ b/xen/arch/x86/hvm/vpmu.c
>> @@ -88,7 +88,8 @@ int vpmu_do_wrmsr(unsigned int msr, uint64_t
>> msr_content)
>>   {
>>       struct vpmu_struct *vpmu = vcpu_vpmu(current);
>>
>> -    if ( !(vpmu_mode & XENPMU_MODE_ON) )
>> +    if ( (vpmu_mode == XENPMU_MODE_OFF) ||
>> +         ((vpmu_mode & XENPMU_MODE_PRIV)
>> && !is_control_domain(current->domain)) )
>>           return 0;
>>
> how about combining above checks into a macro, e.g.
>
> #define vpmu_is_on(mode, vcpu)			\
> 	((mode == XENPMU_MODE_ON) ||	\
> 	((mode & XENPMU_MODE_PRIV) && is_control_domain(vcpu->domain)))

I am not sure it's worth it for just two appearances of this code 
fragment. I'll see if this is more common though.


-boris

>
> Acked-by: Kevin Tian <kevin.tian@intel.com>