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

["Xuquan (Euler)" <xuquan8@xxxxxxxxxx>]

On August 22, 2016 7:40 PM, Yang Zhang wrote:
>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.
>

Yes, 

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

Yes, I think so too. Thanks for your clarification!!

Quan


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