[PATCH v6 0/6] Add OFS support for DFL driver

[PATCH v6 0/6] Add OFS support for DFL driver

[Tianfei Zhang]

This is v6 patchset adding OFS (Open FPGA Stack) support for
DFL driver, based on v5.17-rc8. OFS is a collection of RTL
and open source software providing interface to access the
instantiated RTL easily in an FPGA. OFS leverages the DFL for
the implementation of the FPGA RTL design.

Patch 1, allows for ports without local bar space for "multiple VFs per
PR slot" model.
Patch 2, uses some lowest bits of flags to track the port status which
the AFU was connected to port device or not.
Patch 3, checks the number of released port match the number of
VFs or not in legacy model.
Patch 4, configures port access mode for afu connected with port.
Patch 5, handles dfl's starting with AFU.
Patch 6, adds architecture description about OFS support for DFL
in documentation.

Changelog v5 -> v6:
   - fix documentation with Randy's comment.

Changelog v4 -> v5:
   - fix documentation with Matthew and Randy's comment.

Changelog v3 -> v4:
   - change "features" to "flags" in dfl_fpga_cdev to track the status
     of port device.
   - use dfl_fpga_cdev->flags to check if it need configure the port access
     mode or not.
   - add description about access the AFU on "multiple VFs per PR slot"
     model.

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 ports without local bar space.
  fpga: dfl: support PF/VF starting with DFH

Tianfei zhang (4):
  fpga: dfl: tracking port conntected with AFU
  fpga: dfl: check released_port_num and num_vfs for legacy model
  fpga: dfl: configure port access mode for afu connected with port
  Documentation: fpga: dfl: add description of OFS

 Documentation/fpga/dfl.rst | 114 +++++++++++++++++++++++++++++++++++++
 drivers/fpga/dfl-pci.c     |   9 +++
 drivers/fpga/dfl.c         |  62 ++++++++++++++------
 drivers/fpga/dfl.h         |  22 +++++++
 4 files changed, 191 insertions(+), 16 deletions(-)

-- 
2.26.2

[PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.

[Tianfei Zhang]

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

In OFS, each PR slot (AFU) has one port device which include Port
control, Port user clock control and Port errors. In legacy model,
the AFU MMIO space was connected with Port device, so from port
device point of view, there is a bar space associated with this
port device. But in "Multiple VFs per PR slot" model, the AFU MMIO
space was not connected with Port device. The BarID (3bits field) in
PORTn_OFFSET register indicates which PCI bar space associated with
this port device, the value 0b111 (FME_HDR_NO_PORT_BAR) means that
no PCI bar for this port device.

---
v3: add PCI bar number checking with PCI_STD_NUM_BARS.
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 | 7 +++++++
 drivers/fpga/dfl.h     | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
index 4d68719e608f..2e9abeca3625 100644
--- a/drivers/fpga/dfl-pci.c
+++ b/drivers/fpga/dfl-pci.c
@@ -258,6 +258,13 @@ 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 >= PCI_STD_NUM_BARS ||
+			    bar == FME_HDR_NO_PORT_BAR) {
+				dev_dbg(&pcidev->dev, "skipping port without local 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 v6 2/6] fpga: dfl: tracking port conntected with AFU

[Tianfei Zhang]

From: Tianfei zhang <tianfei.zhang@intel.com>

Introducing flags in dfl_fpga_cdev to track extensions
or new features discovered during DFL enumeration. It uses
some lowest bits of flags to track the port status which
the AFU was connected to port device or not. In 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.

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

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 599bb21d86af..712c53363fda 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1124,8 +1124,10 @@ 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;
+	u32 port;
 	u16 id;
 	u64 v;
 
@@ -1160,8 +1162,15 @@ 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) {
+			port = FIELD_GET(PORT_CAP_PORT_NUM,
+					 readq(binfo->ioaddr + PORT_HDR_CAP));
+			cdev->flags |= dfl_feat_port_connect_afu(port);
+		}
+
 		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..bc56b7e8c01b 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -461,6 +461,16 @@ 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);
 
+/*
+ * Bitfields in flags of dfl_fpga_cdev.
+ *
+ * 0 - (DFL_PORT_CONNECT_BITS -1): AFU was connected with Port device.
+ * DFL_PORT_CONNECT_BITS - 63: reserved.
+ */
+#define dfl_feat_port_connect_afu(port) (BIT_ULL(port))
+#define DFL_PORT_CONNECT_BITS  MAX_DFL_FPGA_PORT_NUM
+#define DFL_FEAT_PORT_CONNECT_MASK ((1UL << (DFL_PORT_CONNECT_BITS)) - 1)
+
 /**
  * struct dfl_fpga_cdev - container device of DFL based FPGA
  *
@@ -470,6 +480,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.
+ * @flags: extensions discovered during DFL enumeration.
  */
 struct dfl_fpga_cdev {
 	struct device *parent;
@@ -478,6 +489,7 @@ struct dfl_fpga_cdev {
 	struct mutex lock;
 	struct list_head port_dev_list;
 	int released_port_num;
+	u64 flags;
 };
 
 struct dfl_fpga_cdev *
-- 
2.26.2

[PATCH v6 3/6] fpga: dfl: check released_port_num and num_vfs for legacy model

[Tianfei Zhang]

From: Tianfei zhang <tianfei.zhang@intel.com>

In OFS legacy model, there is 1:1 mapping for Port device and VF,
so it need to check the number of released port match the number of
VFs or not. But in "Multiple VFs per PR slot" model, there is 1:N
mapping for the Port device and 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 ++++++----
 drivers/fpga/dfl.h |  2 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 712c53363fda..b95b29c5c81d 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1707,11 +1707,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 ((dfl_has_port_connected_afu(cdev) &&
+	     cdev->released_port_num != num_vfs)) {
 		ret = -EINVAL;
 		goto done;
 	}
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index bc56b7e8c01b..83c2c50975e5 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -471,6 +471,8 @@ void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info);
 #define DFL_PORT_CONNECT_BITS  MAX_DFL_FPGA_PORT_NUM
 #define DFL_FEAT_PORT_CONNECT_MASK ((1UL << (DFL_PORT_CONNECT_BITS)) - 1)
 
