thr3ads.net - Xen devel - [PATCH v2 0/9] Various VPMU patches, version 2 [Apr 2013]

If this information is useful, please help other people find it:
Share via:

Boris Ostrovsky

2013-Apr-12 17:59 UTC

[PATCH v2 0/9] Various VPMU patches, version 2

Version 2 of VPMU patches.

Changes:
* Load context on MSR read on SVM and then read MSR from HW (Suravee''s
suggestion)
* Group VPMU state bits separate from feature bits (Dietmar''s
suggestion)
* Clean up (int->unsigned int for loop control, trailing spaces)
* Rename VPMU_STOPPED to VPMU_FROZEN (*slightly* more sensible since it is
possible to be both STOPPED and RUNNING)
* Add patch for dumping VPMU state on AMD
* Add Reviewed-by/Tested-by/Acked-by tags that have been posted for v1

-boris


Boris Ostrovsky (9):
  x86/AMD: Allow more fine-grained control of VMCB MSR Permission Map
  x86/AMD: Do not intercept access to performance counters MSRs
  x86/AMD: Load context when attempting to read VPMU MSRs
  x86/AMD: Stop counters on VPMU save
  x86/VPMU: Add Haswell support
  x86/VPMU: Factor out VPMU common code
  x86/VPMU: Save/restore VPMU only when necessary
  x86/AMD: Clean up context_update() in AMD VPMU code
  x86/AMD: Dump AMD VPMU info

 xen/arch/x86/domain.c                    |  14 ++-
 xen/arch/x86/hvm/svm/svm.c               |  19 ++-
 xen/arch/x86/hvm/svm/vpmu.c              | 197 +++++++++++++++++++++++--------
 xen/arch/x86/hvm/vmx/vmx.c               |   2 -
 xen/arch/x86/hvm/vmx/vpmu_core2.c        |  48 ++++----
 xen/arch/x86/hvm/vpmu.c                  | 114 ++++++++++++++++--
 xen/include/asm-x86/hvm/svm/vmcb.h       |   8 +-
 xen/include/asm-x86/hvm/vmx/vpmu_core2.h |   1 -
 xen/include/asm-x86/hvm/vpmu.h           |  15 ++-
 9 files changed, 312 insertions(+), 106 deletions(-)

-- 
1.8.1.2

Boris Ostrovsky

2013-Apr-12 17:59 UTC

head link

[PATCH v2 1/9] x86/AMD: Allow more fine-grained control of VMCB MSR Permission Map

Currently VMCB''s MSRPM can be updated to either intercept both reads
and
writes to an MSR or not intercept neither. In some cases we may want to
be more selective and intercept one but not the other.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
---
 xen/arch/x86/hvm/svm/svm.c         | 15 +++++++--------
 xen/include/asm-x86/hvm/svm/vmcb.h |  8 ++++++--
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index f170ffb..8ce37c9 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -137,7 +137,7 @@ svm_msrbit(unsigned long *msr_bitmap, uint32_t msr)
     return msr_bit;
 }
 
-void svm_intercept_msr(struct vcpu *v, uint32_t msr, int enable)
+void svm_intercept_msr(struct vcpu *v, uint32_t msr, int flags)
 {
     unsigned long *msr_bit;
 
@@ -145,16 +145,15 @@ void svm_intercept_msr(struct vcpu *v, uint32_t msr, int
enable)
     BUG_ON(msr_bit == NULL);
     msr &= 0x1fff;
 
-    if ( enable )
-    {
-        __set_bit(msr * 2, msr_bit);
+    if ( flags & MSR_INTERCEPT_READ )
+         __set_bit(msr * 2, msr_bit);
+    else
+         __clear_bit(msr * 2, msr_bit);
+
+    if ( flags & MSR_INTERCEPT_WRITE )
         __set_bit(msr * 2 + 1, msr_bit);
-    }
     else
-    {
-        __clear_bit(msr * 2, msr_bit);
         __clear_bit(msr * 2 + 1, msr_bit);
-    }
 }
 
 static void svm_save_dr(struct vcpu *v)
diff --git a/xen/include/asm-x86/hvm/svm/vmcb.h
b/xen/include/asm-x86/hvm/svm/vmcb.h
index b7c0404..ade4bb2 100644
--- a/xen/include/asm-x86/hvm/svm/vmcb.h
+++ b/xen/include/asm-x86/hvm/svm/vmcb.h
@@ -531,9 +531,13 @@ void svm_destroy_vmcb(struct vcpu *v);
 
 void setup_vmcb_dump(void);
 
+#define MSR_INTERCEPT_NONE    0
+#define MSR_INTERCEPT_READ    1
+#define MSR_INTERCEPT_WRITE   2
+#define MSR_INTERCEPT_RW      (MSR_INTERCEPT_WRITE | MSR_INTERCEPT_READ)
 void svm_intercept_msr(struct vcpu *v, uint32_t msr, int enable);
-#define svm_disable_intercept_for_msr(v, msr) svm_intercept_msr((v), (msr), 0)
-#define svm_enable_intercept_for_msr(v, msr) svm_intercept_msr((v), (msr), 1)
+#define svm_disable_intercept_for_msr(v, msr) svm_intercept_msr((v), (msr),
MSR_INTERCEPT_NONE)
+#define svm_enable_intercept_for_msr(v, msr) svm_intercept_msr((v), (msr),
MSR_INTERCEPT_RW)
 
 /*
  * VMCB accessor functions.
-- 
1.8.1.2

Boris Ostrovsky

2013-Apr-12 17:59 UTC

head link

[PATCH v2 2/9] x86/AMD: Do not intercept access to performance counters MSRs

Access to performance counters and reads of event selects don''t
need to always be intercepted.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
---
 xen/arch/x86/hvm/svm/svm.c  |  2 +-
 xen/arch/x86/hvm/svm/vpmu.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 8ce37c9..89e47b3 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1052,8 +1052,8 @@ static int svm_vcpu_initialise(struct vcpu *v)
 
 static void svm_vcpu_destroy(struct vcpu *v)
 {
-    svm_destroy_vmcb(v);
     vpmu_destroy(v);
+    svm_destroy_vmcb(v);
     passive_domain_destroy(v);
 }
 
diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
index 16170da..1e54497 100644
--- a/xen/arch/x86/hvm/svm/vpmu.c
+++ b/xen/arch/x86/hvm/svm/vpmu.c
@@ -88,6 +88,7 @@ struct amd_vpmu_context {
     u64 counters[MAX_NUM_COUNTERS];
     u64 ctrls[MAX_NUM_COUNTERS];
     u32 hw_lapic_lvtpc;
+    bool_t msr_bitmap_set;
 };
 
 static inline int get_pmu_reg_type(u32 addr)
@@ -138,6 +139,36 @@ static inline u32 get_fam15h_addr(u32 addr)
     return addr;
 }
 
+static void amd_vpmu_set_msr_bitmap(struct vcpu *v)
+{
+    unsigned int i;
+    struct vpmu_struct *vpmu = vcpu_vpmu(v);
+    struct amd_vpmu_context *ctxt = vpmu->context;
+
+    for ( i = 0; i < num_counters; i++ )
+    {
+        svm_intercept_msr(v, counters[i], MSR_INTERCEPT_NONE);
+        svm_intercept_msr(v, ctrls[i], MSR_INTERCEPT_WRITE);
+    }
+
+    ctxt->msr_bitmap_set = 1;
+}
+
+static void amd_vpmu_unset_msr_bitmap(struct vcpu *v)
+{
+    unsigned int i;
+    struct vpmu_struct *vpmu = vcpu_vpmu(v);
+    struct amd_vpmu_context *ctxt = vpmu->context;
+
+    for ( i = 0; i < num_counters; i++ )
+    {
+        svm_intercept_msr(v, counters[i], MSR_INTERCEPT_RW);
+        svm_intercept_msr(v, ctrls[i], MSR_INTERCEPT_RW);
+    }
+
+    ctxt->msr_bitmap_set = 0;
+}
+
 static int amd_vpmu_do_interrupt(struct cpu_user_regs *regs)
 {
     struct vcpu *v = current;
@@ -219,6 +250,10 @@ static void amd_vpmu_save(struct vcpu *v)
         return;
 
     context_save(v);
+
+    if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && ctx->msr_bitmap_set )
+        amd_vpmu_unset_msr_bitmap(v);
+
     ctx->hw_lapic_lvtpc = apic_read(APIC_LVTPC);
     apic_write(APIC_LVTPC,  ctx->hw_lapic_lvtpc | APIC_LVT_MASKED);
 }
@@ -267,6 +302,9 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t
msr_content)
             return 1;
         vpmu_set(vpmu, VPMU_RUNNING);
         apic_write(APIC_LVTPC, PMU_APIC_VECTOR);
+
+        if ( !((struct amd_vpmu_context *)vpmu->context)->msr_bitmap_set
)
+            amd_vpmu_set_msr_bitmap(v);
     }
 
     /* stop saving & restore if guest stops first counter */
