From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v24 12/15] x86/VPMU: Handle PMU interrupts for PV(H) guests Date: Tue, 16 Jun 2015 10:14:33 -0400 Message-ID: <55802F49.1070900@oracle.com> References: <1433948671-29504-1-git-send-email-boris.ostrovsky@oracle.com> <1433948671-29504-13-git-send-email-boris.ostrovsky@oracle.com> <557F104C0200007800085040@mail.emea.novell.com> <557F0890.8080008@oracle.com> <557FF01C0200007800085422@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <557FF01C0200007800085422@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: kevin.tian@intel.com, suravee.suthikulpanit@amd.com, andrew.cooper3@citrix.com, tim@xen.org, dietmar.hahn@ts.fujitsu.com, xen-devel@lists.xen.org, Aravind.Gopalakrishnan@amd.com, jun.nakajima@intel.com, dgdegra@tycho.nsa.gov List-Id: xen-devel@lists.xenproject.org On 06/16/2015 03:45 AM, Jan Beulich wrote: >>>> On 15.06.15 at 19:17, 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