Re: [Xen-devel] [PATCH v24 12/15] x86/VPMU: Handle PMU interrupts for PV(H) guests

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 06/16/2015 03:45 AM, Jan Beulich wrote:
> > > > On 15.06.15 at 19:17, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > > >
> > > > +        for ( i = 0; i < num_counters; i++ )
> > > > +        {
> > > > +            if ( (ctrl_regs[i] & CTRL_RSVD_MASK) != ctrl_rsvd[i] )
> > > > +            {
> > > > +                /*
> > > > +                 * Not necessary to re-init context since we should never load
> > > > +                 * it until guest provides valid values. But just to be safe.
> > > > +                 */
> > > > +                amd_vpmu_init_regs(ctxt);
> > > > +                return -EINVAL;
> > > > +            }
> > > > +
> > > > +            if ( is_pmu_enabled(ctrl_regs[i]) )
> > > > +                num_enabled++;
> > > > +        }
> > > > +
> > > > +        if ( num_enabled )
> > > Looks like a boolean flag would do - the exact count doesn't seem to
> > > be of interest here or in later patches?
> > So the reason why I use a counter here is because keeping track of
> > VPMU_RUNNING state is currently broken on AMD, I noticed it when I was
> > updating this patch. amd_vpmu_do_wrmsr() will reset VPMU_RUNNING if the
> > MSR write is disabling current counter, even though there may still be
> > other counters running. This may be related to HVM brokenness of AMD
> > counters that I mentioned a while ago where the guest, when running with
> > multiple counters, sometimes gets unexpected NMIs. (This goes back all
> > the way to to 4.1.)
> >
> > I don't want to fix this in this series but I will likely need to count
> > number of active counters when I do (just like I do for Intel).
> >
> > I can use a boolean now though since I am not dealing with this problem
> > here.
> If another rev is needed, I'd prefer if you did so. But if we can have
> this version go in (provided we get all the necessary acks), I wouldn't
> insist on you doing another round just because of this.
I think there are a couple of (fairly cosmetic) changes that need to be 
done so there will be another rev.
OTOH I just tried a quick-and-dirty fix for this problem and it doesn't 
resolve it so presumably there is more to this. It's not particularly 
invasive but I think it would be rather pointless to put it in as it 
still doesn't allow multiple counters on AMD and I suspect the final fix 
will be touching the same code again.
-boris

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