From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v7 04/19] x86/VPMU: Make vpmu marcos a bit more efficient Date: Fri, 6 Jun 2014 19:13:26 +0100 Message-ID: <539204C6.9000009@citrix.com> References: <1402076415-26475-1-git-send-email-boris.ostrovsky@oracle.com> <1402076415-26475-5-git-send-email-boris.ostrovsky@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1402076415-26475-5-git-send-email-boris.ostrovsky@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Boris Ostrovsky Cc: kevin.tian@intel.com, keir@xen.org, JBeulich@suse.com, jun.nakajima@intel.com, tim@xen.org, dietmar.hahn@ts.fujitsu.com, xen-devel@lists.xen.org, suravee.suthikulpanit@amd.com List-Id: xen-devel@lists.xenproject.org On 06/06/14 18:40, Boris Ostrovsky wrote: > Update macros that modify VPMU flags to allow changing multiple bits at once. Modify how? It appears to only be an introduction of "vcpu_are_all_set()". > > Signed-off-by: Boris Ostrovsky > Acked-by: Kevin Tian > Reviewed-by: Konrad Rzeszutek Wilk > Reviewed-by: Dietmar Hahn > Tested-by: Dietmar Hahn > --- > xen/arch/x86/hvm/vmx/vpmu_core2.c | 5 +---- > xen/arch/x86/hvm/vpmu.c | 3 +-- > xen/include/asm-x86/hvm/vpmu.h | 9 +++++---- > 3 files changed, 7 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c b/xen/arch/x86/hvm/vmx/vpmu_core2.c > index f5f249f..bf3cd33 100644 > --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c > +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c > @@ -326,10 +326,7 @@ static int core2_vpmu_save(struct vcpu *v) > { > struct vpmu_struct *vpmu = vcpu_vpmu(v); > > - if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_SAVE) ) > - return 0; > - > - if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) ) > + if ( !vpmu_are_all_set(vpmu, VPMU_CONTEXT_SAVE | VPMU_CONTEXT_LOADED) ) > return 0; > > __core2_vpmu_save(v); > diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c > index 63765fa..d456aa6 100644 > --- a/xen/arch/x86/hvm/vpmu.c > +++ b/xen/arch/x86/hvm/vpmu.c > @@ -143,8 +143,7 @@ void vpmu_save(struct vcpu *v) > struct vpmu_struct *vpmu = vcpu_vpmu(v); > int pcpu = smp_processor_id(); > > - if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) && > - vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)) ) > + if ( !vpmu_are_all_set(vpmu, VPMU_CONTEXT_ALLOCATED | VPMU_CONTEXT_LOADED) ) > return; > > vpmu->last_pcpu = pcpu; > diff --git a/xen/include/asm-x86/hvm/vpmu.h b/xen/include/asm-x86/hvm/vpmu.h > index 40f63fb..7cd7266 100644 > --- a/xen/include/asm-x86/hvm/vpmu.h > +++ b/xen/include/asm-x86/hvm/vpmu.h > @@ -81,10 +81,11 @@ struct vpmu_struct { > #define VPMU_CPU_HAS_BTS 0x200 /* Has Branch Trace Store */ > > > -#define vpmu_set(_vpmu, _x) ((_vpmu)->flags |= (_x)) > -#define vpmu_reset(_vpmu, _x) ((_vpmu)->flags &= ~(_x)) > -#define vpmu_is_set(_vpmu, _x) ((_vpmu)->flags & (_x)) > -#define vpmu_clear(_vpmu) ((_vpmu)->flags = 0) > +#define vpmu_set(_vpmu, _x) ((_vpmu)->flags |= (_x)) > +#define vpmu_reset(_vpmu, _x) ((_vpmu)->flags &= ~(_x)) > +#define vpmu_is_set(_vpmu, _x) ((_vpmu)->flags & (_x)) > +#define vpmu_are_all_set(_vpmu, _x) (((_vpmu)->flags & (_x)) == (_x)) > +#define vpmu_clear(_vpmu) ((_vpmu)->flags = 0) These macros' implicit types don't quite match their implementation. set, reset and clear are implicitly void. (and frankly clear and reset are confusing given the other bitopt nomenclature, but I don't suggest changing them). is_set and are_all_set are implicitly bool, but don't have !!'s I realise I am straying vastly into personal preference here, but I feel these would be much nicer as static inlines with proper types, and const correctness for {is,are_all}_set(). ~Andrew > > int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content); > int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content);