From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dietmar Hahn Subject: Re: [PATCH] Intel/VPMU: Add support for full-width PMC writes Date: Mon, 29 Jul 2013 11:56:56 +0200 Message-ID: <1448815.0nKLeCxeCq@amur> References: <1374519271-1400-1-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: <1374519271-1400-1-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: xen-devel@lists.xen.org Cc: Boris Ostrovsky , eddie.dong@intel.com, jun.nakajima@intel.com List-Id: xen-devel@lists.xenproject.org Am Montag 22 Juli 2013, 14:54:31 schrieb Boris Ostrovsky: > A recent Linux commit (069e0c3c405814778c7475d95b9fff5318f39834) added > support for full-width PMC writes to performance counter registers, > making these registers default for perf. Since current Xen VPMU does > not support these new MSRs perf will fail to initialise in guests. > > Signed-off-by: Boris Ostrovsky > > Question to Intel folks: Do we need to update MSR bitmap for both original > (0xc1) and alias (0x4c1) registers or will either one suffice? > > --- > xen/arch/x86/hvm/vmx/vpmu_core2.c | 44 +++++++++++++++++++++++++++++++++++---- > xen/include/asm-x86/msr-index.h | 1 + > 2 files changed, 41 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c b/xen/arch/x86/hvm/vmx/vpmu_core2.c > index 15b2036..823d364 100644 > --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c > +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c > @@ -64,6 +64,10 @@ > #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) > > +/* Alias registers (0x4c1) for full-width writes to PMCs */ > +#define MSR_PMC_ALIAS_MASK (~0x400) > +bool_t __read_mostly fw_write; Why not use "full_width_write" (or similar) like in the original linux kernel patch for better readability? > + > /* > * QUIRK to workaround an issue on various family 6 cpus. > * The issue leads to endless PMC interrupt loops on the processor. > @@ -195,6 +199,7 @@ static int core2_get_bitwidth_fix_count(void) > static int is_core2_vpmu_msr(u32 msr_index, int *type, int *index) > { > int i; > + u32 msr_index_pmc; > > for ( i = 0; i < core2_fix_counters.num; i++ ) > { > @@ -224,11 +229,12 @@ static int is_core2_vpmu_msr(u32 msr_index, int *type, int *index) > return 1; > } > > - if ( (msr_index >= MSR_IA32_PERFCTR0) && > - (msr_index < (MSR_IA32_PERFCTR0 + core2_get_pmc_count())) ) > + msr_index_pmc = msr_index & MSR_PMC_ALIAS_MASK; > + if ( (msr_index_pmc >= MSR_IA32_PERFCTR0) && > + (msr_index_pmc < (MSR_IA32_PERFCTR0 + core2_get_pmc_count())) ) > { > *type = MSR_TYPE_ARCH_COUNTER; > - *index = msr_index - MSR_IA32_PERFCTR0; > + *index = msr_index_pmc - MSR_IA32_PERFCTR0; > return 1; > } > > @@ -260,6 +266,15 @@ static void core2_vpmu_set_msr_bitmap(unsigned long *msr_bitmap) > clear_bit(msraddr_to_bitpos(MSR_IA32_PERFCTR0+i), > msr_bitmap + 0x800/BYTES_PER_LONG); > } > + if ( fw_write ) > + { > + for ( i = 0; i < core2_get_pmc_count(); i++ ) > + { > + clear_bit(msraddr_to_bitpos(MSR_IA32_A_PERFCTR0+i), msr_bitmap); > + clear_bit(msraddr_to_bitpos(MSR_IA32_A_PERFCTR0+i), > + msr_bitmap + 0x800/BYTES_PER_LONG); > + } > + } > > /* Allow Read PMU Non-global Controls Directly. */ > for ( i = 0; i < core2_ctrls.num; i++ ) > @@ -284,6 +299,16 @@ static void core2_vpmu_unset_msr_bitmap(unsigned long *msr_bitmap) > set_bit(msraddr_to_bitpos(MSR_IA32_PERFCTR0+i), > msr_bitmap + 0x800/BYTES_PER_LONG); > } > + if ( fw_write ) > + { > + for ( i = 0; i < core2_get_pmc_count(); i++ ) > + { > + set_bit(msraddr_to_bitpos(MSR_IA32_A_PERFCTR0+i), msr_bitmap); > + set_bit(msraddr_to_bitpos(MSR_IA32_A_PERFCTR0+i), > + msr_bitmap + 0x800/BYTES_PER_LONG); > + } > + } > + > for ( i = 0; i < core2_ctrls.num; i++ ) > set_bit(msraddr_to_bitpos(core2_ctrls.msr[i]), msr_bitmap); > for ( i = 0; i < core2_get_pmc_count(); i++ ) > @@ -324,11 +349,17 @@ static inline void __core2_vpmu_load(struct vcpu *v) > { > int i; > struct core2_vpmu_context *core2_vpmu_cxt = vcpu_vpmu(v)->context; > + int pmc_start; > > for ( i = 0; i < core2_fix_counters.num; i++ ) > wrmsrl(core2_fix_counters.msr[i], core2_vpmu_cxt->fix_counters[i]); > + Leading spaces. Reviewed-by: Dietmar Hahn > + if ( fw_write ) > + pmc_start = MSR_IA32_A_PERFCTR0; > + else > + pmc_start = MSR_IA32_PERFCTR0; > for ( i = 0; i < core2_get_pmc_count(); i++ ) > - wrmsrl(MSR_IA32_PERFCTR0+i, core2_vpmu_cxt->arch_msr_pair[i].counter); > + wrmsrl(pmc_start+i, core2_vpmu_cxt->arch_msr_pair[i].counter); > > for ( i = 0; i < core2_ctrls.num; i++ ) > wrmsrl(core2_ctrls.msr[i], core2_vpmu_cxt->ctrls[i]); > @@ -855,6 +886,11 @@ int vmx_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags) > > if ( family == 6 ) > { > + u64 caps; > + > + rdmsrl(MSR_IA32_PERF_CAPABILITIES, caps); > + fw_write = (caps >> 13) & 1; > + > switch ( cpu_model ) > { > /* Core2: */ > diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h > index f500efd..5acaee8 100644 > --- a/xen/include/asm-x86/msr-index.h > +++ b/xen/include/asm-x86/msr-index.h > @@ -34,6 +34,7 @@ > /* Intel MSRs. Some also available on other CPUs */ > #define MSR_IA32_PERFCTR0 0x000000c1 > #define MSR_IA32_PERFCTR1 0x000000c2 > +#define MSR_IA32_A_PERFCTR0 0x000004c1 > #define MSR_FSB_FREQ 0x000000cd > > #define MSR_NHM_SNB_PKG_CST_CFG_CTL 0x000000e2 > -- Company details: http://ts.fujitsu.com/imprint.html