Re: [Xen-devel] [PATCH v5 RESEND 16/17] x86/VPMU: Suport for PVH guests

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

> From: Boris Ostrovsky [mailto:boris.ostrovsky@xxxxxxxxxx]
> Sent: Wednesday, April 23, 2014 8:51 PM
> 
> Add support for PVH guests. Most of operations are performed as in an HVM
> guest.
> However, interrupt management is done in PV-like manner.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>

If others are OK with the various ***_domain checks, I'm OK with it.

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

> ---
>  xen/arch/x86/hvm/hvm.c            |  3 ++-
>  xen/arch/x86/hvm/svm/vpmu.c       | 13 +++++++------
>  xen/arch/x86/hvm/vmx/vpmu_core2.c | 24 ++++++++++++------------
>  xen/arch/x86/hvm/vpmu.c           | 34
> ++++++++++++++++++++++------------
>  4 files changed, 43 insertions(+), 31 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 69f7e74..1e50c35 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3451,7 +3451,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/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
> index 04d3b91..0e5dac4 100644
> --- a/xen/arch/x86/hvm/svm/vpmu.c
> +++ b/xen/arch/x86/hvm/svm/vpmu.c
> @@ -162,6 +162,7 @@ static void amd_vpmu_unset_msr_bitmap(struct vcpu
> *v)
>      ctxt->msr_bitmap_set = 0;
>  }
> 
> +/* Must be NMI-safe */
>  static int amd_vpmu_do_interrupt(struct cpu_user_regs *regs)
>  {
>      return 1;
> @@ -243,7 +244,7 @@ static int amd_vpmu_save(struct vcpu *v)
> 
>      context_save(v);
> 
> -    if ( is_hvm_domain(v->domain) &&
> +    if ( has_hvm_container_domain(v->domain) &&
>          !vpmu_is_set(vpmu, VPMU_RUNNING) && ctx->msr_bitmap_set )
>          amd_vpmu_unset_msr_bitmap(v);
> 
> @@ -286,7 +287,7 @@ static int amd_vpmu_do_wrmsr(unsigned int msr,
> uint64_t msr_content)
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
> 
>      /* For all counters, enable guest only mode for HVM guest */
> -    if ( is_hvm_domain(v->domain) && (get_pmu_reg_type(msr) ==
> MSR_TYPE_CTRL) &&
> +    if ( has_hvm_container_domain(v->domain) &&
> (get_pmu_reg_type(msr) == MSR_TYPE_CTRL) &&
>          !(is_guest_mode(msr_content)) )
>      {
>          set_guest_mode(msr_content);
> @@ -300,7 +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) &&
> +        if ( has_hvm_container_domain(v->domain) &&
>               !((struct xen_pmu_amd_ctxt
> *)vpmu->context)->msr_bitmap_set )
>              amd_vpmu_set_msr_bitmap(v);
>      }
> @@ -310,7 +311,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) &&
> +        if ( has_hvm_container_domain(v->domain) &&
>               ((struct xen_pmu_amd_ctxt
> *)vpmu->context)->msr_bitmap_set )
>              amd_vpmu_unset_msr_bitmap(v);
>          release_pmu_ownship(PMU_OWNER_HVM);
> @@ -382,7 +383,7 @@ static int amd_vpmu_initialise(struct vcpu *v)
>        }
>      }
> 
> -    if ( !is_pv_domain(v->domain) )
> +    if ( has_hvm_container_domain(v->domain) )
>      {
>          ctxt = xzalloc_bytes(sizeof(struct xen_pmu_amd_ctxt) +
>                               sizeof(uint64_t) * AMD_MAX_COUNTERS
> +
> @@ -413,7 +414,7 @@ static void amd_vpmu_destroy(struct vcpu *v)
>      if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
>          return;
> 
> -    if ( is_hvm_domain(v->domain) )
> +    if ( has_hvm_container_domain(v->domain) )
>      {
>          if ( ((struct xen_pmu_amd_ctxt
> *)vpmu->context)->msr_bitmap_set )
>              amd_vpmu_unset_msr_bitmap(v);
> diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> index e214f01..5a07817 100644
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> @@ -299,7 +299,7 @@ static inline void __core2_vpmu_save(struct vcpu *v)
>      for ( i = 0; i < arch_pmc_cnt; i++ )
>          rdmsrl(MSR_IA32_PERFCTR0 + i, xen_pmu_cntr_pair[i].counter);
> 
> -    if ( is_pv_domain(v->domain) )
> +    if ( !has_hvm_container_domain(v->domain) )
>          rdmsrl(MSR_CORE_PERF_GLOBAL_STATUS,
> core2_vpmu_cxt->global_status);
>  }
> 
> @@ -308,7 +308,7 @@ static int core2_vpmu_save(struct vcpu *v)
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
> 
> -    if ( is_pv_domain(v->domain) )
> +    if ( !has_hvm_container_domain(v->domain) )
>          wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, 0);
> 
>      if ( !vpmu_is_set_all(vpmu, VPMU_CONTEXT_SAVE |
> VPMU_CONTEXT_LOADED) )
> @@ -318,7 +318,7 @@ static int core2_vpmu_save(struct vcpu *v)
> 
>      /* Unset PMU MSR bitmap to trap lazy load. */
>      if ( !vpmu_is_set(vpmu, VPMU_RUNNING) &&
> cpu_has_vmx_msr_bitmap
> -        && !is_pv_domain(v->domain) )
> +        && has_hvm_container_domain(v->domain) )
>          core2_vpmu_unset_msr_bitmap(v->arch.hvm_vmx.msr_bitmap);
> 
>      return 1;
> @@ -349,7 +349,7 @@ static inline void __core2_vpmu_load(struct vcpu *v)
>      wrmsrl(MSR_IA32_DS_AREA, core2_vpmu_cxt->ds_area);
>      wrmsrl(MSR_IA32_PEBS_ENABLE, core2_vpmu_cxt->pebs_enable);
> 
> -    if ( is_pv_domain(v->domain) )
> +    if ( !has_hvm_container_domain(v->domain) )
>      {
>          wrmsrl(MSR_CORE_PERF_GLOBAL_OVF_CTRL,
> core2_vpmu_cxt->global_ovf_ctrl);
>          core2_vpmu_cxt->global_ovf_ctrl = 0;
> @@ -436,7 +436,7 @@ static int core2_vpmu_msr_common_check(u32
> msr_index, int *type, int *index)
>      {
>          __core2_vpmu_load(current);
>          vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
> -        if ( cpu_has_vmx_msr_bitmap &&
> is_hvm_domain(current->domain) )
> +        if ( cpu_has_vmx_msr_bitmap &&
> has_hvm_container_domain(current->domain) )
> 
> core2_vpmu_set_msr_bitmap(current->arch.hvm_vmx.msr_bitmap);
>      }
>      return 1;
> @@ -444,7 +444,7 @@ static int core2_vpmu_msr_common_check(u32
> msr_index, int *type, int *index)
> 
>  static void inject_trap(struct vcpu *v, unsigned int trapno)
>  {
> -    if ( !is_pv_domain(v->domain) )
> +    if ( has_hvm_container_domain(v->domain) )
>          hvm_inject_hw_exception(trapno, 0);
>      else
>          send_guest_trap(v->domain, v->vcpu_id, trapno);
> @@ -538,7 +538,7 @@ static int core2_vpmu_do_wrmsr(unsigned int msr,
> uint64_t msr_content)
>          break;
>      case MSR_CORE_PERF_FIXED_CTR_CTRL:
>          non_global_ctrl = msr_content;
> -        if ( !is_pv_domain(v->domain) )
> +        if ( has_hvm_container_domain(v->domain) )
>              vmx_read_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL,
> &global_ctrl);
>          else
>              rdmsrl(MSR_CORE_PERF_GLOBAL_CTRL, global_ctrl);
> @@ -558,7 +558,7 @@ static int core2_vpmu_do_wrmsr(unsigned int msr,
> uint64_t msr_content)
>              struct xen_pmu_cntr_pair *xen_pmu_cntr_pair =
>                  vpmu_reg_pointer(core2_vpmu_cxt, arch_counters);
> 
> -            if ( !is_pv_domain(v->domain) )
> +            if ( has_hvm_container_domain(v->domain) )
>                  vmx_read_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL,
> &global_ctrl);
>              else
>                  rdmsrl(MSR_CORE_PERF_GLOBAL_CTRL, global_ctrl);
> @@ -608,7 +608,7 @@ static int core2_vpmu_do_wrmsr(unsigned int msr,
> uint64_t msr_content)
>      }
>      else
>      {
> -       if ( !is_pv_domain(v->domain) )
> +       if ( has_hvm_container_domain(v->domain) )
>             vmx_write_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL,
> msr_content);
>         else
>             wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, msr_content);
> @@ -636,7 +636,7 @@ static int core2_vpmu_do_rdmsr(unsigned int msr,
> uint64_t *msr_content)
>              *msr_content = core2_vpmu_cxt->global_status;
>              break;
>          case MSR_CORE_PERF_GLOBAL_CTRL:
> -            if ( !is_pv_domain(v->domain) )
> +            if ( has_hvm_container_domain(v->domain) )
>                  vmx_read_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL,
> msr_content);
>              else
>                  rdmsrl(MSR_CORE_PERF_GLOBAL_CTRL, *msr_content);
> @@ -803,7 +803,7 @@ func_out:
>      check_pmc_quirk();
> 
>      /* PV domains can allocate resources immediately */
> -    if ( is_pv_domain(v->domain) && !core2_vpmu_alloc_resource(v) )
> +    if ( !has_hvm_container_domain(v->domain)
> && !core2_vpmu_alloc_resource(v) )
>              return 1;
> 
>      return 0;
> @@ -816,7 +816,7 @@ static void core2_vpmu_destroy(struct vcpu *v)
>      if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
>          return;
> 
> -    if ( is_hvm_domain(v->domain) )
> +    if ( has_hvm_container_domain(v->domain) )
>      {
>          xfree(vpmu->context);
>          if ( cpu_has_vmx_msr_bitmap )
> diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
> index cbe8cfd..3645e4c 100644
> --- a/xen/arch/x86/hvm/vpmu.c
> +++ b/xen/arch/x86/hvm/vpmu.c
> @@ -103,7 +103,7 @@ void vpmu_lvtpc_update(uint32_t val)
>      vpmu->hw_lapic_lvtpc = vpmu_interrupt_type | (val &
> APIC_LVT_MASKED);
> 
>      /* Postpone APIC updates for PV guests if PMU interrupt is pending */
> -    if ( !is_pv_domain(current->domain) ||
> +    if ( !has_hvm_container_domain(current->domain) ||
>           !(current->arch.vpmu.xenpmu_data &&
>             current->arch.vpmu.xenpmu_data->pmu_flags &
> PMU_CACHED) )
>          apic_write(APIC_LVTPC, vpmu->hw_lapic_lvtpc);
> @@ -147,7 +147,7 @@ int vpmu_do_wrmsr(unsigned int msr, uint64_t
> msr_content)
>           * and since do_wrmsr may load VPMU context we should save
>           * (and unload) it again.
>           */
> -        if ( !is_hvm_domain(current->domain) &&
> +        if ( !has_hvm_container_domain(current->domain) &&
>              (current->arch.vpmu.xenpmu_data->pmu_flags &
> PMU_CACHED) )
>          {
>              vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
> @@ -171,7 +171,7 @@ int vpmu_do_rdmsr(unsigned int msr, uint64_t
> *msr_content)
>      {
>          int ret = vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
> 
> -        if ( !is_hvm_domain(current->domain) &&
> +        if ( !has_hvm_container_domain(current->domain) &&
>              (current->arch.vpmu.xenpmu_data->pmu_flags &
> PMU_CACHED) )
>          {
>              vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
> @@ -200,13 +200,17 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
> 
>      if ( !is_hvm_domain(v->domain) || (vpmu_mode &
> XENPMU_MODE_PRIV) )
>      {
> -        /* PV guest or dom0 is doing system profiling */
> +        /* PV(H) guest or dom0 is doing system profiling */
>          struct cpu_user_regs *gregs;
>          int err;
> 
>          if ( v->arch.vpmu.xenpmu_data->pmu_flags & PMU_CACHED )
>              return 1;
> 
> +        if ( is_pvh_domain(current->domain) && !(vpmu_mode &
> XENPMU_MODE_PRIV) &&
> +             !vpmu->arch_vpmu_ops->do_interrupt(regs) )
> +            return 0;
> +
>          /* PV guest will be reading PMU MSRs from xenpmu_data */
>          vpmu_set(vpmu, VPMU_CONTEXT_SAVE |
> VPMU_CONTEXT_LOADED);
>          err = vpmu->arch_vpmu_ops->arch_vpmu_save(v);
> @@ -243,7 +247,7 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
>              else if ( !is_control_domain(current->domain) &&
>                        !is_idle_vcpu(current) )
>              {
> -                /* PV guest */
> +                /* PV(H) guest */
>                  gregs = guest_cpu_user_regs();
>                  memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
>                         gregs, sizeof(struct cpu_user_regs));
> @@ -253,7 +257,15 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
>                         regs, sizeof(struct cpu_user_regs));
> 
>              gregs = &v->arch.vpmu.xenpmu_data->pmu.r.regs;
> -            gregs->cs = (current->arch.flags & TF_kernel_mode) ? 0 : 0x3;
> +            if ( !is_pvh_domain(current->domain) )
> +                gregs->cs = (current->arch.flags & TF_kernel_mode) ? 0 :
> 0x3;
> +            else if ( !(vpmu_interrupt_type & APIC_DM_NMI) )
> +            {
> +                struct segment_register seg_cs;
> +
> +                hvm_get_segment_register(current, x86_seg_cs,
> &seg_cs);
> +                gregs->cs = seg_cs.attr.fields.dpl;
> +            }
>          }
>          else
>          {
> @@ -277,7 +289,8 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
>          v->arch.vpmu.xenpmu_data->vcpu_id = current->vcpu_id;
>          v->arch.vpmu.xenpmu_data->pcpu_id = smp_processor_id();
> 
> -        v->arch.vpmu.xenpmu_data->pmu_flags |= PMU_CACHED;
> +        if ( !is_pvh_domain(current->domain) || (vpmu_mode &
> XENPMU_MODE_PRIV) )
> +            v->arch.vpmu.xenpmu_data->pmu_flags |= PMU_CACHED;
>          apic_write(APIC_LVTPC, vpmu->hw_lapic_lvtpc |
> APIC_LVT_MASKED);
>          vpmu->hw_lapic_lvtpc |= APIC_LVT_MASKED;
> 
> @@ -404,7 +417,7 @@ void vpmu_load(struct vcpu *v)
> 
>      /* Only when PMU is counting, we load PMU context immediately. */
>      if ( !vpmu_is_set(vpmu, VPMU_RUNNING) ||
> -        (is_pv_domain(v->domain) && vpmu->xenpmu_data->pmu_flags &
> PMU_CACHED) )
> +        (!has_hvm_container_domain(v->domain) &&
> vpmu->xenpmu_data->pmu_flags & PMU_CACHED) )
>          return;
> 
>      if ( vpmu->arch_vpmu_ops &&
> vpmu->arch_vpmu_ops->arch_vpmu_load )
> @@ -521,7 +534,7 @@ static void pmu_softnmi(void)
>      }
> 
>      regs = &v->arch.vpmu.xenpmu_data->pmu.r.regs;
> -    if ( !is_pv_domain(sampled->domain) )
> +    if ( has_hvm_container_domain(sampled->domain) )
>      {
>          struct segment_register cs;
> 
> @@ -544,9 +557,6 @@ static int pvpmu_init(struct domain *d,
> xen_pmu_params_t *params)
>      uint64_t gfn = params->d.val;
>      static bool_t __read_mostly pvpmu_initted = 0;
> 
> -    if ( !is_pv_domain(d) )
> -        return -EINVAL;
> -
>      if ( params->vcpu < 0 || params->vcpu >= d->max_vcpus )
>          return -EINVAL;
> 
> --
> 1.8.3.1


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