@@ -275,6 +313,8 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t
msr_content)
     {
         apic_write(APIC_LVTPC, PMU_APIC_VECTOR | APIC_LVT_MASKED);
         vpmu_reset(vpmu, VPMU_RUNNING);
+        if ( ((struct amd_vpmu_context *)vpmu->context)->msr_bitmap_set )
+            amd_vpmu_unset_msr_bitmap(v);
         release_pmu_ownship(PMU_OWNER_HVM);
     }
 
@@ -345,6 +385,9 @@ static void amd_vpmu_destroy(struct vcpu *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);
+
     xfree(vpmu->context);
     vpmu_reset(vpmu, VPMU_CONTEXT_ALLOCATED);
 
-- 
1.8.1.2

Boris Ostrovsky

2013-Apr-12 17:59 UTC

head link

[PATCH v2 3/9] x86/AMD: Load context when attempting to read VPMU MSRs

Load context (and mark it as LOADED) on any MSR access. This will allow
us to always read the most up-to-date value of an MSR: guest may write
into an MSR without enabling it (thus not marking the context as RUNNING)
and then be migrated. Without first loading the context reading this MSR
from HW will not match the pervious write since registers will not be
loaded into HW in amd_vpmu_load().

In addition, we should be saving the context when it is LOADED, not
RUNNING --- otherwise we need to save it any time it becomes non-RUNNING,
which may be a frequent occurrence.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
---
 xen/arch/x86/hvm/svm/vpmu.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
index 1e54497..4be5652 100644
--- a/xen/arch/x86/hvm/svm/vpmu.c
+++ b/xen/arch/x86/hvm/svm/vpmu.c
@@ -225,6 +225,8 @@ static void amd_vpmu_restore(struct vcpu *v)
 
     context_restore(v);
     apic_write(APIC_LVTPC, ctxt->hw_lapic_lvtpc);
+
+    vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
 }
 
 static inline void context_save(struct vcpu *v)
@@ -246,7 +248,7 @@ static void amd_vpmu_save(struct vcpu *v)
     struct amd_vpmu_context *ctx = vpmu->context;
 
     if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) &&
-           vpmu_is_set(vpmu, VPMU_RUNNING)) )
+           vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)) )
         return;
 
     context_save(v);
@@ -256,6 +258,7 @@ static void amd_vpmu_save(struct vcpu *v)
 
     ctx->hw_lapic_lvtpc = apic_read(APIC_LVTPC);
     apic_write(APIC_LVTPC,  ctx->hw_lapic_lvtpc | APIC_LVT_MASKED);
+    vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
 }
 
 static void context_update(unsigned int msr, u64 msr_content)
@@ -318,6 +321,12 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t
msr_content)
         release_pmu_ownship(PMU_OWNER_HVM);
     }
 
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+    {
+        context_restore(v);
+        vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
+    }
+
     /* Update vpmu context immediately */
     context_update(msr, msr_content);
 
@@ -328,7 +337,17 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t
msr_content)
 
 static int amd_vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
 {
+    struct vcpu *v = current;
+    struct vpmu_struct *vpmu = vcpu_vpmu(v);
+
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+    {
+        context_restore(v);
+        vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
+    }
+
     rdmsrl(msr, *msr_content);
+
     return 1;
 }
 
-- 
1.8.1.2

Boris Ostrovsky

2013-Apr-12 17:59 UTC

head link

[PATCH v2 4/9] x86/AMD: Stop counters on VPMU save

Stop the counters during VPMU save operation since they shouldn''t be
running when VPCU that controls them is not. This also makes it
unnecessary to check for overflow in context_restore()

Set LVTPC vector before loading the context during vpmu_restore().
Otherwise it is possible to trigger an interrupt without proper vector.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
---
 xen/arch/x86/hvm/svm/vpmu.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
index 4be5652..51e5495 100644
--- a/xen/arch/x86/hvm/svm/vpmu.c
+++ b/xen/arch/x86/hvm/svm/vpmu.c
@@ -197,20 +197,9 @@ static inline void context_restore(struct vcpu *v)
     struct amd_vpmu_context *ctxt = vpmu->context;
 
     for ( i = 0; i < num_counters; i++ )
