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

[Yang Zhang <yang.zhang.wz@gmail.com>]

On 2016/8/22 18:35, Jan Beulich wrote:
>>>> On 19.08.16 at 14:58, <xuquan8@huawei.com> wrote:
>> From 9b2df963c13ad27e2cffbeddfa3267782ac3da2a Mon Sep 17 00:00:00 2001
>> From: Quan Xu <xuquan8@huawei.com>
>> 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@lists.xen.org
https://lists.xen.org/xen-devel