All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] NTB function for PCIe RC to EP connection
@ 2022-02-15  5:38 Frank Li
  2022-02-15  5:38 ` [RFC PATCH 1/4] PCI: designware-ep: Allow pcie_ep_set_bar change inbound map address Frank Li
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Frank Li @ 2022-02-15  5:38 UTC (permalink / raw)
  To: linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, lznuaa, hongxing.zhu

This implement NTB function for PCIe EP to RC connections.
The existed ntb epf need two PCI EPs and two PCI Host. 

This just need EP to RC connections.

    ┌────────────┐         ┌─────────────────────────────────────┐
    │            │         │                                     │
    ├────────────┤         │                      ┌──────────────┤
    │ NTB        │         │                      │ NTB          │
    │ NetDev     │         │                      │ NetDev       │
    ├────────────┤         │                      ├──────────────┤
    │ NTB        │         │                      │ NTB          │
    │ Transfer   │         │                      │ Transfer     │
    ├────────────┤         │                      ├──────────────┤
    │            │         │                      │              │
    │  PCI NTB   │         │                      │              │
    │    EPF     │         │                      │              │
    │   Driver   │         │                      │ PCI Virtual  │
    │            │         ├───────────────┐      │ NTB Driver   │
    │            │         │ PCI EP NTB    │◄────►│              │
    │            │         │  FN Driver    │      │              │
    ├────────────┤         ├───────────────┤      ├──────────────┤
    │            │         │               │      │              │
    │  PCI BUS   │ ◄─────► │  PCI EP BUS   │      │  Virtual PCI │
    │            │  PCI    │               │      │     BUS      │
    └────────────┘         └───────────────┴──────┴──────────────┘
        PCI RC                        PCI EP



Frank Li (4):
  PCI: designware-ep: Allow pcie_ep_set_bar change inbound map address
  NTB: epf: Added more flexible memory map method
  NTB: EPF: support NTB transfer between PCI RC and EP connection
  Documentation: PCI: Add specification for the PCI vNTB function device

 Documentation/PCI/endpoint/index.rst          |    2 +
 .../PCI/endpoint/pci-vntb-function.rst        |  126 ++
 Documentation/PCI/endpoint/pci-vntb-howto.rst |  161 ++
 drivers/ntb/hw/epf/ntb_hw_epf.c               |   48 +-
 .../pci/controller/dwc/pcie-designware-ep.c   |   10 +-
 drivers/pci/endpoint/functions/Kconfig        |   11 +
 drivers/pci/endpoint/functions/Makefile       |    1 +
 drivers/pci/endpoint/functions/pci-epf-vntb.c | 1425 +++++++++++++++++
 8 files changed, 1770 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/PCI/endpoint/pci-vntb-function.rst
 create mode 100644 Documentation/PCI/endpoint/pci-vntb-howto.rst
 create mode 100644 drivers/pci/endpoint/functions/pci-epf-vntb.c

-- 
2.24.0.rc1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RFC PATCH 1/4] PCI: designware-ep: Allow pcie_ep_set_bar change inbound map address
  2022-02-15  5:38 [RFC PATCH 0/4] NTB function for PCIe RC to EP connection Frank Li
@ 2022-02-15  5:38 ` Frank Li
  2022-02-17 21:53   ` Bjorn Helgaas
  2022-02-15  5:38 ` [RFC PATCH 2/4] NTB: epf: Added more flexible memory map method Frank Li
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Frank Li @ 2022-02-15  5:38 UTC (permalink / raw)
  To: linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, lznuaa, hongxing.zhu

ntb_transfer will set memory map windows after probe.
So the inbound map address need be updated dynamtically.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 998b698f40858..cad5d9ea7cc6c 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -161,7 +161,11 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no,
 	u32 free_win;
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 
-	free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows);
+	if (!ep->bar_to_atu[bar])
+		free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows);
+	else
+		free_win = ep->bar_to_atu[bar];
+
 	if (free_win >= pci->num_ib_windows) {
 		dev_err(pci->dev, "No free inbound window\n");
 		return -EINVAL;
@@ -215,6 +219,7 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	dw_pcie_disable_atu(pci, atu_index, DW_PCIE_REGION_INBOUND);
 	clear_bit(atu_index, ep->ib_window_map);
 	ep->epf_bar[bar] = NULL;
+	ep->bar_to_atu[bar] = 0;
 }
 
 static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
@@ -244,6 +249,9 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	if (ret)
 		return ret;
 
+	if (ep->epf_bar[bar])
+		return 0;
+
 	dw_pcie_dbi_ro_wr_en(pci);
 
 	dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 2/4] NTB: epf: Added more flexible memory map method
  2022-02-15  5:38 [RFC PATCH 0/4] NTB function for PCIe RC to EP connection Frank Li
  2022-02-15  5:38 ` [RFC PATCH 1/4] PCI: designware-ep: Allow pcie_ep_set_bar change inbound map address Frank Li
@ 2022-02-15  5:38 ` Frank Li
  2022-02-17 21:59   ` Bjorn Helgaas
  2022-02-15  5:38 ` [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection Frank Li
  2022-02-15  5:38 ` [RFC PATCH 4/4] Documentation: PCI: Add specification for the PCI vNTB function device Frank Li
  3 siblings, 1 reply; 15+ messages in thread
From: Frank Li @ 2022-02-15  5:38 UTC (permalink / raw)
  To: linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, lznuaa, hongxing.zhu

Supported below memory map method

bar 0: config and spad data
bar 2: door bell
bar 4: memory map windows

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/ntb/hw/epf/ntb_hw_epf.c | 48 ++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
index b019755e4e21b..3ece49cb18ffa 100644
--- a/drivers/ntb/hw/epf/ntb_hw_epf.c
+++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
@@ -45,7 +45,6 @@
 
 #define NTB_EPF_MIN_DB_COUNT	3
 #define NTB_EPF_MAX_DB_COUNT	31
-#define NTB_EPF_MW_OFFSET	2
 
 #define NTB_EPF_COMMAND_TIMEOUT	1000 /* 1 Sec */
 
@@ -67,6 +66,7 @@ struct ntb_epf_dev {
 	enum pci_barno ctrl_reg_bar;
 	enum pci_barno peer_spad_reg_bar;
 	enum pci_barno db_reg_bar;
+	enum pci_barno mw_bar;
 
 	unsigned int mw_count;
 	unsigned int spad_count;
@@ -92,6 +92,8 @@ struct ntb_epf_data {
 	enum pci_barno peer_spad_reg_bar;
 	/* BAR that contains Doorbell region and Memory window '1' */
 	enum pci_barno db_reg_bar;
+	/* BAR that contains memory windows*/
+	enum pci_barno mw_bar;
 };
 
 static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command,
@@ -411,7 +413,7 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx,
 		return -EINVAL;
 	}
 
-	bar = idx + NTB_EPF_MW_OFFSET;
+	bar = idx + ndev->mw_bar;
 
 	mw_size = pci_resource_len(ntb->pdev, bar);
 
@@ -453,7 +455,7 @@ static int ntb_epf_peer_mw_get_addr(struct ntb_dev *ntb, int idx,
 	if (idx == 0)
 		offset = readl(ndev->ctrl_reg + NTB_EPF_MW1_OFFSET);
 
-	bar = idx + NTB_EPF_MW_OFFSET;
+	bar = idx + ndev->mw_bar;
 
 	if (base)
 		*base = pci_resource_start(ndev->ntb.pdev, bar) + offset;
@@ -565,6 +567,7 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev,
 			    struct pci_dev *pdev)
 {
 	struct device *dev = ndev->dev;
+	size_t spad_sz, spad_off;
 	int ret;
 
 	pci_set_drvdata(pdev, ndev);
@@ -599,10 +602,16 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev,
 		goto err_dma_mask;
 	}
 
-	ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0);
-	if (!ndev->peer_spad_reg) {
-		ret = -EIO;
-		goto err_dma_mask;
+	if (ndev->peer_spad_reg_bar) {
+		ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0);
+		if (!ndev->peer_spad_reg) {
+			ret = -EIO;
+			goto err_dma_mask;
+		}
+	} else {
+		spad_sz = 4 * readl(ndev->ctrl_reg + NTB_EPF_SPAD_COUNT);
+		spad_off = readl(ndev->ctrl_reg + NTB_EPF_SPAD_OFFSET);
+		ndev->peer_spad_reg = ndev->ctrl_reg + spad_off  + spad_sz;
 	}
 
 	ndev->db_reg = pci_iomap(pdev, ndev->db_reg_bar, 0);
@@ -657,6 +666,7 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev,
 	enum pci_barno peer_spad_reg_bar = BAR_1;
 	enum pci_barno ctrl_reg_bar = BAR_0;
 	enum pci_barno db_reg_bar = BAR_2;
+	enum pci_barno mw_bar = BAR_2;
 	struct device *dev = &pdev->dev;
 	struct ntb_epf_data *data;
 	struct ntb_epf_dev *ndev;
@@ -671,17 +681,16 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev,
 
 	data = (struct ntb_epf_data *)id->driver_data;
 	if (data) {
-		if (data->peer_spad_reg_bar)
-			peer_spad_reg_bar = data->peer_spad_reg_bar;
-		if (data->ctrl_reg_bar)
-			ctrl_reg_bar = data->ctrl_reg_bar;
-		if (data->db_reg_bar)
-			db_reg_bar = data->db_reg_bar;
+		peer_spad_reg_bar = data->peer_spad_reg_bar;
+		ctrl_reg_bar = data->ctrl_reg_bar;
+		db_reg_bar = data->db_reg_bar;
+		mw_bar = data->mw_bar;
 	}
 
 	ndev->peer_spad_reg_bar = peer_spad_reg_bar;
 	ndev->ctrl_reg_bar = ctrl_reg_bar;
 	ndev->db_reg_bar = db_reg_bar;
+	ndev->mw_bar = mw_bar;
 	ndev->dev = dev;
 
 	ntb_epf_init_struct(ndev, pdev);
@@ -729,6 +738,14 @@ static const struct ntb_epf_data j721e_data = {
 	.ctrl_reg_bar = BAR_0,
 	.peer_spad_reg_bar = BAR_1,
 	.db_reg_bar = BAR_2,
+	.mw_bar = BAR_2,
+};
+
+static const struct ntb_epf_data mx8_data = {
+	.ctrl_reg_bar = BAR_0,
+	.peer_spad_reg_bar = BAR_0,
+	.db_reg_bar = BAR_2,
+	.mw_bar = BAR_4,
 };
 
 static const struct pci_device_id ntb_epf_pci_tbl[] = {
@@ -737,6 +754,11 @@ static const struct pci_device_id ntb_epf_pci_tbl[] = {
 		.class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00,
 		.driver_data = (kernel_ulong_t)&j721e_data,
 	},
+	{
+		PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, 0x0809),
+		.class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00,
+		.driver_data = (kernel_ulong_t)&mx8_data,
+	},
 	{ },
 };
 
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection
  2022-02-15  5:38 [RFC PATCH 0/4] NTB function for PCIe RC to EP connection Frank Li
  2022-02-15  5:38 ` [RFC PATCH 1/4] PCI: designware-ep: Allow pcie_ep_set_bar change inbound map address Frank Li
  2022-02-15  5:38 ` [RFC PATCH 2/4] NTB: epf: Added more flexible memory map method Frank Li
@ 2022-02-15  5:38 ` Frank Li
  2022-02-15  7:57   ` kernel test robot
                     ` (2 more replies)
  2022-02-15  5:38 ` [RFC PATCH 4/4] Documentation: PCI: Add specification for the PCI vNTB function device Frank Li
  3 siblings, 3 replies; 15+ messages in thread
From: Frank Li @ 2022-02-15  5:38 UTC (permalink / raw)
  To: linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, lznuaa, hongxing.zhu

Add NTB function driver and virtual PCI BUS and Virtual NTB driver
to implement communication between PCIe RC and PCIe EP devices

┌────────────┐         ┌─────────────────────────────────────┐
│            │         │                                     │
├────────────┤         │                      ┌──────────────┤
│ NTB        │         │                      │ NTB          │
│ NetDev     │         │                      │ NetDev       │
├────────────┤         │                      ├──────────────┤
│ NTB        │         │                      │ NTB          │
│ Transfer   │         │                      │ Transfer     │
├────────────┤         │                      ├──────────────┤
│            │         │                      │              │
│  PCI NTB   │         │                      │              │
│    EPF     │         │                      │              │
│   Driver   │         │                      │ PCI Virtual  │
│            │         ├───────────────┐      │ NTB Driver   │
│            │         │ PCI EP NTB    │◄────►│              │
│            │         │  FN Driver    │      │              │
├────────────┤         ├───────────────┤      ├──────────────┤
│            │         │               │      │              │
│  PCI BUS   │ ◄─────► │  PCI EP BUS   │      │  Virtual PCI │
│            │  PCI    │               │      │     BUS      │
└────────────┘         └───────────────┴──────┴──────────────┘
    PCI RC                        PCI EP

This driver include 3 part:
 1 PCI EP NTB function driver
 2 Virtual PCI bus
 3 PCI virutal NTB driver, which is loaded only by above virtual pci bus

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/endpoint/functions/Kconfig        |   11 +
 drivers/pci/endpoint/functions/Makefile       |    1 +
 drivers/pci/endpoint/functions/pci-epf-vntb.c | 1425 +++++++++++++++++
 3 files changed, 1437 insertions(+)
 create mode 100644 drivers/pci/endpoint/functions/pci-epf-vntb.c

diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
index 5f1242ca2f4e4..362555b024e8f 100644
--- a/drivers/pci/endpoint/functions/Kconfig
+++ b/drivers/pci/endpoint/functions/Kconfig
@@ -25,3 +25,14 @@ config PCI_EPF_NTB
 	  device tree.
 
 	  If in doubt, say "N" to disable Endpoint NTB driver.
+
+config PCI_EPF_VNTB
+        tristate "PCI Endpoint NTB driver"
+        depends on PCI_ENDPOINT
+        select CONFIGFS_FS
+        help
+          Select this configuration option to enable the Non-Transparent
+          Bridge (NTB) driver for PCI Endpoint. NTB driver implements NTB
+          between PCI host and PCIe Endpoint.
+
+          If in doubt, say "N" to disable Endpoint NTB driver.
diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
index 96ab932a537a2..5c13001deaba1 100644
--- a/drivers/pci/endpoint/functions/Makefile
+++ b/drivers/pci/endpoint/functions/Makefile
@@ -5,3 +5,4 @@
 
 obj-$(CONFIG_PCI_EPF_TEST)		+= pci-epf-test.o
 obj-$(CONFIG_PCI_EPF_NTB)		+= pci-epf-ntb.o
+obj-$(CONFIG_PCI_EPF_VNTB) 		+= pci-epf-vntb.o
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
new file mode 100644
index 0000000000000..ebf7e243eefa4
--- /dev/null
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -0,0 +1,1425 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Endpoint Function Driver to implement Non-Transparent Bridge functionality
+ * Between PCI RC and EP
+ *
+ * Copyright (C) 2020 Texas Instruments
+ * Copyright (C) 2022 NXP
+ *
+ * Based on pci-epf-ntb.c
+ * Author: Frank Li <Frank.Li@nxp.com>
+ * Author: Kishon Vijay Abraham I <kishon@ti.com>
+ */
+
+/**
+ * +------------+         +---------------------------------------+
+ * |            |         |                                       |
+ * +------------+         |                        +--------------+
+ * | NTB        |         |                        | NTB          |
+ * | NetDev     |         |                        | NetDev       |
+ * +------------+         |                        +--------------+
+ * | NTB        |         |                        | NTB          |
+ * | Transfer   |         |                        | Transfer     |
+ * +------------+         |                        +--------------+
+ * |            |         |                        |              |
+ * |  PCI NTB   |         |                        |              |
+ * |    EPF     |         |                        |              |
+ * |   Driver   |         |                        | PCI Virtual  |
+ * |            |         +---------------+        | NTB Driver   |
+ * |            |         | PCI EP NTB    |<------>|              |
+ * |            |         |  FN Driver    |        |              |
+ * +------------+         +---------------+        +--------------+
+ * |            |         |               |        |              |
+ * |  PCI BUS   | <-----> |  PCI EP BUS   |        |  Virtual PCI |
+ * |            |  PCI    |               |        |     BUS      |
+ * +------------+         +---------------+--------+--------------+
+ *     PCI RC                        PCI EP
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include <linux/pci-epc.h>
+#include <linux/pci-epf.h>
+#include <linux/ntb.h>
+
+static struct workqueue_struct *kpcintb_workqueue;
+
+#define COMMAND_CONFIGURE_DOORBELL	1
+#define COMMAND_TEARDOWN_DOORBELL	2
+#define COMMAND_CONFIGURE_MW		3
+#define COMMAND_TEARDOWN_MW		4
+#define COMMAND_LINK_UP			5
+#define COMMAND_LINK_DOWN		6
+
+#define COMMAND_STATUS_OK		1
+#define COMMAND_STATUS_ERROR		2
+
+#define LINK_STATUS_UP			BIT(0)
+
+#define SPAD_COUNT			64
+#define DB_COUNT			4
+#define NTB_MW_OFFSET			2
+#define DB_COUNT_MASK			GENMASK(15, 0)
+#define MSIX_ENABLE			BIT(16)
+#define MAX_DB_COUNT			32
+#define MAX_MW				4
+
+#define VNTB_VID			0x1957
+#define VNTB_PID			0x080A
+
+enum epf_ntb_bar {
+	BAR_CONFIG,
+	BAR_DB,
+	BAR_MW0,
+	BAR_MW1,
+	BAR_MW2,
+};
+
+/*
+ * +--------------------------------------------------+ Base
+ * |                                                  |
+ * |                                                  |
+ * |                                                  |
+ * |          Common Control Register                 |
+ * |                                                  |
+ * |                                                  |
+ * |                                                  |
+ * +-----------------------+--------------------------+ Base+span_offset
+ * |                       |                          |
+ * |    Peer Span Space    |    Span Space            |
+ * |                       |                          |
+ * |                       |                          |
+ * +-----------------------+--------------------------+ Base+span_offset
+ * |                       |                          |     +span_count * 4
+ * |                       |                          |
+ * |     Span Space        |   Peer Span Space        |
+ * |                       |                          |
+ * +-----------------------+--------------------------+
+ *       Virtual PCI             Pcie Endpoint
+ *       NTB Driver               NTB Driver
+ */
+struct epf_ntb_ctrl {
+	u32     command;
+	u32     argument;
+	u16     command_status;
+	u16     link_status;
+	u32     topology;
+	u64     addr;
+	u64     size;
+	u32     num_mws;
+	u32	reserved;
+	u32     spad_offset;
+	u32     spad_count;
+	u32	db_entry_size;
+	u32     db_data[MAX_DB_COUNT];
+	u32     db_offset[MAX_DB_COUNT];
+} __packed;
+
+struct epf_ntb {
+	struct ntb_dev ntb;
+	struct pci_epf *epf;
+	struct config_group group;
+
+	u32 num_mws;
+	u32 db_count;
+	u32 spad_count;
+	u64 mws_size[MAX_MW];
+	u64 db;
+	u32 vbus_number;
+
+	bool linkup;
+	u32 spad_size;
+
+	enum pci_barno epf_ntb_bar[6];
+
+	struct epf_ntb_ctrl *reg;
+
+	phys_addr_t epf_db_phy;
+	void __iomem *epf_db;
+
+	phys_addr_t vpci_mw_phy[MAX_MW];
+	void __iomem *vpci_mw_addr[MAX_MW];
+
+	struct delayed_work cmd_handler;
+};
+
+#define to_epf_ntb(epf_group) container_of((epf_group), struct epf_ntb, group)
+#define ntb_ndev(__ntb) container_of(__ntb, struct epf_ntb, ntb)
+
+static struct pci_epf_header epf_ntb_header = {
+	.vendorid	= PCI_ANY_ID,
+	.deviceid	= PCI_ANY_ID,
+	.baseclass_code	= PCI_BASE_CLASS_MEMORY,
+	.interrupt_pin	= PCI_INTERRUPT_INTA,
+};
+
+/**
+ * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host
+ * @ntb: NTB device that facilitates communication between HOST and VHOST
+ * @link_up: true or false indicating Link is UP or Down
+ *
+ * Once NTB function in HOST invoke ntb_link_enable(),
+ * this NTB function driver will trigger a link event to vhost.
+ */
+static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
+{
+	if (link_up)
+		ntb->reg->link_status |= LINK_STATUS_UP;
+	else
+		ntb->reg->link_status &= ~LINK_STATUS_UP;
+
+	ntb_link_event(&ntb->ntb);
+	return 0;
+}
+
+/**
+ * epf_ntb_configure_mw() - Configure the Outbound Address Space for vhost
+ *   to access the memory window of host
+ * @ntb: NTB device that facilitates communication between host and vhost
+ * @mw: Index of the memory window (either 0, 1, 2 or 3)
+ *
+ *                          EP Outbound Window
+ * +--------+              +-----------+
+ * |        |              |           |
+ * |        |              |           |
+ * |        |              |           |
+ * |        |              |           |
+ * |        |              +-----------+
+ * | Virtual|              | Memory Win|
+ * | NTB    | -----------> |           |
+ * | Driver |              |           |
+ * |        |              +-----------+
+ * |        |              |           |
+ * |        |              |           |
+ * +--------+              +-----------+
+ *  VHost                   PCI EP
+ */
+static int epf_ntb_configure_mw(struct epf_ntb *ntb, u32 mw)
+{
+	phys_addr_t phys_addr;
+	u8 func_no, vfunc_no;
+	u64 addr, size;
+	int ret = 0;
+
+	phys_addr = ntb->vpci_mw_phy[mw];
+	addr = ntb->reg->addr;
+	size = ntb->reg->size;
+
+	func_no = ntb->epf->func_no;
+	vfunc_no = ntb->epf->vfunc_no;
+
+	ret = pci_epc_map_addr(ntb->epf->epc, func_no, vfunc_no, phys_addr, addr, size);
+	if (ret)
+		dev_err(&ntb->epf->epc->dev,
+			"intf: Failed to map memory window %d address\n", mw);
+	return ret;
+}
+
+/**
+ * epf_ntb_teardown_mw() - Teardown the configured OB ATU
+ * @ntb: NTB device that facilitates communication between HOST and vHOST
+ * @mw: Index of the memory window (either 0, 1, 2 or 3)
+ *
+ * Teardown the configured OB ATU configured in epf_ntb_configure_mw() using
+ * pci_epc_unmap_addr()
+ */
+static void epf_ntb_teardown_mw(struct epf_ntb *ntb, u32 mw)
+{
+	pci_epc_unmap_addr(ntb->epf->epc,
+			   ntb->epf->func_no,
+			   ntb->epf->vfunc_no,
+			   ntb->vpci_mw_phy[mw]);
+}
+
+/**
+ * epf_ntb_cmd_handler() - Handle commands provided by the NTB Host
+ * @work: work_struct for the epf_ntb_epc
+ *
+ * Workqueue function that gets invoked for the two epf_ntb_epc
+ * periodically (once every 5ms) to see if it has received any commands
+ * from NTB host. The host can send commands to configure doorbell or
+ * configure memory window or to update link status.
+ */
+static void epf_ntb_cmd_handler(struct work_struct *work)
+{
+	struct epf_ntb_ctrl *ctrl;
+	u32 command, argument;
+	struct epf_ntb *ntb;
+	struct device *dev;
+	int ret;
+	int i;
+
+	ntb = container_of(work, struct epf_ntb, cmd_handler.work);
+
+	for (i = 1; i < ntb->db_count; i++) {
+		if (readl(ntb->epf_db + i * 4)) {
+			if (readl(ntb->epf_db + i * 4))
+				ntb->db |= 1 << (i - 1);
+
+			ntb_db_event(&ntb->ntb, i);
+			writel(0, ntb->epf_db + i * 4);
+		}
+	}
+
+	ctrl = ntb->reg;
+	command = ctrl->command;
+	if (!command)
+		goto reset_handler;
+	argument = ctrl->argument;
+
+	ctrl->command = 0;
+	ctrl->argument = 0;
+
+	ctrl = ntb->reg;
+	dev = &ntb->epf->dev;
+
+	switch (command) {
+	case COMMAND_CONFIGURE_DOORBELL:
+		ctrl->command_status = COMMAND_STATUS_OK;
+		break;
+	case COMMAND_TEARDOWN_DOORBELL:
+		ctrl->command_status = COMMAND_STATUS_OK;
+		break;
+	case COMMAND_CONFIGURE_MW:
+		ret = epf_ntb_configure_mw(ntb, argument);
+		if (ret < 0)
+			ctrl->command_status = COMMAND_STATUS_ERROR;
+		else
+			ctrl->command_status = COMMAND_STATUS_OK;
+		break;
+	case COMMAND_TEARDOWN_MW:
+		epf_ntb_teardown_mw(ntb, argument);
+		ctrl->command_status = COMMAND_STATUS_OK;
+		break;
+	case COMMAND_LINK_UP:
+		ntb->linkup = true;
+		ret = epf_ntb_link_up(ntb, true);
+		if (ret < 0)
+			ctrl->command_status = COMMAND_STATUS_ERROR;
+		else
+			ctrl->command_status = COMMAND_STATUS_OK;
+		goto reset_handler;
+	case COMMAND_LINK_DOWN:
+		ntb->linkup = false;
+		ret = epf_ntb_link_up(ntb, false);
+		if (ret < 0)
+			ctrl->command_status = COMMAND_STATUS_ERROR;
+		else
+			ctrl->command_status = COMMAND_STATUS_OK;
+		break;
+	default:
+		dev_err(dev, "intf UNKNOWN command: %d\n", command);
+		break;
+	}
+
+reset_handler:
+	queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
+			   msecs_to_jiffies(5));
+}
+
+/**
+ * epf_ntb_config_sspad_bar_clear() - Clear Config + Self scratchpad BAR
+ * @ntb_epc: EPC associated with one of the HOST which holds peer's outbound
+ *	     address.
+ *
+ * Clear BAR0 of EP CONTROLLER 1 which contains the HOST1's config and
+ * self scratchpad region (removes inbound ATU configuration). While BAR0 is
+ * the default self scratchpad BAR, an NTB could have other BARs for self
+ * scratchpad (because of reserved BARs). This function can get the exact BAR
+ * used for self scratchpad from epf_ntb_bar[BAR_CONFIG].
+ *
+ * Please note the self scratchpad region and config region is combined to
+ * a single region and mapped using the same BAR. Also note HOST2's peer
+ * scratchpad is HOST1's self scratchpad.
+ */
+static void epf_ntb_config_sspad_bar_clear(struct epf_ntb *ntb)
+{
+	struct pci_epf_bar *epf_bar;
+	enum pci_barno barno;
+
+	barno = ntb->epf_ntb_bar[BAR_CONFIG];
+	epf_bar = &ntb->epf->bar[barno];
+
+	pci_epc_clear_bar(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no, epf_bar);
+}
+
+/**
+ * epf_ntb_config_sspad_bar_set() - Set Config + Self scratchpad BAR
+ * @ntb: NTB device that facilitates communication between HOST and vHOST
+ *
+ * Map BAR0 of EP CONTROLLER 1 which contains the HOST1's config and
+ * self scratchpad region.
+ *
+ * Please note the self scratchpad region and config region is combined to
+ * a single region and mapped using the same BAR.
+ */
+static int epf_ntb_config_sspad_bar_set(struct epf_ntb *ntb)
+{
+	struct pci_epf_bar *epf_bar;
+	enum pci_barno barno;
+	u8 func_no, vfunc_no;
+	struct device *dev;
+	int ret;
+
+	dev = &ntb->epf->dev;
+	func_no = ntb->epf->func_no;
+	vfunc_no = ntb->epf->vfunc_no;
+	barno = ntb->epf_ntb_bar[BAR_CONFIG];
+	epf_bar = &ntb->epf->bar[barno];
+
+	ret = pci_epc_set_bar(ntb->epf->epc, func_no, vfunc_no, epf_bar);
+	if (ret) {
+		dev_err(dev, "inft: Config/Status/SPAD BAR set failed\n");
+		return ret;
+	}
+	return 0;
+}
+
+/**
+ * epf_ntb_config_spad_bar_free() - Free the physical memory associated with
+ *   config + scratchpad region
+ * @ntb: NTB device that facilitates communication between HOST and vHOST
+ */
+static void epf_ntb_config_spad_bar_free(struct epf_ntb *ntb)
+{
+	enum pci_barno barno;
+
+	barno = ntb->epf_ntb_bar[BAR_CONFIG];
+	pci_epf_free_space(ntb->epf, ntb->reg, barno, 0);
+}
+
+/**
+ * epf_ntb_config_spad_bar_alloc() - Allocate memory for config + scratchpad
+ *   region
+ * @ntb: NTB device that facilitates communication between HOST1 and HOST2
+ *
+ * Allocate the Local Memory mentioned in the above diagram. The size of
+ * CONFIG REGION is sizeof(struct epf_ntb_ctrl) and size of SCRATCHPAD REGION
+ * is obtained from "spad-count" configfs entry.
+ */
+static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
+{
+	size_t align;
+	enum pci_barno barno;
+	struct epf_ntb_ctrl *ctrl;
+	u32 spad_size, ctrl_size;
+	u64 size;
+	struct pci_epf *epf = ntb->epf;
+	struct device *dev = &epf->dev;
+	u32 spad_count;
+	void *base;
+	int i;
+	const struct pci_epc_features *epc_features = pci_epc_get_features(epf->epc,
+								epf->func_no,
+								epf->vfunc_no);
+	barno = ntb->epf_ntb_bar[BAR_CONFIG];
+	size = epc_features->bar_fixed_size[barno];
+	align = epc_features->align;
+
+	if ((!IS_ALIGNED(size, align)))
+		return -EINVAL;
+
+	spad_count = ntb->spad_count;
+
+	ctrl_size = sizeof(struct epf_ntb_ctrl);
+	spad_size = 2 * spad_count * 4;
+
+	if (!align) {
+		ctrl_size = roundup_pow_of_two(ctrl_size);
+		spad_size = roundup_pow_of_two(spad_size);
+	} else {
+		ctrl_size = ALIGN(ctrl_size, align);
+		spad_size = ALIGN(spad_size, align);
+	}
+
+	if (!size)
+		size = ctrl_size + spad_size;
+	else if (size < ctrl_size + spad_size)
+		return -EINVAL;
+
+	base = pci_epf_alloc_space(epf, size, barno, align, 0);
+	if (!base) {
+		dev_err(dev, "intf: Config/Status/SPAD alloc region fail\n");
+		return -ENOMEM;
+	}
+
+	ntb->reg = base;
+
+	ctrl = ntb->reg;
+	ctrl->spad_offset = ctrl_size;
+
+	ctrl->spad_count = spad_count;
+	ctrl->num_mws = ntb->num_mws;
+	ntb->spad_size = spad_size;
+
+	ctrl->db_entry_size = 4;
+
+	for (i = 0; i < ntb->db_count; i++) {
+		ntb->reg->db_data[i] = 1 + i;
+		ntb->reg->db_offset[i] = 0;
+	}
+
+	return 0;
+}
+
+/**
+ * epf_ntb_configure_interrupt() - Configure MSI/MSI-X capaiblity
+ * @ntb: NTB device that facilitates communication between HOST and vHOST
+ *
+ * Configure MSI/MSI-X capability for each interface with number of
+ * interrupts equal to "db_count" configfs entry.
+ */
+static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
+{
+	const struct pci_epc_features *epc_features;
+	bool msix_capable, msi_capable;
+	u8 func_no, vfunc_no;
+	struct device *dev;
+	u32 db_count;
+	int ret;
+
+	dev = &ntb->epf->dev;
+
+	epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);
+	msix_capable = epc_features->msix_capable;
+	msi_capable = epc_features->msi_capable;
+
+	if (!(msix_capable || msi_capable)) {
+		dev_err(dev, "MSI or MSI-X is required for doorbell\n");
+		return -EINVAL;
+	}
+
+	func_no = ntb->epf->func_no;
+	vfunc_no = ntb->epf->vfunc_no;
+
+	db_count = ntb->db_count;
+	if (db_count > MAX_DB_COUNT) {
+		dev_err(dev, "DB count cannot be more than %d\n", MAX_DB_COUNT);
+		return -EINVAL;
+	}
+
+	ntb->db_count = db_count;
+
+	if (msi_capable) {
+		ret = pci_epc_set_msi(ntb->epf->epc, func_no, vfunc_no, 16);
+		if (ret) {
+			dev_err(dev, "intf: MSI configuration failed\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * epf_ntb_db_bar_init() - Configure Doorbell window BARs
+ * @ntb: NTB device that facilitates communication between HOST and vHOST
+ *
+ */
+static int epf_ntb_db_bar_init(struct epf_ntb *ntb)
+{
+	const struct pci_epc_features *epc_features;
+	u32 align;
+	struct device *dev = &ntb->epf->dev;
+	int ret;
+	struct pci_epf_bar *epf_bar;
+	void __iomem *mw_addr;
+	enum pci_barno barno;
+	size_t size = 4 * ntb->db_count;
+
+	epc_features = pci_epc_get_features(ntb->epf->epc,
+					    ntb->epf->func_no,
+					    ntb->epf->vfunc_no);
+	align = epc_features->align;
+
+	if (size < 128)
+		size = 128;
+
+	if (align)
+		size = ALIGN(size, align);
+	else
+		size = roundup_pow_of_two(size);
+
+	barno = ntb->epf_ntb_bar[BAR_DB];
+
+	mw_addr = pci_epf_alloc_space(ntb->epf, size, barno, align, 0);
+	if (!mw_addr) {
+		dev_err(dev, "intf: Failed to allocate OB address\n");
+		return -ENOMEM;
+	}
+
+	ntb->epf_db = mw_addr;
+
+	epf_bar = &ntb->epf->bar[barno];
+
+	ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no, epf_bar);
+	if (ret) {
+		dev_err(dev, "intf: DoorBell BAR set failed\n");
+			goto err_alloc_peer_mem;
+	}
+	return ret;
+
+err_alloc_peer_mem:
+	pci_epc_mem_free_addr(ntb->epf->epc, epf_bar->phys_addr, mw_addr, epf_bar->size);
+	return -1;
+}
+
+/**
+ * epf_ntb_db_bar_clear() - Clear doorbell BAR and free memory
+ *   allocated in peers outbound address space
+ * @ntb: NTB device that facilitates communication between HOST and vHOST
+ */
+static void epf_ntb_db_bar_clear(struct epf_ntb *ntb)
+{
+	enum pci_barno barno;
+
+	barno = ntb->epf_ntb_bar[BAR_DB];
+	pci_epf_free_space(ntb->epf, ntb->epf_db, barno, 0);
+	pci_epc_clear_bar(ntb->epf->epc,
+			  ntb->epf->func_no,
+			  ntb->epf->vfunc_no,
+			  &ntb->epf->bar[barno]);
+}
+
+/**
+ * epf_ntb_mw_bar_init() - Configure Memory window BARs
+ * @ntb: NTB device that facilitates communication between HOST and vHOST
+ *
+ */
+static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
+{
+	int ret = 0;
+	int i;
+	u64 size;
+	enum pci_barno barno;
+	struct device *dev = &ntb->epf->dev;
+
+	for (i = 0; i < ntb->num_mws; i++) {
+
+		size = ntb->mws_size[i];
+
+		barno = ntb->epf_ntb_bar[BAR_MW0 + i];
+
+		ntb->epf->bar[barno].barno = barno;
+		ntb->epf->bar[barno].size = size;
+		ntb->epf->bar[barno].addr = 0;
+		ntb->epf->bar[barno].phys_addr = 0;
+		ntb->epf->bar[barno].flags |= upper_32_bits(size) ?
+				PCI_BASE_ADDRESS_MEM_TYPE_64 :
+				PCI_BASE_ADDRESS_MEM_TYPE_32;
+
+		ret = pci_epc_set_bar(ntb->epf->epc,
+				      ntb->epf->func_no,
+				      ntb->epf->vfunc_no,
+				      &ntb->epf->bar[barno]);
+		if (ret) {
+			dev_err(dev, "intf: MW set failed\n");
+			goto err_alloc_mem;
+		}
+
+		/* allocate epc outbound memory windows to vpci vntb device */
+		ntb->vpci_mw_addr[i] = pci_epc_mem_alloc_addr(ntb->epf->epc,
+							      &ntb->vpci_mw_phy[i],
+							      size);
+		if (!ntb->vpci_mw_addr[i]) {
+			dev_err(dev, "Failed to allocate source address\n");
+			goto err_alloc_mem;
+		}
+	}
+
+	return ret;
+err_alloc_mem:
+	return ret;
+}
+
+/**
+ * epf_ntb_mw_bar_clear() - Clear Memory window BARs
+ * @ntb: NTB device that facilitates communication between HOST and vHOST
+ *
+ */
+static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb)
+{
+	enum pci_barno barno;
+	int i;
+
+	for (i = 0; i < ntb->num_mws; i++) {
+		barno = ntb->epf_ntb_bar[BAR_MW0 + i];
+		pci_epc_clear_bar(ntb->epf->epc,
+				  ntb->epf->func_no,
+				  ntb->epf->vfunc_no,
+				  &ntb->epf->bar[barno]);
+
+		pci_epc_mem_free_addr(ntb->epf->epc,
+				      ntb->vpci_mw_phy[i],
+				      ntb->vpci_mw_addr[i],
+				      ntb->mws_size[i]);
+	}
+}
+
+/**
+ * epf_ntb_epc_destroy() - Cleanup NTB EPC interface
+ * @ntb: NTB device that facilitates communication between HOST and vHOST
+ *
+ * Wrapper for epf_ntb_epc_destroy_interface() to cleanup all the NTB interfaces
+ */
+static void epf_ntb_epc_destroy(struct epf_ntb *ntb)
+{
+	pci_epc_remove_epf(ntb->epf->epc, ntb->epf, 0);
+	pci_epc_put(ntb->epf->epc);
+}
+
+/**
+ * epf_ntb_init_epc_bar() - Identify BARs to be used for each of the NTB
+ * constructs (scratchpad region, doorbell, memorywindow)
+ * @ntb: NTB device that facilitates communication between HOST and vHOST
+ *
+ */
+static int epf_ntb_init_epc_bar(struct epf_ntb *ntb)
+{
+	const struct pci_epc_features *epc_features;
+	enum pci_barno barno;
+	enum epf_ntb_bar bar;
+	struct device *dev;
+	u32 num_mws;
+	int i;
+
+	barno = BAR_0;
+	num_mws = ntb->num_mws;
+	dev = &ntb->epf->dev;
+	epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);
+
+	/* These are required BARs which are mandatory for NTB functionality */
+	for (bar = BAR_CONFIG; bar <= BAR_MW0; bar++, barno++) {
+		barno = pci_epc_get_next_free_bar(epc_features, barno);
+		if (barno < 0) {
+			dev_err(dev, "intf: Fail to get NTB function BAR\n");
+			return barno;
+		}
+		ntb->epf_ntb_bar[bar] = barno;
+	}
+
+	/* These are optional BARs which don't impact NTB functionality */
+	for (bar = BAR_MW1, i = 1; i < num_mws; bar++, barno++, i++) {
+		barno = pci_epc_get_next_free_bar(epc_features, barno);
+		if (barno < 0) {
+			ntb->num_mws = i;
+			dev_dbg(dev, "BAR not available for > MW%d\n", i + 1);
+		}
+		ntb->epf_ntb_bar[bar] = barno;
+	}
+
+	return 0;
+}
+
+/**
+ * epf_ntb_epc_init() - Initialize NTB interface
+ * @ntb: NTB device that facilitates communication between HOST and vHOST2
+ *
+ * Wrapper to initialize a particular EPC interface and start the workqueue
+ * to check for commands from host. This function will write to the
+ * EP controller HW for configuring it.
+ */
+static int epf_ntb_epc_init(struct epf_ntb *ntb)
+{
+	u8 func_no, vfunc_no;
+	struct pci_epc *epc;
+	struct pci_epf *epf;
+	struct device *dev;
+	int ret;
+
+	epf = ntb->epf;
+	dev = &epf->dev;
+	epc = epf->epc;
+	func_no = ntb->epf->func_no;
+	vfunc_no = ntb->epf->vfunc_no;
+
+	ret = epf_ntb_config_sspad_bar_set(ntb);
+	if (ret) {
+		dev_err(dev, "intf: Config/self SPAD BAR init failed");
+		return ret;
+	}
+
+	ret = epf_ntb_configure_interrupt(ntb);
+	if (ret) {
+		dev_err(dev, "intf: Interrupt configuration failed\n");
+		goto err_config_interrupt;
+	}
+
+	ret = epf_ntb_db_bar_init(ntb);
+	if (ret) {
+		dev_err(dev, "intf: DB BAR init failed\n");
+		goto err_db_bar_init;
+	}
+
+	ret = epf_ntb_mw_bar_init(ntb);
+	if (ret) {
+		dev_err(dev, "intf: MW BAR init failed\n");
+		goto err_mw_bar_init;
+	}
+
+	if (vfunc_no <= 1) {
+		ret = pci_epc_write_header(epc, func_no, vfunc_no, epf->header);
+		if (ret) {
+			dev_err(dev, "intf: Configuration header write failed\n");
+			goto err_write_header;
+		}
+	}
+
+	INIT_DELAYED_WORK(&ntb->cmd_handler, epf_ntb_cmd_handler);
+	queue_work(kpcintb_workqueue, &ntb->cmd_handler.work);
+
+	return 0;
+
+err_write_header:
+	epf_ntb_mw_bar_clear(ntb);
+err_mw_bar_init:
+	epf_ntb_db_bar_clear(ntb);
+err_db_bar_init:
+err_config_interrupt:
+	epf_ntb_config_sspad_bar_clear(ntb);
+
+	return ret;
+}
+
+
+/**
+ * epf_ntb_epc_cleanup() - Cleanup all NTB interfaces
+ * @ntb: NTB device that facilitates communication between HOST1 and HOST2
+ *
+ * Wrapper to cleanup all NTB interfaces.
+ */
+static void epf_ntb_epc_cleanup(struct epf_ntb *ntb)
+{
+	epf_ntb_db_bar_clear(ntb);
+	epf_ntb_mw_bar_clear(ntb);
+}
+
+#define EPF_NTB_R(_name)						\
+static ssize_t epf_ntb_##_name##_show(struct config_item *item,		\
+				      char *page)			\
+{									\
+	struct config_group *group = to_config_group(item);		\
+	struct epf_ntb *ntb = to_epf_ntb(group);			\
+									\
+	return sprintf(page, "%d\n", ntb->_name);			\
+}
+
+#define EPF_NTB_W(_name)						\
+static ssize_t epf_ntb_##_name##_store(struct config_item *item,	\
+				       const char *page, size_t len)	\
+{									\
+	struct config_group *group = to_config_group(item);		\
+	struct epf_ntb *ntb = to_epf_ntb(group);			\
+	u32 val;							\
+	int ret;							\
+									\
+	ret = kstrtou32(page, 0, &val);					\
+	if (ret)							\
+		return ret;						\
+									\
+	ntb->_name = val;						\
+									\
+	return len;							\
+}
+
+#define EPF_NTB_MW_R(_name)						\
+static ssize_t epf_ntb_##_name##_show(struct config_item *item,		\
+				      char *page)			\
+{									\
+	struct config_group *group = to_config_group(item);		\
+	struct epf_ntb *ntb = to_epf_ntb(group);			\
+	int win_no;							\
+									\
+	sscanf(#_name, "mw%d", &win_no);				\
+									\
+	return sprintf(page, "%lld\n", ntb->mws_size[win_no - 1]);	\
+}
+
+#define EPF_NTB_MW_W(_name)						\
+static ssize_t epf_ntb_##_name##_store(struct config_item *item,	\
+				       const char *page, size_t len)	\
+{									\
+	struct config_group *group = to_config_group(item);		\
+	struct epf_ntb *ntb = to_epf_ntb(group);			\
+	struct device *dev = &ntb->epf->dev;				\
+	int win_no;							\
+	u64 val;							\
+	int ret;							\
+									\
+	ret = kstrtou64(page, 0, &val);					\
+	if (ret)							\
+		return ret;						\
+									\
+	if (sscanf(#_name, "mw%d", &win_no) != 1)			\
+		return -EINVAL;						\
+									\
+	if (ntb->num_mws < win_no) {					\
+		dev_err(dev, "Invalid num_nws: %d value\n", ntb->num_mws); \
+		return -EINVAL;						\
+	}								\
+									\
+	ntb->mws_size[win_no - 1] = val;				\
+									\
+	return len;							\
+}
+
+static ssize_t epf_ntb_num_mws_store(struct config_item *item,
+				     const char *page, size_t len)
+{
+	struct config_group *group = to_config_group(item);
+	struct epf_ntb *ntb = to_epf_ntb(group);
+	u32 val;
+	int ret;
+
+	ret = kstrtou32(page, 0, &val);
+	if (ret)
+		return ret;
+
+	if (val > MAX_MW)
+		return -EINVAL;
+
+	ntb->num_mws = val;
+
+	return len;
+}
+
+EPF_NTB_R(spad_count)
+EPF_NTB_W(spad_count)
+EPF_NTB_R(db_count)
+EPF_NTB_W(db_count)
+EPF_NTB_R(num_mws)
+EPF_NTB_R(vbus_number)
+EPF_NTB_W(vbus_number)
+EPF_NTB_MW_R(mw1)
+EPF_NTB_MW_W(mw1)
+EPF_NTB_MW_R(mw2)
+EPF_NTB_MW_W(mw2)
+EPF_NTB_MW_R(mw3)
+EPF_NTB_MW_W(mw3)
+EPF_NTB_MW_R(mw4)
+EPF_NTB_MW_W(mw4)
+
+CONFIGFS_ATTR(epf_ntb_, spad_count);
+CONFIGFS_ATTR(epf_ntb_, db_count);
+CONFIGFS_ATTR(epf_ntb_, num_mws);
+CONFIGFS_ATTR(epf_ntb_, mw1);
+CONFIGFS_ATTR(epf_ntb_, mw2);
+CONFIGFS_ATTR(epf_ntb_, mw3);
+CONFIGFS_ATTR(epf_ntb_, mw4);
+CONFIGFS_ATTR(epf_ntb_, vbus_number);
+
+static struct configfs_attribute *epf_ntb_attrs[] = {
+	&epf_ntb_attr_spad_count,
+	&epf_ntb_attr_db_count,
+	&epf_ntb_attr_num_mws,
+	&epf_ntb_attr_mw1,
+	&epf_ntb_attr_mw2,
+	&epf_ntb_attr_mw3,
+	&epf_ntb_attr_mw4,
+	&epf_ntb_attr_vbus_number,
+	NULL,
+};
+
+static const struct config_item_type ntb_group_type = {
+	.ct_attrs	= epf_ntb_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+/**
+ * epf_ntb_add_cfs() - Add configfs directory specific to NTB
+ * @epf: NTB endpoint function device
+ * @group: A pointer to the config_group structure referencing a group of
+ *	   config_items of a specific type that belong to a specific sub-system.
+ *
+ * Add configfs directory specific to NTB. This directory will hold
+ * NTB specific properties like db_count, spad_count, num_mws etc.,
+ */
+static struct config_group *epf_ntb_add_cfs(struct pci_epf *epf,
+					    struct config_group *group)
+{
+	struct epf_ntb *ntb = epf_get_drvdata(epf);
+	struct config_group *ntb_group = &ntb->group;
+	struct device *dev = &epf->dev;
+
+	config_group_init_type_name(ntb_group, dev_name(dev), &ntb_group_type);
+
+	return ntb_group;
+}
+
+/*==== virtual PCI bus driver, which only load virutal ntb pci driver ====*/
+
+#define VPCI_BUS_NUM 0x10
+
+uint32_t pci_space[] = {
+	(VNTB_VID | (VNTB_PID << 16)),	//DeviceID, Vendor ID
+	0,		// status, Command
+	0xffffffff,	// Class code, subclass, prog if, revision id
+	0x40,		//bist, header type, latency Timer, cache line size
+	0,		//bar 0
+	0,		//bar 1
+	0,		//bar 2
+	0,		//bar 3
+	0,		//bar 4
+	0,		//bar 5
+	0,		//cardbus cis point
+	0,		//Subsystem ID Subystem vendor id
+	0,		//ROM Base Address
+	0,		//Reserved, Cap. Point
+	0,		//Reserved,
+	0,		//Max Lat, Min Gnt, interrupt pin, interrupt line
+};
+
+int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
+{
+	if (devfn == 0) {
+		memcpy(val, ((uint8_t *)pci_space) + where, size);
+		return 0;
+	}
+	return -1;
+}
+
+int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
+{
+	return 0;
+}
+
+struct pci_ops vpci_ops = {
+	.read = pci_read,
+	.write = pci_write,
+};
+
+static int vpci_bus(void *sysdata)
+{
+	struct pci_bus *vpci_bus;
+
+	vpci_bus = pci_scan_bus(VPCI_BUS_NUM, &vpci_ops, sysdata);
+	if (vpci_bus)
+		pr_err("create pci bus\n");
+
+	pci_bus_add_devices(vpci_bus);
+
+	return 0;
+}
+
+/*==================== Virtual PCIe NTB driver ==========================*/
+
+static int vntb_epf_mw_count(struct ntb_dev *ntb, int pidx)
+{
+	struct epf_ntb *ndev = ntb_ndev(ntb);
+
+	return ndev->num_mws;
+}
+
+static int vntb_epf_spad_count(struct ntb_dev *ntb)
+{
+	return ntb_ndev(ntb)->spad_count;
+}
+
+static int vntb_epf_peer_mw_count(struct ntb_dev *ntb)
+{
+	return ntb_ndev(ntb)->num_mws;
+}
+
+static u64 vntb_epf_db_valid_mask(struct ntb_dev *ntb)
+{
+	return BIT_ULL(ntb_ndev(ntb)->db_count) - 1;
+}
+
+static int vntb_epf_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
+{
+	return 0;
+}
+
+static int vntb_epf_mw_set_trans(struct ntb_dev *ndev, int pidx, int idx,
+		dma_addr_t addr, resource_size_t size)
+{
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+	struct pci_epf_bar *epf_bar;
+	enum pci_barno barno;
+	int ret;
+	struct device *dev;
+
+	dev = &ntb->ntb.dev;
+	barno = ntb->epf_ntb_bar[BAR_MW0 + idx];
+	epf_bar = &ntb->epf->bar[barno];
+	epf_bar->phys_addr = addr;
+	epf_bar->barno = barno;
+	epf_bar->size = size;
+
+	ret = pci_epc_set_bar(ntb->epf->epc, 0, 0, epf_bar);
+	if (ret) {
+		dev_err(dev, "failure set mw trans\n");
+		return ret;
+	}
+	return 0;
+}
+
+static int vntb_epf_mw_clear_trans(struct ntb_dev *ntb, int pidx, int idx)
+{
+	return 0;
+}
+
+static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
+				phys_addr_t *base, resource_size_t *size)
+{
+
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+
+	if (base)
+		*base = ntb->vpci_mw_phy[idx];
+
+	if (size)
+		*size = ntb->mws_size[idx];
+
+	return 0;
+}
+
+static int vntb_epf_link_enable(struct ntb_dev *ntb,
+			enum ntb_speed max_speed,
+			enum ntb_width max_width)
+{
+	return 0;
+}
+
+static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx)
+{
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+	int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * 4;
+	u32 val;
+	void __iomem *base = ntb->reg;
+
+	val = readl(base + off + ct + idx * 4);
+	return val;
+}
+
+static int vntb_epf_spad_write(struct ntb_dev *ndev, int idx, u32 val)
+{
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+	struct epf_ntb_ctrl *ctrl = ntb->reg;
+	int off = ctrl->spad_offset, ct = ctrl->spad_count * 4;
+	void __iomem *base = ntb->reg;
+
+	writel(val, base + off + ct + idx * 4);
+	return 0;
+}
+
+static u32 vntb_epf_peer_spad_read(struct ntb_dev *ndev, int pidx, int idx)
+{
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+	struct epf_ntb_ctrl *ctrl = ntb->reg;
+	int off = ctrl->spad_offset;
+	void __iomem *base = ntb->reg;
+	u32 val;
+
+	val = readl(base + off + idx * 4);
+	return val;
+}
+
+static int vntb_epf_peer_spad_write(struct ntb_dev *ndev, int pidx, int idx, u32 val)
+{
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+	struct epf_ntb_ctrl *ctrl = ntb->reg;
+	int off = ctrl->spad_offset;
+	void __iomem *base = ntb->reg;
+
+	writel(val, base + off + idx * 4);
+	return 0;
+}
+
+static int vntb_epf_peer_db_set(struct ntb_dev *ndev, u64 db_bits)
+{
+	u32 interrupt_num = ffs(db_bits) + 1;
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+	u8 func_no, vfunc_no;
+	int ret;
+
+	func_no = ntb->epf->func_no;
+	vfunc_no = ntb->epf->vfunc_no;
+
+	ret = pci_epc_raise_irq(ntb->epf->epc,
+				func_no,
+				vfunc_no,
+				PCI_EPC_IRQ_MSI,
+				interrupt_num + 1);
+	if (ret) {
+		dev_err(&ntb->ntb.dev, "intf: Failed to raise IRQ\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static u64 vntb_epf_db_read(struct ntb_dev *ndev)
+{
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+
+	return ntb->db;
+}
+
+static int vntb_epf_mw_get_align(struct ntb_dev *ndev, int pidx, int idx,
+			resource_size_t *addr_align,
+			resource_size_t *size_align,
+			resource_size_t *size_max)
+{
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+
+	if (addr_align)
+		*addr_align = SZ_4K;
+
+	if (size_align)
+		*size_align = 1;
+
+	if (size_max)
+		*size_max = ntb->mws_size[idx];
+
+	return 0;
+}
+
+static u64 vntb_epf_link_is_up(struct ntb_dev *ndev,
+			enum ntb_speed *speed,
+			enum ntb_width *width)
+{
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+
+	return ntb->reg->link_status;
+}
+
+static int vntb_epf_db_clear_mask(struct ntb_dev *ndev, u64 db_bits)
+{
+	return 0;
+}
+
+static int vntb_epf_db_clear(struct ntb_dev *ndev, u64 db_bits)
+{
+	struct epf_ntb *ntb = ntb_ndev(ndev);
+
+	ntb->db &= ~db_bits;
+	return 0;
+}
+
+static int vntb_epf_link_disable(struct ntb_dev *ntb)
+{
+	return 0;
+}
+
+static const struct ntb_dev_ops vntb_epf_ops = {
+	.mw_count		= vntb_epf_mw_count,
+	.spad_count		= vntb_epf_spad_count,
+	.peer_mw_count		= vntb_epf_peer_mw_count,
+	.db_valid_mask		= vntb_epf_db_valid_mask,
+	.db_set_mask		= vntb_epf_db_set_mask,
+	.mw_set_trans		= vntb_epf_mw_set_trans,
+	.mw_clear_trans		= vntb_epf_mw_clear_trans,
+	.peer_mw_get_addr	= vntb_epf_peer_mw_get_addr,
+	.link_enable		= vntb_epf_link_enable,
+	.spad_read		= vntb_epf_spad_read,
+	.spad_write		= vntb_epf_spad_write,
+	.peer_spad_read		= vntb_epf_peer_spad_read,
+	.peer_spad_write	= vntb_epf_peer_spad_write,
+	.peer_db_set		= vntb_epf_peer_db_set,
+	.db_read		= vntb_epf_db_read,
+	.mw_get_align		= vntb_epf_mw_get_align,
+	.link_is_up		= vntb_epf_link_is_up,
+	.db_clear_mask		= vntb_epf_db_clear_mask,
+	.db_clear		= vntb_epf_db_clear,
+	.link_disable		= vntb_epf_link_disable,
+};
+
+static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	int ret;
+	struct epf_ntb *ndev = (struct epf_ntb *)pdev->sysdata;
+	struct device *dev = &pdev->dev;
+
+	ndev->ntb.pdev = pdev;
+	ndev->ntb.topo = NTB_TOPO_NONE;
+	ndev->ntb.ops =  &vntb_epf_ops;
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+	if (ret) {
+		dev_err(dev, "Cannot set DMA mask\n");
+		return -1;
+	}
+
+	ret = ntb_register_device(&ndev->ntb);
+	if (ret) {
+		dev_err(dev, "Failed to register NTB device\n");
+		goto err_register_dev;
+	}
+
+	dev_info(dev, "PCI Virtual NTB driver loaded\n");
+	return 0;
+
+err_register_dev:
+	return -1;
+}
+
+static const struct pci_device_id pci_vntb_table[] = {
+	{
+		PCI_DEVICE(VNTB_VID, VNTB_PID),
+	},
+	{},
+};
+
+static struct pci_driver vntb_pci_driver = {
+	.name           = "pci-vntb",
+	.id_table       = pci_vntb_table,
+	.probe          = pci_vntb_probe,
+};
+
+/* ============ PCIe EPF Driver Bind ====================*/
+
+/**
+ * epf_ntb_bind() - Initialize endpoint controller to provide NTB functionality
+ * @epf: NTB endpoint function device
+ *
+ * Initialize both the endpoint controllers associated with NTB function device.
+ * Invoked when a primary interface or secondary interface is bound to EPC
+ * device. This function will succeed only when EPC is bound to both the
+ * interfaces.
+ */
+static int epf_ntb_bind(struct pci_epf *epf)
+{
+	struct epf_ntb *ntb = epf_get_drvdata(epf);
+	struct device *dev = &epf->dev;
+	int ret;
+
+	if (!epf->epc) {
+		dev_dbg(dev, "PRIMARY EPC interface not yet bound\n");
+		return 0;
+	}
+
+	ret = epf_ntb_init_epc_bar(ntb);
+	if (ret) {
+		dev_err(dev, "Failed to create NTB EPC\n");
+		goto err_bar_init;
+	}
+
+	ret = epf_ntb_config_spad_bar_alloc(ntb);
+	if (ret) {
+		dev_err(dev, "Failed to allocate BAR memory\n");
+		goto err_bar_alloc;
+	}
+
+	ret = epf_ntb_epc_init(ntb);
+	if (ret) {
+		dev_err(dev, "Failed to initialize EPC\n");
+		goto err_bar_alloc;
+	}
+
+	epf_set_drvdata(epf, ntb);
+
+	if (pci_register_driver(&vntb_pci_driver)) {
+		dev_err(dev, "failure register vntb pci driver\n");
+		goto err_bar_alloc;
+	}
+
+	vpci_bus(ntb);
+
+	return 0;
+
+err_bar_alloc:
+	epf_ntb_config_spad_bar_free(ntb);
+
+err_bar_init:
+	epf_ntb_epc_destroy(ntb);
+
+	return ret;
+}
+
+/**
+ * epf_ntb_unbind() - Cleanup the initialization from epf_ntb_bind()
+ * @epf: NTB endpoint function device
+ *
+ * Cleanup the initialization from epf_ntb_bind()
+ */
+static void epf_ntb_unbind(struct pci_epf *epf)
+{
+	struct epf_ntb *ntb = epf_get_drvdata(epf);
+
+	epf_ntb_epc_cleanup(ntb);
+	epf_ntb_config_spad_bar_free(ntb);
+	epf_ntb_epc_destroy(ntb);
+
+	pci_unregister_driver(&vntb_pci_driver);
+}
+
+// EPF driver probe
+static struct pci_epf_ops epf_ntb_ops = {
+	.bind   = epf_ntb_bind,
+	.unbind = epf_ntb_unbind,
+	.add_cfs = epf_ntb_add_cfs,
+};
+
+/**
+ * epf_ntb_probe() - Probe NTB function driver
+ * @epf: NTB endpoint function device
+ *
+ * Probe NTB function driver when endpoint function bus detects a NTB
+ * endpoint function.
+ */
+static int epf_ntb_probe(struct pci_epf *epf)
+{
+	struct epf_ntb *ntb;
+	struct device *dev;
+
+	dev = &epf->dev;
+
+	ntb = devm_kzalloc(dev, sizeof(*ntb), GFP_KERNEL);
+	if (!ntb)
+		return -ENOMEM;
+
+	epf->header = &epf_ntb_header;
+	ntb->epf = epf;
+	epf_set_drvdata(epf, ntb);
+
+	dev_info(dev, "pci-ep epf driver loaded\n");
+	return 0;
+}
+
+static const struct pci_epf_device_id epf_ntb_ids[] = {
+	{
+		.name = "pci_epf_vntb",
+	},
+	{},
+};
+
+static struct pci_epf_driver epf_ntb_driver = {
+	.driver.name    = "pci_epf_vntb",
+	.probe          = epf_ntb_probe,
+	.id_table       = epf_ntb_ids,
+	.ops            = &epf_ntb_ops,
+	.owner          = THIS_MODULE,
+};
+
+
+static int __init epf_ntb_init(void)
+{
+	int ret;
+
+	kpcintb_workqueue = alloc_workqueue("kpcintb", WQ_MEM_RECLAIM |
+					    WQ_HIGHPRI, 0);
+	ret = pci_epf_register_driver(&epf_ntb_driver);
+	if (ret) {
+		destroy_workqueue(kpcintb_workqueue);
+		pr_err("Failed to register pci epf ntb driver --> %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+module_init(epf_ntb_init);
+
+static void __exit epf_ntb_exit(void)
+{
+	pci_epf_unregister_driver(&epf_ntb_driver);
+	destroy_workqueue(kpcintb_workqueue);
+}
+module_exit(epf_ntb_exit);
+
+MODULE_DESCRIPTION("PCI EPF NTB DRIVER");
+MODULE_AUTHOR("Frank Li <Frank.li@nxp.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 4/4] Documentation: PCI: Add specification for the PCI vNTB function device
  2022-02-15  5:38 [RFC PATCH 0/4] NTB function for PCIe RC to EP connection Frank Li
                   ` (2 preceding siblings ...)
  2022-02-15  5:38 ` [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection Frank Li
@ 2022-02-15  5:38 ` Frank Li
  3 siblings, 0 replies; 15+ messages in thread
From: Frank Li @ 2022-02-15  5:38 UTC (permalink / raw)
  To: linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, lznuaa, hongxing.zhu

Add specification for the PCI vNTB function device. The endpoint function
driver and the host PCI driver should be created based on this
specification.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 Documentation/PCI/endpoint/index.rst          |   2 +
 .../PCI/endpoint/pci-vntb-function.rst        | 126 ++++++++++++++
 Documentation/PCI/endpoint/pci-vntb-howto.rst | 161 ++++++++++++++++++
 3 files changed, 289 insertions(+)
 create mode 100644 Documentation/PCI/endpoint/pci-vntb-function.rst
 create mode 100644 Documentation/PCI/endpoint/pci-vntb-howto.rst

diff --git a/Documentation/PCI/endpoint/index.rst b/Documentation/PCI/endpoint/index.rst
index 38ea1f604b6d3..4d2333e7ae067 100644
--- a/Documentation/PCI/endpoint/index.rst
+++ b/Documentation/PCI/endpoint/index.rst
@@ -13,6 +13,8 @@ PCI Endpoint Framework
    pci-test-howto
    pci-ntb-function
    pci-ntb-howto
+   pci-vntb-function
+   pci-vntb-howto
 
    function/binding/pci-test
    function/binding/pci-ntb
diff --git a/Documentation/PCI/endpoint/pci-vntb-function.rst b/Documentation/PCI/endpoint/pci-vntb-function.rst
new file mode 100644
index 0000000000000..cad8013e88390
--- /dev/null
+++ b/Documentation/PCI/endpoint/pci-vntb-function.rst
@@ -0,0 +1,126 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=================
+PCI vNTB Function
+=================
+
+:Author: Frank Li <Frank.Li@nxp.com>
+
+The difference between PCI NTB function and PCI vNTB function is
+
+PCI NTB function need at two endpoint instances and connect HOST1
+and HOST2.
+
+PCI vNTB function only use one host and one endpoint(EP), use NTB
+connect EP and PCI host
+
+.. code-block:: text
+
+
+  +------------+         +---------------------------------------+
+  |            |         |                                       |
+  +------------+         |                        +--------------+
+  | NTB        |         |                        | NTB          |
+  | NetDev     |         |                        | NetDev       |
+  +------------+         |                        +--------------+
+  | NTB        |         |                        | NTB          |
+  | Transfer   |         |                        | Transfer     |
+  +------------+         |                        +--------------+
+  |            |         |                        |              |
+  |  PCI NTB   |         |                        |              |
+  |    EPF     |         |                        |              |
+  |   Driver   |         |                        | PCI Virtual  |
+  |            |         +---------------+        | NTB Driver   |
+  |            |         | PCI EP NTB    |<------>|              |
+  |            |         |  FN Driver    |        |              |
+  +------------+         +---------------+        +--------------+
+  |            |         |               |        |              |
+  |  PCI BUS   | <-----> |  PCI EP BUS   |        |  Virtual PCI |
+  |            |  PCI    |               |        |     BUS      |
+  +------------+         +---------------+--------+--------------+
+      PCI RC                        PCI EP
+
+Constructs used for Implementing vNTB
+=====================================
+
+	1) Config Region
+	2) Self Scratchpad Registers
+	3) Peer Scratchpad Registers
+	4) Doorbell (DB) Registers
+	5) Memory Window (MW)
+
+
+Config Region:
+--------------
+
+It is same as PCI NTB Function driver
+
+Scratchpad Registers:
+---------------------
+
+  It is appended after Config region.
+
+  +--------------------------------------------------+ Base
+  |                                                  |
+  |                                                  |
+  |                                                  |
+  |          Common Config Register                  |
+  |                                                  |
+  |                                                  |
+  |                                                  |
+  +-----------------------+--------------------------+ Base + span_offset
+  |                       |                          |
+  |    Peer Span Space    |    Span Space            |
+  |                       |                          |
+  |                       |                          |
+  +-----------------------+--------------------------+ Base + span_offset
+  |                       |                          |      + span_count * 4
+  |                       |                          |
+  |     Span Space        |   Peer Span Space        |
+  |                       |                          |
+  +-----------------------+--------------------------+
+        Virtual PCI             Pcie Endpoint
+        NTB Driver               NTB Driver
+
+
+Doorbell Registers:
+-------------------
+
+  Doorbell Registers are used by the hosts to interrupt each other.
+
+Memory Window:
+--------------
+
+  Actual transfer of data between the two hosts will happen using the
+  memory window.
+
+Modeling Constructs:
+====================
+
+32-bit BARs.
+
+======  ===============
+BAR NO  CONSTRUCTS USED
+======  ===============
+BAR0    Config Region
+BAR1    Doorbell
+BAR2    Memory Window 1
+BAR3    Memory Window 2
+BAR4    Memory Window 3
+BAR5    Memory Window 4
+======  ===============
+
+64-bit BARs.
+
+======  ===============================
+BAR NO  CONSTRUCTS USED
+======  ===============================
+BAR0    Config Region + Scratchpad
+BAR1
+BAR2    Doorbell
+BAR3
+BAR4    Memory Window 1
+BAR5
+======  ===============================
+
+
diff --git a/Documentation/PCI/endpoint/pci-vntb-howto.rst b/Documentation/PCI/endpoint/pci-vntb-howto.rst
new file mode 100644
index 0000000000000..b4a679144692a
--- /dev/null
+++ b/Documentation/PCI/endpoint/pci-vntb-howto.rst
@@ -0,0 +1,161 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================================================================
+PCI Non-Transparent Bridge (NTB) Endpoint Function (EPF) User Guide
+===================================================================
+
+:Author: Frank Li <Frank.Li@nxp.com>
+
+This document is a guide to help users use pci-epf-vntb function driver
+and ntb_hw_epf host driver for NTB functionality. The list of steps to
+be followed in the host side and EP side is given below. For the hardware
+configuration and internals of NTB using configurable endpoints see
+Documentation/PCI/endpoint/pci-vntb-function.rst
+
+Endpoint Device
+===============
+
+Endpoint Controller Devices
+---------------------------
+
+To find the list of endpoint controller devices in the system::
+
+        # ls /sys/class/pci_epc/
+          5f010000.pcie_ep
+
+If PCI_ENDPOINT_CONFIGFS is enabled::
+
+        # ls /sys/kernel/config/pci_ep/controllers
+          5f010000.pcie_ep
+
+Endpoint Function Drivers
+-------------------------
+
+To find the list of endpoint function drivers in the system::
+
+	# ls /sys/bus/pci-epf/drivers
+	pci_epf_ntb  pci_epf_test  pci_epf_vntb
+
+If PCI_ENDPOINT_CONFIGFS is enabled::
+
+	# ls /sys/kernel/config/pci_ep/functions
+	pci_epf_ntb  pci_epf_test  pci_epf_vntb
+
+
+Creating pci-epf-vntb Device
+----------------------------
+
+PCI endpoint function device can be created using the configfs. To create
+pci-epf-vntb device, the following commands can be used::
+
+	# mount -t configfs none /sys/kernel/config
+	# cd /sys/kernel/config/pci_ep/
+	# mkdir functions/pci_epf_vntb/func1
+
+The "mkdir func1" above creates the pci-epf-ntb function device that will
+be probed by pci_epf_vntb driver.
+
+The PCI endpoint framework populates the directory with the following
+configurable fields::
+
+	# ls functions/pci_epf_ntb/func1
+	baseclass_code    deviceid          msi_interrupts    pci-epf-ntb.0
+	progif_code       secondary         subsys_id         vendorid
+	cache_line_size   interrupt_pin     msix_interrupts   primary
+	revid             subclass_code     subsys_vendor_id
+
+The PCI endpoint function driver populates these entries with default values
+when the device is bound to the driver. The pci-epf-vntb driver populates
+vendorid with 0xffff and interrupt_pin with 0x0001::
+
+	# cat functions/pci_epf_vntb/func1/vendorid
+	0xffff
+	# cat functions/pci_epf_vntb/func1/interrupt_pin
+	0x0001
+
+
+Configuring pci-epf-vntb Device
+-------------------------------
+
+The user can configure the pci-epf-vntb device using its configfs entry. In order
+to change the vendorid and the deviceid, the following
+commands can be used::
+
+	# echo 0x1957 > functions/pci_epf_vntb/func1/vendorid
+	# echo 0x0809 > functions/pci_epf_vntb/func1/deviceid
+
+In order to configure NTB specific attributes, a new sub-directory to func1
+should be created::
+
+	# mkdir functions/pci_epf_vntb/func1/pci_epf_vntb.0/
+
+The NTB function driver will populate this directory with various attributes
+that can be configured by the user::
+
+	# ls functions/pci_epf_vntb/func1/pci_epf_vntb.0/
+	db_count    mw1         mw2         mw3         mw4         num_mws
+	spad_count
+
+A sample configuration for NTB function is given below::
+
+	# echo 4 > functions/pci_epf_vntb/func1/pci_epf_vntb.0/db_count
+	# echo 128 > functions/pci_epf_vntb/func1/pci_epf_vntb.0/spad_count
+	# echo 1 > functions/pci_epf_vntb/func1/pci_epf_vntb.0/num_mws
+	# echo 0x100000 > functions/pci_epf_vntb/func1/pci_epf_vntb.0/mw1
+
+Binding pci-epf-ntb Device to EP Controller
+--------------------------------------------
+
+NTB function device should be attached to PCI endpoint controllers
+connected to the host.
+
+	# ln -s controllers/5f010000.pcie_ep functions/pci-epf-ntb/func1/primary
+
+Once the above step is completed, the PCI endpoint controllers are ready to
+establish a link with the host.
+
+
+Start the Link
+--------------
+
+In order for the endpoint device to establish a link with the host, the _start_
+field should be populated with '1'. For NTB, both the PCI endpoint controllers
+should establish link with the host (imx8 don't need this steps)::
+
+	# echo 1 > controllers/5f010000.pcie_ep/start
+
+RootComplex Device
+==================
+
+lspci Output at Host side
+------------------------
+
+Note that the devices listed here correspond to the values populated in
+"Creating pci-epf-ntb Device" section above::
+
+	# lspci
+        00:00.0 PCI bridge: Freescale Semiconductor Inc Device 0000 (rev 01)
+        01:00.0 RAM memory: Freescale Semiconductor Inc Device 0809
+
+Endpoint Device / Virtual PCI bus
+=================================
+
+lspci Output at EP Side / Virtual PCI bus
+-----------------------------------------
+
+Note that the devices listed here correspond to the values populated in
+"Creating pci-epf-ntb Device" section above::
+
+        # lspci
+        10:00.0 Unassigned class [ffff]: Dawicontrol Computersysteme GmbH Device 1234 (rev ff)
+
+Using ntb_hw_epf Device
+-----------------------
+
+The host side software follows the standard NTB software architecture in Linux.
+All the existing client side NTB utilities like NTB Transport Client and NTB
+Netdev, NTB Ping Pong Test Client and NTB Tool Test Client can be used with NTB
+function device.
+
+For more information on NTB see
+:doc:`Non-Transparent Bridge <../../driver-api/ntb>`
-- 
2.24.0.rc1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection
  2022-02-15  5:38 ` [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection Frank Li
@ 2022-02-15  7:57   ` kernel test robot
  2022-02-16  7:13     ` kernel test robot
  2022-02-17 22:38   ` Bjorn Helgaas
  2 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-02-15  7:57 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2724 bytes --]

