2020-03-16 04:20:45

by Xu Yilun

[permalink] [raw]
Subject: [PATCH v2 0/7] Add interrupt support to FPGA DFL drivers

This patchset add interrupt support to FPGA DFL drivers.

With these patches, DFL driver will parse and assign interrupt resources
for enumerated feature devices and their sub features.

This patchset also introduces a set of APIs for user to monitor DFL
interrupts. Three sub features (DFL FME error, DFL AFU error and user
interrupt) drivers now support these APIs.

Patch #1: DFL framework change. Accept interrupt info input from DFL bus
driver, and add interrupt parsing and assignment for feature
sub devices.
Patch #2: DFL pci driver change, add interrupt info on DFL enumeration.
Patch #3: DFL framework change. Add helper functions for feature sub
device drivers to handle interrupt and notify users.
Patch #4: Add interrupt support for AFU error reporting sub feature.
Patch #5: Add interrupt support for FME global error reporting sub
feature.
Patch #6: Add interrupt support for a new sub feature, to handle user
interrupts implemented in AFU.
Patch #7: Documentation for DFL interrupt handling.

Main changes from v1:
- Early validating irq table for each feature in parse_feature_irq()
in Patch #1.
- Changes IOCTL interfaces. use DFL_FPGA_FME/PORT_XXX_GET_IRQ_NUM
instead of DFL_FPGA_FME/PORT_XXX_GET_INFO, delete flag field for
DFL_FPGA_FME/PORT_XXX_SET_IRQ param

Xu Yilun (7):
fpga: dfl: parse interrupt info for feature devices on enumeration
fpga: dfl: pci: add irq info for feature devices enumeration
fpga: dfl: introduce interrupt trigger setting API
fpga: dfl: afu: add interrupt support for error reporting
fpga: dfl: fme: add interrupt support for global error reporting
fpga: dfl: afu: add user interrupt support
Documentation: fpga: dfl: add descriptions for interrupt related
interfaces.

Documentation/fpga/dfl.rst | 17 +++
drivers/fpga/dfl-afu-error.c | 64 +++++++++++
drivers/fpga/dfl-afu-main.c | 78 +++++++++++++
drivers/fpga/dfl-fme-error.c | 66 +++++++++++
drivers/fpga/dfl-fme-main.c | 6 +
drivers/fpga/dfl-pci.c | 78 +++++++++++--
drivers/fpga/dfl.c | 247 +++++++++++++++++++++++++++++++++++++++++-
drivers/fpga/dfl.h | 51 +++++++++
include/uapi/linux/fpga-dfl.h | 75 +++++++++++++
9 files changed, 669 insertions(+), 13 deletions(-)

--
2.7.4


2020-03-16 04:21:00

by Xu Yilun

[permalink] [raw]
Subject: [PATCH v2 4/7] fpga: dfl: afu: add interrupt support for error reporting

Error reporting interrupt is very useful to notify users that some
errors are detected by the hardware. Once users are notified, they
could query hardware logged error states, no need to continuously
poll on these states.

This patch follows the common DFL interrupt notification and handling
mechanism, implements two ioctl commands below for user to query
number of irqs supported, and set/unset interrupt triggers.

Ioctls:
* DFL_FPGA_PORT_ERR_GET_IRQ_NUM
get the number of irqs, which is used to determine whether/how many
interrupts error reporting feature supports.

* DFL_FPGA_PORT_ERR_SET_IRQ
set/unset given eventfds as error interrupt triggers.

Signed-off-by: Luwei Kang <[email protected]>
Signed-off-by: Wu Hao <[email protected]>
Signed-off-by: Xu Yilun <[email protected]>
----
v2: use DFL_FPGA_PORT_ERR_GET_IRQ_NUM instead of
DFL_FPGA_PORT_ERR_GET_INFO
Delete flag field for DFL_FPGA_PORT_ERR_SET_IRQ param
---
drivers/fpga/dfl-afu-error.c | 64 +++++++++++++++++++++++++++++++++++++++++++
drivers/fpga/dfl-afu-main.c | 4 +++
include/uapi/linux/fpga-dfl.h | 29 ++++++++++++++++++++
3 files changed, 97 insertions(+)

