From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9BABC43381 for ; Tue, 19 Mar 2019 10:02:58 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7400D20857 for ; Tue, 19 Mar 2019 10:02:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="kw/Zr4mj" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7400D20857 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=aS8tKF0bLLIlBzv54LGgElXKwCDQNK8iwZrz7U/0Shc=; b=kw/Zr4mjBmMhhR H9xosDigKj/8rVcL2YfXw+n3PMz/YC1ydI3pvvXhEjYoaUEg2a11LZ+N2oUb3l47XUGuKIu3zIjzK Vu4wgomJEMKOwzosDFl6BZ+kZTARfG6ZeO3Fo231M49wMDsDtP8NbHHt5iZyON6NwlBy1j7ti8EFp W7z9tuSKMFv+BdydG3vZjzFSZi+QlcvQ3rngCYSquDDhfA2NwciO6Gj2NLvgyC5T4GXSqM7ix2kTX gG0evCYZErHM0d1Fchf0hNwvc9hnksO9RitWOBpo9U99Yx+EO3HLK3omd/f484LbG7TWAy+xJmd26 FGvyxsCW+gUp+9TiOeoQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h6BaA-0003hs-Ff; Tue, 19 Mar 2019 10:02:54 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h6BZC-0002ai-NQ for linux-arm-kernel@lists.infradead.org; Tue, 19 Mar 2019 10:01:57 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 965481650; Tue, 19 Mar 2019 03:01:52 -0700 (PDT) Received: from why.wild-wind.fr.eu.org (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 23A993F575; Tue, 19 Mar 2019 03:01:48 -0700 (PDT) Date: Tue, 19 Mar 2019 10:01:41 +0000 From: Marc Zyngier To: Zenghui Yu Subject: Re: [RFC PATCH] KVM: arm/arm64: Enable direct irqfd MSI injection Message-ID: <20190319100141.69821f8b@why.wild-wind.fr.eu.org> In-Reply-To: <428b2aac-5a0f-e9da-8d74-8045f99a8c74@huawei.com> References: <1552833373-19828-1-git-send-email-yuzenghui@huawei.com> <86o969z42z.wl-marc.zyngier@arm.com> <428b2aac-5a0f-e9da-8d74-8045f99a8c74@huawei.com> Organization: ARM Ltd X-Mailer: Claws Mail 3.17.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190319_030154_889109_8EBADC2E X-CRM114-Status: GOOD ( 28.66 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: rkrcmar@redhat.com, mst@redhat.com, kvm@vger.kernel.org, julien.thierry@arm.com, "Raslan, KarimAllah" , andre.przywara@arm.com, suzuki.poulose@arm.com, christoffer.dall@arm.com, linux-kernel@vger.kernel.org, eric.auger@redhat.com, james.morse@arm.com, wanghaibin.wang@huawei.com, guoheyi@huawei.com, pbonzini@redhat.com, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 19 Mar 2019 09:09:43 +0800 Zenghui Yu wrote: > Hi all, > > On 2019/3/18 3:35, Marc Zyngier wrote: > > On Sun, 17 Mar 2019 14:36:13 +0000, > > Zenghui Yu wrote: > >> > >> Currently, IRQFD on arm still uses the deferred workqueue mechanism > >> to inject interrupts into guest, which will likely lead to a busy > >> context-switching from/to the kworker thread. This overhead is for > >> no purpose (only in my view ...) and will result in an interrupt > >> performance degradation. > >> > >> Implement kvm_arch_set_irq_inatomic() for arm/arm64 to support direct > >> irqfd MSI injection, by which we can get rid of the annoying latency. > >> As a result, irqfd MSI intensive scenarios (e.g., DPDK with high packet > >> processing workloads) will benefit from it. > >> > >> Signed-off-by: Zenghui Yu > >> --- > >> > >> It seems that only MSI will follow the IRQFD path, did I miss something? > >> > >> This patch is still under test and sent out for early feedback. If I have > >> any mis-understanding, please fix me up and let me know. Thanks! > > > > As mentioned by other folks in the thread, this is clearly wrong. The > > first thing kvm_inject_msi does is to lock the corresponding ITS using > > a mutex. So the "no purpose" bit was a bit too quick. > > > > When doing this kind of work, I suggest you enable lockdep and all the > > related checkers. Also, for any optimisation, please post actual > > numbers for the relevant benchmarks. Saying "application X will > > benefit from it" is meaningless without any actual data. > > > >> > >> --- > >> virt/kvm/arm/vgic/trace.h | 22 ++++++++++++++++++++++ > >> virt/kvm/arm/vgic/vgic-irqfd.c | 21 +++++++++++++++++++++ > >> 2 files changed, 43 insertions(+) > >> > >> diff --git a/virt/kvm/arm/vgic/trace.h b/virt/kvm/arm/vgic/trace.h > >> index 55fed77..bc1f4db 100644 > >> --- a/virt/kvm/arm/vgic/trace.h > >> +++ b/virt/kvm/arm/vgic/trace.h > >> @@ -27,6 +27,28 @@ > >> __entry->vcpu_id, __entry->irq, __entry->level) > >> ); > >> >> +TRACE_EVENT(kvm_arch_set_irq_inatomic, > >> + TP_PROTO(u32 gsi, u32 type, int level, int irq_source_id), > >> + TP_ARGS(gsi, type, level, irq_source_id), > >> + > >> + TP_STRUCT__entry( > >> + __field( u32, gsi ) > >> + __field( u32, type ) > >> + __field( int, level ) > >> + __field( int, irq_source_id ) > >> + ), > >> + > >> + TP_fast_assign( > >> + __entry->gsi = gsi; > >> + __entry->type = type; > >> + __entry->level = level; > >> + __entry->irq_source_id = irq_source_id; > >> + ), > >> + > >> + TP_printk("gsi %u type %u level %d source %d", __entry->gsi, > >> + __entry->type, __entry->level, __entry->irq_source_id) > >> +); > >> + > >> #endif /* _TRACE_VGIC_H */ > >> >> #undef TRACE_INCLUDE_PATH > >> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c > >> index 99e026d..4cfc3f4 100644 > >> --- a/virt/kvm/arm/vgic/vgic-irqfd.c > >> +++ b/virt/kvm/arm/vgic/vgic-irqfd.c > >> @@ -19,6 +19,7 @@ > >> #include > >> #include > >> #include "vgic.h" > >> +#include "trace.h" > >> >> /** > >> * vgic_irqfd_set_irq: inject the IRQ corresponding to the > >> @@ -105,6 +106,26 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, > >> return vgic_its_inject_msi(kvm, &msi); > >> } > >> >> +/** > >> + * kvm_arch_set_irq_inatomic: fast-path for irqfd injection > >> + * > >> + * Currently only direct MSI injecton is supported. > >> + */ > >> +int kvm_arch_set_irq_inatomic(struct kvm_kernel_irq_routing_entry *e, > >> + struct kvm *kvm, int irq_source_id, int level, > >> + bool line_status) > >> +{ > >> + int ret; > >> + > >> + trace_kvm_arch_set_irq_inatomic(e->gsi, e->type, level, irq_source_id); > >> + > >> + if (unlikely(e->type != KVM_IRQ_ROUTING_MSI)) > >> + return -EWOULDBLOCK; > >> + > >> + ret = kvm_set_msi(e, kvm, irq_source_id, level, line_status); > >> + return ret; > >> +} > >> + > > > > Although we've established that the approach is wrong, maybe we can > > look at improving this aspect. > > > > A first approach would be to keep a small cache of the last few > > successful translations for this ITS, cache that could be looked-up by > > holding a spinlock instead. A hit in this cache could directly be > > injected. Any command that invalidates or changes anything (DISCARD, > > INV, INVALL, MAPC with V=0, MAPD with V=0, MOVALL, MOVI) should nuke > > the cache altogether. > > > > Of course, all of that needs to be quantified. > > Thanks for all of your explanations, especially for Marc's suggestions! > It took me long time to figure out my mistakes, since I am not very > familiar with the locking stuff. Now I have to apologize for my noise. No need to apologize. The whole point of this list is to have discussions. Although your approach wasn't working, you did identify potential room for improvement. > As for the its-translation-cache code (a really good news to us), we > have a rough look at it and start testing now! Please let me know about your findings. My initial test doesn't show any improvement, but that could easily be attributed to the system I running this on (a tiny and slightly broken dual A53 system). The sizing of the cache is also important: too small, and you have the overhead of the lookup for no benefit; too big, and you waste memory. Having thought about it a bit more, I think we can drop the invalidation on MOVI/MOVALL, as the LPI is still perfectly valid, and we don't cache the target vcpu. On the other hand, the cache must be nuked when the ITS is turned off. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel