[dpdk-dev] [PATCH v2 1/4] eal: add a new req notifier to eal interrupt
Andrew Rybchenko
arybchenko at solarflare.com
Mon Oct 1 11:46:02 CEST 2018
On 9/30/18 5:16 PM, Jeff Guo wrote:
> Add a new req notifier in eal interrupt for enable vfio hotplug.
>
> Signed-off-by: Jeff Guo <jia.guo at intel.com>
> ---
> v2->v1:
> no change
> ---
> lib/librte_eal/common/include/rte_eal_interrupts.h | 1 +
> lib/librte_eal/linuxapp/eal/eal_interrupts.c | 71 ++++++++++++++++++++++
> 2 files changed, 72 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
> index 6eb4932..2c47738 100644
> --- a/lib/librte_eal/common/include/rte_eal_interrupts.h
> +++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
> @@ -35,6 +35,7 @@ enum rte_intr_handle_type {
> RTE_INTR_HANDLE_EXT, /**< external handler */
> RTE_INTR_HANDLE_VDEV, /**< virtual device */
> RTE_INTR_HANDLE_DEV_EVENT, /**< device event handle */
> + RTE_INTR_HANDLE_VFIO_REQ, /**< vfio device handle (req) */
Alignment looks inconsistent. Is there any reason?
> RTE_INTR_HANDLE_MAX /**< count of elements */
> };
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> index 4076c6d..7f611b3 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> @@ -308,6 +308,64 @@ vfio_disable_msix(const struct rte_intr_handle *intr_handle) {
>
> return ret;
> }
> +
> +/* enable req notifier */
> +static int
> +vfio_enable_req(const struct rte_intr_handle *intr_handle)
> +{
> + int len, ret;
> + char irq_set_buf[IRQ_SET_BUF_LEN];
I see that it is copied from similar functions in the file, but
I'd suggest to initialize it with zeros using '= {};' just to make
sure that uninitialized on-stack data never go to kernel.
> + struct vfio_irq_set *irq_set;
> + int *fd_ptr;
> +
> + len = sizeof(irq_set_buf);
> +
> + irq_set = (struct vfio_irq_set *) irq_set_buf;
> + irq_set->argsz = len;
> + irq_set->count = 1;
> + irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> + VFIO_IRQ_SET_ACTION_TRIGGER;
> + irq_set->index = VFIO_PCI_REQ_IRQ_INDEX;
It looks like it is the only difference (plus error log below) from
vfio_enable_msi(). May be it make sense to restructure code
to avoid duplication. Obviously it is not critical, but please, consider.
Similar comment is applicable to vfio_disable_req() below.
> + irq_set->start = 0;
> + fd_ptr = (int *) &irq_set->data;
> + *fd_ptr = intr_handle->fd;
> +
> + ret = ioctl(intr_handle->vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set);
> +
> + if (ret) {
> + RTE_LOG(ERR, EAL, "Error enabling req interrupts for fd %d\n",
> + intr_handle->fd);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +/* disable req notifier */
> +static int
> +vfio_disable_req(const struct rte_intr_handle *intr_handle)
> +{
> + struct vfio_irq_set *irq_set;
> + char irq_set_buf[IRQ_SET_BUF_LEN];
> + int len, ret;
> +
> + len = sizeof(struct vfio_irq_set);
> +
> + irq_set = (struct vfio_irq_set *) irq_set_buf;
> + irq_set->argsz = len;
> + irq_set->count = 0;
> + irq_set->flags = VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_TRIGGER;
> + irq_set->index = VFIO_PCI_REQ_IRQ_INDEX;
> + irq_set->start = 0;
> +
> + ret = ioctl(intr_handle->vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set);
> +
> + if (ret)
> + RTE_LOG(ERR, EAL, "Error disabling req interrupts for fd %d\n",
> + intr_handle->fd);
> +
> + return ret;
> +}
> #endif
>
> static int
> @@ -556,6 +614,10 @@ rte_intr_enable(const struct rte_intr_handle *intr_handle)
> if (vfio_enable_intx(intr_handle))
> return -1;
> break;
> + case RTE_INTR_HANDLE_VFIO_REQ:
> + if (vfio_enable_req(intr_handle))
> + return -1;
> + break;
> #endif
> /* not used at this moment */
> case RTE_INTR_HANDLE_DEV_EVENT:
> @@ -606,6 +668,11 @@ rte_intr_disable(const struct rte_intr_handle *intr_handle)
> if (vfio_disable_intx(intr_handle))
> return -1;
> break;
> + case RTE_INTR_HANDLE_VFIO_REQ:
> + if (vfio_disable_req(intr_handle))
> + return -1;
> + break;
> +
> #endif
> /* not used at this moment */
> case RTE_INTR_HANDLE_DEV_EVENT:
> @@ -682,6 +749,10 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
> bytes_read = 0;
> call = true;
> break;
> + case RTE_INTR_HANDLE_VFIO_REQ:
> + bytes_read = 0;
> + call = true;
> + break;
> default:
> bytes_read = 1;
> break;
More information about the dev
mailing list