[PATCH v3 0/5] Add OFS support for DFL driver

[PATCH v3 0/5] Add OFS support for DFL driver

[Tianfei zhang]

This is v3 patchset adding OFS (Open FPGA stack) support for
DFL driver. OFS is a collection of RTL and open software providing
interface to access the instantiated RTL easily in FPGA. OFS
leverages the DFL for the implementation of the FPGA RTL design.

Patch 1, allows for ports without specific bar space.
Patch 2, introduces features in dfl_fpga_cdev after DFL enumeration.
On OFS, we will add more extensions or features in DFL in
future, so adding a new member "features"in dfl_fpga_cdev.
Patch 3, fixs VF creation in "Multiple VFs per PR slot" and legacy model.
Patch 4, handles dfl's starting with AFU and allows for VFs to be created.
Patch 5, adds architecture description about OFS support for DFL
in documentation.

Changelog v2 -> v3:
   - no code change, just change the name from IOFS to OFS.

Changelog v1 -> v2:
   - Introducing a new member "features" in dfl_fpga_cdev for feature
     control.
   - Adding new flag DFL_FEAT_PORT_CONNECTED_AFU for OFS legacy model.
   - Updates the documentation for the access models about AFU in OFS.
   - Drop the PCI PID patch and will send it later.

Matthew Gerlach (2):
  fpga: dfl: Allow for ports without specific bar space.
  fpga: dfl: Handle dfl's starting with AFU

Tianfei zhang (3):
  fpga: dfl: add features in dfl_fpga_cdev
  fpga: dfl: fix VF creation in OFS
  Documentation: fpga: dfl: add description of OFS

 Documentation/fpga/dfl.rst | 113 +++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl-pci.c     |  13 ++++-
 drivers/fpga/dfl.c         |  38 ++++++++-----
 drivers/fpga/dfl.h         |   6 ++
 4 files changed, 155 insertions(+), 15 deletions(-)

-- 
2.26.2

[PATCH v3 1/5] fpga: dfl: Allow for ports without specific bar space.

[Tianfei zhang]

From: Matthew Gerlach <matthew.gerlach@linux.intel.com>

In OFS, there is a Port device for each PR slot, like Port
control, Port user clock control and Port errors, those feature
devices are linked with DFL. The DFL of Port device was located
in PCIe Bar 0 MMIO space by default, but it also can put into any
PCIe Bar space. If the BarID (3bits field) in PORTn_OFFSET register
set to invalid means that DFL of Port device is located in the Bar 0
by default, in this case, it don't need add the Bar 0 into dfl list
twice.

---
v2: use FME_HDR_NO_PORT_BAR instead of PCI_STD_NUM_BARS.

Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/fpga/dfl-pci.c | 6 ++++++
 drivers/fpga/dfl.h     | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