diff --git a/drivers/fpga/dfl-afu-error.c b/drivers/fpga/dfl-afu-error.c
index c1467ae..4d478b2f 100644
--- a/drivers/fpga/dfl-afu-error.c
+++ b/drivers/fpga/dfl-afu-error.c
@@ -15,6 +15,7 @@
*/

#include <linux/uaccess.h>
+#include <linux/fpga-dfl.h>

#include "dfl-afu.h"

@@ -219,6 +220,68 @@ static void port_err_uinit(struct platform_device *pdev,
afu_port_err_mask(&pdev->dev, true);
}

+static long
+port_err_get_num_irqs(struct platform_device *pdev,
+ struct dfl_feature *feature, unsigned long arg)
+{
+ if (copy_to_user((void __user *)arg, &feature->nr_irqs,
+ sizeof(feature->nr_irqs)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static long port_err_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;
+}
+
+static long
+port_err_ioctl(struct platform_device *pdev, struct dfl_feature *feature,
+ unsigned int cmd, unsigned long arg)
+{
+ long ret = -ENODEV;
+
+ switch (cmd) {
+ case DFL_FPGA_PORT_ERR_GET_IRQ_NUM:
+ ret = port_err_get_num_irqs(pdev, feature, arg);
+ break;
+ case DFL_FPGA_PORT_ERR_SET_IRQ:
+ ret = port_err_set_irq(pdev, feature, arg);
+ break;
+ default:
+ dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
+ }
+
+ return ret;
+}
+
const struct dfl_feature_id port_err_id_table[] = {
{.id = PORT_FEATURE_ID_ERROR,},
{0,}
@@ -227,4 +290,5 @@ const struct dfl_feature_id port_err_id_table[] = {
const struct dfl_feature_ops port_err_ops = {
.init = port_err_init,
.uinit = port_err_uinit,
+ .ioctl = port_err_ioctl,
};
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 435bde4..fc8b9cf 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -577,6 +577,7 @@ static int afu_release(struct inode *inode, struct file *filp)
{
struct platform_device *pdev = filp->private_data;
struct dfl_feature_platform_data *pdata;
+ struct dfl_feature *feature;

dev_dbg(&pdev->dev, "Device File Release\n");

@@ -586,6 +587,9 @@ static int afu_release(struct inode *inode, struct file *filp)
dfl_feature_dev_use_end(pdata);

if (!dfl_feature_dev_use_count(pdata)) {
+ dfl_fpga_dev_for_each_feature(pdata, feature)
+ dfl_fpga_set_irq_triggers(feature, 0,
+ feature->nr_irqs, NULL);
__port_reset(pdev);
afu_dma_region_destroy(pdata);
}
diff --git a/include/uapi/linux/fpga-dfl.h b/include/uapi/linux/fpga-dfl.h
index ec70a0746..ced859d 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -151,6 +151,35 @@ struct dfl_fpga_port_dma_unmap {

#define DFL_FPGA_PORT_DMA_UNMAP _IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 4)

+/**
+ * DFL_FPGA_PORT_ERR_GET_IRQ_NUM - _IOR(DFL_FPGA_MAGIC, DFL_PORT_BASE + 5,
+ * __u32 num_irqs)
+ *
+ * Get the number of irqs supported by the fpga port error reporting private
+ * feature.
+ * Return: 0 on success, -errno on failure.
+ */
+#define DFL_FPGA_PORT_ERR_GET_IRQ_NUM _IOR(DFL_FPGA_MAGIC, \
+ DFL_PORT_BASE + 5, __u32)
+
+/**
+ * DFL_FPGA_PORT_ERR_SET_IRQ - _IOW(DFL_FPGA_MAGIC, DFL_PORT_BASE + 6,
+ * struct dfl_fpga_irq_set)
+ *
+ * Set fpga port error reporting interrupt trigger if evtfds[n] is valid.
+ * Unset related interrupt trigger if evtfds[n] is a NULL or negative value.
+ * Return: 0 on success, -errno on failure.
+ */
+struct dfl_fpga_irq_set {
+ __u32 start; /* First irq number */
+ __u32 count; /* The number of eventfd handler */
+ __s32 evtfds[]; /* Eventfd handler */
+};
+
+#define DFL_FPGA_PORT_ERR_SET_IRQ _IOW(DFL_FPGA_MAGIC, \
+ DFL_PORT_BASE + 6, \
+ struct dfl_fpga_irq_set)
+
/* IOCTLs for FME file descriptor */

/**
--
2.7.4

2020-03-16 04:21:23

by Xu Yilun

[permalink] [raw]
Subject: [PATCH v2 6/7] fpga: dfl: afu: add user interrupt support

AFU (Accelerated Function Unit) is dynamic region of the DFL based FPGA,
and always defined by users. Some DFL based FPGA cards allow users to
implement their own interrupts in AFU. In order to support this,
hardware implements a new UINT (User Interrupt) private feature with
related capability register which describes the number of supported
user interrupts as well as the local index of the interrupts for
software enumeration, and from software side, driver follows the common
DFL interrupt notification and handling mechanism, and it implements
two ioctls below for user to query number of irqs supported and set/unset
interrupt triggers.

Ioctls:
* DFL_FPGA_PORT_UINT_GET_IRQ_NUM
get the number of irqs, which is used to determine how many interrupts
UINT feature supports.

* DFL_FPGA_PORT_UINT_SET_IRQ
set/unset eventfds as AFU user interrupt triggers.

Signed-off-by: Luwei Kang <[email protected]>
Signed-off-by: Wu Hao <[email protected]>
Signed-off-by: Xu Yilun <[email protected]>
----
v2: use DFL_FPGA_PORT_UINT_GET_IRQ_NUM instead of
DFL_FPGA_PORT_UINT_GET_INFO
Delete flags field for DFL_FPGA_PORT_UINT_SET_IRQ
---
drivers/fpga/dfl-afu-main.c | 74 +++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/fpga-dfl.h | 23 ++++++++++++++
2 files changed, 97 insertions(+)

diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index fc8b9cf..784817c 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -529,6 +529,76 @@ static const struct dfl_feature_ops port_stp_ops = {
.init = port_stp_init,
};

+static long
+port_uint_get_num_irqs(struct platform_device *pdev,
+ struct dfl_feature *feature, unsigned long arg)
+{
+ if (copy_to_user((void __user *)arg, &feature->nr_irqs,
+ sizeof(feature->nr_irqs)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static long port_uint_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;
+}
+
+static long
+port_uint_ioctl(struct platform_device *pdev, struct dfl_feature *feature,
+ unsigned int cmd, unsigned long arg)
+{
+ long ret = -ENODEV;
+
+ switch (cmd) {
+ case DFL_FPGA_PORT_UINT_GET_IRQ_NUM:
+ ret = port_uint_get_num_irqs(pdev, feature, arg);
+ break;
+ case DFL_FPGA_PORT_UINT_SET_IRQ:
+ ret = port_uint_set_irq(pdev, feature, arg);
+ break;
+ default:
+ dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
+ }
+ return ret;
+}
+
+static const struct dfl_feature_id port_uint_id_table[] = {
+ {.id = PORT_FEATURE_ID_UINT,},
+ {0,}
+};
+
+static const struct dfl_feature_ops port_uint_ops = {
+ .ioctl = port_uint_ioctl,
+};
+
static struct dfl_feature_driver port_feature_drvs[] = {
{
.id_table = port_hdr_id_table,
@@ -547,6 +617,10 @@ static struct dfl_feature_driver port_feature_drvs[] = {
.ops = &port_stp_ops,
},
{
+ .id_table = port_uint_id_table,
+ .ops = &port_uint_ops,
+ },
+ {
.ops = NULL,
}
};
diff --git a/include/uapi/linux/fpga-dfl.h b/include/uapi/linux/fpga-dfl.h
index dc8d370..ca2dc6c 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -180,6 +180,29 @@ struct dfl_fpga_irq_set {
DFL_PORT_BASE + 6, \
struct dfl_fpga_irq_set)

+/**
+ * DFL_FPGA_PORT_UINT_GET_IRQ_NUM - _IOR(DFL_FPGA_MAGIC, DFL_PORT_BASE + 7,
+ * __u32 num_irqs)
+ *
+ * Get the number of irqs supported by the fpga AFU user interrupt private
+ * feature.
+ * Return: 0 on success, -errno on failure.
+ */
+#define DFL_FPGA_PORT_UINT_GET_IRQ_NUM _IOR(DFL_FPGA_MAGIC, \
+ DFL_PORT_BASE + 7, __u32)
+
+/**
+ * DFL_FPGA_PORT_UINT_SET_IRQ - _IOW(DFL_FPGA_MAGIC, DFL_PORT_BASE + 8,
+ * struct dfl_fpga_irq_set)
+ *
+ * Set fpga afu user interrupt trigger if evtfds[n] is valid.
+ * Unset related interrupt trigger if evtfds[n] is a NULL or negative value.
+ * Return: 0 on success, -errno on failure.
+ */
+#define DFL_FPGA_PORT_UINT_SET_IRQ _IOW(DFL_FPGA_MAGIC, \
+ DFL_PORT_BASE + 8, \
+ struct dfl_fpga_irq_set)
+
/* IOCTLs for FME file descriptor */

/**
--
2.7.4

2020-03-16 04:22:04

by Xu Yilun

[permalink] [raw]
Subject: [PATCH v2 7/7] Documentation: fpga: dfl: add descriptions for interrupt related interfaces.

This patch adds introductions of interrupt related interfaces for FME
error reporting, port error reporting and AFU user interrupts features.

Signed-off-by: Luwei Kang <[email protected]>
Signed-off-by: Wu Hao <[email protected]>
Signed-off-by: Xu Yilun <[email protected]>
----
v2: Update Documents cause change of irq ioctl interfaces.
---
Documentation/fpga/dfl.rst | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index 094fc8a..c7ed3e4 100644
--- a/Documentation/fpga/dfl.rst
+++ b/Documentation/fpga/dfl.rst
@@ -89,6 +89,8 @@ The following functions are exposed through ioctls:
- Program bitstream (DFL_FPGA_FME_PORT_PR)
- Assign port to PF (DFL_FPGA_FME_PORT_ASSIGN)
- Release port from PF (DFL_FPGA_FME_PORT_RELEASE)
+- Get number of irqs of FME global error (DFL_FPGA_FME_ERR_GET_IRQ_NUM)
+- Set interrupt trigger for FME error (DFL_FPGA_FME_ERR_SET_IRQ)

More functions are exposed through sysfs
(/sys/class/fpga_region/regionX/dfl-fme.n/):
@@ -144,6 +146,10 @@ The following functions are exposed through ioctls:
- Map DMA buffer (DFL_FPGA_PORT_DMA_MAP)
- Unmap DMA buffer (DFL_FPGA_PORT_DMA_UNMAP)
- Reset AFU (DFL_FPGA_PORT_RESET)
+- Get number of irqs of port error (DFL_FPGA_PORT_ERR_GET_IRQ_NUM)
+- Set interrupt trigger for port error (DFL_FPGA_PORT_ERR_SET_IRQ)
+- Get number of irqs of UINT (DFL_FPGA_PORT_UINT_GET_IRQ_NUM)
+- Set interrupt trigger for UINT (DFL_FPGA_PORT_UINT_SET_IRQ)

DFL_FPGA_PORT_RESET:
reset the FPGA Port and its AFU. Userspace can do Port
@@ -378,6 +384,17 @@ The device nodes used for ioctl() or mmap() can be referenced through::
/sys/class/fpga_region/<regionX>/<dfl-port.n>/dev


+Interrupt support
+=================
+Some FME and AFU private features are able to generate interrupts. As mentioned
+above, users could call ioctl (DFL_FPGA_*_GET_IRQ_NUM) to know whether or how
+many interrupts are supported for this private feature. Drivers also implement
+an eventfd based interrupt handling mechanism for users to get notified when
+interrupt happens. Users could set eventfds to driver via
+ioctl (DFL_FPGA_*_SET_IRQ), and then poll/select on these eventfds waiting for
+notification.
+
+
Add new FIUs support
====================
It's possible that developers made some new function blocks (FIUs) under this
--
2.7.4

2020-03-18 09:17:49

by Wu Hao

[permalink] [raw]
Subject: Re: [PATCH v2 4/7] fpga: dfl: afu: add interrupt support for error reporting

On Mon, Mar 16, 2020 at 12:16:59PM +0800, Xu Yilun wrote:
> Error reporting interrupt is very useful to notify users that some
> errors are detected by the hardware. Once users are notified, they
> could query hardware logged error states, no need to continuously
> poll on these states.
>
> This patch follows the common DFL interrupt notification and handling
> mechanism, implements two ioctl commands below for user to query
> number of irqs supported, and set/unset interrupt triggers.
>
> Ioctls:
> * DFL_FPGA_PORT_ERR_GET_IRQ_NUM
> get the number of irqs, which is used to determine whether/how many
> interrupts error reporting feature supports.
>
> * DFL_FPGA_PORT_ERR_SET_IRQ
> set/unset given eventfds as error interrupt triggers.
>
> Signed-off-by: Luwei Kang <[email protected]>
> Signed-off-by: Wu Hao <[email protected]>
> Signed-off-by: Xu Yilun <[email protected]>
> ----
> v2: use DFL_FPGA_PORT_ERR_GET_IRQ_NUM instead of
> DFL_FPGA_PORT_ERR_GET_INFO
> Delete flag field for DFL_FPGA_PORT_ERR_SET_IRQ param
> ---
> drivers/fpga/dfl-afu-error.c | 64 +++++++++++++++++++++++++++++++++++++++++++
> drivers/fpga/dfl-afu-main.c | 4 +++
> include/uapi/linux/fpga-dfl.h | 29 ++++++++++++++++++++
> 3 files changed, 97 insertions(+)
>
> diff --git a/drivers/fpga/dfl-afu-error.c b/drivers/fpga/dfl-afu-error.c
> index c1467ae..4d478b2f 100644
> --- a/drivers/fpga/dfl-afu-error.c
> +++ b/drivers/fpga/dfl-afu-error.c
> @@ -15,6 +15,7 @@
> */
>
> #include <linux/uaccess.h>
> +#include <linux/fpga-dfl.h>
>
> #include "dfl-afu.h"
>
> @@ -219,6 +220,68 @@ static void port_err_uinit(struct platform_device *pdev,
> afu_port_err_mask(&pdev->dev, true);
> }
>
> +static long
> +port_err_get_num_irqs(struct platform_device *pdev,
> + struct dfl_feature *feature, unsigned long arg)
> +{
> + if (copy_to_user((void __user *)arg, &feature->nr_irqs,
> + sizeof(feature->nr_irqs)))
> + return -EFAULT;

maybe use put_user is simpler here.

> +
> + return 0;
> +}
> +
> +static long port_err_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;
> +}
> +
> +static long
> +port_err_ioctl(struct platform_device *pdev, struct dfl_feature *feature,
> + unsigned int cmd, unsigned long arg)
> +{
> + long ret = -ENODEV;
> +
> + switch (cmd) {
> + case DFL_FPGA_PORT_ERR_GET_IRQ_NUM:
> + ret = port_err_get_num_irqs(pdev, feature, arg);
> + break;
> + case DFL_FPGA_PORT_ERR_SET_IRQ:
> + ret = port_err_set_irq(pdev, feature, arg);
> + break;
> + default:
> + dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
> + }
> +
> + return ret;
> +}
> +
> const struct dfl_feature_id port_err_id_table[] = {
> {.id = PORT_FEATURE_ID_ERROR,},
> {0,}
> @@ -227,4 +290,5 @@ const struct dfl_feature_id port_err_id_table[] = {
> const struct dfl_feature_ops port_err_ops = {
> .init = port_err_init,
> .uinit = port_err_uinit,
> + .ioctl = port_err_ioctl,
> };
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index 435bde4..fc8b9cf 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -577,6 +577,7 @@ static int afu_release(struct inode *inode, struct file *filp)
> {
> struct platform_device *pdev = filp->private_data;
> struct dfl_feature_platform_data *pdata;
> + struct dfl_feature *feature;
>
> dev_dbg(&pdev->dev, "Device File Release\n");
>
> @@ -586,6 +587,9 @@ static int afu_release(struct inode *inode, struct file *filp)
> dfl_feature_dev_use_end(pdata);
>
> if (!dfl_feature_dev_use_count(pdata)) {
> + dfl_fpga_dev_for_each_feature(pdata, feature)
> + dfl_fpga_set_irq_triggers(feature, 0,
> + feature->nr_irqs, NULL);
> __port_reset(pdev);
> afu_dma_region_destroy(pdata);
> }
> diff --git a/include/uapi/linux/fpga-dfl.h b/include/uapi/linux/fpga-dfl.h
> index ec70a0746..ced859d 100644
> --- a/include/uapi/linux/fpga-dfl.h
> +++ b/include/uapi/linux/fpga-dfl.h
> @@ -151,6 +151,35 @@ struct dfl_fpga_port_dma_unmap {
>
> #define DFL_FPGA_PORT_DMA_UNMAP _IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 4)
>
> +/**
> + * DFL_FPGA_PORT_ERR_GET_IRQ_NUM - _IOR(DFL_FPGA_MAGIC, DFL_PORT_BASE + 5,
> + * __u32 num_irqs)
> + *
> + * Get the number of irqs supported by the fpga port error reporting private
> + * feature.
> + * Return: 0 on success, -errno on failure.
> + */
> +#define DFL_FPGA_PORT_ERR_GET_IRQ_NUM _IOR(DFL_FPGA_MAGIC, \
> + DFL_PORT_BASE + 5, __u32)
> +
> +/**
> + * DFL_FPGA_PORT_ERR_SET_IRQ - _IOW(DFL_FPGA_MAGIC, DFL_PORT_BASE + 6,
> + * struct dfl_fpga_irq_set)
> + *
> + * Set fpga port error reporting interrupt trigger if evtfds[n] is valid.
> + * Unset related interrupt trigger if evtfds[n] is a NULL or negative value.

Looks like the code only unset trigger if fd < 0, right?

> + * Return: 0 on success, -errno on failure.
> + */
> +struct dfl_fpga_irq_set {
> + __u32 start; /* First irq number */

Sounds a little confusing, what about index of the first irq?


Thanks
Hao

> + __u32 count; /* The number of eventfd handler */
> + __s32 evtfds[]; /* Eventfd handler */
> +};
> +
> +#define DFL_FPGA_PORT_ERR_SET_IRQ _IOW(DFL_FPGA_MAGIC, \
> + DFL_PORT_BASE + 6, \
> + struct dfl_fpga_irq_set)
> +
> /* IOCTLs for FME file descriptor */
>
> /**
> --
> 2.7.4

2020-03-19 14:29:02

by Xu Yilun

[permalink] [raw]
Subject: Re: [PATCH v2 4/7] fpga: dfl: afu: add interrupt support for error reporting

On Wed, Mar 18, 2020 at 04:54:26PM +0800, Wu Hao wrote:
> On Mon, Mar 16, 2020 at 12:16:59PM +0800, Xu Yilun wrote:
> > Error reporting interrupt is very useful to notify users that some
> > errors are detected by the hardware. Once users are notified, they
> > could query hardware logged error states, no need to continuously
> > poll on these states.
> >
> > This patch follows the common DFL interrupt notification and handling
> > mechanism, implements two ioctl commands below for user to query
> > number of irqs supported, and set/unset interrupt triggers.
> >
> > Ioctls:
> > * DFL_FPGA_PORT_ERR_GET_IRQ_NUM
> > get the number of irqs, which is used to determine whether/how many
> > interrupts error reporting feature supports.
> >
> > * DFL_FPGA_PORT_ERR_SET_IRQ
> > set/unset given eventfds as error interrupt triggers.
> >
> > Signed-off-by: Luwei Kang <[email protected]>
> > Signed-off-by: Wu Hao <[email protected]>
> > Signed-off-by: Xu Yilun <[email protected]>
> > ----
> > v2: use DFL_FPGA_PORT_ERR_GET_IRQ_NUM instead of
> > DFL_FPGA_PORT_ERR_GET_INFO
> > Delete flag field for DFL_FPGA_PORT_ERR_SET_IRQ param
> > ---
> > drivers/fpga/dfl-afu-error.c | 64 +++++++++++++++++++++++++++++++++++++++++++
> > drivers/fpga/dfl-afu-main.c | 4 +++
> > include/uapi/linux/fpga-dfl.h | 29 ++++++++++++++++++++
> > 3 files changed, 97 insertions(+)
> >
> > diff --git a/drivers/fpga/dfl-afu-error.c b/drivers/fpga/dfl-afu-error.c
> > index c1467ae..4d478b2f 100644
> > --- a/drivers/fpga/dfl-afu-error.c
> > +++ b/drivers/fpga/dfl-afu-error.c
> > @@ -15,6 +15,7 @@
> > */
> >
> > #include <linux/uaccess.h>
> > +#include <linux/fpga-dfl.h>
> >
> > #include "dfl-afu.h"
> >
> > @@ -219,6 +220,68 @@ static void port_err_uinit(struct platform_device *pdev,
> > afu_port_err_mask(&pdev->dev, true);
> > }
> >
> > +static long
> > +port_err_get_num_irqs(struct platform_device *pdev,
> > + struct dfl_feature *feature, unsigned long arg)
> > +{
> > + if (copy_to_user((void __user *)arg, &feature->nr_irqs,
> > + sizeof(feature->nr_irqs)))
> > + return -EFAULT;
>
> maybe use put_user is simpler here.

I'll change it.

>
> > +
> > + return 0;
> > +}
> > +
> > +static long port_err_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;
> > +}
> > +
> > +static long
> > +port_err_ioctl(struct platform_device *pdev, struct dfl_feature *feature,
> > + unsigned int cmd, unsigned long arg)
> > +{
> > + long ret = -ENODEV;
> > +
> > + switch (cmd) {
> > + case DFL_FPGA_PORT_ERR_GET_IRQ_NUM:
> > + ret = port_err_get_num_irqs(pdev, feature, arg);
> > + break;
> > + case DFL_FPGA_PORT_ERR_SET_IRQ:
> > + ret = port_err_set_irq(pdev, feature, arg);
> > + break;
> > + default:
> > + dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > const struct dfl_feature_id port_err_id_table[] = {
> > {.id = PORT_FEATURE_ID_ERROR,},
> > {0,}
> > @@ -227,4 +290,5 @@ const struct dfl_feature_id port_err_id_table[] = {
> > const struct dfl_feature_ops port_err_ops = {
> > .init = port_err_init,
> > .uinit = port_err_uinit,
> > + .ioctl = port_err_ioctl,
> > };
> > diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> > index 435bde4..fc8b9cf 100644
> > --- a/drivers/fpga/dfl-afu-main.c
> > +++ b/drivers/fpga/dfl-afu-main.c
> > @@ -577,6 +577,7 @@ static int afu_release(struct inode *inode, struct file *filp)
> > {
> > struct platform_device *pdev = filp->private_data;
> > struct dfl_feature_platform_data *pdata;
> > + struct dfl_feature *feature;
> >
> > dev_dbg(&pdev->dev, "Device File Release\n");
> >
> > @@ -586,6 +587,9 @@ static int afu_release(struct inode *inode, struct file *filp)
> > dfl_feature_dev_use_end(pdata);
> >
> > if (!dfl_feature_dev_use_count(pdata)) {
> > + dfl_fpga_dev_for_each_feature(pdata, feature)
> > + dfl_fpga_set_irq_triggers(feature, 0,
> > + feature->nr_irqs, NULL);
> > __port_reset(pdev);
> > afu_dma_region_destroy(pdata);
> > }
> > diff --git a/include/uapi/linux/fpga-dfl.h b/include/uapi/linux/fpga-dfl.h
> > index ec70a0746..ced859d 100644
> > --- a/include/uapi/linux/fpga-dfl.h
> > +++ b/include/uapi/linux/fpga-dfl.h
> > @@ -151,6 +151,35 @@ struct dfl_fpga_port_dma_unmap {
> >
> > #define DFL_FPGA_PORT_DMA_UNMAP _IO(DFL_FPGA_MAGIC, DFL_PORT_BASE + 4)
> >
> > +/**
> > + * DFL_FPGA_PORT_ERR_GET_IRQ_NUM - _IOR(DFL_FPGA_MAGIC, DFL_PORT_BASE + 5,
> > + * __u32 num_irqs)
> > + *
> > + * Get the number of irqs supported by the fpga port error reporting private
> > + * feature.
> > + * Return: 0 on success, -errno on failure.
> > + */
> > +#define DFL_FPGA_PORT_ERR_GET_IRQ_NUM _IOR(DFL_FPGA_MAGIC, \
> > + DFL_PORT_BASE + 5, __u32)
> > +
> > +/**
> > + * DFL_FPGA_PORT_ERR_SET_IRQ - _IOW(DFL_FPGA_MAGIC, DFL_PORT_BASE + 6,
> > + * struct dfl_fpga_irq_set)
> > + *
> > + * Set fpga port error reporting interrupt trigger if evtfds[n] is valid.
> > + * Unset related interrupt trigger if evtfds[n] is a NULL or negative value.
>
> Looks like the code only unset trigger if fd < 0, right?

Yes, will fix it.

>
> > + * Return: 0 on success, -errno on failure.
> > + */
> > +struct dfl_fpga_irq_set {
> > + __u32 start; /* First irq number */
>
> Sounds a little confusing, what about index of the first irq?

I'll take this change.

Thanks.

>
>
> Thanks
> Hao
>
> > + __u32 count; /* The number of eventfd handler */
> > + __s32 evtfds[]; /* Eventfd handler */
> > +};
> > +
> > +#define DFL_FPGA_PORT_ERR_SET_IRQ _IOW(DFL_FPGA_MAGIC, \
> > + DFL_PORT_BASE + 6, \
> > + struct dfl_fpga_irq_set)
> > +
> > /* IOCTLs for FME file descriptor */
> >
> > /**
> > --
> > 2.7.4