From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH] Always save/restore performance counters when HVM guest switching VCPU Date: Fri, 01 Mar 2013 18:02:50 -0500 Message-ID: <5131339A.2070009@oracle.com> References: <1362170975-2884-1-git-send-email-suravee.suthikulpanit@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1362170975-2884-1-git-send-email-suravee.suthikulpanit@amd.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: suravee.suthikulpanit@amd.com Cc: JBeulich@suse.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 03/01/2013 03:49 PM, suravee.suthikulpanit@amd.com wrote: > From: Suravee Suthikulpanit > > Currently, the performance counter registers are saved/restores > when the HVM guest switchs VCPUs only if they are running. > However, PERF has one check where it writes the MSR and read back > the value to check if the MSR is working. This has shown to fails > the check if the VCPU is moved in between rdmsr and wrmsr and > resulting in the values are different. > > Signed-off-by: Suravee Suthikulpanit Description may need to be cleaned up a bit but other than that Acked-by: Boris Ostrovsky > --- > xen/arch/x86/hvm/svm/vpmu.c | 62 ++++++++++++++++++++++++------------------- > 1 file changed, 35 insertions(+), 27 deletions(-) > > diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c > index bf186fe..4854cf3 100644 > --- a/xen/arch/x86/hvm/svm/vpmu.c > +++ b/xen/arch/x86/hvm/svm/vpmu.c > @@ -172,12 +172,16 @@ static inline void context_restore(struct vcpu *v) > { > wrmsrl(counters[i], ctxt->counters[i]); > > + if ( !vpmu_is_set(vpmu, VPMU_RUNNING) ) > + continue; > + > /* Force an interrupt to allow guest reset the counter, > if the value is positive */ > if ( is_overflowed(ctxt->counters[i]) && (ctxt->counters[i] > 0) ) > { > gdprintk(XENLOG_WARNING, "VPMU: Force a performance counter " > - "overflow interrupt!\n"); > + "overflow interrupt! (counter:%u value:0x%lx)\n", > + i, ctxt->counters[i]); > amd_vpmu_do_interrupt(0); > } > } > @@ -188,12 +192,13 @@ static void amd_vpmu_restore(struct vcpu *v) > struct vpmu_struct *vpmu = vcpu_vpmu(v); > struct amd_vpmu_context *ctxt = vpmu->context; > > - if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) && > - vpmu_is_set(vpmu, VPMU_RUNNING)) ) > + if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED)) ) > return; > > context_restore(v); > - apic_write(APIC_LVTPC, ctxt->hw_lapic_lvtpc); > + > + if ( vpmu_is_set(vpmu, VPMU_RUNNING) ) > + apic_write(APIC_LVTPC, ctxt->hw_lapic_lvtpc); > } > > static inline void context_save(struct vcpu *v) > @@ -214,13 +219,16 @@ static void amd_vpmu_save(struct vcpu *v) > struct vpmu_struct *vpmu = vcpu_vpmu(v); > struct amd_vpmu_context *ctx = vpmu->context; > > - if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) && > - vpmu_is_set(vpmu, VPMU_RUNNING)) ) > + if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED)) ) > return; > > context_save(v); > - ctx->hw_lapic_lvtpc = apic_read(APIC_LVTPC); > - apic_write(APIC_LVTPC, ctx->hw_lapic_lvtpc | APIC_LVT_MASKED); > + > + if ( vpmu_is_set(vpmu, VPMU_RUNNING) ) > + { > + ctx->hw_lapic_lvtpc = apic_read(APIC_LVTPC); > + apic_write(APIC_LVTPC, ctx->hw_lapic_lvtpc | APIC_LVT_MASKED); > + } > } > > static void context_update(unsigned int msr, u64 msr_content) > @@ -303,25 +311,25 @@ static int amd_vpmu_initialise(struct vcpu *v) > > if ( counters == NULL ) > { > - switch ( family ) > - { > - case 0x15: > - num_counters = F15H_NUM_COUNTERS; > - counters = AMD_F15H_COUNTERS; > - ctrls = AMD_F15H_CTRLS; > - k7_counters_mirrored = 1; > - break; > - case 0x10: > - case 0x12: > - case 0x14: > - case 0x16: > - default: > - num_counters = F10H_NUM_COUNTERS; > - counters = AMD_F10H_COUNTERS; > - ctrls = AMD_F10H_CTRLS; > - k7_counters_mirrored = 0; > - break; > - } > + switch ( family ) > + { > + case 0x15: > + num_counters = F15H_NUM_COUNTERS; > + counters = AMD_F15H_COUNTERS; > + ctrls = AMD_F15H_CTRLS; > + k7_counters_mirrored = 1; > + break; > + case 0x10: > + case 0x12: > + case 0x14: > + case 0x16: > + default: > + num_counters = F10H_NUM_COUNTERS; > + counters = AMD_F10H_COUNTERS; > + ctrls = AMD_F10H_CTRLS; > + k7_counters_mirrored = 0; > + break; > + } > } > > ctxt = xzalloc(struct amd_vpmu_context);