index 4d68719e608f..33545c999c06 100644
--- a/drivers/fpga/dfl-pci.c
+++ b/drivers/fpga/dfl-pci.c
@@ -258,6 +258,12 @@ static int find_dfls_by_default(struct pci_dev *pcidev,
 			 */
 			bar = FIELD_GET(FME_PORT_OFST_BAR_ID, v);
 			offset = FIELD_GET(FME_PORT_OFST_DFH_OFST, v);
+			if (bar >= FME_HDR_NO_PORT_BAR) {
+				dev_dbg(&pcidev->dev, "skipping port without specific BAR space %d\n",
+					bar);
+				continue;
+			}
+
 			start = pci_resource_start(pcidev, bar) + offset;
 			len = pci_resource_len(pcidev, bar) - offset;
 
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 53572c7aced0..1fd493e82dd8 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -91,6 +91,7 @@
 #define FME_HDR_PORT_OFST(n)	(0x38 + ((n) * 0x8))
 #define FME_HDR_BITSTREAM_ID	0x60
 #define FME_HDR_BITSTREAM_MD	0x68
+#define FME_HDR_NO_PORT_BAR	7
 
 /* FME Fab Capability Register Bitfield */
 #define FME_CAP_FABRIC_VERID	GENMASK_ULL(7, 0)	/* Fabric version ID */
-- 
2.26.2

[PATCH v3 2/5] fpga: dfl: add features in dfl_fpga_cdev

[Tianfei zhang]

Introducing features in dfl_fpga_cdev during DFL enumeration.
On OFS, we will add more extensions or features in DFL in
future, so adding a new member "features"in dfl_fpga_cdev.
For example, in the legacy model, the AFU was connected to
Port device, but in "multiple VFs per PR slot" model, the
AFU or PR slot without connected to Port device directly,
so in this model, we only can access the resource of AFU
or PR slot via VFs. In this patch, we introducing a new
flags DFL_FEAT_PORT_CONNECTED_AFU to distinguish them.

Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
 drivers/fpga/dfl.c | 6 +++++-
 drivers/fpga/dfl.h | 5 +++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 599bb21d86af..5872031c2e9f 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1124,6 +1124,7 @@ static void build_info_complete(struct build_feature_devs_info *binfo)
 static int parse_feature_fiu(struct build_feature_devs_info *binfo,
 			     resource_size_t ofst)
 {
+	struct dfl_fpga_cdev *cdev = binfo->cdev;
 	int ret = 0;
 	u32 offset;
 	u16 id;
@@ -1160,8 +1161,11 @@ static int parse_feature_fiu(struct build_feature_devs_info *binfo,
 	v = readq(binfo->ioaddr + NEXT_AFU);
 
 	offset = FIELD_GET(NEXT_AFU_NEXT_DFH_OFST, v);
-	if (offset)
+	if (offset) {
+		if (dfh_id_to_type(id) == PORT_ID)
+			cdev->features |= DFL_FEAT_PORT_CONNECTED_AFU;
 		return parse_feature_afu(binfo, offset);
+	}
 
 	dev_dbg(binfo->dev, "No AFUs detected on FIU %d\n", id);
 
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 1fd493e82dd8..6171bcdcb3c5 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -461,6 +461,9 @@ 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);
 
+/* in legacy model, the AFU was connected to Port device */
+#define DFL_FEAT_PORT_CONNECTED_AFU  BIT_ULL(0)
+
 /**
  * struct dfl_fpga_cdev - container device of DFL based FPGA
  *
@@ -470,6 +473,7 @@ void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info);
  * @lock: mutex lock to protect the port device list.
  * @port_dev_list: list of all port feature devices under this container device.
  * @released_port_num: released port number under this container device.
+ * @features: features discovered during DFL enumeration.
  */
 struct dfl_fpga_cdev {
 	struct device *parent;
@@ -478,6 +482,7 @@ struct dfl_fpga_cdev {
 	struct mutex lock;
 	struct list_head port_dev_list;
 	int released_port_num;
+	u64 features;
 };
 
 struct dfl_fpga_cdev *
-- 
2.26.2

[PATCH v3 3/5] fpga: dfl: fix VF creation in OFS

[Tianfei zhang]

In OFS legacy model, there is only 1 Port device related to
1 VF, the flag DFL_FEAT_PORT_CONNECTED_AFU will take notes for
this model. In legacy model, it need to check the released port
number match VF device number or not. But in "Multiple VFs per
PR slot" model, the Port device would not connected to AFU/PR
slot, so we don't need to release the Port device before creating
the VFs.

Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
 drivers/fpga/dfl.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 5872031c2e9f..fd04ef5c8b03 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1702,11 +1702,13 @@ int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vfs)
 
 	mutex_lock(&cdev->lock);
 	/*
-	 * can't turn multiple ports into 1 VF device, only 1 port for 1 VF
-	 * device, so if released port number doesn't match VF device number,
-	 * then reject the request with -EINVAL error code.
+	 * In the OFS legacy model, it can't turn multiple ports into 1 VF
+	 * device, because only 1 port conneced to 1 VF device, so if released
+	 * port number doesn't match VF device number, then reject the request
+	 * with -EINVAL error code.
 	 */
-	if (cdev->released_port_num != num_vfs) {
+	if ((cdev->features & DFL_FEAT_PORT_CONNECTED_AFU) &&
+	    cdev->released_port_num != num_vfs) {
 		ret = -EINVAL;
 		goto done;
 	}
-- 
2.26.2

[PATCH v3 4/5] fpga: dfl: Handle dfl's starting with AFU

[Tianfei zhang]

From: Matthew Gerlach <matthew.gerlach@linux.intel.com>

Allow for a Device Feature List (DFL) to start with
a Device Feature Header (DFH) of type Accelerator Function Unit (AFU)
by doing nothing. This allows for PCIe VFs to be created.

Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/fpga/dfl-pci.c |  7 ++++++-
 drivers/fpga/dfl.c     | 22 +++++++++++++---------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
index 33545c999c06..e7d58e7b1bbd 100644
--- a/drivers/fpga/dfl-pci.c
+++ b/drivers/fpga/dfl-pci.c
@@ -275,7 +275,12 @@ static int find_dfls_by_default(struct pci_dev *pcidev,
 
 		dfl_fpga_enum_info_add_dfl(info, start, len);
 	} else {
-		ret = -ENODEV;
+		v = readq(base + DFH);
+		if (FIELD_GET(DFH_TYPE, v) != DFH_TYPE_AFU) {
+			dev_info(&pcidev->dev, "Unknown feature type 0x%llx id 0x%llx\n",
+				 FIELD_GET(DFH_TYPE, v), FIELD_GET(DFH_ID, v));
+			ret = -ENODEV;
+		}
 	}
 
 	/* release I/O mappings for next step enumeration */
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index fd04ef5c8b03..e30bbb3039cd 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -900,9 +900,11 @@ static void build_info_free(struct build_feature_devs_info *binfo)
 		dfl_id_free(feature_dev_id_type(binfo->feature_dev),
 			    binfo->feature_dev->id);
 
-		list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
-			list_del(&finfo->node);
-			kfree(finfo);
+		if (!list_empty(&binfo->sub_features)) {
+			list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
+				list_del(&finfo->node);
+				kfree(finfo);
+			}
 		}
 	}
 
@@ -1439,12 +1441,14 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info)
 	 * start enumeration for all feature devices based on Device Feature
 	 * Lists.
 	 */
