[v2,02/12] raw/ifpga_rawdev/base: add irq support
Checks
Commit Message
From: Tianfei zhang <tianfei.zhang@intel.com>
Add irq support for ifpga FME globle error, port error and uint unit.
We implmented this feature by vfio interrupt mechanism.
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c | 61 +++++++++++++++++++++++
drivers/raw/ifpga_rawdev/base/ifpga_fme_error.c | 22 ++++++++
drivers/raw/ifpga_rawdev/base/ifpga_port.c | 20 ++++++++
drivers/raw/ifpga_rawdev/base/ifpga_port_error.c | 21 ++++++++
4 files changed, 124 insertions(+)
Comments
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Rosen Xu
> Sent: Friday, August 2, 2019 6:49 AM
> To: dev@dpdk.org
> Cc: ferruh.yigit@intel.com; tianfei.zhang@intel.com; rosen.xu@intel.com;
> andy.pei@intel.com; david.lomartire@intel.com; qi.z.zhang@intel.com;
> xiaolong.ye@intel.com
> Subject: [dpdk-dev] [PATCH v2 02/12] raw/ifpga_rawdev/base: add irq
> support
>
> From: Tianfei zhang <tianfei.zhang@intel.com>
>
> Add irq support for ifpga FME globle error, port error and uint unit.
> We implmented this feature by vfio interrupt mechanism.
>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> ---
> drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c | 61
> +++++++++++++++++++++++
> drivers/raw/ifpga_rawdev/base/ifpga_fme_error.c | 22 ++++++++
> drivers/raw/ifpga_rawdev/base/ifpga_port.c | 20 ++++++++
> drivers/raw/ifpga_rawdev/base/ifpga_port_error.c | 21 ++++++++
> 4 files changed, 124 insertions(+)
>
> diff --git a/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c
> b/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c
> index 63c8bcc..6b942e6 100644
> --- a/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c
> +++ b/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c
> @@ -3,6 +3,7 @@
> */
>
> #include <sys/ioctl.h>
> +#include <rte_vfio.h>
>
> #include "ifpga_feature_dev.h"
>
> @@ -331,3 +332,63 @@ int port_hw_init(struct ifpga_port_hw *port)
> port_hw_uinit(port);
> return ret;
> }
> +
> +/*
> + * FIXME: we should get msix vec count during pci enumeration instead
> +of
> + * below hardcode value.
> + */
> +#define FPGA_MSIX_VEC_COUNT 20
> +/* irq set buffer length for interrupt */ #define MSIX_IRQ_SET_BUF_LEN
> +(sizeof(struct vfio_irq_set) + \
> + sizeof(int) * FPGA_MSIX_VEC_COUNT)
> +
> +/* only support msix for now*/
> +static int vfio_msix_enable_block(s32 vfio_dev_fd, unsigned int vec_start,
> + unsigned int count, s32 *fds)
Isn't better to use generic EAL function for the same?
> +{
> + char irq_set_buf[MSIX_IRQ_SET_BUF_LEN];
> + struct vfio_irq_set *irq_set;
> + int len, ret;
> + int *fd_ptr;
> +
> + len = sizeof(irq_set_buf);
> +
> + irq_set = (struct vfio_irq_set *)irq_set_buf;
> + irq_set->argsz = len;
> + irq_set->count = count;
> + irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> + VFIO_IRQ_SET_ACTION_TRIGGER;
> + irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
> + irq_set->start = vec_start;
> +
> + fd_ptr = (int *)&irq_set->data;
> + memcpy(fd_ptr, fds, sizeof(int) * count);
> +
> + ret = ioctl(vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set);
> + if (ret)
> + printf("Error enabling MSI-X interrupts\n");
> +
> + return ret;
> +}
> +
> -----Original Message-----
> From: Jerin Jacob Kollanukkaran [mailto:jerinj@marvell.com]
> Sent: Friday, August 2, 2019 11:58 AM
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Tianfei
> <tianfei.zhang@intel.com>; Pei, Andy <andy.pei@intel.com>; Lomartire,
> David <david.lomartire@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Ye,
> Xiaolong <xiaolong.ye@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 02/12] raw/ifpga_rawdev/base: add irq
> support
>
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Rosen Xu
> > Sent: Friday, August 2, 2019 6:49 AM
> > To: dev@dpdk.org
> > Cc: ferruh.yigit@intel.com; tianfei.zhang@intel.com;
> > rosen.xu@intel.com; andy.pei@intel.com; david.lomartire@intel.com;
> > qi.z.zhang@intel.com; xiaolong.ye@intel.com
> > Subject: [dpdk-dev] [PATCH v2 02/12] raw/ifpga_rawdev/base: add irq
> > support
> >
> > From: Tianfei zhang <tianfei.zhang@intel.com>
> >
> > Add irq support for ifpga FME globle error, port error and uint unit.
> > We implmented this feature by vfio interrupt mechanism.
> >
> > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> > ---
> > drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c | 61
> > +++++++++++++++++++++++
> > drivers/raw/ifpga_rawdev/base/ifpga_fme_error.c | 22 ++++++++
> > drivers/raw/ifpga_rawdev/base/ifpga_port.c | 20 ++++++++
> > drivers/raw/ifpga_rawdev/base/ifpga_port_error.c | 21 ++++++++
> > 4 files changed, 124 insertions(+)
> >
> > diff --git a/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c
> > b/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c
> > index 63c8bcc..6b942e6 100644
> > --- a/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c
> > +++ b/drivers/raw/ifpga_rawdev/base/ifpga_feature_dev.c
> > @@ -3,6 +3,7 @@
> > */
> >
> > #include <sys/ioctl.h>
> > +#include <rte_vfio.h>
> >
> > #include "ifpga_feature_dev.h"
> >
> > @@ -331,3 +332,63 @@ int port_hw_init(struct ifpga_port_hw *port)
> > port_hw_uinit(port);
> > return ret;
> > }
> > +
> > +/*
> > + * FIXME: we should get msix vec count during pci enumeration instead
> > +of
> > + * below hardcode value.
> > + */
> > +#define FPGA_MSIX_VEC_COUNT 20
> > +/* irq set buffer length for interrupt */ #define
> > +MSIX_IRQ_SET_BUF_LEN (sizeof(struct vfio_irq_set) + \
> > + sizeof(int) * FPGA_MSIX_VEC_COUNT)
> > +
> > +/* only support msix for now*/
> > +static int vfio_msix_enable_block(s32 vfio_dev_fd, unsigned int vec_start,
> > + unsigned int count, s32 *fds)
>
> Isn't better to use generic EAL function for the same?
In our PAC N3000 Card, we have 6 PCIe MSI-X vectors, for example:
0~3 for AFU
4 for Port
6 for FME
FME (FPGA Management Engine ) will manage all resources in FPGA, like partition reconfiguration, Power manager, thermal, Error reporting.
Port is a bridge between FME and AFU.
AFU is the accelerator unit which for customers logic.
So, we reserve some MSI-X vectors for end-user/customers to use the AFU, and end-user/customers can use
the AFU for networking acceleration or other acceleration.
The DPDK existing API like rte_intr_enable()->vfio_enable_msix() will bind all of the vectors at the same time and those vectors will register into one evenfd and one interrupt handler function.
That cannot satisfy our design. we hope that, each MSI-X vector bind into VFIO and register the interrupt handler function separately. Because the reserve vectors like
0~3 vectors for AFU, we don't know what exact usage for the end-user/customers in AFU logic, so it had better let them bind VFIO and register interrupt handler themselves.
One suggestion is we expand the vfio_enable_msix() function, let the caller to specify the start vector and the numbers of vectors to bind the VFIO.
static int
vfio_enable_msix(const struct rte_intr_handle *intr_handle, int start, int count) {
...
irq_set->count = count;
irq_set->start = start;
...
return 0;
}
> -----Original Message-----
> From: Zhang, Tianfei <tianfei.zhang@intel.com>
> Sent: Friday, August 2, 2019 3:36 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Xu, Rosen
> <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Pei, Andy <andy.pei@intel.com>;
> Lomartire, David <david.lomartire@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
> Subject: [EXT] RE: [dpdk-dev] [PATCH v2 02/12] raw/ifpga_rawdev/base: add
> irq support
>
> > > +
> > > +/* only support msix for now*/
> > > +static int vfio_msix_enable_block(s32 vfio_dev_fd, unsigned int
> vec_start,
> > > + unsigned int count, s32 *fds)
> >
> > Isn't better to use generic EAL function for the same?
>
> In our PAC N3000 Card, we have 6 PCIe MSI-X vectors, for example:
> 0~3 for AFU
> 4 for Port
> 6 for FME
>
> FME (FPGA Management Engine ) will manage all resources in FPGA, like
> partition reconfiguration, Power manager, thermal, Error reporting.
> Port is a bridge between FME and AFU.
> AFU is the accelerator unit which for customers logic.
>
> So, we reserve some MSI-X vectors for end-user/customers to use the AFU,
> and end-user/customers can use the AFU for networking acceleration or
> other acceleration.
>
> The DPDK existing API like rte_intr_enable()->vfio_enable_msix() will bind all
> of the vectors at the same time and those vectors will register into one
> evenfd and one interrupt handler function.
> That cannot satisfy our design. we hope that, each MSI-X vector bind into
> VFIO and register the interrupt handler function separately. Because the
> reserve vectors like
> 0~3 vectors for AFU, we don't know what exact usage for the end-
> user/customers in AFU logic, so it had better let them bind VFIO and register
> interrupt handler themselves.
>
> One suggestion is we expand the vfio_enable_msix() function, let the caller
> to specify the start vector and the numbers of vectors to bind the VFIO.
Yes, Probably have two variants, vfio_enable_msix() alias to count of 1
> static int
> vfio_enable_msix(const struct rte_intr_handle *intr_handle, int start, int
> count) {
> ...
> irq_set->count = count;
> irq_set->start = start;
> ...
> return 0;
> }
>
@@ -3,6 +3,7 @@
*/
#include <sys/ioctl.h>
+#include <rte_vfio.h>
#include "ifpga_feature_dev.h"
@@ -331,3 +332,63 @@ int port_hw_init(struct ifpga_port_hw *port)
port_hw_uinit(port);
return ret;
}
+
+/*
+ * FIXME: we should get msix vec count during pci enumeration instead of
+ * below hardcode value.
+ */
+#define FPGA_MSIX_VEC_COUNT 20
+/* irq set buffer length for interrupt */
+#define MSIX_IRQ_SET_BUF_LEN (sizeof(struct vfio_irq_set) + \
+ sizeof(int) * FPGA_MSIX_VEC_COUNT)
+
+/* only support msix for now*/
+static int vfio_msix_enable_block(s32 vfio_dev_fd, unsigned int vec_start,
+ unsigned int count, s32 *fds)
+{
+ char irq_set_buf[MSIX_IRQ_SET_BUF_LEN];
+ struct vfio_irq_set *irq_set;
+ int len, ret;
+ int *fd_ptr;
+
+ len = sizeof(irq_set_buf);
+
+ irq_set = (struct vfio_irq_set *)irq_set_buf;
+ irq_set->argsz = len;
+ irq_set->count = count;
+ irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
+ VFIO_IRQ_SET_ACTION_TRIGGER;
+ irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
+ irq_set->start = vec_start;
+
+ fd_ptr = (int *)&irq_set->data;
+ memcpy(fd_ptr, fds, sizeof(int) * count);
+
+ ret = ioctl(vfio_dev_fd, VFIO_DEVICE_SET_IRQS, irq_set);
+ if (ret)
+ printf("Error enabling MSI-X interrupts\n");
+
+ return ret;
+}
+
+int fpga_msix_set_block(struct ifpga_feature *feature, unsigned int start,
+ unsigned int count, s32 *fds)
+{
+ struct feature_irq_ctx *ctx = feature->ctx;
+ unsigned int i;
+ int ret;
+
+ if (start >= feature->ctx_num || start + count > feature->ctx_num)
+ return -EINVAL;
+
+ /* assume that each feature has continuous vector space in msix*/
+ ret = vfio_msix_enable_block(feature->vfio_dev_fd,
+ ctx[start].idx, count, fds);
+ if (!ret) {
+ for (i = 0; i < count; i++)
+ ctx[i].eventfd = fds[i];
+ }
+
+ return ret;
+}
+
@@ -373,9 +373,31 @@ static int fme_global_error_set_prop(struct ifpga_feature *feature,
return -ENOENT;
}
+static int fme_global_err_set_irq(struct ifpga_feature *feature, void *irq_set)
+{
+ struct fpga_fme_err_irq_set *err_irq_set =
+ (struct fpga_fme_err_irq_set *)irq_set;
+ struct ifpga_fme_hw *fme;
+ int ret;
+
+ fme = (struct ifpga_fme_hw *)feature->parent;
+
+ spinlock_lock(&fme->lock);
+ if (!(fme->capability & FPGA_FME_CAP_ERR_IRQ)) {
+ spinlock_unlock(&fme->lock);
+ return -ENODEV;
+ }
+
+ ret = fpga_msix_set_block(feature, 0, 1, &err_irq_set->evtfd);
+ spinlock_unlock(&fme->lock);
+
+ return ret;
+}
+
struct ifpga_feature_ops fme_global_err_ops = {
.init = fme_global_error_init,
.uinit = fme_global_error_uinit,
.get_prop = fme_global_error_get_prop,
.set_prop = fme_global_error_set_prop,
+ .set_irq = fme_global_err_set_irq,
};
@@ -384,9 +384,29 @@ static void port_uint_uinit(struct ifpga_feature *feature)
dev_info(NULL, "PORT UINT UInit.\n");
}
+static int port_uint_set_irq(struct ifpga_feature *feature, void *irq_set)
+{
+ struct fpga_uafu_irq_set *uafu_irq_set = irq_set;
+ struct ifpga_port_hw *port = feature->parent;
+ int ret;
+
+ spinlock_lock(&port->lock);
+ if (!(port->capability & FPGA_PORT_CAP_UAFU_IRQ)) {
+ spinlock_unlock(&port->lock);
+ return -ENODEV;
+ }
+
+ ret = fpga_msix_set_block(feature, uafu_irq_set->start,
+ uafu_irq_set->count, uafu_irq_set->evtfds);
+ spinlock_unlock(&port->lock);
+
+ return ret;
+}
+
struct ifpga_feature_ops ifpga_rawdev_port_uint_ops = {
.init = port_uint_init,
.uinit = port_uint_uinit,
+ .set_irq = port_uint_set_irq,
};
static int port_afu_init(struct ifpga_feature *feature)
@@ -136,9 +136,30 @@ static int port_error_set_prop(struct ifpga_feature *feature,
return -ENOENT;
}
+static int port_error_set_irq(struct ifpga_feature *feature, void *irq_set)
+{
+ struct fpga_port_err_irq_set *err_irq_set = irq_set;
+ struct ifpga_port_hw *port;
+ int ret;
+
+ port = feature->parent;
+
+ spinlock_lock(&port->lock);
+ if (!(port->capability & FPGA_PORT_CAP_ERR_IRQ)) {
+ spinlock_unlock(&port->lock);
+ return -ENODEV;
+ }
+
+ ret = fpga_msix_set_block(feature, 0, 1, &err_irq_set->evtfd);
+ spinlock_unlock(&port->lock);
+
+ return ret;
+}
+
struct ifpga_feature_ops ifpga_rawdev_port_error_ops = {
.init = port_error_init,
.uinit = port_error_uinit,
.get_prop = port_error_get_prop,
.set_prop = port_error_set_prop,
+ .set_irq = port_error_set_irq,
};