-        wrmsrl(ctrls[i], ctxt->ctrls[i]);
-
-    for ( i = 0; i < num_counters; i++ )
     {
         wrmsrl(counters[i], ctxt->counters[i]);
-
-        /* 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");
-            amd_vpmu_do_interrupt(0);
-        }
+        wrmsrl(ctrls[i], ctxt->ctrls[i]);
     }
 }
 
@@ -223,8 +212,8 @@ static void amd_vpmu_restore(struct vcpu *v)
            vpmu_is_set(vpmu, VPMU_RUNNING)) )
         return;
 
-    context_restore(v);
     apic_write(APIC_LVTPC, ctxt->hw_lapic_lvtpc);
+    context_restore(v);
 
     vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
 }
@@ -236,10 +225,11 @@ static inline void context_save(struct vcpu *v)
     struct amd_vpmu_context *ctxt = vpmu->context;
 
     for ( i = 0; i < num_counters; i++ )
-        rdmsrl(counters[i], ctxt->counters[i]);
-
-    for ( i = 0; i < num_counters; i++ )
+    {
         rdmsrl(ctrls[i], ctxt->ctrls[i]);
+        wrmsrl(ctrls[i], 0);
+        rdmsrl(counters[i], ctxt->counters[i]);
+    }
 }
 
 static void amd_vpmu_save(struct vcpu *v)
-- 
1.8.1.2

Boris Ostrovsky

2013-Apr-12 17:59 UTC

head link

[PATCH v2 5/9] x86/VPMU: Add Haswell support

Initialize VPMU on Haswell CPUs.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
---
 xen/arch/x86/hvm/vmx/vpmu_core2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c
b/xen/arch/x86/hvm/vmx/vpmu_core2.c
index 2313e39..7c86a0b 100644
--- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
+++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
@@ -893,6 +893,7 @@ int vmx_vpmu_initialise(struct vcpu *v, unsigned int
vpmu_flags)
 
         case 0x3a: /* IvyBridge */
         case 0x3e: /* IvyBridge EP */
+        case 0x3c: /* Haswell */
             ret = core2_vpmu_initialise(v, vpmu_flags);
             if ( !ret )
                 vpmu->arch_vpmu_ops = &core2_vpmu_ops;
-- 
1.8.1.2

Boris Ostrovsky

2013-Apr-12 17:59 UTC

head link

[PATCH v2 6/9] x86/VPMU: Factor out VPMU common code

Factor out common code from SVM amd VMX into VPMU.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
Tested-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
Acked-by: Jun Nakajima <jun.nakajima@intel.com>
---
 xen/arch/x86/hvm/svm/vpmu.c              | 37 -----------------------
 xen/arch/x86/hvm/vmx/vpmu_core2.c        | 30 +------------------
 xen/arch/x86/hvm/vpmu.c                  | 50 ++++++++++++++++++++++++++++----
 xen/include/asm-x86/hvm/vmx/vpmu_core2.h |  1 -
 xen/include/asm-x86/hvm/vpmu.h           |  1 +
 5 files changed, 47 insertions(+), 72 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
index 51e5495..f2f2d12 100644
--- a/xen/arch/x86/hvm/svm/vpmu.c
+++ b/xen/arch/x86/hvm/svm/vpmu.c
@@ -87,7 +87,6 @@ static const u32 AMD_F15H_CTRLS[] = {
 struct amd_vpmu_context {
     u64 counters[MAX_NUM_COUNTERS];
     u64 ctrls[MAX_NUM_COUNTERS];
-    u32 hw_lapic_lvtpc;
     bool_t msr_bitmap_set;
 };
 
@@ -171,22 +170,6 @@ static void amd_vpmu_unset_msr_bitmap(struct vcpu *v)
 
 static int amd_vpmu_do_interrupt(struct cpu_user_regs *regs)
 {
-    struct vcpu *v = current;
-    struct vlapic *vlapic = vcpu_vlapic(v);
-    u32 vlapic_lvtpc;
-    unsigned char int_vec;
-
-    if ( !is_vlapic_lvtpc_enabled(vlapic) )
-        return 0;
-
-    vlapic_lvtpc = vlapic_get_reg(vlapic, APIC_LVTPC);
-    int_vec = vlapic_lvtpc & APIC_VECTOR_MASK;
-
-    if ( GET_APIC_DELIVERY_MODE(vlapic_lvtpc) == APIC_MODE_FIXED )
-        vlapic_set_irq(vcpu_vlapic(v), int_vec, 0);
-    else
-        v->nmi_pending = 1;
-
     return 1;
 }
 
@@ -205,17 +188,7 @@ static inline void context_restore(struct vcpu *v)
 
 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)) )
-        return;
-
-    apic_write(APIC_LVTPC, ctxt->hw_lapic_lvtpc);
     context_restore(v);
-
-    vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
 }
 
 static inline void context_save(struct vcpu *v)
@@ -237,18 +210,10 @@ 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_CONTEXT_LOADED)) )
-        return;
-
     context_save(v);
 
     if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && ctx->msr_bitmap_set )
         amd_vpmu_unset_msr_bitmap(v);
-
-    ctx->hw_lapic_lvtpc = apic_read(APIC_LVTPC);
-    apic_write(APIC_LVTPC,  ctx->hw_lapic_lvtpc | APIC_LVT_MASKED);
-    vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
 }
 
 static void context_update(unsigned int msr, u64 msr_content)
@@ -271,8 +236,6 @@ static void context_update(unsigned int msr, u64
msr_content)
     for ( i = 0; i < num_counters; i++ )
         if ( msr == ctrls[i] )
             ctxt->ctrls[i] = msr_content;
-
-    ctxt->hw_lapic_lvtpc = apic_read(APIC_LVTPC);
 }
 
 static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c
b/xen/arch/x86/hvm/vmx/vpmu_core2.c
index 7c86a0b..6195bfc 100644
--- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
+++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
@@ -305,25 +305,18 @@ static inline void __core2_vpmu_save(struct vcpu *v)
         rdmsrl(core2_fix_counters.msr[i], core2_vpmu_cxt->fix_counters[i]);
     for ( i = 0; i < core2_get_pmc_count(); i++ )
         rdmsrl(MSR_IA32_PERFCTR0+i,
core2_vpmu_cxt->arch_msr_pair[i].counter);
-    core2_vpmu_cxt->hw_lapic_lvtpc = apic_read(APIC_LVTPC);
-    apic_write(APIC_LVTPC, PMU_APIC_VECTOR | APIC_LVT_MASKED);
 }
 
 static void core2_vpmu_save(struct vcpu *v)
 {
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
 
-    if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) &&
-           vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)) )
-        return;
-
     __core2_vpmu_save(v);
 
     /* Unset PMU MSR bitmap to trap lazy load. */
     if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && cpu_has_vmx_msr_bitmap )
         core2_vpmu_unset_msr_bitmap(v->arch.hvm_vmx.msr_bitmap);
 
-    vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
     return;
 }
 
@@ -341,20 +334,11 @@ static inline void __core2_vpmu_load(struct vcpu *v)
         wrmsrl(core2_ctrls.msr[i], core2_vpmu_cxt->ctrls[i]);
     for ( i = 0; i < core2_get_pmc_count(); i++ )
         wrmsrl(MSR_P6_EVNTSEL0+i, core2_vpmu_cxt->arch_msr_pair[i].control);
-
-    apic_write_around(APIC_LVTPC, core2_vpmu_cxt->hw_lapic_lvtpc);
 }
 
 static void core2_vpmu_load(struct vcpu *v)
 {
-    struct vpmu_struct *vpmu = vcpu_vpmu(v);
-
-    /* Only when PMU is counting, we load PMU context immediately. */
-    if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) &&
-           vpmu_is_set(vpmu, VPMU_RUNNING)) )
-        return;
     __core2_vpmu_load(v);
-    vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
 }
 
 static int core2_vpmu_alloc_resource(struct vcpu *v)
@@ -705,11 +689,8 @@ static int core2_vpmu_do_interrupt(struct cpu_user_regs
*regs)
 {
     struct vcpu *v = current;
     u64 msr_content;
-    u32 vlapic_lvtpc;
-    unsigned char int_vec;
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
     struct core2_vpmu_context *core2_vpmu_cxt = vpmu->context;
-    struct vlapic *vlapic = vcpu_vlapic(v);
 
     rdmsrl(MSR_CORE_PERF_GLOBAL_STATUS, msr_content);
     if ( msr_content )
@@ -728,18 +709,9 @@ static int core2_vpmu_do_interrupt(struct cpu_user_regs
*regs)
             return 0;
     }
 
+    /* HW sets the MASK bit when performance counter interrupt occurs*/
     apic_write_around(APIC_LVTPC, apic_read(APIC_LVTPC) &
~APIC_LVT_MASKED);
 