+#define dfl_has_port_connected_afu(cdev) ((cdev)->flags & DFL_FEAT_PORT_CONNECT_MASK)
+
 /**
  * struct dfl_fpga_cdev - container device of DFL based FPGA
  *
-- 
2.26.2

[PATCH v6 4/6] fpga: dfl: configure port access mode for afu connected with port

[Tianfei Zhang]

From: Tianfei zhang <tianfei.zhang@intel.com>

In legacy model, we should set AfuAccessCtrl (Bit 55) in PORTn_OFFSET
register to switch VF and PF for AFU. But in "multiple VFs per PR slot"
model, the PF/VF mux hardware unit will statically configure the funciton
mapping without set the AfuAccessCtrl by software. This patch check the
port status in dfl_fpga_cdev->flags before configure the port access mode.

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

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index b95b29c5c81d..71e0725b6be0 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1666,6 +1666,17 @@ static void config_port_access_mode(struct device *fme_dev, int port_id,
 #define config_port_vf_mode(dev, id) config_port_access_mode(dev, id, true)
 #define config_port_pf_mode(dev, id) config_port_access_mode(dev, id, false)
 
+static int dfl_check_port_connect_afu(struct device *dev, u64 flags)
+{
+	void __iomem *base;
+	int port;
+
+	base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
+	port = FIELD_GET(PORT_CAP_PORT_NUM, readq(base + PORT_HDR_CAP));
+
+	return flags & dfl_feat_port_connect_afu(port);
+}
+
 /**
  * dfl_fpga_cdev_config_ports_pf - configure ports to PF access mode
  *
@@ -1683,7 +1694,9 @@ void dfl_fpga_cdev_config_ports_pf(struct dfl_fpga_cdev *cdev)
 		if (device_is_registered(&pdata->dev->dev))
 			continue;
 
-		config_port_pf_mode(cdev->fme_dev, pdata->id);
+		/* configure port access mode for AFU connected to Port device */
+		if (dfl_check_port_connect_afu(&pdata->dev->dev, cdev->flags))
+			config_port_pf_mode(cdev->fme_dev, pdata->id);
 	}
 	mutex_unlock(&cdev->lock);
 }
@@ -1722,7 +1735,9 @@ int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vfs)
 		if (device_is_registered(&pdata->dev->dev))
 			continue;
 
-		config_port_vf_mode(cdev->fme_dev, pdata->id);
+		/* configure port access mode for AFU connected to Port device */
+		if (dfl_check_port_connect_afu(&pdata->dev->dev, cdev->flags))
+			config_port_vf_mode(cdev->fme_dev, pdata->id);
 	}
 done:
 	mutex_unlock(&cdev->lock);
-- 
2.26.2

[PATCH v6 5/6] fpga: dfl: support PF/VF starting with DFH

[Tianfei Zhang]

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

In OFS, it allows several PFs and VFs in static region or PR region.
Those PFs and VFs managed by DFL or a specific device, like virtio-net
device. Those PFs and VFs which managed by DFL can start with DFH, and
leverage VFIO to expose to an application or assign to a VM.

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

diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
index 2e9abeca3625..7d8b53330152 100644
--- a/drivers/fpga/dfl-pci.c
+++ b/drivers/fpga/dfl-pci.c
@@ -275,6 +275,8 @@ static int find_dfls_by_default(struct pci_dev *pcidev,
 		len = pci_resource_len(pcidev, 0);
 
 		dfl_fpga_enum_info_add_dfl(info, start, len);
+	} else if (dfl_feature_is_afu(base)) {
+		dev_info(&pcidev->dev, "find AFU\n");
 	} else {
 		ret = -ENODEV;
 	}
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 71e0725b6be0..db676f7482ec 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);
+			}
 		}
 	}
 
@@ -1444,12 +1446,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;
+			}
 		}
 	}
 
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 83c2c50975e5..08edaeeb7f80 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -421,6 +421,13 @@ static inline bool dfl_feature_is_port(void __iomem *base)
 		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
 }
 
+static inline bool dfl_feature_is_afu(void __iomem *base)
+{
+	u64 v = readq(base + DFH);
+
+	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_AFU);
+}
+
 static inline u8 dfl_feature_revision(void __iomem *base)
 {
 	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
-- 
2.26.2

[PATCH v6 6/6] Documentation: fpga: dfl: add description of OFS

[Tianfei Zhang]

From: Tianfei zhang <tianfei.zhang@intel.com>

This patch adds description about OFS support for DFL.

---
v6:
fix documentation with Randy's comment.
v5:
fix documentation with Matthew and Randy's comment.
v4:
add description about access the AFU on "multiple VFs per PR slot" model.
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 | 114 +++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
index ef9eec71f6f3..93f262fe7b8c 100644
--- a/Documentation/fpga/dfl.rst
+++ b/Documentation/fpga/dfl.rst
@@ -556,6 +556,120 @@ 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 (OFS) is a collection of RTL and open source software providing
+interfaces to access the instantiated RTL easily in an 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 use DFLs 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. OFS provides several models to share the AFU resources via
+PR mechanism and hardware-based virtualization schemes.
+
+1. Legacy model.
+   With legacy model FPGA cards like Intel PAC N3000 or N5000, there is
+   a notion that the boundary between the AFU and the shell is also the unit of
+   PR for those FPGA platforms. This model is 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 VFs
+   which have a dedicated PCIe function with their own dedicated MMIO space, or
+   partition a region of MMIO space on a single PCIe function. Intel PAC N6000
+   card has implemented this model.
+   In this model, the AFU/PR slot was not connected to port device. For DFL's view,
+   the Next_AFU pointer in FIU feature header of port device points to NULL in this
+   model, so in AFU driver perspective, there is no AFU MMIO region managed by
+   AFU driver. On the other hand, each VF can start with an AFU feature header without
+   being connected to a FIU Port feature header.
+
+In multiple VFs per PR slot model, the port device can still be accessed using
+ioctls API which expose /dev/dfl-port.h device nodes, like port reset, get
+port info, whose APIs were mentioned in AFU section in this documentation. But
+it cannot access the AFU MMIO space via AFU ioctl APIs like DFL_FPGA_PORT_DMA_MAP
+because there is no AFU MMIO space managed in the AFU driver. Users can access
+the AFU resource by creating VF devices via PCIe SRIOV interface, and then access
+the VF via VFIO driver or assign the VF to VM.
+
+In multiple VFs per PR slot model, the steps to enable VFs are compatible with
+legacy mode which are mentioned in "FPGA virtualization - PCIe SRIOV" section
+in this documentation.
+
+OFS provides the diversity for accessing 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 bind 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 v6 1/6] fpga: dfl: Allow ports without local bar space.

