Re: [Xen-devel] [PATCH 2/3] vpmu intel: Use PMU defines instead of numerals and bit masks

[Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>]

Am Mittwoch 27 MÃrz 2013, 11:25:41 schrieb Konrad Rzeszutek Wilk:
> On Wed, Mar 27, 2013 at 03:13:13PM +0100, Dietmar Hahn wrote:

> >  
> >  /*
> > + * See Intel SDM Vol 2a Instruction Set Referenc for CPUID instruction.
> > + * cpuid 0xa - Architectural Performance Monitoring Leaf
> 
> Chapter ?

OK.

> 
> > + * Register eax
> > + */
> > +#define X86_FEATURE_PMU_VER_OFF   0  /* Version ID */
> > +#define FEATURE_PMU_VER_BITS      8  /* 8 bits 0..7 */
> > +#define X86_FEATURE_NUM_GEN_OFF   8  /* Number of general pmu registers */
> > +#define FEATURE_NUM_GEN_BITS      8  /* 8 bits 8..15 */
> > +#define X86_FEATURE_GEN_WIDTH_OFF 16 /* Width of general pmu registers */
> > +#define FEATURE_GEN_WIDTH_BITS    8  /* 8 bits 16..23 */
> > +/* Register edx */
> > +#define X86_FEATURE_NUM_FIX_OFF   0  /* Number of fixed pmu registers */
> > +#define FEATURE_NUM_FIX_BITS      5  /* 5 bits 0..4 */
> > +#define X86_FEATURE_FIX_WIDTH_OFF 5  /* Width of fixed pmu registers */
> > +#define FEATURE_FIX_WIDTH_BITS    8  /* 8 bits 5..12 */
> 
> I think the 'X86_FEATURE' and 'FEATURE' could be just chopped off?
> Then it is 'PMU_VER_BITS' or 'NUM_GEN_OFF'.  Perhaps also have the
> mask in there?
> 
> That way you can could do:
> 
> PMU_VERSION_SHIFT
> PMU_VERSION_MASK
> 
> And just use those in the logic below.
> 
> And for the other:
> 
> PMU_GENERAL_NR_SHIFT
> PMU_GENERAL_NR_MASK
> and
> PMU_FIXED_NR_SHIFT
> PMU_FIXED_NR_MASK?
> 
> PMU_FIXED_WIDTH_SHIFT
> ... and so on.

Yes, this is much better. Will send another version.
Thanks.

Dietmar


-- 
Company details: http://ts.fujitsu.com/imprint.html

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel