Re: [PATCH v11 for-xen-4.5 18/20] x86/VPMU: Add privileged PMU mode

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Sep 22, 2014 at 07:57:59PM -0400, Boris Ostrovsky wrote:
> Add support for privileged PMU mode (XENPMU_MODE_ALL) which allows privileged
> domain (dom0) profile both itself (and the hypervisor) and the guests. While
> this mode is on profiling in guests is disabled.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  xen/arch/x86/hvm/vpmu.c  | 26 ++++++++++++++++++--------
>  xen/arch/x86/traps.c     | 12 ++++++++++++
>  xen/include/public/pmu.h |  3 +++
>  3 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
> index 4a98a31..1e0575a 100644
> --- a/xen/arch/x86/hvm/vpmu.c
> +++ b/xen/arch/x86/hvm/vpmu.c
> @@ -98,7 +98,9 @@ int vpmu_do_msr(unsigned int msr, uint64_t *msr_content,
>      struct arch_vpmu_ops *ops;
>      int ret = 0;
>  
> -    if ( !(vpmu_mode & (XENPMU_MODE_SELF | XENPMU_MODE_HV)) )
> +    if ( (vpmu_mode == XENPMU_MODE_OFF) ||
> +         ((vpmu_mode & XENPMU_MODE_ALL) &&
> +          !is_hardware_domain(current->domain)) )
>          return 0;
>  
>      curr = current;
> @@ -153,8 +155,12 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
>      struct vcpu *sampled = current, *sampling;
>      struct vpmu_struct *vpmu;
>  
> -    /* dom0 will handle interrupt for special domains (e.g. idle domain) */
> -    if ( sampled->domain->domain_id >= DOMID_FIRST_RESERVED )
> +    /*
> +     * dom0 will handle interrupt for special domains (e.g. idle domain) or,
> +     * in XENPMU_MODE_ALL, for everyone.
> +     */
> +    if ( (vpmu_mode & XENPMU_MODE_ALL) ||
> +         (sampled->domain->domain_id >= DOMID_FIRST_RESERVED) )
>      {
>          sampling = choose_hwdom_vcpu();
>          if ( !sampling )
> @@ -164,7 +170,7 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
>          sampling = sampled;
>  
>      vpmu = vcpu_vpmu(sampling);
> -    if ( !is_hvm_domain(sampling->domain) )
> +    if ( !is_hvm_domain(sampling->domain) || (vpmu_mode & XENPMU_MODE_ALL) )
>      {
>          /* PV(H) guest */
>          const struct cpu_user_regs *cur_regs;
> @@ -176,6 +182,7 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
>              return 1;
>  
>          if ( is_pvh_domain(sampled->domain) &&
> +             !(vpmu_mode & XENPMU_MODE_ALL) &&
>               !vpmu->arch_vpmu_ops->do_interrupt(regs) )
>              return 0;
>  
> @@ -218,7 +225,7 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
>              r->rip = cur_regs->rip;
>              r->rsp = cur_regs->rsp;
>  
> -            if ( !is_pvh_domain(sampled->domain) )
> +            if ( !has_hvm_container_domain(sampled->domain) )
>              {
>                  r->cs = cur_regs->cs;
>                  if ( sampled->arch.flags & TF_kernel_mode )
> @@ -233,7 +240,9 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
>              }
>          }
>  
> -        vpmu->xenpmu_data->domain_id = DOMID_SELF;
> +        vpmu->xenpmu_data->domain_id = (sampled == sampling) ?
> +                                       DOMID_SELF :
> +                                       sampled->domain->domain_id;
>          vpmu->xenpmu_data->vcpu_id = sampled->vcpu_id;
>          vpmu->xenpmu_data->pcpu_id = smp_processor_id();
>  
> @@ -606,7 +615,8 @@ long do_xenpmu_op(int op, XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
>          if ( copy_from_guest(&pmu_params, arg, 1) )
>              return -EFAULT;
>  
> -        if ( pmu_params.val & ~(XENPMU_MODE_SELF | XENPMU_MODE_HV) )
> +        if ( pmu_params.val & ~(XENPMU_MODE_SELF | XENPMU_MODE_HV |
> +                                XENPMU_MODE_ALL) )
>              return -EINVAL;
>  
>          /*
> @@ -620,7 +630,7 @@ long do_xenpmu_op(int op, XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
>          current_mode = vpmu_mode;
>          vpmu_mode = pmu_params.val;
>  
> -        if ( vpmu_mode == XENPMU_MODE_OFF )
> +        if ( (vpmu_mode == XENPMU_MODE_OFF) || (vpmu_mode == XENPMU_MODE_ALL) )
>          {
>              /*
>               * Make sure all (non-dom0) VCPUs have unloaded their VPMUs. This
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index bc0c9c3..4e02359 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -2578,6 +2578,10 @@ static int emulate_privileged_op(struct cpu_user_regs *regs)
>          case MSR_AMD_FAM15H_EVNTSEL0...MSR_AMD_FAM15H_PERFCTR5:
>                  if ( vpmu_msr || (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) )
>                  {
> +                    if ( (vpmu_mode & XENPMU_MODE_ALL) &&
> +                         !is_hardware_domain(v->domain) )
> +                        break;
> +
>                      if ( vpmu_do_wrmsr(regs->ecx, msr_content, 0) )
>                          goto fail;
>                  }
> @@ -2700,6 +2704,14 @@ static int emulate_privileged_op(struct cpu_user_regs *regs)
>          case MSR_AMD_FAM15H_EVNTSEL0...MSR_AMD_FAM15H_PERFCTR5:
>                  if ( vpmu_msr || (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) )
>                  {
> +                    if ( (vpmu_mode & XENPMU_MODE_ALL) &&
> +                         !is_hardware_domain(v->domain) )
> +                    {
> +                        /* Don't leak PMU MSRs to unprivileged domains */
> +                        regs->eax = regs->edx = 0;
> +                        break;
> +                    }
> +
>                      if ( vpmu_do_rdmsr(regs->ecx, &msr_content) )
>                          goto fail;
>  
> diff --git a/xen/include/public/pmu.h b/xen/include/public/pmu.h
> index a1886a5..f900f90 100644
> --- a/xen/include/public/pmu.h
> +++ b/xen/include/public/pmu.h
> @@ -51,10 +51,13 @@ DEFINE_XEN_GUEST_HANDLE(xen_pmu_params_t);
>   * - XENPMU_MODE_SELF:  Guests can profile themselves
>   * - XENPMU_MODE_HV:    Guests can profile themselves, dom0 profiles
>   *                      itself and Xen
> + * - XENPMU_MODE_ALL:   Only dom0 has access to VPMU and it profiles
> + *                      everyone: itself, the hypervisor and the guests.
>   */
>  #define XENPMU_MODE_OFF           0
>  #define XENPMU_MODE_SELF          (1<<0)
>  #define XENPMU_MODE_HV            (1<<1)
> +#define XENPMU_MODE_ALL           (1<<2)
>  
>  /*
>   * PMU features:
> -- 
> 1.8.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel