lkml.org 
[lkml]   [2020]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v5 3/7] fpga: dfl: introduce interrupt trigger setting API
    On Mon, Apr 20, 2020 at 04:11:39PM +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 functions to do eventfds binding
    > with given interrupts.
    >
    > Sub feature drivers are expected to use XXX_GET_IRQ_NUM to query irq
    > info, and XXX_SET_IRQ to set eventfds for interrupts. This patch also
    > introduces helper functions for these 2 ioctls.
    >
    > 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>
    > Acked-by: Wu Hao <hao.wu@intel.com>
    > ----
    > v2: use unsigned int instead of int for irq array indexes in
    > dfl_fpga_set_irq_triggers()
    > Improves comments for NULL fds param in dfl_fpga_set_irq_triggers()
    > v3: Improve comments of dfl_fpga_set_irq_triggers()
    > refines code for dfl_fpga_set_irq_triggers, delete local variable j
    > v4: Introduce 2 helper functions to help handle the XXX_GET_IRQ_NUM &
    > XXX_SET_IRQ ioctls for sub feature drivers.
    > v5: Some minor fix for Hao's comments
    > ---
    > drivers/fpga/dfl.c | 156 ++++++++++++++++++++++++++++++++++++++++++
    > drivers/fpga/dfl.h | 17 +++++
    > include/uapi/linux/fpga-dfl.h | 13 ++++
    > 3 files changed, 186 insertions(+)
    >
    > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
    > index b49fbed..208d8f0 100644
    > --- a/drivers/fpga/dfl.c
    > +++ b/drivers/fpga/dfl.c
    > @@ -10,7 +10,9 @@
    > * Wu Hao <hao.wu@intel.com>
    > * Xiao Guangrong <guangrong.xiao@linux.intel.com>
    > */
    > +#include <linux/fpga-dfl.h>
    > #include <linux/module.h>
    > +#include <linux/uaccess.h>
    >
    > #include "dfl.h"
    >
    > @@ -534,6 +536,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
    > unsigned int i;
    >
    > /* save resource information for each feature */
    > + feature->dev = fdev;
    > feature->id = finfo->fid;
    > feature->resource_index = index;
    > feature->ioaddr = finfo->ioaddr;
    > @@ -1395,6 +1398,159 @@ 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, unsigned int idx,
    > + int fd)
    > +{
    > + struct platform_device *pdev = feature->dev;
    > + struct eventfd_ctx *trigger;
    > + int irq, ret;
    > +
    > + if (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[%u](%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. unbind related irq if fds[n] is negative.
    > + * unbind "count" specified number of irqs if fds ptr is NULL.
    > + *
    > + * Bind given eventfds with irqs in this dfl sub feature. Unbind related irq if
    > + * fds[n] is negative. Unbind "count" specified number of irqs if fds ptr is
    > + * NULL.
    > + *
    > + * 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)
    > +{
    > + unsigned int i;
    > + int ret = 0;
    > +
    > + if (start + count < start || start + count > feature->nr_irqs)
    > + return -EINVAL;
    Up to you, but have you considered to break up the condition above into
    two if () ?
    > +
    > + for (i = 0; i < count; i++) {
    > + int fd = fds ? fds[i] : -1;
    > +
    > + ret = do_set_irq_trigger(feature, start + i, fd);
    > + if (ret) {
    > + while (i--)
    > + do_set_irq_trigger(feature, start + i, -1);
    > + break;
    > + }
    > + }
    > +
    > + return ret;
    > +}
    > +EXPORT_SYMBOL_GPL(dfl_fpga_set_irq_triggers);
    > +
    > +/**
    > + * dfl_feature_ioctl_get_num_irqs - dfl feature _GET_IRQ_NUM ioctl interface.
    > + * @pdev: the feature device which has the sub feature
    > + * @feature: the dfl sub feature
    > + * @arg: ioctl argument
    > + *
    > + * Return: 0 on success, negative error code otherwise.
    > + */
    > +long dfl_feature_ioctl_get_num_irqs(struct platform_device *pdev,
    > + struct dfl_feature *feature,
    > + unsigned long arg)
    > +{
    > + return put_user(feature->nr_irqs, (__u32 __user *)arg);
    > +}
    > +EXPORT_SYMBOL_GPL(dfl_feature_ioctl_get_num_irqs);
    > +
    > +/**
    > + * dfl_feature_ioctl_set_irq - dfl feature _SET_IRQ ioctl interface.
    > + * @pdev: the feature device which has the sub feature
    > + * @feature: the dfl sub feature
    > + * @arg: ioctl argument
    > + *
    > + * Return: 0 on success, negative error code otherwise.
    > + */
    > +long dfl_feature_ioctl_set_irq(struct platform_device *pdev,
    > + struct dfl_feature *feature,
    > + unsigned long arg)
    > +{
    > + struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
    > + struct dfl_fpga_irq_set hdr;
    > + s32 *fds;
    > + long ret;
    > +
    > + if (!feature->nr_irqs)
    > + return -ENOENT;
    > +
    > + if (copy_from_user(&hdr, (void __user *)arg, sizeof(hdr)))
    > + return -EFAULT;
    > +
    > + if (!hdr.count || (hdr.start + hdr.count > feature->nr_irqs) ||
    > + (hdr.start + hdr.count < hdr.start))
    > + return -EINVAL;
    > +
    > + fds = memdup_user((void __user *)(arg + sizeof(hdr)),
    > + hdr.count * sizeof(s32));
    > + if (IS_ERR(fds))
    > + return PTR_ERR(fds);
    > +
    > + mutex_lock(&pdata->lock);
    > + ret = dfl_fpga_set_irq_triggers(feature, hdr.start, hdr.count, fds);
    > + mutex_unlock(&pdata->lock);
    > +
    > + kfree(fds);
    > + return ret;
    > +}
    > +EXPORT_SYMBOL_GPL(dfl_feature_ioctl_set_irq);
    > +
    > static void __exit dfl_fpga_exit(void)
    > {
    > dfl_chardev_uinit();
    > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
    > index 4bc165f..f7a8c59 100644
    > --- a/drivers/fpga/dfl.h
    > +++ b/drivers/fpga/dfl.h
    > @@ -17,7 +17,9 @@
    > #include <linux/bitfield.h>
    > #include <linux/cdev.h>
    > #include <linux/delay.h>
    > +#include <linux/eventfd.h>
    > #include <linux/fs.h>
    > +#include <linux/interrupt.h>
    > #include <linux/iopoll.h>
    > #include <linux/io-64-nonatomic-lo-hi.h>
    > #include <linux/platform_device.h>
    > @@ -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,13 @@ 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);
    > +long dfl_feature_ioctl_get_num_irqs(struct platform_device *pdev,
    > + struct dfl_feature *feature,
    > + unsigned long arg);
    > +long dfl_feature_ioctl_set_irq(struct platform_device *pdev,
    > + struct dfl_feature *feature,
    > + unsigned long arg);
    > +
    > #endif /* __FPGA_DFL_H */
    > diff --git a/include/uapi/linux/fpga-dfl.h b/include/uapi/linux/fpga-dfl.h
    > index ec70a0746..7331350 100644
    > --- a/include/uapi/linux/fpga-dfl.h
    > +++ b/include/uapi/linux/fpga-dfl.h
    > @@ -151,6 +151,19 @@ struct dfl_fpga_port_dma_unmap {
    >
    > #define DFL_FPGA_PORT_DMA_UNMAP _IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 4)
    >
    > +/**
    > + * struct dfl_fpga_irq_set - the argument for DFL_FPGA_XXX_SET_IRQ ioctl.
    > + *
    > + * @start: Index of the first irq.
    > + * @count: The number of eventfd handler.
    > + * @evtfds: Eventfd handlers.
    > + */
    > +struct dfl_fpga_irq_set {
    > + __u32 start;
    > + __u32 count;
    > + __s32 evtfds[];
    > +};
    > +
    > /* IOCTLs for FME file descriptor */
    >
    > /**
    > --
    > 2.7.4
    >
    Thanks,
    Moritz

    \
     
     \ /
      Last update: 2020-05-12 06:18    [W:4.282 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site