From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH v7 04/19] x86/VPMU: Make vpmu marcos a bit
 more efficient
Date: Fri, 06 Jun 2014 15:28:33 -0400
Message-ID: <53921661.6010103@oracle.com>
References: <1402076415-26475-1-git-send-email-boris.ostrovsky@oracle.com>
	<1402076415-26475-5-git-send-email-boris.ostrovsky@oracle.com>
	<539204C6.9000009@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <539204C6.9000009@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Andrew Cooper <andrew.cooper3@citrix.com>
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/2014 02:13 PM, Andrew Cooper wrote:
> 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()".

I meant "vpmu_is_set(foo) + vpmu_is_set(bar)" ==> vpmu_are_all_set(foo|bar).

So probably "Introduce macro that allows testing multiple bits at one".

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

I don't have a preference here. If there is no difference binary-wise 
(code size) between the two I can change these to inlines.

-boris