Message ID | 1583749790-10837-4-git-send-email-yilun.xu@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
Series | Add interrupt support to FPGA DFL drivers | expand |
On Mon, Mar 09, 2020 at 06:29:46PM +0800, Xu Yilun wrote: > FPGA user applications may be interested in interrupts generated by > DFL features. For example, users can implement their own FPGA > logics with interrupts enabled in AFU (Accelerated Function Unit, > dynamic region of DFL based FPGA). So user applications need to be > notified to handle these interrupts. > > In order to allow userspace applications to monitor interrupts, > driver requires userspace to provide eventfds as interrupt > notification channels. Applications then poll/select on the eventfds > to get notified. > > This patch introduces a generic helper function for sub features to > do eventfds binding with given interrupts. > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> > Signed-off-by: Wu Hao <hao.wu@intel.com> > Signed-off-by: Xu Yilun <yilun.xu@intel.com> > --- > drivers/fpga/dfl.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > drivers/fpga/dfl.h | 11 +++++++ > 2 files changed, 104 insertions(+) > > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > index 493822d..ae6baca 100644 > --- a/drivers/fpga/dfl.c > +++ b/drivers/fpga/dfl.c > @@ -535,6 +535,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo) > int i, virq; > > /* save resource information for each feature */ > + feature->dev = fdev; > feature->id = finfo->fid; > feature->resource_index = index; > feature->ioaddr = finfo->ioaddr; > @@ -1373,6 +1374,98 @@ int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vfs) > } > EXPORT_SYMBOL_GPL(dfl_fpga_cdev_config_ports_vf); > > +static irqreturn_t dfl_irq_handler(int irq, void *arg) > +{ > + struct eventfd_ctx *trigger = arg; > + > + eventfd_signal(trigger, 1); > + return IRQ_HANDLED; > +} > + > +static int do_set_irq_trigger(struct dfl_feature *feature, int idx, int fd) > +{ > + struct platform_device *pdev = feature->dev; > + struct eventfd_ctx *trigger; > + int irq, ret; > + > + if (idx < 0 || idx >= feature->nr_irqs) > + return -EINVAL; > + > + irq = feature->irq_ctx[idx].irq; > + > + if (feature->irq_ctx[idx].trigger) { > + free_irq(irq, feature->irq_ctx[idx].trigger); > + kfree(feature->irq_ctx[idx].name); > + eventfd_ctx_put(feature->irq_ctx[idx].trigger); > + feature->irq_ctx[idx].trigger = NULL; > + } > + > + if (fd < 0) > + return 0; > + > + feature->irq_ctx[idx].name = > + kasprintf(GFP_KERNEL, "fpga-irq[%d](%s-%llx)", idx, > + dev_name(&pdev->dev), > + (unsigned long long)feature->id); > + if (!feature->irq_ctx[idx].name) > + return -ENOMEM; > + > + trigger = eventfd_ctx_fdget(fd); > + if (IS_ERR(trigger)) { > + ret = PTR_ERR(trigger); > + goto free_name; > + } > + > + ret = request_irq(irq, dfl_irq_handler, 0, > + feature->irq_ctx[idx].name, trigger); > + if (!ret) { > + feature->irq_ctx[idx].trigger = trigger; > + return ret; > + } > + > + eventfd_ctx_put(trigger); > +free_name: > + kfree(feature->irq_ctx[idx].name); > + > + return ret; > +} > + > +/** > + * dfl_fpga_set_irq_triggers - set eventfd triggers for dfl feature interrupts > + * > + * @feature: dfl sub feature. > + * @start: start of irq index in this dfl sub feature. > + * @count: number of irqs. > + * @fds: eventfds to bind with irqs. > + * > + * Bind given eventfds with irqs in this dfl sub feature. Use negative fds as > + * parameter to unbind irqs. Looks like it accepts NULL for fds to unbind irqs, please add some description here as well. > + * > + * Return: 0 on success, negative error code otherwise. > + */ > +int dfl_fpga_set_irq_triggers(struct dfl_feature *feature, unsigned int start, > + unsigned int count, int32_t *fds) > +{ > + int i, j, ret = 0; > + > + if (start + count < start || start + count > feature->nr_irqs) > + return -EINVAL; > + > + for (i = 0, j = start; i < count && !ret; i++, j++) { > + int fd = fds ? fds[i] : -1; > + > + ret = do_set_irq_trigger(feature, j, fd); > + } > + > + if (ret) { > + for (--j; j >= (int)start; j--) it converts unsigned int start to int, what about upper loop code? should start and count be converted as well? Thanks Hao > + do_set_irq_trigger(feature, j, -1); > + } > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(dfl_fpga_set_irq_triggers); > + > static void __exit dfl_fpga_exit(void) > { > dfl_chardev_uinit(); > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h > index 6a498cd..6b60077 100644 > --- a/drivers/fpga/dfl.h > +++ b/drivers/fpga/dfl.h > @@ -24,6 +24,8 @@ > #include <linux/slab.h> > #include <linux/uuid.h> > #include <linux/fpga/fpga-region.h> > +#include <linux/interrupt.h> > +#include <linux/eventfd.h> > > /* maximum supported number of ports */ > #define MAX_DFL_FPGA_PORT_NUM 4 > @@ -213,14 +215,19 @@ struct dfl_feature_driver { > * struct dfl_feature_irq_ctx - dfl private feature interrupt context > * > * @irq: Linux IRQ number of this interrupt. > + * @trigger: eventfd context to signal when interrupt happens. > + * @name: irq name needed when requesting irq. > */ > struct dfl_feature_irq_ctx { > int irq; > + struct eventfd_ctx *trigger; > + char *name; > }; > > /** > * struct dfl_feature - sub feature of the feature devices > * > + * @dev: ptr to pdev of the feature device which has the sub feature. > * @id: sub feature id. > * @resource_index: each sub feature has one mmio resource for its registers. > * this index is used to find its mmio resource from the > @@ -231,6 +238,7 @@ struct dfl_feature_irq_ctx { > * @ops: ops of this sub feature. > */ > struct dfl_feature { > + struct platform_device *dev; > u64 id; > int resource_index; > void __iomem *ioaddr; > @@ -506,4 +514,7 @@ int dfl_fpga_cdev_release_port(struct dfl_fpga_cdev *cdev, int port_id); > int dfl_fpga_cdev_assign_port(struct dfl_fpga_cdev *cdev, int port_id); > void dfl_fpga_cdev_config_ports_pf(struct dfl_fpga_cdev *cdev); > int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vf); > + > +int dfl_fpga_set_irq_triggers(struct dfl_feature *feature, unsigned int start, > + unsigned int count, int32_t *fds); > #endif /* __FPGA_DFL_H */ > -- > 2.7.4
On Tue, Mar 10, 2020 at 06:30:24PM +0800, Wu Hao wrote: > On Mon, Mar 09, 2020 at 06:29:46PM +0800, Xu Yilun wrote: > > FPGA user applications may be interested in interrupts generated by > > DFL features. For example, users can implement their own FPGA > > logics with interrupts enabled in AFU (Accelerated Function Unit, > > dynamic region of DFL based FPGA). So user applications need to be > > notified to handle these interrupts. > > > > In order to allow userspace applications to monitor interrupts, > > driver requires userspace to provide eventfds as interrupt > > notification channels. Applications then poll/select on the eventfds > > to get notified. > > > > This patch introduces a generic helper function for sub features to > > do eventfds binding with given interrupts. > > > > Signed-off-by: Luwei Kang <luwei.kang@intel.com> > > Signed-off-by: Wu Hao <hao.wu@intel.com> > > Signed-off-by: Xu Yilun <yilun.xu@intel.com> > > --- > > drivers/fpga/dfl.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > drivers/fpga/dfl.h | 11 +++++++ > > 2 files changed, 104 insertions(+) > > > > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > > index 493822d..ae6baca 100644 > > --- a/drivers/fpga/dfl.c > > +++ b/drivers/fpga/dfl.c > > @@ -535,6 +535,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo) > > int i, virq; > > > > /* save resource information for each feature */ > > + feature->dev = fdev; > > feature->id = finfo->fid; > > feature->resource_index = index; > > feature->ioaddr = finfo->ioaddr; > > @@ -1373,6 +1374,98 @@ int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vfs) > > } > > EXPORT_SYMBOL_GPL(dfl_fpga_cdev_config_ports_vf); > > > > +static irqreturn_t dfl_irq_handler(int irq, void *arg) > > +{ > > + struct eventfd_ctx *trigger = arg; > > + > > + eventfd_signal(trigger, 1); > > + return IRQ_HANDLED; > > +} > > + > > +static int do_set_irq_trigger(struct dfl_feature *feature, int idx, int fd) > > +{ > > + struct platform_device *pdev = feature->dev; > > + struct eventfd_ctx *trigger; > > + int irq, ret; > > + > > + if (idx < 0 || idx >= feature->nr_irqs) > > + return -EINVAL; > > + > > + irq = feature->irq_ctx[idx].irq; > > + > > + if (feature->irq_ctx[idx].trigger) { > > + free_irq(irq, feature->irq_ctx[idx].trigger); > > + kfree(feature->irq_ctx[idx].name); > > + eventfd_ctx_put(feature->irq_ctx[idx].trigger); > > + feature->irq_ctx[idx].trigger = NULL; > > + } > > + > > + if (fd < 0) > > + return 0; > > + > > + feature->irq_ctx[idx].name = > > + kasprintf(GFP_KERNEL, "fpga-irq[%d](%s-%llx)", idx, > > + dev_name(&pdev->dev), > > + (unsigned long long)feature->id); > > + if (!feature->irq_ctx[idx].name) > > + return -ENOMEM; > > + > > + trigger = eventfd_ctx_fdget(fd); > > + if (IS_ERR(trigger)) { > > + ret = PTR_ERR(trigger); > > + goto free_name; > > + } > > + > > + ret = request_irq(irq, dfl_irq_handler, 0, > > + feature->irq_ctx[idx].name, trigger); > > + if (!ret) { > > + feature->irq_ctx[idx].trigger = trigger; > > + return ret; > > + } > > + > > + eventfd_ctx_put(trigger); > > +free_name: > > + kfree(feature->irq_ctx[idx].name); > > + > > + return ret; > > +} > > + > > +/** > > + * dfl_fpga_set_irq_triggers - set eventfd triggers for dfl feature interrupts > > + * > > + * @feature: dfl sub feature. > > + * @start: start of irq index in this dfl sub feature. > > + * @count: number of irqs. > > + * @fds: eventfds to bind with irqs. > > + * > > + * Bind given eventfds with irqs in this dfl sub feature. Use negative fds as > > + * parameter to unbind irqs. > > Looks like it accepts NULL for fds to unbind irqs, please add some description > here as well. Sure. > > > + * > > + * Return: 0 on success, negative error code otherwise. > > + */ > > +int dfl_fpga_set_irq_triggers(struct dfl_feature *feature, unsigned int start, > > + unsigned int count, int32_t *fds) > > +{ > > + int i, j, ret = 0; > > + > > + if (start + count < start || start + count > feature->nr_irqs) > > + return -EINVAL; > > + > > + for (i = 0, j = start; i < count && !ret; i++, j++) { > > + int fd = fds ? fds[i] : -1; > > + > > + ret = do_set_irq_trigger(feature, j, fd); > > + } > > + > > + if (ret) { > > + for (--j; j >= (int)start; j--) > > it converts unsigned int start to int, what about upper loop code? > should start and count be converted as well? Using int for i, j seems not suitable. Let me change them to unsigned int and fix other parts accordingly. Thanks Yilun > > Thanks > Hao > > > + do_set_irq_trigger(feature, j, -1); > > + } > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(dfl_fpga_set_irq_triggers); > > + > > static void __exit dfl_fpga_exit(void) > > { > > dfl_chardev_uinit(); > > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h > > index 6a498cd..6b60077 100644 > > --- a/drivers/fpga/dfl.h > > +++ b/drivers/fpga/dfl.h > > @@ -24,6 +24,8 @@ > > #include <linux/slab.h> > > #include <linux/uuid.h> > > #include <linux/fpga/fpga-region.h> > > +#include <linux/interrupt.h> > > +#include <linux/eventfd.h> > > > > /* maximum supported number of ports */ > > #define MAX_DFL_FPGA_PORT_NUM 4 > > @@ -213,14 +215,19 @@ struct dfl_feature_driver { > > * struct dfl_feature_irq_ctx - dfl private feature interrupt context > > * > > * @irq: Linux IRQ number of this interrupt. > > + * @trigger: eventfd context to signal when interrupt happens. > > + * @name: irq name needed when requesting irq. > > */ > > struct dfl_feature_irq_ctx { > > int irq; > > + struct eventfd_ctx *trigger; > > + char *name; > > }; > > > > /** > > * struct dfl_feature - sub feature of the feature devices > > * > > + * @dev: ptr to pdev of the feature device which has the sub feature. > > * @id: sub feature id. > > * @resource_index: each sub feature has one mmio resource for its registers. > > * this index is used to find its mmio resource from the > > @@ -231,6 +238,7 @@ struct dfl_feature_irq_ctx { > > * @ops: ops of this sub feature. > > */ > > struct dfl_feature { > > + struct platform_device *dev; > > u64 id; > > int resource_index; > > void __iomem *ioaddr; > > @@ -506,4 +514,7 @@ int dfl_fpga_cdev_release_port(struct dfl_fpga_cdev *cdev, int port_id); > > int dfl_fpga_cdev_assign_port(struct dfl_fpga_cdev *cdev, int port_id); > > void dfl_fpga_cdev_config_ports_pf(struct dfl_fpga_cdev *cdev); > > int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vf); > > + > > +int dfl_fpga_set_irq_triggers(struct dfl_feature *feature, unsigned int start, > > + unsigned int count, int32_t *fds); > > #endif /* __FPGA_DFL_H */ > > -- > > 2.7.4
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index 493822d..ae6baca 100644 --- a/drivers/fpga/dfl.c +++ b/drivers/fpga/dfl.c @@ -535,6 +535,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo) int i, virq; /* save resource information for each feature */ + feature->dev = fdev; feature->id = finfo->fid; feature->resource_index = index; feature->ioaddr = finfo->ioaddr; @@ -1373,6 +1374,98 @@ int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vfs) } EXPORT_SYMBOL_GPL(dfl_fpga_cdev_config_ports_vf); +static irqreturn_t dfl_irq_handler(int irq, void *arg) +{ + struct eventfd_ctx *trigger = arg; + + eventfd_signal(trigger, 1); + return IRQ_HANDLED; +} + +static int do_set_irq_trigger(struct dfl_feature *feature, int idx, int fd) +{ + struct platform_device *pdev = feature->dev; + struct eventfd_ctx *trigger; + int irq, ret; + + if (idx < 0 || idx >= feature->nr_irqs) + return -EINVAL; + + irq = feature->irq_ctx[idx].irq; + + if (feature->irq_ctx[idx].trigger) { + free_irq(irq, feature->irq_ctx[idx].trigger); + kfree(feature->irq_ctx[idx].name); + eventfd_ctx_put(feature->irq_ctx[idx].trigger); + feature->irq_ctx[idx].trigger = NULL; + } + + if (fd < 0) + return 0; + + feature->irq_ctx[idx].name = + kasprintf(GFP_KERNEL, "fpga-irq[%d](%s-%llx)", idx, + dev_name(&pdev->dev), + (unsigned long long)feature->id); + if (!feature->irq_ctx[idx].name) + return -ENOMEM; + + trigger = eventfd_ctx_fdget(fd); + if (IS_ERR(trigger)) { + ret = PTR_ERR(trigger); + goto free_name; + } + + ret = request_irq(irq, dfl_irq_handler, 0, + feature->irq_ctx[idx].name, trigger); + if (!ret) { + feature->irq_ctx[idx].trigger = trigger; + return ret; + } + + eventfd_ctx_put(trigger); +free_name: + kfree(feature->irq_ctx[idx].name); + + return ret; +} + +/** + * dfl_fpga_set_irq_triggers - set eventfd triggers for dfl feature interrupts + * + * @feature: dfl sub feature. + * @start: start of irq index in this dfl sub feature. + * @count: number of irqs. + * @fds: eventfds to bind with irqs. + * + * Bind given eventfds with irqs in this dfl sub feature. Use negative fds as + * parameter to unbind irqs. + * + * Return: 0 on success, negative error code otherwise. + */ +int dfl_fpga_set_irq_triggers(struct dfl_feature *feature, unsigned int start, + unsigned int count, int32_t *fds) +{ + int i, j, ret = 0; + + if (start + count < start || start + count > feature->nr_irqs) + return -EINVAL; + + for (i = 0, j = start; i < count && !ret; i++, j++) { + int fd = fds ? fds[i] : -1; + + ret = do_set_irq_trigger(feature, j, fd); + } + + if (ret) { + for (--j; j >= (int)start; j--) + do_set_irq_trigger(feature, j, -1); + } + + return ret; +} +EXPORT_SYMBOL_GPL(dfl_fpga_set_irq_triggers); + static void __exit dfl_fpga_exit(void) { dfl_chardev_uinit(); diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h index 6a498cd..6b60077 100644 --- a/drivers/fpga/dfl.h +++ b/drivers/fpga/dfl.h @@ -24,6 +24,8 @@ #include <linux/slab.h> #include <linux/uuid.h> #include <linux/fpga/fpga-region.h> +#include <linux/interrupt.h> +#include <linux/eventfd.h> /* maximum supported number of ports */ #define MAX_DFL_FPGA_PORT_NUM 4 @@ -213,14 +215,19 @@ struct dfl_feature_driver { * struct dfl_feature_irq_ctx - dfl private feature interrupt context * * @irq: Linux IRQ number of this interrupt. + * @trigger: eventfd context to signal when interrupt happens. + * @name: irq name needed when requesting irq. */ struct dfl_feature_irq_ctx { int irq; + struct eventfd_ctx *trigger; + char *name; }; /** * struct dfl_feature - sub feature of the feature devices * + * @dev: ptr to pdev of the feature device which has the sub feature. * @id: sub feature id. * @resource_index: each sub feature has one mmio resource for its registers. * this index is used to find its mmio resource from the @@ -231,6 +238,7 @@ struct dfl_feature_irq_ctx { * @ops: ops of this sub feature. */ struct dfl_feature { + struct platform_device *dev; u64 id; int resource_index; void __iomem *ioaddr; @@ -506,4 +514,7 @@ int dfl_fpga_cdev_release_port(struct dfl_fpga_cdev *cdev, int port_id); int dfl_fpga_cdev_assign_port(struct dfl_fpga_cdev *cdev, int port_id); void dfl_fpga_cdev_config_ports_pf(struct dfl_fpga_cdev *cdev); int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vf); + +int dfl_fpga_set_irq_triggers(struct dfl_feature *feature, unsigned int start, + unsigned int count, int32_t *fds); #endif /* __FPGA_DFL_H */