-    if ( !is_vlapic_lvtpc_enabled(vlapic) )
-        return 1;
-
-    vlapic_lvtpc = vlapic_get_reg(vlapic, APIC_LVTPC);
-    int_vec = vlapic_lvtpc & APIC_VECTOR_MASK;
-    vlapic_set_reg(vlapic, APIC_LVTPC, vlapic_lvtpc | APIC_LVT_MASKED);
-    if ( GET_APIC_DELIVERY_MODE(vlapic_lvtpc) == APIC_MODE_FIXED )
-        vlapic_set_irq(vcpu_vlapic(v), int_vec, 0);
-    else
-        v->nmi_pending = 1;
     return 1;
 }
 
diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
index 3b69580..fa5329d 100644
--- a/xen/arch/x86/hvm/vpmu.c
+++ b/xen/arch/x86/hvm/vpmu.c
@@ -31,7 +31,7 @@
 #include <asm/hvm/vpmu.h>
 #include <asm/hvm/svm/svm.h>
 #include <asm/hvm/svm/vmcb.h>
-
+#include <asm/apic.h>
 
 /*
  * "vpmu" :     vpmu generally enabled
@@ -83,10 +83,31 @@ int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
 
 int vpmu_do_interrupt(struct cpu_user_regs *regs)
 {
-    struct vpmu_struct *vpmu = vcpu_vpmu(current);
+    struct vcpu *v = current;
+    struct vpmu_struct *vpmu = vcpu_vpmu(v);
+
+    if ( vpmu->arch_vpmu_ops )
+    {
+        struct vlapic *vlapic = vcpu_vlapic(v);
+        u32 vlapic_lvtpc;
+        unsigned char int_vec;
+
+        if ( !vpmu->arch_vpmu_ops->do_interrupt(regs) )
+            return 0;
+
+        if ( !is_vlapic_lvtpc_enabled(vlapic) )
+            return 1;
+
+        vlapic_lvtpc = vlapic_get_reg(vlapic, APIC_LVTPC);
+        int_vec = vlapic_lvtpc & APIC_VECTOR_MASK;
+
+        if ( GET_APIC_DELIVERY_MODE(vlapic_lvtpc) == APIC_MODE_FIXED )
+            vlapic_set_irq(vcpu_vlapic(v), int_vec, 0);
+        else
+            v->nmi_pending = 1;
+        return 1;
+    }
 
-    if ( vpmu->arch_vpmu_ops &&
vpmu->arch_vpmu_ops->do_interrupt )
-        return vpmu->arch_vpmu_ops->do_interrupt(regs);
     return 0;
 }
 
@@ -104,16 +125,35 @@ void vpmu_save(struct vcpu *v)
 {
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
 
-    if ( vpmu->arch_vpmu_ops &&
vpmu->arch_vpmu_ops->arch_vpmu_save )
+    if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) &&
+          vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)) )
+       return;
+
+    if ( vpmu->arch_vpmu_ops )
         vpmu->arch_vpmu_ops->arch_vpmu_save(v);
+
+    vpmu->hw_lapic_lvtpc = apic_read(APIC_LVTPC);
+    apic_write(APIC_LVTPC, PMU_APIC_VECTOR | APIC_LVT_MASKED);
+
+    vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
 }
 
 void vpmu_load(struct vcpu *v)
 {
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
 
+    /* Only when PMU is counting, we load PMU context immediately. */
+    if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) &&
+           vpmu_is_set(vpmu, VPMU_RUNNING)) )
+        return;
+
     if ( vpmu->arch_vpmu_ops &&
vpmu->arch_vpmu_ops->arch_vpmu_load )
+    {
+        apic_write_around(APIC_LVTPC, vpmu->hw_lapic_lvtpc);
         vpmu->arch_vpmu_ops->arch_vpmu_load(v);
+    }
+
+    vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
 }
 
 void vpmu_initialise(struct vcpu *v)
diff --git a/xen/include/asm-x86/hvm/vmx/vpmu_core2.h
b/xen/include/asm-x86/hvm/vmx/vpmu_core2.h
index 4128f2a..60b05fd 100644
--- a/xen/include/asm-x86/hvm/vmx/vpmu_core2.h
+++ b/xen/include/asm-x86/hvm/vmx/vpmu_core2.h
@@ -44,7 +44,6 @@ struct core2_vpmu_context {
     u64 fix_counters[VPMU_CORE2_NUM_FIXED];
     u64 ctrls[VPMU_CORE2_NUM_CTRLS];
     u64 global_ovf_status;
-    u32 hw_lapic_lvtpc;
     struct arch_msr_pair arch_msr_pair[1];
 };
 
diff --git a/xen/include/asm-x86/hvm/vpmu.h b/xen/include/asm-x86/hvm/vpmu.h
index cd31f5e..01be976 100644
--- a/xen/include/asm-x86/hvm/vpmu.h
+++ b/xen/include/asm-x86/hvm/vpmu.h
@@ -62,6 +62,7 @@ int svm_vpmu_initialise(struct vcpu *, unsigned int flags);
 
 struct vpmu_struct {
     u32 flags;
+    u32 hw_lapic_lvtpc;
     void *context;
     struct arch_vpmu_ops *arch_vpmu_ops;
 };
-- 
1.8.1.2

Boris Ostrovsky

2013-Apr-12 17:59 UTC

head link

[PATCH v2 7/9] x86/VPMU: Save/restore VPMU only when necessary

VPMU doesn''t need to always be saved during context switch. If we are
comming back to the same processor and no other VPCU has run here we can
simply continue running. This is especailly useful on Intel processors
where Global Control MSR is stored in VMCS, thus not requiring us to stop
the counters during save operation. On AMD we need to explicitly stop the
counters but we don''t need to save them.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
Tested-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
---
 xen/arch/x86/domain.c             | 14 ++++++--
 xen/arch/x86/hvm/svm/svm.c        |  2 --
 xen/arch/x86/hvm/svm/vpmu.c       | 52 +++++++++++++++++++++++----
 xen/arch/x86/hvm/vmx/vmx.c        |  2 --
 xen/arch/x86/hvm/vmx/vpmu_core2.c | 25 +++++++++++--
 xen/arch/x86/hvm/vpmu.c           | 74 +++++++++++++++++++++++++++++++++++----
 xen/include/asm-x86/hvm/vpmu.h    | 14 +++++---
 7 files changed, 156 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 0d7a40c..6f9cbed 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1535,8 +1535,14 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
     if (prev != next)
         update_runstate_area(prev);
 
-    if ( is_hvm_vcpu(prev) &&
!list_empty(&prev->arch.hvm_vcpu.tm_list) )
-        pt_save_timer(prev);
+    if ( is_hvm_vcpu(prev) )
+    {
+        if (prev != next)
+            vpmu_save(prev);
+
+        if ( !list_empty(&prev->arch.hvm_vcpu.tm_list) )
+            pt_save_timer(prev);
+    }
 
     local_irq_disable();
 
@@ -1574,6 +1580,10 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
                            (next->domain->domain_id != 0));
     }
 
+    if (is_hvm_vcpu(next) && (prev != next) )
+        /* Must be done with interrupts enabled */
+        vpmu_load(next);
+
     context_saved(prev);
 
     if (prev != next)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 89e47b3..8123f37 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -860,7 +860,6 @@ static void svm_ctxt_switch_from(struct vcpu *v)
     svm_fpu_leave(v);
 
     svm_save_dr(v);
-    vpmu_save(v);
     svm_lwp_save(v);
     svm_tsc_ratio_save(v);
 
@@ -901,7 +900,6 @@ static void svm_ctxt_switch_to(struct vcpu *v)
     svm_vmsave(per_cpu(root_vmcb, cpu));
     svm_vmload(vmcb);
     vmcb->cleanbits.bytes = 0;
