From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH 2/3 v2] vpmu intel: Use PMU defines instead of numerals and bit masks Date: Fri, 5 Apr 2013 09:31:00 -0400 Message-ID: <20130405133100.GB20093@phenom.dumpdata.com> References: <1452239.A5Lgmt1kI0@amur> <20079614.5xvBTPi963@amur> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20079614.5xvBTPi963@amur> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dietmar Hahn Cc: Eddie Dong , Jun Nakajima , suravee.suthikulpanit@amd.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Thu, Apr 04, 2013 at 02:10:18PM +0200, Dietmar Hahn wrote: > Am Donnerstag 28 M=E4rz 2013, 13:57:50 schrieb Dietmar Hahn: > > This patch uses the new defines in Intel vPMU to replace existing numer= als and > > bit masks. > > Thanks, > > Dietmar. > > = > > Signed-off-by: Dietmar Hahn > = > Ping? Reviewed-by: Konrad Rzeszutek Wilk > = > > Changes from v1: As Konrad suggested the names of the defines are chang= ed too > > and the readability is much better now. > > = > > = > > --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c > > +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c > > @@ -39,6 +39,32 @@ > > #include > > = > > /* > > + * See Intel SDM Vol 2a Instruction Set Reference chapter 3 for CPUID > > + * instruction. > > + * cpuid 0xa - Architectural Performance Monitoring Leaf > > + * Register eax > > + */ > > +#define PMU_VERSION_SHIFT 0 /* Version ID */ > > +#define PMU_VERSION_BITS 8 /* 8 bits 0..7 */ > > +#define PMU_VERSION_MASK (((1 << PMU_VERSION_BITS) - 1) << PMU= _VERSION_SHIFT) > > + > > +#define PMU_GENERAL_NR_SHIFT 8 /* Number of general pmu registers= */ > > +#define PMU_GENERAL_NR_BITS 8 /* 8 bits 8..15 */ > > +#define PMU_GENERAL_NR_MASK (((1 << PMU_GENERAL_NR_BITS) - 1) << = PMU_GENERAL_NR_SHIFT) > > + > > +#define PMU_GENERAL_WIDTH_SHIFT 16 /* Width of general pmu registers = */ > > +#define PMU_GENERAL_WIDTH_BITS 8 /* 8 bits 16..23 */ > > +#define PMU_GENERAL_WIDTH_MASK (((1 << PMU_GENERAL_WIDTH_BITS) - 1) <= < PMU_GENERAL_WIDTH_SHIFT) > > +/* Register edx */ > > +#define PMU_FIXED_NR_SHIFT 0 /* Number of fixed pmu registers */ > > +#define PMU_FIXED_NR_BITS 5 /* 5 bits 0..4 */ > > +#define PMU_FIXED_NR_MASK (((1 << PMU_FIXED_NR_BITS) -1) << PMU= _FIXED_NR_SHIFT) > > + > > +#define PMU_FIXED_WIDTH_SHIFT 5 /* Width of fixed pmu registers */ > > +#define PMU_FIXED_WIDTH_BITS 8 /* 8 bits 5..12 */ > > +#define PMU_FIXED_WIDTH_MASK (((1 << PMU_FIXED_WIDTH_BITS) -1) << = PMU_FIXED_WIDTH_SHIFT) > > + > > +/* > > * QUIRK to workaround an issue on Nehalem processors currently seen > > * on family 6 cpus E5520 (model 26) and X7542 (model 46). > > * The issue leads to endless PMC interrupt loops on the processor. > > @@ -130,6 +156,9 @@ static const struct pmumsr core2_ctrls =3D > > }; > > static int arch_pmc_cnt; > > = > > +/* > > + * Read the number of general counters via CPUID.EAX[0xa].EAX[8..15] > > + */ > > static int core2_get_pmc_count(void) > > { > > u32 eax, ebx, ecx, edx; > > @@ -137,7 +166,7 @@ static int core2_get_pmc_count(void) > > if ( arch_pmc_cnt =3D=3D 0 ) > > { > > cpuid(0xa, &eax, &ebx, &ecx, &edx); > > - arch_pmc_cnt =3D (eax & 0xff00) >> 8; > > + arch_pmc_cnt =3D (eax & PMU_GENERAL_NR_MASK) >> PMU_GENERAL_NR= _SHIFT; > > } > > = > > return arch_pmc_cnt; > > @@ -154,8 +183,9 @@ static u64 core2_calc_intial_glb_ctrl_ms > > static int core2_get_bitwidth_fix_count(void) > > { > > u32 eax, ebx, ecx, edx; > > + > > cpuid(0xa, &eax, &ebx, &ecx, &edx); > > - return ((edx & 0x1fe0) >> 5); > > + return ((edx & PMU_FIXED_WIDTH_MASK) >> PMU_FIXED_WIDTH_SHIFT); > > } > > = > > static int is_core2_vpmu_msr(u32 msr_index, int *type, int *index) > > @@ -731,23 +761,6 @@ struct arch_vpmu_ops core2_vpmu_ops =3D { > > .arch_vpmu_load =3D core2_vpmu_load > > }; > > = > > -/* > > - * See Intel SDM Vol 2a Instruction Set Referenc for CPUID instruction. > > - * cpuid 0xa - Architectural Performance Monitoring Leaf > > - * 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 register= s */ > > -#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 */ > > - > > static void core2_no_vpmu_do_cpuid(unsigned int input, > > unsigned int *eax, unsigned int *ebx, > > unsigned int *ecx, unsigned int *edx) > > @@ -758,12 +771,12 @@ static void core2_no_vpmu_do_cpuid(unsig > > */ > > if ( input =3D=3D 0xa ) > > { > > - *eax &=3D ~(((1 << FEATURE_PMU_VER_BITS) -1) << X86_FEATURE_PM= U_VER_OFF); > > - *eax &=3D ~(((1 << FEATURE_NUM_GEN_BITS) -1) << X86_FEATURE_NU= M_GEN_OFF); > > - *eax &=3D ~(((1 << FEATURE_GEN_WIDTH_BITS) -1) << X86_FEATURE_= GEN_WIDTH_OFF); > > + *eax &=3D ~PMU_VERSION_MASK; > > + *eax &=3D ~PMU_GENERAL_NR_MASK; > > + *eax &=3D ~PMU_GENERAL_WIDTH_MASK; > > = > > - *edx &=3D ~(((1 << FEATURE_NUM_FIX_BITS) -1) << X86_FEATURE_NU= M_FIX_OFF); > > - *edx &=3D ~(((1 << FEATURE_FIX_WIDTH_BITS) -1) << X86_FEATURE_= FIX_WIDTH_OFF); > > + *edx &=3D ~PMU_FIXED_NR_MASK; > > + *edx &=3D ~PMU_FIXED_WIDTH_MASK; > > } > > } > > = > > = > > = > -- = > Company details: http://ts.fujitsu.com/imprint.html