-	list_for_each_entry(dfl, &info->dfls, node) {
-		ret = parse_feature_list(binfo, dfl->start, dfl->len);
-		if (ret) {
-			remove_feature_devs(cdev);
-			build_info_free(binfo);
-			goto unregister_region_exit;
+	if (!list_empty(&info->dfls)) {
+		list_for_each_entry(dfl, &info->dfls, node) {
+			ret = parse_feature_list(binfo, dfl->start, dfl->len);
+			if (ret) {
+				remove_feature_devs(cdev);
+				build_info_free(binfo);
+				goto unregister_region_exit;
+			}
 		}
 	}
 
-- 
2.26.2

[PATCH v3 5/5] Documentation: fpga: dfl: add description of OFS

[Tianfei zhang]

This patch adds description about OFS support for DFL.

---
v3:
change IOFS to OFS in documentation.

v2:
* Fixs some typos.
* Adds more detail description about the models of AFU access which supported in OFS.

Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
 Documentation/fpga/dfl.rst | 113 +++++++++++++++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index ef9eec71f6f3..753507e7b162 100644
--- a/Documentation/fpga/dfl.rst
+++ b/Documentation/fpga/dfl.rst
@@ -556,6 +556,119 @@ new DFL feature via UIO direct access, its feature id should be added to the
 driver's id_table.
 
 
+Open FPGA stack
+=====================
+
+Open FPGA stack aka OFS, a collection of RTL and open software providing interface
+to access the instantiated RTL easily in FPGA. OFS leverages the DFL for the
+implementation of the FPGA RTL design.
+
+OFS designs allow for the arrangement of software interfaces across multiple
+PCIe endpoints. Some of these interfaces may be PFs defined in the static region
+that connect to interfaces in an IP that is loaded via Partial Reconfiguration (PR).
+And some of these interfaces may be VFs defined in the PR region that can be
+reconfigured by the end-user. Furthermore, these PFs/VFs may also be arranged
+using a DFL such that features may be discovered and accessed in user space
+(with the aid of a generic kernel driver like vfio-pci). The diagram below depicts
+an example design with two PFs and two VFs. In this example, it will export the
+management functions via PF0, PF1 will bind with virtio-net driver presenting itself
+as a network interface to the OS. The other functions, VF0 and VF1, leverage VFIO
+to export the MMIO space to an application or assign to a VM.
+::
+
+     +-----------------+  +--------------+  +-------------+  +------------+
+     | FPGA Management |  |   VirtIO     |  |  User App   |  | Virtual    |
+     |      App        |  |     App      |  |             |  | Machine    |
+     +--------+--------+  +------+-------+  +------+------+  +-----+------+
+              |                  |                 |               |
+     +--------+--------+  +------+-------+  +------+------+        |
+     |     DFL Driver  |  |VirtIO driver |  |    VFIO     |        |
+     +--------+--------+--+------+-------+  +------+------+        |
+              |                  |                 |               |
+              |                  |                 |               |
+     +--------+--------+  +------+-------+  +------+------+   +----+------+
+     |     PF0         |  |     PF1      |  |   PF0_VF0   |   |  PF0_VF1  |
+     +-----------------+  +--------------+  +-------------+   +-----------+
+
+As accelerators are specialized hardware, they are typically limited in the
+number installed in a given system. Many use cases require them to be shared
+across multiple software contexts or threads of software execution, either
+through partitioning of individual dedicated resources, or virtualization of
+shared resources. On OFS, it provides several models to share the AFU
+resources via PR mechanism and hardware-based virtualization schemes.
+
+1. Legacy model.
+   In legacy FPGA card platforms (like Intel PAC N3000 or N5000 Card),there is
+   a notion that the boundary between the AFU and the shell is also the unit of
+   PR for those FPGA platforms. In this model, it can only able to handle a
+   single context, because it only has one PR engine, and one PR region which
+   has an associated Port device.
+2. Multiple VFs per PR slot.
+   In this model, available AFU resources may allow instantiation of many of VFs
+   which has a dedicated PCIe function with their own dedicated MMIO space, or
+   partition a region of MMIO space on a single PCIe function.
+   In this model, the Port device would not connected to AFU/PR slot, so we don't
+   need to release the Port device before creating the VFs. For DFL's view, the AFU
+   will not connect to Port device, so the Next_AFU pointer in FIU feature header
+   of port device points to NULL in this model. On the other hand, each VF can start
+   with an AFU feature header without connected to a FIU Port feature header.
+3. Micro-Personas in AFU.
+   OFS introducing a new concept to extend the FPGA usage, Micro-Personas in
+   AFU. It finds some downsides of the legacy model to be unacceptable, because
+   this may be desirable by a customer who intends to switch out one accelerator
+   for another accelerator without having to reconfigure the entire FPGA.
+   Micro-Personas allow the developer to designate their own AFU-defined PR
+   regions. In this model the unit of PR is not the entire AFU, instead
+   the unit of PR can be any size block or blocks inside the AFU.
+   In this model, it has PR capability includes one PR engine and multiple PR regions,
+   and each PR region has an associated port gasket. A PR region may also be
+   referred to as a PR slot. Port gasket is similar with port device in legacy
+   model which include the port control, port user clock control and port errors.
+
+OFS provides the diversity for access the AFU resource to RTL developer.
+An IP designer may choose to add more than one PF for interfacing with IP
+on the FPGA and choose different model to access the AFU resource.
+
+There is one reference architecture design using the "Multiple VFs per PR slot"
+model for OFS as illustrated below. In this reference design, it exports the
+FPGA management functions via PF0. PF1 will bind with virtio-net driver
+presenting itself as a network interface to the OS. PF2 will bound to the
+vfio-pci driver allowing the user space software to discover and interface
+with the specific workload like diagnostic test. To access the AFU resource,
+it uses SR-IOV to partition workload interfaces across various VFs.
+::
+
+                              +----------------------+
+                              |   PF/VF mux/demux    |
+                              +--+--+-----+------+-+-+
+                                 |  |     |      | |
+        +------------------------+  |     |      | |
+  PF0   |                 +---------+   +-+      | |
+    +---+---+             |         +---+----+   | |
+    |  DFH  |             |         |   DFH  |   | |
+    +-------+       +-----+----+    +--------+   | |
+    |  FME  |       |  VirtIO  |    |  Test  |   | |
+    +---+---+       +----------+    +--------+   | |
+        |                PF1            PF2      | |
+        |                                        | |
+        |                             +----------+ |
+        |                             |           ++
+        |                             |           |
+        |                             | PF0_VF0   | PF0_VF1
+        |           +-----------------+-----------+------------+
+        |           |           +-----+-----------+--------+   |
+        |           |           |     |           |        |   |
+        |           | +------+  |  +--+ -+     +--+---+    |   |
+        |           | | Port |  |  | DFH |     |  DFH |    |   |
+        +-----------+ +------+  |  +-----+     +------+    |   |
+                    |           |  | DEV |     |  DEV |    |   |
+                    |           |  +-----+     +------+    |   |
+                    |           |            PR Slot       |   |
+                    |           +--------------------------+   |
+                    | Port Gasket                              |
+                    +------------------------------------------+
+
+
 Open discussion
 ===============
 FME driver exports one ioctl (DFL_FPGA_FME_PORT_PR) for partial reconfiguration
-- 
2.26.2

RE: [PATCH v3 1/5] fpga: dfl: Allow for ports without specific bar space.

[Wu, Hao]

> -----Original Message-----
> From: Zhang, Tianfei <tianfei.zhang@intel.com>
> Sent: Tuesday, March 1, 2022 2:21 PM
> To: Wu, Hao <hao.wu@intel.com>; trix@redhat.com; mdf@kernel.org; Xu, Yilun
> <yilun.xu@intel.com>; linux-fpga@vger.kernel.org; linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; corbet@lwn.net; Matthew Gerlach
> <matthew.gerlach@linux.intel.com>; Zhang, Tianfei <tianfei.zhang@intel.com>
> Subject: [PATCH v3 1/5] fpga: dfl: Allow for ports without specific bar space.
> 
> From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> 
> In OFS, there is a Port device for each PR slot, like Port
> control, Port user clock control and Port errors, those feature
> devices are linked with DFL. The DFL of Port device was located
> in PCIe Bar 0 MMIO space by default, but it also can put into any

There is not default BAR for Port. In OFS implementation it could
be 0, but not default value for other implementation.

> PCIe Bar space. If the BarID (3bits field) in PORTn_OFFSET register
> set to invalid means that DFL of Port device is located in the Bar 0
> by default, in this case, it don't need add the Bar 0 into dfl list
> twice.

So why not just use existing method (e.g. BAR0 + offset) to locate
DFL of port?

The title is confusing too, PORTs still have its BAR location/space.

Hao

> 
> ---
> v2: use FME_HDR_NO_PORT_BAR instead of PCI_STD_NUM_BARS.
> 
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> ---
>  drivers/fpga/dfl-pci.c | 6 ++++++
>  drivers/fpga/dfl.h     | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> index 4d68719e608f..33545c999c06 100644
> --- a/drivers/fpga/dfl-pci.c
> +++ b/drivers/fpga/dfl-pci.c
> @@ -258,6 +258,12 @@ static int find_dfls_by_default(struct pci_dev *pcidev,
>  			 */
>  			bar = FIELD_GET(FME_PORT_OFST_BAR_ID, v);
>  			offset = FIELD_GET(FME_PORT_OFST_DFH_OFST, v);
> +			if (bar >= FME_HDR_NO_PORT_BAR) {
> +				dev_dbg(&pcidev->dev, "skipping port without
> specific BAR space %d\n",
> +					bar);
> +				continue;
> +			}
> +
>  			start = pci_resource_start(pcidev, bar) + offset;
>  			len = pci_resource_len(pcidev, bar) - offset;
> 
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 53572c7aced0..1fd493e82dd8 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -91,6 +91,7 @@
>  #define FME_HDR_PORT_OFST(n)	(0x38 + ((n) * 0x8))
>  #define FME_HDR_BITSTREAM_ID	0x60
>  #define FME_HDR_BITSTREAM_MD	0x68
> +#define FME_HDR_NO_PORT_BAR	7
> 
>  /* FME Fab Capability Register Bitfield */
>  #define FME_CAP_FABRIC_VERID	GENMASK_ULL(7, 0)	/* Fabric
> version ID */
> --
> 2.26.2

RE: [PATCH v3 1/5] fpga: dfl: Allow for ports without specific bar space.

[Zhang, Tianfei]

> -----Original Message-----
> From: Wu, Hao <hao.wu@intel.com>
> Sent: Tuesday, March 1, 2022 2:44 PM
> To: Zhang, Tianfei <tianfei.zhang@intel.com>; trix@redhat.com;
> mdf@kernel.org; Xu, Yilun <yilun.xu@intel.com>; linux-fpga@vger.kernel.org;
> linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; corbet@lwn.net; Matthew Gerlach
> <matthew.gerlach@linux.intel.com>
> Subject: RE: [PATCH v3 1/5] fpga: dfl: Allow for ports without specific bar space.
> 
> > -----Original Message-----
> > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > Sent: Tuesday, March 1, 2022 2:21 PM
> > To: Wu, Hao <hao.wu@intel.com>; trix@redhat.com; mdf@kernel.org; Xu,
> > Yilun <yilun.xu@intel.com>; linux-fpga@vger.kernel.org;
> > linux-doc@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org; corbet@lwn.net; Matthew Gerlach
> > <matthew.gerlach@linux.intel.com>; Zhang, Tianfei
> > <tianfei.zhang@intel.com>
> > Subject: [PATCH v3 1/5] fpga: dfl: Allow for ports without specific bar space.
> >
> > From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> >
> > In OFS, there is a Port device for each PR slot, like Port control,
> > Port user clock control and Port errors, those feature devices are
> > linked with DFL. The DFL of Port device was located in PCIe Bar 0 MMIO
> > space by default, but it also can put into any
> 
> There is not default BAR for Port. In OFS implementation it could be 0, but not
> default value for other implementation.
> 
> > PCIe Bar space. If the BarID (3bits field) in PORTn_OFFSET register
> > set to invalid means that DFL of Port device is located in the Bar 0
> > by default, in this case, it don't need add the Bar 0 into dfl list
> > twice.
> 
> So why not just use existing method (e.g. BAR0 + offset) to locate DFL of port?

For N6000 card, this BarID in PORTn_OFFSET register was set to 0b111 in RTL, which is larger than the maximum PCI BAR numbe PCI_STD_NUM_BARS,
So for software perspective, this means there is no additional the BAR space of this port device, but the DFH of this port device
still located in BAR0.

> 
> The title is confusing too, PORTs still have its BAR location/space.
> 
> Hao
> 
> >
> > ---
> > v2: use FME_HDR_NO_PORT_BAR instead of PCI_STD_NUM_BARS.
> >
> > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> > ---
> >  drivers/fpga/dfl-pci.c | 6 ++++++
> >  drivers/fpga/dfl.h     | 1 +
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
> > 4d68719e608f..33545c999c06 100644
> > --- a/drivers/fpga/dfl-pci.c
> > +++ b/drivers/fpga/dfl-pci.c
> > @@ -258,6 +258,12 @@ static int find_dfls_by_default(struct pci_dev *pcidev,
> >  			 */
> >  			bar = FIELD_GET(FME_PORT_OFST_BAR_ID, v);
> >  			offset = FIELD_GET(FME_PORT_OFST_DFH_OFST, v);
> > +			if (bar >= FME_HDR_NO_PORT_BAR) {
> > +				dev_dbg(&pcidev->dev, "skipping port without
> > specific BAR space %d\n",
> > +					bar);
> > +				continue;
> > +			}
> > +
> >  			start = pci_resource_start(pcidev, bar) + offset;
> >  			len = pci_resource_len(pcidev, bar) - offset;
> >
> > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h index
> > 53572c7aced0..1fd493e82dd8 100644
> > --- a/drivers/fpga/dfl.h
> > +++ b/drivers/fpga/dfl.h
> > @@ -91,6 +91,7 @@
> >  #define FME_HDR_PORT_OFST(n)	(0x38 + ((n) * 0x8))
> >  #define FME_HDR_BITSTREAM_ID	0x60
> >  #define FME_HDR_BITSTREAM_MD	0x68
> > +#define FME_HDR_NO_PORT_BAR	7
> >
> >  /* FME Fab Capability Register Bitfield */
> >  #define FME_CAP_FABRIC_VERID	GENMASK_ULL(7, 0)	/* Fabric
> > version ID */
> > --
> > 2.26.2

RE: [PATCH v3 2/5] fpga: dfl: add features in dfl_fpga_cdev

[Wu, Hao]

> -----Original Message-----
> From: Zhang, Tianfei <tianfei.zhang@intel.com>
> Sent: Tuesday, March 1, 2022 2:21 PM
> To: Wu, Hao <hao.wu@intel.com>; trix@redhat.com; mdf@kernel.org; Xu, Yilun
> <yilun.xu@intel.com>; linux-fpga@vger.kernel.org; linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; corbet@lwn.net; Zhang, Tianfei
> <tianfei.zhang@intel.com>
> Subject: [PATCH v3 2/5] fpga: dfl: add features in dfl_fpga_cdev
> 
> Introducing features in dfl_fpga_cdev during DFL enumeration.

It's a little confusing, maybe flags is a better name.

> On OFS, we will add more extensions or features in DFL in
> future, so adding a new member "features"in dfl_fpga_cdev.
> For example, in the legacy model, the AFU was connected to
> Port device, but in "multiple VFs per PR slot" model, the
> AFU or PR slot without connected to Port device directly,
> so in this model, we only can access the resource of AFU
> or PR slot via VFs. In this patch, we introducing a new
> flags DFL_FEAT_PORT_CONNECTED_AFU to distinguish them.

Please consider where the flags should be, cdev is the container
of all DFLs, is it possible that one cdev contains two ports, one
has AFU, the other one doesn't?

Hao

> 
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> ---
>  drivers/fpga/dfl.c | 6 +++++-
>  drivers/fpga/dfl.h | 5 +++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 599bb21d86af..5872031c2e9f 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -1124,6 +1124,7 @@ static void build_info_complete(struct
> build_feature_devs_info *binfo)
>  static int parse_feature_fiu(struct build_feature_devs_info *binfo,
>  			     resource_size_t ofst)
>  {
> +	struct dfl_fpga_cdev *cdev = binfo->cdev;
>  	int ret = 0;
>  	u32 offset;
>  	u16 id;
> @@ -1160,8 +1161,11 @@ static int parse_feature_fiu(struct
> build_feature_devs_info *binfo,
>  	v = readq(binfo->ioaddr + NEXT_AFU);
> 
>  	offset = FIELD_GET(NEXT_AFU_NEXT_DFH_OFST, v);
> -	if (offset)
> +	if (offset) {
> +		if (dfh_id_to_type(id) == PORT_ID)
> +			cdev->features |= DFL_FEAT_PORT_CONNECTED_AFU;
>  		return parse_feature_afu(binfo, offset);
> +	}
> 
>  	dev_dbg(binfo->dev, "No AFUs detected on FIU %d\n", id);
> 
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 1fd493e82dd8..6171bcdcb3c5 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -461,6 +461,9 @@ 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);
> 
> +/* in legacy model, the AFU was connected to Port device */
> +#define DFL_FEAT_PORT_CONNECTED_AFU  BIT_ULL(0)
> +
>  /**
>   * struct dfl_fpga_cdev - container device of DFL based FPGA
>   *
> @@ -470,6 +473,7 @@ void dfl_fpga_enum_info_free(struct
> dfl_fpga_enum_info *info);
>   * @lock: mutex lock to protect the port device list.
>   * @port_dev_list: list of all port feature devices under this container device.
>   * @released_port_num: released port number under this container device.
> + * @features: features discovered during DFL enumeration.
>   */
>  struct dfl_fpga_cdev {
>  	struct device *parent;
> @@ -478,6 +482,7 @@ struct dfl_fpga_cdev {
>  	struct mutex lock;
>  	struct list_head port_dev_list;
>  	int released_port_num;
> +	u64 features;
>  };
> 
>  struct dfl_fpga_cdev *
> --
> 2.26.2

RE: [PATCH v3 3/5] fpga: dfl: fix VF creation in OFS

[Wu, Hao]

> Subject: [PATCH v3 3/5] fpga: dfl: fix VF creation in OFS

Why this is a fix? 

> 
> In OFS legacy model, there is only 1 Port device related to
> 1 VF, the flag DFL_FEAT_PORT_CONNECTED_AFU will take notes for
> this model. 

What is OFS legacy model? And what is legacy model? They are
the same or not? It's quite confusing.

> In legacy model, it need to check the released port
> number match VF device number or not. But in "Multiple VFs per
> PR slot" model, the Port device would not connected to AFU/PR
> slot, so we don't need to release the Port device before creating
> the VFs.

I think the major difference here is not that PORT has AFU or not, but
If PORT needs to be turned into VFs. Would it be better to be decided
by FME registers controlling the access to PORT?

If we consider following same flow for enable SRIOV to reuse existing
tools, then we need to add FME flags to indicate user (and dfl-pci), 
no need to assign/release port for SRIOV enabling.

Hao

> 
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> ---
>  drivers/fpga/dfl.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 5872031c2e9f..fd04ef5c8b03 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -1702,11 +1702,13 @@ int dfl_fpga_cdev_config_ports_vf(struct
> dfl_fpga_cdev *cdev, int num_vfs)
> 
>  	mutex_lock(&cdev->lock);
>  	/*
> -	 * can't turn multiple ports into 1 VF device, only 1 port for 1 VF
> -	 * device, so if released port number doesn't match VF device number,
> -	 * then reject the request with -EINVAL error code.
> +	 * In the OFS legacy model, it can't turn multiple ports into 1 VF
> +	 * device, because only 1 port conneced to 1 VF device, so if released
> +	 * port number doesn't match VF device number, then reject the request
> +	 * with -EINVAL error code.
>  	 */
> -	if (cdev->released_port_num != num_vfs) {
> +	if ((cdev->features & DFL_FEAT_PORT_CONNECTED_AFU) &&
> +	    cdev->released_port_num != num_vfs) {
>  		ret = -EINVAL;
>  		goto done;
>  	}
> --
> 2.26.2

RE: [PATCH v3 4/5] fpga: dfl: Handle dfl's starting with AFU

[Wu, Hao]

> -----Original Message-----
> From: Zhang, Tianfei <tianfei.zhang@intel.com>
> Sent: Tuesday, March 1, 2022 2:21 PM
> To: Wu, Hao <hao.wu@intel.com>; trix@redhat.com; mdf@kernel.org; Xu, Yilun
> <yilun.xu@intel.com>; linux-fpga@vger.kernel.org; linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; corbet@lwn.net; Matthew Gerlach
> <matthew.gerlach@linux.intel.com>; Zhang, Tianfei <tianfei.zhang@intel.com>
> Subject: [PATCH v3 4/5] fpga: dfl: Handle dfl's starting with AFU
> 
> From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> 
> Allow for a Device Feature List (DFL) to start with
> a Device Feature Header (DFH) of type Accelerator Function Unit (AFU)
> by doing nothing. This allows for PCIe VFs to be created.

Why this is related to VFs creation? We don't have AFU in PF in OFS case, right?

> 
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> ---
>  drivers/fpga/dfl-pci.c |  7 ++++++-
>  drivers/fpga/dfl.c     | 22 +++++++++++++---------
>  2 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> index 33545c999c06..e7d58e7b1bbd 100644
> --- a/drivers/fpga/dfl-pci.c
> +++ b/drivers/fpga/dfl-pci.c
> @@ -275,7 +275,12 @@ static int find_dfls_by_default(struct pci_dev *pcidev,
> 
>  		dfl_fpga_enum_info_add_dfl(info, start, len);
>  	} else {

Can be something like else if dfl_feature_is_afu(base) following the same style.

> -		ret = -ENODEV;
> +		v = readq(base + DFH);
> +		if (FIELD_GET(DFH_TYPE, v) != DFH_TYPE_AFU) {
> +			dev_info(&pcidev->dev, "Unknown feature type 0x%llx
> id 0x%llx\n",
> +				 FIELD_GET(DFH_TYPE, v), FIELD_GET(DFH_ID,
> v));
> +			ret = -ENODEV;
> +		}

But nothing else done for AFU so far? How it works? Sounds like more patches
are required.

>  	}
> 
>  	/* release I/O mappings for next step enumeration */
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index fd04ef5c8b03..e30bbb3039cd 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -900,9 +900,11 @@ static void build_info_free(struct
> build_feature_devs_info *binfo)
>  		dfl_id_free(feature_dev_id_type(binfo->feature_dev),
>  			    binfo->feature_dev->id);
> 
> -		list_for_each_entry_safe(finfo, p, &binfo->sub_features, node)
> {
> -			list_del(&finfo->node);
> -			kfree(finfo);
> +		if (!list_empty(&binfo->sub_features)) {
> +			list_for_each_entry_safe(finfo, p, &binfo-
> >sub_features, node) {
> +				list_del(&finfo->node);
> +				kfree(finfo);
> +			}
>  		}
>  	}
> 
> @@ -1439,12 +1441,14 @@ dfl_fpga_feature_devs_enumerate(struct
> dfl_fpga_enum_info *info)
>  	 * start enumeration for all feature devices based on Device Feature
>  	 * Lists.
>  	 */
> -	list_for_each_entry(dfl, &info->dfls, node) {
> -		ret = parse_feature_list(binfo, dfl->start, dfl->len);
> -		if (ret) {
> -			remove_feature_devs(cdev);
> -			build_info_free(binfo);
> -			goto unregister_region_exit;
> +	if (!list_empty(&info->dfls)) {
> +		list_for_each_entry(dfl, &info->dfls, node) {
> +			ret = parse_feature_list(binfo, dfl->start, dfl->len);
> +			if (ret) {
> +				remove_feature_devs(cdev);
> +				build_info_free(binfo);
> +				goto unregister_region_exit;
> +			}
>  		}
>  	}
> 
> --
> 2.26.2

RE: [PATCH v3 4/5] fpga: dfl: Handle dfl's starting with AFU

[Zhang, Tianfei]

> -----Original Message-----
> From: Wu, Hao <hao.wu@intel.com>
> Sent: Tuesday, March 1, 2022 3:28 PM
> To: Zhang, Tianfei <tianfei.zhang@intel.com>; trix@redhat.com;
> mdf@kernel.org; Xu, Yilun <yilun.xu@intel.com>; linux-fpga@vger.kernel.org;
> linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; corbet@lwn.net; Matthew Gerlach
> <matthew.gerlach@linux.intel.com>
> Subject: RE: [PATCH v3 4/5] fpga: dfl: Handle dfl's starting with AFU
> 
> > -----Original Message-----
> > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > Sent: Tuesday, March 1, 2022 2:21 PM
> > To: Wu, Hao <hao.wu@intel.com>; trix@redhat.com; mdf@kernel.org; Xu,
> > Yilun <yilun.xu@intel.com>; linux-fpga@vger.kernel.org;
> > linux-doc@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org; corbet@lwn.net; Matthew Gerlach
> > <matthew.gerlach@linux.intel.com>; Zhang, Tianfei
> > <tianfei.zhang@intel.com>
> > Subject: [PATCH v3 4/5] fpga: dfl: Handle dfl's starting with AFU
> >
> > From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> >
> > Allow for a Device Feature List (DFL) to start with a Device Feature
> > Header (DFH) of type Accelerator Function Unit (AFU) by doing nothing.
> > This allows for PCIe VFs to be created.
> 
> Why this is related to VFs creation? We don't have AFU in PF in OFS case, right?

Yes, in " Multiple VFs per PR slot" model, the AFU was not connected with Port.

> 
> >
> > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> > ---
> >  drivers/fpga/dfl-pci.c |  7 ++++++-
> >  drivers/fpga/dfl.c     | 22 +++++++++++++---------
> >  2 files changed, 19 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
> > 33545c999c06..e7d58e7b1bbd 100644
> > --- a/drivers/fpga/dfl-pci.c
> > +++ b/drivers/fpga/dfl-pci.c
> > @@ -275,7 +275,12 @@ static int find_dfls_by_default(struct pci_dev
> > *pcidev,
> >
> >  		dfl_fpga_enum_info_add_dfl(info, start, len);
> >  	} else {
> 
> Can be something like else if dfl_feature_is_afu(base) following the same style.

dfl_feature_is_afu(base) will be better.

> 
> > -		ret = -ENODEV;
> > +		v = readq(base + DFH);
> > +		if (FIELD_GET(DFH_TYPE, v) != DFH_TYPE_AFU) {
> > +			dev_info(&pcidev->dev, "Unknown feature type 0x%llx
> > id 0x%llx\n",
> > +				 FIELD_GET(DFH_TYPE, v), FIELD_GET(DFH_ID,
> > v));
> > +			ret = -ENODEV;
> > +		}
> 
> But nothing else done for AFU so far? How it works? Sounds like more patches
> are required.

In " Multiple VFs per PR slot" model, we access the AFU resource via VFs for example VFIO-PIC driver, and we cannot access the AFU MMIO resource by AFU APIs.

> 
> >  	}
> >
> >  	/* release I/O mappings for next step enumeration */ diff --git
> > a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index
> > fd04ef5c8b03..e30bbb3039cd 100644
> > --- a/drivers/fpga/dfl.c
> > +++ b/drivers/fpga/dfl.c
> > @@ -900,9 +900,11 @@ static void build_info_free(struct
> > build_feature_devs_info *binfo)
> >  		dfl_id_free(feature_dev_id_type(binfo->feature_dev),
> >  			    binfo->feature_dev->id);
> >
> > -		list_for_each_entry_safe(finfo, p, &binfo->sub_features, node)
> > {
> > -			list_del(&finfo->node);
> > -			kfree(finfo);
> > +		if (!list_empty(&binfo->sub_features)) {
> > +			list_for_each_entry_safe(finfo, p, &binfo-
> > >sub_features, node) {
> > +				list_del(&finfo->node);
> > +				kfree(finfo);
> > +			}
> >  		}
> >  	}
> >
> > @@ -1439,12 +1441,14 @@ dfl_fpga_feature_devs_enumerate(struct
> > dfl_fpga_enum_info *info)
> >  	 * start enumeration for all feature devices based on Device Feature
> >  	 * Lists.
> >  	 */
> > -	list_for_each_entry(dfl, &info->dfls, node) {
> > -		ret = parse_feature_list(binfo, dfl->start, dfl->len);
> > -		if (ret) {
> > -			remove_feature_devs(cdev);
> > -			build_info_free(binfo);
> > -			goto unregister_region_exit;
> > +	if (!list_empty(&info->dfls)) {
> > +		list_for_each_entry(dfl, &info->dfls, node) {
> > +			ret = parse_feature_list(binfo, dfl->start, dfl->len);
> > +			if (ret) {
> > +				remove_feature_devs(cdev);
> > +				build_info_free(binfo);
> > +				goto unregister_region_exit;
> > +			}
> >  		}
> >  	}
> >
> > --
> > 2.26.2