On 06/24/2014 05:47 PM, Alexander Graf wrote: > > 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. Yep I agree. But currently the usage of routing mechanism is not explicit neither in 4.75 KVM_IRQFD and that was causing my doubts;-) On top of that the doc says: "kvm_irqfd.gsi specifies the irqchip pin toggled by this event" but if there is no routing table set before by user-side, the injection will fail. > >> >> 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? Yes that's correct. but the effort to integrate such feature in the code need to be further assessed. Eric > > > Alex > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
