[PATCH v5 0/7] Add interrupt support to FPGA DFL drivers

[PATCH v5 0/7] Add interrupt support to FPGA DFL drivers

[Xu Yilun]

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

Main changes from v2:
 - put parse_feature_irqs() inside create_feature_instance().
 - refines code for dfl_fpga_set_irq_triggers, delete local variable j.
 - put_user() instead of copy_to_user() for DFL_FPGA_XXX_GET_IRQ_NUM IOCTL

Main changes from v3:
 - rebased to 5.7-rc1.
 - fail the dfl enumeration when irq parsing error happens.
 - Add 2 helper functions in dfl.c to handle generic irq ioctls in feature
   drivers.

Main changes from v4:
 - Minor fixes for Hao's comments.

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 port error reporting
  fpga: dfl: fme: add interrupt support for global error reporting
  fpga: dfl: afu: add AFU interrupt support
  Documentation: fpga: dfl: add descriptions for interrupt related
    interfaces.

 Documentation/fpga/dfl.rst    |  19 +++
 drivers/fpga/dfl-afu-error.c  |  17 +++
 drivers/fpga/dfl-afu-main.c   |  32 +++++
 drivers/fpga/dfl-fme-error.c  |  18 +++
 drivers/fpga/dfl-fme-main.c   |   6 +
 drivers/fpga/dfl-pci.c        |  80 +++++++++--
 drivers/fpga/dfl.c            | 310 ++++++++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl.h            |  57 ++++++++
 include/uapi/linux/fpga-dfl.h |  82 +++++++++++
 9 files changed, 612 insertions(+), 9 deletions(-)

-- 
2.7.4

[PATCH v5 1/7] fpga: dfl: parse interrupt info for feature devices on enumeration

[Xu Yilun]

DFL based FPGA devices could support interrupts for different purposes,
but current DFL framework only supports feature device enumeration with
given MMIO resources information via common DFL headers. This patch
introduces one new API dfl_fpga_enum_info_add_irq for low level bus
drivers (e.g. PCIe device driver) to pass its interrupt resources
information to DFL framework for enumeration, and also adds interrupt
enumeration code in framework to parse and assign interrupt resources
for enumerated feature devices and their own sub features.

With this patch, DFL framework enumerates interrupt resources for core
features, including PORT Error Reporting, FME (FPGA Management Engine)
Error Reporting and also AFU User 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>
Acked-by: Wu Hao <hao.wu@intel.com>
----
v2: early validating irq table for each feature in parse_feature_irq().
    Some code improvement and minor fix for Hao's comments.
v3: put parse_feature_irqs() inside create_feature_instance()
    some minor fixes and more comments
v4: no need to include asm/irq.h.
    fail the dfl enumeration when irq parsing error happens.
v5: Some minor fix for Hao's comments
---
 drivers/fpga/dfl.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl.h |  40 ++++++++++++++
 2 files changed, 194 insertions(+)

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 9909948..b49fbed 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -421,6 +421,9 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister);
  *
  * @dev: device to enumerate.
  * @cdev: the container device for all feature devices.
+ * @nr_irqs: number of irqs for all feature devices.
+ * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of
+ *	       this device.
  * @feature_dev: current feature device.
  * @ioaddr: header register region address of feature device in enumeration.
  * @sub_features: a sub features linked list for feature device in enumeration.
@@ -429,6 +432,9 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister);
 struct build_feature_devs_info {
 	struct device *dev;
 	struct dfl_fpga_cdev *cdev;
+	unsigned int nr_irqs;
+	int *irq_table;
+
 	struct platform_device *feature_dev;
 	void __iomem *ioaddr;
 	struct list_head sub_features;
@@ -442,12 +448,16 @@ struct build_feature_devs_info {
  * @mmio_res: mmio resource of this sub feature.
  * @ioaddr: mapped base address of mmio resource.
  * @node: node in sub_features linked list.
+ * @irq_base: start of irq index in this sub feature.
+ * @nr_irqs: number of irqs of this sub feature.
  */
 struct dfl_feature_info {
 	u64 fid;
 	struct resource mmio_res;
 	void __iomem *ioaddr;
 	struct list_head node;
+	unsigned int irq_base;
+	unsigned int nr_irqs;
 };
 
 static void dfl_fpga_cdev_add_port_dev(struct dfl_fpga_cdev *cdev,
@@ -520,6 +530,8 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
 	/* fill features and resource information for feature dev */
 	list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
 		struct dfl_feature *feature = &pdata->features[index];
+		struct dfl_feature_irq_ctx *ctx;
+		unsigned int i;
 
 		/* save resource information for each feature */
 		feature->id = finfo->fid;
@@ -527,6 +539,20 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
 		feature->ioaddr = finfo->ioaddr;
 		fdev->resource[index++] = finfo->mmio_res;
 
+		if (finfo->nr_irqs) {
+			ctx = devm_kcalloc(binfo->dev, finfo->nr_irqs,
+					   sizeof(*ctx), GFP_KERNEL);
+			if (!ctx)
+				return -ENOMEM;
+
+			for (i = 0; i < finfo->nr_irqs; i++)
+				ctx[i].irq =
+					binfo->irq_table[finfo->irq_base + i];
+
+			feature->irq_ctx = ctx;
+			feature->nr_irqs = finfo->nr_irqs;
+		}
+
 		list_del(&finfo->node);
 		kfree(finfo);
 	}
@@ -638,6 +664,79 @@ static u64 feature_id(void __iomem *start)
 	return 0;
 }
 
+static int parse_feature_irqs(struct build_feature_devs_info *binfo,
+			      resource_size_t ofst, u64 fid,
+			      unsigned int *irq_base, unsigned int *nr_irqs)
+{
+	void __iomem *base = binfo->ioaddr + ofst;
+	unsigned int i, ibase, inr = 0;
+	int virq;
+	u64 v;
+
+	/*
+	 * Ideally DFL framework should only read info from DFL header, but
+	 * current version DFL only provides mmio resources information for
+	 * each feature in DFL Header, no field for interrupt resources.
+	 * Interrupt resource information is provided by specific mmio
+	 * registers of each private feature which supports interrupt. So in
+	 * order to parse and assign irq resources, DFL framework has to look
+	 * into specific capability registers of these private features.
+	 *
+	 * Once future DFL version supports generic interrupt resource
+	 * information in common DFL headers, the generic interrupt parsing
+	 * code will be added. But in order to be compatible to old version
+	 * DFL, driver may still fall back to these quirks.
+	 */
+	switch (fid) {
+	case PORT_FEATURE_ID_UINT:
+		v = readq(base + PORT_UINT_CAP);
+		ibase = FIELD_GET(PORT_UINT_CAP_FST_VECT, v);
+		inr = FIELD_GET(PORT_UINT_CAP_INT_NUM, v);
+		break;
+	case PORT_FEATURE_ID_ERROR:
+		v = readq(base + PORT_ERROR_CAP);
+		ibase = FIELD_GET(PORT_ERROR_CAP_INT_VECT, v);
+		inr = FIELD_GET(PORT_ERROR_CAP_SUPP_INT, v);
+		break;
+	case FME_FEATURE_ID_GLOBAL_ERR:
+		v = readq(base + FME_ERROR_CAP);
+		ibase = FIELD_GET(FME_ERROR_CAP_INT_VECT, v);
+		inr = FIELD_GET(FME_ERROR_CAP_SUPP_INT, v);
+		break;
+	}
+
+	if (!inr) {
+		*irq_base = 0;
+		*nr_irqs = 0;
+		return 0;
+	}
+
+	dev_dbg(binfo->dev, "feature: 0x%llx, irq_base: %u, nr_irqs: %u\n",
+		(unsigned long long)fid, ibase, inr);
+
+	if (ibase + inr > binfo->nr_irqs) {
+		dev_err(binfo->dev,
+			"Invalid interrupt number in feature 0x%llx\n",
+			(unsigned long long)fid);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < inr; i++) {
+		virq = binfo->irq_table[ibase + i];
+		if (virq < 0 || virq > NR_IRQS) {
+			dev_err(binfo->dev,
+				"Invalid irq table entry for feature 0x%llx\n",
+				(unsigned long long)fid);
+			return -EINVAL;
+		}
+	}
+
+	*irq_base = (unsigned int)ibase;
+	*nr_irqs = (unsigned int)inr;
+
+	return 0;
+}
+
 /*
  * when create sub feature instances, for private features, it doesn't need
  * to provide resource size and feature id as they could be read from DFH
@@ -650,7 +749,9 @@ create_feature_instance(struct build_feature_devs_info *binfo,
 			struct dfl_fpga_enum_dfl *dfl, resource_size_t ofst,
 			resource_size_t size, u64 fid)
 {
+	unsigned int irq_base, nr_irqs;
 	struct dfl_feature_info *finfo;
+	int ret;
 
 	/* read feature size and id if inputs are invalid */
 	size = size ? size : feature_size(dfl->ioaddr + ofst);
@@ -659,6 +760,10 @@ create_feature_instance(struct build_feature_devs_info *binfo,
 	if (dfl->len - ofst < size)
 		return -EINVAL;
 
+	ret = parse_feature_irqs(binfo, ofst, fid, &irq_base, &nr_irqs);
+	if (ret)
+		return ret;
+
 	finfo = kzalloc(sizeof(*finfo), GFP_KERNEL);
 	if (!finfo)
 		return -ENOMEM;
@@ -667,6 +772,8 @@ create_feature_instance(struct build_feature_devs_info *binfo,
 	finfo->mmio_res.start = dfl->start + ofst;
 	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
 	finfo->mmio_res.flags = IORESOURCE_MEM;
+	finfo->irq_base = irq_base;
+	finfo->nr_irqs = nr_irqs;
 	finfo->ioaddr = dfl->ioaddr + ofst;
 
 	list_add_tail(&finfo->node, &binfo->sub_features);
@@ -853,6 +960,10 @@ void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info)
 		devm_kfree(dev, dfl);
 	}
 
+	/* remove irq table */
+	if (info->irq_table)
+		devm_kfree(dev, info->irq_table);
+
 	devm_kfree(dev, info);
 	put_device(dev);
 }
@@ -892,6 +1003,45 @@ int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
 }
 EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_add_dfl);
 
+/**
+ * dfl_fpga_enum_info_add_irq - add irq table to enum info
+ *
+ * @info: ptr to dfl_fpga_enum_info
+ * @nr_irqs: number of irqs of the DFL fpga device to be enumerated.
+ * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of
+ *	       this device.
+ *
+ * One FPGA device may have several interrupts. This function adds irq
+ * information of the DFL fpga device to enum info for next step enumeration.
+ * This function should be called before dfl_fpga_feature_devs_enumerate().
+ * As we only support one irq domain for all DFLs in the same enum info, adding
+ * irq table a second time for the same enum info will return error.
+ *
+ * If we need to enumerate DFLs which belong to different irq domains, we
+ * should fill more enum info and enumerate them one by one.
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+int dfl_fpga_enum_info_add_irq(struct dfl_fpga_enum_info *info,
+			       unsigned int nr_irqs, int *irq_table)
+{
+	if (!nr_irqs || !irq_table)
+		return -EINVAL;
+
+	if (info->irq_table)
+		return -EEXIST;
+
+	info->irq_table = devm_kmemdup(info->dev, irq_table,
+				       sizeof(int) * nr_irqs, GFP_KERNEL);
+	if (!info->irq_table)
+		return -ENOMEM;
+
+	info->nr_irqs = nr_irqs;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_add_irq);
+
 static int remove_feature_dev(struct device *dev, void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -959,6 +1109,10 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
 	binfo->dev = info->dev;
 	binfo->cdev = cdev;
 
+	binfo->nr_irqs = info->nr_irqs;
+	if (info->nr_irqs)
+		binfo->irq_table = info->irq_table;
+
 	/*
 	 * start enumeration for all feature devices based on Device Feature
 	 * Lists.
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 74784d3..4bc165f 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -112,6 +112,13 @@
 #define FME_PORT_OFST_ACC_VF	1
 #define FME_PORT_OFST_IMP	BIT_ULL(60)
 
+/* FME Error Capability Register */
+#define FME_ERROR_CAP		0x70
+
+/* FME Error Capability Register Bitfield */
+#define FME_ERROR_CAP_SUPP_INT	BIT_ULL(0)		/* Interrupt Support */
+#define FME_ERROR_CAP_INT_VECT	GENMASK_ULL(12, 1)	/* Interrupt vector */
+
 /* PORT Header Register Set */
 #define PORT_HDR_DFH		DFH
 #define PORT_HDR_GUID_L		GUID_L
@@ -145,6 +152,20 @@
 #define PORT_STS_PWR_STATE_AP2	2			/* 90% throttling */
 #define PORT_STS_PWR_STATE_AP6	6			/* 100% throttling */
 
+/* Port Error Capability Register */
+#define PORT_ERROR_CAP		0x38
+
+/* Port Error Capability Register Bitfield */
+#define PORT_ERROR_CAP_SUPP_INT	BIT_ULL(0)		/* Interrupt Support */
+#define PORT_ERROR_CAP_INT_VECT	GENMASK_ULL(12, 1)	/* Interrupt vector */
+
+/* Port Uint Capability Register */
+#define PORT_UINT_CAP		0x8
+
+/* Port Uint Capability Register Bitfield */
+#define PORT_UINT_CAP_INT_NUM	GENMASK_ULL(11, 0)	/* Interrupts num */
+#define PORT_UINT_CAP_FST_VECT	GENMASK_ULL(23, 12)	/* First Vector */
+
 /**
  * struct dfl_fpga_port_ops - port ops
  *
@@ -189,6 +210,15 @@ struct dfl_feature_driver {
 };
 
 /**
+ * struct dfl_feature_irq_ctx - dfl private feature interrupt context
+ *
+ * @irq: Linux IRQ number of this interrupt.
+ */
+struct dfl_feature_irq_ctx {
+	int irq;
+};
+
+/**
  * struct dfl_feature - sub feature of the feature devices
  *
  * @id: sub feature id.
@@ -196,12 +226,16 @@ struct dfl_feature_driver {
  *		    this index is used to find its mmio resource from the
  *		    feature dev (platform device)'s reources.
  * @ioaddr: mapped mmio resource address.
+ * @irq_ctx: interrupt context list.
+ * @nr_irqs: number of interrupt contexts.
  * @ops: ops of this sub feature.
  */
 struct dfl_feature {
 	u64 id;
 	int resource_index;
 	void __iomem *ioaddr;
+	struct dfl_feature_irq_ctx *irq_ctx;
+	unsigned int nr_irqs;
 	const struct dfl_feature_ops *ops;
 };
 
