From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tian, Kevin" Subject: Re: [PATCH v10 16/20] x86/VPMU: Merge vpmu_rdmsr and vpmu_wrmsr Date: Thu, 18 Sep 2014 04:16:36 +0000 Message-ID: References: <1409802080-6160-1-git-send-email-boris.ostrovsky@oracle.com> <1409802080-6160-17-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: <1409802080-6160-17-git-send-email-boris.ostrovsky@oracle.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Boris Ostrovsky , "jbeulich@suse.com" , "suravee.suthikulpanit@amd.com" , "Dong, Eddie" , "Aravind.Gopalakrishnan@amd.com" Cc: "andrew.cooper3@citrix.com" , "xen-devel@lists.xen.org" , "keir@xen.org" , "Nakajima, Jun" , "tim@xen.org" List-Id: xen-devel@lists.xenproject.org > From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com] > Sent: Wednesday, September 03, 2014 8:41 PM > > The two routines share most of their logic. > > Signed-off-by: Boris Ostrovsky Acked-by: Kevin Tian > --- > xen/arch/x86/hvm/svm/svm.c | 4 +-- > xen/arch/x86/hvm/vmx/vmx.c | 6 ++-- > xen/arch/x86/hvm/vpmu.c | 71 > +++++++++++++++++++----------------------- > xen/arch/x86/traps.c | 6 ++-- > xen/include/asm-x86/hvm/vpmu.h | 7 +++-- > 5 files changed, 46 insertions(+), 48 deletions(-) > > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 326cad9..5174a82 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -1654,7 +1654,7 @@ static int svm_msr_read_intercept(unsigned int msr, > uint64_t *msr_content) > case MSR_AMD_FAM15H_EVNTSEL3: > case MSR_AMD_FAM15H_EVNTSEL4: > case MSR_AMD_FAM15H_EVNTSEL5: > - if ( vpmu_do_rdmsr(msr, msr_content) ) > + if ( vpmu_do_msr(msr, msr_content, 0, VPMU_MSR_READ) ) > goto gpf; > break; > > @@ -1806,7 +1806,7 @@ static int svm_msr_write_intercept(unsigned int msr, > uint64_t msr_content) > case MSR_AMD_FAM15H_EVNTSEL3: > case MSR_AMD_FAM15H_EVNTSEL4: > case MSR_AMD_FAM15H_EVNTSEL5: > - if ( vpmu_do_wrmsr(msr, msr_content, 0) ) > + if ( vpmu_do_msr(msr, &msr_content, 0, VPMU_MSR_WRITE) ) > goto gpf; > break; > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 3c63bb0..f4064ba 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -2083,7 +2083,7 @@ static int vmx_msr_read_intercept(unsigned int msr, > uint64_t *msr_content) > case > MSR_CORE_PERF_FIXED_CTR_CTRL...MSR_CORE_PERF_GLOBAL_OVF_CTRL: > case MSR_IA32_PEBS_ENABLE: > case MSR_IA32_DS_AREA: > - if ( vpmu_do_rdmsr(msr, msr_content) ) > + if ( vpmu_do_msr(msr, msr_content, 0, VPMU_MSR_READ) ) > goto gp_fault; > break; > default: > @@ -2259,7 +2259,7 @@ static int vmx_msr_write_intercept(unsigned int > msr, uint64_t msr_content) > if ( msr_content & ~supported ) > { > /* Perhaps some other bits are supported in vpmu. */ > - if ( vpmu_do_wrmsr(msr, msr_content, supported) ) > + if ( vpmu_do_msr(msr, &msr_content, supported, > VPMU_MSR_WRITE) ) > break; > } > if ( msr_content & IA32_DEBUGCTLMSR_LBR ) > @@ -2293,7 +2293,7 @@ static int vmx_msr_write_intercept(unsigned int > msr, uint64_t msr_content) > case > MSR_CORE_PERF_FIXED_CTR_CTRL...MSR_CORE_PERF_GLOBAL_OVF_CTRL: > case MSR_IA32_PEBS_ENABLE: > case MSR_IA32_DS_AREA: > - if ( vpmu_do_wrmsr(msr, msr_content, 0) ) > + if ( vpmu_do_msr(msr, &msr_content, 0, VPMU_MSR_WRITE) ) > goto gp_fault; > break; > default: > diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c > index edc81be..35a08d2 100644 > --- a/xen/arch/x86/hvm/vpmu.c > +++ b/xen/arch/x86/hvm/vpmu.c > @@ -89,57 +89,50 @@ void vpmu_lvtpc_update(uint32_t val) > apic_write(APIC_LVTPC, vpmu->hw_lapic_lvtpc); > } > > -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t > supported) > +int vpmu_do_msr(unsigned int msr, uint64_t *msr_content, > + uint64_t supported, int rw) > { > struct vcpu *curr = current; > struct vpmu_struct *vpmu = vcpu_vpmu(curr); > + struct arch_vpmu_ops *ops = vpmu->arch_vpmu_ops; > + int ret = 0; > > if ( !(vpmu_mode & XENPMU_MODE_SELF) ) > return 0; > > - if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr ) > + switch ( rw ) > { > - int ret = vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, > supported); > - > - /* > - * We may have received a PMU interrupt during WRMSR handling > - * and since do_wrmsr may load VPMU context we should save > - * (and unload) it again. > - */ > - if ( !is_hvm_domain(curr->domain) && > - vpmu->xenpmu_data && (vpmu->xenpmu_data->pmu_flags > & PMU_CACHED) ) > - { > - vpmu_set(vpmu, VPMU_CONTEXT_SAVE); > - vpmu->arch_vpmu_ops->arch_vpmu_save(curr); > - vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | > VPMU_CONTEXT_LOADED); > - } > - return ret; > - } > - return 0; > -} > + case VPMU_MSR_READ: > + if ( !ops || !ops->do_rdmsr ) > + return 0; > + ret = ops->do_rdmsr(msr, msr_content); > + break; > > -int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content) > -{ > - struct vcpu *curr = current; > - struct vpmu_struct *vpmu = vcpu_vpmu(curr); > + case VPMU_MSR_WRITE: > + if ( !ops || !ops->do_wrmsr ) > + return 0; > + ret = ops->do_wrmsr(msr, *msr_content, supported); > + break; > > - if ( !(vpmu_mode & XENPMU_MODE_SELF) ) > + default: > + ASSERT(0); > return 0; > + } > > - if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_rdmsr ) > + /* > + * We may have received a PMU interrupt while handling MSR access > + * and since do_wr/rdmsr may load VPMU context we should save > + * (and unload) it again. > + */ > + if ( !is_hvm_domain(curr->domain) && > + vpmu->xenpmu_data && (vpmu->xenpmu_data->pmu_flags & > PMU_CACHED) ) > { > - int ret = vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content); > - > - if ( !is_hvm_domain(curr->domain) && > - vpmu->xenpmu_data && (vpmu->xenpmu_data->pmu_flags > & PMU_CACHED) ) > - { > - vpmu_set(vpmu, VPMU_CONTEXT_SAVE); > - vpmu->arch_vpmu_ops->arch_vpmu_save(curr); > - vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | > VPMU_CONTEXT_LOADED); > - } > - return ret; > + vpmu_set(vpmu, VPMU_CONTEXT_SAVE); > + ops->arch_vpmu_save(curr); > + vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | > VPMU_CONTEXT_LOADED); > } > - return 0; > + > + return ret; > } > > static struct vcpu *choose_hwdom_vcpu(void) > @@ -199,11 +192,11 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs) > vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | > VPMU_CONTEXT_LOADED); > > /* Store appropriate registers in xenpmu_data */ > - if ( is_pv_32bit_domain(sampled->domain) ) > + if ( is_pv_32bit_domain(sampling->domain) ) > { > /* > * 32-bit dom0 cannot process Xen's addresses (which are 64 > bit) > - * and therefore we treat it the same way as a non-priviledged > + * and therefore we treat it the same way as a non-privileged > * PV 32-bit domain. > */ > struct compat_cpu_user_regs *cmp; > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > index bc0c9c3..106f522 100644 > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -2578,7 +2578,8 @@ static int emulate_privileged_op(struct > cpu_user_regs *regs) > case > MSR_AMD_FAM15H_EVNTSEL0...MSR_AMD_FAM15H_PERFCTR5: > if ( vpmu_msr || (boot_cpu_data.x86_vendor == > X86_VENDOR_AMD) ) > { > - if ( vpmu_do_wrmsr(regs->ecx, msr_content, 0) ) > + if ( vpmu_do_msr(regs->ecx, &msr_content, > + 0, VPMU_MSR_WRITE) ) > goto fail; > } > break; > @@ -2700,7 +2701,8 @@ static int emulate_privileged_op(struct > cpu_user_regs *regs) > case > MSR_AMD_FAM15H_EVNTSEL0...MSR_AMD_FAM15H_PERFCTR5: > if ( vpmu_msr || (boot_cpu_data.x86_vendor == > X86_VENDOR_AMD) ) > { > - if ( vpmu_do_rdmsr(regs->ecx, &msr_content) ) > + if ( vpmu_do_msr(regs->ecx, &msr_content, > + 0, VPMU_MSR_READ) ) > goto fail; > > regs->eax = (uint32_t)msr_content; > diff --git a/xen/include/asm-x86/hvm/vpmu.h > b/xen/include/asm-x86/hvm/vpmu.h > index a70b1a4..977a44c 100644 > --- a/xen/include/asm-x86/hvm/vpmu.h > +++ b/xen/include/asm-x86/hvm/vpmu.h > @@ -95,9 +95,12 @@ static inline bool_t vpmu_are_all_set(const struct > vpmu_struct *vpmu, > return !!((vpmu->flags & mask) == mask); > } > > +#define VPMU_MSR_READ 0 > +#define VPMU_MSR_WRITE 1 > + > void vpmu_lvtpc_update(uint32_t val); > -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t > supported); > -int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content); > +int vpmu_do_msr(unsigned int msr, uint64_t *msr_content, > + uint64_t supported, int rw); > int vpmu_do_interrupt(struct cpu_user_regs *regs); > void vpmu_do_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx, > unsigned int *ecx, unsigned > int *edx); > -- > 1.8.1.4