[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.