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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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?

>>> +            if ( !has_hvm_container_vcpu(sampled) )
>>> +            {
>>> +                r->ss = cur_regs->ss;
>>> +                r->cs = cur_regs->cs;
>>> +                if ( !(sampled->arch.flags & TF_kernel_mode) )
>>> +                    *flags |= PMU_SAMPLE_USER;
>>> +            }
>>> +            else
>>> +            {
>>> +                struct segment_register seg;
>>> +
>>> +                hvm_get_segment_register(sampled, x86_seg_cs, &seg);
>>> +                r->cs = seg.sel;
>>> +                if ( (r->cs & 3) != 0 )
>>> +                    *flags |= PMU_SAMPLE_USER;
>> So is the VM86 mode case here intentionally being ignored?
> 
> We pass EFLAGS so the guest can check the VM bit. Is this not sufficient?

The PMU_SAMPLE_USER flag is misleading in that case the way you
have things coded currently. But that'll get addressed by switching
to use SS.DPL (see below) anyway.

>> And is
>> there a particular reason you look at the selector's RPL instead of
>> DPL, and CS instead of SS?
> 
> Should be DPL indeed. But why is SS better than CS?

Because SS.DPL is the canonical source for determining CPL.

Jan


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