|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v6 08/19] x86/VPMU: Add public xenpmu.h
Am Dienstag 13 Mai 2014, 11:53:22 schrieb Boris Ostrovsky:
> Add pmu.h header files, move various macros and structures that will be
> shared between hypervisor and PV guests to it.
>
> Move MSR banks out of architectural PMU structures to allow for larger sizes
> in the future. The banks are allocated immediately after the context and
> PMU structures store offsets to them.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> Reviewed-by: Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>
> Tested-by: Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>
It seems I didn't test right because calling
# xl debug-keys q
when running a HVM linux guest crashs the hypervisor.
Please see below.
> ---
> xen/arch/x86/hvm/svm/vpmu.c | 81 ++++++++++++-----------
> xen/arch/x86/hvm/vmx/vpmu_core2.c | 110
> +++++++++++++++++--------------
> xen/arch/x86/hvm/vpmu.c | 1 +
> xen/arch/x86/oprofile/op_model_ppro.c | 6 +-
> xen/include/asm-x86/hvm/vmx/vpmu_core2.h | 32 ---------
> xen/include/asm-x86/hvm/vpmu.h | 16 ++---
> xen/include/public/arch-arm.h | 3 +
> xen/include/public/arch-x86/pmu.h | 62 +++++++++++++++++
> xen/include/public/pmu.h | 38 +++++++++++
> 9 files changed, 220 insertions(+), 129 deletions(-)
> delete mode 100644 xen/include/asm-x86/hvm/vmx/vpmu_core2.h
> create mode 100644 xen/include/public/arch-x86/pmu.h
> create mode 100644 xen/include/public/pmu.h
>
> diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
> index 2fbe2c1..ebdba8e 100644
> --- a/xen/arch/x86/hvm/svm/vpmu.c
> +++ b/xen/arch/x86/hvm/svm/vpmu.c
> @@ -30,10 +30,7 @@
> #include <asm/apic.h>
> #include <asm/hvm/vlapic.h>
> #include <asm/hvm/vpmu.h>
> -
> -#define F10H_NUM_COUNTERS 4
> -#define F15H_NUM_COUNTERS 6
> -#define MAX_NUM_COUNTERS F15H_NUM_COUNTERS
> +#include <public/pmu.h>
>
> #define MSR_F10H_EVNTSEL_GO_SHIFT 40
> #define MSR_F10H_EVNTSEL_EN_SHIFT 22
> @@ -49,6 +46,10 @@ static const u32 __read_mostly *counters;
> static const u32 __read_mostly *ctrls;
> static bool_t __read_mostly k7_counters_mirrored;
>
> +#define F10H_NUM_COUNTERS 4
> +#define F15H_NUM_COUNTERS 6
> +#define AMD_MAX_COUNTERS 6
> +
> /* PMU Counter MSRs. */
> static const u32 AMD_F10H_COUNTERS[] = {
> MSR_K7_PERFCTR0,
> @@ -83,12 +84,10 @@ static const u32 AMD_F15H_CTRLS[] = {
> MSR_AMD_FAM15H_EVNTSEL5
> };
>
> -/* storage for context switching */
> -struct amd_vpmu_context {
> - u64 counters[MAX_NUM_COUNTERS];
> - u64 ctrls[MAX_NUM_COUNTERS];
> - bool_t msr_bitmap_set;
> -};
> +/* Use private context as a flag for MSR bitmap */
> +#define msr_bitmap_on(vpmu) {vpmu->priv_context = (void *)-1;}
> +#define msr_bitmap_off(vpmu) {vpmu->priv_context = NULL;}
> +#define is_msr_bitmap_on(vpmu) (vpmu->priv_context != NULL)
>
> static inline int get_pmu_reg_type(u32 addr)
> {
> @@ -142,7 +141,6 @@ 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++ )
> {
> @@ -150,14 +148,13 @@ static void amd_vpmu_set_msr_bitmap(struct vcpu *v)
> svm_intercept_msr(v, ctrls[i], MSR_INTERCEPT_WRITE);
> }
>
> - ctxt->msr_bitmap_set = 1;
> + msr_bitmap_on(vpmu);
> }
>
> 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++ )
> {
> @@ -165,7 +162,7 @@ static void amd_vpmu_unset_msr_bitmap(struct vcpu *v)
> svm_intercept_msr(v, ctrls[i], MSR_INTERCEPT_RW);
> }
>
> - ctxt->msr_bitmap_set = 0;
> + msr_bitmap_off(vpmu);
> }
>
> static int amd_vpmu_do_interrupt(struct cpu_user_regs *regs)
> @@ -177,19 +174,22 @@ static inline void context_load(struct vcpu *v)
> {
> unsigned int i;
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> - struct amd_vpmu_context *ctxt = vpmu->context;
> + struct xen_pmu_amd_ctxt *ctxt = vpmu->context;
> + uint64_t *counter_regs = vpmu_reg_pointer(ctxt, counters);
> + uint64_t *ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
>
> for ( i = 0; i < num_counters; i++ )
> {
> - wrmsrl(counters[i], ctxt->counters[i]);
> - wrmsrl(ctrls[i], ctxt->ctrls[i]);
> + wrmsrl(counters[i], counter_regs[i]);
> + wrmsrl(ctrls[i], ctrl_regs[i]);
> }
> }
>
> static void amd_vpmu_load(struct vcpu *v)
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> - struct amd_vpmu_context *ctxt = vpmu->context;
> + struct xen_pmu_amd_ctxt *ctxt = vpmu->context;
> + uint64_t *ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
>
> vpmu_reset(vpmu, VPMU_FROZEN);
>
> @@ -198,7 +198,7 @@ static void amd_vpmu_load(struct vcpu *v)
> unsigned int i;
>
> for ( i = 0; i < num_counters; i++ )
> - wrmsrl(ctrls[i], ctxt->ctrls[i]);
> + wrmsrl(ctrls[i], ctrl_regs[i]);
>
> return;
> }
> @@ -212,17 +212,17 @@ static inline void context_save(struct vcpu *v)
> {
> unsigned int i;
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> - struct amd_vpmu_context *ctxt = vpmu->context;
> + struct xen_pmu_amd_ctxt *ctxt = vpmu->context;
> + uint64_t *counter_regs = vpmu_reg_pointer(ctxt, counters);
>
> /* No need to save controls -- they are saved in amd_vpmu_do_wrmsr */
> for ( i = 0; i < num_counters; i++ )
> - rdmsrl(counters[i], ctxt->counters[i]);
> + rdmsrl(counters[i], counter_regs[i]);
> }
>
> 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;
>
> /*
> @@ -245,7 +245,7 @@ static int amd_vpmu_save(struct vcpu *v)
> context_save(v);
>
> if ( is_hvm_domain(v->domain) &&
> - !vpmu_is_set(vpmu, VPMU_RUNNING) && ctx->msr_bitmap_set )
> + !vpmu_is_set(vpmu, VPMU_RUNNING) && is_msr_bitmap_on(vpmu) )
> amd_vpmu_unset_msr_bitmap(v);
>
> return 1;
> @@ -256,7 +256,9 @@ static void context_update(unsigned int msr, u64
> msr_content)
> unsigned int i;
> struct vcpu *v = current;
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> - struct amd_vpmu_context *ctxt = vpmu->context;
> + struct xen_pmu_amd_ctxt *ctxt = vpmu->context;
> + uint64_t *counter_regs = vpmu_reg_pointer(ctxt, counters);
> + uint64_t *ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
>
> if ( k7_counters_mirrored &&
> ((msr >= MSR_K7_EVNTSEL0) && (msr <= MSR_K7_PERFCTR3)) )
> @@ -268,12 +270,12 @@ static void context_update(unsigned int msr, u64
> msr_content)
> {
> if ( msr == ctrls[i] )
> {
> - ctxt->ctrls[i] = msr_content;
> + ctrl_regs[i] = msr_content;
> return;
> }
> else if (msr == counters[i] )
> {
> - ctxt->counters[i] = msr_content;
> + counter_regs[i] = msr_content;
> return;
> }
> }
> @@ -299,8 +301,7 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t
> msr_content)
> return 1;
> vpmu_set(vpmu, VPMU_RUNNING);
>
> - if ( is_hvm_domain(v->domain) &&
> - !((struct amd_vpmu_context *)vpmu->context)->msr_bitmap_set )
> + if ( is_hvm_domain(v->domain) && is_msr_bitmap_on(vpmu) )
> amd_vpmu_set_msr_bitmap(v);
> }
>
> @@ -309,8 +310,7 @@ 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) )
> {
> vpmu_reset(vpmu, VPMU_RUNNING);
> - if ( is_hvm_domain(v->domain) &&
> - ((struct amd_vpmu_context *)vpmu->context)->msr_bitmap_set )
> + if ( is_hvm_domain(v->domain) && is_msr_bitmap_on(vpmu) )
> amd_vpmu_unset_msr_bitmap(v);
> release_pmu_ownship(PMU_OWNER_HVM);
> }
> @@ -351,7 +351,7 @@ static int amd_vpmu_do_rdmsr(unsigned int msr, uint64_t
> *msr_content)
>
> static int amd_vpmu_initialise(struct vcpu *v)
> {
> - struct amd_vpmu_context *ctxt;
> + struct xen_pmu_amd_ctxt *ctxt;
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> uint8_t family = current_cpu_data.x86;
>
> @@ -381,7 +381,9 @@ static int amd_vpmu_initialise(struct vcpu *v)
> }
> }
>
> - ctxt = xzalloc(struct amd_vpmu_context);
> + ctxt = xzalloc_bytes(sizeof(struct xen_pmu_amd_ctxt) +
> + sizeof(uint64_t) * AMD_MAX_COUNTERS +
> + sizeof(uint64_t) * AMD_MAX_COUNTERS);
> if ( !ctxt )
> {
> gdprintk(XENLOG_WARNING, "Insufficient memory for PMU, "
> @@ -390,7 +392,11 @@ static int amd_vpmu_initialise(struct vcpu *v)
> return -ENOMEM;
> }
>
> + ctxt->counters = sizeof(struct xen_pmu_amd_ctxt);
> + ctxt->ctrls = ctxt->counters + sizeof(uint64_t) * AMD_MAX_COUNTERS;
> +
> vpmu->context = ctxt;
> + vpmu->priv_context = NULL;
msr_bitmap_off(vpmu) ?
> vpmu_set(vpmu, VPMU_CONTEXT_ALLOCATED);
> return 0;
> }
> @@ -402,8 +408,7 @@ static void amd_vpmu_destroy(struct vcpu *v)
> if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
> return;
>
> - if ( is_hvm_domain(v->domain) &&
> - ((struct amd_vpmu_context *)vpmu->context)->msr_bitmap_set )
> + if ( is_hvm_domain(v->domain) && is_msr_bitmap_on(vpmu) )
> amd_vpmu_unset_msr_bitmap(v);
>
> xfree(vpmu->context);
> @@ -420,7 +425,9 @@ static void amd_vpmu_destroy(struct vcpu *v)
> static void amd_vpmu_dump(const struct vcpu *v)
> {
> const struct vpmu_struct *vpmu = vcpu_vpmu(v);
> - const struct amd_vpmu_context *ctxt = vpmu->context;
> + const struct xen_pmu_amd_ctxt *ctxt = vpmu->context;
> + uint64_t *counter_regs = vpmu_reg_pointer(ctxt, counters);
> + uint64_t *ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
> unsigned int i;
>
> printk(" VPMU state: 0x%x ", vpmu->flags);
> @@ -450,8 +457,8 @@ static void amd_vpmu_dump(const struct vcpu *v)
> rdmsrl(ctrls[i], ctrl);
> rdmsrl(counters[i], cntr);
> printk(" %#x: %#lx (%#lx in HW) %#x: %#lx (%#lx in HW)\n",
> - ctrls[i], ctxt->ctrls[i], ctrl,
> - counters[i], ctxt->counters[i], cntr);
> + ctrls[i], ctrl_regs[i], ctrl,
> + counters[i], counter_regs[i], cntr);
> }
> }
>
> diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> index dffdd80..1fe583f 100644
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> @@ -35,8 +35,8 @@
> #include <asm/hvm/vmx/vmcs.h>
> #include <public/sched.h>
> #include <public/hvm/save.h>
> +#include <public/pmu.h>
> #include <asm/hvm/vpmu.h>
> -#include <asm/hvm/vmx/vpmu_core2.h>
>
> /*
> * See Intel SDM Vol 2a Instruction Set Reference chapter 3 for CPUID
> @@ -68,6 +68,10 @@
> #define MSR_PMC_ALIAS_MASK (~(MSR_IA32_PERFCTR0 ^ MSR_IA32_A_PERFCTR0))
> static bool_t __read_mostly full_width_write;
>
> +/* Intel-specific VPMU features */
> +#define VPMU_CPU_HAS_DS 0x100 /* Has Debug Store */
> +#define VPMU_CPU_HAS_BTS 0x200 /* Has Branch Trace Store
> */
> +
> /*
> * MSR_CORE_PERF_FIXED_CTR_CTRL contains the configuration of all fixed
> * counters. 4 bits for every counter.
> @@ -75,17 +79,6 @@ static bool_t __read_mostly full_width_write;
> #define FIXED_CTR_CTRL_BITS 4
> #define FIXED_CTR_CTRL_MASK ((1 << FIXED_CTR_CTRL_BITS) - 1)
>
> -#define VPMU_CORE2_MAX_FIXED_PMCS 4
> -struct core2_vpmu_context {
> - u64 fixed_ctrl;
> - u64 ds_area;
> - u64 pebs_enable;
> - u64 global_ovf_status;
> - u64 enabled_cntrs; /* Follows PERF_GLOBAL_CTRL MSR format */
> - u64 fix_counters[VPMU_CORE2_MAX_FIXED_PMCS];
> - struct arch_msr_pair arch_msr_pair[1];
> -};
> -
> /* Number of general-purpose and fixed performance counters */
> static unsigned int __read_mostly arch_pmc_cnt, fixed_pmc_cnt;
>
> @@ -225,6 +218,7 @@ static int is_core2_vpmu_msr(u32 msr_index, int *type,
> int *index)
> return 0;
> }
>
> +#define msraddr_to_bitpos(x) (((x)&0xffff) + ((x)>>31)*0x2000)
> static void core2_vpmu_set_msr_bitmap(unsigned long *msr_bitmap)
> {
> int i;
> @@ -294,12 +288,15 @@ static void core2_vpmu_unset_msr_bitmap(unsigned long
> *msr_bitmap)
> static inline void __core2_vpmu_save(struct vcpu *v)
> {
> int i;
> - struct core2_vpmu_context *core2_vpmu_cxt = vcpu_vpmu(v)->context;
> + struct xen_pmu_intel_ctxt *core2_vpmu_cxt = vcpu_vpmu(v)->context;
> + uint64_t *fixed_counters = vpmu_reg_pointer(core2_vpmu_cxt,
> fixed_counters);
> + struct xen_pmu_cntr_pair *xen_pmu_cntr_pair =
> + vpmu_reg_pointer(core2_vpmu_cxt, arch_counters);
>
> for ( i = 0; i < fixed_pmc_cnt; i++ )
> - rdmsrl(MSR_CORE_PERF_FIXED_CTR0 + i,
> core2_vpmu_cxt->fix_counters[i]);
> + rdmsrl(MSR_CORE_PERF_FIXED_CTR0 + i, fixed_counters[i]);
> for ( i = 0; i < arch_pmc_cnt; i++ )
> - rdmsrl(MSR_IA32_PERFCTR0 + i,
> core2_vpmu_cxt->arch_msr_pair[i].counter);
> + rdmsrl(MSR_IA32_PERFCTR0 + i, xen_pmu_cntr_pair[i].counter);
> }
>
> static int core2_vpmu_save(struct vcpu *v)
> @@ -322,10 +319,13 @@ static int core2_vpmu_save(struct vcpu *v)
> static inline void __core2_vpmu_load(struct vcpu *v)
> {
> unsigned int i, pmc_start;
> - struct core2_vpmu_context *core2_vpmu_cxt = vcpu_vpmu(v)->context;
> + struct xen_pmu_intel_ctxt *core2_vpmu_cxt = vcpu_vpmu(v)->context;
> + uint64_t *fixed_counters = vpmu_reg_pointer(core2_vpmu_cxt,
> fixed_counters);
> + struct xen_pmu_cntr_pair *xen_pmu_cntr_pair =
> + vpmu_reg_pointer(core2_vpmu_cxt, arch_counters);
>
> for ( i = 0; i < fixed_pmc_cnt; i++ )
> - wrmsrl(MSR_CORE_PERF_FIXED_CTR0 + i,
> core2_vpmu_cxt->fix_counters[i]);
> + wrmsrl(MSR_CORE_PERF_FIXED_CTR0 + i, fixed_counters[i]);
>
> if ( full_width_write )
> pmc_start = MSR_IA32_A_PERFCTR0;
> @@ -333,8 +333,8 @@ static inline void __core2_vpmu_load(struct vcpu *v)
> pmc_start = MSR_IA32_PERFCTR0;
> for ( i = 0; i < arch_pmc_cnt; i++ )
> {
> - wrmsrl(pmc_start + i, core2_vpmu_cxt->arch_msr_pair[i].counter);
> - wrmsrl(MSR_P6_EVNTSEL0 + i,
> core2_vpmu_cxt->arch_msr_pair[i].control);
> + wrmsrl(pmc_start + i, xen_pmu_cntr_pair[i].counter);
> + wrmsrl(MSR_P6_EVNTSEL0 + i, xen_pmu_cntr_pair[i].control);
> }
>
> wrmsrl(MSR_CORE_PERF_FIXED_CTR_CTRL, core2_vpmu_cxt->fixed_ctrl);
> @@ -357,7 +357,8 @@ static void core2_vpmu_load(struct vcpu *v)
> static int core2_vpmu_alloc_resource(struct vcpu *v)
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> - struct core2_vpmu_context *core2_vpmu_cxt;
> + struct xen_pmu_intel_ctxt *core2_vpmu_cxt = NULL;
> + uint64_t *p = NULL;
>
> if ( !acquire_pmu_ownership(PMU_OWNER_HVM) )
> return 0;
> @@ -370,12 +371,20 @@ static int core2_vpmu_alloc_resource(struct vcpu *v)
> goto out_err;
> vmx_write_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
>
> - core2_vpmu_cxt = xzalloc_bytes(sizeof(struct core2_vpmu_context) +
> - (arch_pmc_cnt-1)*sizeof(struct arch_msr_pair));
> - if ( !core2_vpmu_cxt )
> + core2_vpmu_cxt = xzalloc_bytes(sizeof(struct xen_pmu_intel_ctxt) +
> + sizeof(uint64_t) * fixed_pmc_cnt +
> + sizeof(struct xen_pmu_cntr_pair) *
> + arch_pmc_cnt);
> + p = xzalloc_bytes(sizeof(uint64_t));
> + if ( !core2_vpmu_cxt || !p )
> goto out_err;
>
> + core2_vpmu_cxt->fixed_counters = sizeof(struct xen_pmu_intel_ctxt);
> + core2_vpmu_cxt->arch_counters = core2_vpmu_cxt->fixed_counters +
> + sizeof(uint64_t) * fixed_pmc_cnt;
> +
> vpmu->context = (void *)core2_vpmu_cxt;
> + vpmu->priv_context = (void *)p;
>
> vpmu_set(vpmu, VPMU_CONTEXT_ALLOCATED);
>
> @@ -386,6 +395,9 @@ out_err:
> vmx_rm_msr(MSR_CORE_PERF_GLOBAL_CTRL, VMX_GUEST_MSR);
> release_pmu_ownship(PMU_OWNER_HVM);
>
> + xfree(core2_vpmu_cxt);
> + xfree(p);
> +
> printk("Failed to allocate VPMU resources for domain %u vcpu %u\n",
> v->vcpu_id, v->domain->domain_id);
>
> @@ -421,7 +433,8 @@ static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t
> msr_content)
> int type = -1, index = -1;
> struct vcpu *v = current;
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> - struct core2_vpmu_context *core2_vpmu_cxt = NULL;
> + struct xen_pmu_intel_ctxt *core2_vpmu_cxt = NULL;
> + uint64_t *enabled_cntrs;
>
> if ( !core2_vpmu_msr_common_check(msr, &type, &index) )
> {
> @@ -447,10 +460,11 @@ static int core2_vpmu_do_wrmsr(unsigned int msr,
> uint64_t msr_content)
> }
>
> core2_vpmu_cxt = vpmu->context;
> + enabled_cntrs = (uint64_t *)vpmu->priv_context;
> switch ( msr )
> {
> case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
> - core2_vpmu_cxt->global_ovf_status &= ~msr_content;
> + core2_vpmu_cxt->global_status &= ~msr_content;
> return 1;
> case MSR_CORE_PERF_GLOBAL_STATUS:
> gdprintk(XENLOG_INFO, "Can not write readonly MSR: "
> @@ -484,15 +498,14 @@ static int core2_vpmu_do_wrmsr(unsigned int msr,
> uint64_t msr_content)
> break;
> case MSR_CORE_PERF_FIXED_CTR_CTRL:
> vmx_read_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL, &global_ctrl);
> - core2_vpmu_cxt->enabled_cntrs &=
> - ~(((1ULL << VPMU_CORE2_MAX_FIXED_PMCS) - 1) << 32);
> + *enabled_cntrs &= ~(((1ULL << fixed_pmc_cnt) - 1) << 32);
> if ( msr_content != 0 )
> {
> u64 val = msr_content;
> for ( i = 0; i < fixed_pmc_cnt; i++ )
> {
> if ( val & 3 )
> - core2_vpmu_cxt->enabled_cntrs |= (1ULL << 32) << i;
> + *enabled_cntrs |= (1ULL << 32) << i;
> val >>= FIXED_CTR_CTRL_BITS;
> }
> }
> @@ -503,19 +516,21 @@ static int core2_vpmu_do_wrmsr(unsigned int msr,
> uint64_t msr_content)
> tmp = msr - MSR_P6_EVNTSEL0;
> if ( tmp >= 0 && tmp < arch_pmc_cnt )
> {
> + struct xen_pmu_cntr_pair *xen_pmu_cntr_pair =
> + vpmu_reg_pointer(core2_vpmu_cxt, arch_counters);
> +
> vmx_read_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL, &global_ctrl);
>
> if ( msr_content & (1ULL << 22) )
> - core2_vpmu_cxt->enabled_cntrs |= 1ULL << tmp;
> + *enabled_cntrs |= 1ULL << tmp;
> else
> - core2_vpmu_cxt->enabled_cntrs &= ~(1ULL << tmp);
> + *enabled_cntrs &= ~(1ULL << tmp);
>
> - core2_vpmu_cxt->arch_msr_pair[tmp].control = msr_content;
> + xen_pmu_cntr_pair[tmp].control = msr_content;
> }
> }
>
> - if ((global_ctrl & core2_vpmu_cxt->enabled_cntrs) ||
> - (core2_vpmu_cxt->ds_area != 0) )
> + if ((global_ctrl & *enabled_cntrs) || (core2_vpmu_cxt->ds_area != 0) )
> vpmu_set(vpmu, VPMU_RUNNING);
> else
> vpmu_reset(vpmu, VPMU_RUNNING);
> @@ -561,7 +576,7 @@ static int core2_vpmu_do_rdmsr(unsigned int msr, uint64_t
> *msr_content)
> int type = -1, index = -1;
> struct vcpu *v = current;
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> - struct core2_vpmu_context *core2_vpmu_cxt = NULL;
> + struct xen_pmu_intel_ctxt *core2_vpmu_cxt = NULL;
>
> if ( core2_vpmu_msr_common_check(msr, &type, &index) )
> {
> @@ -572,7 +587,7 @@ static int core2_vpmu_do_rdmsr(unsigned int msr, uint64_t
> *msr_content)
> *msr_content = 0;
> break;
> case MSR_CORE_PERF_GLOBAL_STATUS:
> - *msr_content = core2_vpmu_cxt->global_ovf_status;
> + *msr_content = core2_vpmu_cxt->global_status;
> break;
> case MSR_CORE_PERF_GLOBAL_CTRL:
> vmx_read_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL, msr_content);
> @@ -621,8 +636,11 @@ static void core2_vpmu_dump(const struct vcpu *v)
> {
> const struct vpmu_struct *vpmu = vcpu_vpmu(v);
> int i;
> - const struct core2_vpmu_context *core2_vpmu_cxt = NULL;
> + const struct xen_pmu_intel_ctxt *core2_vpmu_cxt = NULL;
> u64 val;
> + uint64_t *fixed_counters = vpmu_reg_pointer(core2_vpmu_cxt,
> fixed_counters);
> + struct xen_pmu_cntr_pair *xen_pmu_cntr_pair =
> + vpmu_reg_pointer(core2_vpmu_cxt, arch_counters);
This crashs the hypervisor because of derefencing core2_vpmu_cxt (NULL) in
vpmu_req_pointer().
--> + const struct xen_pmu_intel_ctxt *core2_vpmu_cxt = vpmu->context;
Dietmar.
>
> if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
> return;
> @@ -641,12 +659,9 @@ static void core2_vpmu_dump(const struct vcpu *v)
>
> /* Print the contents of the counter and its configuration msr. */
> for ( i = 0; i < arch_pmc_cnt; i++ )
> - {
> - const struct arch_msr_pair *msr_pair = core2_vpmu_cxt->arch_msr_pair;
> -
> printk(" general_%d: 0x%016lx ctrl: 0x%016lx\n",
> - i, msr_pair[i].counter, msr_pair[i].control);
> - }
> + i, xen_pmu_cntr_pair[i].counter, xen_pmu_cntr_pair[i].control);
> +
> /*
> * The configuration of the fixed counter is 4 bits each in the
> * MSR_CORE_PERF_FIXED_CTR_CTRL.
> @@ -655,7 +670,7 @@ static void core2_vpmu_dump(const struct vcpu *v)
> for ( i = 0; i < fixed_pmc_cnt; i++ )
> {
> printk(" fixed_%d: 0x%016lx ctrl: %#lx\n",
> - i, core2_vpmu_cxt->fix_counters[i],
> + i, fixed_counters[i],
> val & FIXED_CTR_CTRL_MASK);
> val >>= FIXED_CTR_CTRL_BITS;
> }
> @@ -666,14 +681,14 @@ static int core2_vpmu_do_interrupt(struct cpu_user_regs
> *regs)
> struct vcpu *v = current;
> u64 msr_content;
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> - struct core2_vpmu_context *core2_vpmu_cxt = vpmu->context;
> + struct xen_pmu_intel_ctxt *core2_vpmu_cxt = vpmu->context;
>
> rdmsrl(MSR_CORE_PERF_GLOBAL_STATUS, msr_content);
> if ( msr_content )
> {
> if ( is_pmc_quirk )
> handle_pmc_quirk(msr_content);
> - core2_vpmu_cxt->global_ovf_status |= msr_content;
> + core2_vpmu_cxt->global_status |= msr_content;
> msr_content = 0xC000000700000000 | ((1 << arch_pmc_cnt) - 1);
> wrmsrl(MSR_CORE_PERF_GLOBAL_OVF_CTRL, msr_content);
> }
> @@ -736,12 +751,6 @@ func_out:
>
> arch_pmc_cnt = core2_get_arch_pmc_count();
> fixed_pmc_cnt = core2_get_fixed_pmc_count();
> - if ( fixed_pmc_cnt > VPMU_CORE2_MAX_FIXED_PMCS )
> - {
> - fixed_pmc_cnt = VPMU_CORE2_MAX_FIXED_PMCS;
> - printk(XENLOG_G_WARNING "Limiting number of fixed counters to %d\n",
> - fixed_pmc_cnt);
> - }
> check_pmc_quirk();
>
> return 0;
> @@ -755,6 +764,7 @@ static void core2_vpmu_destroy(struct vcpu *v)
> return;
>
> xfree(vpmu->context);
> + xfree(vpmu->priv_context);
> if ( cpu_has_vmx_msr_bitmap && is_hvm_domain(v->domain) )
> core2_vpmu_unset_msr_bitmap(v->arch.hvm_vmx.msr_bitmap);
> release_pmu_ownship(PMU_OWNER_HVM);
> diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
> index 0340e5b..0acc486 100644
> --- a/xen/arch/x86/hvm/vpmu.c
> +++ b/xen/arch/x86/hvm/vpmu.c
> @@ -31,6 +31,7 @@
> #include <asm/hvm/svm/svm.h>
> #include <asm/hvm/svm/vmcb.h>
> #include <asm/apic.h>
> +#include <public/pmu.h>
>
> /*
> * "vpmu" : vpmu generally enabled
> diff --git a/xen/arch/x86/oprofile/op_model_ppro.c
> b/xen/arch/x86/oprofile/op_model_ppro.c
> index 3225937..5aae2e7 100644
> --- a/xen/arch/x86/oprofile/op_model_ppro.c
> +++ b/xen/arch/x86/oprofile/op_model_ppro.c
> @@ -20,11 +20,15 @@
> #include <asm/regs.h>
> #include <asm/current.h>
> #include <asm/hvm/vpmu.h>
> -#include <asm/hvm/vmx/vpmu_core2.h>
>
> #include "op_x86_model.h"
> #include "op_counter.h"
>
> +struct arch_msr_pair {
> + u64 counter;
> + u64 control;
> +};
> +
> /*
> * Intel "Architectural Performance Monitoring" CPUID
> * detection/enumeration details:
> diff --git a/xen/include/asm-x86/hvm/vmx/vpmu_core2.h
> b/xen/include/asm-x86/hvm/vmx/vpmu_core2.h
> deleted file mode 100644
> index 410372d..0000000
> --- a/xen/include/asm-x86/hvm/vmx/vpmu_core2.h
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -
> -/*
> - * vpmu_core2.h: CORE 2 specific PMU virtualization for HVM domain.
> - *
> - * Copyright (c) 2007, Intel Corporation.
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms and conditions of the GNU General Public License,
> - * version 2, as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> - * more details.
> - *
> - * You should have received a copy of the GNU General Public License along
> with
> - * this program; if not, write to the Free Software Foundation, Inc., 59
> Temple
> - * Place - Suite 330, Boston, MA 02111-1307 USA.
> - *
> - * Author: Haitao Shan <haitao.shan@xxxxxxxxx>
> - */
> -
> -#ifndef __ASM_X86_HVM_VPMU_CORE_H_
> -#define __ASM_X86_HVM_VPMU_CORE_H_
> -
> -struct arch_msr_pair {
> - u64 counter;
> - u64 control;
> -};
> -
> -#endif /* __ASM_X86_HVM_VPMU_CORE_H_ */
> -
> diff --git a/xen/include/asm-x86/hvm/vpmu.h b/xen/include/asm-x86/hvm/vpmu.h
> index 7ee0f01..3e5d9de 100644
> --- a/xen/include/asm-x86/hvm/vpmu.h
> +++ b/xen/include/asm-x86/hvm/vpmu.h
> @@ -22,6 +22,8 @@
> #ifndef __ASM_X86_HVM_VPMU_H_
> #define __ASM_X86_HVM_VPMU_H_
>
> +#include <public/pmu.h>
> +
> /*
> * Flag bits given as a string on the hypervisor boot parameter 'vpmu'.
> * See arch/x86/hvm/vpmu.c.
> @@ -29,12 +31,9 @@
> #define VPMU_BOOT_ENABLED 0x1 /* vpmu generally enabled. */
> #define VPMU_BOOT_BTS 0x2 /* Intel BTS feature wanted. */
>
> -
> -#define msraddr_to_bitpos(x) (((x)&0xffff) + ((x)>>31)*0x2000)
> #define vcpu_vpmu(vcpu) (&((vcpu)->arch.hvm_vcpu.vpmu))
> #define vpmu_vcpu(vpmu) (container_of((vpmu), struct vcpu, \
> arch.hvm_vcpu.vpmu))
> -#define vpmu_domain(vpmu) (vpmu_vcpu(vpmu)->domain)
>
> #define MSR_TYPE_COUNTER 0
> #define MSR_TYPE_CTRL 1
> @@ -42,6 +41,9 @@
> #define MSR_TYPE_ARCH_COUNTER 3
> #define MSR_TYPE_ARCH_CTRL 4
>
> +/* Start of PMU register bank */
> +#define vpmu_reg_pointer(ctxt, offset) ((void *)((uintptr_t)ctxt + \
> + (uintptr_t)ctxt->offset))
>
> /* Arch specific operations shared by all vpmus */
> struct arch_vpmu_ops {
> @@ -64,7 +66,8 @@ struct vpmu_struct {
> u32 flags;
> u32 last_pcpu;
> u32 hw_lapic_lvtpc;
> - void *context;
> + void *context; /* May be shared with PV guest */
> + void *priv_context; /* hypervisor-only */
> struct arch_vpmu_ops *arch_vpmu_ops;
> };
>
> @@ -76,11 +79,6 @@ struct vpmu_struct {
> #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))
> #define vpmu_reset(_vpmu, _x) ((_vpmu)->flags &= ~(_x))
> #define vpmu_is_set(_vpmu, _x) ((_vpmu)->flags & (_x))
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 7496556..e982b53 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -388,6 +388,9 @@ typedef uint64_t xen_callback_t;
>
> #endif
>
> +/* Stub definition of PMU structure */
> +typedef struct xen_arch_pmu {} xen_arch_pmu_t;
> +
> #endif /* __XEN_PUBLIC_ARCH_ARM_H__ */
>
> /*
> diff --git a/xen/include/public/arch-x86/pmu.h
> b/xen/include/public/arch-x86/pmu.h
> new file mode 100644
> index 0000000..b4eda67
> --- /dev/null
> +++ b/xen/include/public/arch-x86/pmu.h
> @@ -0,0 +1,62 @@
> +#ifndef __XEN_PUBLIC_ARCH_X86_PMU_H__
> +#define __XEN_PUBLIC_ARCH_X86_PMU_H__
> +
> +/* x86-specific PMU definitions */
> +
> +/* AMD PMU registers and structures */
> +struct xen_pmu_amd_ctxt {
> + uint32_t counters; /* Offset to counter MSRs */
> + uint32_t ctrls; /* Offset to control MSRs */
> +};
> +
> +/* Intel PMU registers and structures */
> +struct xen_pmu_cntr_pair {
> + uint64_t counter;
> + uint64_t control;
> +};
> +
> +struct xen_pmu_intel_ctxt {
> + uint64_t global_ctrl;
> + uint64_t global_ovf_ctrl;
> + uint64_t global_status;
> + uint64_t fixed_ctrl;
> + uint64_t ds_area;
> + uint64_t pebs_enable;
> + uint64_t debugctl;
> + uint32_t fixed_counters; /* Offset to fixed counter MSRs */
> + uint32_t arch_counters; /* Offset to architectural counter MSRs */
> +};
> +
> +#define XENPMU_MAX_CTXT_SZ (sizeof(struct xen_pmu_amd_ctxt) > \
> + sizeof(struct xen_pmu_intel_ctxt) ? \
> + sizeof(struct xen_pmu_amd_ctxt) : \
> + sizeof(struct xen_pmu_intel_ctxt))
> +#define XENPMU_CTXT_PAD_SZ (((XENPMU_MAX_CTXT_SZ + 64) & ~63) + 128)
> +struct xen_arch_pmu {
> + union {
> + struct cpu_user_regs regs;
> + uint8_t pad1[256];
> + } r;
> + union {
> + uint32_t lapic_lvtpc;
> + uint64_t pad2;
> + } l;
> + union {
> + struct xen_pmu_amd_ctxt amd;
> + struct xen_pmu_intel_ctxt intel;
> + uint8_t pad3[XENPMU_CTXT_PAD_SZ];
> + } c;
> +};
> +typedef struct xen_arch_pmu xen_arch_pmu_t;
> +
> +#endif /* __XEN_PUBLIC_ARCH_X86_PMU_H__ */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> +
> diff --git a/xen/include/public/pmu.h b/xen/include/public/pmu.h
> new file mode 100644
> index 0000000..3ffd2cf
> --- /dev/null
> +++ b/xen/include/public/pmu.h
> @@ -0,0 +1,38 @@
> +#ifndef __XEN_PUBLIC_PMU_H__
> +#define __XEN_PUBLIC_PMU_H__
> +
> +#include "xen.h"
> +#if defined(__i386__) || defined(__x86_64__)
> +#include "arch-x86/pmu.h"
> +#elif defined (__arm__) || defined (__aarch64__)
> +#include "arch-arm.h"
> +#else
> +#error "Unsupported architecture"
> +#endif
> +
> +#define XENPMU_VER_MAJ 0
> +#define XENPMU_VER_MIN 0
> +
> +
> +/* Shared between hypervisor and PV domain */
> +struct xen_pmu_data {
> + uint32_t domain_id;
> + uint32_t vcpu_id;
> + uint32_t pcpu_id;
> + uint32_t pmu_flags;
> +
> + xen_arch_pmu_t pmu;
> +};
> +typedef struct xen_pmu_data xen_pmu_data_t;
> +
> +#endif /* __XEN_PUBLIC_PMU_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
>
--
Company details: http://ts.fujitsu.com/imprint.html
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |