Re: [Xen-devel] [PATCH v9 13/20] x86/VPMU: When handling MSR accesses, leave fault injection to callers

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 08/12/2014 12:00 PM, Jan Beulich wrote:
> > > > On 12.08.14 at 17:47, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > On 08/12/2014 08:45 AM, Jan Beulich wrote:
> > > > > > On 08.08.14 at 18:55, <boris.ostrovsky@xxxxxxxxxx> 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


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