[Xen-devel] [PATCH v3] x86/VPMU: Clear last_vcpu when destroying VPMU

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

We need to make sure that last_vcpu is not pointing to VCPU whose
VPMU is being destroyed. Otherwise we may try to dereference it in
the future, when VCPU is gone.

We have to do this atomically since otherwise there is a (somewhat
theoretical) chance that between test and subsequent clearing
of last_vcpu the remote processor (i.e. vpmu->last_pcpu) might do
both vpmu_load() and then vpmu_save() for another VCPU. The former
will clear last_vcpu and the latter will set it to something else.

We should also check for VPMU_CONTEXT_ALLOCATED in vpmu_destroy() to
avoid unnecessary cmpxchg() and arch-specific destroy ops. Thus
checks in AMD and Intel routines are no longer needed.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
---
 xen/arch/x86/hvm/svm/vpmu.c       |    3 ---
 xen/arch/x86/hvm/vmx/vpmu_core2.c |    2 --
 xen/arch/x86/hvm/vpmu.c           |    7 +++++++
 3 files changed, 7 insertions(+), 5 deletions(-)

Changes in v3:
 * Use cmpxchg instead of IPI
 * Use correct routine nemas in commit message
 * Remove duplicate test for VPMU_CONTEXT_ALLOCATED in arch-specific
   destroy ops

Changes in v2:
 * Test last_vcpu locally before IPI
 * Don't handle local pcpu as a special case --- on_selected_cpus
   will take care of that
 * Dont't cast variables unnecessarily

diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
index 8e07a98..4c448bb 100644
--- a/xen/arch/x86/hvm/svm/vpmu.c
+++ b/xen/arch/x86/hvm/svm/vpmu.c
@@ -403,9 +403,6 @@ static void amd_vpmu_destroy(struct vcpu *v)
 {
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
 
-    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
-        return;
-
     if ( ((struct amd_vpmu_context *)vpmu->context)->msr_bitmap_set )
         amd_vpmu_unset_msr_bitmap(v);
 
diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c 
b/xen/arch/x86/hvm/vmx/vpmu_core2.c
index 68b6272..590c2a9 100644
--- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
+++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
@@ -818,8 +818,6 @@ static void core2_vpmu_destroy(struct vcpu *v)
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
     struct core2_vpmu_context *core2_vpmu_cxt = vpmu->context;
 
-    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
-        return;
     xfree(core2_vpmu_cxt->pmu_enable);
     xfree(vpmu->context);
     if ( cpu_has_vmx_msr_bitmap )
diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
index 1df74c2..7cc95ae 100644
--- a/xen/arch/x86/hvm/vpmu.c
+++ b/xen/arch/x86/hvm/vpmu.c
@@ -250,6 +250,13 @@ void vpmu_initialise(struct vcpu *v)
 void vpmu_destroy(struct vcpu *v)
 {
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
+    struct vcpu **last = &per_cpu(last_vcpu, vpmu->last_pcpu);
+
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
+        return;
+
+    /* Need to clear last_vcpu in case it points to v */
+    (void)cmpxchg(last, v, NULL);
 
     if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_destroy )
         vpmu->arch_vpmu_ops->arch_vpmu_destroy(v);
-- 
1.7.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel