| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200310103024.GC28396@hao-dev>
Date: Tue, 10 Mar 2020 18:30:24 +0800
From: Wu Hao <hao.wu@...el.com>
To: Xu Yilun <yilun.xu@...el.com>
Cc: mdf@...nel.org, linux-fpga@...r.kernel.org,
linux-kernel@...r.kernel.org, Luwei Kang <luwei.kang@...el.com>
Subject: Re: [PATCH 3/7] fpga: dfl: introduce interrupt trigger setting API
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@...el.com>
> Signed-off-by: Wu Hao <hao.wu@...el.com>
> Signed-off-by: Xu Yilun <yilun.xu@...el.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
Powered by blists - more mailing lists