@@ -388,10 +422,14 @@ static inline u8 dfl_feature_revision(void __iomem *base)
  *
  * @dev: parent device.
  * @dfls: list of device feature lists.
+ * @nr_irqs: number of irqs for all feature devices.
+ * @irq_table: Linux IRQ numbers for all irqs, indexed by hw irq numbers.
  */
 struct dfl_fpga_enum_info {
 	struct device *dev;
 	struct list_head dfls;
+	unsigned int nr_irqs;
+	int *irq_table;
 };
 
 /**
@@ -415,6 +453,8 @@ struct dfl_fpga_enum_info *dfl_fpga_enum_info_alloc(struct device *dev);
 int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
 			       resource_size_t start, resource_size_t len,
 			       void __iomem *ioaddr);
+int dfl_fpga_enum_info_add_irq(struct dfl_fpga_enum_info *info,
+			       unsigned int nr_irqs, int *irq_table);
 void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info);
 
 /**
-- 
2.7.4

[PATCH v5 2/7] fpga: dfl: pci: add irq info for feature devices enumeration

[Xu Yilun]

Some DFL FPGA PCIe cards (e.g. Intel FPGA Programmable Acceleration
Card) support MSI-X based interrupts. This patch allows PCIe driver
to prepare and pass interrupt resources to DFL via enumeration API.
These interrupt resources could then be assigned to actual features
which use them.

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: put irq resources init code inside cce_enumerate_feature_dev()
    Some minor changes for Hao's comments.
v3: Some minor fix for Hao's comments for v2.
v4: Some minor fix for Hao's comments for v3.
v5: No change.
---
 drivers/fpga/dfl-pci.c | 80 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 9 deletions(-)

diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
index a78c409..2ff1274 100644
--- a/drivers/fpga/dfl-pci.c
+++ b/drivers/fpga/dfl-pci.c
@@ -39,6 +39,27 @@ static void __iomem *cci_pci_ioremap_bar(struct pci_dev *pcidev, int bar)
 	return pcim_iomap_table(pcidev)[bar];
 }
 
+static int cci_pci_alloc_irq(struct pci_dev *pcidev)
+{
+	int ret, nvec = pci_msix_vec_count(pcidev);
+
+	if (nvec <= 0) {
+		dev_dbg(&pcidev->dev, "fpga interrupt not supported\n");
+		return 0;
+	}
+
+	ret = pci_alloc_irq_vectors(pcidev, nvec, nvec, PCI_IRQ_MSIX);
+	if (ret < 0)
+		return ret;
+
+	return nvec;
+}
+
+static void cci_pci_free_irq(struct pci_dev *pcidev)
+{
+	pci_free_irq_vectors(pcidev);
+}
+
 /* PCI Device ID */
 #define PCIE_DEVICE_ID_PF_INT_5_X	0xBCBD
 #define PCIE_DEVICE_ID_PF_INT_6_X	0xBCC0
@@ -78,17 +99,38 @@ static void cci_remove_feature_devs(struct pci_dev *pcidev)
 
 	/* remove all children feature devices */
 	dfl_fpga_feature_devs_remove(drvdata->cdev);
+	cci_pci_free_irq(pcidev);
+}
+
+static int *cci_pci_create_irq_table(struct pci_dev *pcidev, unsigned int nvec)
+{
+	unsigned int i;
+	int *table;
+
+	table = kcalloc(nvec, sizeof(int), GFP_KERNEL);
+	if (table) {
+		for (i = 0; i < nvec; i++)
+			table[i] = pci_irq_vector(pcidev, i);
+	}
+
+	return table;
+}
+
+static void cci_pci_free_irq_table(int *table)
+{
+	kfree(table);
 }
 
 /* enumerate feature devices under pci device */
 static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
 {
 	struct cci_drvdata *drvdata = pci_get_drvdata(pcidev);
+	int port_num, bar, i, nvec, ret = 0;
 	struct dfl_fpga_enum_info *info;
 	struct dfl_fpga_cdev *cdev;
 	resource_size_t start, len;
-	int port_num, bar, i, ret = 0;
 	void __iomem *base;
+	int *irq_table;
 	u32 offset;
 	u64 v;
 
@@ -97,11 +139,30 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
 	if (!info)
 		return -ENOMEM;
 
+	/* add irq info for enumeration if the device support irq */
+	nvec = cci_pci_alloc_irq(pcidev);
+	if (nvec < 0) {
+		dev_err(&pcidev->dev, "Fail to alloc irq %d.\n", nvec);
+		ret = nvec;
+		goto enum_info_free_exit;
+	} else if (nvec) {
+		irq_table = cci_pci_create_irq_table(pcidev, nvec);
+		if (!irq_table) {
+			ret = -ENOMEM;
+			goto irq_free_exit;
+		}
+
+		ret = dfl_fpga_enum_info_add_irq(info, nvec, irq_table);
+		cci_pci_free_irq_table(irq_table);
+		if (ret)
+			goto irq_free_exit;
+	}
+
 	/* start to find Device Feature List from Bar 0 */
 	base = cci_pci_ioremap_bar(pcidev, 0);
 	if (!base) {
 		ret = -ENOMEM;
-		goto enum_info_free_exit;
+		goto irq_free_exit;
 	}
 
 	/*
@@ -154,7 +215,7 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
 		dfl_fpga_enum_info_add_dfl(info, start, len, base);
 	} else {
 		ret = -ENODEV;
-		goto enum_info_free_exit;
+		goto irq_free_exit;
 	}
 
 	/* start enumeration with prepared enumeration information */
@@ -162,11 +223,14 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
 	if (IS_ERR(cdev)) {
 		dev_err(&pcidev->dev, "Enumeration failure\n");
 		ret = PTR_ERR(cdev);
-		goto enum_info_free_exit;
+		goto irq_free_exit;
 	}
 
 	drvdata->cdev = cdev;
 
+irq_free_exit:
+	if (ret)
+		cci_pci_free_irq(pcidev);
 enum_info_free_exit:
 	dfl_fpga_enum_info_free(info);
 
@@ -211,12 +275,10 @@ int cci_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *pcidevid)
 	}
 
 	ret = cci_enumerate_feature_devs(pcidev);
-	if (ret) {
-		dev_err(&pcidev->dev, "enumeration failure %d.\n", ret);
-		goto disable_error_report_exit;
-	}
+	if (!ret)
+		return ret;
 
-	return ret;
+	dev_err(&pcidev->dev, "enumeration failure %d.\n", ret);
 
 disable_error_report_exit:
 	pci_disable_pcie_error_reporting(pcidev);
-- 
2.7.4

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

[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 <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;
+
+	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

[PATCH v5 4/7] fpga: dfl: afu: add interrupt support for port error reporting

[Xu Yilun]

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 adds interrupt support for port error reporting sub feature.
It 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 <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 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
v3: put_user() instead of copy_to_user()
    improves comments
v4: use common functions to handle irq ioctls
v5: minor fixes for Hao's comments
---
 drivers/fpga/dfl-afu-error.c  | 17 +++++++++++++++++
 drivers/fpga/dfl-afu-main.c   |  4 ++++
 include/uapi/linux/fpga-dfl.h | 23 +++++++++++++++++++++++
 3 files changed, 44 insertions(+)

diff --git a/drivers/fpga/dfl-afu-error.c b/drivers/fpga/dfl-afu-error.c
index c1467ae..c469118 100644
--- a/drivers/fpga/dfl-afu-error.c
+++ b/drivers/fpga/dfl-afu-error.c
@@ -14,6 +14,7 @@
  *   Mitchel Henry <henry.mitchel@intel.com>
  */
 
+#include <linux/fpga-dfl.h>
 #include <linux/uaccess.h>
 
 #include "dfl-afu.h"
@@ -219,6 +220,21 @@ static void port_err_uinit(struct platform_device *pdev,
 	afu_port_err_mask(&pdev->dev, true);
 }
 
+static long
+port_err_ioctl(struct platform_device *pdev, struct dfl_feature *feature,
+	       unsigned int cmd, unsigned long arg)
+{
+	switch (cmd) {
+	case DFL_FPGA_PORT_ERR_GET_IRQ_NUM:
+		return dfl_feature_ioctl_get_num_irqs(pdev, feature, arg);
+	case DFL_FPGA_PORT_ERR_SET_IRQ:
+		return dfl_feature_ioctl_set_irq(pdev, feature, arg);
+	default:
+		dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
+		return -ENODEV;
+	}
+}
+
 const struct dfl_feature_id port_err_id_table[] = {
 	{.id = PORT_FEATURE_ID_ERROR,},
 	{0,}
@@ -227,4 +243,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 3fa2c59..b1ed7b4 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -578,6 +578,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");
 
@@ -587,6 +588,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 7331350..6c71c9d 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -164,6 +164,29 @@ struct dfl_fpga_irq_set {
 	__s32 evtfds[];
 };
 
+/**
+ * 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. Currently hardware supports up to 1 irq.
+ * 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 negative value.
+ * Return: 0 on success, -errno on failure.
+ */
+#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

[PATCH v5 5/7] fpga: dfl: fme: add interrupt support for global error reporting

[Xu Yilun]

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 adds interrupt support for fme global error reporting sub
feature. It 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_FME_ERR_GET_IRQ_NUM
   get the number of irqs, which is used to determine whether/how many
   interrupts fme error reporting feature supports.

 * DFL_FPGA_FME_ERR_SET_IRQ
   set/unset given eventfds as fme error reporting interrupt triggers.

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 DFL_FPGA_FME_ERR_GET_IRQ_NUM instead of
    DFL_FPGA_FME_ERR_GET_INFO
    Delete flags field for DFL_FPGA_FME_ERR_SET_IRQ
v3: put_user() instead of copy_to_user()
    improves comments
v4: use common functions to handle irq ioctls
v5: Minor fixes for Hao's comments
---
 drivers/fpga/dfl-fme-error.c  | 18 ++++++++++++++++++
 drivers/fpga/dfl-fme-main.c   |  6 ++++++
 include/uapi/linux/fpga-dfl.h | 23 +++++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/drivers/fpga/dfl-fme-error.c b/drivers/fpga/dfl-fme-error.c
index f897d41..51c2892 100644
--- a/drivers/fpga/dfl-fme-error.c
+++ b/drivers/fpga/dfl-fme-error.c
@@ -15,6 +15,7 @@
  *   Mitchel, Henry <henry.mitchel@intel.com>
  */
 
+#include <linux/fpga-dfl.h>
 #include <linux/uaccess.h>
 
 #include "dfl.h"
@@ -348,6 +349,22 @@ static void fme_global_err_uinit(struct platform_device *pdev,
 	fme_err_mask(&pdev->dev, true);
 }
 
+static long
+fme_global_error_ioctl(struct platform_device *pdev,
+		       struct dfl_feature *feature,
+		       unsigned int cmd, unsigned long arg)
+{
+	switch (cmd) {
+	case DFL_FPGA_FME_ERR_GET_IRQ_NUM:
+		return dfl_feature_ioctl_get_num_irqs(pdev, feature, arg);
+	case DFL_FPGA_FME_ERR_SET_IRQ:
+		return dfl_feature_ioctl_set_irq(pdev, feature, arg);
+	default:
+		dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
+		return -ENODEV;
+	}
+}
+
 const struct dfl_feature_id fme_global_err_id_table[] = {
 	{.id = FME_FEATURE_ID_GLOBAL_ERR,},
 	{0,}
@@ -356,4 +373,5 @@ const struct dfl_feature_id fme_global_err_id_table[] = {
 const struct dfl_feature_ops fme_global_err_ops = {
 	.init = fme_global_err_init,
 	.uinit = fme_global_err_uinit,
+	.ioctl = fme_global_error_ioctl,
 };
diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
index 56d720c..ab3722d 100644
--- a/drivers/fpga/dfl-fme-main.c
+++ b/drivers/fpga/dfl-fme-main.c
@@ -616,11 +616,17 @@ static int fme_release(struct inode *inode, struct file *filp)
 {
 	struct dfl_feature_platform_data *pdata = filp->private_data;
 	struct platform_device *pdev = pdata->dev;
+	struct dfl_feature *feature;
 
 	dev_dbg(&pdev->dev, "Device File Release\n");
 
 	mutex_lock(&pdata->lock);
 	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);
 	mutex_unlock(&pdata->lock);
 
 	return 0;
diff --git a/include/uapi/linux/fpga-dfl.h b/include/uapi/linux/fpga-dfl.h
index 6c71c9d..b6495ea 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -230,4 +230,27 @@ struct dfl_fpga_fme_port_pr {
  */
 #define DFL_FPGA_FME_PORT_ASSIGN     _IOW(DFL_FPGA_MAGIC, DFL_FME_BASE + 2, int)
 
+/**
+ * DFL_FPGA_FME_ERR_GET_IRQ_NUM - _IOR(DFL_FPGA_MAGIC, DFL_FME_BASE + 3,
+ *							__u32 num_irqs)
+ *
+ * Get the number of irqs supported by the fpga fme error reporting private
+ * feature. Currently hardware supports up to 1 irq.
+ * Return: 0 on success, -errno on failure.
+ */
+#define DFL_FPGA_FME_ERR_GET_IRQ_NUM	_IOR(DFL_FPGA_MAGIC,	\
+					     DFL_FME_BASE + 3, __u32)
+
+/**
+ * DFL_FPGA_FME_ERR_SET_IRQ - _IOW(DFL_FPGA_MAGIC, DFL_FME_BASE + 4,
+ *						struct dfl_fpga_irq_set)
+ *
+ * Set fpga fme error reporting interrupt trigger if evtfds[n] is valid.
+ * Unset related interrupt trigger if evtfds[n] is a negative value.
+ * Return: 0 on success, -errno on failure.
+ */
+#define DFL_FPGA_FME_ERR_SET_IRQ	_IOW(DFL_FPGA_MAGIC,	\
+					     DFL_FME_BASE + 4,	\
+					     struct dfl_fpga_irq_set)
+
 #endif /* _UAPI_LINUX_FPGA_DFL_H */
-- 
2.7.4

[PATCH v5 6/7] fpga: dfl: afu: add AFU interrupt support

[Xu Yilun]

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 (AFU Interrupt) private feature with
related capability register which describes the number of supported
AFU 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 interrupt triggers.

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 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
v3: put_user() instead of copy_to_user()
    improves comments
v4: use common functions to handle irq ioctls
v5: Minor fixes for Hao's comments
---
 drivers/fpga/dfl-afu-main.c   | 28 ++++++++++++++++++++++++++++
 include/uapi/linux/fpga-dfl.h | 23 +++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index b1ed7b4..753cda4 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -530,6 +530,30 @@ static const struct dfl_feature_ops port_stp_ops = {
 	.init = port_stp_init,
 };
 
+static long
+port_uint_ioctl(struct platform_device *pdev, struct dfl_feature *feature,
+		unsigned int cmd, unsigned long arg)
+{
+	switch (cmd) {
+	case DFL_FPGA_PORT_UINT_GET_IRQ_NUM:
+		return dfl_feature_ioctl_get_num_irqs(pdev, feature, arg);
+	case DFL_FPGA_PORT_UINT_SET_IRQ:
+		return dfl_feature_ioctl_set_irq(pdev, feature, arg);
+	default:
+		dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
+		return -ENODEV;
+	}
+}
+
+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,
@@ -548,6 +572,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 b6495ea..1621b07 100644
--- a/include/uapi/linux/fpga-dfl.h
+++ b/include/uapi/linux/fpga-dfl.h
@@ -187,6 +187,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 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 interrupt trigger if evtfds[n] is valid.
+ * Unset related interrupt trigger if evtfds[n] is a 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

[PATCH v5 7/7] Documentation: fpga: dfl: add descriptions for interrupt related interfaces.

[Xu Yilun]

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 <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: Update Documents cause change of irq ioctl interfaces.
v3: No change
v4: Update interrupt support part.
v5: No change
---
 Documentation/fpga/dfl.rst | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index 094fc8a..702bf62 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,19 @@ 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.
+In Current DFL, 3 sub features (Port error, FME global error and AFU interrupt)
+support interrupts.
+
+
 Add new FIUs support
 ====================
 It's possible that developers made some new function blocks (FIUs) under this
-- 
2.7.4

Re: [PATCH v5 0/7] Add interrupt support to FPGA DFL drivers

[Xu Yilun]

Hi Moritz:

Hao and I did several rounds of review and fix in the mailing list. Now
the patches are all acked by Hao.

Could you please help review it when you have time?

Thanks! :)

On Mon, Apr 20, 2020 at 04:11:36PM +0800, Xu Yilun wrote:
> 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
> 
> Main changes from v2:
>  - put parse_feature_irqs() inside create_feature_instance().
>  - refines code for dfl_fpga_set_irq_triggers, delete local variable j.
>  - put_user() instead of copy_to_user() for DFL_FPGA_XXX_GET_IRQ_NUM IOCTL
> 
> Main changes from v3:
>  - rebased to 5.7-rc1.
>  - fail the dfl enumeration when irq parsing error happens.
>  - Add 2 helper functions in dfl.c to handle generic irq ioctls in feature
>    drivers.
> 
> Main changes from v4:
>  - Minor fixes for Hao's comments.
> 
> 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 port error reporting
>   fpga: dfl: fme: add interrupt support for global error reporting
>   fpga: dfl: afu: add AFU interrupt support
>   Documentation: fpga: dfl: add descriptions for interrupt related
>     interfaces.
> 
>  Documentation/fpga/dfl.rst    |  19 +++
>  drivers/fpga/dfl-afu-error.c  |  17 +++
>  drivers/fpga/dfl-afu-main.c   |  32 +++++
>  drivers/fpga/dfl-fme-error.c  |  18 +++
>  drivers/fpga/dfl-fme-main.c   |   6 +
>  drivers/fpga/dfl-pci.c        |  80 +++++++++--
>  drivers/fpga/dfl.c            | 310 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/fpga/dfl.h            |  57 ++++++++
>  include/uapi/linux/fpga-dfl.h |  82 +++++++++++
>  9 files changed, 612 insertions(+), 9 deletions(-)
> 
> -- 
> 2.7.4

Re: [PATCH v5 0/7] Add interrupt support to FPGA DFL drivers

[Moritz Fischer]

Hi Xu,

On Tue, May 5, 2020 at 10:13 PM Xu Yilun <yilun.xu@intel.com> wrote:
>
> Hi Moritz:
>
> Hao and I did several rounds of review and fix in the mailing list. Now
> the patches are all acked by Hao.
>
> Could you please help review it when you have time?

I'll get to it this weekend. Sorry for the delay,

Moritz
>
> Thanks! :)
>
> On Mon, Apr 20, 2020 at 04:11:36PM +0800, Xu Yilun wrote:
> > 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
> >
> > Main changes from v2:
> >  - put parse_feature_irqs() inside create_feature_instance().
> >  - refines code for dfl_fpga_set_irq_triggers, delete local variable j.
> >  - put_user() instead of copy_to_user() for DFL_FPGA_XXX_GET_IRQ_NUM IOCTL
> >
> > Main changes from v3:
> >  - rebased to 5.7-rc1.
> >  - fail the dfl enumeration when irq parsing error happens.
> >  - Add 2 helper functions in dfl.c to handle generic irq ioctls in feature
> >    drivers.
> >
> > Main changes from v4:
> >  - Minor fixes for Hao's comments.
> >
> > 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 port error reporting
> >   fpga: dfl: fme: add interrupt support for global error reporting
> >   fpga: dfl: afu: add AFU interrupt support
> >   Documentation: fpga: dfl: add descriptions for interrupt related
> >     interfaces.
> >
> >  Documentation/fpga/dfl.rst    |  19 +++
> >  drivers/fpga/dfl-afu-error.c  |  17 +++
> >  drivers/fpga/dfl-afu-main.c   |  32 +++++
> >  drivers/fpga/dfl-fme-error.c  |  18 +++
> >  drivers/fpga/dfl-fme-main.c   |   6 +
> >  drivers/fpga/dfl-pci.c        |  80 +++++++++--
> >  drivers/fpga/dfl.c            | 310 ++++++++++++++++++++++++++++++++++++++++++
> >  drivers/fpga/dfl.h            |  57 ++++++++
> >  include/uapi/linux/fpga-dfl.h |  82 +++++++++++
> >  9 files changed, 612 insertions(+), 9 deletions(-)
> >
> > --
> > 2.7.4

Re: [PATCH v5 1/7] fpga: dfl: parse interrupt info for feature devices on enumeration

[Moritz Fischer]

On Mon, Apr 20, 2020 at 04:11:37PM +0800, Xu Yilun wrote:
> DFL based FPGA devices could support interrupts for different purposes,
> but current DFL framework only supports feature device enumeration with
> given MMIO resources information via common DFL headers. This patch
> introduces one new API dfl_fpga_enum_info_add_irq for low level bus
> drivers (e.g. PCIe device driver) to pass its interrupt resources
> information to DFL framework for enumeration, and also adds interrupt
> enumeration code in framework to parse and assign interrupt resources
> for enumerated feature devices and their own sub features.
> 
> With this patch, DFL framework enumerates interrupt resources for core
> features, including PORT Error Reporting, FME (FPGA Management Engine)
> Error Reporting and also AFU User 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>
> Acked-by: Wu Hao <hao.wu@intel.com>
> ----
> v2: early validating irq table for each feature in parse_feature_irq().
>     Some code improvement and minor fix for Hao's comments.
> v3: put parse_feature_irqs() inside create_feature_instance()
>     some minor fixes and more comments
> v4: no need to include asm/irq.h.
>     fail the dfl enumeration when irq parsing error happens.
> v5: Some minor fix for Hao's comments
> ---
>  drivers/fpga/dfl.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/fpga/dfl.h |  40 ++++++++++++++
>  2 files changed, 194 insertions(+)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 9909948..b49fbed 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -421,6 +421,9 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister);
>   *
>   * @dev: device to enumerate.
>   * @cdev: the container device for all feature devices.
> + * @nr_irqs: number of irqs for all feature devices.
> + * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of
> + *	       this device.
>   * @feature_dev: current feature device.
>   * @ioaddr: header register region address of feature device in enumeration.
>   * @sub_features: a sub features linked list for feature device in enumeration.
> @@ -429,6 +432,9 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister);
>  struct build_feature_devs_info {
>  	struct device *dev;
>  	struct dfl_fpga_cdev *cdev;
> +	unsigned int nr_irqs;
> +	int *irq_table;
> +
>  	struct platform_device *feature_dev;
>  	void __iomem *ioaddr;
>  	struct list_head sub_features;
> @@ -442,12 +448,16 @@ struct build_feature_devs_info {
>   * @mmio_res: mmio resource of this sub feature.
>   * @ioaddr: mapped base address of mmio resource.
>   * @node: node in sub_features linked list.
> + * @irq_base: start of irq index in this sub feature.
> + * @nr_irqs: number of irqs of this sub feature.
>   */
>  struct dfl_feature_info {
>  	u64 fid;
>  	struct resource mmio_res;
>  	void __iomem *ioaddr;
>  	struct list_head node;
> +	unsigned int irq_base;
> +	unsigned int nr_irqs;
>  };
>  
>  static void dfl_fpga_cdev_add_port_dev(struct dfl_fpga_cdev *cdev,
> @@ -520,6 +530,8 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
>  	/* fill features and resource information for feature dev */
>  	list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
>  		struct dfl_feature *feature = &pdata->features[index];
> +		struct dfl_feature_irq_ctx *ctx;
> +		unsigned int i;
>  
>  		/* save resource information for each feature */
>  		feature->id = finfo->fid;
> @@ -527,6 +539,20 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
>  		feature->ioaddr = finfo->ioaddr;
>  		fdev->resource[index++] = finfo->mmio_res;
>  
> +		if (finfo->nr_irqs) {
> +			ctx = devm_kcalloc(binfo->dev, finfo->nr_irqs,
> +					   sizeof(*ctx), GFP_KERNEL);
> +			if (!ctx)
> +				return -ENOMEM;
> +
> +			for (i = 0; i < finfo->nr_irqs; i++)
> +				ctx[i].irq =
> +					binfo->irq_table[finfo->irq_base + i];
> +
> +			feature->irq_ctx = ctx;
> +			feature->nr_irqs = finfo->nr_irqs;
> +		}
> +
>  		list_del(&finfo->node);
>  		kfree(finfo);
>  	}
> @@ -638,6 +664,79 @@ static u64 feature_id(void __iomem *start)
>  	return 0;
>  }
>  
> +static int parse_feature_irqs(struct build_feature_devs_info *binfo,
> +			      resource_size_t ofst, u64 fid,
> +			      unsigned int *irq_base, unsigned int *nr_irqs)
> +{
> +	void __iomem *base = binfo->ioaddr + ofst;
> +	unsigned int i, ibase, inr = 0;
> +	int virq;
> +	u64 v;
> +
> +	/*
> +	 * Ideally DFL framework should only read info from DFL header, but
> +	 * current version DFL only provides mmio resources information for
> +	 * each feature in DFL Header, no field for interrupt resources.
> +	 * Interrupt resource information is provided by specific mmio
> +	 * registers of each private feature which supports interrupt. So in
> +	 * order to parse and assign irq resources, DFL framework has to look
> +	 * into specific capability registers of these private features.
> +	 *
> +	 * Once future DFL version supports generic interrupt resource
> +	 * information in common DFL headers, the generic interrupt parsing
> +	 * code will be added. But in order to be compatible to old version
> +	 * DFL, driver may still fall back to these quirks.
Nit: *the* driver may still fall ack ...
> +	 */
> +	switch (fid) {
> +	case PORT_FEATURE_ID_UINT:
> +		v = readq(base + PORT_UINT_CAP);
> +		ibase = FIELD_GET(PORT_UINT_CAP_FST_VECT, v);
> +		inr = FIELD_GET(PORT_UINT_CAP_INT_NUM, v);
> +		break;
> +	case PORT_FEATURE_ID_ERROR:
> +		v = readq(base + PORT_ERROR_CAP);
> +		ibase = FIELD_GET(PORT_ERROR_CAP_INT_VECT, v);
> +		inr = FIELD_GET(PORT_ERROR_CAP_SUPP_INT, v);
> +		break;
> +	case FME_FEATURE_ID_GLOBAL_ERR:
> +		v = readq(base + FME_ERROR_CAP);
> +		ibase = FIELD_GET(FME_ERROR_CAP_INT_VECT, v);
> +		inr = FIELD_GET(FME_ERROR_CAP_SUPP_INT, v);
> +		break;
> +	}
> +
> +	if (!inr) {
> +		*irq_base = 0;
> +		*nr_irqs = 0;
> +		return 0;
> +	}
> +
> +	dev_dbg(binfo->dev, "feature: 0x%llx, irq_base: %u, nr_irqs: %u\n",
> +		(unsigned long long)fid, ibase, inr);
Is that cast required?
> +
> +	if (ibase + inr > binfo->nr_irqs) {
> +		dev_err(binfo->dev,
> +			"Invalid interrupt number in feature 0x%llx\n",
> +			(unsigned long long)fid);
Same here?
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < inr; i++) {
> +		virq = binfo->irq_table[ibase + i];
> +		if (virq < 0 || virq > NR_IRQS) {
> +			dev_err(binfo->dev,
> +				"Invalid irq table entry for feature 0x%llx\n",
> +				(unsigned long long)fid);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	*irq_base = (unsigned int)ibase;
> +	*nr_irqs = (unsigned int)inr;
ibase and nr_iqs are already unsigend int?
> +
> +	return 0;
> +}
> +
>  /*
>   * when create sub feature instances, for private features, it doesn't need
>   * to provide resource size and feature id as they could be read from DFH
> @@ -650,7 +749,9 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  			struct dfl_fpga_enum_dfl *dfl, resource_size_t ofst,
>  			resource_size_t size, u64 fid)
>  {
> +	unsigned int irq_base, nr_irqs;
>  	struct dfl_feature_info *finfo;
> +	int ret;
>  
>  	/* read feature size and id if inputs are invalid */
>  	size = size ? size : feature_size(dfl->ioaddr + ofst);
> @@ -659,6 +760,10 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  	if (dfl->len - ofst < size)
>  		return -EINVAL;
>  
> +	ret = parse_feature_irqs(binfo, ofst, fid, &irq_base, &nr_irqs);
> +	if (ret)
> +		return ret;
> +
>  	finfo = kzalloc(sizeof(*finfo), GFP_KERNEL);
>  	if (!finfo)
>  		return -ENOMEM;
> @@ -667,6 +772,8 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  	finfo->mmio_res.start = dfl->start + ofst;
>  	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
>  	finfo->mmio_res.flags = IORESOURCE_MEM;
> +	finfo->irq_base = irq_base;
> +	finfo->nr_irqs = nr_irqs;
>  	finfo->ioaddr = dfl->ioaddr + ofst;
>  
>  	list_add_tail(&finfo->node, &binfo->sub_features);
> @@ -853,6 +960,10 @@ void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info)
>  		devm_kfree(dev, dfl);
>  	}
>  
> +	/* remove irq table */
> +	if (info->irq_table)
> +		devm_kfree(dev, info->irq_table);
> +
>  	devm_kfree(dev, info);
>  	put_device(dev);
>  }
> @@ -892,6 +1003,45 @@ int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
>  }
>  EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_add_dfl);
>  
> +/**
> + * dfl_fpga_enum_info_add_irq - add irq table to enum info
> + *
> + * @info: ptr to dfl_fpga_enum_info
> + * @nr_irqs: number of irqs of the DFL fpga device to be enumerated.
> + * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of
> + *	       this device.
> + *
> + * One FPGA device may have several interrupts. This function adds irq
> + * information of the DFL fpga device to enum info for next step enumeration.
> + * This function should be called before dfl_fpga_feature_devs_enumerate().
> + * As we only support one irq domain for all DFLs in the same enum info, adding
> + * irq table a second time for the same enum info will return error.
> + *
> + * If we need to enumerate DFLs which belong to different irq domains, we
> + * should fill more enum info and enumerate them one by one.
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +int dfl_fpga_enum_info_add_irq(struct dfl_fpga_enum_info *info,
> +			       unsigned int nr_irqs, int *irq_table)
> +{
> +	if (!nr_irqs || !irq_table)
> +		return -EINVAL;
> +
> +	if (info->irq_table)
> +		return -EEXIST;
> +
> +	info->irq_table = devm_kmemdup(info->dev, irq_table,
> +				       sizeof(int) * nr_irqs, GFP_KERNEL);
> +	if (!info->irq_table)
> +		return -ENOMEM;
> +
> +	info->nr_irqs = nr_irqs;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_add_irq);
> +
>  static int remove_feature_dev(struct device *dev, void *data)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
> @@ -959,6 +1109,10 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
>  	binfo->dev = info->dev;
>  	binfo->cdev = cdev;
>  
> +	binfo->nr_irqs = info->nr_irqs;
> +	if (info->nr_irqs)
> +		binfo->irq_table = info->irq_table;
> +
>  	/*
>  	 * start enumeration for all feature devices based on Device Feature
>  	 * Lists.
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 74784d3..4bc165f 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -112,6 +112,13 @@
>  #define FME_PORT_OFST_ACC_VF	1
>  #define FME_PORT_OFST_IMP	BIT_ULL(60)
>  
> +/* FME Error Capability Register */
> +#define FME_ERROR_CAP		0x70
> +
> +/* FME Error Capability Register Bitfield */
> +#define FME_ERROR_CAP_SUPP_INT	BIT_ULL(0)		/* Interrupt Support */
> +#define FME_ERROR_CAP_INT_VECT	GENMASK_ULL(12, 1)	/* Interrupt vector */
> +
>  /* PORT Header Register Set */
>  #define PORT_HDR_DFH		DFH
>  #define PORT_HDR_GUID_L		GUID_L
> @@ -145,6 +152,20 @@
>  #define PORT_STS_PWR_STATE_AP2	2			/* 90% throttling */
>  #define PORT_STS_PWR_STATE_AP6	6			/* 100% throttling */
>  
> +/* Port Error Capability Register */
> +#define PORT_ERROR_CAP		0x38
> +
> +/* Port Error Capability Register Bitfield */
> +#define PORT_ERROR_CAP_SUPP_INT	BIT_ULL(0)		/* Interrupt Support */
> +#define PORT_ERROR_CAP_INT_VECT	GENMASK_ULL(12, 1)	/* Interrupt vector */
> +
> +/* Port Uint Capability Register */
> +#define PORT_UINT_CAP		0x8
> +
> +/* Port Uint Capability Register Bitfield */
> +#define PORT_UINT_CAP_INT_NUM	GENMASK_ULL(11, 0)	/* Interrupts num */
> +#define PORT_UINT_CAP_FST_VECT	GENMASK_ULL(23, 12)	/* First Vector */
> +
>  /**
>   * struct dfl_fpga_port_ops - port ops
>   *
> @@ -189,6 +210,15 @@ struct dfl_feature_driver {
>  };
>  
>  /**
> + * struct dfl_feature_irq_ctx - dfl private feature interrupt context
> + *
> + * @irq: Linux IRQ number of this interrupt.
> + */
> +struct dfl_feature_irq_ctx {
> +	int irq;
> +};
> +
> +/**
>   * struct dfl_feature - sub feature of the feature devices
>   *
>   * @id: sub feature id.
> @@ -196,12 +226,16 @@ struct dfl_feature_driver {
>   *		    this index is used to find its mmio resource from the
>   *		    feature dev (platform device)'s reources.
>   * @ioaddr: mapped mmio resource address.
> + * @irq_ctx: interrupt context list.
> + * @nr_irqs: number of interrupt contexts.
>   * @ops: ops of this sub feature.
>   */
>  struct dfl_feature {
>  	u64 id;
>  	int resource_index;
>  	void __iomem *ioaddr;
> +	struct dfl_feature_irq_ctx *irq_ctx;
> +	unsigned int nr_irqs;
>  	const struct dfl_feature_ops *ops;
>  };
>  
> @@ -388,10 +422,14 @@ static inline u8 dfl_feature_revision(void __iomem *base)
>   *
>   * @dev: parent device.
>   * @dfls: list of device feature lists.
> + * @nr_irqs: number of irqs for all feature devices.
> + * @irq_table: Linux IRQ numbers for all irqs, indexed by hw irq numbers.
>   */
>  struct dfl_fpga_enum_info {
>  	struct device *dev;
>  	struct list_head dfls;
> +	unsigned int nr_irqs;
> +	int *irq_table;
>  };
>  
>  /**
> @@ -415,6 +453,8 @@ struct dfl_fpga_enum_info *dfl_fpga_enum_info_alloc(struct device *dev);
>  int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
>  			       resource_size_t start, resource_size_t len,
>  			       void __iomem *ioaddr);
> +int dfl_fpga_enum_info_add_irq(struct dfl_fpga_enum_info *info,
> +			       unsigned int nr_irqs, int *irq_table);
>  void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info);
>  
>  /**
> -- 
> 2.7.4
> 

Thanks,
Moritz

Re: [PATCH v5 2/7] fpga: dfl: pci: add irq info for feature devices enumeration

[Moritz Fischer]

On Mon, Apr 20, 2020 at 04:11:38PM +0800, Xu Yilun wrote:
> Some DFL FPGA PCIe cards (e.g. Intel FPGA Programmable Acceleration
> Card) support MSI-X based interrupts. This patch allows PCIe driver
> to prepare and pass interrupt resources to DFL via enumeration API.
> These interrupt resources could then be assigned to actual features
> which use them.
> 
> 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: put irq resources init code inside cce_enumerate_feature_dev()
>     Some minor changes for Hao's comments.
> v3: Some minor fix for Hao's comments for v2.
> v4: Some minor fix for Hao's comments for v3.
> v5: No change.
> ---
>  drivers/fpga/dfl-pci.c | 80 ++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 71 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> index a78c409..2ff1274 100644
> --- a/drivers/fpga/dfl-pci.c
> +++ b/drivers/fpga/dfl-pci.c
> @@ -39,6 +39,27 @@ static void __iomem *cci_pci_ioremap_bar(struct pci_dev *pcidev, int bar)
>  	return pcim_iomap_table(pcidev)[bar];
>  }
>  
> +static int cci_pci_alloc_irq(struct pci_dev *pcidev)
> +{
> +	int ret, nvec = pci_msix_vec_count(pcidev);
> +
> +	if (nvec <= 0) {
> +		dev_dbg(&pcidev->dev, "fpga interrupt not supported\n");
> +		return 0;
> +	}
> +
> +	ret = pci_alloc_irq_vectors(pcidev, nvec, nvec, PCI_IRQ_MSIX);
> +	if (ret < 0)
> +		return ret;
> +
> +	return nvec;
> +}
> +
> +static void cci_pci_free_irq(struct pci_dev *pcidev)
> +{
> +	pci_free_irq_vectors(pcidev);
> +}
> +
>  /* PCI Device ID */
>  #define PCIE_DEVICE_ID_PF_INT_5_X	0xBCBD
>  #define PCIE_DEVICE_ID_PF_INT_6_X	0xBCC0
> @@ -78,17 +99,38 @@ static void cci_remove_feature_devs(struct pci_dev *pcidev)
>  
>  	/* remove all children feature devices */
>  	dfl_fpga_feature_devs_remove(drvdata->cdev);
> +	cci_pci_free_irq(pcidev);
> +}
> +
> +static int *cci_pci_create_irq_table(struct pci_dev *pcidev, unsigned int nvec)
> +{
> +	unsigned int i;
> +	int *table;
> +
> +	table = kcalloc(nvec, sizeof(int), GFP_KERNEL);
> +	if (table) {
> +		for (i = 0; i < nvec; i++)
> +			table[i] = pci_irq_vector(pcidev, i);
> +	}

Have you considered:
	if (!table)
		return table;

	for (i = 0; i < nvec; i++)
		table[i] = pci_irq_vector(pcidev, i);

	return table;
> +
> +	return table;
> +}
> +
> +static void cci_pci_free_irq_table(int *table)
> +{
> +	kfree(table);
>  }

