Re: Coupling between KVM_IRQFD and KVM_SET_GSI_ROUTING?

[Alexander Graf <agraf@suse.de>]

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.
>>
>> 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.
>
> 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

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);

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.


Alex