From patchwork Mon May 6 02:52:37 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Zhanghaoyu (A)" X-Patchwork-Id: 241547 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 4068D2C0192 for ; Mon, 6 May 2013 12:54:16 +1000 (EST) Received: from localhost ([::1]:49472 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZBZ8-00038t-4H for incoming@patchwork.ozlabs.org; Sun, 05 May 2013 22:54:14 -0400 Received: from eggs.gnu.org ([208.118.235.92]:52512) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZBYq-00037s-FY for qemu-devel@nongnu.org; Sun, 05 May 2013 22:53:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UZBYo-0006XZ-NB for qemu-devel@nongnu.org; Sun, 05 May 2013 22:53:56 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:41364) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZBYn-0006JH-Rk for qemu-devel@nongnu.org; Sun, 05 May 2013 22:53:54 -0400 Received: from 172.24.2.119 (EHLO szxeml209-edg.china.huawei.com) ([172.24.2.119]) by szxrg02-dlp.huawei.com (MOS 4.3.4-GA FastPath queued) with ESMTP id BBC70859; Mon, 06 May 2013 10:53:35 +0800 (CST) Received: from SZXEML453-HUB.china.huawei.com (10.82.67.196) by szxeml209-edg.china.huawei.com (172.24.2.184) with Microsoft SMTP Server (TLS) id 14.1.323.7; Mon, 6 May 2013 10:52:47 +0800 Received: from szxeml556-mbx.china.huawei.com ([169.254.3.167]) by SZXEML453-HUB.china.huawei.com ([10.82.67.196]) with mapi id 14.01.0323.007; Mon, 6 May 2013 10:52:38 +0800 From: "Zhanghaoyu (A)" To: "Michael S. Tsirkin" Thread-Topic: [PATCH] [KVM] Needless to update msi route when only msi-x entry "control" section changed Thread-Index: Ac5Ip37P2odhksMnQ4uMmyPylhrF1wA6smmAABscFuA= Date: Mon, 6 May 2013 02:52:37 +0000 Message-ID: References: <20130505211307.GB7861@redhat.com> In-Reply-To: <20130505211307.GB7861@redhat.com> Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.135.68.97] MIME-Version: 1.0 X-CFilter-Loop: Reflected X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 119.145.14.65 Cc: luonengjun 00137267 , Marcelo Tosatti , qemu-devel , 00180273 , "q00170852@notesmail.huawei.com.cn" , huangweidong 00164878 Subject: Re: [Qemu-devel] [PATCH] [KVM] Needless to update msi route when only msi-x entry "control" section changed X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org >> With regard to old version linux guest(e.g., rhel-5.5), in ISR processing, mask and unmask msi-x vector every time, which result in VMEXIT, then QEMU will invoke kvm_irqchip_update_msi_route() to ask KVM hypervisor to update the VM irq routing table. In KVM hypervisor, synchronizing RCU needed after updating routing table, so much time consumed for waiting in wait_rcu_gp(). So CPU usage in VM is so high, while from the view of host, VM's total CPU usage is so low. >> Masking/unmasking msi-x vector only set msi-x entry "control" section, needless to update VM irq routing table. >> >> Signed-off-by: Zhang Haoyu >> Signed-off-by: Huang Weidong >> Signed-off-by: Qin Chuanyu >> --- >> hw/i386/kvm/pci-assign.c | 3 +++ >> 1 files changed, 3 insertions(+) >> >> --- a/hw/i386/kvm/pci-assign.c 2013-05-04 15:53:18.000000000 +0800 >> +++ b/hw/i386/kvm/pci-assign.c 2013-05-04 15:50:46.000000000 +0800 >> @@ -1576,6 +1576,8 @@ static void assigned_dev_msix_mmio_write >> MSIMessage msg; >> int ret; >> >> + /* Needless to update msi route when only msi-x entry "control" section changed */ >> + if ((addr & (PCI_MSIX_ENTRY_SIZE - 1)) != >> + PCI_MSIX_ENTRY_VECTOR_CTRL){ >> msg.address = entry->addr_lo | >> ((uint64_t)entry->addr_hi << 32); >> msg.data = entry->data; @@ -1585,6 +1587,7 @@ static >> void assigned_dev_msix_mmio_write >> if (ret) { >> error_report("Error updating irq routing entry (%d)", ret); >> } >> + } >> } >> } >> } >> >> Thanks, >> Zhang Haoyu > > >If guest wants to update the vector, it does it like this: >mask >update >unmask >and it looks like the only point where we update the vector is on unmask, so this patch will mean we don't update the vector ever. > >I'm not sure this combination (old guest + legacy device assignment >framework) is worth optimizing. Can you try VFIO instead? > >But if it is, the right way to do this is probably along the lines of the below patch. Want to try it out? > >diff --git a/kvm-all.c b/kvm-all.c >index 2d92721..afe2327 100644 >--- a/kvm-all.c >+++ b/kvm-all.c >@@ -1006,6 +1006,11 @@ static int kvm_update_routing_entry(KVMState *s, > continue; > } > >+ if (entry->type == new_entry->type && >+ entry->flags == new_entry->flags && >+ entry->u == new_entry->u) { >+ return 0; >+ } > entry->type = new_entry->type; > entry->flags = new_entry->flags; > entry->u = new_entry->u; > union type cannot be directly compared, I tried out below patch instead, Thanks, Zhang Haoyu --- a/kvm-all.c 2013-05-06 09:56:38.000000000 +0800 +++ b/kvm-all.c 2013-05-06 09:56:45.000000000 +0800 @@ -1008,6 +1008,12 @@ static int kvm_update_routing_entry(KVMS continue; } + if (entry->type == new_entry->type && + entry->flags == new_entry->flags && + !memcmp(&entry->u, &new_entry->u, sizeof(entry->u))) { + return 0; + } + entry->type = new_entry->type; entry->flags = new_entry->flags; entry->u = new_entry->u; MST's patch is more universal than my first patch fixed in assigned_dev_msix_mmio_write(). On the case that the msix entry's other section but "control" section is set to the identical value with old entry's, MST's patch also works. MST's patch also works on the non-passthrough scenario. And, after MST's patch applied, the below check in virtio_pci_vq_vector_unmask() can be removed. --- a/hw/virtio/virtio-pci.c 2013-05-04 15:53:20.000000000 +0800 +++ b/hw/virtio/virtio-pci.c 2013-05-06 10:25:58.000000000 +0800 @@ -619,12 +619,10 @@ static int virtio_pci_vq_vector_unmask(V if (proxy->vector_irqfd) { irqfd = &proxy->vector_irqfd[vector]; - if (irqfd->msg.data != msg.data || irqfd->msg.address != msg.address) { ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg); if (ret < 0) { return ret; } - } } /* If guest supports masking, irqfd is already setup, unmask it.