From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v9 13/20] x86/VPMU: When handling MSR accesses, leave fault injection to callers Date: Tue, 12 Aug 2014 12:30:10 -0400 Message-ID: <53EA4112.2080609@oracle.com> References: <1407516946-17833-1-git-send-email-boris.ostrovsky@oracle.com> <1407516946-17833-14-git-send-email-boris.ostrovsky@oracle.com> <53EA2873020000780002B9CD@mail.emea.novell.com> <53EA371C.4040806@oracle.com> <53EA5656020000780002BC1C@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: <53EA5656020000780002BC1C@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, xen-devel@lists.xen.org, jun.nakajima@intel.com List-Id: xen-devel@lists.xenproject.org On 08/12/2014 12:00 PM, Jan Beulich wrote: >>>> On 12.08.14 at 17:47, wrote: >> On 08/12/2014 08:45 AM, Jan Beulich wrote: >>>>>> On 08.08.14 at 18:55, wrote: >>>> @@ -476,18 +476,17 @@ static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content) >>>> { >>>> case MSR_CORE_PERF_GLOBAL_OVF_CTRL: >>>> core2_vpmu_cxt->global_status &= ~msr_content; >>>> - return 1; >>>> + return 0; >>>> case MSR_CORE_PERF_GLOBAL_STATUS: >>>> gdprintk(XENLOG_INFO, "Can not write readonly MSR: " >>>> "MSR_PERF_GLOBAL_STATUS(0x38E)!\n"); >>>> - hvm_inject_hw_exception(TRAP_gp_fault, 0); >>>> return 1; >>> Suspicious: Most return values get flipped, but this one (and at least >>> one more below) doesn't. Any such inconsistencies that are being >>> corrected (assuming this is intentional) as you go should be spelled >>> out in the description. And then the question of course is whether >>> it's really necessary to flip the meaning of this and some similar SVM >>> function's return values anyway. >> The return value of 1 of vpmu_do_msr() will now indicate that the >> routine encountered a fault as opposed to indicating whether the MSR >> access was to a VPMU register. > And how would it now indicate that latter fact? It won't. Which is why I made changes to vmx_msr_read/write_intercept() to only call this routine when we know that a VPMU register is possibly accessed. I thought about tri-stating vpmu_do_msr() returns (-1 for faults, 0 for no registers have been accessed, 1 for successful accesses) but decided against it. -boris