-    vpmu_load(v);
     svm_lwp_load(v);
     svm_tsc_ratio_load(v);
 
diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
index f2f2d12..efe2b7c 100644
--- a/xen/arch/x86/hvm/svm/vpmu.c
+++ b/xen/arch/x86/hvm/svm/vpmu.c
@@ -188,6 +188,21 @@ static inline void context_restore(struct vcpu *v)
 
 static void amd_vpmu_restore(struct vcpu *v)
 {
+    struct vpmu_struct *vpmu = vcpu_vpmu(v);
+    struct amd_vpmu_context *ctxt = vpmu->context;
+
+    vpmu_reset(vpmu, VPMU_FROZEN);
+
+    if ( vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+    {
+        unsigned int i;
+
+        for ( i = 0; i < num_counters; i++ )
+            wrmsrl(ctrls[i], ctxt->ctrls[i]);
+
+        return;
+    }
+
     context_restore(v);
 }
 
@@ -197,23 +212,40 @@ static inline void context_save(struct vcpu *v)
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
     struct amd_vpmu_context *ctxt = vpmu->context;
 
+    /* No need to save controls -- they are saved in amd_vpmu_do_wrmsr */
     for ( i = 0; i < num_counters; i++ )
-    {
-        rdmsrl(ctrls[i], ctxt->ctrls[i]);
-        wrmsrl(ctrls[i], 0);
         rdmsrl(counters[i], ctxt->counters[i]);
-    }
 }
 
-static void amd_vpmu_save(struct vcpu *v)
+static int amd_vpmu_save(struct vcpu *v)
 {
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
     struct amd_vpmu_context *ctx = vpmu->context;
+    unsigned int i;
+
+    /*
+     * Stop the counters. If we came here via vpmu_save_force (i.e.
+     * when VPMU_CONTEXT_SAVE is set) counters are already stopped.
+     */
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_SAVE) )
+    {
+        vpmu_set(vpmu, VPMU_FROZEN);
+
+        for ( i = 0; i < num_counters; i++ )
+            wrmsrl(ctrls[i], 0);
+
+        return 0;
+    }
+
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+        return 0;
 
     context_save(v);
 
     if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && ctx->msr_bitmap_set )
         amd_vpmu_unset_msr_bitmap(v);
+
+    return 1;
 }
 
 static void context_update(unsigned int msr, u64 msr_content)
@@ -258,6 +290,7 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t
msr_content)
             return 1;
         vpmu_set(vpmu, VPMU_RUNNING);
         apic_write(APIC_LVTPC, PMU_APIC_VECTOR);
+        vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR;
 
         if ( !((struct amd_vpmu_context *)vpmu->context)->msr_bitmap_set
)
             amd_vpmu_set_msr_bitmap(v);
@@ -268,16 +301,19 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t
msr_content)
         (is_pmu_enabled(msr_content) == 0) && vpmu_is_set(vpmu,
VPMU_RUNNING) )
     {
         apic_write(APIC_LVTPC, PMU_APIC_VECTOR | APIC_LVT_MASKED);
+        vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR | APIC_LVT_MASKED;
         vpmu_reset(vpmu, VPMU_RUNNING);
         if ( ((struct amd_vpmu_context *)vpmu->context)->msr_bitmap_set )
             amd_vpmu_unset_msr_bitmap(v);
         release_pmu_ownship(PMU_OWNER_HVM);
     }
 
-    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)
+        || vpmu_is_set(vpmu, VPMU_FROZEN) )
     {
         context_restore(v);
         vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
+        vpmu_reset(vpmu, VPMU_FROZEN);
     }
 
     /* Update vpmu context immediately */
@@ -293,10 +329,12 @@ static int amd_vpmu_do_rdmsr(unsigned int msr, uint64_t
*msr_content)
     struct vcpu *v = current;
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
 
-    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)
+        || vpmu_is_set(vpmu, VPMU_FROZEN) )
     {
         context_restore(v);
         vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
+        vpmu_reset(vpmu, VPMU_FROZEN);
     }
 
     rdmsrl(msr, *msr_content);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index a869ed4..e36dbcb 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -615,7 +615,6 @@ static void vmx_ctxt_switch_from(struct vcpu *v)
     vmx_save_guest_msrs(v);
     vmx_restore_host_msrs();
     vmx_save_dr(v);
-    vpmu_save(v);
 }
 
 static void vmx_ctxt_switch_to(struct vcpu *v)
@@ -640,7 +639,6 @@ static void vmx_ctxt_switch_to(struct vcpu *v)
 
     vmx_restore_guest_msrs(v);
     vmx_restore_dr(v);
-    vpmu_load(v);
 }
 
 
diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c
b/xen/arch/x86/hvm/vmx/vpmu_core2.c
index 6195bfc..9f152b4 100644
--- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
+++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
@@ -307,17 +307,23 @@ static inline void __core2_vpmu_save(struct vcpu *v)
         rdmsrl(MSR_IA32_PERFCTR0+i,
core2_vpmu_cxt->arch_msr_pair[i].counter);
 }
 
-static void core2_vpmu_save(struct vcpu *v)
+static int core2_vpmu_save(struct vcpu *v)
 {
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
 
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_SAVE) )
+        return 0;
+
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) ) 
+        return 0;
+
     __core2_vpmu_save(v);
 
     /* Unset PMU MSR bitmap to trap lazy load. */
     if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && cpu_has_vmx_msr_bitmap )
         core2_vpmu_unset_msr_bitmap(v->arch.hvm_vmx.msr_bitmap);
 
-    return;
+    return 1;
 }
 
 static inline void __core2_vpmu_load(struct vcpu *v)
@@ -338,6 +344,11 @@ static inline void __core2_vpmu_load(struct vcpu *v)
 
 static void core2_vpmu_load(struct vcpu *v)
 {
+    struct vpmu_struct *vpmu = vcpu_vpmu(v);
+
+    if ( vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+        return;
+
     __core2_vpmu_load(v);
 }
 
@@ -539,9 +550,15 @@ static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t
msr_content)
     /* Setup LVTPC in local apic */
     if ( vpmu_is_set(vpmu, VPMU_RUNNING) &&
          is_vlapic_lvtpc_enabled(vcpu_vlapic(v)) )
+    {
         apic_write_around(APIC_LVTPC, PMU_APIC_VECTOR);
+        vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR;
+    }
     else
+    {
         apic_write_around(APIC_LVTPC, PMU_APIC_VECTOR | APIC_LVT_MASKED);
+        vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR | APIC_LVT_MASKED;
+    }
 
     core2_vpmu_save_msr_context(v, type, index, msr_content);
     if ( type != MSR_TYPE_GLOBAL )
@@ -616,6 +633,7 @@ static int core2_vpmu_do_rdmsr(unsigned int msr, uint64_t
*msr_content)
         else
             return 0;
     }
+
     return 1;
 }
 
@@ -710,7 +728,8 @@ static int core2_vpmu_do_interrupt(struct cpu_user_regs
*regs)
     }
 
     /* HW sets the MASK bit when performance counter interrupt occurs*/
-    apic_write_around(APIC_LVTPC, apic_read(APIC_LVTPC) &
~APIC_LVT_MASKED);
+    vpmu->hw_lapic_lvtpc = apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED;
+    apic_write_around(APIC_LVTPC, vpmu->hw_lapic_lvtpc);
 
     return 1;
 }
diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
index fa5329d..20a0841 100644
--- a/xen/arch/x86/hvm/vpmu.c
+++ b/xen/arch/x86/hvm/vpmu.c
@@ -18,7 +18,6 @@
  *
  * Author: Haitao Shan <haitao.shan@intel.com>
  */
-
 #include <xen/config.h>
 #include <xen/sched.h>
 #include <xen/xenoprof.h>
@@ -42,6 +41,8 @@ static unsigned int __read_mostly opt_vpmu_enabled;
 static void parse_vpmu_param(char *s);
 custom_param("vpmu", parse_vpmu_param);
 
+static DEFINE_PER_CPU(struct vcpu *, last_vcpu);
+
 static void __init parse_vpmu_param(char *s)
 {
     switch ( parse_bool(s) )
@@ -121,30 +122,89 @@ void vpmu_do_cpuid(unsigned int input,
         vpmu->arch_vpmu_ops->do_cpuid(input, eax, ebx, ecx, edx);
 }
 
+static void vpmu_save_force(void *arg)
+{
+    struct vcpu *v = (struct vcpu *)arg;
+    struct vpmu_struct *vpmu = vcpu_vpmu(v);
+
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+        return;
+
+    if ( vpmu->arch_vpmu_ops )
+        (void)vpmu->arch_vpmu_ops->arch_vpmu_save(v);
+
+    vpmu_reset(vpmu, VPMU_CONTEXT_SAVE);
+
+    per_cpu(last_vcpu, smp_processor_id()) = NULL;
+}
+
 void vpmu_save(struct vcpu *v)
 {
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
+    int pcpu = smp_processor_id();
 
     if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) &&
           vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)) )
        return;
 
+    vpmu->last_pcpu = pcpu;
+    per_cpu(last_vcpu, pcpu) = v;
+
     if ( vpmu->arch_vpmu_ops )
-        vpmu->arch_vpmu_ops->arch_vpmu_save(v);
+        if ( vpmu->arch_vpmu_ops->arch_vpmu_save(v) )
+            vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
 
-    vpmu->hw_lapic_lvtpc = apic_read(APIC_LVTPC);
     apic_write(APIC_LVTPC, PMU_APIC_VECTOR | APIC_LVT_MASKED);
-
-    vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
 }
 
 void vpmu_load(struct vcpu *v)
 {
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
+    int pcpu = smp_processor_id();
+    struct vcpu *prev = NULL;
+
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
+        return;
+
+    /* First time this VCPU is running here */
+    if ( vpmu->last_pcpu != pcpu )
+    {
+        /*
+         * Get the context from last pcpu that we ran on. Note that if another
+         * VCPU is running there it must have saved this VPCU''s
context before
+         * startig to run (see below).
+         * There should be no race since remote pcpu will disable interrupts
+         * before saving the context.
+         */
+        if ( vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+        {
+            vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
+            on_selected_cpus(cpumask_of(vpmu->last_pcpu),
+                             vpmu_save_force, (void *)v, 1);
+            vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
+        }
+    } 
+
+    /* Prevent forced context save from remote CPU */
+    local_irq_disable();
+
+    prev = per_cpu(last_vcpu, pcpu);
+
+    if (prev != v && prev) {
+	vpmu = vcpu_vpmu(prev);
+
+        /* Someone ran here before us */
+        vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
+        vpmu_save_force(prev);
+        vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
+
+        vpmu = vcpu_vpmu(v);
+    }
+
+    local_irq_enable();
 
     /* Only when PMU is counting, we load PMU context immediately. */
-    if ( !(vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) &&
-           vpmu_is_set(vpmu, VPMU_RUNNING)) )
+    if ( !vpmu_is_set(vpmu, VPMU_RUNNING) )
         return;
 
     if ( vpmu->arch_vpmu_ops &&
vpmu->arch_vpmu_ops->arch_vpmu_load )
diff --git a/xen/include/asm-x86/hvm/vpmu.h b/xen/include/asm-x86/hvm/vpmu.h
index 01be976..03b9462 100644
--- a/xen/include/asm-x86/hvm/vpmu.h
+++ b/xen/include/asm-x86/hvm/vpmu.h
@@ -52,7 +52,7 @@ struct arch_vpmu_ops {
                      unsigned int *eax, unsigned int *ebx,
                      unsigned int *ecx, unsigned int *edx);
     void (*arch_vpmu_destroy)(struct vcpu *v);
-    void (*arch_vpmu_save)(struct vcpu *v);
+    int (*arch_vpmu_save)(struct vcpu *v);
     void (*arch_vpmu_load)(struct vcpu *v);
     void (*arch_vpmu_dump)(struct vcpu *v);
 };
@@ -62,17 +62,23 @@ int svm_vpmu_initialise(struct vcpu *, unsigned int flags);
 
 struct vpmu_struct {
     u32 flags;
+    u32 last_pcpu;
     u32 hw_lapic_lvtpc;
     void *context;
     struct arch_vpmu_ops *arch_vpmu_ops;
 };
 
+/* VPMU states */
 #define VPMU_CONTEXT_ALLOCATED              0x1
 #define VPMU_CONTEXT_LOADED                 0x2
 #define VPMU_RUNNING                        0x4
-#define VPMU_PASSIVE_DOMAIN_ALLOCATED       0x8
-#define VPMU_CPU_HAS_DS                     0x10 /* Has Debug Store */
-#define VPMU_CPU_HAS_BTS                    0x20 /* Has Branch Trace Store */
+#define VPMU_CONTEXT_SAVE                   0x8   /* Force context save */
+#define VPMU_FROZEN                         0x10  /* Stop counters while VCPU
is not running */
+#define VPMU_PASSIVE_DOMAIN_ALLOCATED       0x20
+
+/* VPMU features */
+#define VPMU_CPU_HAS_DS                     0x100 /* Has Debug Store */
+#define VPMU_CPU_HAS_BTS                    0x200 /* Has Branch Trace Store */
 
 
 #define vpmu_set(_vpmu, _x)    ((_vpmu)->flags |= (_x))
-- 
1.8.1.2

Boris Ostrovsky

2013-Apr-12 17:59 UTC

head link

[PATCH v2 8/9] x86/AMD: Clean up context_update() in AMD VPMU code

Clean up context_update() in AMD VPMU code.

Rename restore routine to "load" to be consistent with Intel
code and with arch_vpmu_ops names

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
---
 xen/arch/x86/hvm/svm/vpmu.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
index efe2b7c..b36ab2b 100644
--- a/xen/arch/x86/hvm/svm/vpmu.c
+++ b/xen/arch/x86/hvm/svm/vpmu.c
@@ -173,7 +173,7 @@ static int amd_vpmu_do_interrupt(struct cpu_user_regs *regs)
     return 1;
 }
 
-static inline void context_restore(struct vcpu *v)
+static inline void context_load(struct vcpu *v)
 {
     unsigned int i;
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
@@ -186,7 +186,7 @@ static inline void context_restore(struct vcpu *v)
     }
 }
 
-static void amd_vpmu_restore(struct vcpu *v)
+static void amd_vpmu_load(struct vcpu *v)
 {
     struct vpmu_struct *vpmu = vcpu_vpmu(v);
     struct amd_vpmu_context *ctxt = vpmu->context;
@@ -203,7 +203,7 @@ static void amd_vpmu_restore(struct vcpu *v)
         return;
     }
 
