Re: [Xen-devel] [PATCH v12 for-xen-4.5 11/20] x86/VPMU: Interface for setting PMU mode and flags

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 09/26/2014 05:04 PM, Tian, Kevin wrote:
> > From: Boris Ostrovsky [mailto:boris.ostrovsky@xxxxxxxxxx]
> > Sent: Thursday, September 25, 2014 12:29 PM
> >
> > Add runtime interface for setting PMU mode and flags. Three main modes are
> > provided:
> > * XENPMU_MODE_OFF:  PMU is not virtualized
> > * XENPMU_MODE_SELF: Guests can access PMU MSRs and receive PMU
> > interrupts.
> > * XENPMU_MODE_HV: Same as XENPMU_MODE_SELF for non-proviledged
> > guests, dom0
> >    can profile itself and the hypervisor.
> >
> > Note that PMU modes are different from what can be provided at Xen's boot
> > line
> > with 'vpmu' argument. An 'off' (or '0') value is equivalent to
> > XENPMU_MODE_OFF.
> > Any other value, on the other hand, will cause VPMU mode to be set to
> > XENPMU_MODE_SELF during boot.
> >
> > 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: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> I'll need focused time to review this one and several latter big patches, which
> can't be afforded this week. Will do that next week.
Thank you.

> One immediate comment though. Looks below comment is not fixed
> which you said is for XENPMU_MODE_ALL:

XENPMU_MODE_ALL is defined later, in patch 18, and the 'if' below is 
updated there.
(and I think when I said earlier that the comment was incorrect I was 
wrong. I re-read it and I don't see anything wrong with it. I may have 
been thinking about the 'if' statement not being correct, not realizing 
that it is updated by the later patch).

-boris

> > +        if ( vpmu_mode == XENPMU_MODE_OFF )
> > +        {
> > +            /*
> > +             * Make sure all (non-dom0) VCPUs have unloaded their
> > VPMUs. This
> > +             * can be achieved by having all physical processors go
> > through
> > +             * context_switch().
> > +             */
> > +            ret = vpmu_force_context_switch();
> > +            if ( ret )
> > +                vpmu_mode = current_mode;
> > +        }
> > +
> > +        spin_unlock(&xenpmu_mode_lock);
> > +        break;
> > +    }
> > +
> Thanks
> Kevin

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