From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: Coupling between KVM_IRQFD and KVM_SET_GSI_ROUTING? Date: Tue, 24 Jun 2014 17:47:06 +0200 Message-ID: <53A99D7A.20008@suse.de> References: <53A0290B.8040301@linaro.org> <53A85A37.9030103@suse.de> <20140624094713.GA24698@iris.ozlabs.ibm.com> <53A955A7.4060101@suse.de> <53A9939C.20903@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Antonios Motakis , "kvmarm@lists.cs.columbia.edu" , Marc Zyngier , "kvm@vger.kernel.org" , Christoffer Dall , Paolo Bonzini To: Eric Auger , Paul Mackerras Return-path: Received: from cantor2.suse.de ([195.135.220.15]:47671 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752949AbaFXPrJ (ORCPT ); Tue, 24 Jun 2014 11:47:09 -0400 In-Reply-To: <53A9939C.20903@linaro.org> Sender: kvm-owner@vger.kernel.org List-ID: On 24.06.14 17:05, Eric Auger wrote: > On 06/24/2014 12:40 PM, Alexander Graf wrote: >> On 24.06.14 11:47, Paul Mackerras wrote: >>> On Mon, Jun 23, 2014 at 06:47:51PM +0200, Alexander Graf wrote: >>>> On 17.06.14 13:39, Eric Auger wrote: > >>>>> Hello, >>>>> >>>>> I have a question related to KVM_IRQFD and KVM_SET_GSI_ROUTING ioctl >>>>> relationship. >>>>> >>>>> When reading the KVM API documentation I do not understand there is any >>>>> dependency between KVM_IRQFD and KVM_SET_GSI_ROUTING. According to the >>>>> text it seems only the gsi field is used and interpreted as the >>>>> irqchip pin. >>>>> >>>>> However irqchip.c kvm_set_irq code relies on an existing and not dummy >>>>> routing table. >>>>> >>>>> My question is: does anyone agree on the fact the user-side must set a >>>>> consistent routing table using KVM_SET_GSI_ROUTING before using >>>>> KVM_IRQFD? The other alternative would have been to build a default >>>>> identity GSI routing table in the kernel (gsi = irqchip.pin). >>>> I untangled irqfd support from the x86 ioapic emulation a while back. >>>> When I >>>> looked at it, I didn't see any easy way to split it out from the routing >>>> too, so I kept that dependency in. > Hi Alex, Paul, > > thanks for your replies. hence don't you think the KVM API doc deserves > to be clarified then? >>>> If you look at the code, you will see that the irq routing entry is >>>> used as >>>> token for an irqfd. So every irqfd only knows its routing table entry, >>>> nothing else. >>> As far as I can see, the routing table entry is used for only two >>> things: to tell whether the interrupt is an MSI or LSI, and to pass to >>> kvm_set_msi. > > Indeed I noticed irq_entry in _irqfd struct only is set for MSI routing. > For other IRQS - I guess what you call LSI - , routine table map[] is > used to retrieve the irchip.pin(s) associated to that gsi and call set(). >>> One way to tackle the problem would be to abstract out the routing >>> table lookup into a function in irqchip.c, rather than having it >>> open-coded in irqfd_update. Then, if you don't want to have the >>> routing table you write an alternative function that creates a >>> routing entry from scratch. It would need information from the >>> low-level irq chip code as to which interrupts are LSIs and which are >>> MSIs. It also ties you down to having only one kind of interrupt > controller. >> You could also create it along with the irqfd state struct. So instead of >> >> kzalloc(sizeof(*irqfd), GFP_KERNEL); >> >> we could do >> >> kzalloc(sizeof(*irqfd) + sizeof(struct kvm_kernel_irq_routing_entry), >> GFP_KERNEL); > > Wouldn't it make sense, in kvm_irqfd_assign, to test whether there is a > routing entry associated to that gsi. In the negative, create a default > one where irqchip.pin = gsi or something architecture specific?. "no entry" really should be "no entry". I think if we want some other mechanism, we should make it be explicit. > > That way would not need a complete & big routing table to be set by > user-side? > >> and point the routing entry to the inline irqfd one. We'd lose the >> ability to change any routings with that construct, but I think that's >> ok for irq controllers that are not configurable. >> >> Alternatively, we could also have a single routing entry that is a >> wildcard match, ranging from say LSI [x...y]. The irqfd structure would >> then need to carry a local payload to define an offset inside that >> wildcard routing entry. > Also whatever the number of IRQs assigned, we have this big > chip[KVM_NR_IRQCHIPS][KVM_IRQCHIP_NUM_PINS] allocated. Also on ARM we > have some difficulties to define KVM_IRQCHIP_NUM_PINS which becomes > quite dynamic. Right, if you go with the ranged routing entries you should be able to get by with a very small number of NUM_PINS, no? Alex