Re: [PATCH v7 2/2] kvm: KVM_EOIFD, an eventfd for EOIs
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2012-08-02 08:42:37
Also in:
lkml
On Mon, Jul 30, 2012 at 07:12:15PM -0600, Alex Williamson wrote:
quoted
quoted
quoted
quoted
quoted
quoted
kvm_eoifd.fd specifies the eventfd used for +notification. KVM_EOIFD_FLAG_DEASSIGN is used to de-assign an eoifd +once assigned. KVM_EOIFD also requires additional bits set in +kvm_eoifd.flags to bind to the proper interrupt line. The +KVM_EOIFD_FLAG_LEVEL_IRQFD indicates that kvm_eoifd.key is provided +and is a key from a level triggered interrupt (configured from +KVM_IRQFD using KVM_IRQFD_FLAG_LEVEL). The EOI notification is bound +to the same GSI and irqchip input as the irqfd. Both kvm_eoifd.key +and KVM_EOIFD_FLAG_LEVEL_IRQFD must be specified on assignment and +de-assignment of KVM_EOIFD. A level irqfd may only be bound to a +single eoifd. KVM_CAP_EOIFD_LEVEL_IRQFD indicates support of +KVM_EOIFD_FLAG_LEVEL_IRQFD.Hmm returning the key means we'll need to keep refcounting for source IDs around forever. I liked passing the fd better: make implementation match interface and not the other way around.False, a source ID has a finite lifecycle. The fd approach was broken. Holding the irqfd context imposed too many dependencies between eoifd and irqfd necessitating things like one interface disabling another. I thoroughly disagree with that approach.You keep saying this but it is still true: once irqfd is closed eoifd does not get any more interrupts.How does that matter?Well if it does not get events it is disabled. so you have one ifc disabling another, anyway.And a level irqfd without an eoifd can never be de-asserted. Either we make modular components, assemble them to do useful work, and disassemble them independently so they can be used by future interfaces or we bundle eoifd as just an option of irqfd. Which is it gonna be?
I'm fine just making it an option. I think Gleb wanted a separate EOIFD to handle timedrift but it later seemed that eventfd is not suitable for that? -- MST