From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v15 11/21] x86/VPMU: Interface for setting PMU mode and flags Date: Thu, 04 Dec 2014 10:55:03 -0500 Message-ID: <548083D7.70006@oracle.com> References: <1416179271-1221-1-git-send-email-boris.ostrovsky@oracle.com> <1416179271-1221-12-git-send-email-boris.ostrovsky@oracle.com> <547496E0020000780004AB05@mail.emea.novell.com> <5475E46B.9000502@oracle.com> <5476F580020000780004B0E2@mail.emea.novell.com> <547F6EF3.5080304@oracle.com> <54802EB9020000780004C994@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: <54802EB9020000780004C994@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, keir@xen.org, 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 12/04/2014 03:51 AM, Jan Beulich wrote: >>>> On 03.12.14 at 21:13, wrote: >> On 11/27/2014 03:57 AM, Jan Beulich wrote: >>>>>> On 26.11.14 at 15:32, wrote: >>>> On 11/25/2014 08:49 AM, Jan Beulich wrote: >>>>>>>> On 17.11.14 at 00:07, wrote: >>>>>> @@ -244,19 +256,19 @@ void vpmu_initialise(struct vcpu *v) >>>>>> switch ( vendor ) >>>>>> { >>>>>> case X86_VENDOR_AMD: >>>>>> - if ( svm_vpmu_initialise(v, opt_vpmu_enabled) != 0 ) >>>>>> - opt_vpmu_enabled = 0; >>>>>> + if ( svm_vpmu_initialise(v) != 0 ) >>>>>> + vpmu_mode = XENPMU_MODE_OFF; >>>>>> break; >>>>>> >>>>>> case X86_VENDOR_INTEL: >>>>>> - if ( vmx_vpmu_initialise(v, opt_vpmu_enabled) != 0 ) >>>>>> - opt_vpmu_enabled = 0; >>>>>> + if ( vmx_vpmu_initialise(v) != 0 ) >>>>>> + vpmu_mode = XENPMU_MODE_OFF; >>>>>> break; >>>>> So this turns off the vPMU globally upon failure of initializing >>>>> some random vCPU. Why is that? I see this was the case even >>>>> before your entire series, but shouldn't this be fixed _before_ >>>>> enhancing the whole thing to support PV/PVH? >>>> Yes, that's probably too strong. Do you want to fix this as an early >>>> patch (before PV(H)) support gets in? I'd rather do it in the patch that >>>> moves things into initcalls. >>> Yes, I think this should be fixed in a prereq patch, thus allowing it >>> to be easily backported if so desired. >> I started to make this change and then I realized that the next patch >> (12/21) is actually already taking care of this problem: most of the >> *_vpmu_initialise() will be executed as initcalls during host boot and >> if any of those fail then we do want to disable VPMU globally (those >> failures would not be VCPU-specific). > Funny that you say that - it was actually while reviewing that next > patch when I made that observation: svm_vpmu_initialise() get a > -ENOMEM return _added_ there for example, which is contrary to > what I asked for above. Oh, *that* initialization code (I was looking at vpmu_init()). Yes, VPMU should not be turned off there. -boris