Re: [Xen-devel] [PATCH v2] x86/apicv: fix RTC periodic timer and apicv issue

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Xuquan (Quan Xu) [mailto:xuquan8@xxxxxxxxxx]
> Sent: Monday, October 10, 2016 6:49 PM
> 
> On October 10, 2016 5:40 PM, Jan Beulich < JBeulich@xxxxxxxx > wrote:
> >>>>>> >>> On 20.09.16 at 15:30, <xuquan8@xxxxxxxxxx> wrote:
> >>>>>> > --- a/xen/arch/x86/hvm/vlapic.c
> >>>>>> > +++ b/xen/arch/x86/hvm/vlapic.c
> >>>>>> > @@ -433,6 +433,12 @@ void vlapic_EOI_set(struct vlapic *vlapic)
> >>>>>> > void vlapic_handle_EOI(struct vlapic *vlapic, u8 vector)  {
> >>>>>> >      struct domain *d = vlapic_domain(vlapic);
> >>>>>> > +    struct vcpu *v = vlapic_vcpu(vlapic);
> >>>>>> > +    struct hvm_intack pt_intack;
> >>>>>> > +
> >>>>>> > +    pt_intack.vector = vector;
> >>>>>> > +    pt_intack.source = hvm_intsrc_lapic;
> >>>>>> > +    pt_intr_post(v, pt_intack);
> >>>>>>
> >>>>>> This also sits on the EOI LAPIC register write path, i.e. the
> >>>>>> change then also affects non-apicv environments.
> >>>>>
> >>>>>The new logic should be entered only when EOI-induced exit happens.
> >>>>>
> >>>>
> >>>> Yes, more that the EOI-induced exit is conditional, specifically,
> >>>> the bitmap is set by vmx_set_eoi_exit_bitmap().
> >>>> Jan, what do you think? While I recall from v1 discussion, you have
> >>>> the same comment. We can dig it deep..
> >>>
> >>>See my reply to Kevin sent a minute ago. As I'm not sure what Kevin
> >>>means to state with several of his responses, I can't properly respond
> >>>for now. And then what you say doesn't really address my concern -
> >>>things being conditional elsewhere doesn't mean we won't get here too
> >>>in the non-apicv case, at least not in a way that I can follow right away.
> >>
> >> Jan, any idea now?
> >
> >I don't think there was anything left open on the other sub-thread; if there 
> >is,
> >please point out specific aspects which are still unclear.
> >
> 
> Sorry, I overlooked the other sub-thread after holiday(10.1-10.7)..
> I will read it again..
> 
> Quan

Is there any discussion after 10.1? I didn't see it.

Back to the main open before holiday - multiple EOIs may come
to clear irq_issued before guest actually handles the very vpt
injection (possible if vpt vector is shared with other sources). I
don't see a good solution on that open... :/

We've discussed various options which all fail in one or another 
place - either miss an injection, or incur undesired injections. 
Possibly we should consider another direction - fall back to 
non-apicv path when we see vpt vector pending but it's not the
highest one.

Original condition to enter virtual intr delivery:
            else if ( cpu_has_vmx_virtual_intr_delivery &&
              intack.source != hvm_intsrc_pic &&
              intack.source != hvm_intsrc_vector )

now new condition:
            else if ( cpu_has_vmx_virtual_intr_delivery &&
              intack.source != hvm_intsrc_pic &&
              intack.source != hvm_intsrc_vector &&
                        (pt_vector == -1 || intack.vector == pt_vector) )

Thoughts?

Thanks
Kevin

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