From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755836AbbFLTDi (ORCPT ); Fri, 12 Jun 2015 15:03:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46589 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754552AbbFLTDh (ORCPT ); Fri, 12 Jun 2015 15:03:37 -0400 Message-ID: <1434135815.4927.308.camel@redhat.com> Subject: Re: [v4 08/16] KVM: kvm-vfio: User API for IRQ forwarding From: Alex Williamson To: Avi Kivity Cc: "Wu, Feng" , "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "pbonzini@redhat.com" , "mtosatti@redhat.com" , "eric.auger@linaro.org" Date: Fri, 12 Jun 2015 13:03:35 -0600 In-Reply-To: <557B2994.1070900@gmail.com> References: <1434019912-15423-1-git-send-email-feng.wu@intel.com> <1434019912-15423-9-git-send-email-feng.wu@intel.com> <5579E884.3040500@gmail.com> <1434123695.4927.304.camel@redhat.com> <557B2994.1070900@gmail.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2015-06-12 at 21:48 +0300, Avi Kivity wrote: > On 06/12/2015 06:41 PM, Alex Williamson wrote: > > On Fri, 2015-06-12 at 00:23 +0000, Wu, Feng wrote: > >>> -----Original Message----- > >>> From: Avi Kivity [mailto:avi.kivity@gmail.com] > >>> Sent: Friday, June 12, 2015 3:59 AM > >>> To: Wu, Feng; kvm@vger.kernel.org; linux-kernel@vger.kernel.org > >>> Cc: pbonzini@redhat.com; mtosatti@redhat.com; > >>> alex.williamson@redhat.com; eric.auger@linaro.org > >>> Subject: Re: [v4 08/16] KVM: kvm-vfio: User API for IRQ forwarding > >>> > >>> On 06/11/2015 01:51 PM, Feng Wu wrote: > >>>> From: Eric Auger > >>>> > >>>> This patch adds and documents a new KVM_DEV_VFIO_DEVICE group > >>>> and 2 device attributes: KVM_DEV_VFIO_DEVICE_FORWARD_IRQ, > >>>> KVM_DEV_VFIO_DEVICE_UNFORWARD_IRQ. The purpose is to be able > >>>> to set a VFIO device IRQ as forwarded or not forwarded. > >>>> the command takes as argument a handle to a new struct named > >>>> kvm_vfio_dev_irq. > >>> Is there no way to do this automatically? After all, vfio knows that a > >>> device interrupt is forwarded to some eventfd, and kvm knows that some > >>> eventfd is forwarded to a guest interrupt. If they compare notes > >>> through a central registry, they can figure out that the interrupt needs > >>> to be forwarded. > >> Oh, just like Eric mentioned in his reply, this description is out of context of > >> this series, I will remove them in the next version. > > > > I suspect Avi's question was more general. While forward/unforward is > > out of context for this series, it's very similar in nature to > > enabling/disabling posted interrupts. So I think the question remains > > whether we really need userspace to participate in creating this > > shortcut or if kvm and vfio can some how orchestrate figuring it out > > automatically. > > > > Personally I don't know how we could do it automatically. We've always > > relied on userspace to independently setup vfio and kvm such that > > neither have any idea that the other is there and update each side > > independently when anything changes. So it seems consistent to continue > > that here. It doesn't seem like there's much to gain performance-wise > > either, updates should be a relatively rare event I'd expect. > > > > There's really no metadata associated with an eventfd, so "comparing > > notes" automatically might imply some central registration entity. That > > immediately sounds like a much more complex solution, but maybe Avi has > > some ideas to manage it. Thanks, > > > > The idea is to have a central registry maintained by a posted interrupts > manager. Both vfio and kvm pass the filp (along with extra information) > to the posted interrupts manager, which, when it detects a filp match, > tells each of them what to do. > > The advantages are: > - old userspace gains the optimization without change > - a userspace API is more expensive to maintain than internal kernel > interfaces (CVEs, documentation, maintaining backwards compatibility) > - if you can do it without a new interface, this indicates that all the > information in the new interface is redundant. That means you have to > check it for consistency with the existing information, so it's extra > work (likely, it's exactly what the posted interrupt manager would be > doing anyway). Yep, those all sound like good things and I believe that's similar in design to the way we had originally discussed this interaction at LPC/KVM Forum several years ago. I'd be in favor of that approach. Thanks, Alex