Hi Frank,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on jonmason-ntb/ntb-next v5.17-rc4 next-20220214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Frank-Li/NTB-function-for-PCIe-RC-to-EP-connection/20220215-134115
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220215/202202151535.839zuzcU-lkp(a)intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/84bf6254010435435a8c196e30bb2d25fd22763c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Frank-Li/NTB-function-for-PCIe-RC-to-EP-connection/20220215-134115
        git checkout 84bf6254010435435a8c196e30bb2d25fd22763c
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/pci/endpoint/functions/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/pci/endpoint/functions/pci-epf-vntb.c:975:5: warning: no previous prototype for 'pci_read' [-Wmissing-prototypes]
     975 | int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
         |     ^~~~~~~~
>> drivers/pci/endpoint/functions/pci-epf-vntb.c:984:5: warning: no previous prototype for 'pci_write' [-Wmissing-prototypes]
     984 | int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
         |     ^~~~~~~~~


vim +/pci_read +975 drivers/pci/endpoint/functions/pci-epf-vntb.c

   974	
 > 975	int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
   976	{
   977		if (devfn == 0) {
   978			memcpy(val, ((uint8_t *)pci_space) + where, size);
   979			return 0;
   980		}
   981		return -1;
   982	}
   983	
 > 984	int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
   985	{
   986		return 0;
   987	}
   988	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection
  2022-02-15  5:38 ` [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection Frank Li
@ 2022-02-16  7:13     ` kernel test robot
  2022-02-16  7:13     ` kernel test robot
  2022-02-17 22:38   ` Bjorn Helgaas
  2 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-02-16  7:13 UTC (permalink / raw)
  To: Frank Li; +Cc: llvm, kbuild-all

Hi Frank,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on jonmason-ntb/ntb-next v5.17-rc4 next-20220215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Frank-Li/NTB-function-for-PCIe-RC-to-EP-connection/20220215-134115
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20220216/202202161525.BBBVV79O-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0e628a783b935c70c80815db6c061ec84f884af5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/84bf6254010435435a8c196e30bb2d25fd22763c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Frank-Li/NTB-function-for-PCIe-RC-to-EP-connection/20220215-134115
        git checkout 84bf6254010435435a8c196e30bb2d25fd22763c
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/gvt/ drivers/media/i2c/ drivers/pci/endpoint/functions/ kernel/bpf/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/pci/endpoint/functions/pci-epf-vntb.c:975:5: warning: no previous prototype for function 'pci_read' [-Wmissing-prototypes]
   int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
       ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:975:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
   ^
   static 
>> drivers/pci/endpoint/functions/pci-epf-vntb.c:984:5: warning: no previous prototype for function 'pci_write' [-Wmissing-prototypes]
   int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
       ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:984:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
   ^
   static 
   2 warnings generated.


vim +/pci_read +975 drivers/pci/endpoint/functions/pci-epf-vntb.c

   974	
 > 975	int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
   976	{
   977		if (devfn == 0) {
   978			memcpy(val, ((uint8_t *)pci_space) + where, size);
   979			return 0;
   980		}
   981		return -1;
   982	}
   983	
 > 984	int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
   985	{
   986		return 0;
   987	}
   988	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection
@ 2022-02-16  7:13     ` kernel test robot
  0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-02-16  7:13 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3383 bytes --]

Hi Frank,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on jonmason-ntb/ntb-next v5.17-rc4 next-20220215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Frank-Li/NTB-function-for-PCIe-RC-to-EP-connection/20220215-134115
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20220216/202202161525.BBBVV79O-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0e628a783b935c70c80815db6c061ec84f884af5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/84bf6254010435435a8c196e30bb2d25fd22763c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Frank-Li/NTB-function-for-PCIe-RC-to-EP-connection/20220215-134115
        git checkout 84bf6254010435435a8c196e30bb2d25fd22763c
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/gvt/ drivers/media/i2c/ drivers/pci/endpoint/functions/ kernel/bpf/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/pci/endpoint/functions/pci-epf-vntb.c:975:5: warning: no previous prototype for function 'pci_read' [-Wmissing-prototypes]
   int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
       ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:975:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
   ^
   static 
>> drivers/pci/endpoint/functions/pci-epf-vntb.c:984:5: warning: no previous prototype for function 'pci_write' [-Wmissing-prototypes]
   int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
       ^
   drivers/pci/endpoint/functions/pci-epf-vntb.c:984:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
   ^
   static 
   2 warnings generated.


vim +/pci_read +975 drivers/pci/endpoint/functions/pci-epf-vntb.c

   974	
 > 975	int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
   976	{
   977		if (devfn == 0) {
   978			memcpy(val, ((uint8_t *)pci_space) + where, size);
   979			return 0;
   980		}
   981		return -1;
   982	}
   983	
 > 984	int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
   985	{
   986		return 0;
   987	}
   988	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 1/4] PCI: designware-ep: Allow pcie_ep_set_bar change inbound map address
  2022-02-15  5:38 ` [RFC PATCH 1/4] PCI: designware-ep: Allow pcie_ep_set_bar change inbound map address Frank Li
@ 2022-02-17 21:53   ` Bjorn Helgaas
  2022-02-17 22:13     ` Zhi Li
  0 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2022-02-17 21:53 UTC (permalink / raw)
  To: Frank Li
  Cc: linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, lznuaa, hongxing.zhu

In the subject, "pcie_ep_set_bar" looks like *part* of a function
name.  Please include the entire function name and add "()" after it.

On Mon, Feb 14, 2022 at 11:38:41PM -0600, Frank Li wrote:
> ntb_transfer will set memory map windows after probe.
> So the inbound map address need be updated dynamtically.

I don't see "ntb_transfer" in the tree.  If it's a function, please
add "()" after the name.  Otherwise, please say more about what
"ntb_transfer" is.

s/dynamtically/dynamically/

Please make the commit log say what the patch *does*, not just what
needs to happen.