-    context_restore(v);
+    context_load(v);
 }
 
 static inline void context_save(struct vcpu *v)
@@ -262,12 +262,18 @@ static void context_update(unsigned int msr, u64
msr_content)
     }
 
     for ( i = 0; i < num_counters; i++ )
-        if ( msr == counters[i] )
+    {
+       if ( msr == ctrls[i] )
+       {
+           ctxt->ctrls[i] = msr_content;
+           return;
+       }
+        else if (msr == counters[i] )
+        {
             ctxt->counters[i] = msr_content;
-
-    for ( i = 0; i < num_counters; i++ )
-        if ( msr == ctrls[i] )
-            ctxt->ctrls[i] = msr_content;
+            return;
+        }
+    }
 }
 
 static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
@@ -311,7 +317,7 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t
msr_content)
     if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)
         || vpmu_is_set(vpmu, VPMU_FROZEN) )
     {
-        context_restore(v);
+        context_load(v);
         vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
         vpmu_reset(vpmu, VPMU_FROZEN);
     }
@@ -332,7 +338,7 @@ static int amd_vpmu_do_rdmsr(unsigned int msr, uint64_t
*msr_content)
     if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)
         || vpmu_is_set(vpmu, VPMU_FROZEN) )
     {
-        context_restore(v);
+        context_load(v);
         vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
         vpmu_reset(vpmu, VPMU_FROZEN);
     }
@@ -414,7 +420,7 @@ struct arch_vpmu_ops amd_vpmu_ops = {
     .do_interrupt = amd_vpmu_do_interrupt,
     .arch_vpmu_destroy = amd_vpmu_destroy,
     .arch_vpmu_save = amd_vpmu_save,
-    .arch_vpmu_load = amd_vpmu_restore
+    .arch_vpmu_load = amd_vpmu_load
 };
 
 int svm_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags)
-- 
1.8.1.2

Boris Ostrovsky

2013-Apr-12 17:59 UTC

head link

[PATCH v2 9/9] x86/AMD: Dump AMD VPMU info

Dump VPMU registers on AMD in the ''q'' keyhandler.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 xen/arch/x86/hvm/svm/vpmu.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
index b36ab2b..4d1fbc8 100644
--- a/xen/arch/x86/hvm/svm/vpmu.c
+++ b/xen/arch/x86/hvm/svm/vpmu.c
@@ -414,13 +414,53 @@ static void amd_vpmu_destroy(struct vcpu *v)
     }
 }
 
+/* VPMU part of the ''q'' keyhandler */
+static void amd_vpmu_dump(struct vcpu *v)
+{
+    struct vpmu_struct *vpmu = vcpu_vpmu(v);
+    struct amd_vpmu_context *ctxt = vpmu->context;
+    unsigned int i;
+
+    printk("    VPMU state: 0x%x ", vpmu->flags);
+    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
+    {
+         printk("\n");
+         return;
+    }
+
+    printk("(");
+    if ( vpmu_is_set(vpmu, VPMU_PASSIVE_DOMAIN_ALLOCATED) )
+        printk("PASSIVE_DOMAIN_ALLOCATED, ");
+    if ( vpmu_is_set(vpmu, VPMU_FROZEN) )
+        printk("FROZEN, ");
+    if ( vpmu_is_set(vpmu, VPMU_CONTEXT_SAVE) )
+        printk("SAVE, ");
+    if ( vpmu_is_set(vpmu, VPMU_RUNNING) )
+        printk("RUNNING, ");
+    if ( vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
+        printk("LOADED, ");
+    printk("ALLOCATED)\n");
+
+    for ( i = 0; i < num_counters; i++ )
+    {
+        uint64_t ctrl, cntr;
+
+        rdmsrl(ctrls[i], ctrl);
+        rdmsrl(counters[i], cntr);
+        printk("      0x%08x: 0x%lx (0x%lx in HW)    0x%08x: 0x%lx (0x%lx
in HW)\n",
+            ctrls[i], ctxt->ctrls[i], ctrl,
+            counters[i], ctxt->counters[i], cntr);
+    }
+}
+
 struct arch_vpmu_ops amd_vpmu_ops = {
     .do_wrmsr = amd_vpmu_do_wrmsr,
     .do_rdmsr = amd_vpmu_do_rdmsr,
     .do_interrupt = amd_vpmu_do_interrupt,
     .arch_vpmu_destroy = amd_vpmu_destroy,
     .arch_vpmu_save = amd_vpmu_save,
-    .arch_vpmu_load = amd_vpmu_load
+    .arch_vpmu_load = amd_vpmu_load,
+    .arch_vpmu_dump = amd_vpmu_dump
 };
 
 int svm_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags)
-- 
1.8.1.2

Suravee Suthikulanit

2013-Apr-15 17:54 UTC

head link

Re: [PATCH v2 9/9] x86/AMD: Dump AMD VPMU info

This is nice.  Tested and Acked.

Thanks

Suravee

On 4/12/2013 12:59 PM, Boris Ostrovsky wrote:> Dump VPMU registers on AMD in the ''q'' keyhandler.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
>   xen/arch/x86/hvm/svm/vpmu.c | 42
+++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
> index b36ab2b..4d1fbc8 100644
> --- a/xen/arch/x86/hvm/svm/vpmu.c
> +++ b/xen/arch/x86/hvm/svm/vpmu.c
> @@ -414,13 +414,53 @@ static void amd_vpmu_destroy(struct vcpu *v)
>       }
>   }
>   
> +/* VPMU part of the ''q'' keyhandler */
> +static void amd_vpmu_dump(struct vcpu *v)
> +{
> +    struct vpmu_struct *vpmu = vcpu_vpmu(v);
> +    struct amd_vpmu_context *ctxt = vpmu->context;
> +    unsigned int i;
> +
> +    printk("    VPMU state: 0x%x ", vpmu->flags);
> +    if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
> +    {
> +         printk("\n");
> +         return;
> +    }
> +
> +    printk("(");
> +    if ( vpmu_is_set(vpmu, VPMU_PASSIVE_DOMAIN_ALLOCATED) )
> +        printk("PASSIVE_DOMAIN_ALLOCATED, ");
> +    if ( vpmu_is_set(vpmu, VPMU_FROZEN) )
> +        printk("FROZEN, ");
> +    if ( vpmu_is_set(vpmu, VPMU_CONTEXT_SAVE) )
> +        printk("SAVE, ");
> +    if ( vpmu_is_set(vpmu, VPMU_RUNNING) )
> +        printk("RUNNING, ");
> +    if ( vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
> +        printk("LOADED, ");
> +    printk("ALLOCATED)\n");
> +
> +    for ( i = 0; i < num_counters; i++ )
> +    {
> +        uint64_t ctrl, cntr;
> +
> +        rdmsrl(ctrls[i], ctrl);
> +        rdmsrl(counters[i], cntr);
> +        printk("      0x%08x: 0x%lx (0x%lx in HW)    0x%08x: 0x%lx
(0x%lx in HW)\n",
> +            ctrls[i], ctxt->ctrls[i], ctrl,
> +            counters[i], ctxt->counters[i], cntr);
> +    }
> +}
> +
>   struct arch_vpmu_ops amd_vpmu_ops = {
>       .do_wrmsr = amd_vpmu_do_wrmsr,
>       .do_rdmsr = amd_vpmu_do_rdmsr,
>       .do_interrupt = amd_vpmu_do_interrupt,
>       .arch_vpmu_destroy = amd_vpmu_destroy,
>       .arch_vpmu_save = amd_vpmu_save,
> -    .arch_vpmu_load = amd_vpmu_load
> +    .arch_vpmu_load = amd_vpmu_load,
> +    .arch_vpmu_dump = amd_vpmu_dump
>   };
>   
>   int svm_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags)

