Re: [PATCH v7 04/19] x86/VPMU: Make vpmu marcos a bit more efficient

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

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