Re: [Xen-devel] [PATCH v7 11/19] x86/VPMU: Interface for setting PMU mode and flags

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 06/06/2014 03:58 PM, Andrew Cooper wrote:
> On 06/06/14 18:40, Boris Ostrovsky wrote:
> > Add runtime interface for setting PMU mode and flags. Three main modes are
> > provided:
> > * PMU off
> > * PMU on: Guests can access PMU MSRs and receive PMU interrupts. dom0
> >    profiles itself and the hypervisor.
> > * dom0-only PMU: dom0 collects samples for both itself and guests.
> >
> > For feature flags only Intel's BTS is currently supported.
> >
> > Mode and flags are set via HYPERVISOR_xenpmu_op hypercall.
> >
> > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > Reviewed-by: Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>
> > Tested-by: Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>
> > ---
> >
> > @@ -275,3 +283,100 @@ void vpmu_dump(struct vcpu *v)
> >           vpmu->arch_vpmu_ops->arch_vpmu_dump(v);
> >   }
> >   
> > +/* Unload VPMU contexts */
> > +static void vpmu_unload_all(void)
> > +{
> > +    struct domain *d;
> > +    struct vcpu *v;
> > +    struct vpmu_struct *vpmu;
> > +
> > +    for_each_domain( d )
> > +    {
> > +        for_each_vcpu ( d, v )
> > +        {
> > +            if ( v != current )
> > +                vcpu_pause(v);
> > +            vpmu = vcpu_vpmu(v);
> > +
> > +            if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
> > +            {
> > +                if ( v != current )
> > +                    vcpu_unpause(v);
> > +                continue;
> > +            }
> > +
> > +            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);
> > +
> > +            if ( v != current )
> > +                vcpu_unpause(v);
> > +        }
> > +    }
> > +}
> This is a very long function without any preemption.
>
> To reduce its impact greatly, would it be better to have a global mode
> controlling vcpu context switching?  You could enable
> "VCPU_GLOBAL_DISABLING", then IPI all cpus at the same time to save
> their vpmu context.  Once the IPI is complete, all vpmu context is clean.
>
> This avoids a double loop and all the vcpu_pause/unpause()s.
I can see the problem but I am not sure what you are suggesting will 
work (or perhaps I don't quite understand what you are suggesting). If I 
IPI a processor may be in the middle of working on the VPMU context.
Let me think about this.


> > +
> > +
> > +long do_xenpmu_op(int op, XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
> > +{
> > +    int ret = -EINVAL;
> > +    xen_pmu_params_t pmu_params;
> This is probably going to want to be XSM'd up for permissions?
I haven't looked at this since earlier versions of the patch but when I 
did it turned out we were out of bits in the access vector (or whatever 
that structure was). I in fact mentioned this problem in the cover 
letter for the series.
-boris

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