Suravee Suthikulanit

2013-Apr-15 18:00 UTC

head link

Re: [PATCH v2 8/9] x86/AMD: Clean up context_update() in AMD VPMU code

Acked

Thanks,

Suravee
On 4/12/2013 12:59 PM, Boris Ostrovsky wrote:> Clean up context_update() in AMD VPMU code.
>
> Rename restore routine to "load" to be consistent with Intel
> code and with arch_vpmu_ops names
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Reviewed-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
> ---
>   xen/arch/x86/hvm/svm/vpmu.c | 28 +++++++++++++++++-----------
>   1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
> index efe2b7c..b36ab2b 100644
> --- a/xen/arch/x86/hvm/svm/vpmu.c
> +++ b/xen/arch/x86/hvm/svm/vpmu.c
> @@ -173,7 +173,7 @@ static int amd_vpmu_do_interrupt(struct cpu_user_regs
*regs)
>       return 1;
>   }
>   
> -static inline void context_restore(struct vcpu *v)
> +static inline void context_load(struct vcpu *v)
>   {
>       unsigned int i;
>       struct vpmu_struct *vpmu = vcpu_vpmu(v);
> @@ -186,7 +186,7 @@ static inline void context_restore(struct vcpu *v)
>       }
>   }
>   
> -static void amd_vpmu_restore(struct vcpu *v)
> +static void amd_vpmu_load(struct vcpu *v)
>   {
>       struct vpmu_struct *vpmu = vcpu_vpmu(v);
>       struct amd_vpmu_context *ctxt = vpmu->context;
> @@ -203,7 +203,7 @@ static void amd_vpmu_restore(struct vcpu *v)
>           return;
>       }
>   
> -    context_restore(v);
> +    context_load(v);
>   }
>   
>   static inline void context_save(struct vcpu *v)
> @@ -262,12 +262,18 @@ static void context_update(unsigned int msr, u64
msr_content)
>       }
>   
>       for ( i = 0; i < num_counters; i++ )
> -        if ( msr == counters[i] )
> +    {
> +       if ( msr == ctrls[i] )
> +       {
> +           ctxt->ctrls[i] = msr_content;
> +           return;
> +       }
> +        else if (msr == counters[i] )
> +        {
>               ctxt->counters[i] = msr_content;
> -
> -    for ( i = 0; i < num_counters; i++ )
> -        if ( msr == ctrls[i] )
> -            ctxt->ctrls[i] = msr_content;
> +            return;
> +        }
> +    }
>   }
>   
>   static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> @@ -311,7 +317,7 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t
msr_content)
>       if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)
>           || vpmu_is_set(vpmu, VPMU_FROZEN) )
>       {
> -        context_restore(v);
> +        context_load(v);
>           vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
>           vpmu_reset(vpmu, VPMU_FROZEN);
>       }
> @@ -332,7 +338,7 @@ static int amd_vpmu_do_rdmsr(unsigned int msr, uint64_t
*msr_content)
>       if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)
>           || vpmu_is_set(vpmu, VPMU_FROZEN) )
>       {
> -        context_restore(v);
> +        context_load(v);
>           vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
>           vpmu_reset(vpmu, VPMU_FROZEN);
>       }
> @@ -414,7 +420,7 @@ struct arch_vpmu_ops amd_vpmu_ops = {
>       .do_interrupt = amd_vpmu_do_interrupt,
>       .arch_vpmu_destroy = amd_vpmu_destroy,
>       .arch_vpmu_save = amd_vpmu_save,
> -    .arch_vpmu_load = amd_vpmu_restore
> +    .arch_vpmu_load = amd_vpmu_load
>   };
>   
>   int svm_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags)

Suravee Suthikulanit

2013-Apr-15 18:04 UTC

head link

Re: [PATCH v2 0/9] Various VPMU patches, version 2

Overall looks good. Tested.

Thanks,

Suravee

On 4/12/2013 12:59 PM, Boris Ostrovsky wrote:> Version 2 of VPMU patches.
>
> Changes:
> * Load context on MSR read on SVM and then read MSR from HW
(Suravee''s suggestion)
> * Group VPMU state bits separate from feature bits (Dietmar''s
suggestion)
> * Clean up (int->unsigned int for loop control, trailing spaces)
> * Rename VPMU_STOPPED to VPMU_FROZEN (*slightly* more sensible since it is
> possible to be both STOPPED and RUNNING)
> * Add patch for dumping VPMU state on AMD
> * Add Reviewed-by/Tested-by/Acked-by tags that have been posted for v1
>
> -boris
>
>
> Boris Ostrovsky (9):
>    x86/AMD: Allow more fine-grained control of VMCB MSR Permission Map
>    x86/AMD: Do not intercept access to performance counters MSRs
>    x86/AMD: Load context when attempting to read VPMU MSRs
>    x86/AMD: Stop counters on VPMU save
>    x86/VPMU: Add Haswell support
>    x86/VPMU: Factor out VPMU common code
>    x86/VPMU: Save/restore VPMU only when necessary
>    x86/AMD: Clean up context_update() in AMD VPMU code
>    x86/AMD: Dump AMD VPMU info
>
>   xen/arch/x86/domain.c                    |  14 ++-
>   xen/arch/x86/hvm/svm/svm.c               |  19 ++-
>   xen/arch/x86/hvm/svm/vpmu.c              | 197
+++++++++++++++++++++++--------
>   xen/arch/x86/hvm/vmx/vmx.c               |   2 -
>   xen/arch/x86/hvm/vmx/vpmu_core2.c        |  48 ++++----
>   xen/arch/x86/hvm/vpmu.c                  | 114 ++++++++++++++++--
>   xen/include/asm-x86/hvm/svm/vmcb.h       |   8 +-
>   xen/include/asm-x86/hvm/vmx/vpmu_core2.h |   1 -
>   xen/include/asm-x86/hvm/vpmu.h           |  15 ++-
>   9 files changed, 312 insertions(+), 106 deletions(-)
>

Xen devel - Apr 2013 - [PATCH v2 0/9] Various VPMU patches, version 2

[PATCH v2 0/9] Various VPMU patches, version 2

[PATCH v2 1/9] x86/AMD: Allow more fine-grained control of VMCB MSR Permission Map

[PATCH v2 2/9] x86/AMD: Do not intercept access to performance counters MSRs

[PATCH v2 3/9] x86/AMD: Load context when attempting to read VPMU MSRs

[PATCH v2 4/9] x86/AMD: Stop counters on VPMU save

[PATCH v2 5/9] x86/VPMU: Add Haswell support

[PATCH v2 6/9] x86/VPMU: Factor out VPMU common code

[PATCH v2 7/9] x86/VPMU: Save/restore VPMU only when necessary

[PATCH v2 8/9] x86/AMD: Clean up context_update() in AMD VPMU code

[PATCH v2 9/9] x86/AMD: Dump AMD VPMU info

Re: [PATCH v2 9/9] x86/AMD: Dump AMD VPMU info

Re: [PATCH v2 8/9] x86/AMD: Clean up context_update() in AMD VPMU code

Re: [PATCH v2 0/9] Various VPMU patches, version 2