Bjorn

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 2/4] NTB: epf: Added more flexible memory map method
  2022-02-15  5:38 ` [RFC PATCH 2/4] NTB: epf: Added more flexible memory map method Frank Li
@ 2022-02-17 21:59   ` Bjorn Helgaas
  2022-02-17 22:24     ` Zhi Li
  0 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2022-02-17 21:59 UTC (permalink / raw)
  To: Frank Li
  Cc: linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, lznuaa, hongxing.zhu, Jon Mason, Dave Jiang,
	Allen Hubbe, linux-ntb

[+cc Jon, Dave, Allen, linux-ntb, since you need at least an ack from
the NTB folks; beginning of thread:
https://lore.kernel.org/r/20220215053844.7119-1-Frank.Li@nxp.com]

In subject, s/Added/Add/.

But I don't think it's quite right yet, because this doesn't actually add
any functions.

On Mon, Feb 14, 2022 at 11:38:42PM -0600, Frank Li wrote:
> Supported below memory map method
> 
> bar 0: config and spad data
> bar 2: door bell
> bar 4: memory map windows

s/bar/BAR/
s/spad/?/ (I don't know what this is)

Presumably these BAR numbers apply to some specific hardware?  This
probably should specify *which* hardware.

Please make the commit log say what this patch does.

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/ntb/hw/epf/ntb_hw_epf.c | 48 ++++++++++++++++++++++++---------
>  1 file changed, 35 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> index b019755e4e21b..3ece49cb18ffa 100644
> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
> @@ -45,7 +45,6 @@
>  
>  #define NTB_EPF_MIN_DB_COUNT	3
>  #define NTB_EPF_MAX_DB_COUNT	31
> -#define NTB_EPF_MW_OFFSET	2
>  
>  #define NTB_EPF_COMMAND_TIMEOUT	1000 /* 1 Sec */
>  
> @@ -67,6 +66,7 @@ struct ntb_epf_dev {
>  	enum pci_barno ctrl_reg_bar;
>  	enum pci_barno peer_spad_reg_bar;
>  	enum pci_barno db_reg_bar;
> +	enum pci_barno mw_bar;
>  
>  	unsigned int mw_count;
>  	unsigned int spad_count;
> @@ -92,6 +92,8 @@ struct ntb_epf_data {
>  	enum pci_barno peer_spad_reg_bar;
>  	/* BAR that contains Doorbell region and Memory window '1' */
>  	enum pci_barno db_reg_bar;
> +	/* BAR that contains memory windows*/
> +	enum pci_barno mw_bar;
>  };
>  
>  static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command,
> @@ -411,7 +413,7 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx,
>  		return -EINVAL;
>  	}
>  
> -	bar = idx + NTB_EPF_MW_OFFSET;
> +	bar = idx + ndev->mw_bar;
>  
>  	mw_size = pci_resource_len(ntb->pdev, bar);
>  
> @@ -453,7 +455,7 @@ static int ntb_epf_peer_mw_get_addr(struct ntb_dev *ntb, int idx,
>  	if (idx == 0)
>  		offset = readl(ndev->ctrl_reg + NTB_EPF_MW1_OFFSET);
>  
> -	bar = idx + NTB_EPF_MW_OFFSET;
> +	bar = idx + ndev->mw_bar;
>  
>  	if (base)
>  		*base = pci_resource_start(ndev->ntb.pdev, bar) + offset;
> @@ -565,6 +567,7 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev,
>  			    struct pci_dev *pdev)
>  {
>  	struct device *dev = ndev->dev;
> +	size_t spad_sz, spad_off;
>  	int ret;
>  
>  	pci_set_drvdata(pdev, ndev);
> @@ -599,10 +602,16 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev,
>  		goto err_dma_mask;
>  	}
>  
> -	ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0);
> -	if (!ndev->peer_spad_reg) {
> -		ret = -EIO;
> -		goto err_dma_mask;
> +	if (ndev->peer_spad_reg_bar) {
> +		ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0);
> +		if (!ndev->peer_spad_reg) {
> +			ret = -EIO;
> +			goto err_dma_mask;
> +		}
> +	} else {
> +		spad_sz = 4 * readl(ndev->ctrl_reg + NTB_EPF_SPAD_COUNT);
> +		spad_off = readl(ndev->ctrl_reg + NTB_EPF_SPAD_OFFSET);
> +		ndev->peer_spad_reg = ndev->ctrl_reg + spad_off  + spad_sz;
>  	}
>  
>  	ndev->db_reg = pci_iomap(pdev, ndev->db_reg_bar, 0);
> @@ -657,6 +666,7 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev,
>  	enum pci_barno peer_spad_reg_bar = BAR_1;
>  	enum pci_barno ctrl_reg_bar = BAR_0;
>  	enum pci_barno db_reg_bar = BAR_2;
> +	enum pci_barno mw_bar = BAR_2;
>  	struct device *dev = &pdev->dev;
>  	struct ntb_epf_data *data;
>  	struct ntb_epf_dev *ndev;
> @@ -671,17 +681,16 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev,
>  
>  	data = (struct ntb_epf_data *)id->driver_data;
>  	if (data) {
> -		if (data->peer_spad_reg_bar)
> -			peer_spad_reg_bar = data->peer_spad_reg_bar;
> -		if (data->ctrl_reg_bar)
> -			ctrl_reg_bar = data->ctrl_reg_bar;
> -		if (data->db_reg_bar)
> -			db_reg_bar = data->db_reg_bar;
> +		peer_spad_reg_bar = data->peer_spad_reg_bar;
> +		ctrl_reg_bar = data->ctrl_reg_bar;
> +		db_reg_bar = data->db_reg_bar;
> +		mw_bar = data->mw_bar;
>  	}
>  
>  	ndev->peer_spad_reg_bar = peer_spad_reg_bar;
>  	ndev->ctrl_reg_bar = ctrl_reg_bar;
>  	ndev->db_reg_bar = db_reg_bar;
> +	ndev->mw_bar = mw_bar;
>  	ndev->dev = dev;
>  
>  	ntb_epf_init_struct(ndev, pdev);
> @@ -729,6 +738,14 @@ static const struct ntb_epf_data j721e_data = {
>  	.ctrl_reg_bar = BAR_0,
>  	.peer_spad_reg_bar = BAR_1,
>  	.db_reg_bar = BAR_2,
> +	.mw_bar = BAR_2,
> +};
> +
> +static const struct ntb_epf_data mx8_data = {
> +	.ctrl_reg_bar = BAR_0,
> +	.peer_spad_reg_bar = BAR_0,
> +	.db_reg_bar = BAR_2,
> +	.mw_bar = BAR_4,
>  };
>  
>  static const struct pci_device_id ntb_epf_pci_tbl[] = {
> @@ -737,6 +754,11 @@ static const struct pci_device_id ntb_epf_pci_tbl[] = {
>  		.class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00,
>  		.driver_data = (kernel_ulong_t)&j721e_data,
>  	},
> +	{
> +		PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, 0x0809),
> +		.class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00,
> +		.driver_data = (kernel_ulong_t)&mx8_data,
> +	},
>  	{ },
>  };
>  
> -- 
> 2.24.0.rc1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 1/4] PCI: designware-ep: Allow pcie_ep_set_bar change inbound map address
  2022-02-17 21:53   ` Bjorn Helgaas
@ 2022-02-17 22:13     ` Zhi Li
  0 siblings, 0 replies; 15+ messages in thread
From: Zhi Li @ 2022-02-17 22:13 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Frank Li, linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, hongxing.zhu

On Thu, Feb 17, 2022 at 3:54 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> In the subject, "pcie_ep_set_bar" looks like *part* of a function
> name.  Please include the entire function name and add "()" after it.

Okay

>
> On Mon, Feb 14, 2022 at 11:38:41PM -0600, Frank Li wrote:
> > ntb_transfer will set memory map windows after probe.
> > So the inbound map address need be updated dynamtically.
>
> I don't see "ntb_transfer" in the tree.  If it's a function, please
> add "()" after the name.  Otherwise, please say more about what
> "ntb_transfer" is.

Thanks.  That's ntb_mw_set_trans() at drivers/ntb/ntb_transport.c.

>
> s/dynamtically/dynamically/
>
> Please make the commit log say what the patch *does*, not just what
> needs to happen.

Okay,  this patch allows changing PCI BAR's address.  For example,
previous BAR0 map to physical address to 0xF1000000, now can change
to new physical address 0xF2000000 by call function pci_epc_set_bar()

The patch 3/4 is totally new method to make NTB work at RC and EP system,
I hope the basic structure is acceptable.

>
> Bjorn

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 2/4] NTB: epf: Added more flexible memory map method
  2022-02-17 21:59   ` Bjorn Helgaas
@ 2022-02-17 22:24     ` Zhi Li
  2022-02-17 22:43       ` Bjorn Helgaas
  0 siblings, 1 reply; 15+ messages in thread
From: Zhi Li @ 2022-02-17 22:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Frank Li, linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, hongxing.zhu, Jon Mason, Dave Jiang,
	Allen Hubbe, linux-ntb

