| Message ID | 1486952997-19445-3-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/02/17 02:29, Boris Ostrovsky wrote: > vpmu_count should be decremented even if VPMU_CONTEXT_ALLOCATED > is not set because on Intel processors the context is allocated > lazily and, in fact, might never happen. > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> The code in vpmu_initialise() already subtracts 1 from the vpmu_count in the Intel case. Won't this now cause an underflow when shutting down a VM which didn't enable vpmu to start with? ~Andrew
On 02/13/2017 05:38 AM, Andrew Cooper wrote: > On 13/02/17 02:29, Boris Ostrovsky wrote: >> vpmu_count should be decremented even if VPMU_CONTEXT_ALLOCATED >> is not set because on Intel processors the context is allocated >> lazily and, in fact, might never happen. >> >> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > > The code in vpmu_initialise() already subtracts 1 from the vpmu_count in > the Intel case. > > Won't this now cause an underflow when shutting down a VM which didn't > enable vpmu to start with? Right. I think the comment about Intel always needing to initialize VPMU ops is no longer true so we should only be decrementing the count on error. But then we'll still need to know whether or not to decrement it in vpmu_destroy(). How about #define vpmu_enabled(v) !!vcpu_vpmu(v)->arch_vpmu_ops and drop the first patch in the series? I'll add a comment in each vendor's vpmu_initialize() that assignment of arch_vpmu_ops should be the last thing? -boris
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c index 0252171..9fa8a18 100644 --- a/xen/arch/x86/cpu/vpmu.c +++ b/xen/arch/x86/cpu/vpmu.c @@ -536,6 +536,11 @@ void vpmu_destroy(struct vcpu *v) { struct vpmu_struct *vpmu = vcpu_vpmu(v); + spin_lock(&vpmu_lock); + if ( !is_hardware_domain(v->domain) ) + vpmu_count--; + spin_unlock(&vpmu_lock); + if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) ) return; @@ -557,11 +562,6 @@ void vpmu_destroy(struct vcpu *v) vpmu_save_force, v, 1); vpmu->arch_vpmu_ops->arch_vpmu_destroy(v); } - - spin_lock(&vpmu_lock); - if ( !is_hardware_domain(v->domain) ) - vpmu_count--; - spin_unlock(&vpmu_lock); } static int pvpmu_init(struct domain *d, xen_pmu_params_t *params)
vpmu_count should be decremented even if VPMU_CONTEXT_ALLOCATED is not set because on Intel processors the context is allocated lazily and, in fact, might never happen. Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> --- xen/arch/x86/cpu/vpmu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)