Re: [PATCH v7 3/7] fpga: dfl: introduce interrupt trigger setting API

2020-06-28 Thread Moritz Fischer
On Tue, Jun 16, 2020 at 12:08:44PM +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 
> Signed-off-by: Wu Hao 
> Signed-off-by: Xu Yilun 
> Signed-off-by: Tom Rix 
> Reviewed-by: Marcelo Tosatti 
> Acked-by: Wu Hao 
> ---
> 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
> v6: Remove unnecessary type casting
> v7: Split the check and wrap the overflow check with the unlikely macro
> for dfl_fpga_set_irq_triggers()
> remove the redunant check in do_set_irq_trigger()
> ---
>  drivers/fpga/dfl.c| 157 
> ++
>  drivers/fpga/dfl.h|  16 +
>  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 02c1ec4..b51db80 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -10,7 +10,9 @@
>   *   Wu Hao 
>   *   Xiao Guangrong 
>   */
> +#include 
>  #include 
> +#include 
>  
>  #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;
> @@ -1394,6 +1397,160 @@ 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;
> +
> + 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(>dev), 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 

[PATCH v7 3/7] fpga: dfl: introduce interrupt trigger setting API

2020-06-15 Thread Xu Yilun
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 
Signed-off-by: Wu Hao 
Signed-off-by: Xu Yilun 
Signed-off-by: Tom Rix 
Reviewed-by: Marcelo Tosatti 
Acked-by: Wu Hao 
---
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
v6: Remove unnecessary type casting
v7: Split the check and wrap the overflow check with the unlikely macro
for dfl_fpga_set_irq_triggers()
remove the redunant check in do_set_irq_trigger()
---
 drivers/fpga/dfl.c| 157 ++
 drivers/fpga/dfl.h|  16 +
 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 02c1ec4..b51db80 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -10,7 +10,9 @@
  *   Wu Hao 
  *   Xiao Guangrong 
  */
+#include 
 #include 
+#include 
 
 #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;
@@ -1394,6 +1397,160 @@ 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;
+
+   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(>dev), 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;
+
+   /* overflow */
+   if (unlikely(start + count < start))
+   return -EINVAL;
+
+   /* exceeds