Re: [Xen-devel] [PATCH v14 for-xen-4.5 17/21] x86/VPMU: Handle PMU interrupts for PV guests

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 10/28/2014 05:30 AM, Jan Beulich wrote:
> > > > On 27.10.14 at 20:43, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > On 10/27/2014 12:54 PM, Jan Beulich wrote:
> > > > > > On 17.10.14 at 23:18, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > > > --- a/xen/arch/x86/hvm/vpmu.c
> > > > +++ b/xen/arch/x86/hvm/vpmu.c
> > > > @@ -81,46 +81,206 @@ static void __init parse_vpmu_param(char *s)
> > > >    
> > > >    void vpmu_lvtpc_update(uint32_t val)
> > > >    {
> > > > -    struct vpmu_struct *vpmu = vcpu_vpmu(current);
> > > > +    struct vcpu *curr = current;
> > > > +    struct vpmu_struct *vpmu = vcpu_vpmu(curr);
> > > >    
> > > >        vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR | (val & APIC_LVT_MASKED);
> > > > -    apic_write(APIC_LVTPC, vpmu->hw_lapic_lvtpc);
> > > > +
> > > > +    /* Postpone APIC updates for PV(H) guests if PMU interrupt is pending
> > */
> > > > +    if ( is_hvm_vcpu(curr) || !vpmu->xenpmu_data ||
> > > > +         !(vpmu->xenpmu_data->pmu.pmu_flags & PMU_CACHED) )
> > > Isn't this the pointer that pvpmu_finish() deallocates (and needs to
> > > clear? If so, there's a race between it being cleared and used. If you
> > > need it in places like this, perhaps you'd be better off never clearing
> > > it and leaving the MFN allocated?
> > This will be one of the places that check for VPMU_CONTEXT_ALLOCATED.
> But how will adding this check make this race free?

This VCPU is paused while we are tearing down its pvpmu so we can't be 
in the middle if this (or other, such as vpmu_do_msr()) hypercall, can we?
Or it's not paused if the VCPU is the same one that is doing the 
teardown. In which case we certainly are not in this hypercall.
So, in fact, the check for VPMU_CONTEXT_ALLOCATED may not be needed at all.

-boris

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