From patchwork Fri Sep 9 03:02:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Xuquan (Euler)" X-Patchwork-Id: 9322359 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8ABE360231 for ; Fri, 9 Sep 2016 03:05:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 74E5D29AAE for ; Fri, 9 Sep 2016 03:05:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 67A2729B0B; Fri, 9 Sep 2016 03:05:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 356BD29AAE for ; Fri, 9 Sep 2016 03:05:02 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1biC5A-00058I-BZ; Fri, 09 Sep 2016 03:02:24 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1biC59-00058C-03 for xen-devel@lists.xen.org; Fri, 09 Sep 2016 03:02:23 +0000 Received: from [85.158.137.68] by server-9.bemta-3.messagelabs.com id FD/5C-27233-C3622D75; Fri, 09 Sep 2016 03:02:20 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprJKsWRWlGSWpSXmKPExsVSPpHPUddG7VK 4waFNwhZLPi5mcWD0OLr7N1MAYxRrZl5SfkUCa8bqJdOYCl4aVfQuec7cwDhTs4uRi0NI4BSj xNRte9kgnA2MEv/mn2ftYuTkYBPQlpjcswzMFhFIk1jUt4wRpIhZ4AKTxM4j/ewgCWEBL4lHC w+zQxR5SzT8WckCYRtJTP/dygZiswioSKzvfswMYvMKBEusP3ADatscRonN/96CNXAKhEg0LT 8DNohRQEzi+6k1TCA2s4C4xNxps8CukBAQlFg0ew8zhC0m8W/XQzYIW1FiT98HVoh6HYkFuz+ xQdjaEssWvoZaLChxcuYTFoh6SYmDK26wgBwhIXCBUeL6482MEAlTiRnztjNNYBSfhWT3LCRz ZyGZOwvJ3AWMLKsY1YtTi8pSi3QN9ZKKMtMzSnITM3N0DQ2M9XJTi4sT01NzEpOK9ZLzczcxA qOMAQh2MC7/6HSIUZKDSUmU97PspXAhvqT8lMqMxOKM+KLSnNTiQ4wyHBxKErxuKkA5waLU9N SKtMwcYLzDpCU4eJREeDeCpHmLCxJzizPTIVKnGBWlxHm9QRICIImM0jy4NliKucQoKyXMywh 0iBBPQWpRbmYJqvwrRnEORiVh3kqQKTyZeSVw018BLWYCWix06jzI4pJEhJRUA+N+JgPmpSXp H+SnrJ3vuDenW+OV/c/L7TkPHQOf3zojJHU4ofj9jj3iKuLxaZMd7V//m7j/T9iJe99Ozt4Qp PnBZppGkezdTO60q1UnxFxe3VXX2B21r1l8HVvhtZm9nDsUC5l2VHI/VWVassaGd7nmKtcZ/p qrDzUuPuXUUajMrjm5jlnv42olluKMREMt5qLiRADXQydFLAMAAA== X-Env-Sender: xuquan8@huawei.com X-Msg-Ref: server-7.tower-31.messagelabs.com!1473390136!52216517!1 X-Originating-IP: [119.145.14.65] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTE5LjE0NS4xNC42NSA9PiA3NzQ2Mw==\n X-StarScan-Received: X-StarScan-Version: 8.84; banners=-,-,- X-VirusChecked: Checked Received: (qmail 44737 invoked from network); 9 Sep 2016 03:02:19 -0000 Received: from szxga02-in.huawei.com (HELO szxga02-in.huawei.com) (119.145.14.65) by server-7.tower-31.messagelabs.com with RC4-SHA encrypted SMTP; 9 Sep 2016 03:02:19 -0000 Received: from 172.24.1.60 (EHLO SZXEMI415-HUB.china.huawei.com) ([172.24.1.60]) by szxrg02-dlp.huawei.com (MOS 4.3.7-GA FastPath queued) with ESMTP id DMW01770; Fri, 09 Sep 2016 11:02:13 +0800 (CST) Received: from SZXEMI506-MBX.china.huawei.com ([169.254.5.194]) by SZXEMI415-HUB.china.huawei.com ([10.86.210.48]) with mapi id 14.03.0235.001; Fri, 9 Sep 2016 11:02:11 +0800 From: "Xuquan (Euler)" To: "Tian, Kevin" , "xen-devel@lists.xen.org" Thread-Topic: [RFC PATCH] x86/apicv: fix RTC periodic timer and apicv issue Thread-Index: AQHSAoOI0gJ1eNYjIUW6VScgIQM/5KBwcbzQ Date: Fri, 9 Sep 2016 03:02:10 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.142.69.246] MIME-Version: 1.0 X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020202.57D22637.0149, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0, ip=169.254.5.194, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 88dda0c7acc504159837203870bfa23f Cc: "yang.zhang.wz@gmail.com" , "jbeulich@suse.com" , "George.Dunlap@eu.citrix.com" , Andrew Cooper , "Nakajima, Jun" Subject: Re: [Xen-devel] [RFC PATCH] x86/apicv: fix RTC periodic timer and apicv issue X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP On August 30, 2016 1:58 PM, Tian Kevin < kevin.tian@intel.com > wrote: >> From: Xuquan (Euler) [mailto:xuquan8@huawei.com] >> Sent: Friday, August 19, 2016 8:59 PM >> >> 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. >> >> Signed-off-by: Yifei Jiang >> Signed-off-by: Rongguang He >> Signed-off-by: Quan Xu >> --- >> Why RFC: >> 1. I am not quite sure for other cases, such as nested case. >> 2. Within VMX non-root operation, an Asynchronous Enclave Exit (including >external >> interrupts, non-maskable interrupt system-management interrrupts, >exceptions >> and VM exit) may occur before delivery of a periodic timer interrupt, the >periodic >> timer interrupt may be lost when a coming periodic timer interrupt is >delivered. >> Actually, and so current code is. >> --- >> xen/arch/x86/hvm/vmx/intr.c | 16 +++++++++++++++- >> 1 file changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c >> index 8fca08c..d3a034e 100644 >> --- 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); >> } > >Here is my thought. w/o above patch one pending pt interrupt may result in >multiple injections of guest timer interrupt, as you identified, when pt_vector >happens to be not the highest pending vector. Then w/ your patch, multiple >pending pt interrupt instances may be combined as one injection of guest timer >interrupt. Especially intack.vector >>pt_vector doesn't mean pt_intr is eligible for injection, which might >be blocked due to other reasons. As Jan pointed out, this path is very fragile, so >better we can have a fix to sustain the original behavior with one pending pt >instance causing one actual injection. > >What about doing pt_intr_post in EOI-induced VM-exit instead? EOI exit bit for >pt_intr is already set to 1 which means every injection would incur an >EOI-induced VM-exit: > > /* > * 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 > */ > if (pt_vector != -1) > vmx_set_eoi_exit_bitmap(v, pt_vector); > >I don't think delaying post makes much difference here. Even we do post earlier >like your patch, further pending instances have to wait until current instance is >completed. So as long as post happens before EOI is completed, it should be >fine. Kevin, I verify your suggestion with below modification. wall clock time is __still__ faster. I hope this modification is correct to your suggestion. I __guess__ that the vm-entry is more frequent than PT interrupt, Specifically, if there is a PT interrupt pending, the count (pending_intr_nr) is 1.. before next PT interrupt coming, each PT interrupt injection may not incur an EOI-induced VM-exit directly, multiple PT interrupt inject for a pending PT interrupt, then multiple EOI-induced VM-exit incur with multiple pt_intr_post() to decrease the count (pending_intr_nr), but the count (pending_intr_nr) is still 1 before next PT interrupt coming, then only one pt_intr_post() is effective.. Thanks for your time!! Quan ======= modification ======== diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 1d5d287..cc247c3 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -433,6 +433,11 @@ void vlapic_EOI_set(struct vlapic *vlapic) void vlapic_handle_EOI(struct vlapic *vlapic, u8 vector) { struct domain *d = vlapic_domain(vlapic); + struct hvm_intack pt_intack; + + pt_intack.vector = vector; + pt_intack.source = hvm_intsrc_lapic; + pt_intr_post(vlapic_vcpu(vlapic), pt_intack); if ( vlapic_test_and_clear_vector(vector, &vlapic->regs->data[APIC_TMR]) ) vioapic_update_EOI(d, vector); diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c index 8fca08c..29d9bbf 100644 --- a/xen/arch/x86/hvm/vmx/intr.c +++ b/xen/arch/x86/hvm/vmx/intr.c @@ -333,8 +333,6 @@ void vmx_intr_assist(void) clear_bit(i, &v->arch.hvm_vmx.eoi_exitmap_changed); __vmwrite(EOI_EXIT_BITMAP(i), v->arch.hvm_vmx.eoi_exit_bitmap[i]); } - - pt_intr_post(v, intack); } else {