Have you considered just using kfree()?
>  
>  /* enumerate feature devices under pci device */
>  static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  {
>  	struct cci_drvdata *drvdata = pci_get_drvdata(pcidev);
> +	int port_num, bar, i, nvec, ret = 0;
>  	struct dfl_fpga_enum_info *info;
>  	struct dfl_fpga_cdev *cdev;
>  	resource_size_t start, len;
> -	int port_num, bar, i, ret = 0;
>  	void __iomem *base;
> +	int *irq_table;
>  	u32 offset;
>  	u64 v;
>  
> @@ -97,11 +139,30 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  	if (!info)
>  		return -ENOMEM;
>  
> +	/* add irq info for enumeration if the device support irq */
> +	nvec = cci_pci_alloc_irq(pcidev);
> +	if (nvec < 0) {
> +		dev_err(&pcidev->dev, "Fail to alloc irq %d.\n", nvec);
> +		ret = nvec;
> +		goto enum_info_free_exit;
> +	} else if (nvec) {
> +		irq_table = cci_pci_create_irq_table(pcidev, nvec);
> +		if (!irq_table) {
> +			ret = -ENOMEM;
> +			goto irq_free_exit;
> +		}
> +
> +		ret = dfl_fpga_enum_info_add_irq(info, nvec, irq_table);
> +		cci_pci_free_irq_table(irq_table);
> +		if (ret)
> +			goto irq_free_exit;
> +	}
> +
>  	/* start to find Device Feature List from Bar 0 */
>  	base = cci_pci_ioremap_bar(pcidev, 0);
>  	if (!base) {
>  		ret = -ENOMEM;
> -		goto enum_info_free_exit;
> +		goto irq_free_exit;
>  	}
>  
>  	/*
> @@ -154,7 +215,7 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  		dfl_fpga_enum_info_add_dfl(info, start, len, base);
>  	} else {
>  		ret = -ENODEV;
> -		goto enum_info_free_exit;
> +		goto irq_free_exit;
>  	}
>  
>  	/* start enumeration with prepared enumeration information */
> @@ -162,11 +223,14 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  	if (IS_ERR(cdev)) {
>  		dev_err(&pcidev->dev, "Enumeration failure\n");
>  		ret = PTR_ERR(cdev);
> -		goto enum_info_free_exit;
> +		goto irq_free_exit;
>  	}
>  
>  	drvdata->cdev = cdev;
>  
> +irq_free_exit:
> +	if (ret)
> +		cci_pci_free_irq(pcidev);
>  enum_info_free_exit:
>  	dfl_fpga_enum_info_free(info);
>  
> @@ -211,12 +275,10 @@ int cci_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *pcidevid)
>  	}
>  
>  	ret = cci_enumerate_feature_devs(pcidev);
> -	if (ret) {
> -		dev_err(&pcidev->dev, "enumeration failure %d.\n", ret);
> -		goto disable_error_report_exit;
> -	}
> +	if (!ret)
> +		return ret;
>  
> -	return ret;
> +	dev_err(&pcidev->dev, "enumeration failure %d.\n", ret);
>  
>  disable_error_report_exit:
>  	pci_disable_pcie_error_reporting(pcidev);
> -- 
> 2.7.4
> 

Thanks,
Moritz

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

[Moritz Fischer]

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

Re: [PATCH v5 1/7] fpga: dfl: parse interrupt info for feature devices on enumeration

[Xu Yilun]

On Mon, May 11, 2020 at 09:09:07PM -0700, Moritz Fischer wrote:
> On Mon, Apr 20, 2020 at 04:11:37PM +0800, Xu Yilun wrote:
> > DFL based FPGA devices could support interrupts for different purposes,
> > but current DFL framework only supports feature device enumeration with
> > given MMIO resources information via common DFL headers. This patch
> > introduces one new API dfl_fpga_enum_info_add_irq for low level bus
> > drivers (e.g. PCIe device driver) to pass its interrupt resources
> > information to DFL framework for enumeration, and also adds interrupt
> > enumeration code in framework to parse and assign interrupt resources
> > for enumerated feature devices and their own sub features.
> > 
> > With this patch, DFL framework enumerates interrupt resources for core
> > features, including PORT Error Reporting, FME (FPGA Management Engine)
> > Error Reporting and also AFU User 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>
> > Acked-by: Wu Hao <hao.wu@intel.com>
> > ----
> > v2: early validating irq table for each feature in parse_feature_irq().
> >     Some code improvement and minor fix for Hao's comments.
> > v3: put parse_feature_irqs() inside create_feature_instance()
> >     some minor fixes and more comments
> > v4: no need to include asm/irq.h.
> >     fail the dfl enumeration when irq parsing error happens.
> > v5: Some minor fix for Hao's comments
> > ---
> >  drivers/fpga/dfl.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/fpga/dfl.h |  40 ++++++++++++++
> >  2 files changed, 194 insertions(+)
> > 
> > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> > index 9909948..b49fbed 100644
> > --- a/drivers/fpga/dfl.c
> > +++ b/drivers/fpga/dfl.c
> > @@ -421,6 +421,9 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister);
> >   *
> >   * @dev: device to enumerate.
> >   * @cdev: the container device for all feature devices.
> > + * @nr_irqs: number of irqs for all feature devices.
> > + * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of
> > + *	       this device.
> >   * @feature_dev: current feature device.
> >   * @ioaddr: header register region address of feature device in enumeration.
> >   * @sub_features: a sub features linked list for feature device in enumeration.
> > @@ -429,6 +432,9 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister);
> >  struct build_feature_devs_info {
> >  	struct device *dev;
> >  	struct dfl_fpga_cdev *cdev;
> > +	unsigned int nr_irqs;
> > +	int *irq_table;
> > +
> >  	struct platform_device *feature_dev;
> >  	void __iomem *ioaddr;
> >  	struct list_head sub_features;
> > @@ -442,12 +448,16 @@ struct build_feature_devs_info {
> >   * @mmio_res: mmio resource of this sub feature.
> >   * @ioaddr: mapped base address of mmio resource.
> >   * @node: node in sub_features linked list.
> > + * @irq_base: start of irq index in this sub feature.
> > + * @nr_irqs: number of irqs of this sub feature.
> >   */
> >  struct dfl_feature_info {
> >  	u64 fid;
> >  	struct resource mmio_res;
> >  	void __iomem *ioaddr;
> >  	struct list_head node;
> > +	unsigned int irq_base;
> > +	unsigned int nr_irqs;
> >  };
> >  
> >  static void dfl_fpga_cdev_add_port_dev(struct dfl_fpga_cdev *cdev,
> > @@ -520,6 +530,8 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
> >  	/* fill features and resource information for feature dev */
> >  	list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
> >  		struct dfl_feature *feature = &pdata->features[index];
> > +		struct dfl_feature_irq_ctx *ctx;
> > +		unsigned int i;
> >  
> >  		/* save resource information for each feature */
> >  		feature->id = finfo->fid;
> > @@ -527,6 +539,20 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
> >  		feature->ioaddr = finfo->ioaddr;
> >  		fdev->resource[index++] = finfo->mmio_res;
> >  
> > +		if (finfo->nr_irqs) {
> > +			ctx = devm_kcalloc(binfo->dev, finfo->nr_irqs,
> > +					   sizeof(*ctx), GFP_KERNEL);
> > +			if (!ctx)
> > +				return -ENOMEM;
> > +
> > +			for (i = 0; i < finfo->nr_irqs; i++)
> > +				ctx[i].irq =
> > +					binfo->irq_table[finfo->irq_base + i];
> > +
> > +			feature->irq_ctx = ctx;
> > +			feature->nr_irqs = finfo->nr_irqs;
> > +		}
> > +
> >  		list_del(&finfo->node);
> >  		kfree(finfo);
> >  	}
> > @@ -638,6 +664,79 @@ static u64 feature_id(void __iomem *start)
> >  	return 0;
> >  }
> >  
> > +static int parse_feature_irqs(struct build_feature_devs_info *binfo,
> > +			      resource_size_t ofst, u64 fid,
> > +			      unsigned int *irq_base, unsigned int *nr_irqs)
> > +{
> > +	void __iomem *base = binfo->ioaddr + ofst;
> > +	unsigned int i, ibase, inr = 0;
> > +	int virq;
> > +	u64 v;
> > +
> > +	/*
> > +	 * Ideally DFL framework should only read info from DFL header, but
> > +	 * current version DFL only provides mmio resources information for
> > +	 * each feature in DFL Header, no field for interrupt resources.
> > +	 * Interrupt resource information is provided by specific mmio
> > +	 * registers of each private feature which supports interrupt. So in
> > +	 * order to parse and assign irq resources, DFL framework has to look
> > +	 * into specific capability registers of these private features.
> > +	 *
> > +	 * Once future DFL version supports generic interrupt resource
> > +	 * information in common DFL headers, the generic interrupt parsing
> > +	 * code will be added. But in order to be compatible to old version
> > +	 * DFL, driver may still fall back to these quirks.
> Nit: *the* driver may still fall ack ...

Thanks for the correction, I'll fix it in next version.

> > +	 */
> > +	switch (fid) {
> > +	case PORT_FEATURE_ID_UINT:
> > +		v = readq(base + PORT_UINT_CAP);
> > +		ibase = FIELD_GET(PORT_UINT_CAP_FST_VECT, v);
> > +		inr = FIELD_GET(PORT_UINT_CAP_INT_NUM, v);
> > +		break;
> > +	case PORT_FEATURE_ID_ERROR:
> > +		v = readq(base + PORT_ERROR_CAP);
> > +		ibase = FIELD_GET(PORT_ERROR_CAP_INT_VECT, v);
> > +		inr = FIELD_GET(PORT_ERROR_CAP_SUPP_INT, v);
> > +		break;
> > +	case FME_FEATURE_ID_GLOBAL_ERR:
> > +		v = readq(base + FME_ERROR_CAP);
> > +		ibase = FIELD_GET(FME_ERROR_CAP_INT_VECT, v);
> > +		inr = FIELD_GET(FME_ERROR_CAP_SUPP_INT, v);
> > +		break;
> > +	}
> > +
> > +	if (!inr) {
> > +		*irq_base = 0;
> > +		*nr_irqs = 0;
> > +		return 0;
> > +	}
> > +
> > +	dev_dbg(binfo->dev, "feature: 0x%llx, irq_base: %u, nr_irqs: %u\n",
> > +		(unsigned long long)fid, ibase, inr);
> Is that cast required?

I read the printk-formats.txt, the u64 to unsigned long long cast is not
required in kernel since long time ago. I'll fix it.

> > +
> > +	if (ibase + inr > binfo->nr_irqs) {
> > +		dev_err(binfo->dev,
> > +			"Invalid interrupt number in feature 0x%llx\n",
> > +			(unsigned long long)fid);
> Same here?

Yes.

> > +		return -EINVAL;
> > +	}
> > +
> > +	for (i = 0; i < inr; i++) {
> > +		virq = binfo->irq_table[ibase + i];
> > +		if (virq < 0 || virq > NR_IRQS) {
> > +			dev_err(binfo->dev,
> > +				"Invalid irq table entry for feature 0x%llx\n",
> > +				(unsigned long long)fid);
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	*irq_base = (unsigned int)ibase;
> > +	*nr_irqs = (unsigned int)inr;
> ibase and nr_iqs are already unsigend int?

Yes, the cast is not needed.

Thanks,
Yilun.

> > +
> > +	return 0;
> > +}
> > +
> >  /*
> >   * when create sub feature instances, for private features, it doesn't need
> >   * to provide resource size and feature id as they could be read from DFH
> > @@ -650,7 +749,9 @@ create_feature_instance(struct build_feature_devs_info *binfo,
> >  			struct dfl_fpga_enum_dfl *dfl, resource_size_t ofst,
> >  			resource_size_t size, u64 fid)
> >  {
> > +	unsigned int irq_base, nr_irqs;
> >  	struct dfl_feature_info *finfo;
> > +	int ret;
> >  
> >  	/* read feature size and id if inputs are invalid */
> >  	size = size ? size : feature_size(dfl->ioaddr + ofst);
> > @@ -659,6 +760,10 @@ create_feature_instance(struct build_feature_devs_info *binfo,
> >  	if (dfl->len - ofst < size)
> >  		return -EINVAL;
> >  
> > +	ret = parse_feature_irqs(binfo, ofst, fid, &irq_base, &nr_irqs);
> > +	if (ret)
> > +		return ret;
> > +
> >  	finfo = kzalloc(sizeof(*finfo), GFP_KERNEL);
> >  	if (!finfo)
> >  		return -ENOMEM;
> > @@ -667,6 +772,8 @@ create_feature_instance(struct build_feature_devs_info *binfo,
> >  	finfo->mmio_res.start = dfl->start + ofst;
> >  	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
> >  	finfo->mmio_res.flags = IORESOURCE_MEM;
> > +	finfo->irq_base = irq_base;
> > +	finfo->nr_irqs = nr_irqs;
> >  	finfo->ioaddr = dfl->ioaddr + ofst;
> >  
> >  	list_add_tail(&finfo->node, &binfo->sub_features);
> > @@ -853,6 +960,10 @@ void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info)
> >  		devm_kfree(dev, dfl);
> >  	}
> >  
> > +	/* remove irq table */
> > +	if (info->irq_table)
> > +		devm_kfree(dev, info->irq_table);
> > +
> >  	devm_kfree(dev, info);
> >  	put_device(dev);
> >  }
> > @@ -892,6 +1003,45 @@ int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
> >  }
> >  EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_add_dfl);
> >  
> > +/**
> > + * dfl_fpga_enum_info_add_irq - add irq table to enum info
> > + *
> > + * @info: ptr to dfl_fpga_enum_info
> > + * @nr_irqs: number of irqs of the DFL fpga device to be enumerated.
> > + * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of
> > + *	       this device.
> > + *
> > + * One FPGA device may have several interrupts. This function adds irq
> > + * information of the DFL fpga device to enum info for next step enumeration.
> > + * This function should be called before dfl_fpga_feature_devs_enumerate().
> > + * As we only support one irq domain for all DFLs in the same enum info, adding
> > + * irq table a second time for the same enum info will return error.
> > + *
> > + * If we need to enumerate DFLs which belong to different irq domains, we
> > + * should fill more enum info and enumerate them one by one.
> > + *
> > + * Return: 0 on success, negative error code otherwise.
> > + */
> > +int dfl_fpga_enum_info_add_irq(struct dfl_fpga_enum_info *info,
> > +			       unsigned int nr_irqs, int *irq_table)
> > +{
> > +	if (!nr_irqs || !irq_table)
> > +		return -EINVAL;
> > +
> > +	if (info->irq_table)
> > +		return -EEXIST;
> > +
> > +	info->irq_table = devm_kmemdup(info->dev, irq_table,
> > +				       sizeof(int) * nr_irqs, GFP_KERNEL);
> > +	if (!info->irq_table)
> > +		return -ENOMEM;
> > +
> > +	info->nr_irqs = nr_irqs;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_add_irq);
> > +
> >  static int remove_feature_dev(struct device *dev, void *data)
> >  {
> >  	struct platform_device *pdev = to_platform_device(dev);
> > @@ -959,6 +1109,10 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
> >  	binfo->dev = info->dev;
> >  	binfo->cdev = cdev;
> >  
> > +	binfo->nr_irqs = info->nr_irqs;
> > +	if (info->nr_irqs)
> > +		binfo->irq_table = info->irq_table;
> > +
> >  	/*
> >  	 * start enumeration for all feature devices based on Device Feature
> >  	 * Lists.
> > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > index 74784d3..4bc165f 100644
> > --- a/drivers/fpga/dfl.h
> > +++ b/drivers/fpga/dfl.h
> > @@ -112,6 +112,13 @@
> >  #define FME_PORT_OFST_ACC_VF	1
> >  #define FME_PORT_OFST_IMP	BIT_ULL(60)
> >  
> > +/* FME Error Capability Register */
> > +#define FME_ERROR_CAP		0x70
> > +
> > +/* FME Error Capability Register Bitfield */
> > +#define FME_ERROR_CAP_SUPP_INT	BIT_ULL(0)		/* Interrupt Support */
> > +#define FME_ERROR_CAP_INT_VECT	GENMASK_ULL(12, 1)	/* Interrupt vector */
> > +
> >  /* PORT Header Register Set */
> >  #define PORT_HDR_DFH		DFH
> >  #define PORT_HDR_GUID_L		GUID_L
> > @@ -145,6 +152,20 @@
> >  #define PORT_STS_PWR_STATE_AP2	2			/* 90% throttling */
> >  #define PORT_STS_PWR_STATE_AP6	6			/* 100% throttling */
> >  
> > +/* Port Error Capability Register */
> > +#define PORT_ERROR_CAP		0x38
> > +
> > +/* Port Error Capability Register Bitfield */
> > +#define PORT_ERROR_CAP_SUPP_INT	BIT_ULL(0)		/* Interrupt Support */
> > +#define PORT_ERROR_CAP_INT_VECT	GENMASK_ULL(12, 1)	/* Interrupt vector */
> > +
> > +/* Port Uint Capability Register */
> > +#define PORT_UINT_CAP		0x8
> > +
> > +/* Port Uint Capability Register Bitfield */
> > +#define PORT_UINT_CAP_INT_NUM	GENMASK_ULL(11, 0)	/* Interrupts num */
> > +#define PORT_UINT_CAP_FST_VECT	GENMASK_ULL(23, 12)	/* First Vector */
> > +
> >  /**
> >   * struct dfl_fpga_port_ops - port ops
> >   *
> > @@ -189,6 +210,15 @@ struct dfl_feature_driver {
> >  };
> >  
> >  /**
> > + * struct dfl_feature_irq_ctx - dfl private feature interrupt context
> > + *
> > + * @irq: Linux IRQ number of this interrupt.
> > + */
> > +struct dfl_feature_irq_ctx {
> > +	int irq;
> > +};
> > +
> > +/**
> >   * struct dfl_feature - sub feature of the feature devices
> >   *
> >   * @id: sub feature id.
> > @@ -196,12 +226,16 @@ struct dfl_feature_driver {
> >   *		    this index is used to find its mmio resource from the
> >   *		    feature dev (platform device)'s reources.
> >   * @ioaddr: mapped mmio resource address.
> > + * @irq_ctx: interrupt context list.
> > + * @nr_irqs: number of interrupt contexts.
> >   * @ops: ops of this sub feature.
> >   */
> >  struct dfl_feature {
> >  	u64 id;
> >  	int resource_index;
> >  	void __iomem *ioaddr;
> > +	struct dfl_feature_irq_ctx *irq_ctx;
> > +	unsigned int nr_irqs;
> >  	const struct dfl_feature_ops *ops;
> >  };
> >  
> > @@ -388,10 +422,14 @@ static inline u8 dfl_feature_revision(void __iomem *base)
> >   *
> >   * @dev: parent device.
> >   * @dfls: list of device feature lists.
> > + * @nr_irqs: number of irqs for all feature devices.
> > + * @irq_table: Linux IRQ numbers for all irqs, indexed by hw irq numbers.
> >   */
> >  struct dfl_fpga_enum_info {
> >  	struct device *dev;
> >  	struct list_head dfls;
> > +	unsigned int nr_irqs;
> > +	int *irq_table;
> >  };
> >  
> >  /**
> > @@ -415,6 +453,8 @@ struct dfl_fpga_enum_info *dfl_fpga_enum_info_alloc(struct device *dev);
> >  int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
> >  			       resource_size_t start, resource_size_t len,
> >  			       void __iomem *ioaddr);
> > +int dfl_fpga_enum_info_add_irq(struct dfl_fpga_enum_info *info,
> > +			       unsigned int nr_irqs, int *irq_table);
> >  void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info);
> >  
> >  /**
> > -- 
> > 2.7.4
> > 
> 
> Thanks,
> Moritz

Re: [PATCH v5 2/7] fpga: dfl: pci: add irq info for feature devices enumeration

[Xu Yilun]

On Mon, May 11, 2020 at 09:13:01PM -0700, Moritz Fischer wrote:
> On Mon, Apr 20, 2020 at 04:11:38PM +0800, Xu Yilun wrote:
> > Some DFL FPGA PCIe cards (e.g. Intel FPGA Programmable Acceleration
> > Card) support MSI-X based interrupts. This patch allows PCIe driver
> > to prepare and pass interrupt resources to DFL via enumeration API.
> > These interrupt resources could then be assigned to actual features
> > which use them.
> > 
> > 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: put irq resources init code inside cce_enumerate_feature_dev()
> >     Some minor changes for Hao's comments.
> > v3: Some minor fix for Hao's comments for v2.
> > v4: Some minor fix for Hao's comments for v3.
> > v5: No change.
> > ---
> >  drivers/fpga/dfl-pci.c | 80 ++++++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 71 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> > index a78c409..2ff1274 100644
> > --- a/drivers/fpga/dfl-pci.c
> > +++ b/drivers/fpga/dfl-pci.c
> > @@ -39,6 +39,27 @@ static void __iomem *cci_pci_ioremap_bar(struct pci_dev *pcidev, int bar)
> >  	return pcim_iomap_table(pcidev)[bar];
> >  }
> >  
> > +static int cci_pci_alloc_irq(struct pci_dev *pcidev)
> > +{
> > +	int ret, nvec = pci_msix_vec_count(pcidev);
> > +
> > +	if (nvec <= 0) {
> > +		dev_dbg(&pcidev->dev, "fpga interrupt not supported\n");
> > +		return 0;
> > +	}
> > +
> > +	ret = pci_alloc_irq_vectors(pcidev, nvec, nvec, PCI_IRQ_MSIX);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return nvec;
> > +}
> > +
> > +static void cci_pci_free_irq(struct pci_dev *pcidev)
> > +{
> > +	pci_free_irq_vectors(pcidev);
> > +}
> > +
> >  /* PCI Device ID */
> >  #define PCIE_DEVICE_ID_PF_INT_5_X	0xBCBD
> >  #define PCIE_DEVICE_ID_PF_INT_6_X	0xBCC0
> > @@ -78,17 +99,38 @@ static void cci_remove_feature_devs(struct pci_dev *pcidev)
> >  
> >  	/* remove all children feature devices */
> >  	dfl_fpga_feature_devs_remove(drvdata->cdev);
> > +	cci_pci_free_irq(pcidev);
> > +}
> > +
> > +static int *cci_pci_create_irq_table(struct pci_dev *pcidev, unsigned int nvec)
> > +{
> > +	unsigned int i;
> > +	int *table;
> > +
> > +	table = kcalloc(nvec, sizeof(int), GFP_KERNEL);
> > +	if (table) {
> > +		for (i = 0; i < nvec; i++)
> > +			table[i] = pci_irq_vector(pcidev, i);
> > +	}
> 
> Have you considered:
> 	if (!table)
> 		return table;
> 
> 	for (i = 0; i < nvec; i++)
> 		table[i] = pci_irq_vector(pcidev, i);
> 
> 	return table;

I'm fine with this. I'll change it.

> > +
> > +	return table;
> > +}
> > +
> > +static void cci_pci_free_irq_table(int *table)
> > +{
> > +	kfree(table);
> >  }
> 
> Have you considered just using kfree()?

