Re: [Xen-devel] [PATCH v12 for-xen-4.5 12/20] x86/VPMU: Initialize PMU for PV(H) guests

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Boris Ostrovsky [mailto:boris.ostrovsky@xxxxxxxxxx]
> Sent: Thursday, September 25, 2014 12:29 PM
> 
> Code for initializing/tearing down PMU for PV guests
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>

> ---
>  tools/flask/policy/policy/modules/xen/xen.te |  4 ++
>  xen/arch/x86/hvm/hvm.c                       |  3 +-
>  xen/arch/x86/hvm/svm/svm.c                   |  4 +-
>  xen/arch/x86/hvm/svm/vpmu.c                  | 43 +++++++++-----
>  xen/arch/x86/hvm/vmx/vmx.c                   |  4 +-
>  xen/arch/x86/hvm/vmx/vpmu_core2.c            | 83
> ++++++++++++++++++++--------
>  xen/arch/x86/hvm/vpmu.c                      | 81
> +++++++++++++++++++++++++++
>  xen/common/event_channel.c                   |  1 +
>  xen/include/asm-x86/hvm/vpmu.h               |  1 +
>  xen/include/public/pmu.h                     |  2 +
>  xen/include/public/xen.h                     |  1 +
>  xen/include/xsm/dummy.h                      |  3 +
>  xen/xsm/flask/hooks.c                        |  4 ++
>  xen/xsm/flask/policy/access_vectors          |  2 +
>  14 files changed, 195 insertions(+), 41 deletions(-)
> 
> diff --git a/tools/flask/policy/policy/modules/xen/xen.te
> b/tools/flask/policy/policy/modules/xen/xen.te
> index fb761cd..6744c36 100644
> --- a/tools/flask/policy/policy/modules/xen/xen.te
> +++ b/tools/flask/policy/policy/modules/xen/xen.te
> @@ -116,6 +116,10 @@ domain_comms(dom0_t, dom0_t)
>  # Allow all domains to use (unprivileged parts of) the tmem hypercall
>  allow domain_type xen_t:xen tmem_op;
> 
> +# Allow all domains to use PMU (but not to change its settings --- that's 
> what
> +# pmu_ctrl is for)
> +allow domain_type xen_t:xen2 pmu_use;
> +
> 
> ################################################################
> ###############
>  #
>  # Domain creation
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index bb45593..ec4a021 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4832,7 +4832,8 @@ static hvm_hypercall_t *const
> pvh_hypercall64_table[NR_hypercalls] = {
>      [ __HYPERVISOR_physdev_op ]      = (hvm_hypercall_t
> *)hvm_physdev_op,
>      HYPERCALL(hvm_op),
>      HYPERCALL(sysctl),
> -    HYPERCALL(domctl)
> +    HYPERCALL(domctl),
> +    HYPERCALL(xenpmu_op)
>  };
> 
>  int hvm_do_hypercall(struct cpu_user_regs *regs)
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 5d404ce..319e5da 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1157,7 +1157,9 @@ static int svm_vcpu_initialise(struct vcpu *v)
>          return rc;
>      }
> 
> -    vpmu_initialise(v);
> +    /* PVH's VPMU is initialized via hypercall */
> +    if ( is_hvm_domain(v->domain) )
> +        vpmu_initialise(v);
> 
>      svm_guest_osvw_init(v);
> 
> diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
> index 37d8228..be3ab27 100644
> --- a/xen/arch/x86/hvm/svm/vpmu.c
> +++ b/xen/arch/x86/hvm/svm/vpmu.c
> @@ -362,6 +362,7 @@ static int amd_vpmu_initialise(struct vcpu *v)
>      struct xen_pmu_amd_ctxt *ctxt;
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
>      uint8_t family = current_cpu_data.x86;
> +    unsigned int regs_size;
> 
>      if ( vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
>          return 0;
> @@ -389,14 +390,26 @@ static int amd_vpmu_initialise(struct vcpu *v)
>        }
>      }
> 
> -    ctxt = xzalloc_bytes(sizeof(struct xen_pmu_amd_ctxt) +
> -                         2 * sizeof(uint64_t) * AMD_MAX_COUNTERS);
> -    if ( !ctxt )
> +    regs_size = 2 * sizeof(uint64_t) * AMD_MAX_COUNTERS;
> +    if ( is_hvm_domain(v->domain) )
>      {
> -        gdprintk(XENLOG_WARNING, "Insufficient memory for PMU, "
> -            " PMU feature is unavailable on domain %d vcpu %d.\n",
> -            v->vcpu_id, v->domain->domain_id);
> -        return -ENOMEM;
> +        ctxt = xzalloc_bytes(sizeof(struct xen_pmu_amd_ctxt) + regs_size);
> +        if ( !ctxt )
> +        {
> +            gdprintk(XENLOG_WARNING, "Insufficient memory for PMU, "
> +                "PMU feature is unavailable\n");
> +            return -ENOMEM;
> +        }
> +    }
> +    else
> +    {
> +        if ( sizeof(struct xen_pmu_data) + regs_size > PAGE_SIZE )
> +        {
> +            gdprintk(XENLOG_WARNING,
> +                    "Register bank does not fit into VPMU shared
> page\n");
> +            return -ENOSPC;
> +        }
> +        ctxt = &v->arch.vpmu.xenpmu_data->pmu.c.amd;
>      }
> 
>      ctxt->counters = sizeof(struct xen_pmu_amd_ctxt);
> @@ -415,17 +428,19 @@ static void amd_vpmu_destroy(struct vcpu *v)
>      if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
>          return;
> 
> -    if ( has_hvm_container_domain(v->domain) &&
> is_msr_bitmap_on(vpmu) )
> -        amd_vpmu_unset_msr_bitmap(v);
> +    if ( has_hvm_container_domain(v->domain) )
> +    {
> +        if ( is_msr_bitmap_on(vpmu) )
> +            amd_vpmu_unset_msr_bitmap(v);
> 
> -    xfree(vpmu->context);
> -    vpmu_reset(vpmu, VPMU_CONTEXT_ALLOCATED);
> +        if ( is_hvm_domain(v->domain) )
> +            xfree(vpmu->context);
> 
> -    if ( vpmu_is_set(vpmu, VPMU_RUNNING) )
> -    {
> -        vpmu_reset(vpmu, VPMU_RUNNING);
>          release_pmu_ownship(PMU_OWNER_HVM);
>      }
> +
> +    vpmu->context = NULL;
> +    vpmu_clear(vpmu);
>  }
> 
>  /* VPMU part of the 'q' keyhandler */
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 84119ed..bebe879 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -116,7 +116,9 @@ static int vmx_vcpu_initialise(struct vcpu *v)
>          return rc;
>      }
> 
> -    vpmu_initialise(v);
> +    /* PVH's VPMU is initialized via hypercall */
> +    if ( is_hvm_domain(v->domain) )
> +        vpmu_initialise(v);
> 
>      vmx_install_vlapic_mapping(v);
> 
> diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> index c0a45cd..5c0f99a 100644
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> @@ -356,25 +356,45 @@ static int core2_vpmu_alloc_resource(struct vcpu
> *v)
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
>      struct xen_pmu_intel_ctxt *core2_vpmu_cxt = NULL;
>      uint64_t *p = NULL;
> +    unsigned int regs_size;
> 
> -    if ( !acquire_pmu_ownership(PMU_OWNER_HVM) )
> -        return 0;
> -
> -    wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, 0);
> -    if ( vmx_add_host_load_msr(MSR_CORE_PERF_GLOBAL_CTRL) )
> +    p = xzalloc_bytes(sizeof(uint64_t));
> +    if ( !p )
>          goto out_err;
> 
> -    if ( vmx_add_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL) )
> -        goto out_err;
> -    vmx_write_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
> -
> -    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(uint64_t);
> -    if ( !core2_vpmu_cxt || !p )
> -        goto out_err;
> +    if ( has_hvm_container_domain(v->domain) )
> +    {
> +        if ( is_hvm_domain(v->domain)
> && !acquire_pmu_ownership(PMU_OWNER_HVM) )
> +            goto out_err;
> +
> +        wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, 0);
> +        if ( vmx_add_host_load_msr(MSR_CORE_PERF_GLOBAL_CTRL) )
> +            goto out_err_hvm;
> +        if ( vmx_add_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL) )
> +            goto out_err_hvm;
> +        vmx_write_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
> +    }
> +
> +    regs_size = sizeof(uint64_t) * fixed_pmc_cnt +
> +                sizeof(struct xen_pmu_cntr_pair) * arch_pmc_cnt;
> +    if ( is_hvm_domain(v->domain) )
> +    {
> +        core2_vpmu_cxt = xzalloc_bytes(sizeof(struct xen_pmu_intel_ctxt)
> +
> +                                       regs_size);
> +        if ( !core2_vpmu_cxt )
> +            goto out_err_hvm;
> +    }
> +    else
> +    {
> +        if ( sizeof(struct xen_pmu_data) + regs_size > PAGE_SIZE )
> +        {
> +            printk(XENLOG_WARNING
> +                   "Register bank does not fit into VPMU share page\n");
> +            goto out_err_hvm;
> +        }
> +
> +        core2_vpmu_cxt = &v->arch.vpmu.xenpmu_data->pmu.c.intel;
> +    }
> 
>      core2_vpmu_cxt->fixed_counters = sizeof(struct xen_pmu_intel_ctxt);
>      core2_vpmu_cxt->arch_counters = core2_vpmu_cxt->fixed_counters +
> @@ -387,10 +407,12 @@ static int core2_vpmu_alloc_resource(struct vcpu
> *v)
> 
>      return 1;
> 
> -out_err:
> -    release_pmu_ownship(PMU_OWNER_HVM);
> -
> + out_err_hvm:
>      xfree(core2_vpmu_cxt);
> +    if ( is_hvm_domain(v->domain) )
> +        release_pmu_ownship(PMU_OWNER_HVM);
> +
> + out_err:
>      xfree(p);
> 
>      printk("Failed to allocate VPMU resources for domain %u vcpu %u\n",
> @@ -756,6 +778,11 @@ static int core2_vpmu_initialise(struct vcpu *v)
>      arch_pmc_cnt = core2_get_arch_pmc_count();
>      fixed_pmc_cnt = core2_get_fixed_pmc_count();
>      check_pmc_quirk();
> +
> +    /* PV domains can allocate resources immediately */
> +    if ( is_pv_domain(v->domain) && !core2_vpmu_alloc_resource(v) )
> +        return -EIO;
> +
>      return 0;
>  }
> 
> @@ -766,12 +793,20 @@ static void core2_vpmu_destroy(struct vcpu *v)
>      if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
>          return;
> 
> -    xfree(vpmu->context);
> +    if ( has_hvm_container_domain(v->domain) )
> +    {
> +        if ( cpu_has_vmx_msr_bitmap )
> +
> core2_vpmu_unset_msr_bitmap(v->arch.hvm_vmx.msr_bitmap);
> +
> +        if ( is_hvm_domain(v->domain) )
> +            xfree(vpmu->context);
> +
> +        release_pmu_ownship(PMU_OWNER_HVM);
> +    }
> +
>      xfree(vpmu->priv_context);
> -    if ( has_hvm_container_domain(v->domain) &&
> cpu_has_vmx_msr_bitmap )
> -        core2_vpmu_unset_msr_bitmap(v->arch.hvm_vmx.msr_bitmap);
> -    release_pmu_ownship(PMU_OWNER_HVM);
> -    vpmu_reset(vpmu, VPMU_CONTEXT_ALLOCATED);
> +    vpmu->context = NULL;
> +    vpmu_clear(vpmu);
>  }
> 
>  struct arch_vpmu_ops core2_vpmu_ops = {
> diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
> index 5fcee0e..dde3367 100644
> --- a/xen/arch/x86/hvm/vpmu.c
> +++ b/xen/arch/x86/hvm/vpmu.c
> @@ -26,6 +26,7 @@
>  #include <asm/regs.h>
>  #include <asm/types.h>
>  #include <asm/msr.h>
> +#include <asm/p2m.h>
>  #include <asm/hvm/support.h>
>  #include <asm/hvm/vmx/vmx.h>
>  #include <asm/hvm/vmx/vmcs.h>
> @@ -256,6 +257,7 @@ void vpmu_initialise(struct vcpu *v)
>          vpmu_destroy(v);
>      vpmu_clear(vpmu);
>      vpmu->context = NULL;
> +    vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR | APIC_LVT_MASKED;
> 
>      switch ( vendor )
>      {
> @@ -282,7 +284,74 @@ void vpmu_destroy(struct vcpu *v)
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
> 
>      if ( vpmu->arch_vpmu_ops &&
> vpmu->arch_vpmu_ops->arch_vpmu_destroy )
> +    {
> +        /* Unload VPMU first. This will stop counters */
> +        on_selected_cpus(cpumask_of(vcpu_vpmu(v)->last_pcpu),
> +                         vpmu_save_force, v, 1);
> +
>          vpmu->arch_vpmu_ops->arch_vpmu_destroy(v);
> +    }
> +}
> +
> +static int pvpmu_init(struct domain *d, xen_pmu_params_t *params)
> +{
> +    struct vcpu *v;
> +    struct page_info *page;
> +    uint64_t gfn = params->val;
> +
> +    if ( (params->vcpu >= d->max_vcpus) || (d->vcpu == NULL) ||
> +         (d->vcpu[params->vcpu] == NULL) )
> +        return -EINVAL;
> +
> +    page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
> +    if ( !page )
> +        return -EINVAL;
> +
> +    if ( !get_page_type(page, PGT_writable_page) )
> +    {
> +        put_page(page);
> +        return -EINVAL;
> +    }
> +
> +    v = d->vcpu[params->vcpu];
> +    v->arch.vpmu.xenpmu_data = __map_domain_page_global(page);
> +    if ( !v->arch.vpmu.xenpmu_data )
> +    {
> +        put_page_and_type(page);
> +        return -EINVAL;
> +    }
> +
> +    vpmu_initialise(v);
> +
> +    return 0;
> +}
> +
> +static void pvpmu_finish(struct domain *d, xen_pmu_params_t *params)
> +{
> +    struct vcpu *v;
> +    uint64_t mfn;
> +
> +    if ( (params->vcpu >= d->max_vcpus) || (d->vcpu == NULL) ||
> +         (d->vcpu[params->vcpu] == NULL) )
> +        return;
> +
> +    v = d->vcpu[params->vcpu];
> +    if ( v != current )
> +        vcpu_pause(v);
> +
> +    if ( v->arch.vpmu.xenpmu_data )
> +    {
> +        mfn = domain_page_map_to_mfn(v->arch.vpmu.xenpmu_data);
> +        if ( mfn_valid(mfn) )
> +        {
> +            unmap_domain_page_global(v->arch.vpmu.xenpmu_data);
> +            put_page_and_type(mfn_to_page(mfn));
> +        }
> +    }
> +    vpmu_destroy(v);
> +
> +    if ( v != current )
> +        vcpu_unpause(v);
>  }
> 
>  /* Dump some vpmu informations on console. Used in keyhandler
> dump_domains(). */
> @@ -460,6 +529,18 @@ long do_xenpmu_op(int op,
> XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
>              return -EFAULT;
>          break;
> 
> +    case XENPMU_init:
> +        if ( copy_from_guest(&pmu_params, arg, 1) )
> +            return -EFAULT;
> +        ret = pvpmu_init(current->domain, &pmu_params);
> +        break;
> +
> +    case XENPMU_finish:
> +        if ( copy_from_guest(&pmu_params, arg, 1) )
> +            return -EFAULT;
> +        pvpmu_finish(current->domain, &pmu_params);
> +        break;
> +
>      default:
>          ret = -EINVAL;
>      }
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 7d6de54..a991b2d 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -108,6 +108,7 @@ static int virq_is_global(uint32_t virq)
>      case VIRQ_TIMER:
>      case VIRQ_DEBUG:
>      case VIRQ_XENOPROF:
> +    case VIRQ_XENPMU:
>          rc = 0;
>          break;
>      case VIRQ_ARCH_0 ... VIRQ_ARCH_7:
> diff --git a/xen/include/asm-x86/hvm/vpmu.h
> b/xen/include/asm-x86/hvm/vpmu.h
> index c612e1a..93f1fc2 100644
> --- a/xen/include/asm-x86/hvm/vpmu.h
> +++ b/xen/include/asm-x86/hvm/vpmu.h
> @@ -62,6 +62,7 @@ struct vpmu_struct {
>      void *context;      /* May be shared with PV guest */
>      void *priv_context; /* hypervisor-only */
>      struct arch_vpmu_ops *arch_vpmu_ops;
> +    xen_pmu_data_t *xenpmu_data;
>  };
> 
>  /* VPMU states */
> diff --git a/xen/include/public/pmu.h b/xen/include/public/pmu.h
> index c2293be..b8c5682 100644
> --- a/xen/include/public/pmu.h
> +++ b/xen/include/public/pmu.h
> @@ -25,6 +25,8 @@
>  #define XENPMU_mode_set        1
>  #define XENPMU_feature_get     2
>  #define XENPMU_feature_set     3
> +#define XENPMU_init            4
> +#define XENPMU_finish          5
>  /* ` } */
> 
>  /* Parameters structure for HYPERVISOR_xenpmu_op call */
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 0766790..e4d0b79 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -161,6 +161,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
>  #define VIRQ_MEM_EVENT  10 /* G. (DOM0) A memory event has occured
> */
>  #define VIRQ_XC_RESERVED 11 /* G. Reserved for XenClient
> */
>  #define VIRQ_ENOMEM     12 /* G. (DOM0) Low on heap memory
> */
> +#define VIRQ_XENPMU     13 /* V.  PMC interrupt
> */
> 
>  /* Architecture-specific VIRQ definitions. */
>  #define VIRQ_ARCH_0    16
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index d423c1c..29dae2e 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -663,6 +663,9 @@ static XSM_INLINE int xsm_pmu_op
> (XSM_DEFAULT_ARG struct domain *d, int op)
>      case XENPMU_feature_set:
>      case XENPMU_feature_get:
>          return xsm_default_action(XSM_PRIV, d, current->domain);
> +    case XENPMU_init:
> +    case XENPMU_finish:
> +        return xsm_default_action(XSM_HOOK, d, current->domain);
>      default:
>          return -EPERM;
>      }
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index b437a24..8bd4a3d 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1498,6 +1498,10 @@ static int flask_pmu_op (struct domain *d, int op)
>      case XENPMU_feature_get:
>          return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_XEN2,
>                              XEN2__PMU_CTRL, NULL);
> +    case XENPMU_init:
> +    case XENPMU_finish:
> +        return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_XEN2,
> +                            XEN2__PMU_USE, NULL);
>      default:
>          return -EPERM;
>      }
> diff --git a/xen/xsm/flask/policy/access_vectors
> b/xen/xsm/flask/policy/access_vectors
> index 64c7378..36b69c6 100644
> --- a/xen/xsm/flask/policy/access_vectors
> +++ b/xen/xsm/flask/policy/access_vectors
> @@ -83,6 +83,8 @@ class xen2
>      get_symbol
>  # PMU control
>      pmu_ctrl
> +# PMU use (anyone has access)
> +    pmu_use
>  }
> 
>  # Classes domain and domain2 consist of operations that a domain performs
> on
> --
> 1.8.1.4


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