[Wu, Hao]

> -----Original Message-----
> From: Zhang, Tianfei <tianfei.zhang@intel.com>
> Sent: Wednesday, March 16, 2022 3:08 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;
> linux-kernel@vger.kernel.org; rdunlap@infradead.org
> Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>;
> Zhang, Tianfei <tianfei.zhang@intel.com>
> Subject: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> 
> From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> 
> In OFS, each PR slot (AFU) has one port device which include Port
> control, Port user clock control and Port errors. In legacy model,
> the AFU MMIO space was connected with Port device, so from port
> device point of view, there is a bar space associated with this
> port device. But in "Multiple VFs per PR slot" model, the AFU MMIO
> space was not connected with Port device. The BarID (3bits field) in
> PORTn_OFFSET register indicates which PCI bar space associated with
> this port device, the value 0b111 (FME_HDR_NO_PORT_BAR) means that
> no PCI bar for this port device.

The commit message is not matching the change, it's not related to AFU...

Current usage (FME DFL and PORT DFL are not linked together)

FME DFL 
PORT DFL (located by FME's PORTn_OFFSET register, BAR + offset)

Your proposed new usage is (FME DFL and PORT DFL are linked together)

FME DFL -> PORT DFL
So FME's PORTn_OFFSET can be marked, then driver could skip it.

Is my understanding correct? If yes, please update your title and commit
message, and add some comments in code as well.

Again, the change you did in dfl core code, is not only impacting your
OFS device, but also future DFL devices, it's an extension to DFL.

Thanks
Hao

> 
> ---
> v3: add PCI bar number checking with PCI_STD_NUM_BARS.
> 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 | 7 +++++++
>  drivers/fpga/dfl.h     | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> index 4d68719e608f..2e9abeca3625 100644
> --- a/drivers/fpga/dfl-pci.c
> +++ b/drivers/fpga/dfl-pci.c
> @@ -258,6 +258,13 @@ 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 >= PCI_STD_NUM_BARS ||
> +			    bar == FME_HDR_NO_PORT_BAR) {
> +				dev_dbg(&pcidev->dev, "skipping port without
> local 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 v6 1/6] fpga: dfl: Allow ports without local bar space.

[Zhang, Tianfei]

> -----Original Message-----
> From: Wu, Hao <hao.wu@intel.com>
> Sent: Thursday, March 17, 2022 10:05 AM
> 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; linux-kernel@vger.kernel.org;
> rdunlap@infradead.org
> Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> 
> > -----Original Message-----
> > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > Sent: Wednesday, March 16, 2022 3:08 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; linux-kernel@vger.kernel.org;
> > rdunlap@infradead.org
> > Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>;
> > Zhang, Tianfei <tianfei.zhang@intel.com>
> > Subject: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> >
> > From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> >
> > In OFS, each PR slot (AFU) has one port device which include Port
> > control, Port user clock control and Port errors. In legacy model, the
> > AFU MMIO space was connected with Port device, so from port device
> > point of view, there is a bar space associated with this port device.
> > But in "Multiple VFs per PR slot" model, the AFU MMIO space was not
> > connected with Port device. The BarID (3bits field) in PORTn_OFFSET
> > register indicates which PCI bar space associated with this port
> > device, the value 0b111 (FME_HDR_NO_PORT_BAR) means that no PCI bar
> > for this port device.
> 
> The commit message is not matching the change, it's not related to AFU...
> 
> Current usage (FME DFL and PORT DFL are not linked together)

This usage is only on Intel PAC N3000 and N5000 card. 
In my understand, the space of Port can put into any PCI bar space. 
In the previous use case, the space of port was located on Bar 2.
For OFS, it allows the port without specific bar space.

> 
> FME DFL
> PORT DFL (located by FME's PORTn_OFFSET register, BAR + offset)
> 
> Your proposed new usage is (FME DFL and PORT DFL are linked together)
> 
> FME DFL -> PORT DFL
> So FME's PORTn_OFFSET can be marked, then driver could skip it.
> 
> Is my understanding correct? If yes, please update your title and commit
> message, and add some comments in code as well.

From DLF perspective, I think it is yes.

How about the title:  "fpga: dfl: Allow Port and FME's DFL link together" ?

I will also add some comments in code.
Here is the new git commit for this patch, any comments? 

In previous FPGA platform like Intel PAC N3000 and N5000, The BarID (3bits field) in PORTn_OFFSET
register indicated which PCI bar space was associated with this port device. In this case, the DFL of Port device
was located in the specific PCI bar space, and then the FME and Port's DFL were not linked. But in OFS, we extend
the usage, it allows the FME and Port's DFL  linked together when there was no local PCI bar space specified by 
the Port device. The value 0b111 (FME_HDR_NO_PORT_BAR) of BarID means that no specific PCI bar space 
was associated with the port device.

> 
> Again, the change you did in dfl core code, is not only impacting your OFS
> device, but also future DFL devices, it's an extension to DFL.

Yes, I agree that is an extended usage.

> 
> Thanks
> Hao
> 
> >
> > ---
> > v3: add PCI bar number checking with PCI_STD_NUM_BARS.
> > 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 | 7 +++++++
> >  drivers/fpga/dfl.h     | 1 +
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
> > 4d68719e608f..2e9abeca3625 100644
> > --- a/drivers/fpga/dfl-pci.c
> > +++ b/drivers/fpga/dfl-pci.c
> > @@ -258,6 +258,13 @@ 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 >= PCI_STD_NUM_BARS ||
> > +			    bar == FME_HDR_NO_PORT_BAR) {
> > +				dev_dbg(&pcidev->dev, "skipping port without
> > local 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 v6 1/6] fpga: dfl: Allow ports without local bar space.

[Wu, Hao]

> > -----Original Message-----
> > From: Wu, Hao <hao.wu@intel.com>
> > Sent: Thursday, March 17, 2022 10:05 AM
> > 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; linux-kernel@vger.kernel.org;
> > rdunlap@infradead.org
> > Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> >
> > > -----Original Message-----
> > > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > > Sent: Wednesday, March 16, 2022 3:08 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; linux-kernel@vger.kernel.org;
> > > rdunlap@infradead.org
> > > Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>;
> > > Zhang, Tianfei <tianfei.zhang@intel.com>
> > > Subject: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> > >
> > > From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > >
> > > In OFS, each PR slot (AFU) has one port device which include Port
> > > control, Port user clock control and Port errors. In legacy model, the
> > > AFU MMIO space was connected with Port device, so from port device
> > > point of view, there is a bar space associated with this port device.
> > > But in "Multiple VFs per PR slot" model, the AFU MMIO space was not
> > > connected with Port device. The BarID (3bits field) in PORTn_OFFSET
> > > register indicates which PCI bar space associated with this port
> > > device, the value 0b111 (FME_HDR_NO_PORT_BAR) means that no PCI bar
> > > for this port device.
> >
> > The commit message is not matching the change, it's not related to AFU...
> >
> > Current usage (FME DFL and PORT DFL are not linked together)
> 
> This usage is only on Intel PAC N3000 and N5000 card.
> In my understand, the space of Port can put into any PCI bar space.
> In the previous use case, the space of port was located on Bar 2.
> For OFS, it allows the port without specific bar space.

I didn't understand what you mean. Without your change, existing
driver supports Port in any BAR indicated by PORTn_OFFSET, it's fine
you put Port to BAR 0, or same BAR as FME. What do you mean by
"port without specific bar space"?

> 
> >
> > FME DFL
> > PORT DFL (located by FME's PORTn_OFFSET register, BAR + offset)
> >
> > Your proposed new usage is (FME DFL and PORT DFL are linked together)
> >
> > FME DFL -> PORT DFL
> > So FME's PORTn_OFFSET can be marked, then driver could skip it.
> >
> > Is my understanding correct? If yes, please update your title and commit
> > message, and add some comments in code as well.
> 
> From DLF perspective, I think it is yes.
> 
> How about the title:  "fpga: dfl: Allow Port and FME's DFL link together" ?

"Allow Port to be linked to FME's DFL" should be better, as we don't
encourage that people to connect FME DFL to Port DFL or any mixed order.

> 
> I will also add some comments in code.
> Here is the new git commit for this patch, any comments?
> 
> In previous FPGA platform like Intel PAC N3000 and N5000, The BarID (3bits field)
> in PORTn_OFFSET
> register indicated which PCI bar space was associated with this port device. In
> this case, the DFL of Port device
> was located in the specific PCI bar space, and then the FME and Port's DFL were
> not linked. But in OFS, we extend
> the usage, it allows the FME and Port's DFL  linked together when there was no
> local PCI bar space specified by
> the Port device. The value 0b111 (FME_HDR_NO_PORT_BAR) of BarID means
> that no specific PCI bar space
> was associated with the port device.

Currently we use PORTn_OFFSET to locate PORT DFLs, and PORT DFLs are not
connected FME DFL. But for some cases (e.g. Intel Open FPGA Stack device), 
PORT DFLs are connected to FME DFL directly, so we don't need to search
PORT DFLs via PORTn_OFFSET again. If BAR value of PORTn_OFFSET is 0x7
(FME_PORT_OFST_BAR_SKIP/INVALID - depends the description added to
DFL spec) then driver will skip searching the DFL for that port.

> 
> >
> > Again, the change you did in dfl core code, is not only impacting your OFS
> > device, but also future DFL devices, it's an extension to DFL.
> 
> Yes, I agree that is an extended usage.

Please make sure related changes documented in DFL spec as well.

> 
> >
> > Thanks
> > Hao
> >
> > >
> > > ---
> > > v3: add PCI bar number checking with PCI_STD_NUM_BARS.
> > > 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 | 7 +++++++
> > >  drivers/fpga/dfl.h     | 1 +
> > >  2 files changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
> > > 4d68719e608f..2e9abeca3625 100644
> > > --- a/drivers/fpga/dfl-pci.c
> > > +++ b/drivers/fpga/dfl-pci.c
> > > @@ -258,6 +258,13 @@ 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 >= PCI_STD_NUM_BARS ||
> > > +			    bar == FME_HDR_NO_PORT_BAR) {
> > > +				dev_dbg(&pcidev->dev, "skipping port without
> > > local 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 v6 2/6] fpga: dfl: tracking port conntected with AFU

[Wu, Hao]

> -----Original Message-----
> From: Zhang, Tianfei <tianfei.zhang@intel.com>
> Sent: Wednesday, March 16, 2022 3:08 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;
> linux-kernel@vger.kernel.org; rdunlap@infradead.org
> Cc: corbet@lwn.net; Zhang, Tianfei <tianfei.zhang@intel.com>; Matthew
> Gerlach <matthew.gerlach@linux.intel.com>
> Subject: [PATCH v6 2/6] fpga: dfl: tracking port conntected with AFU
> 

"conntected" -> "connected"

> From: Tianfei zhang <tianfei.zhang@intel.com>
> 
> Introducing flags in dfl_fpga_cdev to track extensions
> or new features discovered during DFL enumeration. It uses
> some lowest bits of flags to track the port status which
> the AFU was connected to port device or not. In 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.

It's "Port with AFU vs "Port without AFU", so why we have this flag
in container device not for each port device? 
and probably you need to describe why we need this flag. What I am
thinking is that port driver can use this flag to decide if afu related
interface will be visible or not...does it make more sense to have
this flag for port device itself?


> 
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> ---
>  drivers/fpga/dfl.c | 11 ++++++++++-
>  drivers/fpga/dfl.h | 12 ++++++++++++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 599bb21d86af..712c53363fda 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -1124,8 +1124,10 @@ 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;
> +	u32 port;
>  	u16 id;
>  	u64 v;
> 
> @@ -1160,8 +1162,15 @@ 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) {
> +			port = FIELD_GET(PORT_CAP_PORT_NUM,
> +					 readq(binfo->ioaddr +
> PORT_HDR_CAP));
> +			cdev->flags |= dfl_feat_port_connect_afu(port);
> +		}
> +
>  		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..bc56b7e8c01b 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -461,6 +461,16 @@ 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);
> 
> +/*
> + * Bitfields in flags of dfl_fpga_cdev.
> + *
> + * 0 - (DFL_PORT_CONNECT_BITS -1): AFU was connected with Port device.
> + * DFL_PORT_CONNECT_BITS - 63: reserved.
> + */
> +#define dfl_feat_port_connect_afu(port) (BIT_ULL(port))
> +#define DFL_PORT_CONNECT_BITS  MAX_DFL_FPGA_PORT_NUM
> +#define DFL_FEAT_PORT_CONNECT_MASK ((1UL <<
> (DFL_PORT_CONNECT_BITS)) - 1)
> +
>  /**
>   * struct dfl_fpga_cdev - container device of DFL based FPGA
>   *
> @@ -470,6 +480,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.
> + * @flags: extensions discovered during DFL enumeration.
>   */
>  struct dfl_fpga_cdev {
>  	struct device *parent;
> @@ -478,6 +489,7 @@ struct dfl_fpga_cdev {
>  	struct mutex lock;
>  	struct list_head port_dev_list;
>  	int released_port_num;
> +	u64 flags;
>  };
> 
>  struct dfl_fpga_cdev *
> --
> 2.26.2

RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.

[Zhang, Tianfei]

> -----Original Message-----
> From: Wu, Hao <hao.wu@intel.com>
> Sent: Thursday, March 17, 2022 4:18 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; linux-kernel@vger.kernel.org;
> rdunlap@infradead.org
> Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> 
> > > -----Original Message-----
> > > From: Wu, Hao <hao.wu@intel.com>
> > > Sent: Thursday, March 17, 2022 10:05 AM
> > > 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;
> > > linux-kernel@vger.kernel.org; rdunlap@infradead.org
> > > Cc: corbet@lwn.net; Matthew Gerlach
> > > <matthew.gerlach@linux.intel.com>
> > > Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> > >
> > > > -----Original Message-----
> > > > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > > > Sent: Wednesday, March 16, 2022 3:08 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; linux-kernel@vger.kernel.org;
> > > > rdunlap@infradead.org
> > > > Cc: corbet@lwn.net; Matthew Gerlach
> > > > <matthew.gerlach@linux.intel.com>;
> > > > Zhang, Tianfei <tianfei.zhang@intel.com>
> > > > Subject: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> > > >
> > > > From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > >
> > > > In OFS, each PR slot (AFU) has one port device which include Port
> > > > control, Port user clock control and Port errors. In legacy model,
> > > > the AFU MMIO space was connected with Port device, so from port
> > > > device point of view, there is a bar space associated with this port device.
> > > > But in "Multiple VFs per PR slot" model, the AFU MMIO space was
> > > > not connected with Port device. The BarID (3bits field) in
> > > > PORTn_OFFSET register indicates which PCI bar space associated
> > > > with this port device, the value 0b111 (FME_HDR_NO_PORT_BAR) means
> > > > that no PCI bar for this port device.
> > >
> > > The commit message is not matching the change, it's not related to AFU...
> > >
> > > Current usage (FME DFL and PORT DFL are not linked together)
> >
> > This usage is only on Intel PAC N3000 and N5000 card.
> > In my understand, the space of Port can put into any PCI bar space.
> > In the previous use case, the space of port was located on Bar 2.
> > For OFS, it allows the port without specific bar space.
> 
> I didn't understand what you mean. Without your change, existing driver
> supports Port in any BAR indicated by PORTn_OFFSET, it's fine you put Port to
> BAR 0, or same BAR as FME. What do you mean by "port without specific bar
> space"?

"port with specific bar space" means that the port has a dedicated bar space, including the DFL, AFU, this is use 
case in N3000/N5000 card.

"port without specific bar space" means the port without specific bar space, and the Port linked with FME for DFL
perspective.

> 
> >
> > >
> > > FME DFL
> > > PORT DFL (located by FME's PORTn_OFFSET register, BAR + offset)
> > >
> > > Your proposed new usage is (FME DFL and PORT DFL are linked
> > > together)
> > >
> > > FME DFL -> PORT DFL
> > > So FME's PORTn_OFFSET can be marked, then driver could skip it.
> > >
> > > Is my understanding correct? If yes, please update your title and
> > > commit message, and add some comments in code as well.
> >
> > From DLF perspective, I think it is yes.
> >
> > How about the title:  "fpga: dfl: Allow Port and FME's DFL link together" ?
> 
> "Allow Port to be linked to FME's DFL" should be better, as we don't encourage
> that people to connect FME DFL to Port DFL or any mixed order.

Looks good.

> 
> >
> > I will also add some comments in code.
> > Here is the new git commit for this patch, any comments?
> >
> > In previous FPGA platform like Intel PAC N3000 and N5000, The BarID
> > (3bits field) in PORTn_OFFSET register indicated which PCI bar space
> > was associated with this port device. In this case, the DFL of Port
> > device was located in the specific PCI bar space, and then the FME and
> > Port's DFL were not linked. But in OFS, we extend the usage, it allows
> > the FME and Port's DFL  linked together when there was no local PCI
> > bar space specified by the Port device. The value 0b111
> > (FME_HDR_NO_PORT_BAR) of BarID means that no specific PCI bar space
> > was associated with the port device.
> 
> Currently we use PORTn_OFFSET to locate PORT DFLs, and PORT DFLs are not
> connected FME DFL. But for some cases (e.g. Intel Open FPGA Stack device),
> PORT DFLs are connected to FME DFL directly, so we don't need to search PORT
> DFLs via PORTn_OFFSET again. If BAR value of PORTn_OFFSET is 0x7
> (FME_PORT_OFST_BAR_SKIP/INVALID - depends the description added to DFL
> spec) then driver will skip searching the DFL for that port.

It is good for me.

> 
> >
> > >
> > > Again, the change you did in dfl core code, is not only impacting
> > > your OFS device, but also future DFL devices, it's an extension to DFL.
> >
> > Yes, I agree that is an extended usage.
> 
> Please make sure related changes documented in DFL spec as well.

I will add it on documentation.

> 
> >
> > >
> > > Thanks
> > > Hao
> > >
> > > >
> > > > ---
> > > > v3: add PCI bar number checking with PCI_STD_NUM_BARS.
> > > > 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 | 7 +++++++
> > > >  drivers/fpga/dfl.h     | 1 +
> > > >  2 files changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
> > > > 4d68719e608f..2e9abeca3625 100644
> > > > --- a/drivers/fpga/dfl-pci.c
> > > > +++ b/drivers/fpga/dfl-pci.c
> > > > @@ -258,6 +258,13 @@ 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 >= PCI_STD_NUM_BARS ||
> > > > +			    bar == FME_HDR_NO_PORT_BAR) {
> > > > +				dev_dbg(&pcidev->dev, "skipping port without
> > > > local 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 v6 6/6] Documentation: fpga: dfl: add description of OFS

[Wu, Hao]

> Subject: [PATCH v6 6/6] Documentation: fpga: dfl: add description of OFS
> 
> From: Tianfei zhang <tianfei.zhang@intel.com>
> 
> This patch adds description about OFS support for DFL.

I think another major extension is for SRIOV support, DFL needs to be extended to support
another SRIOV usage model, could we have some more descriptions for that?
e.g. In the new usage model, that PORT is not turned into VF and still can be accessed
in PF? how port release will be handled (or ignored) in this new model? 

> 
> ---
> v6:
> fix documentation with Randy's comment.
> v5:
> fix documentation with Matthew and Randy's comment.
> v4:
> add description about access the AFU on "multiple VFs per PR slot" model.
> 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 | 114 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 114 insertions(+)
> 
> diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
> index ef9eec71f6f3..93f262fe7b8c 100644
> --- a/Documentation/fpga/dfl.rst
> +++ b/Documentation/fpga/dfl.rst
> @@ -556,6 +556,120 @@ 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 (OFS) is a collection of RTL and open source software
> providing
> +interfaces to access the instantiated RTL easily in an 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 use DFLs 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. OFS provides several models to share the AFU resources via
> +PR mechanism and hardware-based virtualization schemes.
> +
> +1. Legacy model.
> +   With legacy model FPGA cards like Intel PAC N3000 or N5000, there is
> +   a notion that the boundary between the AFU and the shell is also the unit of
> +   PR for those FPGA platforms. This model is 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 VFs
> +   which have a dedicated PCIe function with their own dedicated MMIO space,
> or
> +   partition a region of MMIO space on a single PCIe function. Intel PAC N6000
> +   card has implemented this model.
> +   In this model, the AFU/PR slot was not connected to port device. For DFL's
> view,
> +   the Next_AFU pointer in FIU feature header of port device points to NULL in
> this
> +   model, so in AFU driver perspective, there is no AFU MMIO region managed
> by
> +   AFU driver. On the other hand, each VF can start with an AFU feature header
> without
> +   being connected to a FIU Port feature header.
> +
> +In multiple VFs per PR slot model, the port device can still be accessed using
> +ioctls API which expose /dev/dfl-port.h device nodes, like port reset, get
> +port info, whose APIs were mentioned in AFU section in this documentation.
> But
> +it cannot access the AFU MMIO space via AFU ioctl APIs like
> DFL_FPGA_PORT_DMA_MAP
> +because there is no AFU MMIO space managed in the AFU driver. Users can
> access
> +the AFU resource by creating VF devices via PCIe SRIOV interface, and then
> access
> +the VF via VFIO driver or assign the VF to VM.
> +
> +In multiple VFs per PR slot model, the steps to enable VFs are compatible with
> +legacy mode which are mentioned in "FPGA virtualization - PCIe SRIOV" section
> +in this documentation.
> +
> +OFS provides the diversity for accessing 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 bind 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 v6 3/6] fpga: dfl: check released_port_num and num_vfs for legacy model

[Wu, Hao]

> Subject: [PATCH v6 3/6] fpga: dfl: check released_port_num and num_vfs for
> legacy model
> 
> From: Tianfei zhang <tianfei.zhang@intel.com>
> 
> In OFS legacy model, there is 1:1 mapping for Port device and VF,
> so it need to check the number of released port match the number of
> VFs or not. But in "Multiple VFs per PR slot" model, there is 1:N
> mapping for the Port device and VFs.

The title and commit message seems not matching the code..
From code it sounds like we are trying to skip the PORT 
(PF access-> VF access) function, as new SRIOV usage model is introduced.
Probably we can skip it early in this function or even skip this function
directly. It doesn't matter it's 1:N or 1:1, we always want to keep PF
access to port, right?

> 
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> ---
>  drivers/fpga/dfl.c | 10 ++++++----
>  drivers/fpga/dfl.h |  2 ++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 712c53363fda..b95b29c5c81d 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -1707,11 +1707,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 ((dfl_has_port_connected_afu(cdev) &&

Could we really use this as indication for which SRIOV model of hardware?

> +	     cdev->released_port_num != num_vfs)) {
>  		ret = -EINVAL;
>  		goto done;
>  	}
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index bc56b7e8c01b..83c2c50975e5 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -471,6 +471,8 @@ void dfl_fpga_enum_info_free(struct
> dfl_fpga_enum_info *info);
>  #define DFL_PORT_CONNECT_BITS  MAX_DFL_FPGA_PORT_NUM
>  #define DFL_FEAT_PORT_CONNECT_MASK ((1UL <<
> (DFL_PORT_CONNECT_BITS)) - 1)
> 
> +#define dfl_has_port_connected_afu(cdev) ((cdev)->flags &
> DFL_FEAT_PORT_CONNECT_MASK)
> +
>  /**
>   * struct dfl_fpga_cdev - container device of DFL based FPGA
>   *
> --
> 2.26.2

RE: [PATCH v6 2/6] fpga: dfl: tracking port conntected with AFU

[Zhang, Tianfei]

> -----Original Message-----
> From: Wu, Hao <hao.wu@intel.com>
> Sent: Thursday, March 17, 2022 4:26 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; linux-kernel@vger.kernel.org;
> rdunlap@infradead.org
> Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Subject: RE: [PATCH v6 2/6] fpga: dfl: tracking port conntected with AFU
> 
> > -----Original Message-----
> > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > Sent: Wednesday, March 16, 2022 3:08 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; linux-kernel@vger.kernel.org;
> > rdunlap@infradead.org
> > Cc: corbet@lwn.net; Zhang, Tianfei <tianfei.zhang@intel.com>; Matthew
> > Gerlach <matthew.gerlach@linux.intel.com>
> > Subject: [PATCH v6 2/6] fpga: dfl: tracking port conntected with AFU
> >
> 
> "conntected" -> "connected"
> 
> > From: Tianfei zhang <tianfei.zhang@intel.com>
> >
> > Introducing flags in dfl_fpga_cdev to track extensions or new features
> > discovered during DFL enumeration. It uses some lowest bits of flags
> > to track the port status which the AFU was connected to port device or
> > not. In 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.
> 
> It's "Port with AFU vs "Port without AFU", so why we have this flag in container
> device not for each port device?

I think this is a global flag during the DFL enumeration. 
After the DFL enumeration, we can know that the Port connected with AFU or not.

> and probably you need to describe why we need this flag. 

The next patch will use those flags for VF creation.

> What I am thinking is
> that port driver can use this flag to decide if afu related interface will be visible
> or not...does it make more sense to have this flag for port device itself?

The different between "port with AFU" and "port without AFU" was that the AFU MMIO space add into 
AFU driver or not. In "port without AFU" case, we cannot access the AFU via mmap API. But for both cases,
we still can access the port device by "/dev/dfl-port.h" device nodes.

The flag was used for VF creation. If we want to put the flag in port device, I think it only can add the flag in port device's dfl_feature_platform_data,
but I don't think this is a good idea. 

> 
> 
> >
> > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> > ---
> >  drivers/fpga/dfl.c | 11 ++++++++++-
> >  drivers/fpga/dfl.h | 12 ++++++++++++
> >  2 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index
> > 599bb21d86af..712c53363fda 100644
> > --- a/drivers/fpga/dfl.c
> > +++ b/drivers/fpga/dfl.c
> > @@ -1124,8 +1124,10 @@ 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;
> > +	u32 port;
> >  	u16 id;
> >  	u64 v;
> >
> > @@ -1160,8 +1162,15 @@ 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) {
> > +			port = FIELD_GET(PORT_CAP_PORT_NUM,
> > +					 readq(binfo->ioaddr +
> > PORT_HDR_CAP));
> > +			cdev->flags |= dfl_feat_port_connect_afu(port);
> > +		}
> > +
> >  		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..bc56b7e8c01b 100644
> > --- a/drivers/fpga/dfl.h
> > +++ b/drivers/fpga/dfl.h
> > @@ -461,6 +461,16 @@ 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);
> >
> > +/*
> > + * Bitfields in flags of dfl_fpga_cdev.
> > + *
> > + * 0 - (DFL_PORT_CONNECT_BITS -1): AFU was connected with Port device.
> > + * DFL_PORT_CONNECT_BITS - 63: reserved.
> > + */
> > +#define dfl_feat_port_connect_afu(port) (BIT_ULL(port)) #define
> > +DFL_PORT_CONNECT_BITS  MAX_DFL_FPGA_PORT_NUM #define
> > +DFL_FEAT_PORT_CONNECT_MASK ((1UL <<
> > (DFL_PORT_CONNECT_BITS)) - 1)
> > +
> >  /**
> >   * struct dfl_fpga_cdev - container device of DFL based FPGA
> >   *
> > @@ -470,6 +480,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.
> > + * @flags: extensions discovered during DFL enumeration.
> >   */
> >  struct dfl_fpga_cdev {
> >  	struct device *parent;
> > @@ -478,6 +489,7 @@ struct dfl_fpga_cdev {
> >  	struct mutex lock;
> >  	struct list_head port_dev_list;
> >  	int released_port_num;
> > +	u64 flags;
> >  };
> >
> >  struct dfl_fpga_cdev *
> > --
> > 2.26.2

RE: [PATCH v6 3/6] fpga: dfl: check released_port_num and num_vfs for legacy model

[Zhang, Tianfei]

> -----Original Message-----
> From: Wu, Hao <hao.wu@intel.com>
> Sent: Thursday, March 17, 2022 4:49 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; linux-kernel@vger.kernel.org;
> rdunlap@infradead.org
> Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Subject: RE: [PATCH v6 3/6] fpga: dfl: check released_port_num and num_vfs for
> legacy model
> 
> > Subject: [PATCH v6 3/6] fpga: dfl: check released_port_num and num_vfs
> > for legacy model
> >
> > From: Tianfei zhang <tianfei.zhang@intel.com>
> >
> > In OFS legacy model, there is 1:1 mapping for Port device and VF, so
> > it need to check the number of released port match the number of VFs
> > or not. But in "Multiple VFs per PR slot" model, there is 1:N mapping
> > for the Port device and VFs.
> 
> The title and commit message seems not matching the code..
> From code it sounds like we are trying to skip the PORT (PF access-> VF access)
> function, as new SRIOV usage model is introduced.
> Probably we can skip it early in this function or even skip this function directly. It
> doesn't matter it's 1:N or 1:1, we always want to keep PF access to port, right?

For 1:1, there is 1 port mapping the 1 VFs, and we check the num_vfs was equal  with released ports or not.
But for 1:N, it break this checking.

> 
> >
> > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> > ---
> >  drivers/fpga/dfl.c | 10 ++++++----
> >  drivers/fpga/dfl.h |  2 ++
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index
> > 712c53363fda..b95b29c5c81d 100644
> > --- a/drivers/fpga/dfl.c
> > +++ b/drivers/fpga/dfl.c
> > @@ -1707,11 +1707,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 ((dfl_has_port_connected_afu(cdev) &&
> 
> Could we really use this as indication for which SRIOV model of hardware?
> 
> > +	     cdev->released_port_num != num_vfs)) {
> >  		ret = -EINVAL;
> >  		goto done;
> >  	}
> > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h index
> > bc56b7e8c01b..83c2c50975e5 100644
> > --- a/drivers/fpga/dfl.h
> > +++ b/drivers/fpga/dfl.h
> > @@ -471,6 +471,8 @@ void dfl_fpga_enum_info_free(struct
> > dfl_fpga_enum_info *info);  #define DFL_PORT_CONNECT_BITS
> > MAX_DFL_FPGA_PORT_NUM  #define DFL_FEAT_PORT_CONNECT_MASK
> ((1UL <<
> > (DFL_PORT_CONNECT_BITS)) - 1)
> >
> > +#define dfl_has_port_connected_afu(cdev) ((cdev)->flags &
> > DFL_FEAT_PORT_CONNECT_MASK)
> > +
> >  /**
> >   * struct dfl_fpga_cdev - container device of DFL based FPGA
> >   *
> > --
> > 2.26.2

RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.

[Zhang, Tianfei]

> -----Original Message-----
> From: Zhang, Tianfei <tianfei.zhang@intel.com>
> Sent: Thursday, March 17, 2022 4:33 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;
> linux-kernel@vger.kernel.org; rdunlap@infradead.org
> Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> 
> 
> 
> > -----Original Message-----
> > From: Wu, Hao <hao.wu@intel.com>
> > Sent: Thursday, March 17, 2022 4:18 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;
> > linux-kernel@vger.kernel.org; rdunlap@infradead.org
> > Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> >
> > > > -----Original Message-----
> > > > From: Wu, Hao <hao.wu@intel.com>
> > > > Sent: Thursday, March 17, 2022 10:05 AM
> > > > 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;
> > > > linux-kernel@vger.kernel.org; rdunlap@infradead.org
> > > > Cc: corbet@lwn.net; Matthew Gerlach
> > > > <matthew.gerlach@linux.intel.com>
> > > > Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> > > >
> > > > > -----Original Message-----
> > > > > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > > > > Sent: Wednesday, March 16, 2022 3:08 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; linux-kernel@vger.kernel.org;
> > > > > rdunlap@infradead.org
> > > > > Cc: corbet@lwn.net; Matthew Gerlach
> > > > > <matthew.gerlach@linux.intel.com>;
> > > > > Zhang, Tianfei <tianfei.zhang@intel.com>
> > > > > Subject: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> > > > >
> > > > > From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > > > >
> > > > > In OFS, each PR slot (AFU) has one port device which include
> > > > > Port control, Port user clock control and Port errors. In legacy
> > > > > model, the AFU MMIO space was connected with Port device, so
> > > > > from port device point of view, there is a bar space associated with this
> port device.
> > > > > But in "Multiple VFs per PR slot" model, the AFU MMIO space was
> > > > > not connected with Port device. The BarID (3bits field) in
> > > > > PORTn_OFFSET register indicates which PCI bar space associated
> > > > > with this port device, the value 0b111 (FME_HDR_NO_PORT_BAR)
> > > > > means that no PCI bar for this port device.
> > > >
> > > > The commit message is not matching the change, it's not related to AFU...
> > > >
> > > > Current usage (FME DFL and PORT DFL are not linked together)
> > >
> > > This usage is only on Intel PAC N3000 and N5000 card.
> > > In my understand, the space of Port can put into any PCI bar space.
> > > In the previous use case, the space of port was located on Bar 2.
> > > For OFS, it allows the port without specific bar space.
> >
> > I didn't understand what you mean. Without your change, existing
> > driver supports Port in any BAR indicated by PORTn_OFFSET, it's fine
> > you put Port to BAR 0, or same BAR as FME. What do you mean by "port
> > without specific bar space"?
> 
> "port with specific bar space" means that the port has a dedicated bar space,
> including the DFL, AFU, this is use case in N3000/N5000 card.
> 
> "port without specific bar space" means the port without specific bar space, and
> the Port linked with FME for DFL perspective.
> 
> >
> > >
> > > >
> > > > FME DFL
> > > > PORT DFL (located by FME's PORTn_OFFSET register, BAR + offset)
> > > >
> > > > Your proposed new usage is (FME DFL and PORT DFL are linked
> > > > together)
> > > >
> > > > FME DFL -> PORT DFL
> > > > So FME's PORTn_OFFSET can be marked, then driver could skip it.
> > > >
> > > > Is my understanding correct? If yes, please update your title and
> > > > commit message, and add some comments in code as well.
> > >
> > > From DLF perspective, I think it is yes.
> > >
> > > How about the title:  "fpga: dfl: Allow Port and FME's DFL link together" ?
> >
> > "Allow Port to be linked to FME's DFL" should be better, as we don't
> > encourage that people to connect FME DFL to Port DFL or any mixed order.
> 
> Looks good.
> 
> >
> > >
> > > I will also add some comments in code.
> > > Here is the new git commit for this patch, any comments?
> > >
> > > In previous FPGA platform like Intel PAC N3000 and N5000, The BarID
> > > (3bits field) in PORTn_OFFSET register indicated which PCI bar space
> > > was associated with this port device. In this case, the DFL of Port
> > > device was located in the specific PCI bar space, and then the FME
> > > and Port's DFL were not linked. But in OFS, we extend the usage, it
> > > allows the FME and Port's DFL  linked together when there was no
> > > local PCI bar space specified by the Port device. The value 0b111
> > > (FME_HDR_NO_PORT_BAR) of BarID means that no specific PCI bar space
> > > was associated with the port device.
> >
> > Currently we use PORTn_OFFSET to locate PORT DFLs, and PORT DFLs are
> > not connected FME DFL. But for some cases (e.g. Intel Open FPGA Stack
> > device), PORT DFLs are connected to FME DFL directly, so we don't need
> > to search PORT DFLs via PORTn_OFFSET again. If BAR value of
> > PORTn_OFFSET is 0x7 (FME_PORT_OFST_BAR_SKIP/INVALID - depends the
> > description added to DFL
> > spec) then driver will skip searching the DFL for that port.
>

I like to split the patch to a sperate patch, so this patchset will divide into 2, one for port bar, one for VF creation.