I thought there is a cci_pci_create_irq_table(), and a symmetric
xx_free_xx() function would make the code easier to understand.

How about we keep this?

Thanks,
Yilun.

> >  
> >  /* enumerate feature devices under pci device */
> >  static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
> >  {
> >  	struct cci_drvdata *drvdata = pci_get_drvdata(pcidev);
> > +	int port_num, bar, i, nvec, ret = 0;
> >  	struct dfl_fpga_enum_info *info;
> >  	struct dfl_fpga_cdev *cdev;
> >  	resource_size_t start, len;
> > -	int port_num, bar, i, ret = 0;
> >  	void __iomem *base;
> > +	int *irq_table;
> >  	u32 offset;
> >  	u64 v;
> >  
> > @@ -97,11 +139,30 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
> >  	if (!info)
> >  		return -ENOMEM;
> >  
> > +	/* add irq info for enumeration if the device support irq */
> > +	nvec = cci_pci_alloc_irq(pcidev);
> > +	if (nvec < 0) {
> > +		dev_err(&pcidev->dev, "Fail to alloc irq %d.\n", nvec);
> > +		ret = nvec;
> > +		goto enum_info_free_exit;
> > +	} else if (nvec) {
> > +		irq_table = cci_pci_create_irq_table(pcidev, nvec);
> > +		if (!irq_table) {
> > +			ret = -ENOMEM;
> > +			goto irq_free_exit;
> > +		}
> > +
> > +		ret = dfl_fpga_enum_info_add_irq(info, nvec, irq_table);
> > +		cci_pci_free_irq_table(irq_table);
> > +		if (ret)
> > +			goto irq_free_exit;
> > +	}
> > +
> >  	/* start to find Device Feature List from Bar 0 */
> >  	base = cci_pci_ioremap_bar(pcidev, 0);
> >  	if (!base) {
> >  		ret = -ENOMEM;
> > -		goto enum_info_free_exit;
> > +		goto irq_free_exit;
> >  	}
> >  
> >  	/*
> > @@ -154,7 +215,7 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
> >  		dfl_fpga_enum_info_add_dfl(info, start, len, base);
> >  	} else {
> >  		ret = -ENODEV;
> > -		goto enum_info_free_exit;
> > +		goto irq_free_exit;
> >  	}
> >  
> >  	/* start enumeration with prepared enumeration information */
> > @@ -162,11 +223,14 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
> >  	if (IS_ERR(cdev)) {
> >  		dev_err(&pcidev->dev, "Enumeration failure\n");
> >  		ret = PTR_ERR(cdev);
> > -		goto enum_info_free_exit;
> > +		goto irq_free_exit;
> >  	}
> >  
> >  	drvdata->cdev = cdev;
> >  
> > +irq_free_exit:
> > +	if (ret)
> > +		cci_pci_free_irq(pcidev);
> >  enum_info_free_exit:
> >  	dfl_fpga_enum_info_free(info);
> >  
> > @@ -211,12 +275,10 @@ int cci_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *pcidevid)
> >  	}
> >  
> >  	ret = cci_enumerate_feature_devs(pcidev);
> > -	if (ret) {
> > -		dev_err(&pcidev->dev, "enumeration failure %d.\n", ret);
> > -		goto disable_error_report_exit;
> > -	}
> > +	if (!ret)
> > +		return ret;
> >  
> > -	return ret;
> > +	dev_err(&pcidev->dev, "enumeration failure %d.\n", ret);
> >  
> >  disable_error_report_exit:
> >  	pci_disable_pcie_error_reporting(pcidev);
> > -- 
> > 2.7.4
> > 
> 
> Thanks,
> Moritz

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

[Xu Yilun]

On Mon, May 11, 2020 at 09:16:34PM -0700, Moritz Fischer wrote:
> 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 () ?

I think maybe we could keep this. The 2 conditions are all for start &
count check, that's why I prefer to put them in one code block.

Thanks,
Yilun.

> > +
> > +	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

Re: [PATCH v5 0/7] Add interrupt support to FPGA DFL drivers

[Xu Yilun]

Hi Moritz:

Thanks for your comments for first 3 patches. And do you have more comments
on last 4 patches?

I have done the changes for those fixes. I'm not sure if I should send
the v6 patchset now?

Thanks,
Yilun.

On Wed, May 06, 2020 at 06:17:27PM -0700, Moritz Fischer wrote:
> Hi Xu,
> 
> On Tue, May 5, 2020 at 10:13 PM Xu Yilun <yilun.xu@intel.com> wrote:
> >
> > Hi Moritz:
> >
> > Hao and I did several rounds of review and fix in the mailing list. Now
> > the patches are all acked by Hao.
> >
> > Could you please help review it when you have time?
> 
> I'll get to it this weekend. Sorry for the delay,
> 
> Moritz
> >
> > Thanks! :)
> >
> > On Mon, Apr 20, 2020 at 04:11:36PM +0800, Xu Yilun wrote:
> > > 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
> > >
> > > Main changes from v2:
> > >  - put parse_feature_irqs() inside create_feature_instance().
> > >  - refines code for dfl_fpga_set_irq_triggers, delete local variable j.
> > >  - put_user() instead of copy_to_user() for DFL_FPGA_XXX_GET_IRQ_NUM IOCTL
> > >
> > > Main changes from v3:
> > >  - rebased to 5.7-rc1.
> > >  - fail the dfl enumeration when irq parsing error happens.
> > >  - Add 2 helper functions in dfl.c to handle generic irq ioctls in feature
> > >    drivers.
> > >
> > > Main changes from v4:
> > >  - Minor fixes for Hao's comments.
> > >
> > > 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 port error reporting
> > >   fpga: dfl: fme: add interrupt support for global error reporting
> > >   fpga: dfl: afu: add AFU interrupt support
> > >   Documentation: fpga: dfl: add descriptions for interrupt related
> > >     interfaces.
> > >
> > >  Documentation/fpga/dfl.rst    |  19 +++
> > >  drivers/fpga/dfl-afu-error.c  |  17 +++
> > >  drivers/fpga/dfl-afu-main.c   |  32 +++++
> > >  drivers/fpga/dfl-fme-error.c  |  18 +++
> > >  drivers/fpga/dfl-fme-main.c   |   6 +
> > >  drivers/fpga/dfl-pci.c        |  80 +++++++++--
> > >  drivers/fpga/dfl.c            | 310 ++++++++++++++++++++++++++++++++++++++++++
> > >  drivers/fpga/dfl.h            |  57 ++++++++
> > >  include/uapi/linux/fpga-dfl.h |  82 +++++++++++
> > >  9 files changed, 612 insertions(+), 9 deletions(-)
> > >
> > > --
> > > 2.7.4

Re: [PATCH v5 1/7] fpga: dfl: parse interrupt info for feature devices on enumeration

[Marcelo Tosatti]

On Mon, Apr 20, 2020 at 04:11:37PM +0800, Xu Yilun wrote:
> DFL based FPGA devices could support interrupts for different purposes,
> but current DFL framework only supports feature device enumeration with
> given MMIO resources information via common DFL headers. This patch
> introduces one new API dfl_fpga_enum_info_add_irq for low level bus
> drivers (e.g. PCIe device driver) to pass its interrupt resources
> information to DFL framework for enumeration, and also adds interrupt
> enumeration code in framework to parse and assign interrupt resources
> for enumerated feature devices and their own sub features.
> 
> With this patch, DFL framework enumerates interrupt resources for core
> features, including PORT Error Reporting, FME (FPGA Management Engine)
> Error Reporting and also AFU User 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>
> Acked-by: Wu Hao <hao.wu@intel.com>
> ----
> v2: early validating irq table for each feature in parse_feature_irq().
>     Some code improvement and minor fix for Hao's comments.
> v3: put parse_feature_irqs() inside create_feature_instance()
>     some minor fixes and more comments
> v4: no need to include asm/irq.h.
>     fail the dfl enumeration when irq parsing error happens.
> v5: Some minor fix for Hao's comments
> ---
>  drivers/fpga/dfl.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/fpga/dfl.h |  40 ++++++++++++++
>  2 files changed, 194 insertions(+)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 9909948..b49fbed 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -421,6 +421,9 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister);
>   *
>   * @dev: device to enumerate.
>   * @cdev: the container device for all feature devices.
> + * @nr_irqs: number of irqs for all feature devices.
> + * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of
> + *	       this device.
>   * @feature_dev: current feature device.
>   * @ioaddr: header register region address of feature device in enumeration.
>   * @sub_features: a sub features linked list for feature device in enumeration.
> @@ -429,6 +432,9 @@ EXPORT_SYMBOL_GPL(dfl_fpga_dev_ops_unregister);
>  struct build_feature_devs_info {
>  	struct device *dev;
>  	struct dfl_fpga_cdev *cdev;
> +	unsigned int nr_irqs;
> +	int *irq_table;
> +
>  	struct platform_device *feature_dev;
>  	void __iomem *ioaddr;
>  	struct list_head sub_features;
> @@ -442,12 +448,16 @@ struct build_feature_devs_info {
>   * @mmio_res: mmio resource of this sub feature.
>   * @ioaddr: mapped base address of mmio resource.
>   * @node: node in sub_features linked list.
> + * @irq_base: start of irq index in this sub feature.
> + * @nr_irqs: number of irqs of this sub feature.
>   */
>  struct dfl_feature_info {
>  	u64 fid;
>  	struct resource mmio_res;
>  	void __iomem *ioaddr;
>  	struct list_head node;
> +	unsigned int irq_base;
> +	unsigned int nr_irqs;
>  };
>  
>  static void dfl_fpga_cdev_add_port_dev(struct dfl_fpga_cdev *cdev,
> @@ -520,6 +530,8 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
>  	/* fill features and resource information for feature dev */
>  	list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
>  		struct dfl_feature *feature = &pdata->features[index];
> +		struct dfl_feature_irq_ctx *ctx;
> +		unsigned int i;
>  
>  		/* save resource information for each feature */
>  		feature->id = finfo->fid;
> @@ -527,6 +539,20 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
>  		feature->ioaddr = finfo->ioaddr;
>  		fdev->resource[index++] = finfo->mmio_res;
>  
> +		if (finfo->nr_irqs) {
> +			ctx = devm_kcalloc(binfo->dev, finfo->nr_irqs,
> +					   sizeof(*ctx), GFP_KERNEL);
> +			if (!ctx)
> +				return -ENOMEM;
> +
> +			for (i = 0; i < finfo->nr_irqs; i++)
> +				ctx[i].irq =
> +					binfo->irq_table[finfo->irq_base + i];
> +
> +			feature->irq_ctx = ctx;
> +			feature->nr_irqs = finfo->nr_irqs;
> +		}
> +
>  		list_del(&finfo->node);
>  		kfree(finfo);
>  	}
> @@ -638,6 +664,79 @@ static u64 feature_id(void __iomem *start)
>  	return 0;
>  }
>  
> +static int parse_feature_irqs(struct build_feature_devs_info *binfo,
> +			      resource_size_t ofst, u64 fid,
> +			      unsigned int *irq_base, unsigned int *nr_irqs)
> +{
> +	void __iomem *base = binfo->ioaddr + ofst;
> +	unsigned int i, ibase, inr = 0;
> +	int virq;
> +	u64 v;
> +
> +	/*
> +	 * Ideally DFL framework should only read info from DFL header, but
> +	 * current version DFL only provides mmio resources information for
> +	 * each feature in DFL Header, no field for interrupt resources.
> +	 * Interrupt resource information is provided by specific mmio
> +	 * registers of each private feature which supports interrupt. So in
> +	 * order to parse and assign irq resources, DFL framework has to look
> +	 * into specific capability registers of these private features.
> +	 *
> +	 * Once future DFL version supports generic interrupt resource
> +	 * information in common DFL headers, the generic interrupt parsing
> +	 * code will be added. But in order to be compatible to old version
> +	 * DFL, driver may still fall back to these quirks.
> +	 */
> +	switch (fid) {
> +	case PORT_FEATURE_ID_UINT:
> +		v = readq(base + PORT_UINT_CAP);
> +		ibase = FIELD_GET(PORT_UINT_CAP_FST_VECT, v);
> +		inr = FIELD_GET(PORT_UINT_CAP_INT_NUM, v);
> +		break;
> +	case PORT_FEATURE_ID_ERROR:
> +		v = readq(base + PORT_ERROR_CAP);
> +		ibase = FIELD_GET(PORT_ERROR_CAP_INT_VECT, v);
> +		inr = FIELD_GET(PORT_ERROR_CAP_SUPP_INT, v);
> +		break;
> +	case FME_FEATURE_ID_GLOBAL_ERR:
> +		v = readq(base + FME_ERROR_CAP);
> +		ibase = FIELD_GET(FME_ERROR_CAP_INT_VECT, v);
> +		inr = FIELD_GET(FME_ERROR_CAP_SUPP_INT, v);
> +		break;
> +	}
> +
> +	if (!inr) {
> +		*irq_base = 0;
> +		*nr_irqs = 0;
> +		return 0;
> +	}
> +
> +	dev_dbg(binfo->dev, "feature: 0x%llx, irq_base: %u, nr_irqs: %u\n",
> +		(unsigned long long)fid, ibase, inr);
> +
> +	if (ibase + inr > binfo->nr_irqs) {
> +		dev_err(binfo->dev,
> +			"Invalid interrupt number in feature 0x%llx\n",
> +			(unsigned long long)fid);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < inr; i++) {
> +		virq = binfo->irq_table[ibase + i];
> +		if (virq < 0 || virq > NR_IRQS) {
> +			dev_err(binfo->dev,
> +				"Invalid irq table entry for feature 0x%llx\n",
> +				(unsigned long long)fid);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	*irq_base = (unsigned int)ibase;
> +	*nr_irqs = (unsigned int)inr;
> +
> +	return 0;
> +}
> +
>  /*
>   * when create sub feature instances, for private features, it doesn't need
>   * to provide resource size and feature id as they could be read from DFH
> @@ -650,7 +749,9 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  			struct dfl_fpga_enum_dfl *dfl, resource_size_t ofst,
>  			resource_size_t size, u64 fid)
>  {
> +	unsigned int irq_base, nr_irqs;
>  	struct dfl_feature_info *finfo;
> +	int ret;
>  
>  	/* read feature size and id if inputs are invalid */
>  	size = size ? size : feature_size(dfl->ioaddr + ofst);
> @@ -659,6 +760,10 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  	if (dfl->len - ofst < size)
>  		return -EINVAL;
>  
> +	ret = parse_feature_irqs(binfo, ofst, fid, &irq_base, &nr_irqs);
> +	if (ret)
> +		return ret;
> +
>  	finfo = kzalloc(sizeof(*finfo), GFP_KERNEL);
>  	if (!finfo)
>  		return -ENOMEM;
> @@ -667,6 +772,8 @@ create_feature_instance(struct build_feature_devs_info *binfo,
>  	finfo->mmio_res.start = dfl->start + ofst;
>  	finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
>  	finfo->mmio_res.flags = IORESOURCE_MEM;
> +	finfo->irq_base = irq_base;
> +	finfo->nr_irqs = nr_irqs;
>  	finfo->ioaddr = dfl->ioaddr + ofst;
>  
>  	list_add_tail(&finfo->node, &binfo->sub_features);
> @@ -853,6 +960,10 @@ void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info)
>  		devm_kfree(dev, dfl);
>  	}
>  
> +	/* remove irq table */
> +	if (info->irq_table)
> +		devm_kfree(dev, info->irq_table);
> +
>  	devm_kfree(dev, info);
>  	put_device(dev);
>  }
> @@ -892,6 +1003,45 @@ int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
>  }
>  EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_add_dfl);
>  
> +/**
> + * dfl_fpga_enum_info_add_irq - add irq table to enum info
> + *
> + * @info: ptr to dfl_fpga_enum_info
> + * @nr_irqs: number of irqs of the DFL fpga device to be enumerated.
> + * @irq_table: Linux IRQ numbers for all irqs, indexed by local irq index of
> + *	       this device.
> + *
> + * One FPGA device may have several interrupts. This function adds irq
> + * information of the DFL fpga device to enum info for next step enumeration.
> + * This function should be called before dfl_fpga_feature_devs_enumerate().
> + * As we only support one irq domain for all DFLs in the same enum info, adding
> + * irq table a second time for the same enum info will return error.
> + *
> + * If we need to enumerate DFLs which belong to different irq domains, we
> + * should fill more enum info and enumerate them one by one.
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +int dfl_fpga_enum_info_add_irq(struct dfl_fpga_enum_info *info,
> +			       unsigned int nr_irqs, int *irq_table)
> +{
> +	if (!nr_irqs || !irq_table)
> +		return -EINVAL;
> +
> +	if (info->irq_table)
> +		return -EEXIST;
> +
> +	info->irq_table = devm_kmemdup(info->dev, irq_table,
> +				       sizeof(int) * nr_irqs, GFP_KERNEL);
> +	if (!info->irq_table)
> +		return -ENOMEM;
> +
> +	info->nr_irqs = nr_irqs;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dfl_fpga_enum_info_add_irq);
> +
>  static int remove_feature_dev(struct device *dev, void *data)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
> @@ -959,6 +1109,10 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
>  	binfo->dev = info->dev;
>  	binfo->cdev = cdev;
>  
> +	binfo->nr_irqs = info->nr_irqs;
> +	if (info->nr_irqs)
> +		binfo->irq_table = info->irq_table;
> +
>  	/*
>  	 * start enumeration for all feature devices based on Device Feature
>  	 * Lists.
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 74784d3..4bc165f 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -112,6 +112,13 @@
>  #define FME_PORT_OFST_ACC_VF	1
>  #define FME_PORT_OFST_IMP	BIT_ULL(60)
>  
> +/* FME Error Capability Register */
> +#define FME_ERROR_CAP		0x70
> +
> +/* FME Error Capability Register Bitfield */
> +#define FME_ERROR_CAP_SUPP_INT	BIT_ULL(0)		/* Interrupt Support */
> +#define FME_ERROR_CAP_INT_VECT	GENMASK_ULL(12, 1)	/* Interrupt vector */
> +
>  /* PORT Header Register Set */
>  #define PORT_HDR_DFH		DFH
>  #define PORT_HDR_GUID_L		GUID_L
> @@ -145,6 +152,20 @@
>  #define PORT_STS_PWR_STATE_AP2	2			/* 90% throttling */
>  #define PORT_STS_PWR_STATE_AP6	6			/* 100% throttling */
>  
> +/* Port Error Capability Register */
> +#define PORT_ERROR_CAP		0x38
> +
> +/* Port Error Capability Register Bitfield */
> +#define PORT_ERROR_CAP_SUPP_INT	BIT_ULL(0)		/* Interrupt Support */
> +#define PORT_ERROR_CAP_INT_VECT	GENMASK_ULL(12, 1)	/* Interrupt vector */
> +
> +/* Port Uint Capability Register */
> +#define PORT_UINT_CAP		0x8
> +
> +/* Port Uint Capability Register Bitfield */
> +#define PORT_UINT_CAP_INT_NUM	GENMASK_ULL(11, 0)	/* Interrupts num */
> +#define PORT_UINT_CAP_FST_VECT	GENMASK_ULL(23, 12)	/* First Vector */
> +
>  /**
>   * struct dfl_fpga_port_ops - port ops
>   *
> @@ -189,6 +210,15 @@ struct dfl_feature_driver {
>  };
>  
>  /**
> + * struct dfl_feature_irq_ctx - dfl private feature interrupt context
> + *
> + * @irq: Linux IRQ number of this interrupt.
> + */
> +struct dfl_feature_irq_ctx {
> +	int irq;
> +};
> +
> +/**
>   * struct dfl_feature - sub feature of the feature devices
>   *
>   * @id: sub feature id.
> @@ -196,12 +226,16 @@ struct dfl_feature_driver {
>   *		    this index is used to find its mmio resource from the
>   *		    feature dev (platform device)'s reources.
>   * @ioaddr: mapped mmio resource address.
> + * @irq_ctx: interrupt context list.
> + * @nr_irqs: number of interrupt contexts.
>   * @ops: ops of this sub feature.
>   */
>  struct dfl_feature {
>  	u64 id;
>  	int resource_index;
>  	void __iomem *ioaddr;
> +	struct dfl_feature_irq_ctx *irq_ctx;
> +	unsigned int nr_irqs;
>  	const struct dfl_feature_ops *ops;
>  };
>  
> @@ -388,10 +422,14 @@ static inline u8 dfl_feature_revision(void __iomem *base)
>   *
>   * @dev: parent device.
>   * @dfls: list of device feature lists.
> + * @nr_irqs: number of irqs for all feature devices.
> + * @irq_table: Linux IRQ numbers for all irqs, indexed by hw irq numbers.
>   */
>  struct dfl_fpga_enum_info {
>  	struct device *dev;
>  	struct list_head dfls;
> +	unsigned int nr_irqs;
> +	int *irq_table;
>  };
>  
>  /**
> @@ -415,6 +453,8 @@ struct dfl_fpga_enum_info *dfl_fpga_enum_info_alloc(struct device *dev);
>  int dfl_fpga_enum_info_add_dfl(struct dfl_fpga_enum_info *info,
>  			       resource_size_t start, resource_size_t len,
>  			       void __iomem *ioaddr);
> +int dfl_fpga_enum_info_add_irq(struct dfl_fpga_enum_info *info,
> +			       unsigned int nr_irqs, int *irq_table);
>  void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info);
>  
>  /**
> -- 
> 2.7.4


Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>

Re: [PATCH v5 2/7] fpga: dfl: pci: add irq info for feature devices enumeration

[Marcelo Tosatti]

On Mon, Apr 20, 2020 at 04:11:38PM +0800, Xu Yilun wrote:
> Some DFL FPGA PCIe cards (e.g. Intel FPGA Programmable Acceleration
> Card) support MSI-X based interrupts. This patch allows PCIe driver
> to prepare and pass interrupt resources to DFL via enumeration API.
> These interrupt resources could then be assigned to actual features
> which use them.
> 
> 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: put irq resources init code inside cce_enumerate_feature_dev()
>     Some minor changes for Hao's comments.
> v3: Some minor fix for Hao's comments for v2.
> v4: Some minor fix for Hao's comments for v3.
> v5: No change.
> ---
>  drivers/fpga/dfl-pci.c | 80 ++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 71 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> index a78c409..2ff1274 100644
> --- a/drivers/fpga/dfl-pci.c
> +++ b/drivers/fpga/dfl-pci.c
> @@ -39,6 +39,27 @@ static void __iomem *cci_pci_ioremap_bar(struct pci_dev *pcidev, int bar)
>  	return pcim_iomap_table(pcidev)[bar];
>  }
>  
> +static int cci_pci_alloc_irq(struct pci_dev *pcidev)
> +{
> +	int ret, nvec = pci_msix_vec_count(pcidev);
> +
> +	if (nvec <= 0) {
> +		dev_dbg(&pcidev->dev, "fpga interrupt not supported\n");
> +		return 0;
> +	}
> +
> +	ret = pci_alloc_irq_vectors(pcidev, nvec, nvec, PCI_IRQ_MSIX);
> +	if (ret < 0)
> +		return ret;
> +
> +	return nvec;
> +}
> +
> +static void cci_pci_free_irq(struct pci_dev *pcidev)
> +{
> +	pci_free_irq_vectors(pcidev);
> +}
> +
>  /* PCI Device ID */
>  #define PCIE_DEVICE_ID_PF_INT_5_X	0xBCBD
>  #define PCIE_DEVICE_ID_PF_INT_6_X	0xBCC0
> @@ -78,17 +99,38 @@ static void cci_remove_feature_devs(struct pci_dev *pcidev)
>  
>  	/* remove all children feature devices */
>  	dfl_fpga_feature_devs_remove(drvdata->cdev);
> +	cci_pci_free_irq(pcidev);
> +}
> +
> +static int *cci_pci_create_irq_table(struct pci_dev *pcidev, unsigned int nvec)
> +{
> +	unsigned int i;
> +	int *table;
> +
> +	table = kcalloc(nvec, sizeof(int), GFP_KERNEL);
> +	if (table) {
> +		for (i = 0; i < nvec; i++)
> +			table[i] = pci_irq_vector(pcidev, i);
> +	}
> +
> +	return table;
> +}
> +
> +static void cci_pci_free_irq_table(int *table)
> +{
> +	kfree(table);
>  }
>  
>  /* enumerate feature devices under pci device */
>  static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  {
>  	struct cci_drvdata *drvdata = pci_get_drvdata(pcidev);
> +	int port_num, bar, i, nvec, ret = 0;
>  	struct dfl_fpga_enum_info *info;
>  	struct dfl_fpga_cdev *cdev;
>  	resource_size_t start, len;
> -	int port_num, bar, i, ret = 0;
>  	void __iomem *base;
> +	int *irq_table;
>  	u32 offset;
>  	u64 v;
>  
> @@ -97,11 +139,30 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  	if (!info)
>  		return -ENOMEM;
>  
> +	/* add irq info for enumeration if the device support irq */
> +	nvec = cci_pci_alloc_irq(pcidev);
> +	if (nvec < 0) {
> +		dev_err(&pcidev->dev, "Fail to alloc irq %d.\n", nvec);
> +		ret = nvec;
> +		goto enum_info_free_exit;
> +	} else if (nvec) {
> +		irq_table = cci_pci_create_irq_table(pcidev, nvec);
> +		if (!irq_table) {
> +			ret = -ENOMEM;
> +			goto irq_free_exit;
> +		}
> +
> +		ret = dfl_fpga_enum_info_add_irq(info, nvec, irq_table);
> +		cci_pci_free_irq_table(irq_table);
> +		if (ret)
> +			goto irq_free_exit;
> +	}
> +
>  	/* start to find Device Feature List from Bar 0 */
>  	base = cci_pci_ioremap_bar(pcidev, 0);
>  	if (!base) {
>  		ret = -ENOMEM;
> -		goto enum_info_free_exit;
> +		goto irq_free_exit;
>  	}
>  
>  	/*
> @@ -154,7 +215,7 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  		dfl_fpga_enum_info_add_dfl(info, start, len, base);
>  	} else {
>  		ret = -ENODEV;
> -		goto enum_info_free_exit;
> +		goto irq_free_exit;
>  	}
>  
>  	/* start enumeration with prepared enumeration information */
> @@ -162,11 +223,14 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev)
>  	if (IS_ERR(cdev)) {
>  		dev_err(&pcidev->dev, "Enumeration failure\n");
>  		ret = PTR_ERR(cdev);
> -		goto enum_info_free_exit;
> +		goto irq_free_exit;
>  	}
>  
>  	drvdata->cdev = cdev;
>  
> +irq_free_exit:
> +	if (ret)
> +		cci_pci_free_irq(pcidev);
>  enum_info_free_exit:
>  	dfl_fpga_enum_info_free(info);
>  
> @@ -211,12 +275,10 @@ int cci_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *pcidevid)
>  	}
>  
>  	ret = cci_enumerate_feature_devs(pcidev);
> -	if (ret) {
> -		dev_err(&pcidev->dev, "enumeration failure %d.\n", ret);
> -		goto disable_error_report_exit;
> -	}
> +	if (!ret)
> +		return ret;
>  
> -	return ret;
> +	dev_err(&pcidev->dev, "enumeration failure %d.\n", ret);
>  
>  disable_error_report_exit:
>  	pci_disable_pcie_error_reporting(pcidev);
> -- 
> 2.7.4


Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>

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

