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

[Yang Zhang <yang.zhang.wz@xxxxxxxxx>]

On 2016/8/22 18:35, Jan Beulich wrote:
> > > > On 19.08.16 at 14:58, <xuquan8@xxxxxxxxxx> wrote:
> > From 9b2df963c13ad27e2cffbeddfa3267782ac3da2a Mon Sep 17 00:00:00 2001
> > From: Quan Xu <xuquan8@xxxxxxxxxx>
> > Date: Fri, 19 Aug 2016 20:40:31 +0800
> > Subject: [RFC PATCH] x86/apicv: fix RTC periodic timer and apicv issue
> >
> > When Xen apicv is enabled, wall clock time is faster on Windows7-32 guest
> > with high payload (with 2vCPU, captured from xentrace, in high payload, the
> > count of IPI interrupt increases rapidly between these vCPUs).
> >
> > If IPI intrrupt (vector 0xe1) and periodic timer interrupt (vector 0xd1)
> > are both pending (index of bit set in VIRR), unfortunately, the IPI intrrupt
> > is high priority than periodic timer interrupt. Xen updates IPI interrupt bit
> > set in VIRR to guest interrupt status (RVI) as a high priority and apicv
> > (Virtual-Interrupt Delivery) delivers IPI interrupt within VMX non-root
> > operation without a VM exit. Within VMX non-root operation, if periodic timer
> > interrupt index of bit is set in VIRR and highest, the apicv delivers periodic
> > timer interrupt within VMX non-root operation as well.
> >
> > But in current code, if Xen doesn't update periodic timer interrupt bit set
> > in VIRR to guest interrupt status (RVI) directly, Xen is not aware of this
> > case to decrease the count (pending_intr_nr) of pending periodic timer 
> > interrupt,
> > then Xen will deliver a periodic timer interrupt again. The guest receives more
> > periodic timer interrupt.
> >
> > If the periodic timer interrut is delivered and not the highest priority, make
> > Xen be aware of this case to decrease the count of pending periodic timer
> > interrupt.
> I can see the issue you're trying to address, but for one - doesn't
> this lead to other double accounting, namely once the pt irq becomes
> the highest priority one?
"intack.vector > pt_vector"
I think above check in code can avoid the double accounting problem. It 
recalculate pending timer interrupt only when the highest priority 
interrupt is not timer.
> > --- a/xen/arch/x86/hvm/vmx/intr.c
> > +++ b/xen/arch/x86/hvm/vmx/intr.c
> > @@ -334,7 +334,21 @@ void vmx_intr_assist(void)
> >              __vmwrite(EOI_EXIT_BITMAP(i), v->arch.hvm_vmx.eoi_exit_bitmap[i]);
> >          }
> >
> > -        pt_intr_post(v, intack);
> > +       /*
> > +        * If the periodic timer interrut is delivered and not the highest 
> > priority,
> > +        * make Xen be aware of this case to decrease the count of pending 
> > periodic
> > +        * timer interrupt.
> > +        */
> > +        if ( pt_vector != -1 && intack.vector > pt_vector )
> > +        {
> > +            struct hvm_intack pt_intack;
> > +
> > +            pt_intack.vector = pt_vector;
> > +            pt_intack.source = hvm_intsrc_lapic;
> > +            pt_intr_post(v, pt_intack);
> > +        }
> > +        else
> > +            pt_intr_post(v, intack);
> And then I'm having two problems with this change: Processing other
> than the highest priority irq here looks to be non-architectural. From
> looking over pt_intr_post() there's only pt related accounting and no
> actual interrupt handling there, but I'd like this to be (a) confirmed
> and (b) called out in the comment.
>
> Plus the change above is in no way apicv specific, so I'd also like to
> see clarification why this change is valid (i.e. at least not making
> things worse) even without apicv in use.
It is apicv specific case. Above code will execute only when cpu has 
virtual interrupt delivery which is part of apicv feature.
> And of course in any event VMX maintainer feedback is going to be
> needed here.
>
> Jan

--
Yang
Alibaba Cloud Computing

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