On Thu, Feb 17, 2022 at 3:59 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> [+cc Jon, Dave, Allen, linux-ntb, since you need at least an ack from
> the NTB folks; beginning of thread:
> https://lore.kernel.org/r/20220215053844.7119-1-Frank.Li@nxp.com]
>
> In subject, s/Added/Add/.
>
> But I don't think it's quite right yet, because this doesn't actually add
> any functions.

How about "Allow more flexibility in the memory bar map method"?

>
> On Mon, Feb 14, 2022 at 11:38:42PM -0600, Frank Li wrote:
> > Supported below memory map method
> >
> > bar 0: config and spad data
> > bar 2: door bell
> > bar 4: memory map windows
>
> s/bar/BAR/
> s/spad/?/ (I don't know what this is)

SCRATCHPAD REGION
https://www.kernel.org/doc/html/latest/driver-api/ntb.html

>
> Presumably these BAR numbers apply to some specific hardware?  This
> probably should specify *which* hardware.

Yes, BAR number on the EP side is configurable, which is controlled in
the 3/4 patch.
The Original BAR memory method can't be used in the new RC->EP NTB usage mode.
Most Other code can be reused, so I enhanced the bar number config part.

>
> Please make the commit log say what this patch does.

Does it help if attach ASCII diagram in patch 3/4 or cover letter one

>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/ntb/hw/epf/ntb_hw_epf.c | 48 ++++++++++++++++++++++++---------
> >  1 file changed, 35 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> > index b019755e4e21b..3ece49cb18ffa 100644
> > --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> > +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
> > @@ -45,7 +45,6 @@
> >
> >  #define NTB_EPF_MIN_DB_COUNT 3
> >  #define NTB_EPF_MAX_DB_COUNT 31
> > -#define NTB_EPF_MW_OFFSET    2
> >
> >  #define NTB_EPF_COMMAND_TIMEOUT      1000 /* 1 Sec */
> >
> > @@ -67,6 +66,7 @@ struct ntb_epf_dev {
> >       enum pci_barno ctrl_reg_bar;
> >       enum pci_barno peer_spad_reg_bar;
> >       enum pci_barno db_reg_bar;
> > +     enum pci_barno mw_bar;
> >
> >       unsigned int mw_count;
> >       unsigned int spad_count;
> > @@ -92,6 +92,8 @@ struct ntb_epf_data {
> >       enum pci_barno peer_spad_reg_bar;
> >       /* BAR that contains Doorbell region and Memory window '1' */
> >       enum pci_barno db_reg_bar;
> > +     /* BAR that contains memory windows*/
> > +     enum pci_barno mw_bar;
> >  };
> >
> >  static int ntb_epf_send_command(struct ntb_epf_dev *ndev, u32 command,
> > @@ -411,7 +413,7 @@ static int ntb_epf_mw_set_trans(struct ntb_dev *ntb, int pidx, int idx,
> >               return -EINVAL;
> >       }
> >
> > -     bar = idx + NTB_EPF_MW_OFFSET;
> > +     bar = idx + ndev->mw_bar;
> >
> >       mw_size = pci_resource_len(ntb->pdev, bar);
> >
> > @@ -453,7 +455,7 @@ static int ntb_epf_peer_mw_get_addr(struct ntb_dev *ntb, int idx,
> >       if (idx == 0)
> >               offset = readl(ndev->ctrl_reg + NTB_EPF_MW1_OFFSET);
> >
> > -     bar = idx + NTB_EPF_MW_OFFSET;
> > +     bar = idx + ndev->mw_bar;
> >
> >       if (base)
> >               *base = pci_resource_start(ndev->ntb.pdev, bar) + offset;
> > @@ -565,6 +567,7 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev,
> >                           struct pci_dev *pdev)
> >  {
> >       struct device *dev = ndev->dev;
> > +     size_t spad_sz, spad_off;
> >       int ret;
> >
> >       pci_set_drvdata(pdev, ndev);
> > @@ -599,10 +602,16 @@ static int ntb_epf_init_pci(struct ntb_epf_dev *ndev,
> >               goto err_dma_mask;
> >       }
> >
> > -     ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0);
> > -     if (!ndev->peer_spad_reg) {
> > -             ret = -EIO;
> > -             goto err_dma_mask;
> > +     if (ndev->peer_spad_reg_bar) {
> > +             ndev->peer_spad_reg = pci_iomap(pdev, ndev->peer_spad_reg_bar, 0);
> > +             if (!ndev->peer_spad_reg) {
> > +                     ret = -EIO;
> > +                     goto err_dma_mask;
> > +             }
> > +     } else {
> > +             spad_sz = 4 * readl(ndev->ctrl_reg + NTB_EPF_SPAD_COUNT);
> > +             spad_off = readl(ndev->ctrl_reg + NTB_EPF_SPAD_OFFSET);
> > +             ndev->peer_spad_reg = ndev->ctrl_reg + spad_off  + spad_sz;
> >       }
> >
> >       ndev->db_reg = pci_iomap(pdev, ndev->db_reg_bar, 0);
> > @@ -657,6 +666,7 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev,
> >       enum pci_barno peer_spad_reg_bar = BAR_1;
> >       enum pci_barno ctrl_reg_bar = BAR_0;
> >       enum pci_barno db_reg_bar = BAR_2;
> > +     enum pci_barno mw_bar = BAR_2;
> >       struct device *dev = &pdev->dev;
> >       struct ntb_epf_data *data;
> >       struct ntb_epf_dev *ndev;
> > @@ -671,17 +681,16 @@ static int ntb_epf_pci_probe(struct pci_dev *pdev,
> >
> >       data = (struct ntb_epf_data *)id->driver_data;
> >       if (data) {
> > -             if (data->peer_spad_reg_bar)
> > -                     peer_spad_reg_bar = data->peer_spad_reg_bar;
> > -             if (data->ctrl_reg_bar)
> > -                     ctrl_reg_bar = data->ctrl_reg_bar;
> > -             if (data->db_reg_bar)
> > -                     db_reg_bar = data->db_reg_bar;
> > +             peer_spad_reg_bar = data->peer_spad_reg_bar;
> > +             ctrl_reg_bar = data->ctrl_reg_bar;
> > +             db_reg_bar = data->db_reg_bar;
> > +             mw_bar = data->mw_bar;
> >       }
> >
> >       ndev->peer_spad_reg_bar = peer_spad_reg_bar;
> >       ndev->ctrl_reg_bar = ctrl_reg_bar;
> >       ndev->db_reg_bar = db_reg_bar;
> > +     ndev->mw_bar = mw_bar;
> >       ndev->dev = dev;
> >
> >       ntb_epf_init_struct(ndev, pdev);
> > @@ -729,6 +738,14 @@ static const struct ntb_epf_data j721e_data = {
> >       .ctrl_reg_bar = BAR_0,
> >       .peer_spad_reg_bar = BAR_1,
> >       .db_reg_bar = BAR_2,
> > +     .mw_bar = BAR_2,
> > +};
> > +
> > +static const struct ntb_epf_data mx8_data = {
> > +     .ctrl_reg_bar = BAR_0,
> > +     .peer_spad_reg_bar = BAR_0,
> > +     .db_reg_bar = BAR_2,
> > +     .mw_bar = BAR_4,
> >  };
> >
> >  static const struct pci_device_id ntb_epf_pci_tbl[] = {
> > @@ -737,6 +754,11 @@ static const struct pci_device_id ntb_epf_pci_tbl[] = {
> >               .class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00,
> >               .driver_data = (kernel_ulong_t)&j721e_data,
> >       },
> > +     {
> > +             PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, 0x0809),
> > +             .class = PCI_CLASS_MEMORY_RAM << 8, .class_mask = 0xffff00,
> > +             .driver_data = (kernel_ulong_t)&mx8_data,
> > +     },
> >       { },
> >  };
> >
> > --
> > 2.24.0.rc1
> >

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection
  2022-02-15  5:38 ` [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection Frank Li
  2022-02-15  7:57   ` kernel test robot
  2022-02-16  7:13     ` kernel test robot
@ 2022-02-17 22:38   ` Bjorn Helgaas
  2022-02-17 22:59     ` Zhi Li
  2 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2022-02-17 22:38 UTC (permalink / raw)
  To: Frank Li
  Cc: linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, lznuaa, hongxing.zhu, Jon Mason, Dave Jiang,
	Allen Hubbe, linux-ntb

[+cc Jon, Dave, Allen, linux-ntb]

This patch only touches drivers/pci; the subject line should reflect
that to help people know which things to look at.

Maybe something like:

  PCI: endpoint: Support NTB transfer between RC and EP

On Mon, Feb 14, 2022 at 11:38:43PM -0600, Frank Li wrote:
> Add NTB function driver and virtual PCI BUS and Virtual NTB driver
> to implement communication between PCIe RC and PCIe EP devices

s/PCI BUS/PCI Bus/
s/PCIe RC/PCIe Root Port/ (I think; the RC itself doesn't have a
software representation)

> ┌────────────┐         ┌─────────────────────────────────────┐
> │            │         │                                     │
> ├────────────┤         │                      ┌──────────────┤
> │ NTB        │         │                      │ NTB          │
> │ NetDev     │         │                      │ NetDev       │
> ├────────────┤         │                      ├──────────────┤
> │ NTB        │         │                      │ NTB          │
> │ Transfer   │         │                      │ Transfer     │
> ├────────────┤         │                      ├──────────────┤
> │            │         │                      │              │
> │  PCI NTB   │         │                      │              │
> │    EPF     │         │                      │              │
> │   Driver   │         │                      │ PCI Virtual  │
> │            │         ├───────────────┐      │ NTB Driver   │
> │            │         │ PCI EP NTB    │◄────►│              │
> │            │         │  FN Driver    │      │              │
> ├────────────┤         ├───────────────┤      ├──────────────┤
> │            │         │               │      │              │
> │  PCI BUS   │ ◄─────► │  PCI EP BUS   │      │  Virtual PCI │
> │            │  PCI    │               │      │     BUS      │
> └────────────┘         └───────────────┴──────┴──────────────┘

s/PCI BUS/PCI Bus/

>     PCI RC                        PCI EP

s/PCI RC/PCIe Root Port/ ?

> This driver include 3 part:
>  1 PCI EP NTB function driver
>  2 Virtual PCI bus
>  3 PCI virutal NTB driver, which is loaded only by above virtual pci bus

s/include 3 part/includes 3 parts/
s/virutal/virtual/
s/pci bus/PCI bus/

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/pci/endpoint/functions/Kconfig        |   11 +
>  drivers/pci/endpoint/functions/Makefile       |    1 +
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 1425 +++++++++++++++++
>  3 files changed, 1437 insertions(+)
>  create mode 100644 drivers/pci/endpoint/functions/pci-epf-vntb.c
> 
> diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
> index 5f1242ca2f4e4..362555b024e8f 100644
> --- a/drivers/pci/endpoint/functions/Kconfig
> +++ b/drivers/pci/endpoint/functions/Kconfig
> @@ -25,3 +25,14 @@ config PCI_EPF_NTB
>  	  device tree.
>  
>  	  If in doubt, say "N" to disable Endpoint NTB driver.
> +
> +config PCI_EPF_VNTB
> +        tristate "PCI Endpoint NTB driver"
> +        depends on PCI_ENDPOINT
> +        select CONFIGFS_FS
> +        help
> +          Select this configuration option to enable the Non-Transparent
> +          Bridge (NTB) driver for PCI Endpoint. NTB driver implements NTB
> +          between PCI host and PCIe Endpoint.

"PCI Endpoint" vs "PCIe Endpoint".  Pick one.  It seems like you
require an upstream bridge, i.e., a Root Port, so maybe "PCIe Root
Port" and "PCIe Endpoint" is what you want?

> +          If in doubt, say "N" to disable Endpoint NTB driver.
> diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
> index 96ab932a537a2..5c13001deaba1 100644
> --- a/drivers/pci/endpoint/functions/Makefile
> +++ b/drivers/pci/endpoint/functions/Makefile
> @@ -5,3 +5,4 @@
>  
>  obj-$(CONFIG_PCI_EPF_TEST)		+= pci-epf-test.o
>  obj-$(CONFIG_PCI_EPF_NTB)		+= pci-epf-ntb.o
> +obj-$(CONFIG_PCI_EPF_VNTB) 		+= pci-epf-vntb.o
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> new file mode 100644
> index 0000000000000..ebf7e243eefa4
> --- /dev/null
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -0,0 +1,1425 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Endpoint Function Driver to implement Non-Transparent Bridge functionality
> + * Between PCI RC and EP
> + *
> + * Copyright (C) 2020 Texas Instruments
> + * Copyright (C) 2022 NXP
> + *
> + * Based on pci-epf-ntb.c
> + * Author: Frank Li <Frank.Li@nxp.com>
> + * Author: Kishon Vijay Abraham I <kishon@ti.com>
> + */
> +
> +/**
> + * +------------+         +---------------------------------------+
> + * |            |         |                                       |
> + * +------------+         |                        +--------------+
> + * | NTB        |         |                        | NTB          |
> + * | NetDev     |         |                        | NetDev       |
> + * +------------+         |                        +--------------+
> + * | NTB        |         |                        | NTB          |
> + * | Transfer   |         |                        | Transfer     |
> + * +------------+         |                        +--------------+
> + * |            |         |                        |              |
> + * |  PCI NTB   |         |                        |              |
> + * |    EPF     |         |                        |              |
> + * |   Driver   |         |                        | PCI Virtual  |
> + * |            |         +---------------+        | NTB Driver   |
> + * |            |         | PCI EP NTB    |<------>|              |
> + * |            |         |  FN Driver    |        |              |
> + * +------------+         +---------------+        +--------------+
> + * |            |         |               |        |              |
> + * |  PCI BUS   | <-----> |  PCI EP BUS   |        |  Virtual PCI |
> + * |            |  PCI    |               |        |     BUS      |
> + * +------------+         +---------------+--------+--------------+
> + *     PCI RC                        PCI EP

s/PCI RC/PCIe Root Port/ ?

> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +#include <linux/pci-epc.h>
> +#include <linux/pci-epf.h>
> +#include <linux/ntb.h>
> +
> +static struct workqueue_struct *kpcintb_workqueue;
> +
> +#define COMMAND_CONFIGURE_DOORBELL	1
> +#define COMMAND_TEARDOWN_DOORBELL	2
> +#define COMMAND_CONFIGURE_MW		3
> +#define COMMAND_TEARDOWN_MW		4
> +#define COMMAND_LINK_UP			5
> +#define COMMAND_LINK_DOWN		6
> +
> +#define COMMAND_STATUS_OK		1
> +#define COMMAND_STATUS_ERROR		2
> +
> +#define LINK_STATUS_UP			BIT(0)
> +
> +#define SPAD_COUNT			64
> +#define DB_COUNT			4
> +#define NTB_MW_OFFSET			2
> +#define DB_COUNT_MASK			GENMASK(15, 0)
> +#define MSIX_ENABLE			BIT(16)
> +#define MAX_DB_COUNT			32
> +#define MAX_MW				4
> +
> +#define VNTB_VID			0x1957

This looks like PCI_VENDOR_ID_NXP (or PCI_VENDOR_ID_FREESCALE).
How did you choose that, why is it safe to use, and why don't use use
PCI_VENDOR_ID_NXP instead?

> +#define VNTB_PID			0x080A
> +
> +enum epf_ntb_bar {
> +	BAR_CONFIG,
> +	BAR_DB,
> +	BAR_MW0,
> +	BAR_MW1,
> +	BAR_MW2,
> +};
> +
> +/*
> + * +--------------------------------------------------+ Base
> + * |                                                  |
> + * |                                                  |
> + * |                                                  |
> + * |          Common Control Register                 |
> + * |                                                  |
> + * |                                                  |
> + * |                                                  |
> + * +-----------------------+--------------------------+ Base+span_offset
> + * |                       |                          |
> + * |    Peer Span Space    |    Span Space            |
> + * |                       |                          |
> + * |                       |                          |
> + * +-----------------------+--------------------------+ Base+span_offset
> + * |                       |                          |     +span_count * 4
> + * |                       |                          |
> + * |     Span Space        |   Peer Span Space        |
> + * |                       |                          |
> + * +-----------------------+--------------------------+
> + *       Virtual PCI             Pcie Endpoint
> + *       NTB Driver               NTB Driver

s/Pcie/PCIe/

> + */
> +struct epf_ntb_ctrl {
> +	u32     command;
> +	u32     argument;
> +	u16     command_status;
> +	u16     link_status;
> +	u32     topology;
> +	u64     addr;
> +	u64     size;
> +	u32     num_mws;
> +	u32	reserved;
> +	u32     spad_offset;
> +	u32     spad_count;
> +	u32	db_entry_size;
> +	u32     db_data[MAX_DB_COUNT];
> +	u32     db_offset[MAX_DB_COUNT];
> +} __packed;
> +
> +struct epf_ntb {
> +	struct ntb_dev ntb;
> +	struct pci_epf *epf;
> +	struct config_group group;
> +
> +	u32 num_mws;
> +	u32 db_count;
> +	u32 spad_count;
> +	u64 mws_size[MAX_MW];
> +	u64 db;
> +	u32 vbus_number;
> +
> +	bool linkup;
> +	u32 spad_size;
> +
> +	enum pci_barno epf_ntb_bar[6];
> +
> +	struct epf_ntb_ctrl *reg;
> +
> +	phys_addr_t epf_db_phy;
> +	void __iomem *epf_db;
> +
> +	phys_addr_t vpci_mw_phy[MAX_MW];
> +	void __iomem *vpci_mw_addr[MAX_MW];
> +
> +	struct delayed_work cmd_handler;
> +};
> +
> +#define to_epf_ntb(epf_group) container_of((epf_group), struct epf_ntb, group)
> +#define ntb_ndev(__ntb) container_of(__ntb, struct epf_ntb, ntb)
> +
> +static struct pci_epf_header epf_ntb_header = {
> +	.vendorid	= PCI_ANY_ID,
> +	.deviceid	= PCI_ANY_ID,
> +	.baseclass_code	= PCI_BASE_CLASS_MEMORY,
> +	.interrupt_pin	= PCI_INTERRUPT_INTA,
> +};
> +
> +/**
> + * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host
> + * @ntb: NTB device that facilitates communication between HOST and VHOST
> + * @link_up: true or false indicating Link is UP or Down
> + *
> + * Once NTB function in HOST invoke ntb_link_enable(),
> + * this NTB function driver will trigger a link event to vhost.
> + */
> +static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
> +{
> +	if (link_up)
> +		ntb->reg->link_status |= LINK_STATUS_UP;
> +	else
> +		ntb->reg->link_status &= ~LINK_STATUS_UP;
> +
> +	ntb_link_event(&ntb->ntb);
> +	return 0;
> +}
> +
> +/**
> + * epf_ntb_configure_mw() - Configure the Outbound Address Space for vhost
> + *   to access the memory window of host
> + * @ntb: NTB device that facilitates communication between host and vhost
> + * @mw: Index of the memory window (either 0, 1, 2 or 3)
> + *
> + *                          EP Outbound Window
> + * +--------+              +-----------+
> + * |        |              |           |
> + * |        |              |           |
> + * |        |              |           |
> + * |        |              |           |
> + * |        |              +-----------+
> + * | Virtual|              | Memory Win|
> + * | NTB    | -----------> |           |
> + * | Driver |              |           |
> + * |        |              +-----------+
> + * |        |              |           |
> + * |        |              |           |
> + * +--------+              +-----------+
> + *  VHost                   PCI EP
> + */
> +static int epf_ntb_configure_mw(struct epf_ntb *ntb, u32 mw)
> +{
> +	phys_addr_t phys_addr;
> +	u8 func_no, vfunc_no;
> +	u64 addr, size;
> +	int ret = 0;
> +
> +	phys_addr = ntb->vpci_mw_phy[mw];
> +	addr = ntb->reg->addr;
> +	size = ntb->reg->size;
> +
> +	func_no = ntb->epf->func_no;
> +	vfunc_no = ntb->epf->vfunc_no;
> +
> +	ret = pci_epc_map_addr(ntb->epf->epc, func_no, vfunc_no, phys_addr, addr, size);
> +	if (ret)
> +		dev_err(&ntb->epf->epc->dev,
> +			"intf: Failed to map memory window %d address\n", mw);

Lots of these messages start with "intf:".  What is that telling us?
Is it useful?

> +	return ret;
> +}
> +
> +/**
> + * epf_ntb_teardown_mw() - Teardown the configured OB ATU
> + * @ntb: NTB device that facilitates communication between HOST and vHOST
> + * @mw: Index of the memory window (either 0, 1, 2 or 3)
> + *
> + * Teardown the configured OB ATU configured in epf_ntb_configure_mw() using
> + * pci_epc_unmap_addr()
> + */
> +static void epf_ntb_teardown_mw(struct epf_ntb *ntb, u32 mw)
> +{
> +	pci_epc_unmap_addr(ntb->epf->epc,
> +			   ntb->epf->func_no,
> +			   ntb->epf->vfunc_no,
> +			   ntb->vpci_mw_phy[mw]);
> +}
> +
> +/**
> + * epf_ntb_cmd_handler() - Handle commands provided by the NTB Host
> + * @work: work_struct for the epf_ntb_epc
> + *
> + * Workqueue function that gets invoked for the two epf_ntb_epc
> + * periodically (once every 5ms) to see if it has received any commands
> + * from NTB host. The host can send commands to configure doorbell or
> + * configure memory window or to update link status.
> + */
> +static void epf_ntb_cmd_handler(struct work_struct *work)
> +{
> +	struct epf_ntb_ctrl *ctrl;
> +	u32 command, argument;
> +	struct epf_ntb *ntb;
> +	struct device *dev;
> +	int ret;
> +	int i;
> +
> +	ntb = container_of(work, struct epf_ntb, cmd_handler.work);
> +
> +	for (i = 1; i < ntb->db_count; i++) {
> +		if (readl(ntb->epf_db + i * 4)) {
> +			if (readl(ntb->epf_db + i * 4))
> +				ntb->db |= 1 << (i - 1);
> +
> +			ntb_db_event(&ntb->ntb, i);
> +			writel(0, ntb->epf_db + i * 4);
> +		}
> +	}
> +
> +	ctrl = ntb->reg;
> +	command = ctrl->command;
> +	if (!command)
> +		goto reset_handler;
> +	argument = ctrl->argument;
> +
> +	ctrl->command = 0;
> +	ctrl->argument = 0;
> +
> +	ctrl = ntb->reg;
> +	dev = &ntb->epf->dev;
> +
> +	switch (command) {
> +	case COMMAND_CONFIGURE_DOORBELL:
> +		ctrl->command_status = COMMAND_STATUS_OK;
> +		break;
> +	case COMMAND_TEARDOWN_DOORBELL:
> +		ctrl->command_status = COMMAND_STATUS_OK;
> +		break;
> +	case COMMAND_CONFIGURE_MW:
> +		ret = epf_ntb_configure_mw(ntb, argument);
> +		if (ret < 0)
> +			ctrl->command_status = COMMAND_STATUS_ERROR;
> +		else
> +			ctrl->command_status = COMMAND_STATUS_OK;
> +		break;
> +	case COMMAND_TEARDOWN_MW:
> +		epf_ntb_teardown_mw(ntb, argument);
> +		ctrl->command_status = COMMAND_STATUS_OK;
> +		break;
> +	case COMMAND_LINK_UP:
> +		ntb->linkup = true;
> +		ret = epf_ntb_link_up(ntb, true);
> +		if (ret < 0)
> +			ctrl->command_status = COMMAND_STATUS_ERROR;
> +		else
> +			ctrl->command_status = COMMAND_STATUS_OK;
> +		goto reset_handler;
> +	case COMMAND_LINK_DOWN:
> +		ntb->linkup = false;
> +		ret = epf_ntb_link_up(ntb, false);
> +		if (ret < 0)
> +			ctrl->command_status = COMMAND_STATUS_ERROR;
> +		else
> +			ctrl->command_status = COMMAND_STATUS_OK;
> +		break;
> +	default:
> +		dev_err(dev, "intf UNKNOWN command: %d\n", command);
> +		break;
> +	}
> +
> +reset_handler:
> +	queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
> +			   msecs_to_jiffies(5));
> +}
> +
> +/**
> + * epf_ntb_config_sspad_bar_clear() - Clear Config + Self scratchpad BAR
> + * @ntb_epc: EPC associated with one of the HOST which holds peer's outbound
> + *	     address.
> + *
> + * Clear BAR0 of EP CONTROLLER 1 which contains the HOST1's config and
> + * self scratchpad region (removes inbound ATU configuration). While BAR0 is
> + * the default self scratchpad BAR, an NTB could have other BARs for self
> + * scratchpad (because of reserved BARs). This function can get the exact BAR
> + * used for self scratchpad from epf_ntb_bar[BAR_CONFIG].
> + *
> + * Please note the self scratchpad region and config region is combined to
> + * a single region and mapped using the same BAR. Also note HOST2's peer
> + * scratchpad is HOST1's self scratchpad.
> + */
> +static void epf_ntb_config_sspad_bar_clear(struct epf_ntb *ntb)
> +{
> +	struct pci_epf_bar *epf_bar;
> +	enum pci_barno barno;
> +
> +	barno = ntb->epf_ntb_bar[BAR_CONFIG];
> +	epf_bar = &ntb->epf->bar[barno];
> +
> +	pci_epc_clear_bar(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no, epf_bar);
> +}
> +
> +/**
> + * epf_ntb_config_sspad_bar_set() - Set Config + Self scratchpad BAR
> + * @ntb: NTB device that facilitates communication between HOST and vHOST
> + *
> + * Map BAR0 of EP CONTROLLER 1 which contains the HOST1's config and
> + * self scratchpad region.
> + *
> + * Please note the self scratchpad region and config region is combined to
> + * a single region and mapped using the same BAR.
> + */
> +static int epf_ntb_config_sspad_bar_set(struct epf_ntb *ntb)
> +{
> +	struct pci_epf_bar *epf_bar;
> +	enum pci_barno barno;
> +	u8 func_no, vfunc_no;
> +	struct device *dev;
> +	int ret;
> +
> +	dev = &ntb->epf->dev;
> +	func_no = ntb->epf->func_no;
> +	vfunc_no = ntb->epf->vfunc_no;
> +	barno = ntb->epf_ntb_bar[BAR_CONFIG];
> +	epf_bar = &ntb->epf->bar[barno];
> +
> +	ret = pci_epc_set_bar(ntb->epf->epc, func_no, vfunc_no, epf_bar);
> +	if (ret) {
> +		dev_err(dev, "inft: Config/Status/SPAD BAR set failed\n");
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +/**
> + * epf_ntb_config_spad_bar_free() - Free the physical memory associated with
> + *   config + scratchpad region
> + * @ntb: NTB device that facilitates communication between HOST and vHOST
> + */
> +static void epf_ntb_config_spad_bar_free(struct epf_ntb *ntb)
> +{
> +	enum pci_barno barno;
> +
> +	barno = ntb->epf_ntb_bar[BAR_CONFIG];
> +	pci_epf_free_space(ntb->epf, ntb->reg, barno, 0);
> +}
> +
> +/**
> + * epf_ntb_config_spad_bar_alloc() - Allocate memory for config + scratchpad
> + *   region
> + * @ntb: NTB device that facilitates communication between HOST1 and HOST2
> + *
> + * Allocate the Local Memory mentioned in the above diagram. The size of
> + * CONFIG REGION is sizeof(struct epf_ntb_ctrl) and size of SCRATCHPAD REGION
> + * is obtained from "spad-count" configfs entry.
> + */
> +static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
> +{
> +	size_t align;
> +	enum pci_barno barno;
> +	struct epf_ntb_ctrl *ctrl;
> +	u32 spad_size, ctrl_size;
> +	u64 size;
> +	struct pci_epf *epf = ntb->epf;
> +	struct device *dev = &epf->dev;
> +	u32 spad_count;
> +	void *base;
> +	int i;
> +	const struct pci_epc_features *epc_features = pci_epc_get_features(epf->epc,
> +								epf->func_no,
> +								epf->vfunc_no);
> +	barno = ntb->epf_ntb_bar[BAR_CONFIG];
> +	size = epc_features->bar_fixed_size[barno];
> +	align = epc_features->align;
> +
> +	if ((!IS_ALIGNED(size, align)))
> +		return -EINVAL;
> +
> +	spad_count = ntb->spad_count;
> +
> +	ctrl_size = sizeof(struct epf_ntb_ctrl);
> +	spad_size = 2 * spad_count * 4;
> +
> +	if (!align) {
> +		ctrl_size = roundup_pow_of_two(ctrl_size);
> +		spad_size = roundup_pow_of_two(spad_size);
> +	} else {
> +		ctrl_size = ALIGN(ctrl_size, align);
> +		spad_size = ALIGN(spad_size, align);
> +	}
> +
> +	if (!size)
> +		size = ctrl_size + spad_size;
> +	else if (size < ctrl_size + spad_size)
> +		return -EINVAL;
> +
> +	base = pci_epf_alloc_space(epf, size, barno, align, 0);
> +	if (!base) {
> +		dev_err(dev, "intf: Config/Status/SPAD alloc region fail\n");
> +		return -ENOMEM;
> +	}
> +
> +	ntb->reg = base;
> +
> +	ctrl = ntb->reg;
> +	ctrl->spad_offset = ctrl_size;
> +
> +	ctrl->spad_count = spad_count;
> +	ctrl->num_mws = ntb->num_mws;
> +	ntb->spad_size = spad_size;
> +
> +	ctrl->db_entry_size = 4;
> +
> +	for (i = 0; i < ntb->db_count; i++) {
> +		ntb->reg->db_data[i] = 1 + i;
> +		ntb->reg->db_offset[i] = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * epf_ntb_configure_interrupt() - Configure MSI/MSI-X capaiblity
> + * @ntb: NTB device that facilitates communication between HOST and vHOST
> + *
> + * Configure MSI/MSI-X capability for each interface with number of
> + * interrupts equal to "db_count" configfs entry.
> + */
> +static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
> +{
> +	const struct pci_epc_features *epc_features;
> +	bool msix_capable, msi_capable;
> +	u8 func_no, vfunc_no;
> +	struct device *dev;
> +	u32 db_count;
> +	int ret;
> +
> +	dev = &ntb->epf->dev;
> +
> +	epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);
> +	msix_capable = epc_features->msix_capable;
> +	msi_capable = epc_features->msi_capable;
> +
> +	if (!(msix_capable || msi_capable)) {

I don't think the "msix_capable" and "msi_capable" local variables
really add any readability since they're only used once.

Not sure about "func_no", "vfunc_no", either.  If you keep them, you
could at least assign them earlier and use them in the
pci_epc_get_features() call.

> +		dev_err(dev, "MSI or MSI-X is required for doorbell\n");
> +		return -EINVAL;
> +	}
> +
> +	func_no = ntb->epf->func_no;
> +	vfunc_no = ntb->epf->vfunc_no;
> +
> +	db_count = ntb->db_count;
> +	if (db_count > MAX_DB_COUNT) {
> +		dev_err(dev, "DB count cannot be more than %d\n", MAX_DB_COUNT);
> +		return -EINVAL;
> +	}
> +
> +	ntb->db_count = db_count;
> +
> +	if (msi_capable) {
> +		ret = pci_epc_set_msi(ntb->epf->epc, func_no, vfunc_no, 16);
> +		if (ret) {
> +			dev_err(dev, "intf: MSI configuration failed\n");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * epf_ntb_db_bar_init() - Configure Doorbell window BARs
> + * @ntb: NTB device that facilitates communication between HOST and vHOST
> + *

Spurious blank line.

> + */
> +static int epf_ntb_db_bar_init(struct epf_ntb *ntb)
> +{
> +	const struct pci_epc_features *epc_features;
> +	u32 align;
> +	struct device *dev = &ntb->epf->dev;
> +	int ret;
> +	struct pci_epf_bar *epf_bar;
> +	void __iomem *mw_addr;
> +	enum pci_barno barno;
> +	size_t size = 4 * ntb->db_count;
> +
> +	epc_features = pci_epc_get_features(ntb->epf->epc,
> +					    ntb->epf->func_no,
> +					    ntb->epf->vfunc_no);
> +	align = epc_features->align;
> +
> +	if (size < 128)
> +		size = 128;
> +
> +	if (align)
> +		size = ALIGN(size, align);
> +	else
> +		size = roundup_pow_of_two(size);
> +
> +	barno = ntb->epf_ntb_bar[BAR_DB];
> +
> +	mw_addr = pci_epf_alloc_space(ntb->epf, size, barno, align, 0);
> +	if (!mw_addr) {
> +		dev_err(dev, "intf: Failed to allocate OB address\n");
> +		return -ENOMEM;
> +	}
> +
> +	ntb->epf_db = mw_addr;
> +
> +	epf_bar = &ntb->epf->bar[barno];
> +
> +	ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no, epf_bar);
> +	if (ret) {
> +		dev_err(dev, "intf: DoorBell BAR set failed\n");

s/DoorBell/Doorbell/

> +			goto err_alloc_peer_mem;
> +	}
> +	return ret;
> +
> +err_alloc_peer_mem:
> +	pci_epc_mem_free_addr(ntb->epf->epc, epf_bar->phys_addr, mw_addr, epf_bar->size);
> +	return -1;
> +}
> +
> +/**
> + * epf_ntb_db_bar_clear() - Clear doorbell BAR and free memory
> + *   allocated in peers outbound address space

s/peers/peer's/

> + * @ntb: NTB device that facilitates communication between HOST and vHOST
> + */
> +static void epf_ntb_db_bar_clear(struct epf_ntb *ntb)
> +{
> +	enum pci_barno barno;
> +
> +	barno = ntb->epf_ntb_bar[BAR_DB];
> +	pci_epf_free_space(ntb->epf, ntb->epf_db, barno, 0);
> +	pci_epc_clear_bar(ntb->epf->epc,
> +			  ntb->epf->func_no,
> +			  ntb->epf->vfunc_no,
> +			  &ntb->epf->bar[barno]);
> +}
> +
> +/**
> + * epf_ntb_mw_bar_init() - Configure Memory window BARs
> + * @ntb: NTB device that facilitates communication between HOST and vHOST
> + *
> + */
> +static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
> +{
> +	int ret = 0;
> +	int i;
> +	u64 size;
> +	enum pci_barno barno;
> +	struct device *dev = &ntb->epf->dev;
> +
> +	for (i = 0; i < ntb->num_mws; i++) {
> +

Spurious blank line.

> +		size = ntb->mws_size[i];
> +
> +		barno = ntb->epf_ntb_bar[BAR_MW0 + i];
> +
> +		ntb->epf->bar[barno].barno = barno;
> +		ntb->epf->bar[barno].size = size;
> +		ntb->epf->bar[barno].addr = 0;
> +		ntb->epf->bar[barno].phys_addr = 0;
> +		ntb->epf->bar[barno].flags |= upper_32_bits(size) ?
> +				PCI_BASE_ADDRESS_MEM_TYPE_64 :
> +				PCI_BASE_ADDRESS_MEM_TYPE_32;
> +
> +		ret = pci_epc_set_bar(ntb->epf->epc,
> +				      ntb->epf->func_no,
> +				      ntb->epf->vfunc_no,
> +				      &ntb->epf->bar[barno]);
> +		if (ret) {
> +			dev_err(dev, "intf: MW set failed\n");
> +			goto err_alloc_mem;
> +		}
> +
> +		/* allocate epc outbound memory windows to vpci vntb device */

s/allocate/Allocate/ to match other comments
s/epc/EPC/

> +		ntb->vpci_mw_addr[i] = pci_epc_mem_alloc_addr(ntb->epf->epc,
> +							      &ntb->vpci_mw_phy[i],
> +							      size);
> +		if (!ntb->vpci_mw_addr[i]) {
> +			dev_err(dev, "Failed to allocate source address\n");
> +			goto err_alloc_mem;
> +		}
> +	}
> +
> +	return ret;
> +err_alloc_mem:
> +	return ret;
> +}
> +
> +/**
> + * epf_ntb_mw_bar_clear() - Clear Memory window BARs
> + * @ntb: NTB device that facilitates communication between HOST and vHOST
> + *

Spurious blank line.

> + */
> +static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb)
> +{
> +	enum pci_barno barno;
> +	int i;
> +
> +	for (i = 0; i < ntb->num_mws; i++) {
> +		barno = ntb->epf_ntb_bar[BAR_MW0 + i];
> +		pci_epc_clear_bar(ntb->epf->epc,
> +				  ntb->epf->func_no,
> +				  ntb->epf->vfunc_no,
> +				  &ntb->epf->bar[barno]);
> +
> +		pci_epc_mem_free_addr(ntb->epf->epc,
> +				      ntb->vpci_mw_phy[i],
> +				      ntb->vpci_mw_addr[i],
> +				      ntb->mws_size[i]);
> +	}
> +}
> +
> +/**
> + * epf_ntb_epc_destroy() - Cleanup NTB EPC interface
> + * @ntb: NTB device that facilitates communication between HOST and vHOST
> + *
> + * Wrapper for epf_ntb_epc_destroy_interface() to cleanup all the NTB interfaces
> + */
> +static void epf_ntb_epc_destroy(struct epf_ntb *ntb)
> +{
> +	pci_epc_remove_epf(ntb->epf->epc, ntb->epf, 0);
> +	pci_epc_put(ntb->epf->epc);
> +}
> +
> +/**
> + * epf_ntb_init_epc_bar() - Identify BARs to be used for each of the NTB
> + * constructs (scratchpad region, doorbell, memorywindow)
> + * @ntb: NTB device that facilitates communication between HOST and vHOST
> + *

Spurious blank line.

> + */
> +static int epf_ntb_init_epc_bar(struct epf_ntb *ntb)
> +{
> +	const struct pci_epc_features *epc_features;
> +	enum pci_barno barno;
> +	enum epf_ntb_bar bar;
> +	struct device *dev;
> +	u32 num_mws;
> +	int i;
> +
> +	barno = BAR_0;
> +	num_mws = ntb->num_mws;
> +	dev = &ntb->epf->dev;
> +	epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);
> +
> +	/* These are required BARs which are mandatory for NTB functionality */
> +	for (bar = BAR_CONFIG; bar <= BAR_MW0; bar++, barno++) {
> +		barno = pci_epc_get_next_free_bar(epc_features, barno);
> +		if (barno < 0) {
> +			dev_err(dev, "intf: Fail to get NTB function BAR\n");
> +			return barno;
> +		}
> +		ntb->epf_ntb_bar[bar] = barno;
> +	}
> +
> +	/* These are optional BARs which don't impact NTB functionality */
> +	for (bar = BAR_MW1, i = 1; i < num_mws; bar++, barno++, i++) {
> +		barno = pci_epc_get_next_free_bar(epc_features, barno);
> +		if (barno < 0) {
> +			ntb->num_mws = i;
> +			dev_dbg(dev, "BAR not available for > MW%d\n", i + 1);
> +		}
> +		ntb->epf_ntb_bar[bar] = barno;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * epf_ntb_epc_init() - Initialize NTB interface
> + * @ntb: NTB device that facilitates communication between HOST and vHOST2
> + *
> + * Wrapper to initialize a particular EPC interface and start the workqueue
> + * to check for commands from host. This function will write to the
> + * EP controller HW for configuring it.
> + */
> +static int epf_ntb_epc_init(struct epf_ntb *ntb)
> +{
> +	u8 func_no, vfunc_no;
> +	struct pci_epc *epc;
> +	struct pci_epf *epf;
> +	struct device *dev;
> +	int ret;
> +
> +	epf = ntb->epf;
> +	dev = &epf->dev;
> +	epc = epf->epc;
> +	func_no = ntb->epf->func_no;
> +	vfunc_no = ntb->epf->vfunc_no;
> +
> +	ret = epf_ntb_config_sspad_bar_set(ntb);
> +	if (ret) {
> +		dev_err(dev, "intf: Config/self SPAD BAR init failed");
> +		return ret;
> +	}
> +
> +	ret = epf_ntb_configure_interrupt(ntb);
> +	if (ret) {
> +		dev_err(dev, "intf: Interrupt configuration failed\n");
> +		goto err_config_interrupt;
> +	}
> +
> +	ret = epf_ntb_db_bar_init(ntb);
> +	if (ret) {
> +		dev_err(dev, "intf: DB BAR init failed\n");
> +		goto err_db_bar_init;
> +	}
> +
> +	ret = epf_ntb_mw_bar_init(ntb);
> +	if (ret) {
> +		dev_err(dev, "intf: MW BAR init failed\n");
> +		goto err_mw_bar_init;
> +	}
> +
> +	if (vfunc_no <= 1) {
> +		ret = pci_epc_write_header(epc, func_no, vfunc_no, epf->header);
> +		if (ret) {
> +			dev_err(dev, "intf: Configuration header write failed\n");
> +			goto err_write_header;
> +		}
> +	}
> +
> +	INIT_DELAYED_WORK(&ntb->cmd_handler, epf_ntb_cmd_handler);
> +	queue_work(kpcintb_workqueue, &ntb->cmd_handler.work);
> +
> +	return 0;
> +
> +err_write_header:
> +	epf_ntb_mw_bar_clear(ntb);
> +err_mw_bar_init:
> +	epf_ntb_db_bar_clear(ntb);
> +err_db_bar_init:
> +err_config_interrupt:
> +	epf_ntb_config_sspad_bar_clear(ntb);
> +
> +	return ret;
> +}
> +
> +
> +/**
> + * epf_ntb_epc_cleanup() - Cleanup all NTB interfaces
> + * @ntb: NTB device that facilitates communication between HOST1 and HOST2
> + *
> + * Wrapper to cleanup all NTB interfaces.
> + */
> +static void epf_ntb_epc_cleanup(struct epf_ntb *ntb)
> +{
> +	epf_ntb_db_bar_clear(ntb);
> +	epf_ntb_mw_bar_clear(ntb);
> +}
> +
> +#define EPF_NTB_R(_name)						\
> +static ssize_t epf_ntb_##_name##_show(struct config_item *item,		\
> +				      char *page)			\
> +{									\
> +	struct config_group *group = to_config_group(item);		\
> +	struct epf_ntb *ntb = to_epf_ntb(group);			\
> +									\
> +	return sprintf(page, "%d\n", ntb->_name);			\
> +}
> +
> +#define EPF_NTB_W(_name)						\
> +static ssize_t epf_ntb_##_name##_store(struct config_item *item,	\
> +				       const char *page, size_t len)	\
> +{									\
> +	struct config_group *group = to_config_group(item);		\
> +	struct epf_ntb *ntb = to_epf_ntb(group);			\
> +	u32 val;							\
> +	int ret;							\
> +									\
> +	ret = kstrtou32(page, 0, &val);					\
> +	if (ret)							\
> +		return ret;						\
> +									\
> +	ntb->_name = val;						\
> +									\
> +	return len;							\
> +}
> +
> +#define EPF_NTB_MW_R(_name)						\
> +static ssize_t epf_ntb_##_name##_show(struct config_item *item,		\
> +				      char *page)			\
> +{									\
> +	struct config_group *group = to_config_group(item);		\
> +	struct epf_ntb *ntb = to_epf_ntb(group);			\
> +	int win_no;							\
> +									\
> +	sscanf(#_name, "mw%d", &win_no);				\
> +									\
> +	return sprintf(page, "%lld\n", ntb->mws_size[win_no - 1]);	\
> +}
> +
> +#define EPF_NTB_MW_W(_name)						\
> +static ssize_t epf_ntb_##_name##_store(struct config_item *item,	\
> +				       const char *page, size_t len)	\
> +{									\
> +	struct config_group *group = to_config_group(item);		\
> +	struct epf_ntb *ntb = to_epf_ntb(group);			\
> +	struct device *dev = &ntb->epf->dev;				\
> +	int win_no;							\
> +	u64 val;							\
> +	int ret;							\
> +									\
> +	ret = kstrtou64(page, 0, &val);					\
> +	if (ret)							\
> +		return ret;						\
> +									\
> +	if (sscanf(#_name, "mw%d", &win_no) != 1)			\
> +		return -EINVAL;						\
> +									\
> +	if (ntb->num_mws < win_no) {					\
> +		dev_err(dev, "Invalid num_nws: %d value\n", ntb->num_mws); \
> +		return -EINVAL;						\
> +	}								\
> +									\
> +	ntb->mws_size[win_no - 1] = val;				\
> +									\
> +	return len;							\
> +}
> +
> +static ssize_t epf_ntb_num_mws_store(struct config_item *item,
> +				     const char *page, size_t len)
> +{
> +	struct config_group *group = to_config_group(item);
> +	struct epf_ntb *ntb = to_epf_ntb(group);
> +	u32 val;
> +	int ret;
> +
> +	ret = kstrtou32(page, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val > MAX_MW)
> +		return -EINVAL;
> +
> +	ntb->num_mws = val;
> +
> +	return len;
> +}
> +
> +EPF_NTB_R(spad_count)
> +EPF_NTB_W(spad_count)
> +EPF_NTB_R(db_count)
> +EPF_NTB_W(db_count)
> +EPF_NTB_R(num_mws)
> +EPF_NTB_R(vbus_number)
> +EPF_NTB_W(vbus_number)
> +EPF_NTB_MW_R(mw1)
> +EPF_NTB_MW_W(mw1)
> +EPF_NTB_MW_R(mw2)
> +EPF_NTB_MW_W(mw2)
> +EPF_NTB_MW_R(mw3)
> +EPF_NTB_MW_W(mw3)
> +EPF_NTB_MW_R(mw4)
> +EPF_NTB_MW_W(mw4)
> +
> +CONFIGFS_ATTR(epf_ntb_, spad_count);
> +CONFIGFS_ATTR(epf_ntb_, db_count);
> +CONFIGFS_ATTR(epf_ntb_, num_mws);
> +CONFIGFS_ATTR(epf_ntb_, mw1);
> +CONFIGFS_ATTR(epf_ntb_, mw2);
> +CONFIGFS_ATTR(epf_ntb_, mw3);
> +CONFIGFS_ATTR(epf_ntb_, mw4);
> +CONFIGFS_ATTR(epf_ntb_, vbus_number);
> +
> +static struct configfs_attribute *epf_ntb_attrs[] = {
> +	&epf_ntb_attr_spad_count,
> +	&epf_ntb_attr_db_count,
> +	&epf_ntb_attr_num_mws,
> +	&epf_ntb_attr_mw1,
> +	&epf_ntb_attr_mw2,
> +	&epf_ntb_attr_mw3,
> +	&epf_ntb_attr_mw4,
> +	&epf_ntb_attr_vbus_number,
> +	NULL,
> +};
> +
> +static const struct config_item_type ntb_group_type = {
> +	.ct_attrs	= epf_ntb_attrs,
> +	.ct_owner	= THIS_MODULE,
> +};
> +
> +/**
> + * epf_ntb_add_cfs() - Add configfs directory specific to NTB
> + * @epf: NTB endpoint function device
> + * @group: A pointer to the config_group structure referencing a group of
> + *	   config_items of a specific type that belong to a specific sub-system.
> + *
> + * Add configfs directory specific to NTB. This directory will hold
> + * NTB specific properties like db_count, spad_count, num_mws etc.,
> + */
> +static struct config_group *epf_ntb_add_cfs(struct pci_epf *epf,
> +					    struct config_group *group)
> +{
> +	struct epf_ntb *ntb = epf_get_drvdata(epf);
> +	struct config_group *ntb_group = &ntb->group;
> +	struct device *dev = &epf->dev;
> +
> +	config_group_init_type_name(ntb_group, dev_name(dev), &ntb_group_type);
> +
> +	return ntb_group;
> +}
> +
> +/*==== virtual PCI bus driver, which only load virutal ntb pci driver ====*/

s/virutal ntb pci/virtual NTB PCI/

> +#define VPCI_BUS_NUM 0x10

How did you pick this number and how do we know it is available?

> +uint32_t pci_space[] = {

"static u32"?

> +	(VNTB_VID | (VNTB_PID << 16)),	//DeviceID, Vendor ID
> +	0,		// status, Command
> +	0xffffffff,	// Class code, subclass, prog if, revision id
> +	0x40,		//bist, header type, latency Timer, cache line size
> +	0,		//bar 0
> +	0,		//bar 1
> +	0,		//bar 2
> +	0,		//bar 3
> +	0,		//bar 4
> +	0,		//bar 5

s/bar/BAR/

> +	0,		//cardbus cis point
> +	0,		//Subsystem ID Subystem vendor id
> +	0,		//ROM Base Address
> +	0,		//Reserved, Cap. Point
> +	0,		//Reserved,
> +	0,		//Max Lat, Min Gnt, interrupt pin, interrupt line

Use /* */ instead of //, like the rest of drivers/pci/

> +};
> +
> +int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
> +{
> +	if (devfn == 0) {
> +		memcpy(val, ((uint8_t *)pci_space) + where, size);

"u8 *"?

> +		return 0;
> +	}
> +	return -1;

These should return PCIBIOS_SUCCESSFUL or PCIBIOS_DEVICE_NOT_FOUND.

> +}
> +
> +int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
> +{
> +	return 0;
> +}
> +
> +struct pci_ops vpci_ops = {
> +	.read = pci_read,
> +	.write = pci_write,
> +};
> +
> +static int vpci_bus(void *sysdata)

The function name should say something about what it does.  Maybe
vpci_scan_bus() or something?

> +{
> +	struct pci_bus *vpci_bus;
> +
> +	vpci_bus = pci_scan_bus(VPCI_BUS_NUM, &vpci_ops, sysdata);
> +	if (vpci_bus)
> +		pr_err("create pci bus\n");
> +
> +	pci_bus_add_devices(vpci_bus);
> +
> +	return 0;
> +}
> +
> +/*==================== Virtual PCIe NTB driver ==========================*/
> +
> +static int vntb_epf_mw_count(struct ntb_dev *ntb, int pidx)
> +{
> +	struct epf_ntb *ndev = ntb_ndev(ntb);
> +
> +	return ndev->num_mws;
> +}
> +
> +static int vntb_epf_spad_count(struct ntb_dev *ntb)
> +{
> +	return ntb_ndev(ntb)->spad_count;
> +}
> +
> +static int vntb_epf_peer_mw_count(struct ntb_dev *ntb)
> +{
> +	return ntb_ndev(ntb)->num_mws;
> +}
> +
> +static u64 vntb_epf_db_valid_mask(struct ntb_dev *ntb)
> +{
> +	return BIT_ULL(ntb_ndev(ntb)->db_count) - 1;
> +}
> +
> +static int vntb_epf_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
> +{
> +	return 0;
> +}
> +
> +static int vntb_epf_mw_set_trans(struct ntb_dev *ndev, int pidx, int idx,
> +		dma_addr_t addr, resource_size_t size)
> +{
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +	struct pci_epf_bar *epf_bar;
> +	enum pci_barno barno;
> +	int ret;
> +	struct device *dev;
> +
> +	dev = &ntb->ntb.dev;
> +	barno = ntb->epf_ntb_bar[BAR_MW0 + idx];
> +	epf_bar = &ntb->epf->bar[barno];
> +	epf_bar->phys_addr = addr;
> +	epf_bar->barno = barno;
> +	epf_bar->size = size;
> +
> +	ret = pci_epc_set_bar(ntb->epf->epc, 0, 0, epf_bar);
> +	if (ret) {
> +		dev_err(dev, "failure set mw trans\n");
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static int vntb_epf_mw_clear_trans(struct ntb_dev *ntb, int pidx, int idx)
> +{
> +	return 0;
> +}
> +
> +static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
> +				phys_addr_t *base, resource_size_t *size)
> +{
> +
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +
> +	if (base)
> +		*base = ntb->vpci_mw_phy[idx];
> +
> +	if (size)
> +		*size = ntb->mws_size[idx];
> +
> +	return 0;
> +}
> +
> +static int vntb_epf_link_enable(struct ntb_dev *ntb,
> +			enum ntb_speed max_speed,
> +			enum ntb_width max_width)
> +{
> +	return 0;
> +}
> +
> +static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx)
> +{
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +	int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * 4;
> +	u32 val;
> +	void __iomem *base = ntb->reg;
> +
> +	val = readl(base + off + ct + idx * 4);
> +	return val;
> +}
> +
> +static int vntb_epf_spad_write(struct ntb_dev *ndev, int idx, u32 val)
> +{
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +	struct epf_ntb_ctrl *ctrl = ntb->reg;
> +	int off = ctrl->spad_offset, ct = ctrl->spad_count * 4;
> +	void __iomem *base = ntb->reg;
> +
> +	writel(val, base + off + ct + idx * 4);
> +	return 0;
> +}
> +
> +static u32 vntb_epf_peer_spad_read(struct ntb_dev *ndev, int pidx, int idx)
> +{
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +	struct epf_ntb_ctrl *ctrl = ntb->reg;
> +	int off = ctrl->spad_offset;
> +	void __iomem *base = ntb->reg;
> +	u32 val;
> +
> +	val = readl(base + off + idx * 4);
> +	return val;
> +}
> +
> +static int vntb_epf_peer_spad_write(struct ntb_dev *ndev, int pidx, int idx, u32 val)
> +{
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +	struct epf_ntb_ctrl *ctrl = ntb->reg;
> +	int off = ctrl->spad_offset;
> +	void __iomem *base = ntb->reg;
> +
> +	writel(val, base + off + idx * 4);
> +	return 0;
> +}
> +
> +static int vntb_epf_peer_db_set(struct ntb_dev *ndev, u64 db_bits)
> +{
> +	u32 interrupt_num = ffs(db_bits) + 1;
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +	u8 func_no, vfunc_no;
> +	int ret;
> +
> +	func_no = ntb->epf->func_no;
> +	vfunc_no = ntb->epf->vfunc_no;
> +
> +	ret = pci_epc_raise_irq(ntb->epf->epc,
> +				func_no,
> +				vfunc_no,
> +				PCI_EPC_IRQ_MSI,
> +				interrupt_num + 1);
> +	if (ret) {
> +		dev_err(&ntb->ntb.dev, "intf: Failed to raise IRQ\n");
> +		return ret;
> +	}
> +
> +	return 0;

Equivalent to:

  if (ret)
    dev_err(...);

  return ret;

> +}
> +
> +static u64 vntb_epf_db_read(struct ntb_dev *ndev)
> +{
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +
> +	return ntb->db;
> +}
> +
> +static int vntb_epf_mw_get_align(struct ntb_dev *ndev, int pidx, int idx,
> +			resource_size_t *addr_align,
> +			resource_size_t *size_align,
> +			resource_size_t *size_max)
> +{
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +
> +	if (addr_align)
> +		*addr_align = SZ_4K;
> +
> +	if (size_align)
> +		*size_align = 1;
> +
> +	if (size_max)
> +		*size_max = ntb->mws_size[idx];
> +
> +	return 0;
> +}
> +
> +static u64 vntb_epf_link_is_up(struct ntb_dev *ndev,
> +			enum ntb_speed *speed,
> +			enum ntb_width *width)
> +{
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +
> +	return ntb->reg->link_status;
> +}
> +
> +static int vntb_epf_db_clear_mask(struct ntb_dev *ndev, u64 db_bits)
> +{
> +	return 0;
> +}
> +
> +static int vntb_epf_db_clear(struct ntb_dev *ndev, u64 db_bits)
> +{
> +	struct epf_ntb *ntb = ntb_ndev(ndev);
> +
> +	ntb->db &= ~db_bits;
> +	return 0;
> +}
> +
> +static int vntb_epf_link_disable(struct ntb_dev *ntb)
> +{
> +	return 0;
> +}
> +
> +static const struct ntb_dev_ops vntb_epf_ops = {
> +	.mw_count		= vntb_epf_mw_count,
> +	.spad_count		= vntb_epf_spad_count,
> +	.peer_mw_count		= vntb_epf_peer_mw_count,
> +	.db_valid_mask		= vntb_epf_db_valid_mask,
> +	.db_set_mask		= vntb_epf_db_set_mask,
> +	.mw_set_trans		= vntb_epf_mw_set_trans,
> +	.mw_clear_trans		= vntb_epf_mw_clear_trans,
> +	.peer_mw_get_addr	= vntb_epf_peer_mw_get_addr,
> +	.link_enable		= vntb_epf_link_enable,
> +	.spad_read		= vntb_epf_spad_read,
> +	.spad_write		= vntb_epf_spad_write,
> +	.peer_spad_read		= vntb_epf_peer_spad_read,
> +	.peer_spad_write	= vntb_epf_peer_spad_write,
> +	.peer_db_set		= vntb_epf_peer_db_set,
> +	.db_read		= vntb_epf_db_read,
> +	.mw_get_align		= vntb_epf_mw_get_align,
> +	.link_is_up		= vntb_epf_link_is_up,
> +	.db_clear_mask		= vntb_epf_db_clear_mask,
> +	.db_clear		= vntb_epf_db_clear,
> +	.link_disable		= vntb_epf_link_disable,
> +};
> +
> +static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	int ret;
> +	struct epf_ntb *ndev = (struct epf_ntb *)pdev->sysdata;
> +	struct device *dev = &pdev->dev;
> +
> +	ndev->ntb.pdev = pdev;
> +	ndev->ntb.topo = NTB_TOPO_NONE;
> +	ndev->ntb.ops =  &vntb_epf_ops;
> +
> +	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +	if (ret) {
> +		dev_err(dev, "Cannot set DMA mask\n");
> +		return -1;

return -EINVAL;

> +	}
> +
> +	ret = ntb_register_device(&ndev->ntb);
> +	if (ret) {
> +		dev_err(dev, "Failed to register NTB device\n");
> +		goto err_register_dev;
> +	}
> +
> +	dev_info(dev, "PCI Virtual NTB driver loaded\n");
> +	return 0;
> +
> +err_register_dev:
> +	return -1;

return -EINVAL;

> +}
> +
> +static const struct pci_device_id pci_vntb_table[] = {
> +	{
> +		PCI_DEVICE(VNTB_VID, VNTB_PID),
> +	},
> +	{},
> +};
> +
> +static struct pci_driver vntb_pci_driver = {
> +	.name           = "pci-vntb",
> +	.id_table       = pci_vntb_table,
> +	.probe          = pci_vntb_probe,
> +};
> +
> +/* ============ PCIe EPF Driver Bind ====================*/
> +
> +/**
> + * epf_ntb_bind() - Initialize endpoint controller to provide NTB functionality
> + * @epf: NTB endpoint function device
> + *
> + * Initialize both the endpoint controllers associated with NTB function device.
> + * Invoked when a primary interface or secondary interface is bound to EPC
> + * device. This function will succeed only when EPC is bound to both the
> + * interfaces.
> + */
> +static int epf_ntb_bind(struct pci_epf *epf)
> +{
> +	struct epf_ntb *ntb = epf_get_drvdata(epf);
> +	struct device *dev = &epf->dev;
> +	int ret;
> +
> +	if (!epf->epc) {
> +		dev_dbg(dev, "PRIMARY EPC interface not yet bound\n");
> +		return 0;
> +	}
> +
> +	ret = epf_ntb_init_epc_bar(ntb);
> +	if (ret) {
> +		dev_err(dev, "Failed to create NTB EPC\n");
> +		goto err_bar_init;
> +	}
> +
> +	ret = epf_ntb_config_spad_bar_alloc(ntb);
> +	if (ret) {
> +		dev_err(dev, "Failed to allocate BAR memory\n");
> +		goto err_bar_alloc;
> +	}
> +
> +	ret = epf_ntb_epc_init(ntb);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize EPC\n");
> +		goto err_bar_alloc;
> +	}
> +
> +	epf_set_drvdata(epf, ntb);
> +
> +	if (pci_register_driver(&vntb_pci_driver)) {
> +		dev_err(dev, "failure register vntb pci driver\n");
> +		goto err_bar_alloc;
> +	}
> +
> +	vpci_bus(ntb);
> +
> +	return 0;
> +
> +err_bar_alloc:
> +	epf_ntb_config_spad_bar_free(ntb);
> +
> +err_bar_init:
> +	epf_ntb_epc_destroy(ntb);
> +
> +	return ret;
> +}
> +
> +/**
> + * epf_ntb_unbind() - Cleanup the initialization from epf_ntb_bind()
> + * @epf: NTB endpoint function device
> + *
> + * Cleanup the initialization from epf_ntb_bind()
> + */
> +static void epf_ntb_unbind(struct pci_epf *epf)
> +{
> +	struct epf_ntb *ntb = epf_get_drvdata(epf);
> +
> +	epf_ntb_epc_cleanup(ntb);
> +	epf_ntb_config_spad_bar_free(ntb);
> +	epf_ntb_epc_destroy(ntb);
> +
> +	pci_unregister_driver(&vntb_pci_driver);
> +}
> +
> +// EPF driver probe
> +static struct pci_epf_ops epf_ntb_ops = {
> +	.bind   = epf_ntb_bind,
> +	.unbind = epf_ntb_unbind,
> +	.add_cfs = epf_ntb_add_cfs,
> +};
> +
> +/**
> + * epf_ntb_probe() - Probe NTB function driver
> + * @epf: NTB endpoint function device
> + *
> + * Probe NTB function driver when endpoint function bus detects a NTB
> + * endpoint function.
> + */
> +static int epf_ntb_probe(struct pci_epf *epf)
> +{
> +	struct epf_ntb *ntb;
> +	struct device *dev;
> +
> +	dev = &epf->dev;
> +
> +	ntb = devm_kzalloc(dev, sizeof(*ntb), GFP_KERNEL);
> +	if (!ntb)
> +		return -ENOMEM;
> +
> +	epf->header = &epf_ntb_header;
> +	ntb->epf = epf;
> +	epf_set_drvdata(epf, ntb);
> +
> +	dev_info(dev, "pci-ep epf driver loaded\n");

I think most drivers don't announce when they're loaded because it
really doesn't help the user.

> +	return 0;
> +}
> +
> +static const struct pci_epf_device_id epf_ntb_ids[] = {
> +	{
> +		.name = "pci_epf_vntb",
> +	},
> +	{},
> +};
> +
> +static struct pci_epf_driver epf_ntb_driver = {
> +	.driver.name    = "pci_epf_vntb",
> +	.probe          = epf_ntb_probe,
> +	.id_table       = epf_ntb_ids,
> +	.ops            = &epf_ntb_ops,
> +	.owner          = THIS_MODULE,
> +};
> +
> +

Spurious blank line.

> +static int __init epf_ntb_init(void)
> +{
> +	int ret;
> +
> +	kpcintb_workqueue = alloc_workqueue("kpcintb", WQ_MEM_RECLAIM |
> +					    WQ_HIGHPRI, 0);
> +	ret = pci_epf_register_driver(&epf_ntb_driver);
> +	if (ret) {
> +		destroy_workqueue(kpcintb_workqueue);
> +		pr_err("Failed to register pci epf ntb driver --> %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +module_init(epf_ntb_init);
> +
> +static void __exit epf_ntb_exit(void)
> +{
> +	pci_epf_unregister_driver(&epf_ntb_driver);
> +	destroy_workqueue(kpcintb_workqueue);
> +}
> +module_exit(epf_ntb_exit);
> +
> +MODULE_DESCRIPTION("PCI EPF NTB DRIVER");
> +MODULE_AUTHOR("Frank Li <Frank.li@nxp.com>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.24.0.rc1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 2/4] NTB: epf: Added more flexible memory map method
  2022-02-17 22:24     ` Zhi Li
@ 2022-02-17 22:43       ` Bjorn Helgaas
  0 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2022-02-17 22:43 UTC (permalink / raw)
  To: Zhi Li
  Cc: Frank Li, linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, hongxing.zhu, Jon Mason, Dave Jiang,
	Allen Hubbe, linux-ntb

On Thu, Feb 17, 2022 at 04:24:56PM -0600, Zhi Li wrote:
> On Thu, Feb 17, 2022 at 3:59 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > [+cc Jon, Dave, Allen, linux-ntb, since you need at least an ack from
> > the NTB folks; beginning of thread:
> > https://lore.kernel.org/r/20220215053844.7119-1-Frank.Li@nxp.com]
> >
> > In subject, s/Added/Add/.
> >
> > But I don't think it's quite right yet, because this doesn't actually add
> > any functions.
> 
> How about "Allow more flexibility in the memory bar map method"?

s/bar/BAR/

Are you saying that the BAR numbers were fixed before, and you're
adding the ability to change the BAR numbers?  It would be useful to
know what sort of flexibility you're adding.

> > On Mon, Feb 14, 2022 at 11:38:42PM -0600, Frank Li wrote:
> > > Supported below memory map method
> > >
> > > bar 0: config and spad data
> > > bar 2: door bell
> > > bar 4: memory map windows
> >
> > s/bar/BAR/
> > s/spad/?/ (I don't know what this is)
> 
> SCRATCHPAD REGION
> https://www.kernel.org/doc/html/latest/driver-api/ntb.html

Just spelling out "scrachpad" is probably enough.

And s/door bell/doorbell/ to match your usage elsewhere.

> > Please make the commit log say what this patch does.
> 
> Does it help if attach ASCII diagram in patch 3/4 or cover letter one

The diagram is nice, but doesn't need to be replicated everywhere.  A
description of what the patch does would be better.

Bjorn

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection
  2022-02-17 22:38   ` Bjorn Helgaas
@ 2022-02-17 22:59     ` Zhi Li
  0 siblings, 0 replies; 15+ messages in thread
From: Zhi Li @ 2022-02-17 22:59 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Frank Li, linux-pci, kishon, lorenzo.pieralisi, kw, jingoohan1,
	gustavo.pimentel, hongxing.zhu, Jon Mason, Dave Jiang,
	Allen Hubbe, linux-ntb

On Thu, Feb 17, 2022 at 4:38 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> [+cc Jon, Dave, Allen, linux-ntb]
>
> This patch only touches drivers/pci; the subject line should reflect
> that to help people know which things to look at.
>
> Maybe something like:
>
>   PCI: endpoint: Support NTB transfer between RC and EP
>
> On Mon, Feb 14, 2022 at 11:38:43PM -0600, Frank Li wrote:
> > Add NTB function driver and virtual PCI BUS and Virtual NTB driver
> > to implement communication between PCIe RC and PCIe EP devices
>
> s/PCI BUS/PCI Bus/
> s/PCIe RC/PCIe Root Port/ (I think; the RC itself doesn't have a
> software representation)
>
> > ┌────────────┐         ┌─────────────────────────────────────┐
> > │            │         │                                     │
> > ├────────────┤         │                      ┌──────────────┤
> > │ NTB        │         │                      │ NTB          │
> > │ NetDev     │         │                      │ NetDev       │
> > ├────────────┤         │                      ├──────────────┤
> > │ NTB        │         │                      │ NTB          │
> > │ Transfer   │         │                      │ Transfer     │
> > ├────────────┤         │                      ├──────────────┤
> > │            │         │                      │              │
> > │  PCI NTB   │         │                      │              │
> > │    EPF     │         │                      │              │
> > │   Driver   │         │                      │ PCI Virtual  │
> > │            │         ├───────────────┐      │ NTB Driver   │
> > │            │         │ PCI EP NTB    │◄────►│              │
> > │            │         │  FN Driver    │      │              │
> > ├────────────┤         ├───────────────┤      ├──────────────┤
> > │            │         │               │      │              │
> > │  PCI BUS   │ ◄─────► │  PCI EP BUS   │      │  Virtual PCI │
> > │            │  PCI    │               │      │     BUS      │
> > └────────────┘         └───────────────┴──────┴──────────────┘
>
> s/PCI BUS/PCI Bus/
>
> >     PCI RC                        PCI EP
>
> s/PCI RC/PCIe Root Port/ ?
>
> > This driver include 3 part:
> >  1 PCI EP NTB function driver
> >  2 Virtual PCI bus
> >  3 PCI virutal NTB driver, which is loaded only by above virtual pci bus
>
> s/include 3 part/includes 3 parts/
> s/virutal/virtual/
> s/pci bus/PCI bus/
>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/pci/endpoint/functions/Kconfig        |   11 +
> >  drivers/pci/endpoint/functions/Makefile       |    1 +
> >  drivers/pci/endpoint/functions/pci-epf-vntb.c | 1425 +++++++++++++++++
> >  3 files changed, 1437 insertions(+)
> >  create mode 100644 drivers/pci/endpoint/functions/pci-epf-vntb.c
> >
> > diff --git a/drivers/pci/endpoint/functions/Kconfig b/drivers/pci/endpoint/functions/Kconfig
> > index 5f1242ca2f4e4..362555b024e8f 100644
> > --- a/drivers/pci/endpoint/functions/Kconfig
> > +++ b/drivers/pci/endpoint/functions/Kconfig
> > @@ -25,3 +25,14 @@ config PCI_EPF_NTB
> >         device tree.
> >
> >         If in doubt, say "N" to disable Endpoint NTB driver.
> > +
> > +config PCI_EPF_VNTB
> > +        tristate "PCI Endpoint NTB driver"
> > +        depends on PCI_ENDPOINT
> > +        select CONFIGFS_FS
> > +        help
> > +          Select this configuration option to enable the Non-Transparent
> > +          Bridge (NTB) driver for PCI Endpoint. NTB driver implements NTB
> > +          between PCI host and PCIe Endpoint.
>
> "PCI Endpoint" vs "PCIe Endpoint".  Pick one.  It seems like you
> require an upstream bridge, i.e., a Root Port, so maybe "PCIe Root
> Port" and "PCIe Endpoint" is what you want?
>
> > +          If in doubt, say "N" to disable Endpoint NTB driver.
> > diff --git a/drivers/pci/endpoint/functions/Makefile b/drivers/pci/endpoint/functions/Makefile
> > index 96ab932a537a2..5c13001deaba1 100644
> > --- a/drivers/pci/endpoint/functions/Makefile
> > +++ b/drivers/pci/endpoint/functions/Makefile
> > @@ -5,3 +5,4 @@
> >
> >  obj-$(CONFIG_PCI_EPF_TEST)           += pci-epf-test.o
> >  obj-$(CONFIG_PCI_EPF_NTB)            += pci-epf-ntb.o
> > +obj-$(CONFIG_PCI_EPF_VNTB)           += pci-epf-vntb.o
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > new file mode 100644
> > index 0000000000000..ebf7e243eefa4
> > --- /dev/null
> > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > @@ -0,0 +1,1425 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Endpoint Function Driver to implement Non-Transparent Bridge functionality
> > + * Between PCI RC and EP
> > + *
> > + * Copyright (C) 2020 Texas Instruments
> > + * Copyright (C) 2022 NXP
> > + *
> > + * Based on pci-epf-ntb.c
> > + * Author: Frank Li <Frank.Li@nxp.com>
> > + * Author: Kishon Vijay Abraham I <kishon@ti.com>
> > + */
> > +
> > +/**
> > + * +------------+         +---------------------------------------+
> > + * |            |         |                                       |
> > + * +------------+         |                        +--------------+
> > + * | NTB        |         |                        | NTB          |
> > + * | NetDev     |         |                        | NetDev       |
> > + * +------------+         |                        +--------------+
> > + * | NTB        |         |                        | NTB          |
> > + * | Transfer   |         |                        | Transfer     |
> > + * +------------+         |                        +--------------+
> > + * |            |         |                        |              |
> > + * |  PCI NTB   |         |                        |              |
> > + * |    EPF     |         |                        |              |
> > + * |   Driver   |         |                        | PCI Virtual  |
> > + * |            |         +---------------+        | NTB Driver   |
> > + * |            |         | PCI EP NTB    |<------>|              |
> > + * |            |         |  FN Driver    |        |              |
> > + * +------------+         +---------------+        +--------------+
> > + * |            |         |               |        |              |
> > + * |  PCI BUS   | <-----> |  PCI EP BUS   |        |  Virtual PCI |
> > + * |            |  PCI    |               |        |     BUS      |
> > + * +------------+         +---------------+--------+--------------+
> > + *     PCI RC                        PCI EP
>
> s/PCI RC/PCIe Root Port/ ?
>
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +
> > +#include <linux/pci-epc.h>
> > +#include <linux/pci-epf.h>
> > +#include <linux/ntb.h>
> > +
> > +static struct workqueue_struct *kpcintb_workqueue;
> > +
> > +#define COMMAND_CONFIGURE_DOORBELL   1
> > +#define COMMAND_TEARDOWN_DOORBELL    2
> > +#define COMMAND_CONFIGURE_MW         3
> > +#define COMMAND_TEARDOWN_MW          4
> > +#define COMMAND_LINK_UP                      5
> > +#define COMMAND_LINK_DOWN            6
> > +
> > +#define COMMAND_STATUS_OK            1
> > +#define COMMAND_STATUS_ERROR         2
> > +
> > +#define LINK_STATUS_UP                       BIT(0)
> > +
> > +#define SPAD_COUNT                   64
> > +#define DB_COUNT                     4
> > +#define NTB_MW_OFFSET                        2
> > +#define DB_COUNT_MASK                        GENMASK(15, 0)
> > +#define MSIX_ENABLE                  BIT(16)
> > +#define MAX_DB_COUNT                 32
> > +#define MAX_MW                               4
> > +
> > +#define VNTB_VID                     0x1957
>
> This looks like PCI_VENDOR_ID_NXP (or PCI_VENDOR_ID_FREESCALE).
> How did you choose that, why is it safe to use, and why don't use use
> PCI_VENDOR_ID_NXP instead?

This is a problem.  I am working on NXP.  But I have not found an apartment,
who managed the PCIe PID list yet.

I can put it in the configfs, let the user change it. Or someone can
donate one PID/VID.

>
> > +#define VNTB_PID                     0x080A
> > +
> > +enum epf_ntb_bar {
> > +     BAR_CONFIG,
> > +     BAR_DB,
> > +     BAR_MW0,
> > +     BAR_MW1,
> > +     BAR_MW2,
> > +};
> > +
> > +/*
> > + * +--------------------------------------------------+ Base
> > + * |                                                  |
> > + * |                                                  |
> > + * |                                                  |
> > + * |          Common Control Register                 |
> > + * |                                                  |
> > + * |                                                  |
> > + * |                                                  |
> > + * +-----------------------+--------------------------+ Base+span_offset
> > + * |                       |                          |
> > + * |    Peer Span Space    |    Span Space            |
> > + * |                       |                          |
> > + * |                       |                          |
> > + * +-----------------------+--------------------------+ Base+span_offset
> > + * |                       |                          |     +span_count * 4
> > + * |                       |                          |
> > + * |     Span Space        |   Peer Span Space        |
> > + * |                       |                          |
> > + * +-----------------------+--------------------------+
> > + *       Virtual PCI             Pcie Endpoint
> > + *       NTB Driver               NTB Driver
>
> s/Pcie/PCIe/
>
> > + */
> > +struct epf_ntb_ctrl {
> > +     u32     command;
> > +     u32     argument;
> > +     u16     command_status;
> > +     u16     link_status;
> > +     u32     topology;
> > +     u64     addr;
> > +     u64     size;
> > +     u32     num_mws;
> > +     u32     reserved;
> > +     u32     spad_offset;
> > +     u32     spad_count;
> > +     u32     db_entry_size;
> > +     u32     db_data[MAX_DB_COUNT];
> > +     u32     db_offset[MAX_DB_COUNT];
> > +} __packed;
> > +
> > +struct epf_ntb {
> > +     struct ntb_dev ntb;
> > +     struct pci_epf *epf;
> > +     struct config_group group;
> > +
> > +     u32 num_mws;
> > +     u32 db_count;
> > +     u32 spad_count;
> > +     u64 mws_size[MAX_MW];
> > +     u64 db;
> > +     u32 vbus_number;
> > +
> > +     bool linkup;
> > +     u32 spad_size;
> > +
> > +     enum pci_barno epf_ntb_bar[6];
> > +
> > +     struct epf_ntb_ctrl *reg;
> > +
> > +     phys_addr_t epf_db_phy;
> > +     void __iomem *epf_db;
> > +
> > +     phys_addr_t vpci_mw_phy[MAX_MW];
> > +     void __iomem *vpci_mw_addr[MAX_MW];
> > +
> > +     struct delayed_work cmd_handler;
> > +};
> > +
> > +#define to_epf_ntb(epf_group) container_of((epf_group), struct epf_ntb, group)
> > +#define ntb_ndev(__ntb) container_of(__ntb, struct epf_ntb, ntb)
> > +
> > +static struct pci_epf_header epf_ntb_header = {
> > +     .vendorid       = PCI_ANY_ID,
> > +     .deviceid       = PCI_ANY_ID,
> > +     .baseclass_code = PCI_BASE_CLASS_MEMORY,
> > +     .interrupt_pin  = PCI_INTERRUPT_INTA,
> > +};
> > +
> > +/**
> > + * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host
> > + * @ntb: NTB device that facilitates communication between HOST and VHOST
> > + * @link_up: true or false indicating Link is UP or Down
> > + *
> > + * Once NTB function in HOST invoke ntb_link_enable(),
> > + * this NTB function driver will trigger a link event to vhost.
> > + */
> > +static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
> > +{
> > +     if (link_up)
> > +             ntb->reg->link_status |= LINK_STATUS_UP;
> > +     else
> > +             ntb->reg->link_status &= ~LINK_STATUS_UP;
> > +
> > +     ntb_link_event(&ntb->ntb);
> > +     return 0;
> > +}
> > +
> > +/**
> > + * epf_ntb_configure_mw() - Configure the Outbound Address Space for vhost
> > + *   to access the memory window of host
> > + * @ntb: NTB device that facilitates communication between host and vhost
> > + * @mw: Index of the memory window (either 0, 1, 2 or 3)
> > + *
> > + *                          EP Outbound Window
> > + * +--------+              +-----------+
> > + * |        |              |           |
> > + * |        |              |           |
> > + * |        |              |           |
> > + * |        |              |           |
> > + * |        |              +-----------+
> > + * | Virtual|              | Memory Win|
> > + * | NTB    | -----------> |           |
> > + * | Driver |              |           |
> > + * |        |              +-----------+
> > + * |        |              |           |
> > + * |        |              |           |
> > + * +--------+              +-----------+
> > + *  VHost                   PCI EP
> > + */
> > +static int epf_ntb_configure_mw(struct epf_ntb *ntb, u32 mw)
> > +{
> > +     phys_addr_t phys_addr;
> > +     u8 func_no, vfunc_no;
> > +     u64 addr, size;
> > +     int ret = 0;
> > +
> > +     phys_addr = ntb->vpci_mw_phy[mw];
> > +     addr = ntb->reg->addr;
> > +     size = ntb->reg->size;
> > +
> > +     func_no = ntb->epf->func_no;
> > +     vfunc_no = ntb->epf->vfunc_no;
> > +
> > +     ret = pci_epc_map_addr(ntb->epf->epc, func_no, vfunc_no, phys_addr, addr, size);
> > +     if (ret)
> > +             dev_err(&ntb->epf->epc->dev,
> > +                     "intf: Failed to map memory window %d address\n", mw);
>
> Lots of these messages start with "intf:".  What is that telling us?
> Is it useful?

I will clear it.

>
> > +     return ret;
> > +}
> > +
> > +/**
> > + * epf_ntb_teardown_mw() - Teardown the configured OB ATU
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST
> > + * @mw: Index of the memory window (either 0, 1, 2 or 3)
> > + *
> > + * Teardown the configured OB ATU configured in epf_ntb_configure_mw() using
> > + * pci_epc_unmap_addr()
> > + */
> > +static void epf_ntb_teardown_mw(struct epf_ntb *ntb, u32 mw)
> > +{
> > +     pci_epc_unmap_addr(ntb->epf->epc,
> > +                        ntb->epf->func_no,
> > +                        ntb->epf->vfunc_no,
> > +                        ntb->vpci_mw_phy[mw]);
> > +}
> > +
> > +/**
> > + * epf_ntb_cmd_handler() - Handle commands provided by the NTB Host
> > + * @work: work_struct for the epf_ntb_epc
> > + *
> > + * Workqueue function that gets invoked for the two epf_ntb_epc
> > + * periodically (once every 5ms) to see if it has received any commands
> > + * from NTB host. The host can send commands to configure doorbell or
> > + * configure memory window or to update link status.
> > + */
> > +static void epf_ntb_cmd_handler(struct work_struct *work)
> > +{
> > +     struct epf_ntb_ctrl *ctrl;
> > +     u32 command, argument;
> > +     struct epf_ntb *ntb;
> > +     struct device *dev;
> > +     int ret;
> > +     int i;
> > +
> > +     ntb = container_of(work, struct epf_ntb, cmd_handler.work);
> > +
> > +     for (i = 1; i < ntb->db_count; i++) {
> > +             if (readl(ntb->epf_db + i * 4)) {
> > +                     if (readl(ntb->epf_db + i * 4))
> > +                             ntb->db |= 1 << (i - 1);
> > +
> > +                     ntb_db_event(&ntb->ntb, i);
> > +                     writel(0, ntb->epf_db + i * 4);
> > +             }
> > +     }
> > +
> > +     ctrl = ntb->reg;
> > +     command = ctrl->command;
> > +     if (!command)
> > +             goto reset_handler;
> > +     argument = ctrl->argument;
> > +
> > +     ctrl->command = 0;
> > +     ctrl->argument = 0;
> > +
> > +     ctrl = ntb->reg;
> > +     dev = &ntb->epf->dev;
> > +
> > +     switch (command) {
> > +     case COMMAND_CONFIGURE_DOORBELL:
> > +             ctrl->command_status = COMMAND_STATUS_OK;
> > +             break;
> > +     case COMMAND_TEARDOWN_DOORBELL:
> > +             ctrl->command_status = COMMAND_STATUS_OK;
> > +             break;
> > +     case COMMAND_CONFIGURE_MW:
> > +             ret = epf_ntb_configure_mw(ntb, argument);
> > +             if (ret < 0)
> > +                     ctrl->command_status = COMMAND_STATUS_ERROR;
> > +             else
> > +                     ctrl->command_status = COMMAND_STATUS_OK;
> > +             break;
> > +     case COMMAND_TEARDOWN_MW:
> > +             epf_ntb_teardown_mw(ntb, argument);
> > +             ctrl->command_status = COMMAND_STATUS_OK;
> > +             break;
> > +     case COMMAND_LINK_UP:
> > +             ntb->linkup = true;
> > +             ret = epf_ntb_link_up(ntb, true);
> > +             if (ret < 0)
> > +                     ctrl->command_status = COMMAND_STATUS_ERROR;
> > +             else
> > +                     ctrl->command_status = COMMAND_STATUS_OK;
> > +             goto reset_handler;
> > +     case COMMAND_LINK_DOWN:
> > +             ntb->linkup = false;
> > +             ret = epf_ntb_link_up(ntb, false);
> > +             if (ret < 0)
> > +                     ctrl->command_status = COMMAND_STATUS_ERROR;
> > +             else
> > +                     ctrl->command_status = COMMAND_STATUS_OK;
> > +             break;
> > +     default:
> > +             dev_err(dev, "intf UNKNOWN command: %d\n", command);
> > +             break;
> > +     }
> > +
> > +reset_handler:
> > +     queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
> > +                        msecs_to_jiffies(5));
> > +}
> > +
> > +/**
> > + * epf_ntb_config_sspad_bar_clear() - Clear Config + Self scratchpad BAR
> > + * @ntb_epc: EPC associated with one of the HOST which holds peer's outbound
> > + *        address.
> > + *
> > + * Clear BAR0 of EP CONTROLLER 1 which contains the HOST1's config and
> > + * self scratchpad region (removes inbound ATU configuration). While BAR0 is
> > + * the default self scratchpad BAR, an NTB could have other BARs for self
> > + * scratchpad (because of reserved BARs). This function can get the exact BAR
> > + * used for self scratchpad from epf_ntb_bar[BAR_CONFIG].
> > + *
> > + * Please note the self scratchpad region and config region is combined to
> > + * a single region and mapped using the same BAR. Also note HOST2's peer
> > + * scratchpad is HOST1's self scratchpad.
> > + */
> > +static void epf_ntb_config_sspad_bar_clear(struct epf_ntb *ntb)
> > +{
> > +     struct pci_epf_bar *epf_bar;
> > +     enum pci_barno barno;
> > +
> > +     barno = ntb->epf_ntb_bar[BAR_CONFIG];
> > +     epf_bar = &ntb->epf->bar[barno];
> > +
> > +     pci_epc_clear_bar(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no, epf_bar);
> > +}
> > +
> > +/**
> > + * epf_ntb_config_sspad_bar_set() - Set Config + Self scratchpad BAR
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST
> > + *
> > + * Map BAR0 of EP CONTROLLER 1 which contains the HOST1's config and
> > + * self scratchpad region.
> > + *
> > + * Please note the self scratchpad region and config region is combined to
> > + * a single region and mapped using the same BAR.
> > + */
> > +static int epf_ntb_config_sspad_bar_set(struct epf_ntb *ntb)
> > +{
> > +     struct pci_epf_bar *epf_bar;
> > +     enum pci_barno barno;
> > +     u8 func_no, vfunc_no;
> > +     struct device *dev;
> > +     int ret;
> > +
> > +     dev = &ntb->epf->dev;
> > +     func_no = ntb->epf->func_no;
> > +     vfunc_no = ntb->epf->vfunc_no;
> > +     barno = ntb->epf_ntb_bar[BAR_CONFIG];
> > +     epf_bar = &ntb->epf->bar[barno];
> > +
> > +     ret = pci_epc_set_bar(ntb->epf->epc, func_no, vfunc_no, epf_bar);
> > +     if (ret) {
> > +             dev_err(dev, "inft: Config/Status/SPAD BAR set failed\n");
> > +             return ret;
> > +     }
> > +     return 0;
> > +}
> > +
> > +/**
> > + * epf_ntb_config_spad_bar_free() - Free the physical memory associated with
> > + *   config + scratchpad region
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST
> > + */
> > +static void epf_ntb_config_spad_bar_free(struct epf_ntb *ntb)
> > +{
> > +     enum pci_barno barno;
> > +
> > +     barno = ntb->epf_ntb_bar[BAR_CONFIG];
> > +     pci_epf_free_space(ntb->epf, ntb->reg, barno, 0);
> > +}
> > +
> > +/**
> > + * epf_ntb_config_spad_bar_alloc() - Allocate memory for config + scratchpad
> > + *   region
> > + * @ntb: NTB device that facilitates communication between HOST1 and HOST2
> > + *
> > + * Allocate the Local Memory mentioned in the above diagram. The size of
> > + * CONFIG REGION is sizeof(struct epf_ntb_ctrl) and size of SCRATCHPAD REGION
> > + * is obtained from "spad-count" configfs entry.
> > + */
> > +static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
> > +{
> > +     size_t align;
> > +     enum pci_barno barno;
> > +     struct epf_ntb_ctrl *ctrl;
> > +     u32 spad_size, ctrl_size;
> > +     u64 size;
> > +     struct pci_epf *epf = ntb->epf;
> > +     struct device *dev = &epf->dev;
> > +     u32 spad_count;
> > +     void *base;
> > +     int i;
> > +     const struct pci_epc_features *epc_features = pci_epc_get_features(epf->epc,
> > +                                                             epf->func_no,
> > +                                                             epf->vfunc_no);
> > +     barno = ntb->epf_ntb_bar[BAR_CONFIG];
> > +     size = epc_features->bar_fixed_size[barno];
> > +     align = epc_features->align;
> > +
> > +     if ((!IS_ALIGNED(size, align)))
> > +             return -EINVAL;
> > +
> > +     spad_count = ntb->spad_count;
> > +
> > +     ctrl_size = sizeof(struct epf_ntb_ctrl);
> > +     spad_size = 2 * spad_count * 4;
> > +
> > +     if (!align) {
> > +             ctrl_size = roundup_pow_of_two(ctrl_size);
> > +             spad_size = roundup_pow_of_two(spad_size);
> > +     } else {
> > +             ctrl_size = ALIGN(ctrl_size, align);
> > +             spad_size = ALIGN(spad_size, align);
> > +     }
> > +
> > +     if (!size)
> > +             size = ctrl_size + spad_size;
> > +     else if (size < ctrl_size + spad_size)
> > +             return -EINVAL;
> > +
> > +     base = pci_epf_alloc_space(epf, size, barno, align, 0);
> > +     if (!base) {
> > +             dev_err(dev, "intf: Config/Status/SPAD alloc region fail\n");
> > +             return -ENOMEM;
> > +     }
> > +
> > +     ntb->reg = base;
> > +
> > +     ctrl = ntb->reg;
> > +     ctrl->spad_offset = ctrl_size;
> > +
> > +     ctrl->spad_count = spad_count;
> > +     ctrl->num_mws = ntb->num_mws;
> > +     ntb->spad_size = spad_size;
> > +
> > +     ctrl->db_entry_size = 4;
> > +
> > +     for (i = 0; i < ntb->db_count; i++) {
> > +             ntb->reg->db_data[i] = 1 + i;
> > +             ntb->reg->db_offset[i] = 0;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +/**
> > + * epf_ntb_configure_interrupt() - Configure MSI/MSI-X capaiblity
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST
> > + *
> > + * Configure MSI/MSI-X capability for each interface with number of
> > + * interrupts equal to "db_count" configfs entry.
> > + */
> > +static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
> > +{
> > +     const struct pci_epc_features *epc_features;
> > +     bool msix_capable, msi_capable;
> > +     u8 func_no, vfunc_no;
> > +     struct device *dev;
> > +     u32 db_count;
> > +     int ret;
> > +
> > +     dev = &ntb->epf->dev;
> > +
> > +     epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);
> > +     msix_capable = epc_features->msix_capable;
> > +     msi_capable = epc_features->msi_capable;
> > +
> > +     if (!(msix_capable || msi_capable)) {
>
> I don't think the "msix_capable" and "msi_capable" local variables
> really add any readability since they're only used once.
>
> Not sure about "func_no", "vfunc_no", either.  If you keep them, you
> could at least assign them earlier and use them in the
> pci_epc_get_features() call.
>
> > +             dev_err(dev, "MSI or MSI-X is required for doorbell\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     func_no = ntb->epf->func_no;
> > +     vfunc_no = ntb->epf->vfunc_no;
> > +
> > +     db_count = ntb->db_count;
> > +     if (db_count > MAX_DB_COUNT) {
> > +             dev_err(dev, "DB count cannot be more than %d\n", MAX_DB_COUNT);
> > +             return -EINVAL;
> > +     }
> > +
> > +     ntb->db_count = db_count;
> > +
> > +     if (msi_capable) {
> > +             ret = pci_epc_set_msi(ntb->epf->epc, func_no, vfunc_no, 16);
> > +             if (ret) {
> > +                     dev_err(dev, "intf: MSI configuration failed\n");
> > +                     return ret;
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +/**
> > + * epf_ntb_db_bar_init() - Configure Doorbell window BARs
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST
> > + *
>
> Spurious blank line.
>
> > + */
> > +static int epf_ntb_db_bar_init(struct epf_ntb *ntb)
> > +{
> > +     const struct pci_epc_features *epc_features;
> > +     u32 align;
> > +     struct device *dev = &ntb->epf->dev;
> > +     int ret;
> > +     struct pci_epf_bar *epf_bar;
> > +     void __iomem *mw_addr;
> > +     enum pci_barno barno;
> > +     size_t size = 4 * ntb->db_count;
> > +
> > +     epc_features = pci_epc_get_features(ntb->epf->epc,
> > +                                         ntb->epf->func_no,
> > +                                         ntb->epf->vfunc_no);
> > +     align = epc_features->align;
> > +
> > +     if (size < 128)
> > +             size = 128;
> > +
> > +     if (align)
> > +             size = ALIGN(size, align);
> > +     else
> > +             size = roundup_pow_of_two(size);
> > +
> > +     barno = ntb->epf_ntb_bar[BAR_DB];
> > +
> > +     mw_addr = pci_epf_alloc_space(ntb->epf, size, barno, align, 0);
> > +     if (!mw_addr) {
> > +             dev_err(dev, "intf: Failed to allocate OB address\n");
> > +             return -ENOMEM;
> > +     }
> > +
> > +     ntb->epf_db = mw_addr;
> > +
> > +     epf_bar = &ntb->epf->bar[barno];
> > +
> > +     ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no, epf_bar);
> > +     if (ret) {
> > +             dev_err(dev, "intf: DoorBell BAR set failed\n");
>
> s/DoorBell/Doorbell/
>
> > +                     goto err_alloc_peer_mem;
> > +     }
> > +     return ret;
> > +
> > +err_alloc_peer_mem:
> > +     pci_epc_mem_free_addr(ntb->epf->epc, epf_bar->phys_addr, mw_addr, epf_bar->size);
> > +     return -1;
> > +}
> > +
> > +/**
> > + * epf_ntb_db_bar_clear() - Clear doorbell BAR and free memory
> > + *   allocated in peers outbound address space
>
> s/peers/peer's/
>
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST
> > + */
> > +static void epf_ntb_db_bar_clear(struct epf_ntb *ntb)
> > +{
> > +     enum pci_barno barno;
> > +
> > +     barno = ntb->epf_ntb_bar[BAR_DB];
> > +     pci_epf_free_space(ntb->epf, ntb->epf_db, barno, 0);
> > +     pci_epc_clear_bar(ntb->epf->epc,
> > +                       ntb->epf->func_no,
> > +                       ntb->epf->vfunc_no,
> > +                       &ntb->epf->bar[barno]);
> > +}
> > +
> > +/**
> > + * epf_ntb_mw_bar_init() - Configure Memory window BARs
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST
> > + *
> > + */
> > +static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
> > +{
> > +     int ret = 0;
> > +     int i;
> > +     u64 size;
> > +     enum pci_barno barno;
> > +     struct device *dev = &ntb->epf->dev;
> > +
> > +     for (i = 0; i < ntb->num_mws; i++) {
> > +
>
> Spurious blank line.
>
> > +             size = ntb->mws_size[i];
> > +
> > +             barno = ntb->epf_ntb_bar[BAR_MW0 + i];
> > +
> > +             ntb->epf->bar[barno].barno = barno;
> > +             ntb->epf->bar[barno].size = size;
> > +             ntb->epf->bar[barno].addr = 0;
> > +             ntb->epf->bar[barno].phys_addr = 0;
> > +             ntb->epf->bar[barno].flags |= upper_32_bits(size) ?
> > +                             PCI_BASE_ADDRESS_MEM_TYPE_64 :
> > +                             PCI_BASE_ADDRESS_MEM_TYPE_32;
> > +
> > +             ret = pci_epc_set_bar(ntb->epf->epc,
> > +                                   ntb->epf->func_no,
> > +                                   ntb->epf->vfunc_no,
> > +                                   &ntb->epf->bar[barno]);
> > +             if (ret) {
> > +                     dev_err(dev, "intf: MW set failed\n");
> > +                     goto err_alloc_mem;
> > +             }
> > +
> > +             /* allocate epc outbound memory windows to vpci vntb device */
>
> s/allocate/Allocate/ to match other comments
> s/epc/EPC/
>
> > +             ntb->vpci_mw_addr[i] = pci_epc_mem_alloc_addr(ntb->epf->epc,
> > +                                                           &ntb->vpci_mw_phy[i],
> > +                                                           size);
> > +             if (!ntb->vpci_mw_addr[i]) {
> > +                     dev_err(dev, "Failed to allocate source address\n");
> > +                     goto err_alloc_mem;
> > +             }
> > +     }
> > +
> > +     return ret;
> > +err_alloc_mem:
> > +     return ret;
> > +}
> > +
> > +/**
> > + * epf_ntb_mw_bar_clear() - Clear Memory window BARs
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST
> > + *
>
> Spurious blank line.
>
> > + */
> > +static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb)
> > +{
> > +     enum pci_barno barno;
> > +     int i;
> > +
> > +     for (i = 0; i < ntb->num_mws; i++) {
> > +             barno = ntb->epf_ntb_bar[BAR_MW0 + i];
> > +             pci_epc_clear_bar(ntb->epf->epc,
> > +                               ntb->epf->func_no,
> > +                               ntb->epf->vfunc_no,
> > +                               &ntb->epf->bar[barno]);
> > +
> > +             pci_epc_mem_free_addr(ntb->epf->epc,
> > +                                   ntb->vpci_mw_phy[i],
> > +                                   ntb->vpci_mw_addr[i],
> > +                                   ntb->mws_size[i]);
> > +     }
> > +}
> > +
> > +/**
> > + * epf_ntb_epc_destroy() - Cleanup NTB EPC interface
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST
> > + *
> > + * Wrapper for epf_ntb_epc_destroy_interface() to cleanup all the NTB interfaces
> > + */
> > +static void epf_ntb_epc_destroy(struct epf_ntb *ntb)
> > +{
> > +     pci_epc_remove_epf(ntb->epf->epc, ntb->epf, 0);
> > +     pci_epc_put(ntb->epf->epc);
> > +}
> > +
> > +/**
> > + * epf_ntb_init_epc_bar() - Identify BARs to be used for each of the NTB
> > + * constructs (scratchpad region, doorbell, memorywindow)
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST
> > + *
>
> Spurious blank line.
>
> > + */
> > +static int epf_ntb_init_epc_bar(struct epf_ntb *ntb)
> > +{
> > +     const struct pci_epc_features *epc_features;
> > +     enum pci_barno barno;
> > +     enum epf_ntb_bar bar;
> > +     struct device *dev;
> > +     u32 num_mws;
> > +     int i;
> > +
> > +     barno = BAR_0;
> > +     num_mws = ntb->num_mws;
> > +     dev = &ntb->epf->dev;
> > +     epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);
> > +
> > +     /* These are required BARs which are mandatory for NTB functionality */
> > +     for (bar = BAR_CONFIG; bar <= BAR_MW0; bar++, barno++) {
> > +             barno = pci_epc_get_next_free_bar(epc_features, barno);
> > +             if (barno < 0) {
> > +                     dev_err(dev, "intf: Fail to get NTB function BAR\n");
> > +                     return barno;
> > +             }
> > +             ntb->epf_ntb_bar[bar] = barno;
> > +     }
> > +
> > +     /* These are optional BARs which don't impact NTB functionality */
> > +     for (bar = BAR_MW1, i = 1; i < num_mws; bar++, barno++, i++) {
> > +             barno = pci_epc_get_next_free_bar(epc_features, barno);
> > +             if (barno < 0) {
> > +                     ntb->num_mws = i;
> > +                     dev_dbg(dev, "BAR not available for > MW%d\n", i + 1);
> > +             }
> > +             ntb->epf_ntb_bar[bar] = barno;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +/**
> > + * epf_ntb_epc_init() - Initialize NTB interface
> > + * @ntb: NTB device that facilitates communication between HOST and vHOST2
> > + *
> > + * Wrapper to initialize a particular EPC interface and start the workqueue
> > + * to check for commands from host. This function will write to the
> > + * EP controller HW for configuring it.
> > + */
> > +static int epf_ntb_epc_init(struct epf_ntb *ntb)
> > +{
> > +     u8 func_no, vfunc_no;
> > +     struct pci_epc *epc;
> > +     struct pci_epf *epf;
> > +     struct device *dev;
> > +     int ret;
> > +
> > +     epf = ntb->epf;
> > +     dev = &epf->dev;
> > +     epc = epf->epc;
> > +     func_no = ntb->epf->func_no;
> > +     vfunc_no = ntb->epf->vfunc_no;
> > +
> > +     ret = epf_ntb_config_sspad_bar_set(ntb);
> > +     if (ret) {
> > +             dev_err(dev, "intf: Config/self SPAD BAR init failed");
> > +             return ret;
> > +     }
> > +
> > +     ret = epf_ntb_configure_interrupt(ntb);
> > +     if (ret) {
> > +             dev_err(dev, "intf: Interrupt configuration failed\n");
> > +             goto err_config_interrupt;
> > +     }
> > +
> > +     ret = epf_ntb_db_bar_init(ntb);
> > +     if (ret) {
> > +             dev_err(dev, "intf: DB BAR init failed\n");
> > +             goto err_db_bar_init;
> > +     }
> > +
> > +     ret = epf_ntb_mw_bar_init(ntb);
> > +     if (ret) {
> > +             dev_err(dev, "intf: MW BAR init failed\n");
> > +             goto err_mw_bar_init;
> > +     }
> > +
> > +     if (vfunc_no <= 1) {
> > +             ret = pci_epc_write_header(epc, func_no, vfunc_no, epf->header);
> > +             if (ret) {
> > +                     dev_err(dev, "intf: Configuration header write failed\n");
> > +                     goto err_write_header;
> > +             }
> > +     }
> > +
> > +     INIT_DELAYED_WORK(&ntb->cmd_handler, epf_ntb_cmd_handler);
> > +     queue_work(kpcintb_workqueue, &ntb->cmd_handler.work);
> > +
> > +     return 0;
> > +
> > +err_write_header:
> > +     epf_ntb_mw_bar_clear(ntb);
> > +err_mw_bar_init:
> > +     epf_ntb_db_bar_clear(ntb);
> > +err_db_bar_init:
> > +err_config_interrupt:
> > +     epf_ntb_config_sspad_bar_clear(ntb);
> > +
> > +     return ret;
> > +}
> > +
> > +
> > +/**
> > + * epf_ntb_epc_cleanup() - Cleanup all NTB interfaces
> > + * @ntb: NTB device that facilitates communication between HOST1 and HOST2
> > + *
> > + * Wrapper to cleanup all NTB interfaces.
> > + */
> > +static void epf_ntb_epc_cleanup(struct epf_ntb *ntb)
> > +{
> > +     epf_ntb_db_bar_clear(ntb);
> > +     epf_ntb_mw_bar_clear(ntb);
> > +}
> > +
> > +#define EPF_NTB_R(_name)                                             \
> > +static ssize_t epf_ntb_##_name##_show(struct config_item *item,              \
> > +                                   char *page)                       \
> > +{                                                                    \
> > +     struct config_group *group = to_config_group(item);             \
> > +     struct epf_ntb *ntb = to_epf_ntb(group);                        \
> > +                                                                     \
> > +     return sprintf(page, "%d\n", ntb->_name);                       \
> > +}
> > +
> > +#define EPF_NTB_W(_name)                                             \
> > +static ssize_t epf_ntb_##_name##_store(struct config_item *item,     \
> > +                                    const char *page, size_t len)    \
> > +{                                                                    \
> > +     struct config_group *group = to_config_group(item);             \
> > +     struct epf_ntb *ntb = to_epf_ntb(group);                        \
> > +     u32 val;                                                        \
> > +     int ret;                                                        \
> > +                                                                     \
> > +     ret = kstrtou32(page, 0, &val);                                 \
> > +     if (ret)                                                        \
> > +             return ret;                                             \
> > +                                                                     \
> > +     ntb->_name = val;                                               \
> > +                                                                     \
> > +     return len;                                                     \
> > +}
> > +
> > +#define EPF_NTB_MW_R(_name)                                          \
> > +static ssize_t epf_ntb_##_name##_show(struct config_item *item,              \
> > +                                   char *page)                       \
> > +{                                                                    \
> > +     struct config_group *group = to_config_group(item);             \
> > +     struct epf_ntb *ntb = to_epf_ntb(group);                        \
> > +     int win_no;                                                     \
> > +                                                                     \
> > +     sscanf(#_name, "mw%d", &win_no);                                \
> > +                                                                     \
> > +     return sprintf(page, "%lld\n", ntb->mws_size[win_no - 1]);      \
> > +}
> > +
> > +#define EPF_NTB_MW_W(_name)                                          \
> > +static ssize_t epf_ntb_##_name##_store(struct config_item *item,     \
> > +                                    const char *page, size_t len)    \
> > +{                                                                    \
> > +     struct config_group *group = to_config_group(item);             \
> > +     struct epf_ntb *ntb = to_epf_ntb(group);                        \
> > +     struct device *dev = &ntb->epf->dev;                            \
> > +     int win_no;                                                     \
> > +     u64 val;                                                        \
> > +     int ret;                                                        \
> > +                                                                     \
> > +     ret = kstrtou64(page, 0, &val);                                 \
> > +     if (ret)                                                        \
> > +             return ret;                                             \
> > +                                                                     \
> > +     if (sscanf(#_name, "mw%d", &win_no) != 1)                       \
> > +             return -EINVAL;                                         \
> > +                                                                     \
> > +     if (ntb->num_mws < win_no) {                                    \
> > +             dev_err(dev, "Invalid num_nws: %d value\n", ntb->num_mws); \
> > +             return -EINVAL;                                         \
> > +     }                                                               \
> > +                                                                     \
> > +     ntb->mws_size[win_no - 1] = val;                                \
> > +                                                                     \
> > +     return len;                                                     \
> > +}
> > +
> > +static ssize_t epf_ntb_num_mws_store(struct config_item *item,
> > +                                  const char *page, size_t len)
> > +{
> > +     struct config_group *group = to_config_group(item);
> > +     struct epf_ntb *ntb = to_epf_ntb(group);
> > +     u32 val;
> > +     int ret;
> > +
> > +     ret = kstrtou32(page, 0, &val);
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (val > MAX_MW)
> > +             return -EINVAL;
> > +
> > +     ntb->num_mws = val;
> > +
> > +     return len;
> > +}
> > +
> > +EPF_NTB_R(spad_count)
> > +EPF_NTB_W(spad_count)
> > +EPF_NTB_R(db_count)
> > +EPF_NTB_W(db_count)
> > +EPF_NTB_R(num_mws)
> > +EPF_NTB_R(vbus_number)
> > +EPF_NTB_W(vbus_number)
> > +EPF_NTB_MW_R(mw1)
> > +EPF_NTB_MW_W(mw1)
> > +EPF_NTB_MW_R(mw2)
> > +EPF_NTB_MW_W(mw2)
> > +EPF_NTB_MW_R(mw3)
> > +EPF_NTB_MW_W(mw3)
> > +EPF_NTB_MW_R(mw4)
> > +EPF_NTB_MW_W(mw4)
> > +
> > +CONFIGFS_ATTR(epf_ntb_, spad_count);
> > +CONFIGFS_ATTR(epf_ntb_, db_count);
> > +CONFIGFS_ATTR(epf_ntb_, num_mws);
> > +CONFIGFS_ATTR(epf_ntb_, mw1);
> > +CONFIGFS_ATTR(epf_ntb_, mw2);
> > +CONFIGFS_ATTR(epf_ntb_, mw3);
> > +CONFIGFS_ATTR(epf_ntb_, mw4);
> > +CONFIGFS_ATTR(epf_ntb_, vbus_number);
> > +
> > +static struct configfs_attribute *epf_ntb_attrs[] = {
> > +     &epf_ntb_attr_spad_count,
> > +     &epf_ntb_attr_db_count,
> > +     &epf_ntb_attr_num_mws,
> > +     &epf_ntb_attr_mw1,
> > +     &epf_ntb_attr_mw2,
> > +     &epf_ntb_attr_mw3,
> > +     &epf_ntb_attr_mw4,
> > +     &epf_ntb_attr_vbus_number,
> > +     NULL,
> > +};
> > +
> > +static const struct config_item_type ntb_group_type = {
> > +     .ct_attrs       = epf_ntb_attrs,
> > +     .ct_owner       = THIS_MODULE,
> > +};
> > +
> > +/**
> > + * epf_ntb_add_cfs() - Add configfs directory specific to NTB
> > + * @epf: NTB endpoint function device
> > + * @group: A pointer to the config_group structure referencing a group of
> > + *      config_items of a specific type that belong to a specific sub-system.
> > + *
> > + * Add configfs directory specific to NTB. This directory will hold
> > + * NTB specific properties like db_count, spad_count, num_mws etc.,
> > + */
> > +static struct config_group *epf_ntb_add_cfs(struct pci_epf *epf,
> > +                                         struct config_group *group)
> > +{
> > +     struct epf_ntb *ntb = epf_get_drvdata(epf);
> > +     struct config_group *ntb_group = &ntb->group;
> > +     struct device *dev = &epf->dev;
> > +
> > +     config_group_init_type_name(ntb_group, dev_name(dev), &ntb_group_type);
> > +
> > +     return ntb_group;
> > +}
> > +
> > +/*==== virtual PCI bus driver, which only load virutal ntb pci driver ====*/
>
> s/virutal ntb pci/virtual NTB PCI/
>
> > +#define VPCI_BUS_NUM 0x10
>
> How did you pick this number and how do we know it is available?

This is problem. Do you know auto detect bus number method?
or can I put it to configfs, let user to config it.

>
> > +uint32_t pci_space[] = {
>
> "static u32"?
>
> > +     (VNTB_VID | (VNTB_PID << 16)),  //DeviceID, Vendor ID
> > +     0,              // status, Command
> > +     0xffffffff,     // Class code, subclass, prog if, revision id
> > +     0x40,           //bist, header type, latency Timer, cache line size
> > +     0,              //bar 0
> > +     0,              //bar 1
> > +     0,              //bar 2
> > +     0,              //bar 3
> > +     0,              //bar 4
> > +     0,              //bar 5
>
> s/bar/BAR/
>
> > +     0,              //cardbus cis point
> > +     0,              //Subsystem ID Subystem vendor id
> > +     0,              //ROM Base Address
> > +     0,              //Reserved, Cap. Point
> > +     0,              //Reserved,
> > +     0,              //Max Lat, Min Gnt, interrupt pin, interrupt line
>
> Use /* */ instead of //, like the rest of drivers/pci/
>
> > +};
> > +
> > +int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val)
> > +{
> > +     if (devfn == 0) {
> > +             memcpy(val, ((uint8_t *)pci_space) + where, size);
>
> "u8 *"?
>
> > +             return 0;
> > +     }
> > +     return -1;
>
> These should return PCIBIOS_SUCCESSFUL or PCIBIOS_DEVICE_NOT_FOUND.
>
> > +}
> > +
> > +int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val)
> > +{
> > +     return 0;
> > +}
> > +
> > +struct pci_ops vpci_ops = {
> > +     .read = pci_read,
> > +     .write = pci_write,
> > +};
> > +
> > +static int vpci_bus(void *sysdata)
>
> The function name should say something about what it does.  Maybe
> vpci_scan_bus() or something?
>
> > +{
> > +     struct pci_bus *vpci_bus;
> > +
> > +     vpci_bus = pci_scan_bus(VPCI_BUS_NUM, &vpci_ops, sysdata);
> > +     if (vpci_bus)
> > +             pr_err("create pci bus\n");
> > +
> > +     pci_bus_add_devices(vpci_bus);
> > +
> > +     return 0;
> > +}
> > +
> > +/*==================== Virtual PCIe NTB driver ==========================*/
> > +
> > +static int vntb_epf_mw_count(struct ntb_dev *ntb, int pidx)
> > +{
> > +     struct epf_ntb *ndev = ntb_ndev(ntb);
> > +
> > +     return ndev->num_mws;
> > +}
> > +
> > +static int vntb_epf_spad_count(struct ntb_dev *ntb)
> > +{
> > +     return ntb_ndev(ntb)->spad_count;
> > +}
> > +
> > +static int vntb_epf_peer_mw_count(struct ntb_dev *ntb)
> > +{
> > +     return ntb_ndev(ntb)->num_mws;
> > +}
> > +
> > +static u64 vntb_epf_db_valid_mask(struct ntb_dev *ntb)
> > +{
> > +     return BIT_ULL(ntb_ndev(ntb)->db_count) - 1;
> > +}
> > +
> > +static int vntb_epf_db_set_mask(struct ntb_dev *ntb, u64 db_bits)
> > +{
> > +     return 0;
> > +}
> > +
> > +static int vntb_epf_mw_set_trans(struct ntb_dev *ndev, int pidx, int idx,
> > +             dma_addr_t addr, resource_size_t size)
> > +{
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +     struct pci_epf_bar *epf_bar;
> > +     enum pci_barno barno;
> > +     int ret;
> > +     struct device *dev;
> > +
> > +     dev = &ntb->ntb.dev;
> > +     barno = ntb->epf_ntb_bar[BAR_MW0 + idx];
> > +     epf_bar = &ntb->epf->bar[barno];
> > +     epf_bar->phys_addr = addr;
> > +     epf_bar->barno = barno;
> > +     epf_bar->size = size;
> > +
> > +     ret = pci_epc_set_bar(ntb->epf->epc, 0, 0, epf_bar);
> > +     if (ret) {
> > +             dev_err(dev, "failure set mw trans\n");
> > +             return ret;
> > +     }
> > +     return 0;
> > +}
> > +
> > +static int vntb_epf_mw_clear_trans(struct ntb_dev *ntb, int pidx, int idx)
> > +{
> > +     return 0;
> > +}
> > +
> > +static int vntb_epf_peer_mw_get_addr(struct ntb_dev *ndev, int idx,
> > +                             phys_addr_t *base, resource_size_t *size)
> > +{
> > +
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +
> > +     if (base)
> > +             *base = ntb->vpci_mw_phy[idx];
> > +
> > +     if (size)
> > +             *size = ntb->mws_size[idx];
> > +
> > +     return 0;
> > +}
> > +
> > +static int vntb_epf_link_enable(struct ntb_dev *ntb,
> > +                     enum ntb_speed max_speed,
> > +                     enum ntb_width max_width)
> > +{
> > +     return 0;
> > +}
> > +
> > +static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx)
> > +{
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +     int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * 4;
> > +     u32 val;
> > +     void __iomem *base = ntb->reg;
> > +
> > +     val = readl(base + off + ct + idx * 4);
> > +     return val;
> > +}
> > +
> > +static int vntb_epf_spad_write(struct ntb_dev *ndev, int idx, u32 val)
> > +{
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +     struct epf_ntb_ctrl *ctrl = ntb->reg;
> > +     int off = ctrl->spad_offset, ct = ctrl->spad_count * 4;
> > +     void __iomem *base = ntb->reg;
> > +
> > +     writel(val, base + off + ct + idx * 4);
> > +     return 0;
> > +}
> > +
> > +static u32 vntb_epf_peer_spad_read(struct ntb_dev *ndev, int pidx, int idx)
> > +{
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +     struct epf_ntb_ctrl *ctrl = ntb->reg;
> > +     int off = ctrl->spad_offset;
> > +     void __iomem *base = ntb->reg;
> > +     u32 val;
> > +
> > +     val = readl(base + off + idx * 4);
> > +     return val;
> > +}
> > +
> > +static int vntb_epf_peer_spad_write(struct ntb_dev *ndev, int pidx, int idx, u32 val)
> > +{
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +     struct epf_ntb_ctrl *ctrl = ntb->reg;
> > +     int off = ctrl->spad_offset;
> > +     void __iomem *base = ntb->reg;
> > +
> > +     writel(val, base + off + idx * 4);
> > +     return 0;
> > +}
> > +
> > +static int vntb_epf_peer_db_set(struct ntb_dev *ndev, u64 db_bits)
> > +{
> > +     u32 interrupt_num = ffs(db_bits) + 1;
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +     u8 func_no, vfunc_no;
> > +     int ret;
> > +
> > +     func_no = ntb->epf->func_no;
> > +     vfunc_no = ntb->epf->vfunc_no;
> > +
> > +     ret = pci_epc_raise_irq(ntb->epf->epc,
> > +                             func_no,
> > +                             vfunc_no,
> > +                             PCI_EPC_IRQ_MSI,
> > +                             interrupt_num + 1);
> > +     if (ret) {
> > +             dev_err(&ntb->ntb.dev, "intf: Failed to raise IRQ\n");
> > +             return ret;
> > +     }
> > +
> > +     return 0;
>
> Equivalent to:
>
>   if (ret)
>     dev_err(...);
>
>   return ret;
>
> > +}
> > +
> > +static u64 vntb_epf_db_read(struct ntb_dev *ndev)
> > +{
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +
> > +     return ntb->db;
> > +}
> > +
> > +static int vntb_epf_mw_get_align(struct ntb_dev *ndev, int pidx, int idx,
> > +                     resource_size_t *addr_align,
> > +                     resource_size_t *size_align,
> > +                     resource_size_t *size_max)
> > +{
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +
> > +     if (addr_align)
> > +             *addr_align = SZ_4K;
> > +
> > +     if (size_align)
> > +             *size_align = 1;
> > +
> > +     if (size_max)
> > +             *size_max = ntb->mws_size[idx];
> > +
> > +     return 0;
> > +}
> > +
> > +static u64 vntb_epf_link_is_up(struct ntb_dev *ndev,
> > +                     enum ntb_speed *speed,
> > +                     enum ntb_width *width)
> > +{
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +
> > +     return ntb->reg->link_status;
> > +}
> > +
> > +static int vntb_epf_db_clear_mask(struct ntb_dev *ndev, u64 db_bits)
> > +{
> > +     return 0;
> > +}
> > +
> > +static int vntb_epf_db_clear(struct ntb_dev *ndev, u64 db_bits)
> > +{
> > +     struct epf_ntb *ntb = ntb_ndev(ndev);
> > +
> > +     ntb->db &= ~db_bits;
> > +     return 0;
> > +}
> > +
> > +static int vntb_epf_link_disable(struct ntb_dev *ntb)
> > +{
> > +     return 0;
> > +}
> > +
> > +static const struct ntb_dev_ops vntb_epf_ops = {
> > +     .mw_count               = vntb_epf_mw_count,
> > +     .spad_count             = vntb_epf_spad_count,
> > +     .peer_mw_count          = vntb_epf_peer_mw_count,
> > +     .db_valid_mask          = vntb_epf_db_valid_mask,
> > +     .db_set_mask            = vntb_epf_db_set_mask,
> > +     .mw_set_trans           = vntb_epf_mw_set_trans,
> > +     .mw_clear_trans         = vntb_epf_mw_clear_trans,
> > +     .peer_mw_get_addr       = vntb_epf_peer_mw_get_addr,
> > +     .link_enable            = vntb_epf_link_enable,
> > +     .spad_read              = vntb_epf_spad_read,
> > +     .spad_write             = vntb_epf_spad_write,
> > +     .peer_spad_read         = vntb_epf_peer_spad_read,
> > +     .peer_spad_write        = vntb_epf_peer_spad_write,
> > +     .peer_db_set            = vntb_epf_peer_db_set,
> > +     .db_read                = vntb_epf_db_read,
> > +     .mw_get_align           = vntb_epf_mw_get_align,
> > +     .link_is_up             = vntb_epf_link_is_up,
> > +     .db_clear_mask          = vntb_epf_db_clear_mask,
> > +     .db_clear               = vntb_epf_db_clear,
> > +     .link_disable           = vntb_epf_link_disable,
> > +};
> > +
> > +static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > +{
> > +     int ret;
> > +     struct epf_ntb *ndev = (struct epf_ntb *)pdev->sysdata;
> > +     struct device *dev = &pdev->dev;
> > +
> > +     ndev->ntb.pdev = pdev;
> > +     ndev->ntb.topo = NTB_TOPO_NONE;
> > +     ndev->ntb.ops =  &vntb_epf_ops;
> > +
> > +     ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > +     if (ret) {
> > +             dev_err(dev, "Cannot set DMA mask\n");
> > +             return -1;
>
> return -EINVAL;
>
> > +     }
> > +
> > +     ret = ntb_register_device(&ndev->ntb);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to register NTB device\n");
> > +             goto err_register_dev;
> > +     }
> > +
> > +     dev_info(dev, "PCI Virtual NTB driver loaded\n");
> > +     return 0;
> > +
> > +err_register_dev:
> > +     return -1;
>
> return -EINVAL;
>
> > +}
> > +
> > +static const struct pci_device_id pci_vntb_table[] = {
> > +     {
> > +             PCI_DEVICE(VNTB_VID, VNTB_PID),
> > +     },
> > +     {},
> > +};
> > +
> > +static struct pci_driver vntb_pci_driver = {
> > +     .name           = "pci-vntb",
> > +     .id_table       = pci_vntb_table,
> > +     .probe          = pci_vntb_probe,
> > +};
> > +
> > +/* ============ PCIe EPF Driver Bind ====================*/
> > +
> > +/**
> > + * epf_ntb_bind() - Initialize endpoint controller to provide NTB functionality
> > + * @epf: NTB endpoint function device
> > + *
> > + * Initialize both the endpoint controllers associated with NTB function device.
> > + * Invoked when a primary interface or secondary interface is bound to EPC
> > + * device. This function will succeed only when EPC is bound to both the
> > + * interfaces.
> > + */
> > +static int epf_ntb_bind(struct pci_epf *epf)
> > +{
> > +     struct epf_ntb *ntb = epf_get_drvdata(epf);
> > +     struct device *dev = &epf->dev;
> > +     int ret;
> > +
> > +     if (!epf->epc) {
> > +             dev_dbg(dev, "PRIMARY EPC interface not yet bound\n");
> > +             return 0;
> > +     }
> > +
> > +     ret = epf_ntb_init_epc_bar(ntb);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to create NTB EPC\n");
> > +             goto err_bar_init;
> > +     }
> > +
> > +     ret = epf_ntb_config_spad_bar_alloc(ntb);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to allocate BAR memory\n");
> > +             goto err_bar_alloc;
> > +     }
> > +
> > +     ret = epf_ntb_epc_init(ntb);
> > +     if (ret) {
> > +             dev_err(dev, "Failed to initialize EPC\n");
> > +             goto err_bar_alloc;
> > +     }
> > +
> > +     epf_set_drvdata(epf, ntb);
> > +
> > +     if (pci_register_driver(&vntb_pci_driver)) {
> > +             dev_err(dev, "failure register vntb pci driver\n");
> > +             goto err_bar_alloc;
> > +     }
> > +
> > +     vpci_bus(ntb);
> > +
> > +     return 0;
> > +
> > +err_bar_alloc:
> > +     epf_ntb_config_spad_bar_free(ntb);
> > +
> > +err_bar_init:
> > +     epf_ntb_epc_destroy(ntb);
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> > + * epf_ntb_unbind() - Cleanup the initialization from epf_ntb_bind()
> > + * @epf: NTB endpoint function device
> > + *
> > + * Cleanup the initialization from epf_ntb_bind()
> > + */
> > +static void epf_ntb_unbind(struct pci_epf *epf)
> > +{
> > +     struct epf_ntb *ntb = epf_get_drvdata(epf);
> > +
> > +     epf_ntb_epc_cleanup(ntb);
> > +     epf_ntb_config_spad_bar_free(ntb);
> > +     epf_ntb_epc_destroy(ntb);
> > +
> > +     pci_unregister_driver(&vntb_pci_driver);
> > +}
> > +
> > +// EPF driver probe
> > +static struct pci_epf_ops epf_ntb_ops = {
> > +     .bind   = epf_ntb_bind,
> > +     .unbind = epf_ntb_unbind,
> > +     .add_cfs = epf_ntb_add_cfs,
> > +};
> > +
> > +/**
> > + * epf_ntb_probe() - Probe NTB function driver
> > + * @epf: NTB endpoint function device
> > + *
> > + * Probe NTB function driver when endpoint function bus detects a NTB
> > + * endpoint function.
> > + */
> > +static int epf_ntb_probe(struct pci_epf *epf)
> > +{
> > +     struct epf_ntb *ntb;
> > +     struct device *dev;
> > +
> > +     dev = &epf->dev;
> > +
> > +     ntb = devm_kzalloc(dev, sizeof(*ntb), GFP_KERNEL);
> > +     if (!ntb)
> > +             return -ENOMEM;
> > +
> > +     epf->header = &epf_ntb_header;
> > +     ntb->epf = epf;
> > +     epf_set_drvdata(epf, ntb);
> > +
> > +     dev_info(dev, "pci-ep epf driver loaded\n");
>
> I think most drivers don't announce when they're loaded because it
> really doesn't help the user.
>
> > +     return 0;
> > +}
> > +
> > +static const struct pci_epf_device_id epf_ntb_ids[] = {
> > +     {
> > +             .name = "pci_epf_vntb",
> > +     },
> > +     {},
> > +};
> > +
> > +static struct pci_epf_driver epf_ntb_driver = {
> > +     .driver.name    = "pci_epf_vntb",
> > +     .probe          = epf_ntb_probe,
> > +     .id_table       = epf_ntb_ids,
> > +     .ops            = &epf_ntb_ops,
> > +     .owner          = THIS_MODULE,
> > +};
> > +
> > +
>
> Spurious blank line.
>
> > +static int __init epf_ntb_init(void)
> > +{
> > +     int ret;
> > +
> > +     kpcintb_workqueue = alloc_workqueue("kpcintb", WQ_MEM_RECLAIM |
> > +                                         WQ_HIGHPRI, 0);
> > +     ret = pci_epf_register_driver(&epf_ntb_driver);
> > +     if (ret) {
> > +             destroy_workqueue(kpcintb_workqueue);
> > +             pr_err("Failed to register pci epf ntb driver --> %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +module_init(epf_ntb_init);
> > +
> > +static void __exit epf_ntb_exit(void)
> > +{
> > +     pci_epf_unregister_driver(&epf_ntb_driver);
> > +     destroy_workqueue(kpcintb_workqueue);
> > +}
> > +module_exit(epf_ntb_exit);
> > +
> > +MODULE_DESCRIPTION("PCI EPF NTB DRIVER");
> > +MODULE_AUTHOR("Frank Li <Frank.li@nxp.com>");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.24.0.rc1
> >

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-02-17 22:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-15  5:38 [RFC PATCH 0/4] NTB function for PCIe RC to EP connection Frank Li
2022-02-15  5:38 ` [RFC PATCH 1/4] PCI: designware-ep: Allow pcie_ep_set_bar change inbound map address Frank Li
2022-02-17 21:53   ` Bjorn Helgaas
2022-02-17 22:13     ` Zhi Li
2022-02-15  5:38 ` [RFC PATCH 2/4] NTB: epf: Added more flexible memory map method Frank Li
2022-02-17 21:59   ` Bjorn Helgaas
2022-02-17 22:24     ` Zhi Li
2022-02-17 22:43       ` Bjorn Helgaas
2022-02-15  5:38 ` [RFC PATCH 3/4] NTB: EPF: support NTB transfer between PCI RC and EP connection Frank Li
2022-02-15  7:57   ` kernel test robot
2022-02-16  7:13   ` kernel test robot
2022-02-16  7:13     ` kernel test robot
2022-02-17 22:38   ` Bjorn Helgaas
2022-02-17 22:59     ` Zhi Li
2022-02-15  5:38 ` [RFC PATCH 4/4] Documentation: PCI: Add specification for the PCI vNTB function device Frank Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.