From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Marc Zyngier <maz@kernel.org>,
Oliver Upton <oliver.upton@linux.dev>,
Sean Christopherson <seanjc@google.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
linux-riscv@lists.infradead.org,
David Matlack <dmatlack@google.com>,
Juergen Gross <jgross@suse.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Subject: [PATCH 07/12] KVM: Disallow binding multiple irqfds to an eventfd with a priority waiter
Date: Tue, 1 Apr 2025 13:44:19 -0700 [thread overview]
Message-ID: <20250401204425.904001-8-seanjc@google.com> (raw)
In-Reply-To: <20250401204425.904001-1-seanjc@google.com>
Disallow binding an irqfd to an eventfd that already has a priority waiter,
i.e. to an eventfd that already has an attached irqfd. KVM always
operates in exclusive mode for EPOLL_IN (unconditionally returns '1'),
i.e. only the first waiter will be notified.
KVM already disallows binding multiple irqfds to an eventfd in a single
VM, but doesn't guard against multiple VMs binding to an eventfd. Adding
the extra protection reduces the pain of a userspace VMM bug, e.g. if
userspace fails to de-assign before re-assigning when transferring state
for intra-host migration, then the migration will explicitly fail as
opposed to dropping IRQs on the destination VM.
Temporarily keep KVM's manual check on irqfds.items, but add a WARN, e.g.
to allow sanity checking the waitqueue enforcement.
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: David Matlack <dmatlack@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
virt/kvm/eventfd.c | 54 +++++++++++++++++++++++++++++++---------------
1 file changed, 37 insertions(+), 17 deletions(-)
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index a33c10bd042a..25c360ed2e1e 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -291,37 +291,57 @@ static void kvm_irqfd_register(struct file *file, wait_queue_head_t *wqh,
struct kvm_kernel_irqfd *tmp;
struct kvm *kvm = p->kvm;
+ /*
+ * Note, irqfds.lock protects the irqfd's irq_entry, i.e. its routing,
+ * and irqfds.items. It does NOT protect registering with the eventfd.
+ */
spin_lock_irq(&kvm->irqfds.lock);
- list_for_each_entry(tmp, &kvm->irqfds.items, list) {
- if (irqfd->eventfd != tmp->eventfd)
- continue;
- /* This fd is used for another irq already. */
- p->ret = -EBUSY;
- spin_unlock_irq(&kvm->irqfds.lock);
- return;
- }
-
+ /*
+ * Initialize the routing information prior to adding the irqfd to the
+ * eventfd's waitqueue, as irqfd_wakeup() can be invoked as soon as the
+ * irqfd is registered.
+ */
irqfd_update(kvm, irqfd);
- list_add_tail(&irqfd->list, &kvm->irqfds.items);
-
/*
* Add the irqfd as a priority waiter on the eventfd, with a custom
* wake-up handler, so that KVM *and only KVM* is notified whenever the
- * underlying eventfd is signaled. Temporarily lie to lockdep about
- * holding irqfds.lock to avoid a false positive regarding potential
- * deadlock with irqfd_wakeup() (see irqfd_wakeup() for details).
+ * underlying eventfd is signaled.
*/
init_waitqueue_func_entry(&irqfd->wait, irqfd_wakeup);
+ /*
+ * Temporarily lie to lockdep about holding irqfds.lock to avoid a
+ * false positive regarding potential deadlock with irqfd_wakeup()
+ * (see irqfd_wakeup() for details).
+ *
+ * Adding to the wait queue will fail if there is already a priority
+ * waiter, i.e. if the eventfd is associated with another irqfd (in any
+ * VM). Note, kvm_irqfd_deassign() waits for all in-flight shutdown
+ * jobs to complete, i.e. ensures the irqfd has been removed from the
+ * eventfd's waitqueue before returning to userspace.
+ */
spin_release(&kvm->irqfds.lock.dep_map, _RET_IP_);
- add_wait_queue_priority(wqh, &irqfd->wait);
+ p->ret = add_wait_queue_priority_exclusive(wqh, &irqfd->wait);
spin_acquire(&kvm->irqfds.lock.dep_map, 0, 0, _RET_IP_);
+ if (p->ret)
+ goto out;
+ list_for_each_entry(tmp, &kvm->irqfds.items, list) {
+ if (irqfd->eventfd != tmp->eventfd)
+ continue;
+
+ WARN_ON_ONCE(1);
+ /* This fd is used for another irq already. */
+ p->ret = -EBUSY;
+ goto out;
+ }
+
+ list_add_tail(&irqfd->list, &kvm->irqfds.items);
+
+out:
spin_unlock_irq(&kvm->irqfds.lock);
-
- p->ret = 0;
}
#ifdef CONFIG_HAVE_KVM_IRQ_BYPASS
--
2.49.0.504.g3bcea36a83-goog
WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Marc Zyngier <maz@kernel.org>,
Oliver Upton <oliver.upton@linux.dev>,
Sean Christopherson <seanjc@google.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
linux-riscv@lists.infradead.org,
David Matlack <dmatlack@google.com>,
Juergen Gross <jgross@suse.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Subject: [PATCH 07/12] KVM: Disallow binding multiple irqfds to an eventfd with a priority waiter
Date: Tue, 1 Apr 2025 13:44:19 -0700 [thread overview]
Message-ID: <20250401204425.904001-8-seanjc@google.com> (raw)
In-Reply-To: <20250401204425.904001-1-seanjc@google.com>
Disallow binding an irqfd to an eventfd that already has a priority waiter,
i.e. to an eventfd that already has an attached irqfd. KVM always
operates in exclusive mode for EPOLL_IN (unconditionally returns '1'),
i.e. only the first waiter will be notified.
KVM already disallows binding multiple irqfds to an eventfd in a single
VM, but doesn't guard against multiple VMs binding to an eventfd. Adding
the extra protection reduces the pain of a userspace VMM bug, e.g. if
userspace fails to de-assign before re-assigning when transferring state
for intra-host migration, then the migration will explicitly fail as
opposed to dropping IRQs on the destination VM.
Temporarily keep KVM's manual check on irqfds.items, but add a WARN, e.g.
to allow sanity checking the waitqueue enforcement.
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: David Matlack <dmatlack@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
virt/kvm/eventfd.c | 54 +++++++++++++++++++++++++++++++---------------
1 file changed, 37 insertions(+), 17 deletions(-)
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index a33c10bd042a..25c360ed2e1e 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -291,37 +291,57 @@ static void kvm_irqfd_register(struct file *file, wait_queue_head_t *wqh,
struct kvm_kernel_irqfd *tmp;
struct kvm *kvm = p->kvm;
+ /*
+ * Note, irqfds.lock protects the irqfd's irq_entry, i.e. its routing,
+ * and irqfds.items. It does NOT protect registering with the eventfd.
+ */
spin_lock_irq(&kvm->irqfds.lock);
- list_for_each_entry(tmp, &kvm->irqfds.items, list) {
- if (irqfd->eventfd != tmp->eventfd)
- continue;
- /* This fd is used for another irq already. */
- p->ret = -EBUSY;
- spin_unlock_irq(&kvm->irqfds.lock);
- return;
- }
-
+ /*
+ * Initialize the routing information prior to adding the irqfd to the
+ * eventfd's waitqueue, as irqfd_wakeup() can be invoked as soon as the
+ * irqfd is registered.
+ */
irqfd_update(kvm, irqfd);
- list_add_tail(&irqfd->list, &kvm->irqfds.items);
-
/*
* Add the irqfd as a priority waiter on the eventfd, with a custom
* wake-up handler, so that KVM *and only KVM* is notified whenever the
- * underlying eventfd is signaled. Temporarily lie to lockdep about
- * holding irqfds.lock to avoid a false positive regarding potential
- * deadlock with irqfd_wakeup() (see irqfd_wakeup() for details).
+ * underlying eventfd is signaled.
*/
init_waitqueue_func_entry(&irqfd->wait, irqfd_wakeup);
+ /*
+ * Temporarily lie to lockdep about holding irqfds.lock to avoid a
+ * false positive regarding potential deadlock with irqfd_wakeup()
+ * (see irqfd_wakeup() for details).
+ *
+ * Adding to the wait queue will fail if there is already a priority
+ * waiter, i.e. if the eventfd is associated with another irqfd (in any
+ * VM). Note, kvm_irqfd_deassign() waits for all in-flight shutdown
+ * jobs to complete, i.e. ensures the irqfd has been removed from the
+ * eventfd's waitqueue before returning to userspace.
+ */
spin_release(&kvm->irqfds.lock.dep_map, _RET_IP_);
- add_wait_queue_priority(wqh, &irqfd->wait);
+ p->ret = add_wait_queue_priority_exclusive(wqh, &irqfd->wait);
spin_acquire(&kvm->irqfds.lock.dep_map, 0, 0, _RET_IP_);
+ if (p->ret)
+ goto out;
+ list_for_each_entry(tmp, &kvm->irqfds.items, list) {
+ if (irqfd->eventfd != tmp->eventfd)
+ continue;
+
+ WARN_ON_ONCE(1);
+ /* This fd is used for another irq already. */
+ p->ret = -EBUSY;
+ goto out;
+ }
+
+ list_add_tail(&irqfd->list, &kvm->irqfds.items);
+
+out:
spin_unlock_irq(&kvm->irqfds.lock);
-
- p->ret = 0;
}
#ifdef CONFIG_HAVE_KVM_IRQ_BYPASS
--
2.49.0.504.g3bcea36a83-goog
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2025-04-01 20:47 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-01 20:44 [PATCH 00/12] KVM: Make irqfd registration globally unique Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-01 20:44 ` [PATCH 01/12] KVM: Use a local struct to do the initial vfs_poll() on an irqfd Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-01 20:44 ` [PATCH 02/12] KVM: Acquire SCRU lock outside of irqfds.lock during assignment Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-01 20:44 ` [PATCH 03/12] KVM: Initialize irqfd waitqueue callback when adding to the queue Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-01 20:44 ` [PATCH 04/12] KVM: Add irqfd to KVM's list via the vfs_poll() callback Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-01 20:44 ` [PATCH 05/12] KVM: Add irqfd to eventfd's waitqueue while holding irqfds.lock Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-01 20:44 ` [PATCH 06/12] sched/wait: Add a waitqueue helper for fully exclusive priority waiters Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-02 18:40 ` K Prateek Nayak
2025-04-02 18:40 ` K Prateek Nayak
2025-04-01 20:44 ` Sean Christopherson [this message]
2025-04-01 20:44 ` [PATCH 07/12] KVM: Disallow binding multiple irqfds to an eventfd with a priority waiter Sean Christopherson
2025-04-01 20:44 ` [PATCH 08/12] sched/wait: Drop WQ_FLAG_EXCLUSIVE from add_wait_queue_priority() Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-01 20:44 ` [PATCH 09/12] KVM: Drop sanity check that per-VM list of irqfds is unique Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-01 20:44 ` [PATCH 10/12] KVM: selftests: Assert that eventfd() succeeds in Xen shinfo test Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-01 20:44 ` [PATCH 11/12] KVM: selftests: Add utilities to create eventfds and do KVM_IRQFD Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
2025-04-01 20:44 ` [PATCH 12/12] KVM: selftests: Add a KVM_IRQFD test to verify uniqueness requirements Sean Christopherson
2025-04-01 20:44 ` Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250401204425.904001-8-seanjc@google.com \
--to=seanjc@google.com \
--cc=aou@eecs.berkeley.edu \
--cc=dmatlack@google.com \
--cc=jgross@suse.com \
--cc=juri.lelli@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=maz@kernel.org \
--cc=mingo@redhat.com \
--cc=oleksandr_tyshchenko@epam.com \
--cc=oliver.upton@linux.dev \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=sstabellini@kernel.org \
--cc=vincent.guittot@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.