Acked-by: Nithin Dabilpuram <[email protected]> --
On 11/10/26, 7:45 PM, "Mohit Kumar Parjapat" <[email protected]> wrote: All MSI-X vectors were registered on pci_dev->intr_handle, rewriting its fd before each rte_intr_callback_register(). Since EAL references the caller's handle instead of a copy, all vectors share one interrupt source whose fd is read live, so only the last vector is polled. Mailbox replies are lost, probe times out, and the cleanup path leaves a closed fd in the source, making the interrupt thread panic in epoll_ctl(). Register each vector on its own handle, tracked in a list so that unregister uses and frees the same handle. Reject duplicate vector registration, close the vector's own eventfd on unregister, and unwind all error paths of plt_irq_register(). Fixes: c663018eb7f3 ("interrupts: do not duplicate interrupt handles") Signed-off-by: Mohit Kumar Parjapat <[email protected]> Signed-off-by: Harman Kalra <[email protected]> --- .mailmap | 1 + drivers/common/cnxk/roc_platform.c | 134 ++++++++++++++++++++++++----- 2 files changed, 115 insertions(+), 20 deletions(-) diff --git a/.mailmap b/.mailmap index 754dd9e760..8ca51f72d7 100644 --- a/.mailmap +++ b/.mailmap @@ -1175,6 +1175,7 @@ Mohammad Iqbal Ahmad <[email protected]> Mohammad Shuab Siddique <[email protected]> Mohammed Gamal <[email protected]> Mohand Alrasheed <[email protected]> +Mohit Kumar Parjapat <[email protected]> Mohsin Kazmi <[email protected]> Mohsin Mazhar Shaikh <[email protected]> Mohsin Shaikh <[email protected]> diff --git a/drivers/common/cnxk/roc_platform.c b/drivers/common/cnxk/roc_platform.c index 59d662fab4..0a1b96092b 100644 --- a/drivers/common/cnxk/roc_platform.c +++ b/drivers/common/cnxk/roc_platform.c @@ -16,8 +16,49 @@ #include <stdlib.h> #include <sys/eventfd.h> #include <sys/ioctl.h> +#include <sys/queue.h> #include <unistd.h> +/* + * EAL interrupt sources are keyed by interrupt handle pointer and read the + * handle FD at epoll/dispatch time, so each MSI-X vector needs its own + * handle holding that vector's eventfd. + */ +struct plt_irq_vec { + TAILQ_ENTRY(plt_irq_vec) next; + const struct plt_intr_handle *dev_handle; + struct plt_intr_handle *handle; + plt_intr_callback_fn cb; + void *data; + unsigned int vec; +}; + +static TAILQ_HEAD(, plt_irq_vec) plt_irq_vec_list = TAILQ_HEAD_INITIALIZER(plt_irq_vec_list); +static plt_spinlock_t plt_irq_vec_lock = RTE_SPINLOCK_INITIALIZER; + +static struct plt_irq_vec * +irq_vec_find(const struct plt_intr_handle *dev_handle, unsigned int vec) +{ + struct plt_irq_vec *irq_vec; + + TAILQ_FOREACH(irq_vec, &plt_irq_vec_list, next) { + if (irq_vec->dev_handle == dev_handle && irq_vec->vec == vec) + return irq_vec; + } + + return NULL; +} + +static void +irq_vec_free(struct plt_irq_vec *irq_vec) +{ + if (irq_vec->handle != NULL) { + plt_intr_fd_close(irq_vec->handle); + plt_intr_instance_free(irq_vec->handle); + } + free(irq_vec); +} + static int irq_get_info(struct plt_intr_handle *intr_handle) { @@ -150,7 +191,7 @@ int plt_irq_register(struct plt_intr_handle *intr_handle, plt_intr_callback_fn cb, void *data, unsigned int vec) { - struct plt_intr_handle *tmp_handle; + struct plt_irq_vec *irq_vec; uint32_t nb_efd, tmp_nb_efd; int rc, fd; @@ -166,25 +207,61 @@ plt_irq_register(struct plt_intr_handle *intr_handle, plt_intr_callback_fn cb, v return -EINVAL; } - tmp_handle = intr_handle; + irq_vec = calloc(1, sizeof(*irq_vec)); + if (irq_vec == NULL) + return -ENOMEM; + irq_vec->dev_handle = intr_handle; + irq_vec->cb = cb; + irq_vec->data = data; + irq_vec->vec = vec; + + irq_vec->handle = plt_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE); + if (irq_vec->handle == NULL) { + rc = -ENOMEM; + goto free_vec; + } + /* Create new eventfd for interrupt vector */ fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); - if (fd == -1) - return -ENODEV; + if (fd == -1) { + rc = -ENODEV; + goto free_vec; + } - if (plt_intr_fd_set(tmp_handle, fd)) - return -errno; + if (plt_intr_fd_set(irq_vec->handle, fd)) { + close(fd); + rc = -EINVAL; + goto free_vec; + } + + /* EAL reads the 8 byte eventfd counter only for VFIO handle types */ + if (plt_intr_type_set(irq_vec->handle, plt_intr_type_get(intr_handle))) { + rc = -EINVAL; + goto free_vec; + } + + plt_spinlock_lock(&plt_irq_vec_lock); + if (irq_vec_find(intr_handle, vec) != NULL) { + plt_spinlock_unlock(&plt_irq_vec_lock); + plt_err("Vector:0x%x irq already registered", vec); + rc = -EEXIST; + goto free_vec; + } + TAILQ_INSERT_TAIL(&plt_irq_vec_list, irq_vec, next); + plt_spinlock_unlock(&plt_irq_vec_lock); /* Register vector interrupt callback */ - rc = plt_intr_callback_register(tmp_handle, cb, data); + rc = plt_intr_callback_register(irq_vec->handle, cb, data); if (rc) { plt_err("Failed to register vector:0x%x irq callback.", vec); - return rc; + goto remove_vec; } rc = plt_intr_efds_index_set(intr_handle, vec, fd); - if (rc) - return rc; + if (rc) { + plt_intr_callback_unregister(irq_vec->handle, cb, data); + goto remove_vec; + } nb_efd = (vec > (uint32_t)plt_intr_nb_efd_get(intr_handle)) ? vec : @@ -199,15 +276,23 @@ plt_irq_register(struct plt_intr_handle *intr_handle, plt_intr_callback_fn cb, v /* Enable MSIX vectors to VFIO */ return irq_config(intr_handle, vec); + +remove_vec: + plt_spinlock_lock(&plt_irq_vec_lock); + TAILQ_REMOVE(&plt_irq_vec_list, irq_vec, next); + plt_spinlock_unlock(&plt_irq_vec_lock); +free_vec: + irq_vec_free(irq_vec); + return rc; } void plt_irq_unregister(struct plt_intr_handle *intr_handle, plt_intr_callback_fn cb, void *data, unsigned int vec) { - struct plt_intr_handle *tmp_handle; + struct plt_irq_vec *irq_vec; uint8_t retries = 5; /* 5 ms */ - int rc, fd; + int rc; if (vec > (uint32_t)plt_intr_max_intr_get(intr_handle)) { plt_err("Error unregistering MSI-X interrupts vec:%d > %d", vec, @@ -215,17 +300,23 @@ plt_irq_unregister(struct plt_intr_handle *intr_handle, plt_intr_callback_fn cb, return; } - tmp_handle = intr_handle; - fd = plt_intr_efds_index_get(intr_handle, vec); - if (fd == -1) + plt_spinlock_lock(&plt_irq_vec_lock); + irq_vec = irq_vec_find(intr_handle, vec); + plt_spinlock_unlock(&plt_irq_vec_lock); + if (irq_vec == NULL) { + if (plt_intr_efds_index_get(intr_handle, vec) != -1) + plt_err("Error unregistering MSI-X vec %d cb, not registered", vec); return; + } - if (plt_intr_fd_set(tmp_handle, fd)) + if (irq_vec->cb != cb || irq_vec->data != data) { + plt_err("MSI-X vec %d registered with a different callback", vec); return; + } do { /* Un-register callback func from platform lib */ - rc = plt_intr_callback_unregister(tmp_handle, cb, data); + rc = plt_intr_callback_unregister(irq_vec->handle, cb, data); /* Retry only if -EAGAIN */ if (rc != -EAGAIN) break; @@ -238,15 +329,18 @@ plt_irq_unregister(struct plt_intr_handle *intr_handle, plt_intr_callback_fn cb, return; } + plt_spinlock_lock(&plt_irq_vec_lock); + TAILQ_REMOVE(&plt_irq_vec_list, irq_vec, next); + plt_spinlock_unlock(&plt_irq_vec_lock); + plt_base_dbg("Disable vector:0x%x for vfio (efds: %d, max:%d)", vec, plt_intr_nb_efd_get(intr_handle), plt_intr_max_intr_get(intr_handle)); - if (plt_intr_efds_index_get(intr_handle, vec) != -1) - close(plt_intr_efds_index_get(intr_handle, vec)); /* Disable MSIX vectors from VFIO */ plt_intr_efds_index_set(intr_handle, vec, -1); - irq_config(intr_handle, vec); + + irq_vec_free(irq_vec); } #endif -- 2.43.0
