Coupling between KVM_IRQFD and KVM_SET_GSI_ROUTING?

DISCUSSION Eric Auger 3 participants 8 messages Jun 17, 2014 – Jun 24, 2014 KVM

lore

Share & tools

lore

Eric Auger

author

2014-06-17 11:40 UTC #0 lore raw

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

In the positive, shouldn't we clarify the KVM API documentation?

Thank you in advance

Best Regards

Eric

Eric Auger

author

2014-06-19 14:03 UTC #1 lore raw

Hi,

If I am not wrong I did not get any reply to this question. Can anyone share his/her experience/knowledge on this gsi routing/irqfd use case?

Thank you in advance

Best Regards

Eric

Quoted 25 lines · Eric Auger

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).
In the positive, shouldn't we clarify the KVM API documentation?
Thank you in advance
Best Regards
Eric

Alexander Graf

2014-06-23 16:47 UTC #2 lore raw

In reply to Eric Auger

Quoted 9 lines · Eric Auger

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.

Quoted 7 lines

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.

Splitting that dependency out is certainly quite tedious work. However, I'm sure the IBM folks will be grateful if you do it :).

Alex

Paul Mackerras

2014-06-24 09:47 UTC #3 lore raw

Quoted 19 lines · Alexander Graf

Quoted 9 lines · Eric Auger
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.
Quoted 7 lines
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).

Quoted 7 lines

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

The other way I see to solve my particular problem, which is that userspace tends to use hardware interrupt numbers starting at 4096, is to replace the arrays in struct kvm_irq_routing_table with idr structures or something similar.

Quoted 2 lines

Splitting that dependency out is certainly quite tedious work. However, I'm sure the IBM folks will be grateful if you do it :).

Indeed. :)

Paul.

Alexander Graf

2014-06-24 10:40 UTC #4 lore raw

Quoted 25 lines · Paul Mackerras

Quoted 17 lines · Alexander Graf
Quoted 9 lines · Eric Auger
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.
Quoted 7 lines
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).
Quoted 7 lines
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.

Quoted 7 lines

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

Eric Auger

author

2014-06-24 15:05 UTC #5 lore raw

Quoted 19 lines · Alexander Graf

Quoted 17 lines · Paul Mackerras
Quoted 16 lines · Alexander Graf
Quoted 13 lines · Eric Auger
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.

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
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?

Quoted 8 lines

Quoted 8 lines
Quoted 5 lines
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().

Quoted 8 lines

Quoted 8 lines
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.

Quoted 9 lines

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?

Quoted 9 lines

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

Quoted 4 lines

Alex

Alexander Graf

2014-06-24 15:47 UTC #6 lore raw

Quoted 53 lines · Eric Auger

Quoted 17 lines · Alexander Graf
Quoted 16 lines · Paul Mackerras
Quoted 15 lines · Alexander Graf
Quoted 13 lines · Eric Auger
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.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
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?
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
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().
Quoted 7 lines
Quoted 7 lines
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.
Quoted 4 lines
You could also create it along with the irqfd state struct. So instead of
   kzalloc(sizeof(*irqfd), GFP_KERNEL);

Quoted 8 lines

Quoted 4 lines
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.

Quoted 8 lines

That way would not need a complete & big routing table to be set by user-side?
Quoted 4 lines
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.

Quoted 8 lines

Quoted 4 lines
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

Eric Auger

author

2014-06-24 16:12 UTC #7 lore raw

Quoted 66 lines · Alexander Graf

Quoted 64 lines · Eric Auger
Quoted 25 lines · Alexander Graf
Quoted 24 lines · Paul Mackerras
Quoted 23 lines · Alexander Graf
Quoted 21 lines · Eric Auger
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).
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
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?
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
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().
Quoted 7 lines
Quoted 7 lines
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.
Quoted 9 lines
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);

Quoted 6 lines

Quoted 3 lines
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.

Quoted 13 lines

Quoted 13 lines
That way would not need a complete & big routing table to be set by user-side?
Quoted 8 lines
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.

Quoted 7 lines

Quoted 4 lines
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

Quoted 8 lines

Alex

Signature · 3 lines

To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html