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?. 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. Best Regards 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