[Marcelo Tosatti]

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;
> +
> +	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


Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>

Re: [PATCH v5 4/7] fpga: dfl: afu: add interrupt support for port error reporting

[Marcelo Tosatti]

On Mon, Apr 20, 2020 at 04:11:40PM +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 adds interrupt support for port error reporting sub feature.
> It 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 <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 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
> v3: put_user() instead of copy_to_user()
>     improves comments
> v4: use common functions to handle irq ioctls
> v5: minor fixes for Hao's comments
> ---
>  drivers/fpga/dfl-afu-error.c  | 17 +++++++++++++++++
>  drivers/fpga/dfl-afu-main.c   |  4 ++++
>  include/uapi/linux/fpga-dfl.h | 23 +++++++++++++++++++++++
>  3 files changed, 44 insertions(+)
> 
> diff --git a/drivers/fpga/dfl-afu-error.c b/drivers/fpga/dfl-afu-error.c
> index c1467ae..c469118 100644
> --- a/drivers/fpga/dfl-afu-error.c
> +++ b/drivers/fpga/dfl-afu-error.c
> @@ -14,6 +14,7 @@
>   *   Mitchel Henry <henry.mitchel@intel.com>
>   */
>  
> +#include <linux/fpga-dfl.h>
>  #include <linux/uaccess.h>
>  
>  #include "dfl-afu.h"
> @@ -219,6 +220,21 @@ static void port_err_uinit(struct platform_device *pdev,
>  	afu_port_err_mask(&pdev->dev, true);
>  }
>  
> +static long
> +port_err_ioctl(struct platform_device *pdev, struct dfl_feature *feature,
> +	       unsigned int cmd, unsigned long arg)
> +{
> +	switch (cmd) {
> +	case DFL_FPGA_PORT_ERR_GET_IRQ_NUM:
> +		return dfl_feature_ioctl_get_num_irqs(pdev, feature, arg);
> +	case DFL_FPGA_PORT_ERR_SET_IRQ:
> +		return dfl_feature_ioctl_set_irq(pdev, feature, arg);
> +	default:
> +		dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
> +		return -ENODEV;
> +	}
> +}
> +
>  const struct dfl_feature_id port_err_id_table[] = {
>  	{.id = PORT_FEATURE_ID_ERROR,},
>  	{0,}
> @@ -227,4 +243,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 3fa2c59..b1ed7b4 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -578,6 +578,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");
>  
> @@ -587,6 +588,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 7331350..6c71c9d 100644
> --- a/include/uapi/linux/fpga-dfl.h
> +++ b/include/uapi/linux/fpga-dfl.h
> @@ -164,6 +164,29 @@ struct dfl_fpga_irq_set {
>  	__s32 evtfds[];
>  };
>  
> +/**
> + * 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. Currently hardware supports up to 1 irq.
> + * 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 negative value.
> + * Return: 0 on success, -errno on failure.
> + */
> +#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


Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>

Re: [PATCH v5 5/7] fpga: dfl: fme: add interrupt support for global error reporting

[Marcelo Tosatti]

On Mon, Apr 20, 2020 at 04:11:41PM +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 adds interrupt support for fme global error reporting sub
> feature. It 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_FME_ERR_GET_IRQ_NUM
>    get the number of irqs, which is used to determine whether/how many
>    interrupts fme error reporting feature supports.
> 
>  * DFL_FPGA_FME_ERR_SET_IRQ
>    set/unset given eventfds as fme error reporting interrupt triggers.
> 
> 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 DFL_FPGA_FME_ERR_GET_IRQ_NUM instead of
>     DFL_FPGA_FME_ERR_GET_INFO
>     Delete flags field for DFL_FPGA_FME_ERR_SET_IRQ
> v3: put_user() instead of copy_to_user()
>     improves comments
> v4: use common functions to handle irq ioctls
> v5: Minor fixes for Hao's comments
> ---
>  drivers/fpga/dfl-fme-error.c  | 18 ++++++++++++++++++
>  drivers/fpga/dfl-fme-main.c   |  6 ++++++
>  include/uapi/linux/fpga-dfl.h | 23 +++++++++++++++++++++++
>  3 files changed, 47 insertions(+)
> 
> diff --git a/drivers/fpga/dfl-fme-error.c b/drivers/fpga/dfl-fme-error.c
> index f897d41..51c2892 100644
> --- a/drivers/fpga/dfl-fme-error.c
> +++ b/drivers/fpga/dfl-fme-error.c
> @@ -15,6 +15,7 @@
>   *   Mitchel, Henry <henry.mitchel@intel.com>
>   */
>  
> +#include <linux/fpga-dfl.h>
>  #include <linux/uaccess.h>
>  
>  #include "dfl.h"
> @@ -348,6 +349,22 @@ static void fme_global_err_uinit(struct platform_device *pdev,
>  	fme_err_mask(&pdev->dev, true);
>  }
>  
> +static long
> +fme_global_error_ioctl(struct platform_device *pdev,
> +		       struct dfl_feature *feature,
> +		       unsigned int cmd, unsigned long arg)
> +{
> +	switch (cmd) {
> +	case DFL_FPGA_FME_ERR_GET_IRQ_NUM:
> +		return dfl_feature_ioctl_get_num_irqs(pdev, feature, arg);
> +	case DFL_FPGA_FME_ERR_SET_IRQ:
> +		return dfl_feature_ioctl_set_irq(pdev, feature, arg);
> +	default:
> +		dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
> +		return -ENODEV;
> +	}
> +}
> +
>  const struct dfl_feature_id fme_global_err_id_table[] = {
>  	{.id = FME_FEATURE_ID_GLOBAL_ERR,},
>  	{0,}
> @@ -356,4 +373,5 @@ const struct dfl_feature_id fme_global_err_id_table[] = {
>  const struct dfl_feature_ops fme_global_err_ops = {
>  	.init = fme_global_err_init,
>  	.uinit = fme_global_err_uinit,
> +	.ioctl = fme_global_error_ioctl,
>  };
> diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
> index 56d720c..ab3722d 100644
> --- a/drivers/fpga/dfl-fme-main.c
> +++ b/drivers/fpga/dfl-fme-main.c
> @@ -616,11 +616,17 @@ static int fme_release(struct inode *inode, struct file *filp)
>  {
>  	struct dfl_feature_platform_data *pdata = filp->private_data;
>  	struct platform_device *pdev = pdata->dev;
> +	struct dfl_feature *feature;
>  
>  	dev_dbg(&pdev->dev, "Device File Release\n");
>  
>  	mutex_lock(&pdata->lock);
>  	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);
>  	mutex_unlock(&pdata->lock);
>  
>  	return 0;
> diff --git a/include/uapi/linux/fpga-dfl.h b/include/uapi/linux/fpga-dfl.h
> index 6c71c9d..b6495ea 100644
> --- a/include/uapi/linux/fpga-dfl.h
> +++ b/include/uapi/linux/fpga-dfl.h
> @@ -230,4 +230,27 @@ struct dfl_fpga_fme_port_pr {
>   */
>  #define DFL_FPGA_FME_PORT_ASSIGN     _IOW(DFL_FPGA_MAGIC, DFL_FME_BASE + 2, int)
>  
> +/**
> + * DFL_FPGA_FME_ERR_GET_IRQ_NUM - _IOR(DFL_FPGA_MAGIC, DFL_FME_BASE + 3,
> + *							__u32 num_irqs)
> + *
> + * Get the number of irqs supported by the fpga fme error reporting private
> + * feature. Currently hardware supports up to 1 irq.
> + * Return: 0 on success, -errno on failure.
> + */
> +#define DFL_FPGA_FME_ERR_GET_IRQ_NUM	_IOR(DFL_FPGA_MAGIC,	\
> +					     DFL_FME_BASE + 3, __u32)
> +
> +/**
> + * DFL_FPGA_FME_ERR_SET_IRQ - _IOW(DFL_FPGA_MAGIC, DFL_FME_BASE + 4,
> + *						struct dfl_fpga_irq_set)
> + *
> + * Set fpga fme error reporting interrupt trigger if evtfds[n] is valid.
> + * Unset related interrupt trigger if evtfds[n] is a negative value.
> + * Return: 0 on success, -errno on failure.
> + */
> +#define DFL_FPGA_FME_ERR_SET_IRQ	_IOW(DFL_FPGA_MAGIC,	\
> +					     DFL_FME_BASE + 4,	\
> +					     struct dfl_fpga_irq_set)
> +
>  #endif /* _UAPI_LINUX_FPGA_DFL_H */
> -- 
> 2.7.4

Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>

Re: [PATCH v5 6/7] fpga: dfl: afu: add AFU interrupt support

[Marcelo Tosatti]

On Mon, Apr 20, 2020 at 04:11:42PM +0800, Xu Yilun wrote:
> 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 (AFU Interrupt) private feature with
> related capability register which describes the number of supported
> AFU 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 interrupt triggers.
> 
> 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 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
> v3: put_user() instead of copy_to_user()
>     improves comments
> v4: use common functions to handle irq ioctls
> v5: Minor fixes for Hao's comments
> ---
>  drivers/fpga/dfl-afu-main.c   | 28 ++++++++++++++++++++++++++++
>  include/uapi/linux/fpga-dfl.h | 23 +++++++++++++++++++++++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index b1ed7b4..753cda4 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -530,6 +530,30 @@ static const struct dfl_feature_ops port_stp_ops = {
>  	.init = port_stp_init,
>  };
>  
> +static long
> +port_uint_ioctl(struct platform_device *pdev, struct dfl_feature *feature,
> +		unsigned int cmd, unsigned long arg)
> +{
> +	switch (cmd) {
> +	case DFL_FPGA_PORT_UINT_GET_IRQ_NUM:
> +		return dfl_feature_ioctl_get_num_irqs(pdev, feature, arg);
> +	case DFL_FPGA_PORT_UINT_SET_IRQ:
> +		return dfl_feature_ioctl_set_irq(pdev, feature, arg);
> +	default:
> +		dev_dbg(&pdev->dev, "%x cmd not handled", cmd);
> +		return -ENODEV;
> +	}
> +}
> +
> +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,
> @@ -548,6 +572,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 b6495ea..1621b07 100644
> --- a/include/uapi/linux/fpga-dfl.h
> +++ b/include/uapi/linux/fpga-dfl.h
> @@ -187,6 +187,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 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 interrupt trigger if evtfds[n] is valid.
> + * Unset related interrupt trigger if evtfds[n] is a 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


Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>

Re: [PATCH v5 7/7] Documentation: fpga: dfl: add descriptions for interrupt related interfaces.

[Marcelo Tosatti]

On Mon, Apr 20, 2020 at 04:11:43PM +0800, Xu Yilun wrote:
> 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 <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: Update Documents cause change of irq ioctl interfaces.
> v3: No change
> v4: Update interrupt support part.
> v5: No change
> ---
>  Documentation/fpga/dfl.rst | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
> index 094fc8a..702bf62 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,19 @@ 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.
> +In Current DFL, 3 sub features (Port error, FME global error and AFU interrupt)
> +support interrupts.
> +
> +
>  Add new FIUs support
>  ====================
>  It's possible that developers made some new function blocks (FIUs) under this
> -- 
> 2.7.4


Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>