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

[Chao Gao <chao.gao@xxxxxxxxx>]

Hi, xuquan.
I have tested it on my skylake server. W/o this patch the inaccurate
wall clock time issue only exists in Win7-32 guest. Win7-64, Win8-32, Win8-64,
Win10-32 ,Win10-64 and linux-4.8.0+ guests don't have this issue.
W/ this v4 patch, the issue disappears in Win7-32 guest and no wall lock time
related regression is found on Win7-64, Win8-32, Win8-64, Win10-32, Win10-64 
and linux-4.8.0+ guest.

In windows guest, the test procedure is
1. Create a windows guest with 2 vCPU
2. run the following .bat in guest
    :abcd
    echo 111111
    goto abcd

3. Start a stop-watch outside the guest and monitor the clock at the lower right
   corner in guest. After 120 seconds according the guest clock, stop the 
stop-watch.
   If the time shows in the stop-watch is about 120 seconds, then I think
   there is no the above issue in the guest. Otherwise, the time is inaccurate. 

In Win7-32 case, the stop-watch time is about 70 seconds, so the clock in guest 
is
obviously inaccurate.

In linux guest, the test procedure is
1. Create a linux guest with 4 vCPU
2. insmod the following linux module
   (through output of /proc/interrupt, about 850000 ipis in 13 seconds)
3. use date command to get guest time, others are same as test in windows guest

#include <linux/init.h>                                                         
#include <linux/module.h>                                                       
#include <linux/kthread.h>                                                      
#include <linux/sched.h>                                                        
#include <asm/delay.h>                                                          
MODULE_LICENSE("GPL");                                                          
void workload(void *info)                                                       
{                                                                               
        asm volatile("nop");                                                    
}                                                                               
                                                                                
void msleep(unsigned int msecs);                                                
static int ipi_generator(void * info)                                           
{                                                                               
        int i;                                                                  
        while (!kthread_should_stop()) {                                        
                for(i=0; i< 5 * 10000; i++)                                     
                {                                                               
                        smp_call_function(workload, NULL,1);                    
                }                                                               
                msleep(1);                                                      
        }                                                                       
        return 0;                                                               
}                                                                               
struct task_struct *thread;                                                     
static int __init ipi_init(void)                                                
{                                                                               
        thread = kthread_run(ipi_generator, NULL, "IPI");                       
        if (IS_ERR(thread))                                                     
                return PTR_ERR(thread);                                         
        return 0;                                                               
}                                                                               
                                                                                
static void __exit ipi_exit(void)                                               
{                                                                               
        kthread_stop(thread);                                                   
}                                                                               
module_init(ipi_init);                                                          
module_exit(ipi_exit);

Are these tests sufficient? Please let me know if you have any other thoughts.

On Wed, Dec 21, 2016 at 05:44:08AM +0000, Xuquan (Quan Xu) wrote:
>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.
>
>And that we update periodic timer interrupt in every VM-entry, there is
>a chance that already-injected instance (before EOI-induced exit happens)
>will incur another pending IRR setting if there is a VM-exit happens
>between virtual interrupt injection (vIRR->0, vISR->1) and EOI-induced
>exit (vISR->0), since pt_intr_post hasn't been invoked yet, then the
>guest receives more periodic timer interrupt.
>
>So we set eoi_exit_bitmap for intack.vector when it's higher than
>pending periodic time interrupts. This way we can guarantee there's
>always a chance to post periodic time interrupts when periodic time
>interrupts becomes the highest one.
>
>Signed-off-by: Quan Xu <xuquan8@xxxxxxxxxx>
>---
> xen/arch/x86/hvm/vmx/intr.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
>index 639a705..0cf26b4 100644
>--- a/xen/arch/x86/hvm/vmx/intr.c
>+++ b/xen/arch/x86/hvm/vmx/intr.c
>@@ -315,9 +315,13 @@ void vmx_intr_assist(void)
>         * Set eoi_exit_bitmap for periodic timer interrup to cause 
> EOI-induced VM
>         * exit, then pending periodic time interrups have the chance to be 
> injected
>         * for compensation
>+        * Set eoi_exit_bitmap for intack.vector when it's higher than pending
>+        * periodic time interrupts. This way we can guarantee there's always 
>a chance
>+        * to post periodic time interrupts when periodic time interrupts 
>becomes the
>+        * highest one
>         */
>         if (pt_vector != -1)
>-            vmx_set_eoi_exit_bitmap(v, pt_vector);
>+            vmx_set_eoi_exit_bitmap(v, intack.vector);
>
>         /* we need update the RVI field */
>         __vmread(GUEST_INTR_STATUS, &status);
>@@ -334,7 +338,8 @@ void vmx_intr_assist(void)
>             __vmwrite(EOI_EXIT_BITMAP(i), v->arch.hvm_vmx.eoi_exit_bitmap[i]);
>         }
>
>-        pt_intr_post(v, intack);
>+        if ( intack.vector == pt_vector )
>+            pt_intr_post(v, intack);
>     }
>     else
>     {
>--
>1.8.3.4

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