Re: [Xen-devel] [PATCH v19 07/14] x86/VPMU: Initialize PMU for PV(H) guests

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 03/24/2015 10:08 AM, Jan Beulich wrote:
> > > > On 17.03.15 at 15:54, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > @@ -263,14 +264,14 @@ void vpmu_initialise(struct vcpu *v)
> >       BUILD_BUG_ON(sizeof(struct xen_pmu_intel_ctxt) > XENPMU_CTXT_PAD_SZ);
> >       BUILD_BUG_ON(sizeof(struct xen_pmu_amd_ctxt) > XENPMU_CTXT_PAD_SZ);
> >   
> > -    if ( is_pvh_vcpu(v) )
> > -        return;
> > -
> >       ASSERT(!vpmu->flags && !vpmu->context);
> >   
> > -    spin_lock(&vpmu_lock);
> > -    vpmu_count++; /* Prevent vpmu_mode from changing until we are done */
> > -    spin_unlock(&vpmu_lock);
> > +    if ( v->domain != hardware_domain )
> > +    {
> > +        spin_lock(&vpmu_lock);
> > +        vpmu_count++; /* Prevent vpmu_mode from changing until we are done */
> > +        spin_unlock(&vpmu_lock);
> > +    }
> So why is this being made conditional here? A patch this size would
> certainly benefit from a slightly more verbose description...
I don't want to include dom0's VPMU since the count is used for deciding 
whether or not we can switch the mode. If other guests have active VPMUs 
we sometimes can't do the switch. dom0's VPMUs are always there and we 
should be able to switch the mode with those VPMUs on, which is why 
there is no reason to count them.
I suppose I could check whether the count is smaller than number of 
dom0's CPUs but that's not particularly convenient since we could have 
had errors during initialization (so not all dom0's VCPUs have VPMUs), 
we may have to guard against hotplug events, etc.
But yes, an explanation would be useful. I'll add something.

> And then if the conditional is indeed needed - why don't you use
> is_hardware_domain()?
No specific reason. I'll switch to is_hardware_domain()

-boris



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