From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dietmar Hahn Subject: Re: [PATCH 2 of 2] vpmu: Add the BTS extension Date: Tue, 14 Feb 2012 13:59:54 +0100 Message-ID: <1805920.cM9rJVskbD@amur> References: <6587132.0KbvunDmjW@amur> <4F3A58DB0200007800072DE8@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4F3A58DB0200007800072DE8@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com Cc: Haitao Shan , Jan Beulich List-Id: xen-devel@lists.xenproject.org Am Dienstag 14 Februar 2012, 11:51:39 schrieb Jan Beulich: > >>> On 13.02.12 at 14:01, Dietmar Hahn wrote: > > @@ -401,7 +401,31 @@ static int core2_vpmu_do_wrmsr(unsigned > > struct core2_vpmu_context *core2_vpmu_cxt = NULL; > > > > if ( !core2_vpmu_msr_common_check(msr, &type, &index) ) > > + { > > + /* Special handling for BTS */ > > + if ( msr == MSR_IA32_DEBUGCTLMSR ) > > + { > > + uint64_t supported = IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS | > > + IA32_DEBUGCTLMSR_BTINT; > > Was the code to make BTINT work magically in place already? I can't > spot anything to the effect in the patch... No, BTINT wasn't handled before. The writing of the MSR's is done in the calling function vmx_msr_write_intercept() in xen/arch/x86/hvm/vmx/vmx.c. There I added the call of vpmu_do_wrmsr() in the case of MSR_IA32_DEBUGCTLMSR. If vpmu_do_wrmsr() returns 1 the MSR gets written in the line __vmwrite(GUEST_IA32_DEBUGCTL, msr_content); Maybe I can change this and write the MSR here in this function. > > > + > > + if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) ) > > + { > > + supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS | > > + IA32_DEBUGCTLMSR_BTS_OFF_USR; > > + } > > + if ( msr_content & supported ) > > + { > > + if ( !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) ) > > + { > > + gdprintk(XENLOG_WARNING, "Debug Store is not supported on this cpu\n"); > > + vmx_inject_hw_exception(TRAP_gp_fault, 0); > > + return 0; > > + } > > + return 1; > > + } > > + } > > return 0; > > + } > > > > core2_vpmu_cxt = vpmu->context; > > switch ( msr ) > > @@ -420,8 +444,26 @@ static int core2_vpmu_do_wrmsr(unsigned > > "which is not supported.\n"); > > return 1; > > case MSR_IA32_DS_AREA: > > - gdprintk(XENLOG_WARNING, "Guest setting of DTS is ignored.\n"); > > - return 1; > > + if ( vpmu_is_set(vpmu, VPMU_CPU_HAS_DS) ) > > + { > > + if (!msr_content || !is_canonical_address(msr_content)) > > + { > > + gdprintk(XENLOG_WARNING, "Illegal address for IA32_DS_AREA: 0x%lx\n", > > + msr_content); > > + vmx_inject_hw_exception(TRAP_gp_fault, 0); > > + return 1; > > + } > > + else > > + { > > + core2_vpmu_cxt->pmu_enable->ds_area_enable = msr_content ? 1 : 0; > > + break; > > How do you manage to get away without storing the value the guest > attempted to write? In the case of MSR_IA32_DS_AREA the value is stored some lines later core2_vpmu_save_msr_context(v, type, index, msr_content); in an internal data structure. The values of this structure are loaded - core2_vpmu_load() - and stored - core2_vpmu_save() - on context switch. Thanks. Dietmar. > > Jan > > > + } > > + } > > + else > > + { > > + gdprintk(XENLOG_WARNING, "Guest setting of DTS is ignored.\n"); > > + return 1; > > + } > > case MSR_CORE_PERF_GLOBAL_CTRL: > > global_ctrl = msr_content; > > for ( i = 0; i < core2_get_pmc_count(); i++ ) > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > -- Company details: http://ts.fujitsu.com/imprint.html