PCI: endpoint: Add DMA to Linux PCI EP Framework

Commit Message

Alan Mikhak May 23, 2019, 10:24 p.m. UTC

This patch depends on patch the following patches:
[PATCH v2 1/2] tools: PCI: Fix broken pcitest compilation
[PATCH v2 2/2] tools: PCI: Fix compiler warning in pcitest

The Linux PCI Endpoint Framework currently does not support initiation of
DMA read and write operations. This patch extends the Linux PCI Endpoint
Framework by adding support for the user of pcitest to inititate DMA
read and write operations via PCIe endpoint controller drivers. This patch
provides the means but leaves it up to individual PCIe endpoint controller
drivers to implement DMA support, if desired.

This patch provides support for the pcitest user to instruct the endpoint
to initiate local DMA transfers consisting of single or linked-list blocks
into endpoint buffers using the endpoint DMA controller. It anticipates
that future patches would add support for the pcitest user to instruct
the endpoint to participate in remote DMA transfers initiated from the
root complex into endpoint buffers using the endpoint DMA controller.

This patch depends on the first two patches in its patchset to resolve
a pre-existing pcitest compilation error.

* Add -d flag to pcitest command line options so user can specify
that a read or write command should execute using local DMA to be
initiated by endpoint.

* Add -L flag to pcitest command line options so user can specify
that DMA operation should execute in linked-list mode.

* Add struct pcitest_dma for pcitest to communicate DMA options
from host userspace to pci_endpoint_test driver in host kernel
via two new ioctls PCITEST_WRITE_DMA and PCITEST_READ_DMA.

* Add command flags so pci_endpoint_test driver running on host
can communicate DMA read and write options across the PCI bus 
to pci-epf-test driver running on endpoint.

* Add struct pci_epc_dma so pci-epf-test driver can create DMA
read and write descriptors for single or linked-list DMA operations
and pass such descriptors to pci-epc-core via new functions
pci_epc_dma_read() and pci_epc_dma_write().

* Add four new functions in pci-epf-test driver to implement
new DMA read and write tests by initiating local DMA transfers
in linked-list and single modes via PCIe endpoint controller
drivers: pci_epf_test_read_dma(), pci_epf_test_read_dma_list(),
pci_epf_test_write_dma(), and pci_epf_test_write_dma_list().

* Add dma_read and dma_write functions to struct pci_epc_ops
so pci_epc_dma_read() and pci_epc_dma_write() functions can
pass DMA descriptors down the stack to pcie-designware-ep layer.

* Add dma_read and dma_write functions to struct dw_pcie_ep_ops
so pcie-designware-ep layer can communicate DMA descriptors down
the stack to vendor PCIe endpoint controller drivers.

* Add dma_base pointer to struct dw_pcie for vendor PCIe endpoint
controller driver to set if it implements DMA operations.

* Add two common pcie-designware functions dw_pcie_writel_dma()
and dw_pcie_readl_dma() for use by vendor PCIe endpoint
controllers to access DMA registers via the dma_base pointer.

Signed-off-by: Alan Mikhak <[e-mail]>
---
 drivers/misc/pci_endpoint_test.c                |  72 +++++++-
 drivers/pci/controller/dwc/pcie-designware-ep.c |  22 +++
 drivers/pci/controller/dwc/pcie-designware.h    |  13 ++
 drivers/pci/endpoint/functions/pci-epf-test.c   | 211 +++++++++++++++++++++++-
 drivers/pci/endpoint/pci-epc-core.c             |  46 ++++++
 include/linux/pci-epc.h                         |  45 +++++
 include/uapi/linux/pcitest.h                    |   7 +
 tools/pci/pcitest.c                             |  29 +++-
 8 files changed, 432 insertions(+), 13 deletions(-)

Comments

Gustavo Pimentel May 24, 2019, 8:59 a.m. UTC | #1

Hi Alan,

This patch implementation is very HW implementation dependent and 
requires the DMA to exposed through PCIe BARs, which aren't always the 
case. Besides, you are defining some control bits on 
include/linux/pci-epc.h that may not have any meaning to other types of 
DMA.

I don't think this was what Kishon had in mind when he developed the 
pcitest, but let see what Kishon was to say about it.

I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API 
and which I submitted some days ago.
By having a DMA driver which implemented using DMAengine API, means the 
pcitest can use the DMAengine client API, which will be completely 
generic to any other DMA implementation.

My DMA driver for DWC PCI was done thinking on my use case, which is was 
interacting from the Root Complex side with DMA IP implemented on the 
Endpoint, which was exposed through PCI BARs. However, I think it would 
be possible to reuse the same core code and instead of using the PCI-glue 
to adapt it and be used easily on the Endpoint side and to be triggered 
there.

Gustavo

-----Original Message-----
From: Alan Mikhak <[e-mail]> 

Sent: 23 de maio de 2019 23:24
To: [e-mail]; 
[e-mail]; [e-mail]; [e-mail]; 
[e-mail]; [e-mail]; [e-mail]; 
[e-mail]; [e-mail]; 
[e-mail]; [e-mail]; [e-mail]; 
[e-mail]; [e-mail]
Cc: Alan Mikhak 
<[e-mail]>
Subject: [PATCH] PCI: endpoint: Add DMA to Linux 
PCI EP Framework

This patch depends on patch the following patches:
[PATCH v2 1/2] tools: PCI: Fix broken pcitest compilation
[PATCH v2 2/2] tools: PCI: Fix compiler warning in pcitest

The Linux PCI Endpoint Framework currently does not support initiation of
DMA read and write operations. This patch extends the Linux PCI Endpoint
Framework by adding support for the user of pcitest to inititate DMA
read and write operations via PCIe endpoint controller drivers. This 
patch
provides the means but leaves it up to individual PCIe endpoint 
controller
drivers to implement DMA support, if desired.

This patch provides support for the pcitest user to instruct the endpoint
to initiate local DMA transfers consisting of single or linked-list 
blocks
into endpoint buffers using the endpoint DMA controller. It anticipates
that future patches would add support for the pcitest user to instruct
the endpoint to participate in remote DMA transfers initiated from the
root complex into endpoint buffers using the endpoint DMA controller.

This patch depends on the first two patches in its patchset to resolve
a pre-existing pcitest compilation error.

* Add -d flag to pcitest command line options so user can specify
that a read or write command should execute using local DMA to be
initiated by endpoint.

* Add -L flag to pcitest command line options so user can specify
that DMA operation should execute in linked-list mode.

* Add struct pcitest_dma for pcitest to communicate DMA options
from host userspace to pci_endpoint_test driver in host kernel
via two new ioctls PCITEST_WRITE_DMA and PCITEST_READ_DMA.

* Add command flags so pci_endpoint_test driver running on host
can communicate DMA read and write options across the PCI bus 
to pci-epf-test driver running on endpoint.

* Add struct pci_epc_dma so pci-epf-test driver can create DMA
read and write descriptors for single or linked-list DMA operations
and pass such descriptors to pci-epc-core via new functions
pci_epc_dma_read() and pci_epc_dma_write().

* Add four new functions in pci-epf-test driver to implement
new DMA read and write tests by initiating local DMA transfers
in linked-list and single modes via PCIe endpoint controller
drivers: pci_epf_test_read_dma(), pci_epf_test_read_dma_list(),
pci_epf_test_write_dma(), and pci_epf_test_write_dma_list().

* Add dma_read and dma_write functions to struct pci_epc_ops
so pci_epc_dma_read() and pci_epc_dma_write() functions can
pass DMA descriptors down the stack to pcie-designware-ep layer.

* Add dma_read and dma_write functions to struct dw_pcie_ep_ops
so pcie-designware-ep layer can communicate DMA descriptors down
the stack to vendor PCIe endpoint controller drivers.

* Add dma_base pointer to struct dw_pcie for vendor PCIe endpoint
controller driver to set if it implements DMA operations.

* Add two common pcie-designware functions dw_pcie_writel_dma()
and dw_pcie_readl_dma() for use by vendor PCIe endpoint
controllers to access DMA registers via the dma_base pointer.

Signed-off-by: Alan Mikhak <[e-mail]>
---
 
drivers/misc/pci_endpoint_test.c                |  72 +++++++-
 drivers/pci/controller/dwc/pcie-designware-ep.c |  22 +++
 drivers/pci/controller/dwc/pcie-designware.h    |  13 ++
 drivers/pci/endpoint/functions/pci-epf-test.c   | 211 
+++++++++++++++++++++++-
 drivers/pci/endpoint/pci-epc-core.c             |  46 ++++++
 include/linux/pci-epc.h                         |  45 +++++
 include/uapi/linux/pcitest.h                    |   7 +
 tools/pci/pcitest.c                             |  29 +++-
 8 files changed, 432 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/pci_endpoint_test.c 
b/drivers/misc/pci_endpoint_test.c
index 7b015f2a1c6f..63b86d81a6b5 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -34,6 +34,7 @@
 #include <linux/pci_regs.h>
 
 #include <uapi/linux/pcitest.h>
+#include <linux/uaccess.h>
 
 #define DRV_MODULE_NAME				"pci-endpoint-test"
 
@@ -51,6 +52,25 @@
 #define COMMAND_READ				BIT(3)
 #define COMMAND_WRITE				BIT(4)
 #define COMMAND_COPY				BIT(5)
+#define COMMAND_FLAG2				BIT(30)
+#define COMMAND_FLAG1				BIT(31)
+
+#define COMMAND_FLAGS				(COMMAND_FLAG1 | \
+						 COMMAND_FLAG2)
+
+#define COMMAND_FLAG_NONE			0
+#define COMMAND_FLAG_DMA			COMMAND_FLAG1
+#define COMMAND_FLAG_DMA_LIST			COMMAND_FLAG2
+
+#define COMMAND_READ_DMA			(COMMAND_READ | \
+						 COMMAND_FLAG_DMA)
+#define COMMAND_READ_DMA_LIST			(COMMAND_READ_DMA | \
+						 COMMAND_FLAG_DMA_LIST)
+
+#define COMMAND_WRITE_DMA			(COMMAND_WRITE | \
+						 COMMAND_FLAG_DMA)
+#define COMMAND_WRITE_DMA_LIST			(COMMAND_WRITE_DMA | \
+						 COMMAND_FLAG_DMA_LIST)
 
 #define PCI_ENDPOINT_TEST_STATUS		0x8
 #define STATUS_READ_SUCCESS			BIT(0)
@@ -425,7 +445,9 @@ static bool pci_endpoint_test_copy(struct 
pci_endpoint_test *test, size_t size)
 	return ret;
 }
 
-static bool pci_endpoint_test_write(struct pci_endpoint_test *test, 
size_t size)
+static bool pci_endpoint_test_write(struct pci_endpoint_test *test,
+				    size_t size,
+				    u32 flags)
 {
 	bool ret = false;
 	u32 reg;
@@ -480,7 +502,7 @@ static bool pci_endpoint_test_write(struct 
pci_endpoint_test *test, size_t size)
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
-				 COMMAND_READ);
+				 COMMAND_READ | flags);
 
 	wait_for_completion(&test->irq_raised);
 
@@ -494,7 +516,24 @@ static bool pci_endpoint_test_write(struct 
pci_endpoint_test *test, size_t size)
 	return ret;
 }
 
-static bool pci_endpoint_test_read(struct pci_endpoint_test *test, 
size_t size)
+static bool pci_endpoint_test_write_dma(struct pci_endpoint_test *test,
+					unsigned long arg)
+{
+	u32 flags = COMMAND_FLAG_DMA;
+	struct pcitest_dma dma;
+
+	if (copy_from_user(&dma, (void *)arg, sizeof(struct pcitest_dma)))
+		return -EACCES;
+
+	if (dma.list)
+		flags |= COMMAND_FLAG_DMA_LIST;
+
+	return pci_endpoint_test_write(test, dma.size, flags);
+}
+
+static bool pci_endpoint_test_read(struct pci_endpoint_test *test,
+				   size_t size,
+				   u32 flags)
 {
 	bool ret = false;
 	void *addr;
@@ -542,7 +581,7 @@ static bool pci_endpoint_test_read(struct 
pci_endpoint_test *test, size_t size)
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
-				 COMMAND_WRITE);
+				 COMMAND_WRITE | flags);
 
 	wait_for_completion(&test->irq_raised);
 
@@ -555,6 +594,21 @@ static bool pci_endpoint_test_read(struct 
pci_endpoint_test *test, size_t size)
 	return ret;
 }
 
+static bool pci_endpoint_test_read_dma(struct pci_endpoint_test *test,
+				       unsigned long arg)
+{
+	u32 flags = COMMAND_FLAG_DMA;
+	struct pcitest_dma dma;
+
+	if (copy_from_user(&dma, (void *)arg, sizeof(struct pcitest_dma)))
+		return -EACCES;
+
+	if (dma.list)
+		flags |= COMMAND_FLAG_DMA_LIST;
+
+	return pci_endpoint_test_read(test, dma.size, flags);
+}
+
 static bool pci_endpoint_test_set_irq(struct pci_endpoint_test *test,
 				      int req_irq_type)
 {
@@ -612,11 +666,17 @@ static long pci_endpoint_test_ioctl(struct file 
*file, unsigned int cmd,
 	case PCITEST_MSIX:
 		ret = pci_endpoint_test_msi_irq(test, arg, cmd == PCITEST_MSIX);
 		break;
+	case PCITEST_WRITE_DMA:
+		ret = pci_endpoint_test_write_dma(test, arg);
+		break;
+	case PCITEST_READ_DMA:
+		ret = pci_endpoint_test_read_dma(test, arg);
+		break;
 	case PCITEST_WRITE:
-		ret = pci_endpoint_test_write(test, arg);
+		ret = pci_endpoint_test_write(test, arg, COMMAND_FLAG_NONE);
 		break;
 	case PCITEST_READ:
-		ret = pci_endpoint_test_read(test, arg);
+		ret = pci_endpoint_test_read(test, arg, COMMAND_FLAG_NONE);
 		break;
 	case PCITEST_COPY:
 		ret = pci_endpoint_test_copy(test, arg);
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c 
b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 2bf5a35c0570..7e25c0f5edf1 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -366,6 +366,26 @@ dw_pcie_ep_get_features(struct pci_epc *epc, u8 
func_no)
 	return ep->ops->get_features(ep);
 }
 
+static int dw_pcie_ep_dma_read(struct pci_epc *epc, struct pci_epc_dma 
*dma)
+{
+	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+
+	if (!ep->ops->dma_read)
+		return -EINVAL;
+
+	return ep->ops->dma_read(ep, dma);
+}
+
+static int dw_pcie_ep_dma_write(struct pci_epc *epc, struct pci_epc_dma 
*dma)
+{
+	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+
+	if (!ep->ops->dma_write)
+		return -EINVAL;
+
+	return ep->ops->dma_write(ep, dma);
+}
+
 static const struct pci_epc_ops epc_ops = {
 	.write_header		= dw_pcie_ep_write_header,
 	.set_bar		= dw_pcie_ep_set_bar,
@@ -380,6 +400,8 @@ static const struct pci_epc_ops epc_ops = {
 	.start			= dw_pcie_ep_start,
 	.stop			= dw_pcie_ep_stop,
 	.get_features		= dw_pcie_ep_get_features,
+	.dma_read		= dw_pcie_ep_dma_read,
+	.dma_write		= dw_pcie_ep_dma_write
 };
 
 int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h 
b/drivers/pci/controller/dwc/pcie-designware.h
index b8993f2b78df..11d44ec8acc7 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -197,6 +197,8 @@ struct dw_pcie_ep_ops {
 	int	(*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
 			     enum pci_epc_irq_type type, u16 interrupt_num);
 	const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
+	int	(*dma_read)(struct dw_pcie_ep *ep, struct pci_epc_dma *dma);
+	int	(*dma_write)(struct dw_pcie_ep *ep, struct pci_epc_dma *dma);
 };
 
 struct dw_pcie_ep {
@@ -238,6 +240,7 @@ struct dw_pcie {
 	void __iomem		*dbi_base2;
 	/* Used when iatu_unroll_enabled is true */
 	void __iomem		*atu_base;
+	void __iomem		*dma_base;
 	u32			num_viewport;
 	u8			iatu_unroll_enabled;
 	struct pcie_port	pp;
@@ -323,6 +326,16 @@ static inline u32 dw_pcie_readl_atu(struct dw_pcie 
*pci, u32 reg)
 	return __dw_pcie_read_dbi(pci, pci->atu_base, reg, 0x4);
 }
 
+static inline void dw_pcie_writel_dma(struct dw_pcie *pci, u32 reg, u32 
val)
+{
+	__dw_pcie_write_dbi(pci, pci->dma_base, reg, 0x4, val);
+}
+
+static inline u32 dw_pcie_readl_dma(struct dw_pcie *pci, u32 reg)
+{
+	return __dw_pcie_read_dbi(pci, pci->dma_base, reg, 0x4);
+}
+
 static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
 {
 	u32 reg;
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c 
b/drivers/pci/endpoint/functions/pci-epf-test.c
index 27806987e93b..3910073712e9 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -28,6 +28,25 @@
 #define COMMAND_READ			BIT(3)
 #define COMMAND_WRITE			BIT(4)
 #define COMMAND_COPY			BIT(5)
+#define COMMAND_FLAG2			BIT(30)
+#define COMMAND_FLAG1			BIT(31)
+
+#define COMMAND_FLAGS			(COMMAND_FLAG1 | \
+					 COMMAND_FLAG2)
+
+#define COMMAND_FLAG_NONE		0
+#define COMMAND_FLAG_DMA		COMMAND_FLAG1
+#define COMMAND_FLAG_DMA_LIST		COMMAND_FLAG2
+
+#define COMMAND_READ_DMA		(COMMAND_READ | \
+					 COMMAND_FLAG_DMA)
+#define COMMAND_READ_DMA_LIST		(COMMAND_READ_DMA | \
+					 COMMAND_FLAG_DMA_LIST)
+
+#define COMMAND_WRITE_DMA		(COMMAND_WRITE | \
+					 COMMAND_FLAG_DMA)
+#define COMMAND_WRITE_DMA_LIST		(COMMAND_WRITE_DMA | \
+					 COMMAND_FLAG_DMA_LIST)
 
 #define STATUS_READ_SUCCESS		BIT(0)
 #define STATUS_READ_FAIL		BIT(1)
@@ -187,6 +206,93 @@ static int pci_epf_test_read(struct pci_epf_test 
*epf_test)
 	return ret;
 }
 
+static int pci_epf_test_read_dma(struct pci_epf_test *epf_test)
+{
+	int ret = -ENOMEM;
+	struct pci_epf *epf = epf_test->epf;
+	struct device *dev = &epf->dev;
+	struct pci_epc *epc = epf->epc;
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+	struct pci_epc_dma dma;
+	u32 crc32;
+	void *buf;
+
+	buf = kzalloc(reg->size, GFP_KERNEL);
+	if (buf) {
+		dma.control = PCI_EPC_DMA_CONTROL_LIE;
+		dma.size = reg->size;
+		dma.sar = reg->src_addr;
+		dma.dar = virt_to_phys(buf);
+
+		ret = pci_epc_dma_read(epc, &dma);
+		if (ret) {
+			dev_err(dev, "pci_epc_dma_read %d\n", ret);
+		} else {
+			crc32 = crc32_le(~0, buf, reg->size);
+			if (crc32 != reg->checksum)
+				ret = -EIO;
+		}
+
+		kfree(buf);
+	}
+
+	return ret;
+}
+
+static int pci_epf_test_read_dma_list(struct pci_epf_test *epf_test)
+{
+	int ret = -ENOMEM;
+	struct pci_epf *epf = epf_test->epf;
+	struct device *dev = &epf->dev;
+	struct pci_epc *epc = epf->epc;
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+	struct pci_epc_dma *dma;
+	u32 crc32;
+	void *buf;
+
+	dma = kcalloc(3, sizeof(*dma), GFP_KERNEL);
+	if (dma) {
+		buf = kzalloc(reg->size, GFP_KERNEL);
+		if (buf) {
+			int half_size = reg->size >> 1;
+			phys_addr_t phys_addr = virt_to_phys(buf);
+
+			dma[0].control = PCI_EPC_DMA_CONTROL_CB;
+			dma[0].size = half_size ? half_size : 1;
+			dma[0].sar = reg->src_addr;
+			dma[0].dar = phys_addr;
+
+			dma[1].control = PCI_EPC_DMA_CONTROL_CB |
+					 PCI_EPC_DMA_CONTROL_LIE;
+			dma[1].size = reg->size - half_size;
+			dma[1].sar = reg->src_addr + half_size;
+			dma[1].dar = phys_addr + half_size;
+
+			dma[2].control = PCI_EPC_DMA_CONTROL_EOL;
+			dma[2].size = 0;
+			dma[2].sar = virt_to_phys(dma);
+			dma[2].dar = 0;
+
+			ret = pci_epc_dma_read(epc, dma);
+			if (ret) {
+				dev_err(dev, "pci_epc_dma_read %d\n", ret);
+			} else {
+				crc32 = crc32_le(~0, buf, reg->size);
+				if (crc32 != reg->checksum)
+					ret = -EIO;
+			}
+
+			kfree(buf);
+		}
+
+		kfree(dma);
+	}
+
+	return ret;
+}
+
 static int pci_epf_test_write(struct pci_epf_test *epf_test)
 {
 	int ret;
@@ -244,6 +350,87 @@ static int pci_epf_test_write(struct pci_epf_test 
*epf_test)
 	return ret;
 }
 
+static int pci_epf_test_write_dma(struct pci_epf_test *epf_test)
+{
+	int ret = -ENOMEM;
+	struct pci_epf *epf = epf_test->epf;
+	struct device *dev = &epf->dev;
+	struct pci_epc *epc = epf->epc;
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+	struct pci_epc_dma dma;
+	void *buf;
+
+	buf = kzalloc(reg->size, GFP_KERNEL);
+	if (buf) {
+		get_random_bytes(buf, reg->size);
+		reg->checksum = crc32_le(~0, buf, reg->size);
+
+		dma.control = PCI_EPC_DMA_CONTROL_LIE;
+		dma.size = reg->size;
+		dma.sar = virt_to_phys(buf);
+		dma.dar = reg->dst_addr;
+
+		ret = pci_epc_dma_write(epc, &dma);
+		if (ret)
+			dev_err(dev, "pci_epc_dma_write %d\n", ret);
+
+		kfree(buf);
+	}
+
+	return ret;
+}
+
+static int pci_epf_test_write_dma_list(struct pci_epf_test *epf_test)
+{
+	int ret = -ENOMEM;
+	struct pci_epf *epf = epf_test->epf;
+	struct device *dev = &epf->dev;
+	struct pci_epc *epc = epf->epc;
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+	struct pci_epc_dma *dma;
+	void *buf;
+
+	dma = kcalloc(3, sizeof(*dma), GFP_KERNEL);
+	if (dma) {
+		buf = kzalloc(reg->size, GFP_KERNEL);
+		if (buf) {
+			int half_size = reg->size >> 1;
+			phys_addr_t phys_addr = virt_to_phys(buf);
+
+			get_random_bytes(buf, reg->size);
+			reg->checksum = crc32_le(~0, buf, reg->size);
+
+			dma[0].control = PCI_EPC_DMA_CONTROL_CB;
+			dma[0].size = half_size ? half_size : 1;
+			dma[0].sar = phys_addr;
+			dma[0].dar = reg->dst_addr;
+
+			dma[1].control = PCI_EPC_DMA_CONTROL_CB |
+					 PCI_EPC_DMA_CONTROL_LIE;
+			dma[1].size = reg->size - half_size;
+			dma[1].sar = phys_addr + half_size;
+			dma[1].dar = reg->dst_addr + half_size;
+
+			dma[2].control = PCI_EPC_DMA_CONTROL_EOL;
+			dma[2].size = 0;
+			dma[2].sar = virt_to_phys(dma);
+			dma[2].dar = 0;
+
+			ret = pci_epc_dma_write(epc, dma);
+			if (ret)
+				dev_err(dev, "pci_epc_dma_write %d\n", ret);
+
+			kfree(buf);
+		}
+
+		kfree(dma);
+	}
+
+	return ret;
+}
+
 static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test, u8 
irq_type,
 				   u16 irq)
 {
@@ -303,18 +490,34 @@ static void pci_epf_test_cmd_handler(struct 
work_struct *work)
 	}
 
 	if (command & COMMAND_WRITE) {
-		ret = pci_epf_test_write(epf_test);
-		if (ret)
-			reg->status |= STATUS_WRITE_FAIL;
+		command &= (COMMAND_WRITE | COMMAND_FLAGS);
+		if (command == COMMAND_WRITE)
+			ret = pci_epf_test_write(epf_test);
+		else if (command == COMMAND_WRITE_DMA)
+			ret = pci_epf_test_write_dma(epf_test);
+		else if (command == COMMAND_WRITE_DMA_LIST)
+			ret = pci_epf_test_write_dma_list(epf_test);
 		else
+			ret = -EINVAL;
+		if (!ret)
 			reg->status |= STATUS_WRITE_SUCCESS;
+		else
+			reg->status |= STATUS_WRITE_FAIL;
 		pci_epf_test_raise_irq(epf_test, reg->irq_type,
 				       reg->irq_number);
 		goto reset_handler;
 	}
 
 	if (command & COMMAND_READ) {
-		ret = pci_epf_test_read(epf_test);
+		command &= (COMMAND_READ | COMMAND_FLAGS);
+		if (command == COMMAND_READ)
+			ret = pci_epf_test_read(epf_test);
+		else if (command == COMMAND_READ_DMA)
+			ret = pci_epf_test_read_dma(epf_test);
+		else if (command == COMMAND_READ_DMA_LIST)
+			ret = pci_epf_test_read_dma_list(epf_test);
+		else
+			ret = -EINVAL;
 		if (!ret)
 			reg->status |= STATUS_READ_SUCCESS;
 		else
diff --git a/drivers/pci/endpoint/pci-epc-core.c 
b/drivers/pci/endpoint/pci-epc-core.c
index e4712a0f249c..a57e501d4abc 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -107,6 +107,52 @@ unsigned int pci_epc_get_first_free_bar(const struct 
pci_epc_features
 EXPORT_SYMBOL_GPL(pci_epc_get_first_free_bar);
 
 /**
+ * pci_epc_dma_write() - DMA a block of memory to remote address
+ * @epc: the EPC device on which to perform DMA transfer
+ * @dma: DMA descriptors array
+ *
+ * Write contents of local memory to remote memory by DMA.
+ */
+int pci_epc_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma)
+{
+	int ret;
+	unsigned long flags;
+
+	if (IS_ERR(epc) || !epc->ops->dma_write)
+		return -EINVAL;
+
+	spin_lock_irqsave(&epc->lock, flags);
+	ret = epc->ops->dma_write(epc, dma);
+	spin_unlock_irqrestore(&epc->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_epc_dma_write);
+
+/**
+ * pci_epc_dma_read() - DMA a block of memory from remote address
+ * @epc: the EPC device on which to perform DMA transfer
+ * @dma: DMA descriptors array
+ *
+ * Read contents of remote memory into local memory by DMA.
+ */
+int pci_epc_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma)
+{
+	int ret;
+	unsigned long flags;
+
+	if (IS_ERR(epc) || !epc->ops->dma_read)
+		return -EINVAL;
+
+	spin_lock_irqsave(&epc->lock, flags);
+	ret = epc->ops->dma_read(epc, dma);
+	spin_unlock_irqrestore(&epc->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_epc_dma_read);
+
+/**
  * pci_epc_get_features() - get the features supported by EPC
  * @epc: the features supported by *this* EPC device will be returned
  * @func_no: the features supported by the EPC device specific to the
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index f641badc2c61..d845f13d0baf 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -21,6 +21,45 @@ enum pci_epc_irq_type {
 };
 
 /**
+ * struct pci_epc_dma - descriptor for a DMA transfer element
+ * @control: DMA channel control bits for read or write transfer
+ * @size: size of DMA transfer element
+ * @sar: source addrees for DMA transfer element
+ * @dar: destination address for DMA transfer element
+ */
+
+struct pci_epc_dma {
+	u32	control;
+	u32	size;
+	u64	sar;
+	u64	dar;
+};
+
+#define PCI_EPC_DMA_CONTROL_CB          (BIT(0))
+#define PCI_EPC_DMA_CONTROL_TCB         (BIT(1))
+#define PCI_EPC_DMA_CONTROL_LLP         (BIT(2))
+#define PCI_EPC_DMA_CONTROL_LIE         (BIT(3))
+#define PCI_EPC_DMA_CONTROL_RIE         (BIT(4))
+#define PCI_EPC_DMA_CONTROL_CS          (BIT(5) | BIT(6))
+#define PCI_EPC_DMA_CONTROL_CCS         (BIT(8))
+#define PCI_EPC_DMA_CONTROL_LLE         (BIT(9))
+#define PCI_EPC_DMA_CONTROL_FUNC        (BIT(12) | BIT(13) | BIT(14) | \
+					 BIT(15) | BIT(16))
+#define PCI_EPC_DMA_CONTROL_NS_DST      (BIT(23))
+#define PCI_EPC_DMA_CONTROL_NS_SRC      (BIT(24))
+#define PCI_EPC_DMA_CONTROL_RO          (BIT(25))
+#define PCI_EPC_DMA_CONTROL_TC          (BIT(27) | BIT(28) | BIT(29))
+#define PCI_EPC_DMA_CONTROL_AT          (BIT(30) | BIT(31))
+
+#define PCI_EPC_DMA_CONTROL_EOL         (PCI_EPC_DMA_CONTROL_TCB | \
+					 PCI_EPC_DMA_CONTROL_LLP)
+
+#define PCI_EPC_DMA_CONTROL_LIST        (PCI_EPC_DMA_CONTROL_CB | \
+					 PCI_EPC_DMA_CONTROL_EOL| \
+					 PCI_EPC_DMA_CONTROL_CCS | \
+					 PCI_EPC_DMA_CONTROL_LLE)
+
+/**
  * struct pci_epc_ops - set of function pointers for performing EPC 
operations
  * @write_header: ops to populate configuration space header
  * @set_bar: ops to configure the BAR
@@ -38,6 +77,8 @@ enum pci_epc_irq_type {
  * @raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
  * @start: ops to start the PCI link
  * @stop: ops to stop the PCI link
+ * @dma_read: ops to read remote memory into local memory by DMA
+ * @dma_write: ops to write local memory to remote memory by DMA
  * @owner: the module owner containing the ops
  */
 struct pci_epc_ops {
@@ -61,6 +102,8 @@ struct pci_epc_ops {
 	void	(*stop)(struct pci_epc *epc);
 	const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
 						       u8 func_no);
+	int	(*dma_read)(struct pci_epc *epc, struct pci_epc_dma *dma);
+	int	(*dma_write)(struct pci_epc *epc, struct pci_epc_dma *dma);
 	struct module *owner;
 };
 
@@ -152,6 +195,8 @@ void pci_epc_destroy(struct pci_epc *epc);
 int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf);
 void pci_epc_linkup(struct pci_epc *epc);
 void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf);
+int pci_epc_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma);
+int pci_epc_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma);
 int pci_epc_write_header(struct pci_epc *epc, u8 func_no,
 			 struct pci_epf_header *hdr);
 int pci_epc_set_bar(struct pci_epc *epc, u8 func_no,
diff --git a/include/uapi/linux/pcitest.h b/include/uapi/linux/pcitest.h
index cbf422e56696..505f4a3811c2 100644
--- a/include/uapi/linux/pcitest.h
+++ b/include/uapi/linux/pcitest.h
@@ -10,11 +10,18 @@
 #ifndef __UAPI_LINUX_PCITEST_H
 #define __UAPI_LINUX_PCITEST_H
 
+struct pcitest_dma {
+	size_t	size;
+	bool	list;
+};
+
 #define PCITEST_BAR		_IO('P', 0x1)
 #define PCITEST_LEGACY_IRQ	_IO('P', 0x2)
 #define PCITEST_MSI		_IOW('P', 0x3, int)
 #define PCITEST_WRITE		_IOW('P', 0x4, unsigned long)
+#define PCITEST_WRITE_DMA	_IOW('P', 0x4, struct pcitest_dma)
 #define PCITEST_READ		_IOW('P', 0x5, unsigned long)
+#define PCITEST_READ_DMA	_IOW('P', 0x5, struct pcitest_dma)
 #define PCITEST_COPY		_IOW('P', 0x6, unsigned long)
 #define PCITEST_MSIX		_IOW('P', 0x7, int)
 #define PCITEST_SET_IRQTYPE	_IOW('P', 0x8, int)
diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
index 6f1303104d84..66cd19acf18c 100644
--- a/tools/pci/pcitest.c
+++ b/tools/pci/pcitest.c
@@ -44,11 +44,14 @@ struct pci_test {
 	bool		read;
 	bool		write;
 	bool		copy;
+	bool		dma;
+	bool		dma_list;
 	unsigned long	size;
 };
 
 static int run_test(struct pci_test *test)
 {
+	struct pcitest_dma dma;
 	int ret = -EINVAL;
 	int fd;
 
@@ -113,7 +116,13 @@ static int run_test(struct pci_test *test)
 	}
 
 	if (test->write) {
-		ret = ioctl(fd, PCITEST_WRITE, test->size);
+		if (test->dma) {
+			dma.size = test->size;
+			dma.list = test->dma_list;
+			ret = ioctl(fd, PCITEST_WRITE_DMA, &dma);
+		} else {
+			ret = ioctl(fd, PCITEST_WRITE, test->size);
+		}
 		fprintf(stdout, "WRITE (%7ld bytes):\t\t", test->size);
 		if (ret < 0)
 			fprintf(stdout, "TEST FAILED\n");
@@ -122,7 +131,13 @@ static int run_test(struct pci_test *test)
 	}
 
 	if (test->read) {
-		ret = ioctl(fd, PCITEST_READ, test->size);
+		if (test->dma) {
+			dma.size = test->size;
+			dma.list = test->dma_list;
+			ret = ioctl(fd, PCITEST_READ_DMA, &dma);
+		} else {
+			ret = ioctl(fd, PCITEST_READ, test->size);
+		}
 		fprintf(stdout, "READ (%7ld bytes):\t\t", test->size);
 		if (ret < 0)
 			fprintf(stdout, "TEST FAILED\n");
@@ -163,7 +178,7 @@ int main(int argc, char **argv)
 	/* set default endpoint device */
 	test->device = "/dev/pci-endpoint-test.0";
 
-	while ((c = getopt(argc, argv, "D:b:m:x:i:Ilhrwcs:")) != EOF)
+	while ((c = getopt(argc, argv, "D:b:m:x:i:IlhrwcdLs:")) != EOF)
 	switch (c) {
 	case 'D':
 		test->device = optarg;
@@ -204,6 +219,12 @@ int main(int argc, char **argv)
 	case 'c':
 		test->copy = true;
 		continue;
+	case 'd':
+		test->dma = true;
+		continue;
+	case 'L':
+		test->dma_list = true;
+		continue;
 	case 's':
 		test->size = strtoul(optarg, NULL, 0);
 		continue;
@@ -223,6 +244,8 @@ int main(int argc, char **argv)
 			"\t-r			Read buffer test\n"
 			"\t-w			Write buffer test\n"
 			"\t-c			Copy buffer test\n"
+			"\t-d			DMA mode for read or write test\n"
+			"\t-L			Linked-List DMA flag for DMA mode\n"
 			"\t-s <size>		Size of buffer {default: 100KB}\n"
 			"\t-h			Print this help message\n",
 			argv[0]);

Alan Mikhak May 24, 2019, 7:42 p.m. UTC | #2

On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
<[e-mail]> wrote:
>
> Hi Alan,
>
> This patch implementation is very HW implementation dependent and
> requires the DMA to exposed through PCIe BARs, which aren't always the
> case. Besides, you are defining some control bits on
> include/linux/pci-epc.h that may not have any meaning to other types of
> DMA.
>
> I don't think this was what Kishon had in mind when he developed the
> pcitest, but let see what Kishon was to say about it.
>
> I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
> and which I submitted some days ago.
> By having a DMA driver which implemented using DMAengine API, means the
> pcitest can use the DMAengine client API, which will be completely
> generic to any other DMA implementation.
>
> My DMA driver for DWC PCI was done thinking on my use case, which is was
> interacting from the Root Complex side with DMA IP implemented on the
> Endpoint, which was exposed through PCI BARs. However, I think it would
> be possible to reuse the same core code and instead of using the PCI-glue
> to adapt it and be used easily on the Endpoint side and to be triggered
> there.
>
> Gustavo

Hi Gustavo,

I saw your patches for the DMA driver for DWC PCI which added the EDDA
driver to pci_endpoint_test.c on the host side. This suggested to me
that the user would invoke pcitest on the host side to exercise your
driver on the endpoint.

However, I didn't see any changes to pci-epf-test.c to modify the
pci_epf_test_write() and pci_epf_test_read() functions to use DMA
instead of memcpy_toio() and memcpy_fromio(), respectively.

I have four separate DMA requirements in mind as motivation for this patch.

The following two requirements are implemented by this patch:
Local DMA write or read transfer initiated by endpoint under user
command from the host between system and endpoint memory buffers using
the endpoint DMA controller with a local interrupt.
1) single block
2) linked-list mode

This patch anticipates two more requirements yet to be implemented in
future patches:
Remote DMA write or read transfer initiated by host between system and
endpoint memory buffers using the endpoint DMA controller with a local
interrupt to the endpoint processor and a remote interrupt to the host
process.
1) single block
2) linked-list mode

The descriptor format defined in pci-epc.h allows for pci-epf-test.c
to be expanded over time to initiate more elaborate DMA tests to
exercise other endpoint DMA scenarios.

The following is a sample output of the pcitest usage for exercising
DMA operations on the endpoint:

$ pcitest -r -d
READ ( 102400 bytes): OKAY
$ pcitest -w -d
WRITE ( 102400 bytes): OKAY
$ pcitest -w -d -L
WRITE ( 102400 bytes): OKAY
$ pcitest -r -d -L
READ ( 102400 bytes): OKAY

The above are executed using DMA operations as opposed to the
following which use memcpy_toio() and memcpy_fromio().

$ pcitest -r
READ ( 102400 bytes): OKAY
$ pcitest -w
WRITE ( 102400 bytes): OKAY

Regards,
Alan Mikhak



>
> -----Original Message-----
> From: Alan Mikhak <[e-mail]>
>
> Sent: 23 de maio de 2019 23:24
> To: [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> Cc: Alan Mikhak
> <[e-mail]>
> Subject: [PATCH] PCI: endpoint: Add DMA to Linux
> PCI EP Framework
>
> This patch depends on patch the following patches:
> [PATCH v2 1/2] tools: PCI: Fix broken pcitest compilation
> [PATCH v2 2/2] tools: PCI: Fix compiler warning in pcitest
>
> The Linux PCI Endpoint Framework currently does not support initiation of
> DMA read and write operations. This patch extends the Linux PCI Endpoint
> Framework by adding support for the user of pcitest to inititate DMA
> read and write operations via PCIe endpoint controller drivers. This
> patch
> provides the means but leaves it up to individual PCIe endpoint
> controller
> drivers to implement DMA support, if desired.
>
> This patch provides support for the pcitest user to instruct the endpoint
> to initiate local DMA transfers consisting of single or linked-list
> blocks
> into endpoint buffers using the endpoint DMA controller. It anticipates
> that future patches would add support for the pcitest user to instruct
> the endpoint to participate in remote DMA transfers initiated from the
> root complex into endpoint buffers using the endpoint DMA controller.
>
> This patch depends on the first two patches in its patchset to resolve
> a pre-existing pcitest compilation error.
>
> * Add -d flag to pcitest command line options so user can specify
> that a read or write command should execute using local DMA to be
> initiated by endpoint.
>
> * Add -L flag to pcitest command line options so user can specify
> that DMA operation should execute in linked-list mode.
>
> * Add struct pcitest_dma for pcitest to communicate DMA options
> from host userspace to pci_endpoint_test driver in host kernel
> via two new ioctls PCITEST_WRITE_DMA and PCITEST_READ_DMA.
>
> * Add command flags so pci_endpoint_test driver running on host
> can communicate DMA read and write options across the PCI bus
> to pci-epf-test driver running on endpoint.
>
> * Add struct pci_epc_dma so pci-epf-test driver can create DMA
> read and write descriptors for single or linked-list DMA operations
> and pass such descriptors to pci-epc-core via new functions
> pci_epc_dma_read() and pci_epc_dma_write().
>
> * Add four new functions in pci-epf-test driver to implement
> new DMA read and write tests by initiating local DMA transfers
> in linked-list and single modes via PCIe endpoint controller
> drivers: pci_epf_test_read_dma(), pci_epf_test_read_dma_list(),
> pci_epf_test_write_dma(), and pci_epf_test_write_dma_list().
>
> * Add dma_read and dma_write functions to struct pci_epc_ops
> so pci_epc_dma_read() and pci_epc_dma_write() functions can
> pass DMA descriptors down the stack to pcie-designware-ep layer.
>
> * Add dma_read and dma_write functions to struct dw_pcie_ep_ops
> so pcie-designware-ep layer can communicate DMA descriptors down
> the stack to vendor PCIe endpoint controller drivers.
>
> * Add dma_base pointer to struct dw_pcie for vendor PCIe endpoint
> controller driver to set if it implements DMA operations.
>
> * Add two common pcie-designware functions dw_pcie_writel_dma()
> and dw_pcie_readl_dma() for use by vendor PCIe endpoint
> controllers to access DMA registers via the dma_base pointer.
>
> Signed-off-by: Alan Mikhak <[e-mail]>
> ---
>
> drivers/misc/pci_endpoint_test.c                |  72 +++++++-
>  drivers/pci/controller/dwc/pcie-designware-ep.c |  22 +++
>  drivers/pci/controller/dwc/pcie-designware.h    |  13 ++
>  drivers/pci/endpoint/functions/pci-epf-test.c   | 211
> +++++++++++++++++++++++-
>  drivers/pci/endpoint/pci-epc-core.c             |  46 ++++++
>  include/linux/pci-epc.h                         |  45 +++++
>  include/uapi/linux/pcitest.h                    |   7 +
>  tools/pci/pcitest.c                             |  29 +++-
>  8 files changed, 432 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/misc/pci_endpoint_test.c
> b/drivers/misc/pci_endpoint_test.c
> index 7b015f2a1c6f..63b86d81a6b5 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -34,6 +34,7 @@
>  #include <linux/pci_regs.h>
>
>  #include <uapi/linux/pcitest.h>
> +#include <linux/uaccess.h>
>
>  #define DRV_MODULE_NAME                                "pci-endpoint-test"
>
> @@ -51,6 +52,25 @@
>  #define COMMAND_READ                           BIT(3)
>  #define COMMAND_WRITE                          BIT(4)
>  #define COMMAND_COPY                           BIT(5)
> +#define COMMAND_FLAG2                          BIT(30)
> +#define COMMAND_FLAG1                          BIT(31)
> +
> +#define COMMAND_FLAGS                          (COMMAND_FLAG1 | \
> +                                                COMMAND_FLAG2)
> +
> +#define COMMAND_FLAG_NONE                      0
> +#define COMMAND_FLAG_DMA                       COMMAND_FLAG1
> +#define COMMAND_FLAG_DMA_LIST                  COMMAND_FLAG2
> +
> +#define COMMAND_READ_DMA                       (COMMAND_READ | \
> +                                                COMMAND_FLAG_DMA)
> +#define COMMAND_READ_DMA_LIST                  (COMMAND_READ_DMA | \
> +                                                COMMAND_FLAG_DMA_LIST)
> +
> +#define COMMAND_WRITE_DMA                      (COMMAND_WRITE | \
> +                                                COMMAND_FLAG_DMA)
> +#define COMMAND_WRITE_DMA_LIST                 (COMMAND_WRITE_DMA | \
> +                                                COMMAND_FLAG_DMA_LIST)
>
>  #define PCI_ENDPOINT_TEST_STATUS               0x8
>  #define STATUS_READ_SUCCESS                    BIT(0)
> @@ -425,7 +445,9 @@ static bool pci_endpoint_test_copy(struct
> pci_endpoint_test *test, size_t size)
>         return ret;
>  }
>
> -static bool pci_endpoint_test_write(struct pci_endpoint_test *test,
> size_t size)
> +static bool pci_endpoint_test_write(struct pci_endpoint_test *test,
> +                                   size_t size,
> +                                   u32 flags)
>  {
>         bool ret = false;
>         u32 reg;
> @@ -480,7 +502,7 @@ static bool pci_endpoint_test_write(struct
> pci_endpoint_test *test, size_t size)
>         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
>         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
>         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> -                                COMMAND_READ);
> +                                COMMAND_READ | flags);
>
>         wait_for_completion(&test->irq_raised);
>
> @@ -494,7 +516,24 @@ static bool pci_endpoint_test_write(struct
> pci_endpoint_test *test, size_t size)
>         return ret;
>  }
>
> -static bool pci_endpoint_test_read(struct pci_endpoint_test *test,
> size_t size)
> +static bool pci_endpoint_test_write_dma(struct pci_endpoint_test *test,
> +                                       unsigned long arg)
> +{
> +       u32 flags = COMMAND_FLAG_DMA;
> +       struct pcitest_dma dma;
> +
> +       if (copy_from_user(&dma, (void *)arg, sizeof(struct pcitest_dma)))
> +               return -EACCES;
> +
> +       if (dma.list)
> +               flags |= COMMAND_FLAG_DMA_LIST;
> +
> +       return pci_endpoint_test_write(test, dma.size, flags);
> +}
> +
> +static bool pci_endpoint_test_read(struct pci_endpoint_test *test,
> +                                  size_t size,
> +                                  u32 flags)
>  {
>         bool ret = false;
>         void *addr;
> @@ -542,7 +581,7 @@ static bool pci_endpoint_test_read(struct
> pci_endpoint_test *test, size_t size)
>         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
>         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
>         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> -                                COMMAND_WRITE);
> +                                COMMAND_WRITE | flags);
>
>         wait_for_completion(&test->irq_raised);
>
> @@ -555,6 +594,21 @@ static bool pci_endpoint_test_read(struct
> pci_endpoint_test *test, size_t size)
>         return ret;
>  }
>
> +static bool pci_endpoint_test_read_dma(struct pci_endpoint_test *test,
> +                                      unsigned long arg)
> +{
> +       u32 flags = COMMAND_FLAG_DMA;
> +       struct pcitest_dma dma;
> +
> +       if (copy_from_user(&dma, (void *)arg, sizeof(struct pcitest_dma)))
> +               return -EACCES;
> +
> +       if (dma.list)
> +               flags |= COMMAND_FLAG_DMA_LIST;
> +
> +       return pci_endpoint_test_read(test, dma.size, flags);
> +}
> +
>  static bool pci_endpoint_test_set_irq(struct pci_endpoint_test *test,
>                                       int req_irq_type)
>  {
> @@ -612,11 +666,17 @@ static long pci_endpoint_test_ioctl(struct file
> *file, unsigned int cmd,
>         case PCITEST_MSIX:
>                 ret = pci_endpoint_test_msi_irq(test, arg, cmd == PCITEST_MSIX);
>                 break;
> +       case PCITEST_WRITE_DMA:
> +               ret = pci_endpoint_test_write_dma(test, arg);
> +               break;
> +       case PCITEST_READ_DMA:
> +               ret = pci_endpoint_test_read_dma(test, arg);
> +               break;
>         case PCITEST_WRITE:
> -               ret = pci_endpoint_test_write(test, arg);
> +               ret = pci_endpoint_test_write(test, arg, COMMAND_FLAG_NONE);
>                 break;
>         case PCITEST_READ:
> -               ret = pci_endpoint_test_read(test, arg);
> +               ret = pci_endpoint_test_read(test, arg, COMMAND_FLAG_NONE);
>                 break;
>         case PCITEST_COPY:
>                 ret = pci_endpoint_test_copy(test, arg);
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 2bf5a35c0570..7e25c0f5edf1 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -366,6 +366,26 @@ dw_pcie_ep_get_features(struct pci_epc *epc, u8
> func_no)
>         return ep->ops->get_features(ep);
>  }
>
> +static int dw_pcie_ep_dma_read(struct pci_epc *epc, struct pci_epc_dma
> *dma)
> +{
> +       struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> +
> +       if (!ep->ops->dma_read)
> +               return -EINVAL;
> +
> +       return ep->ops->dma_read(ep, dma);
> +}
> +
> +static int dw_pcie_ep_dma_write(struct pci_epc *epc, struct pci_epc_dma
> *dma)
> +{
> +       struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> +
> +       if (!ep->ops->dma_write)
> +               return -EINVAL;
> +
> +       return ep->ops->dma_write(ep, dma);
> +}
> +
>  static const struct pci_epc_ops epc_ops = {
>         .write_header           = dw_pcie_ep_write_header,
>         .set_bar                = dw_pcie_ep_set_bar,
> @@ -380,6 +400,8 @@ static const struct pci_epc_ops epc_ops = {
>         .start                  = dw_pcie_ep_start,
>         .stop                   = dw_pcie_ep_stop,
>         .get_features           = dw_pcie_ep_get_features,
> +       .dma_read               = dw_pcie_ep_dma_read,
> +       .dma_write              = dw_pcie_ep_dma_write
>  };
>
>  int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> b/drivers/pci/controller/dwc/pcie-designware.h
> index b8993f2b78df..11d44ec8acc7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -197,6 +197,8 @@ struct dw_pcie_ep_ops {
>         int     (*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
>                              enum pci_epc_irq_type type, u16 interrupt_num);
>         const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
> +       int     (*dma_read)(struct dw_pcie_ep *ep, struct pci_epc_dma *dma);
> +       int     (*dma_write)(struct dw_pcie_ep *ep, struct pci_epc_dma *dma);
>  };
>
>  struct dw_pcie_ep {
> @@ -238,6 +240,7 @@ struct dw_pcie {
>         void __iomem            *dbi_base2;
>         /* Used when iatu_unroll_enabled is true */
>         void __iomem            *atu_base;
> +       void __iomem            *dma_base;
>         u32                     num_viewport;
>         u8                      iatu_unroll_enabled;
>         struct pcie_port        pp;
> @@ -323,6 +326,16 @@ static inline u32 dw_pcie_readl_atu(struct dw_pcie
> *pci, u32 reg)
>         return __dw_pcie_read_dbi(pci, pci->atu_base, reg, 0x4);
>  }
>
> +static inline void dw_pcie_writel_dma(struct dw_pcie *pci, u32 reg, u32
> val)
> +{
> +       __dw_pcie_write_dbi(pci, pci->dma_base, reg, 0x4, val);
> +}
> +
> +static inline u32 dw_pcie_readl_dma(struct dw_pcie *pci, u32 reg)
> +{
> +       return __dw_pcie_read_dbi(pci, pci->dma_base, reg, 0x4);
> +}
> +
>  static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
>  {
>         u32 reg;
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c
> b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 27806987e93b..3910073712e9 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -28,6 +28,25 @@
>  #define COMMAND_READ                   BIT(3)
>  #define COMMAND_WRITE                  BIT(4)
>  #define COMMAND_COPY                   BIT(5)
> +#define COMMAND_FLAG2                  BIT(30)
> +#define COMMAND_FLAG1                  BIT(31)
> +
> +#define COMMAND_FLAGS                  (COMMAND_FLAG1 | \
> +                                        COMMAND_FLAG2)
> +
> +#define COMMAND_FLAG_NONE              0
> +#define COMMAND_FLAG_DMA               COMMAND_FLAG1
> +#define COMMAND_FLAG_DMA_LIST          COMMAND_FLAG2
> +
> +#define COMMAND_READ_DMA               (COMMAND_READ | \
> +                                        COMMAND_FLAG_DMA)
> +#define COMMAND_READ_DMA_LIST          (COMMAND_READ_DMA | \
> +                                        COMMAND_FLAG_DMA_LIST)
> +
> +#define COMMAND_WRITE_DMA              (COMMAND_WRITE | \
> +                                        COMMAND_FLAG_DMA)
> +#define COMMAND_WRITE_DMA_LIST         (COMMAND_WRITE_DMA | \
> +                                        COMMAND_FLAG_DMA_LIST)
>
>  #define STATUS_READ_SUCCESS            BIT(0)
>  #define STATUS_READ_FAIL               BIT(1)
> @@ -187,6 +206,93 @@ static int pci_epf_test_read(struct pci_epf_test
> *epf_test)
>         return ret;
>  }
>
> +static int pci_epf_test_read_dma(struct pci_epf_test *epf_test)
> +{
> +       int ret = -ENOMEM;
> +       struct pci_epf *epf = epf_test->epf;
> +       struct device *dev = &epf->dev;
> +       struct pci_epc *epc = epf->epc;
> +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> +       struct pci_epc_dma dma;
> +       u32 crc32;
> +       void *buf;
> +
> +       buf = kzalloc(reg->size, GFP_KERNEL);
> +       if (buf) {
> +               dma.control = PCI_EPC_DMA_CONTROL_LIE;
> +               dma.size = reg->size;
> +               dma.sar = reg->src_addr;
> +               dma.dar = virt_to_phys(buf);
> +
> +               ret = pci_epc_dma_read(epc, &dma);
> +               if (ret) {
> +                       dev_err(dev, "pci_epc_dma_read %d\n", ret);
> +               } else {
> +                       crc32 = crc32_le(~0, buf, reg->size);
> +                       if (crc32 != reg->checksum)
> +                               ret = -EIO;
> +               }
> +
> +               kfree(buf);
> +       }
> +
> +       return ret;
> +}
> +
> +static int pci_epf_test_read_dma_list(struct pci_epf_test *epf_test)
> +{
> +       int ret = -ENOMEM;
> +       struct pci_epf *epf = epf_test->epf;
> +       struct device *dev = &epf->dev;
> +       struct pci_epc *epc = epf->epc;
> +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> +       struct pci_epc_dma *dma;
> +       u32 crc32;
> +       void *buf;
> +
> +       dma = kcalloc(3, sizeof(*dma), GFP_KERNEL);
> +       if (dma) {
> +               buf = kzalloc(reg->size, GFP_KERNEL);
> +               if (buf) {
> +                       int half_size = reg->size >> 1;
> +                       phys_addr_t phys_addr = virt_to_phys(buf);
> +
> +                       dma[0].control = PCI_EPC_DMA_CONTROL_CB;
> +                       dma[0].size = half_size ? half_size : 1;
> +                       dma[0].sar = reg->src_addr;
> +                       dma[0].dar = phys_addr;
> +
> +                       dma[1].control = PCI_EPC_DMA_CONTROL_CB |
> +                                        PCI_EPC_DMA_CONTROL_LIE;
> +                       dma[1].size = reg->size - half_size;
> +                       dma[1].sar = reg->src_addr + half_size;
> +                       dma[1].dar = phys_addr + half_size;
> +
> +                       dma[2].control = PCI_EPC_DMA_CONTROL_EOL;
> +                       dma[2].size = 0;
> +                       dma[2].sar = virt_to_phys(dma);
> +                       dma[2].dar = 0;
> +
> +                       ret = pci_epc_dma_read(epc, dma);
> +                       if (ret) {
> +                               dev_err(dev, "pci_epc_dma_read %d\n", ret);
> +                       } else {
> +                               crc32 = crc32_le(~0, buf, reg->size);
> +                               if (crc32 != reg->checksum)
> +                                       ret = -EIO;
> +                       }
> +
> +                       kfree(buf);
> +               }
> +
> +               kfree(dma);
> +       }
> +
> +       return ret;
> +}
> +
>  static int pci_epf_test_write(struct pci_epf_test *epf_test)
>  {
>         int ret;
> @@ -244,6 +350,87 @@ static int pci_epf_test_write(struct pci_epf_test
> *epf_test)
>         return ret;
>  }
>
> +static int pci_epf_test_write_dma(struct pci_epf_test *epf_test)
> +{
> +       int ret = -ENOMEM;
> +       struct pci_epf *epf = epf_test->epf;
> +       struct device *dev = &epf->dev;
> +       struct pci_epc *epc = epf->epc;
> +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> +       struct pci_epc_dma dma;
> +       void *buf;
> +
> +       buf = kzalloc(reg->size, GFP_KERNEL);
> +       if (buf) {
> +               get_random_bytes(buf, reg->size);
> +               reg->checksum = crc32_le(~0, buf, reg->size);
> +
> +               dma.control = PCI_EPC_DMA_CONTROL_LIE;
> +               dma.size = reg->size;
> +               dma.sar = virt_to_phys(buf);
> +               dma.dar = reg->dst_addr;
> +
> +               ret = pci_epc_dma_write(epc, &dma);
> +               if (ret)
> +                       dev_err(dev, "pci_epc_dma_write %d\n", ret);
> +
> +               kfree(buf);
> +       }
> +
> +       return ret;
> +}
> +
> +static int pci_epf_test_write_dma_list(struct pci_epf_test *epf_test)
> +{
> +       int ret = -ENOMEM;
> +       struct pci_epf *epf = epf_test->epf;
> +       struct device *dev = &epf->dev;
> +       struct pci_epc *epc = epf->epc;
> +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> +       struct pci_epc_dma *dma;
> +       void *buf;
> +
> +       dma = kcalloc(3, sizeof(*dma), GFP_KERNEL);
> +       if (dma) {
> +               buf = kzalloc(reg->size, GFP_KERNEL);
> +               if (buf) {
> +                       int half_size = reg->size >> 1;
> +                       phys_addr_t phys_addr = virt_to_phys(buf);
> +
> +                       get_random_bytes(buf, reg->size);
> +                       reg->checksum = crc32_le(~0, buf, reg->size);
> +
> +                       dma[0].control = PCI_EPC_DMA_CONTROL_CB;
> +                       dma[0].size = half_size ? half_size : 1;
> +                       dma[0].sar = phys_addr;
> +                       dma[0].dar = reg->dst_addr;
> +
> +                       dma[1].control = PCI_EPC_DMA_CONTROL_CB |
> +                                        PCI_EPC_DMA_CONTROL_LIE;
> +                       dma[1].size = reg->size - half_size;
> +                       dma[1].sar = phys_addr + half_size;
> +                       dma[1].dar = reg->dst_addr + half_size;
> +
> +                       dma[2].control = PCI_EPC_DMA_CONTROL_EOL;
> +                       dma[2].size = 0;
> +                       dma[2].sar = virt_to_phys(dma);
> +                       dma[2].dar = 0;
> +
> +                       ret = pci_epc_dma_write(epc, dma);
> +                       if (ret)
> +                               dev_err(dev, "pci_epc_dma_write %d\n", ret);
> +
> +                       kfree(buf);
> +               }
> +
> +               kfree(dma);
> +       }
> +
> +       return ret;
> +}
> +
>  static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test, u8
> irq_type,
>                                    u16 irq)
>  {
> @@ -303,18 +490,34 @@ static void pci_epf_test_cmd_handler(struct
> work_struct *work)
>         }
>
>         if (command & COMMAND_WRITE) {
> -               ret = pci_epf_test_write(epf_test);
> -               if (ret)
> -                       reg->status |= STATUS_WRITE_FAIL;
> +               command &= (COMMAND_WRITE | COMMAND_FLAGS);
> +               if (command == COMMAND_WRITE)
> +                       ret = pci_epf_test_write(epf_test);
> +               else if (command == COMMAND_WRITE_DMA)
> +                       ret = pci_epf_test_write_dma(epf_test);
> +               else if (command == COMMAND_WRITE_DMA_LIST)
> +                       ret = pci_epf_test_write_dma_list(epf_test);
>                 else
> +                       ret = -EINVAL;
> +               if (!ret)
>                         reg->status |= STATUS_WRITE_SUCCESS;
> +               else
> +                       reg->status |= STATUS_WRITE_FAIL;
>                 pci_epf_test_raise_irq(epf_test, reg->irq_type,
>                                        reg->irq_number);
>                 goto reset_handler;
>         }
>
>         if (command & COMMAND_READ) {
> -               ret = pci_epf_test_read(epf_test);
> +               command &= (COMMAND_READ | COMMAND_FLAGS);
> +               if (command == COMMAND_READ)
> +                       ret = pci_epf_test_read(epf_test);
> +               else if (command == COMMAND_READ_DMA)
> +                       ret = pci_epf_test_read_dma(epf_test);
> +               else if (command == COMMAND_READ_DMA_LIST)
> +                       ret = pci_epf_test_read_dma_list(epf_test);
> +               else
> +                       ret = -EINVAL;
>                 if (!ret)
>                         reg->status |= STATUS_READ_SUCCESS;
>                 else
> diff --git a/drivers/pci/endpoint/pci-epc-core.c
> b/drivers/pci/endpoint/pci-epc-core.c
> index e4712a0f249c..a57e501d4abc 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -107,6 +107,52 @@ unsigned int pci_epc_get_first_free_bar(const struct
> pci_epc_features
>  EXPORT_SYMBOL_GPL(pci_epc_get_first_free_bar);
>
>  /**
> + * pci_epc_dma_write() - DMA a block of memory to remote address
> + * @epc: the EPC device on which to perform DMA transfer
> + * @dma: DMA descriptors array
> + *
> + * Write contents of local memory to remote memory by DMA.
> + */
> +int pci_epc_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma)
> +{
> +       int ret;
> +       unsigned long flags;
> +
> +       if (IS_ERR(epc) || !epc->ops->dma_write)
> +               return -EINVAL;
> +
> +       spin_lock_irqsave(&epc->lock, flags);
> +       ret = epc->ops->dma_write(epc, dma);
> +       spin_unlock_irqrestore(&epc->lock, flags);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_dma_write);
> +
> +/**
> + * pci_epc_dma_read() - DMA a block of memory from remote address
> + * @epc: the EPC device on which to perform DMA transfer
> + * @dma: DMA descriptors array
> + *
> + * Read contents of remote memory into local memory by DMA.
> + */
> +int pci_epc_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma)
> +{
> +       int ret;
> +       unsigned long flags;
> +
> +       if (IS_ERR(epc) || !epc->ops->dma_read)
> +               return -EINVAL;
> +
> +       spin_lock_irqsave(&epc->lock, flags);
> +       ret = epc->ops->dma_read(epc, dma);
> +       spin_unlock_irqrestore(&epc->lock, flags);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_dma_read);
> +
> +/**
>   * pci_epc_get_features() - get the features supported by EPC
>   * @epc: the features supported by *this* EPC device will be returned
>   * @func_no: the features supported by the EPC device specific to the
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index f641badc2c61..d845f13d0baf 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -21,6 +21,45 @@ enum pci_epc_irq_type {
>  };
>
>  /**
> + * struct pci_epc_dma - descriptor for a DMA transfer element
> + * @control: DMA channel control bits for read or write transfer
> + * @size: size of DMA transfer element
> + * @sar: source addrees for DMA transfer element
> + * @dar: destination address for DMA transfer element
> + */
> +
> +struct pci_epc_dma {
> +       u32     control;
> +       u32     size;
> +       u64     sar;
> +       u64     dar;
> +};
> +
> +#define PCI_EPC_DMA_CONTROL_CB          (BIT(0))
> +#define PCI_EPC_DMA_CONTROL_TCB         (BIT(1))
> +#define PCI_EPC_DMA_CONTROL_LLP         (BIT(2))
> +#define PCI_EPC_DMA_CONTROL_LIE         (BIT(3))
> +#define PCI_EPC_DMA_CONTROL_RIE         (BIT(4))
> +#define PCI_EPC_DMA_CONTROL_CS          (BIT(5) | BIT(6))
> +#define PCI_EPC_DMA_CONTROL_CCS         (BIT(8))
> +#define PCI_EPC_DMA_CONTROL_LLE         (BIT(9))
> +#define PCI_EPC_DMA_CONTROL_FUNC        (BIT(12) | BIT(13) | BIT(14) | \
> +                                        BIT(15) | BIT(16))
> +#define PCI_EPC_DMA_CONTROL_NS_DST      (BIT(23))
> +#define PCI_EPC_DMA_CONTROL_NS_SRC      (BIT(24))
> +#define PCI_EPC_DMA_CONTROL_RO          (BIT(25))
> +#define PCI_EPC_DMA_CONTROL_TC          (BIT(27) | BIT(28) | BIT(29))
> +#define PCI_EPC_DMA_CONTROL_AT          (BIT(30) | BIT(31))
> +
> +#define PCI_EPC_DMA_CONTROL_EOL         (PCI_EPC_DMA_CONTROL_TCB | \
> +                                        PCI_EPC_DMA_CONTROL_LLP)
> +
> +#define PCI_EPC_DMA_CONTROL_LIST        (PCI_EPC_DMA_CONTROL_CB | \
> +                                        PCI_EPC_DMA_CONTROL_EOL| \
> +                                        PCI_EPC_DMA_CONTROL_CCS | \
> +                                        PCI_EPC_DMA_CONTROL_LLE)
> +
> +/**
>   * struct pci_epc_ops - set of function pointers for performing EPC
> operations
>   * @write_header: ops to populate configuration space header
>   * @set_bar: ops to configure the BAR
> @@ -38,6 +77,8 @@ enum pci_epc_irq_type {
>   * @raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
>   * @start: ops to start the PCI link
>   * @stop: ops to stop the PCI link
> + * @dma_read: ops to read remote memory into local memory by DMA
> + * @dma_write: ops to write local memory to remote memory by DMA
>   * @owner: the module owner containing the ops
>   */
>  struct pci_epc_ops {
> @@ -61,6 +102,8 @@ struct pci_epc_ops {
>         void    (*stop)(struct pci_epc *epc);
>         const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
>                                                        u8 func_no);
> +       int     (*dma_read)(struct pci_epc *epc, struct pci_epc_dma *dma);
> +       int     (*dma_write)(struct pci_epc *epc, struct pci_epc_dma *dma);
>         struct module *owner;
>  };
>
> @@ -152,6 +195,8 @@ void pci_epc_destroy(struct pci_epc *epc);
>  int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf);
>  void pci_epc_linkup(struct pci_epc *epc);
>  void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf);
> +int pci_epc_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma);
> +int pci_epc_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma);
>  int pci_epc_write_header(struct pci_epc *epc, u8 func_no,
>                          struct pci_epf_header *hdr);
>  int pci_epc_set_bar(struct pci_epc *epc, u8 func_no,
> diff --git a/include/uapi/linux/pcitest.h b/include/uapi/linux/pcitest.h
> index cbf422e56696..505f4a3811c2 100644
> --- a/include/uapi/linux/pcitest.h
> +++ b/include/uapi/linux/pcitest.h
> @@ -10,11 +10,18 @@
>  #ifndef __UAPI_LINUX_PCITEST_H
>  #define __UAPI_LINUX_PCITEST_H
>
> +struct pcitest_dma {
> +       size_t  size;
> +       bool    list;
> +};
> +
>  #define PCITEST_BAR            _IO('P', 0x1)
>  #define PCITEST_LEGACY_IRQ     _IO('P', 0x2)
>  #define PCITEST_MSI            _IOW('P', 0x3, int)
>  #define PCITEST_WRITE          _IOW('P', 0x4, unsigned long)
> +#define PCITEST_WRITE_DMA      _IOW('P', 0x4, struct pcitest_dma)
>  #define PCITEST_READ           _IOW('P', 0x5, unsigned long)
> +#define PCITEST_READ_DMA       _IOW('P', 0x5, struct pcitest_dma)
>  #define PCITEST_COPY           _IOW('P', 0x6, unsigned long)
>  #define PCITEST_MSIX           _IOW('P', 0x7, int)
>  #define PCITEST_SET_IRQTYPE    _IOW('P', 0x8, int)
> diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
> index 6f1303104d84..66cd19acf18c 100644
> --- a/tools/pci/pcitest.c
> +++ b/tools/pci/pcitest.c
> @@ -44,11 +44,14 @@ struct pci_test {
>         bool            read;
>         bool            write;
>         bool            copy;
> +       bool            dma;
> +       bool            dma_list;
>         unsigned long   size;
>  };
>
>  static int run_test(struct pci_test *test)
>  {
> +       struct pcitest_dma dma;
>         int ret = -EINVAL;
>         int fd;
>
> @@ -113,7 +116,13 @@ static int run_test(struct pci_test *test)
>         }
>
>         if (test->write) {
> -               ret = ioctl(fd, PCITEST_WRITE, test->size);
> +               if (test->dma) {
> +                       dma.size = test->size;
> +                       dma.list = test->dma_list;
> +                       ret = ioctl(fd, PCITEST_WRITE_DMA, &dma);
> +               } else {
> +                       ret = ioctl(fd, PCITEST_WRITE, test->size);
> +               }
>                 fprintf(stdout, "WRITE (%7ld bytes):\t\t", test->size);
>                 if (ret < 0)
>                         fprintf(stdout, "TEST FAILED\n");
> @@ -122,7 +131,13 @@ static int run_test(struct pci_test *test)
>         }
>
>         if (test->read) {
> -               ret = ioctl(fd, PCITEST_READ, test->size);
> +               if (test->dma) {
> +                       dma.size = test->size;
> +                       dma.list = test->dma_list;
> +                       ret = ioctl(fd, PCITEST_READ_DMA, &dma);
> +               } else {
> +                       ret = ioctl(fd, PCITEST_READ, test->size);
> +               }
>                 fprintf(stdout, "READ (%7ld bytes):\t\t", test->size);
>                 if (ret < 0)
>                         fprintf(stdout, "TEST FAILED\n");
> @@ -163,7 +178,7 @@ int main(int argc, char **argv)
>         /* set default endpoint device */
>         test->device = "/dev/pci-endpoint-test.0";
>
> -       while ((c = getopt(argc, argv, "D:b:m:x:i:Ilhrwcs:")) != EOF)
> +       while ((c = getopt(argc, argv, "D:b:m:x:i:IlhrwcdLs:")) != EOF)
>         switch (c) {
>         case 'D':
>                 test->device = optarg;
> @@ -204,6 +219,12 @@ int main(int argc, char **argv)
>         case 'c':
>                 test->copy = true;
>                 continue;
> +       case 'd':
> +               test->dma = true;
> +               continue;
> +       case 'L':
> +               test->dma_list = true;
> +               continue;
>         case 's':
>                 test->size = strtoul(optarg, NULL, 0);
>                 continue;
> @@ -223,6 +244,8 @@ int main(int argc, char **argv)
>                         "\t-r                   Read buffer test\n"
>                         "\t-w                   Write buffer test\n"
>                         "\t-c                   Copy buffer test\n"
> +                       "\t-d                   DMA mode for read or write test\n"
> +                       "\t-L                   Linked-List DMA flag for DMA mode\n"
>                         "\t-s <size>            Size of buffer {default: 100KB}\n"
>                         "\t-h                   Print this help message\n",
>                         argv[0]);
> --
> 2.7.4
>

Gustavo Pimentel May 27, 2019, 9:09 a.m. UTC | #3

On Fri, May 24, 2019 at 20:42:43, Alan Mikhak <[e-mail]> 
wrote:

Hi Alan,

> On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
> <[e-mail]> wrote:
> >
> > Hi Alan,
> >
> > This patch implementation is very HW implementation dependent and
> > requires the DMA to exposed through PCIe BARs, which aren't always the
> > case. Besides, you are defining some control bits on
> > include/linux/pci-epc.h that may not have any meaning to other types of
> > DMA.
> >
> > I don't think this was what Kishon had in mind when he developed the
> > pcitest, but let see what Kishon was to say about it.
> >
> > I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
> > and which I submitted some days ago.
> > By having a DMA driver which implemented using DMAengine API, means the
> > pcitest can use the DMAengine client API, which will be completely
> > generic to any other DMA implementation.
> >
> > My DMA driver for DWC PCI was done thinking on my use case, which is was
> > interacting from the Root Complex side with DMA IP implemented on the
> > Endpoint, which was exposed through PCI BARs. However, I think it would
> > be possible to reuse the same core code and instead of using the PCI-glue
> > to adapt it and be used easily on the Endpoint side and to be triggered
> > there.
> >
> > Gustavo
> 
> Hi Gustavo,
> 
> I saw your patches for the DMA driver for DWC PCI which added the EDDA
> driver to pci_endpoint_test.c on the host side. This suggested to me
> that the user would invoke pcitest on the host side to exercise your
> driver on the endpoint.
> 
> However, I didn't see any changes to pci-epf-test.c to modify the
> pci_epf_test_write() and pci_epf_test_read() functions to use DMA
> instead of memcpy_toio() and memcpy_fromio(), respectively.

I didn't add the DMA engine client API to the pcitest yet. I was waiting 
for the eDMA driver to be integrated on Kernel before doing any on 
pcitest.

> 
> I have four separate DMA requirements in mind as motivation for this patch.
> 
> The following two requirements are implemented by this patch:
> Local DMA write or read transfer initiated by endpoint under user
> command from the host between system and endpoint memory buffers using
> the endpoint DMA controller with a local interrupt.
> 1) single block
> 2) linked-list mode

However, in most cases, you will not have the DMA block registers or 
linked list memory accessible or defined on PCI BARs and based on I have 
seen in your patch, the whole patch is based on that premise. The current 
implementation is not generic and highly dependent on IP and design.
I strongly believe with some slight modifications it would be possible to 
run the DMA engine controller driver with a platform glue-logic on the 
Endpoint side, which could interact with pci_epf_test driver. This way 
the pcitest would be always generic to all products.

> 
> This patch anticipates two more requirements yet to be implemented in
> future patches:
> Remote DMA write or read transfer initiated by host between system and
> endpoint memory buffers using the endpoint DMA controller with a local
> interrupt to the endpoint processor and a remote interrupt to the host
> process.
> 1) single block
> 2) linked-list mode

That's what the submitted patch driver does. I didn't develop the 
interaction with to pcitest, however, while developing DW eDMA IP driver, 
I've sent in some an RFC patch containing dw-edma-test driver that 
stimulates the DW eDMA.

> 
> The descriptor format defined in pci-epc.h allows for pci-epf-test.c
> to be expanded over time to initiate more elaborate DMA tests to
> exercise other endpoint DMA scenarios.

Yes, but that's not the original issue been discussed here. But let's see 
what Kishon as to say, In the end, he is the maintainer of this tool.

Regards,
Gustavo

> 
> The following is a sample output of the pcitest usage for exercising
> DMA operations on the endpoint:
> 
> $ pcitest -r -d
> READ ( 102400 bytes): OKAY
> $ pcitest -w -d
> WRITE ( 102400 bytes): OKAY
> $ pcitest -w -d -L
> WRITE ( 102400 bytes): OKAY
> $ pcitest -r -d -L
> READ ( 102400 bytes): OKAY
> 
> The above are executed using DMA operations as opposed to the
> following which use memcpy_toio() and memcpy_fromio().
> 
> $ pcitest -r
> READ ( 102400 bytes): OKAY
> $ pcitest -w
> WRITE ( 102400 bytes): OKAY
> 
> Regards,
> Alan Mikhak
> 
> 
> 
> >
> > -----Original Message-----
> > From: Alan Mikhak <[e-mail]>
> >
> > Sent: 23 de maio de 2019 23:24
> > To: [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]
> > Cc: Alan Mikhak
> > <[e-mail]>
> > Subject: [PATCH] PCI: endpoint: Add DMA to Linux
> > PCI EP Framework
> >
> > This patch depends on patch the following patches:
> > [PATCH v2 1/2] tools: PCI: Fix broken pcitest compilation
> > [PATCH v2 2/2] tools: PCI: Fix compiler warning in pcitest
> >
> > The Linux PCI Endpoint Framework currently does not support initiation of
> > DMA read and write operations. This patch extends the Linux PCI Endpoint
> > Framework by adding support for the user of pcitest to inititate DMA
> > read and write operations via PCIe endpoint controller drivers. This
> > patch
> > provides the means but leaves it up to individual PCIe endpoint
> > controller
> > drivers to implement DMA support, if desired.
> >
> > This patch provides support for the pcitest user to instruct the endpoint
> > to initiate local DMA transfers consisting of single or linked-list
> > blocks
> > into endpoint buffers using the endpoint DMA controller. It anticipates
> > that future patches would add support for the pcitest user to instruct
> > the endpoint to participate in remote DMA transfers initiated from the
> > root complex into endpoint buffers using the endpoint DMA controller.
> >
> > This patch depends on the first two patches in its patchset to resolve
> > a pre-existing pcitest compilation error.
> >
> > * Add -d flag to pcitest command line options so user can specify
> > that a read or write command should execute using local DMA to be
> > initiated by endpoint.
> >
> > * Add -L flag to pcitest command line options so user can specify
> > that DMA operation should execute in linked-list mode.
> >
> > * Add struct pcitest_dma for pcitest to communicate DMA options
> > from host userspace to pci_endpoint_test driver in host kernel
> > via two new ioctls PCITEST_WRITE_DMA and PCITEST_READ_DMA.
> >
> > * Add command flags so pci_endpoint_test driver running on host
> > can communicate DMA read and write options across the PCI bus
> > to pci-epf-test driver running on endpoint.
> >
> > * Add struct pci_epc_dma so pci-epf-test driver can create DMA
> > read and write descriptors for single or linked-list DMA operations
> > and pass such descriptors to pci-epc-core via new functions
> > pci_epc_dma_read() and pci_epc_dma_write().
> >
> > * Add four new functions in pci-epf-test driver to implement
> > new DMA read and write tests by initiating local DMA transfers
> > in linked-list and single modes via PCIe endpoint controller
> > drivers: pci_epf_test_read_dma(), pci_epf_test_read_dma_list(),
> > pci_epf_test_write_dma(), and pci_epf_test_write_dma_list().
> >
> > * Add dma_read and dma_write functions to struct pci_epc_ops
> > so pci_epc_dma_read() and pci_epc_dma_write() functions can
> > pass DMA descriptors down the stack to pcie-designware-ep layer.
> >
> > * Add dma_read and dma_write functions to struct dw_pcie_ep_ops
> > so pcie-designware-ep layer can communicate DMA descriptors down
> > the stack to vendor PCIe endpoint controller drivers.
> >
> > * Add dma_base pointer to struct dw_pcie for vendor PCIe endpoint
> > controller driver to set if it implements DMA operations.
> >
> > * Add two common pcie-designware functions dw_pcie_writel_dma()
> > and dw_pcie_readl_dma() for use by vendor PCIe endpoint
> > controllers to access DMA registers via the dma_base pointer.
> >
> > Signed-off-by: Alan Mikhak <[e-mail]>
> > ---
> >
> > drivers/misc/pci_endpoint_test.c                |  72 +++++++-
> >  drivers/pci/controller/dwc/pcie-designware-ep.c |  22 +++
> >  drivers/pci/controller/dwc/pcie-designware.h    |  13 ++
> >  drivers/pci/endpoint/functions/pci-epf-test.c   | 211
> > +++++++++++++++++++++++-
> >  drivers/pci/endpoint/pci-epc-core.c             |  46 ++++++
> >  include/linux/pci-epc.h                         |  45 +++++
> >  include/uapi/linux/pcitest.h                    |   7 +
> >  tools/pci/pcitest.c                             |  29 +++-
> >  8 files changed, 432 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/misc/pci_endpoint_test.c
> > b/drivers/misc/pci_endpoint_test.c
> > index 7b015f2a1c6f..63b86d81a6b5 100644
> > --- a/drivers/misc/pci_endpoint_test.c
> > +++ b/drivers/misc/pci_endpoint_test.c
> > @@ -34,6 +34,7 @@
> >  #include <linux/pci_regs.h>
> >
> >  #include <uapi/linux/pcitest.h>
> > +#include <linux/uaccess.h>
> >
> >  #define DRV_MODULE_NAME                                "pci-endpoint-test"
> >
> > @@ -51,6 +52,25 @@
> >  #define COMMAND_READ                           BIT(3)
> >  #define COMMAND_WRITE                          BIT(4)
> >  #define COMMAND_COPY                           BIT(5)
> > +#define COMMAND_FLAG2                          BIT(30)
> > +#define COMMAND_FLAG1                          BIT(31)
> > +
> > +#define COMMAND_FLAGS                          (COMMAND_FLAG1 | \
> > +                                                COMMAND_FLAG2)
> > +
> > +#define COMMAND_FLAG_NONE                      0
> > +#define COMMAND_FLAG_DMA                       COMMAND_FLAG1
> > +#define COMMAND_FLAG_DMA_LIST                  COMMAND_FLAG2
> > +
> > +#define COMMAND_READ_DMA                       (COMMAND_READ | \
> > +                                                COMMAND_FLAG_DMA)
> > +#define COMMAND_READ_DMA_LIST                  (COMMAND_READ_DMA | \
> > +                                                COMMAND_FLAG_DMA_LIST)
> > +
> > +#define COMMAND_WRITE_DMA                      (COMMAND_WRITE | \
> > +                                                COMMAND_FLAG_DMA)
> > +#define COMMAND_WRITE_DMA_LIST                 (COMMAND_WRITE_DMA | \
> > +                                                COMMAND_FLAG_DMA_LIST)
> >
> >  #define PCI_ENDPOINT_TEST_STATUS               0x8
> >  #define STATUS_READ_SUCCESS                    BIT(0)
> > @@ -425,7 +445,9 @@ static bool pci_endpoint_test_copy(struct
> > pci_endpoint_test *test, size_t size)
> >         return ret;
> >  }
> >
> > -static bool pci_endpoint_test_write(struct pci_endpoint_test *test,
> > size_t size)
> > +static bool pci_endpoint_test_write(struct pci_endpoint_test *test,
> > +                                   size_t size,
> > +                                   u32 flags)
> >  {
> >         bool ret = false;
> >         u32 reg;
> > @@ -480,7 +502,7 @@ static bool pci_endpoint_test_write(struct
> > pci_endpoint_test *test, size_t size)
> >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
> >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
> >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> > -                                COMMAND_READ);
> > +                                COMMAND_READ | flags);
> >
> >         wait_for_completion(&test->irq_raised);
> >
> > @@ -494,7 +516,24 @@ static bool pci_endpoint_test_write(struct
> > pci_endpoint_test *test, size_t size)
> >         return ret;
> >  }
> >
> > -static bool pci_endpoint_test_read(struct pci_endpoint_test *test,
> > size_t size)
> > +static bool pci_endpoint_test_write_dma(struct pci_endpoint_test *test,
> > +                                       unsigned long arg)
> > +{
> > +       u32 flags = COMMAND_FLAG_DMA;
> > +       struct pcitest_dma dma;
> > +
> > +       if (copy_from_user(&dma, (void *)arg, sizeof(struct pcitest_dma)))
> > +               return -EACCES;
> > +
> > +       if (dma.list)
> > +               flags |= COMMAND_FLAG_DMA_LIST;
> > +
> > +       return pci_endpoint_test_write(test, dma.size, flags);
> > +}
> > +
> > +static bool pci_endpoint_test_read(struct pci_endpoint_test *test,
> > +                                  size_t size,
> > +                                  u32 flags)
> >  {
> >         bool ret = false;
> >         void *addr;
> > @@ -542,7 +581,7 @@ static bool pci_endpoint_test_read(struct
> > pci_endpoint_test *test, size_t size)
> >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
> >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
> >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> > -                                COMMAND_WRITE);
> > +                                COMMAND_WRITE | flags);
> >
> >         wait_for_completion(&test->irq_raised);
> >
> > @@ -555,6 +594,21 @@ static bool pci_endpoint_test_read(struct
> > pci_endpoint_test *test, size_t size)
> >         return ret;
> >  }
> >
> > +static bool pci_endpoint_test_read_dma(struct pci_endpoint_test *test,
> > +                                      unsigned long arg)
> > +{
> > +       u32 flags = COMMAND_FLAG_DMA;
> > +       struct pcitest_dma dma;
> > +
> > +       if (copy_from_user(&dma, (void *)arg, sizeof(struct pcitest_dma)))
> > +               return -EACCES;
> > +
> > +       if (dma.list)
> > +               flags |= COMMAND_FLAG_DMA_LIST;
> > +
> > +       return pci_endpoint_test_read(test, dma.size, flags);
> > +}
> > +
> >  static bool pci_endpoint_test_set_irq(struct pci_endpoint_test *test,
> >                                       int req_irq_type)
> >  {
> > @@ -612,11 +666,17 @@ static long pci_endpoint_test_ioctl(struct file
> > *file, unsigned int cmd,
> >         case PCITEST_MSIX:
> >                 ret = pci_endpoint_test_msi_irq(test, arg, cmd == PCITEST_MSIX);
> >                 break;
> > +       case PCITEST_WRITE_DMA:
> > +               ret = pci_endpoint_test_write_dma(test, arg);
> > +               break;
> > +       case PCITEST_READ_DMA:
> > +               ret = pci_endpoint_test_read_dma(test, arg);
> > +               break;
> >         case PCITEST_WRITE:
> > -               ret = pci_endpoint_test_write(test, arg);
> > +               ret = pci_endpoint_test_write(test, arg, COMMAND_FLAG_NONE);
> >                 break;
> >         case PCITEST_READ:
> > -               ret = pci_endpoint_test_read(test, arg);
> > +               ret = pci_endpoint_test_read(test, arg, COMMAND_FLAG_NONE);
> >                 break;
> >         case PCITEST_COPY:
> >                 ret = pci_endpoint_test_copy(test, arg);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 2bf5a35c0570..7e25c0f5edf1 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -366,6 +366,26 @@ dw_pcie_ep_get_features(struct pci_epc *epc, u8
> > func_no)
> >         return ep->ops->get_features(ep);
> >  }
> >
> > +static int dw_pcie_ep_dma_read(struct pci_epc *epc, struct pci_epc_dma
> > *dma)
> > +{
> > +       struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > +
> > +       if (!ep->ops->dma_read)
> > +               return -EINVAL;
> > +
> > +       return ep->ops->dma_read(ep, dma);
> > +}
> > +
> > +static int dw_pcie_ep_dma_write(struct pci_epc *epc, struct pci_epc_dma
> > *dma)
> > +{
> > +       struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > +
> > +       if (!ep->ops->dma_write)
> > +               return -EINVAL;
> > +
> > +       return ep->ops->dma_write(ep, dma);
> > +}
> > +
> >  static const struct pci_epc_ops epc_ops = {
> >         .write_header           = dw_pcie_ep_write_header,
> >         .set_bar                = dw_pcie_ep_set_bar,
> > @@ -380,6 +400,8 @@ static const struct pci_epc_ops epc_ops = {
> >         .start                  = dw_pcie_ep_start,
> >         .stop                   = dw_pcie_ep_stop,
> >         .get_features           = dw_pcie_ep_get_features,
> > +       .dma_read               = dw_pcie_ep_dma_read,
> > +       .dma_write              = dw_pcie_ep_dma_write
> >  };
> >
> >  int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index b8993f2b78df..11d44ec8acc7 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -197,6 +197,8 @@ struct dw_pcie_ep_ops {
> >         int     (*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
> >                              enum pci_epc_irq_type type, u16 interrupt_num);
> >         const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
> > +       int     (*dma_read)(struct dw_pcie_ep *ep, struct pci_epc_dma *dma);
> > +       int     (*dma_write)(struct dw_pcie_ep *ep, struct pci_epc_dma *dma);
> >  };
> >
> >  struct dw_pcie_ep {
> > @@ -238,6 +240,7 @@ struct dw_pcie {
> >         void __iomem            *dbi_base2;
> >         /* Used when iatu_unroll_enabled is true */
> >         void __iomem            *atu_base;
> > +       void __iomem            *dma_base;
> >         u32                     num_viewport;
> >         u8                      iatu_unroll_enabled;
> >         struct pcie_port        pp;
> > @@ -323,6 +326,16 @@ static inline u32 dw_pcie_readl_atu(struct dw_pcie
> > *pci, u32 reg)
> >         return __dw_pcie_read_dbi(pci, pci->atu_base, reg, 0x4);
> >  }
> >
> > +static inline void dw_pcie_writel_dma(struct dw_pcie *pci, u32 reg, u32
> > val)
> > +{
> > +       __dw_pcie_write_dbi(pci, pci->dma_base, reg, 0x4, val);
> > +}
> > +
> > +static inline u32 dw_pcie_readl_dma(struct dw_pcie *pci, u32 reg)
> > +{
> > +       return __dw_pcie_read_dbi(pci, pci->dma_base, reg, 0x4);
> > +}
> > +
> >  static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
> >  {
> >         u32 reg;
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c
> > b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 27806987e93b..3910073712e9 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -28,6 +28,25 @@
> >  #define COMMAND_READ                   BIT(3)
> >  #define COMMAND_WRITE                  BIT(4)
> >  #define COMMAND_COPY                   BIT(5)
> > +#define COMMAND_FLAG2                  BIT(30)
> > +#define COMMAND_FLAG1                  BIT(31)
> > +
> > +#define COMMAND_FLAGS                  (COMMAND_FLAG1 | \
> > +                                        COMMAND_FLAG2)
> > +
> > +#define COMMAND_FLAG_NONE              0
> > +#define COMMAND_FLAG_DMA               COMMAND_FLAG1
> > +#define COMMAND_FLAG_DMA_LIST          COMMAND_FLAG2
> > +
> > +#define COMMAND_READ_DMA               (COMMAND_READ | \
> > +                                        COMMAND_FLAG_DMA)
> > +#define COMMAND_READ_DMA_LIST          (COMMAND_READ_DMA | \
> > +                                        COMMAND_FLAG_DMA_LIST)
> > +
> > +#define COMMAND_WRITE_DMA              (COMMAND_WRITE | \
> > +                                        COMMAND_FLAG_DMA)
> > +#define COMMAND_WRITE_DMA_LIST         (COMMAND_WRITE_DMA | \
> > +                                        COMMAND_FLAG_DMA_LIST)
> >
> >  #define STATUS_READ_SUCCESS            BIT(0)
> >  #define STATUS_READ_FAIL               BIT(1)
> > @@ -187,6 +206,93 @@ static int pci_epf_test_read(struct pci_epf_test
> > *epf_test)
> >         return ret;
> >  }
> >
> > +static int pci_epf_test_read_dma(struct pci_epf_test *epf_test)
> > +{
> > +       int ret = -ENOMEM;
> > +       struct pci_epf *epf = epf_test->epf;
> > +       struct device *dev = &epf->dev;
> > +       struct pci_epc *epc = epf->epc;
> > +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> > +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> > +       struct pci_epc_dma dma;
> > +       u32 crc32;
> > +       void *buf;
> > +
> > +       buf = kzalloc(reg->size, GFP_KERNEL);
> > +       if (buf) {
> > +               dma.control = PCI_EPC_DMA_CONTROL_LIE;
> > +               dma.size = reg->size;
> > +               dma.sar = reg->src_addr;
> > +               dma.dar = virt_to_phys(buf);
> > +
> > +               ret = pci_epc_dma_read(epc, &dma);
> > +               if (ret) {
> > +                       dev_err(dev, "pci_epc_dma_read %d\n", ret);
> > +               } else {
> > +                       crc32 = crc32_le(~0, buf, reg->size);
> > +                       if (crc32 != reg->checksum)
> > +                               ret = -EIO;
> > +               }
> > +
> > +               kfree(buf);
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +static int pci_epf_test_read_dma_list(struct pci_epf_test *epf_test)
> > +{
> > +       int ret = -ENOMEM;
> > +       struct pci_epf *epf = epf_test->epf;
> > +       struct device *dev = &epf->dev;
> > +       struct pci_epc *epc = epf->epc;
> > +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> > +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> > +       struct pci_epc_dma *dma;
> > +       u32 crc32;
> > +       void *buf;
> > +
> > +       dma = kcalloc(3, sizeof(*dma), GFP_KERNEL);
> > +       if (dma) {
> > +               buf = kzalloc(reg->size, GFP_KERNEL);
> > +               if (buf) {
> > +                       int half_size = reg->size >> 1;
> > +                       phys_addr_t phys_addr = virt_to_phys(buf);
> > +
> > +                       dma[0].control = PCI_EPC_DMA_CONTROL_CB;
> > +                       dma[0].size = half_size ? half_size : 1;
> > +                       dma[0].sar = reg->src_addr;
> > +                       dma[0].dar = phys_addr;
> > +
> > +                       dma[1].control = PCI_EPC_DMA_CONTROL_CB |
> > +                                        PCI_EPC_DMA_CONTROL_LIE;
> > +                       dma[1].size = reg->size - half_size;
> > +                       dma[1].sar = reg->src_addr + half_size;
> > +                       dma[1].dar = phys_addr + half_size;
> > +
> > +                       dma[2].control = PCI_EPC_DMA_CONTROL_EOL;
> > +                       dma[2].size = 0;
> > +                       dma[2].sar = virt_to_phys(dma);
> > +                       dma[2].dar = 0;
> > +
> > +                       ret = pci_epc_dma_read(epc, dma);
> > +                       if (ret) {
> > +                               dev_err(dev, "pci_epc_dma_read %d\n", ret);
> > +                       } else {
> > +                               crc32 = crc32_le(~0, buf, reg->size);
> > +                               if (crc32 != reg->checksum)
> > +                                       ret = -EIO;
> > +                       }
> > +
> > +                       kfree(buf);
> > +               }
> > +
> > +               kfree(dma);
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> >  static int pci_epf_test_write(struct pci_epf_test *epf_test)
> >  {
> >         int ret;
> > @@ -244,6 +350,87 @@ static int pci_epf_test_write(struct pci_epf_test
> > *epf_test)
> >         return ret;
> >  }
> >
> > +static int pci_epf_test_write_dma(struct pci_epf_test *epf_test)
> > +{
> > +       int ret = -ENOMEM;
> > +       struct pci_epf *epf = epf_test->epf;
> > +       struct device *dev = &epf->dev;
> > +       struct pci_epc *epc = epf->epc;
> > +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> > +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> > +       struct pci_epc_dma dma;
> > +       void *buf;
> > +
> > +       buf = kzalloc(reg->size, GFP_KERNEL);
> > +       if (buf) {
> > +               get_random_bytes(buf, reg->size);
> > +               reg->checksum = crc32_le(~0, buf, reg->size);
> > +
> > +               dma.control = PCI_EPC_DMA_CONTROL_LIE;
> > +               dma.size = reg->size;
> > +               dma.sar = virt_to_phys(buf);
> > +               dma.dar = reg->dst_addr;
> > +
> > +               ret = pci_epc_dma_write(epc, &dma);
> > +               if (ret)
> > +                       dev_err(dev, "pci_epc_dma_write %d\n", ret);
> > +
> > +               kfree(buf);
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +static int pci_epf_test_write_dma_list(struct pci_epf_test *epf_test)
> > +{
> > +       int ret = -ENOMEM;
> > +       struct pci_epf *epf = epf_test->epf;
> > +       struct device *dev = &epf->dev;
> > +       struct pci_epc *epc = epf->epc;
> > +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> > +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> > +       struct pci_epc_dma *dma;
> > +       void *buf;
> > +
> > +       dma = kcalloc(3, sizeof(*dma), GFP_KERNEL);
> > +       if (dma) {
> > +               buf = kzalloc(reg->size, GFP_KERNEL);
> > +               if (buf) {
> > +                       int half_size = reg->size >> 1;
> > +                       phys_addr_t phys_addr = virt_to_phys(buf);
> > +
> > +                       get_random_bytes(buf, reg->size);
> > +                       reg->checksum = crc32_le(~0, buf, reg->size);
> > +
> > +                       dma[0].control = PCI_EPC_DMA_CONTROL_CB;
> > +                       dma[0].size = half_size ? half_size : 1;
> > +                       dma[0].sar = phys_addr;
> > +                       dma[0].dar = reg->dst_addr;
> > +
> > +                       dma[1].control = PCI_EPC_DMA_CONTROL_CB |
> > +                                        PCI_EPC_DMA_CONTROL_LIE;
> > +                       dma[1].size = reg->size - half_size;
> > +                       dma[1].sar = phys_addr + half_size;
> > +                       dma[1].dar = reg->dst_addr + half_size;
> > +
> > +                       dma[2].control = PCI_EPC_DMA_CONTROL_EOL;
> > +                       dma[2].size = 0;
> > +                       dma[2].sar = virt_to_phys(dma);
> > +                       dma[2].dar = 0;
> > +
> > +                       ret = pci_epc_dma_write(epc, dma);
> > +                       if (ret)
> > +                               dev_err(dev, "pci_epc_dma_write %d\n", ret);
> > +
> > +                       kfree(buf);
> > +               }
> > +
> > +               kfree(dma);
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> >  static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test, u8
> > irq_type,
> >                                    u16 irq)
> >  {
> > @@ -303,18 +490,34 @@ static void pci_epf_test_cmd_handler(struct
> > work_struct *work)
> >         }
> >
> >         if (command & COMMAND_WRITE) {
> > -               ret = pci_epf_test_write(epf_test);
> > -               if (ret)
> > -                       reg->status |= STATUS_WRITE_FAIL;
> > +               command &= (COMMAND_WRITE | COMMAND_FLAGS);
> > +               if (command == COMMAND_WRITE)
> > +                       ret = pci_epf_test_write(epf_test);
> > +               else if (command == COMMAND_WRITE_DMA)
> > +                       ret = pci_epf_test_write_dma(epf_test);
> > +               else if (command == COMMAND_WRITE_DMA_LIST)
> > +                       ret = pci_epf_test_write_dma_list(epf_test);
> >                 else
> > +                       ret = -EINVAL;
> > +               if (!ret)
> >                         reg->status |= STATUS_WRITE_SUCCESS;
> > +               else
> > +                       reg->status |= STATUS_WRITE_FAIL;
> >                 pci_epf_test_raise_irq(epf_test, reg->irq_type,
> >                                        reg->irq_number);
> >                 goto reset_handler;
> >         }
> >
> >         if (command & COMMAND_READ) {
> > -               ret = pci_epf_test_read(epf_test);
> > +               command &= (COMMAND_READ | COMMAND_FLAGS);
> > +               if (command == COMMAND_READ)
> > +                       ret = pci_epf_test_read(epf_test);
> > +               else if (command == COMMAND_READ_DMA)
> > +                       ret = pci_epf_test_read_dma(epf_test);
> > +               else if (command == COMMAND_READ_DMA_LIST)
> > +                       ret = pci_epf_test_read_dma_list(epf_test);
> > +               else
> > +                       ret = -EINVAL;
> >                 if (!ret)
> >                         reg->status |= STATUS_READ_SUCCESS;
> >                 else
> > diff --git a/drivers/pci/endpoint/pci-epc-core.c
> > b/drivers/pci/endpoint/pci-epc-core.c
> > index e4712a0f249c..a57e501d4abc 100644
> > --- a/drivers/pci/endpoint/pci-epc-core.c
> > +++ b/drivers/pci/endpoint/pci-epc-core.c
> > @@ -107,6 +107,52 @@ unsigned int pci_epc_get_first_free_bar(const struct
> > pci_epc_features
> >  EXPORT_SYMBOL_GPL(pci_epc_get_first_free_bar);
> >
> >  /**
> > + * pci_epc_dma_write() - DMA a block of memory to remote address
> > + * @epc: the EPC device on which to perform DMA transfer
> > + * @dma: DMA descriptors array
> > + *
> > + * Write contents of local memory to remote memory by DMA.
> > + */
> > +int pci_epc_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma)
> > +{
> > +       int ret;
> > +       unsigned long flags;
> > +
> > +       if (IS_ERR(epc) || !epc->ops->dma_write)
> > +               return -EINVAL;
> > +
> > +       spin_lock_irqsave(&epc->lock, flags);
> > +       ret = epc->ops->dma_write(epc, dma);
> > +       spin_unlock_irqrestore(&epc->lock, flags);
> > +
> > +       return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_epc_dma_write);
> > +
> > +/**
> > + * pci_epc_dma_read() - DMA a block of memory from remote address
> > + * @epc: the EPC device on which to perform DMA transfer
> > + * @dma: DMA descriptors array
> > + *
> > + * Read contents of remote memory into local memory by DMA.
> > + */
> > +int pci_epc_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma)
> > +{
> > +       int ret;
> > +       unsigned long flags;
> > +
> > +       if (IS_ERR(epc) || !epc->ops->dma_read)
> > +               return -EINVAL;
> > +
> > +       spin_lock_irqsave(&epc->lock, flags);
> > +       ret = epc->ops->dma_read(epc, dma);
> > +       spin_unlock_irqrestore(&epc->lock, flags);
> > +
> > +       return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_epc_dma_read);
> > +
> > +/**
> >   * pci_epc_get_features() - get the features supported by EPC
> >   * @epc: the features supported by *this* EPC device will be returned
> >   * @func_no: the features supported by the EPC device specific to the
> > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> > index f641badc2c61..d845f13d0baf 100644
> > --- a/include/linux/pci-epc.h
> > +++ b/include/linux/pci-epc.h
> > @@ -21,6 +21,45 @@ enum pci_epc_irq_type {
> >  };
> >
> >  /**
> > + * struct pci_epc_dma - descriptor for a DMA transfer element
> > + * @control: DMA channel control bits for read or write transfer
> > + * @size: size of DMA transfer element
> > + * @sar: source addrees for DMA transfer element
> > + * @dar: destination address for DMA transfer element
> > + */
> > +
> > +struct pci_epc_dma {
> > +       u32     control;
> > +       u32     size;
> > +       u64     sar;
> > +       u64     dar;
> > +};
> > +
> > +#define PCI_EPC_DMA_CONTROL_CB          (BIT(0))
> > +#define PCI_EPC_DMA_CONTROL_TCB         (BIT(1))
> > +#define PCI_EPC_DMA_CONTROL_LLP         (BIT(2))
> > +#define PCI_EPC_DMA_CONTROL_LIE         (BIT(3))
> > +#define PCI_EPC_DMA_CONTROL_RIE         (BIT(4))
> > +#define PCI_EPC_DMA_CONTROL_CS          (BIT(5) | BIT(6))
> > +#define PCI_EPC_DMA_CONTROL_CCS         (BIT(8))
> > +#define PCI_EPC_DMA_CONTROL_LLE         (BIT(9))
> > +#define PCI_EPC_DMA_CONTROL_FUNC        (BIT(12) | BIT(13) | BIT(14) | \
> > +                                        BIT(15) | BIT(16))
> > +#define PCI_EPC_DMA_CONTROL_NS_DST      (BIT(23))
> > +#define PCI_EPC_DMA_CONTROL_NS_SRC      (BIT(24))
> > +#define PCI_EPC_DMA_CONTROL_RO          (BIT(25))
> > +#define PCI_EPC_DMA_CONTROL_TC          (BIT(27) | BIT(28) | BIT(29))
> > +#define PCI_EPC_DMA_CONTROL_AT          (BIT(30) | BIT(31))
> > +
> > +#define PCI_EPC_DMA_CONTROL_EOL         (PCI_EPC_DMA_CONTROL_TCB | \
> > +                                        PCI_EPC_DMA_CONTROL_LLP)
> > +
> > +#define PCI_EPC_DMA_CONTROL_LIST        (PCI_EPC_DMA_CONTROL_CB | \
> > +                                        PCI_EPC_DMA_CONTROL_EOL| \
> > +                                        PCI_EPC_DMA_CONTROL_CCS | \
> > +                                        PCI_EPC_DMA_CONTROL_LLE)
> > +
> > +/**
> >   * struct pci_epc_ops - set of function pointers for performing EPC
> > operations
> >   * @write_header: ops to populate configuration space header
> >   * @set_bar: ops to configure the BAR
> > @@ -38,6 +77,8 @@ enum pci_epc_irq_type {
> >   * @raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
> >   * @start: ops to start the PCI link
> >   * @stop: ops to stop the PCI link
> > + * @dma_read: ops to read remote memory into local memory by DMA
> > + * @dma_write: ops to write local memory to remote memory by DMA
> >   * @owner: the module owner containing the ops
> >   */
> >  struct pci_epc_ops {
> > @@ -61,6 +102,8 @@ struct pci_epc_ops {
> >         void    (*stop)(struct pci_epc *epc);
> >         const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
> >                                                        u8 func_no);
> > +       int     (*dma_read)(struct pci_epc *epc, struct pci_epc_dma *dma);
> > +       int     (*dma_write)(struct pci_epc *epc, struct pci_epc_dma *dma);
> >         struct module *owner;
> >  };
> >
> > @@ -152,6 +195,8 @@ void pci_epc_destroy(struct pci_epc *epc);
> >  int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf);
> >  void pci_epc_linkup(struct pci_epc *epc);
> >  void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf);
> > +int pci_epc_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma);
> > +int pci_epc_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma);
> >  int pci_epc_write_header(struct pci_epc *epc, u8 func_no,
> >                          struct pci_epf_header *hdr);
> >  int pci_epc_set_bar(struct pci_epc *epc, u8 func_no,
> > diff --git a/include/uapi/linux/pcitest.h b/include/uapi/linux/pcitest.h
> > index cbf422e56696..505f4a3811c2 100644
> > --- a/include/uapi/linux/pcitest.h
> > +++ b/include/uapi/linux/pcitest.h
> > @@ -10,11 +10,18 @@
> >  #ifndef __UAPI_LINUX_PCITEST_H
> >  #define __UAPI_LINUX_PCITEST_H
> >
> > +struct pcitest_dma {
> > +       size_t  size;
> > +       bool    list;
> > +};
> > +
> >  #define PCITEST_BAR            _IO('P', 0x1)
> >  #define PCITEST_LEGACY_IRQ     _IO('P', 0x2)
> >  #define PCITEST_MSI            _IOW('P', 0x3, int)
> >  #define PCITEST_WRITE          _IOW('P', 0x4, unsigned long)
> > +#define PCITEST_WRITE_DMA      _IOW('P', 0x4, struct pcitest_dma)
> >  #define PCITEST_READ           _IOW('P', 0x5, unsigned long)
> > +#define PCITEST_READ_DMA       _IOW('P', 0x5, struct pcitest_dma)
> >  #define PCITEST_COPY           _IOW('P', 0x6, unsigned long)
> >  #define PCITEST_MSIX           _IOW('P', 0x7, int)
> >  #define PCITEST_SET_IRQTYPE    _IOW('P', 0x8, int)
> > diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
> > index 6f1303104d84..66cd19acf18c 100644
> > --- a/tools/pci/pcitest.c
> > +++ b/tools/pci/pcitest.c
> > @@ -44,11 +44,14 @@ struct pci_test {
> >         bool            read;
> >         bool            write;
> >         bool            copy;
> > +       bool            dma;
> > +       bool            dma_list;
> >         unsigned long   size;
> >  };
> >
> >  static int run_test(struct pci_test *test)
> >  {
> > +       struct pcitest_dma dma;
> >         int ret = -EINVAL;
> >         int fd;
> >
> > @@ -113,7 +116,13 @@ static int run_test(struct pci_test *test)
> >         }
> >
> >         if (test->write) {
> > -               ret = ioctl(fd, PCITEST_WRITE, test->size);
> > +               if (test->dma) {
> > +                       dma.size = test->size;
> > +                       dma.list = test->dma_list;
> > +                       ret = ioctl(fd, PCITEST_WRITE_DMA, &dma);
> > +               } else {
> > +                       ret = ioctl(fd, PCITEST_WRITE, test->size);
> > +               }
> >                 fprintf(stdout, "WRITE (%7ld bytes):\t\t", test->size);
> >                 if (ret < 0)
> >                         fprintf(stdout, "TEST FAILED\n");
> > @@ -122,7 +131,13 @@ static int run_test(struct pci_test *test)
> >         }
> >
> >         if (test->read) {
> > -               ret = ioctl(fd, PCITEST_READ, test->size);
> > +               if (test->dma) {
> > +                       dma.size = test->size;
> > +                       dma.list = test->dma_list;
> > +                       ret = ioctl(fd, PCITEST_READ_DMA, &dma);
> > +               } else {
> > +                       ret = ioctl(fd, PCITEST_READ, test->size);
> > +               }
> >                 fprintf(stdout, "READ (%7ld bytes):\t\t", test->size);
> >                 if (ret < 0)
> >                         fprintf(stdout, "TEST FAILED\n");
> > @@ -163,7 +178,7 @@ int main(int argc, char **argv)
> >         /* set default endpoint device */
> >         test->device = "/dev/pci-endpoint-test.0";
> >
> > -       while ((c = getopt(argc, argv, "D:b:m:x:i:Ilhrwcs:")) != EOF)
> > +       while ((c = getopt(argc, argv, "D:b:m:x:i:IlhrwcdLs:")) != EOF)
> >         switch (c) {
> >         case 'D':
> >                 test->device = optarg;
> > @@ -204,6 +219,12 @@ int main(int argc, char **argv)
> >         case 'c':
> >                 test->copy = true;
> >                 continue;
> > +       case 'd':
> > +               test->dma = true;
> > +               continue;
> > +       case 'L':
> > +               test->dma_list = true;
> > +               continue;
> >         case 's':
> >                 test->size = strtoul(optarg, NULL, 0);
> >                 continue;
> > @@ -223,6 +244,8 @@ int main(int argc, char **argv)
> >                         "\t-r                   Read buffer test\n"
> >                         "\t-w                   Write buffer test\n"
> >                         "\t-c                   Copy buffer test\n"
> > +                       "\t-d                   DMA mode for read or write test\n"
> > +                       "\t-L                   Linked-List DMA flag for DMA mode\n"
> >                         "\t-s <size>            Size of buffer {default: 100KB}\n"
> >                         "\t-h                   Print this help message\n",
> >                         argv[0]);
> > --
> > 2.7.4
> >

Alan Mikhak May 29, 2019, 10:37 p.m. UTC | #4

On Mon, May 27, 2019 at 2:09 AM Gustavo Pimentel
<[e-mail]> wrote:
>
> On Fri, May 24, 2019 at 20:42:43, Alan Mikhak <[e-mail]>
> wrote:
>
> Hi Alan,
>
> > On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
> > <[e-mail]> wrote:
> > >
> > > Hi Alan,
> > >
> > > This patch implementation is very HW implementation dependent and
> > > requires the DMA to exposed through PCIe BARs, which aren't always the
> > > case. Besides, you are defining some control bits on
> > > include/linux/pci-epc.h that may not have any meaning to other types of
> > > DMA.
> > >
> > > I don't think this was what Kishon had in mind when he developed the
> > > pcitest, but let see what Kishon was to say about it.
> > >
> > > I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
> > > and which I submitted some days ago.
> > > By having a DMA driver which implemented using DMAengine API, means the
> > > pcitest can use the DMAengine client API, which will be completely
> > > generic to any other DMA implementation.
> > >
> > > My DMA driver for DWC PCI was done thinking on my use case, which is was
> > > interacting from the Root Complex side with DMA IP implemented on the
> > > Endpoint, which was exposed through PCI BARs. However, I think it would
> > > be possible to reuse the same core code and instead of using the PCI-glue
> > > to adapt it and be used easily on the Endpoint side and to be triggered
> > > there.
> > >
> > > Gustavo
> >
> > Hi Gustavo,
> >
> > I saw your patches for the DMA driver for DWC PCI which added the EDDA
> > driver to pci_endpoint_test.c on the host side. This suggested to me
> > that the user would invoke pcitest on the host side to exercise your
> > driver on the endpoint.
> >
> > However, I didn't see any changes to pci-epf-test.c to modify the
> > pci_epf_test_write() and pci_epf_test_read() functions to use DMA
> > instead of memcpy_toio() and memcpy_fromio(), respectively.
>
> I didn't add the DMA engine client API to the pcitest yet. I was waiting
> for the eDMA driver to be integrated on Kernel before doing any on
> pcitest.
>
> >
> > I have four separate DMA requirements in mind as motivation for this patch.
> >
> > The following two requirements are implemented by this patch:
> > Local DMA write or read transfer initiated by endpoint under user
> > command from the host between system and endpoint memory buffers using
> > the endpoint DMA controller with a local interrupt.
> > 1) single block
> > 2) linked-list mode
>
> However, in most cases, you will not have the DMA block registers or
> linked list memory accessible or defined on PCI BARs and based on I have
> seen in your patch, the whole patch is based on that premise. The current
> implementation is not generic and highly dependent on IP and design.
> I strongly believe with some slight modifications it would be possible to
> run the DMA engine controller driver with a platform glue-logic on the
> Endpoint side, which could interact with pci_epf_test driver. This way
> the pcitest would be always generic to all products.
>
> >
> > This patch anticipates two more requirements yet to be implemented in
> > future patches:
> > Remote DMA write or read transfer initiated by host between system and
> > endpoint memory buffers using the endpoint DMA controller with a local
> > interrupt to the endpoint processor and a remote interrupt to the host
> > process.
> > 1) single block
> > 2) linked-list mode
>
> That's what the submitted patch driver does. I didn't develop the
> interaction with to pcitest, however, while developing DW eDMA IP driver,
> I've sent in some an RFC patch containing dw-edma-test driver that
> stimulates the DW eDMA.
>
> >
> > The descriptor format defined in pci-epc.h allows for pci-epf-test.c
> > to be expanded over time to initiate more elaborate DMA tests to
> > exercise other endpoint DMA scenarios.
>
> Yes, but that's not the original issue been discussed here. But let's see
> what Kishon as to say, In the end, he is the maintainer of this tool.
>
> Regards,
> Gustavo
>
Thanks Gustavo for your comments. I accept that the DMAengine API
offers a generic solution that would be desirable. I look forward to
seeing your subsequent patches which integrate the DMAengine client
API with pcitest.

> >
> > The following is a sample output of the pcitest usage for exercising
> > DMA operations on the endpoint:
> >
> > $ pcitest -r -d
> > READ ( 102400 bytes): OKAY
> > $ pcitest -w -d
> > WRITE ( 102400 bytes): OKAY
> > $ pcitest -w -d -L
> > WRITE ( 102400 bytes): OKAY
> > $ pcitest -r -d -L
> > READ ( 102400 bytes): OKAY
> >
> > The above are executed using DMA operations as opposed to the
> > following which use memcpy_toio() and memcpy_fromio().
> >
> > $ pcitest -r
> > READ ( 102400 bytes): OKAY
> > $ pcitest -w
> > WRITE ( 102400 bytes): OKAY
> >
> > Regards,
> > Alan Mikhak
> >
> >
> >
> > >
> > > -----Original Message-----
> > > From: Alan Mikhak <[e-mail]>
> > >
> > > Sent: 23 de maio de 2019 23:24
> > > To: [e-mail];
> > > [e-mail]; [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]
> > > Cc: Alan Mikhak
> > > <[e-mail]>
> > > Subject: [PATCH] PCI: endpoint: Add DMA to Linux
> > > PCI EP Framework
> > >
> > > This patch depends on patch the following patches:
> > > [PATCH v2 1/2] tools: PCI: Fix broken pcitest compilation
> > > [PATCH v2 2/2] tools: PCI: Fix compiler warning in pcitest
> > >
> > > The Linux PCI Endpoint Framework currently does not support initiation of
> > > DMA read and write operations. This patch extends the Linux PCI Endpoint
> > > Framework by adding support for the user of pcitest to inititate DMA
> > > read and write operations via PCIe endpoint controller drivers. This
> > > patch
> > > provides the means but leaves it up to individual PCIe endpoint
> > > controller
> > > drivers to implement DMA support, if desired.
> > >
> > > This patch provides support for the pcitest user to instruct the endpoint
> > > to initiate local DMA transfers consisting of single or linked-list
> > > blocks
> > > into endpoint buffers using the endpoint DMA controller. It anticipates
> > > that future patches would add support for the pcitest user to instruct
> > > the endpoint to participate in remote DMA transfers initiated from the
> > > root complex into endpoint buffers using the endpoint DMA controller.
> > >
> > > This patch depends on the first two patches in its patchset to resolve
> > > a pre-existing pcitest compilation error.
> > >
> > > * Add -d flag to pcitest command line options so user can specify
> > > that a read or write command should execute using local DMA to be
> > > initiated by endpoint.
> > >
> > > * Add -L flag to pcitest command line options so user can specify
> > > that DMA operation should execute in linked-list mode.
> > >
> > > * Add struct pcitest_dma for pcitest to communicate DMA options
> > > from host userspace to pci_endpoint_test driver in host kernel
> > > via two new ioctls PCITEST_WRITE_DMA and PCITEST_READ_DMA.
> > >
> > > * Add command flags so pci_endpoint_test driver running on host
> > > can communicate DMA read and write options across the PCI bus
> > > to pci-epf-test driver running on endpoint.
> > >
> > > * Add struct pci_epc_dma so pci-epf-test driver can create DMA
> > > read and write descriptors for single or linked-list DMA operations
> > > and pass such descriptors to pci-epc-core via new functions
> > > pci_epc_dma_read() and pci_epc_dma_write().
> > >
> > > * Add four new functions in pci-epf-test driver to implement
> > > new DMA read and write tests by initiating local DMA transfers
> > > in linked-list and single modes via PCIe endpoint controller
> > > drivers: pci_epf_test_read_dma(), pci_epf_test_read_dma_list(),
> > > pci_epf_test_write_dma(), and pci_epf_test_write_dma_list().
> > >
> > > * Add dma_read and dma_write functions to struct pci_epc_ops
> > > so pci_epc_dma_read() and pci_epc_dma_write() functions can
> > > pass DMA descriptors down the stack to pcie-designware-ep layer.
> > >
> > > * Add dma_read and dma_write functions to struct dw_pcie_ep_ops
> > > so pcie-designware-ep layer can communicate DMA descriptors down
> > > the stack to vendor PCIe endpoint controller drivers.
> > >
> > > * Add dma_base pointer to struct dw_pcie for vendor PCIe endpoint
> > > controller driver to set if it implements DMA operations.
> > >
> > > * Add two common pcie-designware functions dw_pcie_writel_dma()
> > > and dw_pcie_readl_dma() for use by vendor PCIe endpoint
> > > controllers to access DMA registers via the dma_base pointer.
> > >
> > > Signed-off-by: Alan Mikhak <[e-mail]>
> > > ---
> > >
> > > drivers/misc/pci_endpoint_test.c                |  72 +++++++-
> > >  drivers/pci/controller/dwc/pcie-designware-ep.c |  22 +++
> > >  drivers/pci/controller/dwc/pcie-designware.h    |  13 ++
> > >  drivers/pci/endpoint/functions/pci-epf-test.c   | 211
> > > +++++++++++++++++++++++-
> > >  drivers/pci/endpoint/pci-epc-core.c             |  46 ++++++
> > >  include/linux/pci-epc.h                         |  45 +++++
> > >  include/uapi/linux/pcitest.h                    |   7 +
> > >  tools/pci/pcitest.c                             |  29 +++-
> > >  8 files changed, 432 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/misc/pci_endpoint_test.c
> > > b/drivers/misc/pci_endpoint_test.c
> > > index 7b015f2a1c6f..63b86d81a6b5 100644
> > > --- a/drivers/misc/pci_endpoint_test.c
> > > +++ b/drivers/misc/pci_endpoint_test.c
> > > @@ -34,6 +34,7 @@
> > >  #include <linux/pci_regs.h>
> > >
> > >  #include <uapi/linux/pcitest.h>
> > > +#include <linux/uaccess.h>
> > >
> > >  #define DRV_MODULE_NAME                                "pci-endpoint-test"
> > >
> > > @@ -51,6 +52,25 @@
> > >  #define COMMAND_READ                           BIT(3)
> > >  #define COMMAND_WRITE                          BIT(4)
> > >  #define COMMAND_COPY                           BIT(5)
> > > +#define COMMAND_FLAG2                          BIT(30)
> > > +#define COMMAND_FLAG1                          BIT(31)
> > > +
> > > +#define COMMAND_FLAGS                          (COMMAND_FLAG1 | \
> > > +                                                COMMAND_FLAG2)
> > > +
> > > +#define COMMAND_FLAG_NONE                      0
> > > +#define COMMAND_FLAG_DMA                       COMMAND_FLAG1
> > > +#define COMMAND_FLAG_DMA_LIST                  COMMAND_FLAG2
> > > +
> > > +#define COMMAND_READ_DMA                       (COMMAND_READ | \
> > > +                                                COMMAND_FLAG_DMA)
> > > +#define COMMAND_READ_DMA_LIST                  (COMMAND_READ_DMA | \
> > > +                                                COMMAND_FLAG_DMA_LIST)
> > > +
> > > +#define COMMAND_WRITE_DMA                      (COMMAND_WRITE | \
> > > +                                                COMMAND_FLAG_DMA)
> > > +#define COMMAND_WRITE_DMA_LIST                 (COMMAND_WRITE_DMA | \
> > > +                                                COMMAND_FLAG_DMA_LIST)
> > >
> > >  #define PCI_ENDPOINT_TEST_STATUS               0x8
> > >  #define STATUS_READ_SUCCESS                    BIT(0)
> > > @@ -425,7 +445,9 @@ static bool pci_endpoint_test_copy(struct
> > > pci_endpoint_test *test, size_t size)
> > >         return ret;
> > >  }
> > >
> > > -static bool pci_endpoint_test_write(struct pci_endpoint_test *test,
> > > size_t size)
> > > +static bool pci_endpoint_test_write(struct pci_endpoint_test *test,
> > > +                                   size_t size,
> > > +                                   u32 flags)
> > >  {
> > >         bool ret = false;
> > >         u32 reg;
> > > @@ -480,7 +502,7 @@ static bool pci_endpoint_test_write(struct
> > > pci_endpoint_test *test, size_t size)
> > >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
> > >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
> > >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> > > -                                COMMAND_READ);
> > > +                                COMMAND_READ | flags);
> > >
> > >         wait_for_completion(&test->irq_raised);
> > >
> > > @@ -494,7 +516,24 @@ static bool pci_endpoint_test_write(struct
> > > pci_endpoint_test *test, size_t size)
> > >         return ret;
> > >  }
> > >
> > > -static bool pci_endpoint_test_read(struct pci_endpoint_test *test,
> > > size_t size)
> > > +static bool pci_endpoint_test_write_dma(struct pci_endpoint_test *test,
> > > +                                       unsigned long arg)
> > > +{
> > > +       u32 flags = COMMAND_FLAG_DMA;
> > > +       struct pcitest_dma dma;
> > > +
> > > +       if (copy_from_user(&dma, (void *)arg, sizeof(struct pcitest_dma)))
> > > +               return -EACCES;
> > > +
> > > +       if (dma.list)
> > > +               flags |= COMMAND_FLAG_DMA_LIST;
> > > +
> > > +       return pci_endpoint_test_write(test, dma.size, flags);
> > > +}
> > > +
> > > +static bool pci_endpoint_test_read(struct pci_endpoint_test *test,
> > > +                                  size_t size,
> > > +                                  u32 flags)
> > >  {
> > >         bool ret = false;
> > >         void *addr;
> > > @@ -542,7 +581,7 @@ static bool pci_endpoint_test_read(struct
> > > pci_endpoint_test *test, size_t size)
> > >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
> > >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
> > >         pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> > > -                                COMMAND_WRITE);
> > > +                                COMMAND_WRITE | flags);
> > >
> > >         wait_for_completion(&test->irq_raised);
> > >
> > > @@ -555,6 +594,21 @@ static bool pci_endpoint_test_read(struct
> > > pci_endpoint_test *test, size_t size)
> > >         return ret;
> > >  }
> > >
> > > +static bool pci_endpoint_test_read_dma(struct pci_endpoint_test *test,
> > > +                                      unsigned long arg)
> > > +{
> > > +       u32 flags = COMMAND_FLAG_DMA;
> > > +       struct pcitest_dma dma;
> > > +
> > > +       if (copy_from_user(&dma, (void *)arg, sizeof(struct pcitest_dma)))
> > > +               return -EACCES;
> > > +
> > > +       if (dma.list)
> > > +               flags |= COMMAND_FLAG_DMA_LIST;
> > > +
> > > +       return pci_endpoint_test_read(test, dma.size, flags);
> > > +}
> > > +
> > >  static bool pci_endpoint_test_set_irq(struct pci_endpoint_test *test,
> > >                                       int req_irq_type)
> > >  {
> > > @@ -612,11 +666,17 @@ static long pci_endpoint_test_ioctl(struct file
> > > *file, unsigned int cmd,
> > >         case PCITEST_MSIX:
> > >                 ret = pci_endpoint_test_msi_irq(test, arg, cmd == PCITEST_MSIX);
> > >                 break;
> > > +       case PCITEST_WRITE_DMA:
> > > +               ret = pci_endpoint_test_write_dma(test, arg);
> > > +               break;
> > > +       case PCITEST_READ_DMA:
> > > +               ret = pci_endpoint_test_read_dma(test, arg);
> > > +               break;
> > >         case PCITEST_WRITE:
> > > -               ret = pci_endpoint_test_write(test, arg);
> > > +               ret = pci_endpoint_test_write(test, arg, COMMAND_FLAG_NONE);
> > >                 break;
> > >         case PCITEST_READ:
> > > -               ret = pci_endpoint_test_read(test, arg);
> > > +               ret = pci_endpoint_test_read(test, arg, COMMAND_FLAG_NONE);
> > >                 break;
> > >         case PCITEST_COPY:
> > >                 ret = pci_endpoint_test_copy(test, arg);
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > index 2bf5a35c0570..7e25c0f5edf1 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > @@ -366,6 +366,26 @@ dw_pcie_ep_get_features(struct pci_epc *epc, u8
> > > func_no)
> > >         return ep->ops->get_features(ep);
> > >  }
> > >
> > > +static int dw_pcie_ep_dma_read(struct pci_epc *epc, struct pci_epc_dma
> > > *dma)
> > > +{
> > > +       struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > +
> > > +       if (!ep->ops->dma_read)
> > > +               return -EINVAL;
> > > +
> > > +       return ep->ops->dma_read(ep, dma);
> > > +}
> > > +
> > > +static int dw_pcie_ep_dma_write(struct pci_epc *epc, struct pci_epc_dma
> > > *dma)
> > > +{
> > > +       struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > +
> > > +       if (!ep->ops->dma_write)
> > > +               return -EINVAL;
> > > +
> > > +       return ep->ops->dma_write(ep, dma);
> > > +}
> > > +
> > >  static const struct pci_epc_ops epc_ops = {
> > >         .write_header           = dw_pcie_ep_write_header,
> > >         .set_bar                = dw_pcie_ep_set_bar,
> > > @@ -380,6 +400,8 @@ static const struct pci_epc_ops epc_ops = {
> > >         .start                  = dw_pcie_ep_start,
> > >         .stop                   = dw_pcie_ep_stop,
> > >         .get_features           = dw_pcie_ep_get_features,
> > > +       .dma_read               = dw_pcie_ep_dma_read,
> > > +       .dma_write              = dw_pcie_ep_dma_write
> > >  };
> > >
> > >  int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > > b/drivers/pci/controller/dwc/pcie-designware.h
> > > index b8993f2b78df..11d44ec8acc7 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > @@ -197,6 +197,8 @@ struct dw_pcie_ep_ops {
> > >         int     (*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
> > >                              enum pci_epc_irq_type type, u16 interrupt_num);
> > >         const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
> > > +       int     (*dma_read)(struct dw_pcie_ep *ep, struct pci_epc_dma *dma);
> > > +       int     (*dma_write)(struct dw_pcie_ep *ep, struct pci_epc_dma *dma);
> > >  };
> > >
> > >  struct dw_pcie_ep {
> > > @@ -238,6 +240,7 @@ struct dw_pcie {
> > >         void __iomem            *dbi_base2;
> > >         /* Used when iatu_unroll_enabled is true */
> > >         void __iomem            *atu_base;
> > > +       void __iomem            *dma_base;
> > >         u32                     num_viewport;
> > >         u8                      iatu_unroll_enabled;
> > >         struct pcie_port        pp;
> > > @@ -323,6 +326,16 @@ static inline u32 dw_pcie_readl_atu(struct dw_pcie
> > > *pci, u32 reg)
> > >         return __dw_pcie_read_dbi(pci, pci->atu_base, reg, 0x4);
> > >  }
> > >
> > > +static inline void dw_pcie_writel_dma(struct dw_pcie *pci, u32 reg, u32
> > > val)
> > > +{
> > > +       __dw_pcie_write_dbi(pci, pci->dma_base, reg, 0x4, val);
> > > +}
> > > +
> > > +static inline u32 dw_pcie_readl_dma(struct dw_pcie *pci, u32 reg)
> > > +{
> > > +       return __dw_pcie_read_dbi(pci, pci->dma_base, reg, 0x4);
> > > +}
> > > +
> > >  static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
> > >  {
> > >         u32 reg;
> > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c
> > > b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > index 27806987e93b..3910073712e9 100644
> > > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > @@ -28,6 +28,25 @@
> > >  #define COMMAND_READ                   BIT(3)
> > >  #define COMMAND_WRITE                  BIT(4)
> > >  #define COMMAND_COPY                   BIT(5)
> > > +#define COMMAND_FLAG2                  BIT(30)
> > > +#define COMMAND_FLAG1                  BIT(31)
> > > +
> > > +#define COMMAND_FLAGS                  (COMMAND_FLAG1 | \
> > > +                                        COMMAND_FLAG2)
> > > +
> > > +#define COMMAND_FLAG_NONE              0
> > > +#define COMMAND_FLAG_DMA               COMMAND_FLAG1
> > > +#define COMMAND_FLAG_DMA_LIST          COMMAND_FLAG2
> > > +
> > > +#define COMMAND_READ_DMA               (COMMAND_READ | \
> > > +                                        COMMAND_FLAG_DMA)
> > > +#define COMMAND_READ_DMA_LIST          (COMMAND_READ_DMA | \
> > > +                                        COMMAND_FLAG_DMA_LIST)
> > > +
> > > +#define COMMAND_WRITE_DMA              (COMMAND_WRITE | \
> > > +                                        COMMAND_FLAG_DMA)
> > > +#define COMMAND_WRITE_DMA_LIST         (COMMAND_WRITE_DMA | \
> > > +                                        COMMAND_FLAG_DMA_LIST)
> > >
> > >  #define STATUS_READ_SUCCESS            BIT(0)
> > >  #define STATUS_READ_FAIL               BIT(1)
> > > @@ -187,6 +206,93 @@ static int pci_epf_test_read(struct pci_epf_test
> > > *epf_test)
> > >         return ret;
> > >  }
> > >
> > > +static int pci_epf_test_read_dma(struct pci_epf_test *epf_test)
> > > +{
> > > +       int ret = -ENOMEM;
> > > +       struct pci_epf *epf = epf_test->epf;
> > > +       struct device *dev = &epf->dev;
> > > +       struct pci_epc *epc = epf->epc;
> > > +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> > > +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> > > +       struct pci_epc_dma dma;
> > > +       u32 crc32;
> > > +       void *buf;
> > > +
> > > +       buf = kzalloc(reg->size, GFP_KERNEL);
> > > +       if (buf) {
> > > +               dma.control = PCI_EPC_DMA_CONTROL_LIE;
> > > +               dma.size = reg->size;
> > > +               dma.sar = reg->src_addr;
> > > +               dma.dar = virt_to_phys(buf);
> > > +
> > > +               ret = pci_epc_dma_read(epc, &dma);
> > > +               if (ret) {
> > > +                       dev_err(dev, "pci_epc_dma_read %d\n", ret);
> > > +               } else {
> > > +                       crc32 = crc32_le(~0, buf, reg->size);
> > > +                       if (crc32 != reg->checksum)
> > > +                               ret = -EIO;
> > > +               }
> > > +
> > > +               kfree(buf);
> > > +       }
> > > +
> > > +       return ret;
> > > +}
> > > +
> > > +static int pci_epf_test_read_dma_list(struct pci_epf_test *epf_test)
> > > +{
> > > +       int ret = -ENOMEM;
> > > +       struct pci_epf *epf = epf_test->epf;
> > > +       struct device *dev = &epf->dev;
> > > +       struct pci_epc *epc = epf->epc;
> > > +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> > > +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> > > +       struct pci_epc_dma *dma;
> > > +       u32 crc32;
> > > +       void *buf;
> > > +
> > > +       dma = kcalloc(3, sizeof(*dma), GFP_KERNEL);
> > > +       if (dma) {
> > > +               buf = kzalloc(reg->size, GFP_KERNEL);
> > > +               if (buf) {
> > > +                       int half_size = reg->size >> 1;
> > > +                       phys_addr_t phys_addr = virt_to_phys(buf);
> > > +
> > > +                       dma[0].control = PCI_EPC_DMA_CONTROL_CB;
> > > +                       dma[0].size = half_size ? half_size : 1;
> > > +                       dma[0].sar = reg->src_addr;
> > > +                       dma[0].dar = phys_addr;
> > > +
> > > +                       dma[1].control = PCI_EPC_DMA_CONTROL_CB |
> > > +                                        PCI_EPC_DMA_CONTROL_LIE;
> > > +                       dma[1].size = reg->size - half_size;
> > > +                       dma[1].sar = reg->src_addr + half_size;
> > > +                       dma[1].dar = phys_addr + half_size;
> > > +
> > > +                       dma[2].control = PCI_EPC_DMA_CONTROL_EOL;
> > > +                       dma[2].size = 0;
> > > +                       dma[2].sar = virt_to_phys(dma);
> > > +                       dma[2].dar = 0;
> > > +
> > > +                       ret = pci_epc_dma_read(epc, dma);
> > > +                       if (ret) {
> > > +                               dev_err(dev, "pci_epc_dma_read %d\n", ret);
> > > +                       } else {
> > > +                               crc32 = crc32_le(~0, buf, reg->size);
> > > +                               if (crc32 != reg->checksum)
> > > +                                       ret = -EIO;
> > > +                       }
> > > +
> > > +                       kfree(buf);
> > > +               }
> > > +
> > > +               kfree(dma);
> > > +       }
> > > +
> > > +       return ret;
> > > +}
> > > +
> > >  static int pci_epf_test_write(struct pci_epf_test *epf_test)
> > >  {
> > >         int ret;
> > > @@ -244,6 +350,87 @@ static int pci_epf_test_write(struct pci_epf_test
> > > *epf_test)
> > >         return ret;
> > >  }
> > >
> > > +static int pci_epf_test_write_dma(struct pci_epf_test *epf_test)
> > > +{
> > > +       int ret = -ENOMEM;
> > > +       struct pci_epf *epf = epf_test->epf;
> > > +       struct device *dev = &epf->dev;
> > > +       struct pci_epc *epc = epf->epc;
> > > +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> > > +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> > > +       struct pci_epc_dma dma;
> > > +       void *buf;
> > > +
> > > +       buf = kzalloc(reg->size, GFP_KERNEL);
> > > +       if (buf) {
> > > +               get_random_bytes(buf, reg->size);
> > > +               reg->checksum = crc32_le(~0, buf, reg->size);
> > > +
> > > +               dma.control = PCI_EPC_DMA_CONTROL_LIE;
> > > +               dma.size = reg->size;
> > > +               dma.sar = virt_to_phys(buf);
> > > +               dma.dar = reg->dst_addr;
> > > +
> > > +               ret = pci_epc_dma_write(epc, &dma);
> > > +               if (ret)
> > > +                       dev_err(dev, "pci_epc_dma_write %d\n", ret);
> > > +
> > > +               kfree(buf);
> > > +       }
> > > +
> > > +       return ret;
> > > +}
> > > +
> > > +static int pci_epf_test_write_dma_list(struct pci_epf_test *epf_test)
> > > +{
> > > +       int ret = -ENOMEM;
> > > +       struct pci_epf *epf = epf_test->epf;
> > > +       struct device *dev = &epf->dev;
> > > +       struct pci_epc *epc = epf->epc;
> > > +       enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> > > +       struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> > > +       struct pci_epc_dma *dma;
> > > +       void *buf;
> > > +
> > > +       dma = kcalloc(3, sizeof(*dma), GFP_KERNEL);
> > > +       if (dma) {
> > > +               buf = kzalloc(reg->size, GFP_KERNEL);
> > > +               if (buf) {
> > > +                       int half_size = reg->size >> 1;
> > > +                       phys_addr_t phys_addr = virt_to_phys(buf);
> > > +
> > > +                       get_random_bytes(buf, reg->size);
> > > +                       reg->checksum = crc32_le(~0, buf, reg->size);
> > > +
> > > +                       dma[0].control = PCI_EPC_DMA_CONTROL_CB;
> > > +                       dma[0].size = half_size ? half_size : 1;
> > > +                       dma[0].sar = phys_addr;
> > > +                       dma[0].dar = reg->dst_addr;
> > > +
> > > +                       dma[1].control = PCI_EPC_DMA_CONTROL_CB |
> > > +                                        PCI_EPC_DMA_CONTROL_LIE;
> > > +                       dma[1].size = reg->size - half_size;
> > > +                       dma[1].sar = phys_addr + half_size;
> > > +                       dma[1].dar = reg->dst_addr + half_size;
> > > +
> > > +                       dma[2].control = PCI_EPC_DMA_CONTROL_EOL;
> > > +                       dma[2].size = 0;
> > > +                       dma[2].sar = virt_to_phys(dma);
> > > +                       dma[2].dar = 0;
> > > +
> > > +                       ret = pci_epc_dma_write(epc, dma);
> > > +                       if (ret)
> > > +                               dev_err(dev, "pci_epc_dma_write %d\n", ret);
> > > +
> > > +                       kfree(buf);
> > > +               }
> > > +
> > > +               kfree(dma);
> > > +       }
> > > +
> > > +       return ret;
> > > +}
> > > +
> > >  static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test, u8
> > > irq_type,
> > >                                    u16 irq)
> > >  {
> > > @@ -303,18 +490,34 @@ static void pci_epf_test_cmd_handler(struct
> > > work_struct *work)
> > >         }
> > >
> > >         if (command & COMMAND_WRITE) {
> > > -               ret = pci_epf_test_write(epf_test);
> > > -               if (ret)
> > > -                       reg->status |= STATUS_WRITE_FAIL;
> > > +               command &= (COMMAND_WRITE | COMMAND_FLAGS);
> > > +               if (command == COMMAND_WRITE)
> > > +                       ret = pci_epf_test_write(epf_test);
> > > +               else if (command == COMMAND_WRITE_DMA)
> > > +                       ret = pci_epf_test_write_dma(epf_test);
> > > +               else if (command == COMMAND_WRITE_DMA_LIST)
> > > +                       ret = pci_epf_test_write_dma_list(epf_test);
> > >                 else
> > > +                       ret = -EINVAL;
> > > +               if (!ret)
> > >                         reg->status |= STATUS_WRITE_SUCCESS;
> > > +               else
> > > +                       reg->status |= STATUS_WRITE_FAIL;
> > >                 pci_epf_test_raise_irq(epf_test, reg->irq_type,
> > >                                        reg->irq_number);
> > >                 goto reset_handler;
> > >         }
> > >
> > >         if (command & COMMAND_READ) {
> > > -               ret = pci_epf_test_read(epf_test);
> > > +               command &= (COMMAND_READ | COMMAND_FLAGS);
> > > +               if (command == COMMAND_READ)
> > > +                       ret = pci_epf_test_read(epf_test);
> > > +               else if (command == COMMAND_READ_DMA)
> > > +                       ret = pci_epf_test_read_dma(epf_test);
> > > +               else if (command == COMMAND_READ_DMA_LIST)
> > > +                       ret = pci_epf_test_read_dma_list(epf_test);
> > > +               else
> > > +                       ret = -EINVAL;
> > >                 if (!ret)
> > >                         reg->status |= STATUS_READ_SUCCESS;
> > >                 else
> > > diff --git a/drivers/pci/endpoint/pci-epc-core.c
> > > b/drivers/pci/endpoint/pci-epc-core.c
> > > index e4712a0f249c..a57e501d4abc 100644
> > > --- a/drivers/pci/endpoint/pci-epc-core.c
> > > +++ b/drivers/pci/endpoint/pci-epc-core.c
> > > @@ -107,6 +107,52 @@ unsigned int pci_epc_get_first_free_bar(const struct
> > > pci_epc_features
> > >  EXPORT_SYMBOL_GPL(pci_epc_get_first_free_bar);
> > >
> > >  /**
> > > + * pci_epc_dma_write() - DMA a block of memory to remote address
> > > + * @epc: the EPC device on which to perform DMA transfer
> > > + * @dma: DMA descriptors array
> > > + *
> > > + * Write contents of local memory to remote memory by DMA.
> > > + */
> > > +int pci_epc_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma)
> > > +{
> > > +       int ret;
> > > +       unsigned long flags;
> > > +
> > > +       if (IS_ERR(epc) || !epc->ops->dma_write)
> > > +               return -EINVAL;
> > > +
> > > +       spin_lock_irqsave(&epc->lock, flags);
> > > +       ret = epc->ops->dma_write(epc, dma);
> > > +       spin_unlock_irqrestore(&epc->lock, flags);
> > > +
> > > +       return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_epc_dma_write);
> > > +
> > > +/**
> > > + * pci_epc_dma_read() - DMA a block of memory from remote address
> > > + * @epc: the EPC device on which to perform DMA transfer
> > > + * @dma: DMA descriptors array
> > > + *
> > > + * Read contents of remote memory into local memory by DMA.
> > > + */
> > > +int pci_epc_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma)
> > > +{
> > > +       int ret;
> > > +       unsigned long flags;
> > > +
> > > +       if (IS_ERR(epc) || !epc->ops->dma_read)
> > > +               return -EINVAL;
> > > +
> > > +       spin_lock_irqsave(&epc->lock, flags);
> > > +       ret = epc->ops->dma_read(epc, dma);
> > > +       spin_unlock_irqrestore(&epc->lock, flags);
> > > +
> > > +       return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_epc_dma_read);
> > > +
> > > +/**
> > >   * pci_epc_get_features() - get the features supported by EPC
> > >   * @epc: the features supported by *this* EPC device will be returned
> > >   * @func_no: the features supported by the EPC device specific to the
> > > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> > > index f641badc2c61..d845f13d0baf 100644
> > > --- a/include/linux/pci-epc.h
> > > +++ b/include/linux/pci-epc.h
> > > @@ -21,6 +21,45 @@ enum pci_epc_irq_type {
> > >  };
> > >
> > >  /**
> > > + * struct pci_epc_dma - descriptor for a DMA transfer element
> > > + * @control: DMA channel control bits for read or write transfer
> > > + * @size: size of DMA transfer element
> > > + * @sar: source addrees for DMA transfer element
> > > + * @dar: destination address for DMA transfer element
> > > + */
> > > +
> > > +struct pci_epc_dma {
> > > +       u32     control;
> > > +       u32     size;
> > > +       u64     sar;
> > > +       u64     dar;
> > > +};
> > > +
> > > +#define PCI_EPC_DMA_CONTROL_CB          (BIT(0))
> > > +#define PCI_EPC_DMA_CONTROL_TCB         (BIT(1))
> > > +#define PCI_EPC_DMA_CONTROL_LLP         (BIT(2))
> > > +#define PCI_EPC_DMA_CONTROL_LIE         (BIT(3))
> > > +#define PCI_EPC_DMA_CONTROL_RIE         (BIT(4))
> > > +#define PCI_EPC_DMA_CONTROL_CS          (BIT(5) | BIT(6))
> > > +#define PCI_EPC_DMA_CONTROL_CCS         (BIT(8))
> > > +#define PCI_EPC_DMA_CONTROL_LLE         (BIT(9))
> > > +#define PCI_EPC_DMA_CONTROL_FUNC        (BIT(12) | BIT(13) | BIT(14) | \
> > > +                                        BIT(15) | BIT(16))
> > > +#define PCI_EPC_DMA_CONTROL_NS_DST      (BIT(23))
> > > +#define PCI_EPC_DMA_CONTROL_NS_SRC      (BIT(24))
> > > +#define PCI_EPC_DMA_CONTROL_RO          (BIT(25))
> > > +#define PCI_EPC_DMA_CONTROL_TC          (BIT(27) | BIT(28) | BIT(29))
> > > +#define PCI_EPC_DMA_CONTROL_AT          (BIT(30) | BIT(31))
> > > +
> > > +#define PCI_EPC_DMA_CONTROL_EOL         (PCI_EPC_DMA_CONTROL_TCB | \
> > > +                                        PCI_EPC_DMA_CONTROL_LLP)
> > > +
> > > +#define PCI_EPC_DMA_CONTROL_LIST        (PCI_EPC_DMA_CONTROL_CB | \
> > > +                                        PCI_EPC_DMA_CONTROL_EOL| \
> > > +                                        PCI_EPC_DMA_CONTROL_CCS | \
> > > +                                        PCI_EPC_DMA_CONTROL_LLE)
> > > +
> > > +/**
> > >   * struct pci_epc_ops - set of function pointers for performing EPC
> > > operations
> > >   * @write_header: ops to populate configuration space header
> > >   * @set_bar: ops to configure the BAR
> > > @@ -38,6 +77,8 @@ enum pci_epc_irq_type {
> > >   * @raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
> > >   * @start: ops to start the PCI link
> > >   * @stop: ops to stop the PCI link
> > > + * @dma_read: ops to read remote memory into local memory by DMA
> > > + * @dma_write: ops to write local memory to remote memory by DMA
> > >   * @owner: the module owner containing the ops
> > >   */
> > >  struct pci_epc_ops {
> > > @@ -61,6 +102,8 @@ struct pci_epc_ops {
> > >         void    (*stop)(struct pci_epc *epc);
> > >         const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
> > >                                                        u8 func_no);
> > > +       int     (*dma_read)(struct pci_epc *epc, struct pci_epc_dma *dma);
> > > +       int     (*dma_write)(struct pci_epc *epc, struct pci_epc_dma *dma);
> > >         struct module *owner;
> > >  };
> > >
> > > @@ -152,6 +195,8 @@ void pci_epc_destroy(struct pci_epc *epc);
> > >  int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf);
> > >  void pci_epc_linkup(struct pci_epc *epc);
> > >  void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf);
> > > +int pci_epc_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma);
> > > +int pci_epc_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma);
> > >  int pci_epc_write_header(struct pci_epc *epc, u8 func_no,
> > >                          struct pci_epf_header *hdr);
> > >  int pci_epc_set_bar(struct pci_epc *epc, u8 func_no,
> > > diff --git a/include/uapi/linux/pcitest.h b/include/uapi/linux/pcitest.h
> > > index cbf422e56696..505f4a3811c2 100644
> > > --- a/include/uapi/linux/pcitest.h
> > > +++ b/include/uapi/linux/pcitest.h
> > > @@ -10,11 +10,18 @@
> > >  #ifndef __UAPI_LINUX_PCITEST_H
> > >  #define __UAPI_LINUX_PCITEST_H
> > >
> > > +struct pcitest_dma {
> > > +       size_t  size;
> > > +       bool    list;
> > > +};
> > > +
> > >  #define PCITEST_BAR            _IO('P', 0x1)
> > >  #define PCITEST_LEGACY_IRQ     _IO('P', 0x2)
> > >  #define PCITEST_MSI            _IOW('P', 0x3, int)
> > >  #define PCITEST_WRITE          _IOW('P', 0x4, unsigned long)
> > > +#define PCITEST_WRITE_DMA      _IOW('P', 0x4, struct pcitest_dma)
> > >  #define PCITEST_READ           _IOW('P', 0x5, unsigned long)
> > > +#define PCITEST_READ_DMA       _IOW('P', 0x5, struct pcitest_dma)
> > >  #define PCITEST_COPY           _IOW('P', 0x6, unsigned long)
> > >  #define PCITEST_MSIX           _IOW('P', 0x7, int)
> > >  #define PCITEST_SET_IRQTYPE    _IOW('P', 0x8, int)
> > > diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
> > > index 6f1303104d84..66cd19acf18c 100644
> > > --- a/tools/pci/pcitest.c
> > > +++ b/tools/pci/pcitest.c
> > > @@ -44,11 +44,14 @@ struct pci_test {
> > >         bool            read;
> > >         bool            write;
> > >         bool            copy;
> > > +       bool            dma;
> > > +       bool            dma_list;
> > >         unsigned long   size;
> > >  };
> > >
> > >  static int run_test(struct pci_test *test)
> > >  {
> > > +       struct pcitest_dma dma;
> > >         int ret = -EINVAL;
> > >         int fd;
> > >
> > > @@ -113,7 +116,13 @@ static int run_test(struct pci_test *test)
> > >         }
> > >
> > >         if (test->write) {
> > > -               ret = ioctl(fd, PCITEST_WRITE, test->size);
> > > +               if (test->dma) {
> > > +                       dma.size = test->size;
> > > +                       dma.list = test->dma_list;
> > > +                       ret = ioctl(fd, PCITEST_WRITE_DMA, &dma);
> > > +               } else {
> > > +                       ret = ioctl(fd, PCITEST_WRITE, test->size);
> > > +               }
> > >                 fprintf(stdout, "WRITE (%7ld bytes):\t\t", test->size);
> > >                 if (ret < 0)
> > >                         fprintf(stdout, "TEST FAILED\n");
> > > @@ -122,7 +131,13 @@ static int run_test(struct pci_test *test)
> > >         }
> > >
> > >         if (test->read) {
> > > -               ret = ioctl(fd, PCITEST_READ, test->size);
> > > +               if (test->dma) {
> > > +                       dma.size = test->size;
> > > +                       dma.list = test->dma_list;
> > > +                       ret = ioctl(fd, PCITEST_READ_DMA, &dma);
> > > +               } else {
> > > +                       ret = ioctl(fd, PCITEST_READ, test->size);
> > > +               }
> > >                 fprintf(stdout, "READ (%7ld bytes):\t\t", test->size);
> > >                 if (ret < 0)
> > >                         fprintf(stdout, "TEST FAILED\n");
> > > @@ -163,7 +178,7 @@ int main(int argc, char **argv)
> > >         /* set default endpoint device */
> > >         test->device = "/dev/pci-endpoint-test.0";
> > >
> > > -       while ((c = getopt(argc, argv, "D:b:m:x:i:Ilhrwcs:")) != EOF)
> > > +       while ((c = getopt(argc, argv, "D:b:m:x:i:IlhrwcdLs:")) != EOF)
> > >         switch (c) {
> > >         case 'D':
> > >                 test->device = optarg;
> > > @@ -204,6 +219,12 @@ int main(int argc, char **argv)
> > >         case 'c':
> > >                 test->copy = true;
> > >                 continue;
> > > +       case 'd':
> > > +               test->dma = true;
> > > +               continue;
> > > +       case 'L':
> > > +               test->dma_list = true;
> > > +               continue;
> > >         case 's':
> > >                 test->size = strtoul(optarg, NULL, 0);
> > >                 continue;
> > > @@ -223,6 +244,8 @@ int main(int argc, char **argv)
> > >                         "\t-r                   Read buffer test\n"
> > >                         "\t-w                   Write buffer test\n"
> > >                         "\t-c                   Copy buffer test\n"
> > > +                       "\t-d                   DMA mode for read or write test\n"
> > > +                       "\t-L                   Linked-List DMA flag for DMA mode\n"
> > >                         "\t-s <size>            Size of buffer {default: 100KB}\n"
> > >                         "\t-h                   Print this help message\n",
> > >                         argv[0]);
> > > --
> > > 2.7.4
> > >
>
>

Kishon Vijay Abraham I May 30, 2019, 5:46 a.m. UTC | #5

+Vinod Koul

Hi,

On 30/05/19 4:07 AM, Alan Mikhak wrote:
> On Mon, May 27, 2019 at 2:09 AM Gustavo Pimentel
> <[e-mail]> wrote:
>>
>> On Fri, May 24, 2019 at 20:42:43, Alan Mikhak <[e-mail]>
>> wrote:
>>
>> Hi Alan,
>>
>>> On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
>>> <[e-mail]> wrote:
>>>>
>>>> Hi Alan,
>>>>
>>>> This patch implementation is very HW implementation dependent and
>>>> requires the DMA to exposed through PCIe BARs, which aren't always the
>>>> case. Besides, you are defining some control bits on
>>>> include/linux/pci-epc.h that may not have any meaning to other types of
>>>> DMA.
>>>>
>>>> I don't think this was what Kishon had in mind when he developed the
>>>> pcitest, but let see what Kishon was to say about it.
>>>>
>>>> I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
>>>> and which I submitted some days ago.
>>>> By having a DMA driver which implemented using DMAengine API, means the
>>>> pcitest can use the DMAengine client API, which will be completely
>>>> generic to any other DMA implementation.

right, my initial thought process was to use only dmaengine APIs in
pci-epf-test so that the system DMA or DMA within the PCIe controller can be
used transparently. But can we register DMA within the PCIe controller to the
DMA subsystem? AFAIK only system DMA should register with the DMA subsystem.
(ADMA in SDHCI doesn't use dmaengine). Vinod Koul can confirm.

If DMA within the PCIe controller cannot be registered in DMA subsystem, we
should use something like what Alan has done in this patch with dma_read ops.
The dma_read ops implementation in the EP controller can either use dmaengine
APIs or use the DMA within the PCIe controller.

I'll review the patch separately.

Thanks
Kishon

Alan Mikhak May 30, 2019, 5:56 p.m. UTC | #6

On Wed, May 29, 2019 at 10:48 PM Kishon Vijay Abraham I <[e-mail]> wrote:
>
> +Vinod Koul
>
> Hi,
>
> >>> On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
> >>> <[e-mail]> wrote:
> >>>>
> >>>> Hi Alan,
> >>>>
> >>>> This patch implementation is very HW implementation dependent and
> >>>> requires the DMA to exposed through PCIe BARs, which aren't always the
> >>>> case. Besides, you are defining some control bits on
> >>>> include/linux/pci-epc.h that may not have any meaning to other types of
> >>>> DMA.
> >>>>
> >>>> I don't think this was what Kishon had in mind when he developed the
> >>>> pcitest, but let see what Kishon was to say about it.
> >>>>
> >>>> I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
> >>>> and which I submitted some days ago.
> >>>> By having a DMA driver which implemented using DMAengine API, means the
> >>>> pcitest can use the DMAengine client API, which will be completely
> >>>> generic to any other DMA implementation.
>
> right, my initial thought process was to use only dmaengine APIs in
> pci-epf-test so that the system DMA or DMA within the PCIe controller can be
> used transparently. But can we register DMA within the PCIe controller to the
> DMA subsystem? AFAIK only system DMA should register with the DMA subsystem.
> (ADMA in SDHCI doesn't use dmaengine). Vinod Koul can confirm.
>
> If DMA within the PCIe controller cannot be registered in DMA subsystem, we
> should use something like what Alan has done in this patch with dma_read ops.
> The dma_read ops implementation in the EP controller can either use dmaengine
> APIs or use the DMA within the PCIe controller.
>
> I'll review the patch separately.
>
> Thanks
> Kishon

Hi Kishon,

I have some improvements in mind for a v2 patch in response to
feedback from Gustavo Pimentel that the current implementation is HW
specific. I hesitate from submitting a v2 patch because it seems best
to seek comment on possible directions this may be taking.

One alternative is to wait for or modify test functions in
pci-epf-test.c to call DMAengine client APIs, if possible. I imagine
pci-epf-test.c test functions would still allocate the necessary local
buffer on the endpoint side for the same canned tests for everyone to
use. They would prepare the buffer in the existing manner by filling
it with random bytes and calculate CRC in the case of a write test.
However, they would then initiate DMA operations by using DMAengine
client APIs in a generic way instead of calling memcpy_toio() and
memcpy_fromio(). They would post-process the buffer in the existing
manner such as the checking for CRC in the case of a read test.
Finally, they would release the resources and report results back to
the user of pcitest across the PCIe bus through the existing methods.

Another alternative I have in mind for v2 is to change the struct
pci_epc_dma that this patch added to pci-epc.h from the following:

struct pci_epc_dma {
        u32     control;
        u32     size;
        u64     sar;
        u64     dar;
};

to something similar to the following:

struct pci_epc_dma {
        size_t  size;
        void *buffer;
        int flags;
};

The 'flags' field can be a bit field or separate boolean values to
specify such things as linked-list mode vs single-block, etc.
Associated #defines would be removed from pci-epc.h to be replaced if
needed with something generic. The 'size' field specifies the size of
DMA transfer that can fit in the buffer.

That way the dma test functions in pci-epf-test.c can simply kmalloc
and prepare a local buffer on the endpoint side for the DMA transfer
and pass its pointer down the stack using the 'buffer' field to lower
layers. This would allow different PCIe controller drivers to
implement DMA or not according to their needs. Each implementer can
decide to use DMAengine client API, which would be preferable, or
directly read or write to DMA hardware registers to suit their needs.

I would appreciate feedback and comment on such choices as part of this review.

Regards,
Alan Mikhak

Kishon Vijay Abraham I May 31, 2019, 5:06 a.m. UTC | #7

Hi Alan,

On 30/05/19 11:26 PM, Alan Mikhak wrote:
> On Wed, May 29, 2019 at 10:48 PM Kishon Vijay Abraham I <[e-mail]> wrote:
>>
>> +Vinod Koul
>>
>> Hi,
>>
>>>>> On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
>>>>> <[e-mail]> wrote:
>>>>>>
>>>>>> Hi Alan,
>>>>>>
>>>>>> This patch implementation is very HW implementation dependent and
>>>>>> requires the DMA to exposed through PCIe BARs, which aren't always the
>>>>>> case. Besides, you are defining some control bits on
>>>>>> include/linux/pci-epc.h that may not have any meaning to other types of
>>>>>> DMA.
>>>>>>
>>>>>> I don't think this was what Kishon had in mind when he developed the
>>>>>> pcitest, but let see what Kishon was to say about it.
>>>>>>
>>>>>> I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
>>>>>> and which I submitted some days ago.
>>>>>> By having a DMA driver which implemented using DMAengine API, means the
>>>>>> pcitest can use the DMAengine client API, which will be completely
>>>>>> generic to any other DMA implementation.
>>
>> right, my initial thought process was to use only dmaengine APIs in
>> pci-epf-test so that the system DMA or DMA within the PCIe controller can be
>> used transparently. But can we register DMA within the PCIe controller to the
>> DMA subsystem? AFAIK only system DMA should register with the DMA subsystem.
>> (ADMA in SDHCI doesn't use dmaengine). Vinod Koul can confirm.
>>
>> If DMA within the PCIe controller cannot be registered in DMA subsystem, we
>> should use something like what Alan has done in this patch with dma_read ops.
>> The dma_read ops implementation in the EP controller can either use dmaengine
>> APIs or use the DMA within the PCIe controller.
>>
>> I'll review the patch separately.
>>
>> Thanks
>> Kishon
> 
> Hi Kishon,
> 
> I have some improvements in mind for a v2 patch in response to
> feedback from Gustavo Pimentel that the current implementation is HW
> specific. I hesitate from submitting a v2 patch because it seems best
> to seek comment on possible directions this may be taking.
> 
> One alternative is to wait for or modify test functions in
> pci-epf-test.c to call DMAengine client APIs, if possible. I imagine
> pci-epf-test.c test functions would still allocate the necessary local
> buffer on the endpoint side for the same canned tests for everyone to
> use. They would prepare the buffer in the existing manner by filling
> it with random bytes and calculate CRC in the case of a write test.
> However, they would then initiate DMA operations by using DMAengine
> client APIs in a generic way instead of calling memcpy_toio() and
> memcpy_fromio(). They would post-process the buffer in the existing

No, you can't remove memcpy_toio/memcpy_fromio APIs. There could be platforms
without system DMA or they could have system DMA but without MEMCOPY channels
or without DMA in their PCI controller.
> manner such as the checking for CRC in the case of a read test.
> Finally, they would release the resources and report results back to
> the user of pcitest across the PCIe bus through the existing methods.
> 
> Another alternative I have in mind for v2 is to change the struct
> pci_epc_dma that this patch added to pci-epc.h from the following:
> 
> struct pci_epc_dma {
>         u32     control;
>         u32     size;
>         u64     sar;
>         u64     dar;
> };
> 
> to something similar to the following:
> 
> struct pci_epc_dma {
>         size_t  size;
>         void *buffer;
>         int flags;
> };
> 
> The 'flags' field can be a bit field or separate boolean values to
> specify such things as linked-list mode vs single-block, etc.
> Associated #defines would be removed from pci-epc.h to be replaced if
> needed with something generic. The 'size' field specifies the size of
> DMA transfer that can fit in the buffer.

I still have to look closer into your DMA patch but linked-list mode or single
block mode shouldn't be an user select-able option but should be determined by
the size of transfer.
> 
> That way the dma test functions in pci-epf-test.c can simply kmalloc
> and prepare a local buffer on the endpoint side for the DMA transfer
> and pass its pointer down the stack using the 'buffer' field to lower
> layers. This would allow different PCIe controller drivers to
> implement DMA or not according to their needs. Each implementer can
> decide to use DMAengine client API, which would be preferable, or
> directly read or write to DMA hardware registers to suit their needs.

yes, that would be my preferred method as well. In fact I had implemented
pci_epf_tx() in [1], as a way for pci-epf-test to pass buffer address to
endpoint controller driver. I had also implemented helpers for platforms using
system DMA (i.e uses DMAengine).

Thanks
Kishon

[1] ->
http://git.ti.com/cgit/cgit.cgi/ti-linux-kernel/ti-linux-kernel.git/tree/drivers/pci/endpoint/pci-epf-core.c?h=ti-linux-4.19.y
> 
> I would appreciate feedback and comment on such choices as part of this review.
> 
> Regards,
> Alan Mikhak
>

Vinod Koul May 31, 2019, 5:07 a.m. UTC | #8

Hi Kishon,

On 30-05-19, 11:16, Kishon Vijay Abraham I wrote:
> +Vinod Koul
> 
> Hi,
> 
> On 30/05/19 4:07 AM, Alan Mikhak wrote:
> > On Mon, May 27, 2019 at 2:09 AM Gustavo Pimentel
> > <[e-mail]> wrote:
> >>
> >> On Fri, May 24, 2019 at 20:42:43, Alan Mikhak <[e-mail]>
> >> wrote:
> >>
> >> Hi Alan,
> >>
> >>> On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
> >>> <[e-mail]> wrote:
> >>>>
> >>>> Hi Alan,
> >>>>
> >>>> This patch implementation is very HW implementation dependent and
> >>>> requires the DMA to exposed through PCIe BARs, which aren't always the
> >>>> case. Besides, you are defining some control bits on
> >>>> include/linux/pci-epc.h that may not have any meaning to other types of
> >>>> DMA.
> >>>>
> >>>> I don't think this was what Kishon had in mind when he developed the
> >>>> pcitest, but let see what Kishon was to say about it.
> >>>>
> >>>> I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
> >>>> and which I submitted some days ago.
> >>>> By having a DMA driver which implemented using DMAengine API, means the
> >>>> pcitest can use the DMAengine client API, which will be completely
> >>>> generic to any other DMA implementation.
> 
> right, my initial thought process was to use only dmaengine APIs in
> pci-epf-test so that the system DMA or DMA within the PCIe controller can be
> used transparently. But can we register DMA within the PCIe controller to the
> DMA subsystem? AFAIK only system DMA should register with the DMA subsystem.
> (ADMA in SDHCI doesn't use dmaengine). Vinod Koul can confirm.

So would this DMA be dedicated for PCI and all PCI devices on the bus?
If so I do not see a reason why this cannot be using dmaengine. The use
case would be memcpy for DMA right or mem to device (vice versa) transfers?

Btw many driver in sdhci do use dmaengine APIs and yes we are missing
support in framework than individual drivers

> If DMA within the PCIe controller cannot be registered in DMA subsystem, we
> should use something like what Alan has done in this patch with dma_read ops.
> The dma_read ops implementation in the EP controller can either use dmaengine
> APIs or use the DMA within the PCIe controller.
> 
> I'll review the patch separately.
> 
> Thanks
> Kishon

Kishon Vijay Abraham I May 31, 2019, 5:20 a.m. UTC | #9

Hi Vinod,

On 31/05/19 10:37 AM, Vinod Koul wrote:
> Hi Kishon,
> 
> On 30-05-19, 11:16, Kishon Vijay Abraham I wrote:
>> +Vinod Koul
>>
>> Hi,
>>
>> On 30/05/19 4:07 AM, Alan Mikhak wrote:
>>> On Mon, May 27, 2019 at 2:09 AM Gustavo Pimentel
>>> <[e-mail]> wrote:
>>>>
>>>> On Fri, May 24, 2019 at 20:42:43, Alan Mikhak <[e-mail]>
>>>> wrote:
>>>>
>>>> Hi Alan,
>>>>
>>>>> On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
>>>>> <[e-mail]> wrote:
>>>>>>
>>>>>> Hi Alan,
>>>>>>
>>>>>> This patch implementation is very HW implementation dependent and
>>>>>> requires the DMA to exposed through PCIe BARs, which aren't always the
>>>>>> case. Besides, you are defining some control bits on
>>>>>> include/linux/pci-epc.h that may not have any meaning to other types of
>>>>>> DMA.
>>>>>>
>>>>>> I don't think this was what Kishon had in mind when he developed the
>>>>>> pcitest, but let see what Kishon was to say about it.
>>>>>>
>>>>>> I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
>>>>>> and which I submitted some days ago.
>>>>>> By having a DMA driver which implemented using DMAengine API, means the
>>>>>> pcitest can use the DMAengine client API, which will be completely
>>>>>> generic to any other DMA implementation.
>>
>> right, my initial thought process was to use only dmaengine APIs in
>> pci-epf-test so that the system DMA or DMA within the PCIe controller can be
>> used transparently. But can we register DMA within the PCIe controller to the
>> DMA subsystem? AFAIK only system DMA should register with the DMA subsystem.
>> (ADMA in SDHCI doesn't use dmaengine). Vinod Koul can confirm.
> 
> So would this DMA be dedicated for PCI and all PCI devices on the bus?

Yes, this DMA will be used only by PCI ($patch is w.r.t PCIe device mode. So
all endpoint functions both physical and virtual functions will use the DMA in
the controller).
> If so I do not see a reason why this cannot be using dmaengine. The use

Thanks for clarifying. I was under the impression any DMA within a peripheral
controller shouldn't use DMAengine.
> case would be memcpy for DMA right or mem to device (vice versa) transfers?

The device is memory mapped so it would be only memcopy.
> 
> Btw many driver in sdhci do use dmaengine APIs and yes we are missing
> support in framework than individual drivers

I think dmaengine APIs is used only when the platform uses system DMA and not
ADMA within the SDHCI controller. IOW there is no dma_async_device_register()
to register ADMA in SDHCI with DMA subsystem.

Thanks
Kishon

Vinod Koul May 31, 2019, 6:32 a.m. UTC | #10

On 31-05-19, 10:50, Kishon Vijay Abraham I wrote:
> Hi Vinod,
> 
> On 31/05/19 10:37 AM, Vinod Koul wrote:
> > Hi Kishon,
> > 
> > On 30-05-19, 11:16, Kishon Vijay Abraham I wrote:
> >> +Vinod Koul
> >>
> >> Hi,
> >>
> >> On 30/05/19 4:07 AM, Alan Mikhak wrote:
> >>> On Mon, May 27, 2019 at 2:09 AM Gustavo Pimentel
> >>> <[e-mail]> wrote:
> >>>>
> >>>> On Fri, May 24, 2019 at 20:42:43, Alan Mikhak <[e-mail]>
> >>>> wrote:
> >>>>
> >>>> Hi Alan,
> >>>>
> >>>>> On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
> >>>>> <[e-mail]> wrote:
> >>>>>>
> >>>>>> Hi Alan,
> >>>>>>
> >>>>>> This patch implementation is very HW implementation dependent and
> >>>>>> requires the DMA to exposed through PCIe BARs, which aren't always the
> >>>>>> case. Besides, you are defining some control bits on
> >>>>>> include/linux/pci-epc.h that may not have any meaning to other types of
> >>>>>> DMA.
> >>>>>>
> >>>>>> I don't think this was what Kishon had in mind when he developed the
> >>>>>> pcitest, but let see what Kishon was to say about it.
> >>>>>>
> >>>>>> I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
> >>>>>> and which I submitted some days ago.
> >>>>>> By having a DMA driver which implemented using DMAengine API, means the
> >>>>>> pcitest can use the DMAengine client API, which will be completely
> >>>>>> generic to any other DMA implementation.
> >>
> >> right, my initial thought process was to use only dmaengine APIs in
> >> pci-epf-test so that the system DMA or DMA within the PCIe controller can be
> >> used transparently. But can we register DMA within the PCIe controller to the
> >> DMA subsystem? AFAIK only system DMA should register with the DMA subsystem.
> >> (ADMA in SDHCI doesn't use dmaengine). Vinod Koul can confirm.
> > 
> > So would this DMA be dedicated for PCI and all PCI devices on the bus?
> 
> Yes, this DMA will be used only by PCI ($patch is w.r.t PCIe device mode. So
> all endpoint functions both physical and virtual functions will use the DMA in
> the controller).
> > If so I do not see a reason why this cannot be using dmaengine. The use
> 
> Thanks for clarifying. I was under the impression any DMA within a peripheral
> controller shouldn't use DMAengine.

That is indeed a correct assumption. The dmaengine helps in cases where
we have a dma controller with multiple users, for a single user case it
might be overhead to setup dma driver and then use it thru framework.

Someone needs to see the benefit and cost of using the framework and
decide.

> > case would be memcpy for DMA right or mem to device (vice versa) transfers?
> 
> The device is memory mapped so it would be only memcopy.
> > 
> > Btw many driver in sdhci do use dmaengine APIs and yes we are missing
> > support in framework than individual drivers
> 
> I think dmaengine APIs is used only when the platform uses system DMA and not
> ADMA within the SDHCI controller. IOW there is no dma_async_device_register()
> to register ADMA in SDHCI with DMA subsystem.

We are looking it from the different point of view. You are looking for
dmaengine drivers in that (which would be in drivers/dma/) and I am
pointing to users of dmaengine in that.

So the users in mmc would be ones using dmaengine APIs:
$git grep -l dmaengine_prep_* drivers/mmc/

which tells me 17 drivers!

HTH

Arnd Bergmann May 31, 2019, 7:49 a.m. UTC | #11

On Fri, May 31, 2019 at 8:32 AM Vinod Koul <[e-mail]> wrote:
> On 31-05-19, 10:50, Kishon Vijay Abraham I wrote:
> > On 31/05/19 10:37 AM, Vinod Koul wrote:
> > > On 30-05-19, 11:16, Kishon Vijay Abraham I wrote:
> > >>
> > >> right, my initial thought process was to use only dmaengine APIs in
> > >> pci-epf-test so that the system DMA or DMA within the PCIe controller can be
> > >> used transparently. But can we register DMA within the PCIe controller to the
> > >> DMA subsystem? AFAIK only system DMA should register with the DMA subsystem.
> > >> (ADMA in SDHCI doesn't use dmaengine). Vinod Koul can confirm.
> > >
> > > So would this DMA be dedicated for PCI and all PCI devices on the bus?
> >
> > Yes, this DMA will be used only by PCI ($patch is w.r.t PCIe device mode. So
> > all endpoint functions both physical and virtual functions will use the DMA in
> > the controller).
> > > If so I do not see a reason why this cannot be using dmaengine. The use
> >
> > Thanks for clarifying. I was under the impression any DMA within a peripheral
> > controller shouldn't use DMAengine.
>
> That is indeed a correct assumption. The dmaengine helps in cases where
> we have a dma controller with multiple users, for a single user case it
> might be overhead to setup dma driver and then use it thru framework.
>
> Someone needs to see the benefit and cost of using the framework and
> decide.

I think the main question is about how generalized we want this to be.
There are lots of difference PCIe endpoint implementations, and in
case of some licensable IP cores like the designware PCIe there are
many variants, as each SoC will do the implementation in a slightly
different way.

If we can have a single endpoint driver than can either have an
integrated DMA engine or use an external one, then abstracting that
DMA engine helps make the driver work more readily either way.

Similarly, there may be PCIe endpoint implementations that have
a dedicated DMA engine in them that is not usable for anything else,
but that is closely related to an IP core we already have a dmaengine
driver for. In this case, we can avoid duplication.

      Arnd

Alan Mikhak May 31, 2019, 6:16 p.m. UTC | #12

On Thu, May 30, 2019 at 10:08 PM Kishon Vijay Abraham I <[e-mail]> wrote:
> Hi Alan,
> >
> > Hi Kishon,
> >
> > I have some improvements in mind for a v2 patch in response to
> > feedback from Gustavo Pimentel that the current implementation is HW
> > specific. I hesitate from submitting a v2 patch because it seems best
> > to seek comment on possible directions this may be taking.
> >
> > One alternative is to wait for or modify test functions in
> > pci-epf-test.c to call DMAengine client APIs, if possible. I imagine
> > pci-epf-test.c test functions would still allocate the necessary local
> > buffer on the endpoint side for the same canned tests for everyone to
> > use. They would prepare the buffer in the existing manner by filling
> > it with random bytes and calculate CRC in the case of a write test.
> > However, they would then initiate DMA operations by using DMAengine
> > client APIs in a generic way instead of calling memcpy_toio() and
> > memcpy_fromio(). They would post-process the buffer in the existing
>
> No, you can't remove memcpy_toio/memcpy_fromio APIs. There could be platforms
> without system DMA or they could have system DMA but without MEMCOPY channels
> or without DMA in their PCI controller.

I agree. I wouldn't remove memcpy_toio/fromio. That is the reason this
patch introduces the '-d' flag for pcitest to communicate that user
intent across the PCIe bus to pci-epf-test so the endpoint can
initiate the transfer using either memcpy_toio/fromio or DMA.

> > manner such as the checking for CRC in the case of a read test.
> > Finally, they would release the resources and report results back to
> > the user of pcitest across the PCIe bus through the existing methods.
> >
> > Another alternative I have in mind for v2 is to change the struct
> > pci_epc_dma that this patch added to pci-epc.h from the following:
> >
> > struct pci_epc_dma {
> >         u32     control;
> >         u32     size;
> >         u64     sar;
> >         u64     dar;
> > };
> >
> > to something similar to the following:
> >
> > struct pci_epc_dma {
> >         size_t  size;
> >         void *buffer;
> >         int flags;
> > };
> >
> > The 'flags' field can be a bit field or separate boolean values to
> > specify such things as linked-list mode vs single-block, etc.
> > Associated #defines would be removed from pci-epc.h to be replaced if
> > needed with something generic. The 'size' field specifies the size of
> > DMA transfer that can fit in the buffer.
>
> I still have to look closer into your DMA patch but linked-list mode or single
> block mode shouldn't be an user select-able option but should be determined by
> the size of transfer.

Please consider the following when taking a closer look at this patch.

In my specific use case, I need to verify that any valid block size,
including a one byte transfer, can be transferred across the PCIe bus
by memcpy_toio/fromio() or by DMA either as a single block or as
linked-list. That is why, instead of deciding based on transfer size,
this patch introduces the '-L' flag for pcitest to communicate the
user intent across the PCIe bus to pci-epf-test so the endpoint can
initiate the DMA transfer using a single block or in linked-list mode.

When user issues 'pcitest -r' to perform a read buffer test,
pci-epf-test calls pci_epf_test_write() which uses memcpy_toio(). As
before, a read from the user point of view is a write from the
endpoint point of view.
When user issues 'pcitest -r -d', pci-epf-test calls a new function
pci_epf_test_write_dma() to initiate a single block DMA transfer.
When user issues 'pcitest -r -d -L', pci-epf-test calls a new function
pci_epf_test_write_dma_list() to initiate a linked-list DMA transfer.

The '-d' and '-L' flags also apply to the '-w' flag when the user
performs a write buffer test. The user can specify any valid transfer
size for any of the above examples using the '-s' flag as before.

> > That way the dma test functions in pci-epf-test.c can simply kmalloc
> > and prepare a local buffer on the endpoint side for the DMA transfer
> > and pass its pointer down the stack using the 'buffer' field to lower
> > layers. This would allow different PCIe controller drivers to
> > implement DMA or not according to their needs. Each implementer can
> > decide to use DMAengine client API, which would be preferable, or
> > directly read or write to DMA hardware registers to suit their needs.
>
> yes, that would be my preferred method as well. In fact I had implemented
> pci_epf_tx() in [1], as a way for pci-epf-test to pass buffer address to
> endpoint controller driver. I had also implemented helpers for platforms using
> system DMA (i.e uses DMAengine).
>
> Thanks
> Kishon
>
> [1] ->
> http://git.ti.com/cgit/cgit.cgi/ti-linux-kernel/ti-linux-kernel.git/tree/drivers/pci/endpoint/pci-epf-core.c?h=ti-linux-4.19.y
> >
> > I would appreciate feedback and comment on such choices as part of this review.

Thanks for all your comments and providing the link to your
implementation of pci_epf_tx() in [1] above. It clarifies a lot and
provides a very useful reference.

Regards,
Alan Mikhak

Kishon Vijay Abraham I June 3, 2019, 4:24 a.m. UTC | #13

Hi Vinod,

On 31/05/19 12:02 PM, Vinod Koul wrote:
> On 31-05-19, 10:50, Kishon Vijay Abraham I wrote:
>> Hi Vinod,
>>
>> On 31/05/19 10:37 AM, Vinod Koul wrote:
>>> Hi Kishon,
>>>
>>> On 30-05-19, 11:16, Kishon Vijay Abraham I wrote:
>>>> +Vinod Koul
>>>>
>>>> Hi,
>>>>
>>>> On 30/05/19 4:07 AM, Alan Mikhak wrote:
>>>>> On Mon, May 27, 2019 at 2:09 AM Gustavo Pimentel
>>>>> <[e-mail]> wrote:
>>>>>>
>>>>>> On Fri, May 24, 2019 at 20:42:43, Alan Mikhak <[e-mail]>
>>>>>> wrote:
>>>>>>
>>>>>> Hi Alan,
>>>>>>
>>>>>>> On Fri, May 24, 2019 at 1:59 AM Gustavo Pimentel
>>>>>>> <[e-mail]> wrote:
>>>>>>>>
>>>>>>>> Hi Alan,
>>>>>>>>
>>>>>>>> This patch implementation is very HW implementation dependent and
>>>>>>>> requires the DMA to exposed through PCIe BARs, which aren't always the
>>>>>>>> case. Besides, you are defining some control bits on
>>>>>>>> include/linux/pci-epc.h that may not have any meaning to other types of
>>>>>>>> DMA.
>>>>>>>>
>>>>>>>> I don't think this was what Kishon had in mind when he developed the
>>>>>>>> pcitest, but let see what Kishon was to say about it.
>>>>>>>>
>>>>>>>> I've developed a DMA driver for DWC PCI using Linux Kernel DMAengine API
>>>>>>>> and which I submitted some days ago.
>>>>>>>> By having a DMA driver which implemented using DMAengine API, means the
>>>>>>>> pcitest can use the DMAengine client API, which will be completely
>>>>>>>> generic to any other DMA implementation.
>>>>
>>>> right, my initial thought process was to use only dmaengine APIs in
>>>> pci-epf-test so that the system DMA or DMA within the PCIe controller can be
>>>> used transparently. But can we register DMA within the PCIe controller to the
>>>> DMA subsystem? AFAIK only system DMA should register with the DMA subsystem.
>>>> (ADMA in SDHCI doesn't use dmaengine). Vinod Koul can confirm.
>>>
>>> So would this DMA be dedicated for PCI and all PCI devices on the bus?
>>
>> Yes, this DMA will be used only by PCI ($patch is w.r.t PCIe device mode. So
>> all endpoint functions both physical and virtual functions will use the DMA in
>> the controller).
>>> If so I do not see a reason why this cannot be using dmaengine. The use
>>
>> Thanks for clarifying. I was under the impression any DMA within a peripheral
>> controller shouldn't use DMAengine.
> 
> That is indeed a correct assumption. The dmaengine helps in cases where
> we have a dma controller with multiple users, for a single user case it
> might be overhead to setup dma driver and then use it thru framework.
> 
> Someone needs to see the benefit and cost of using the framework and
> decide.

The DMA within the endpoint controller can indeed be used by multiple users for
e.g in the case of multi function EP devices or SR-IOV devices, all the
function drivers can use the DMA in the endpoint controller.

I think it makes sense to use dmaengine for DMA within the endpoint controller.
> 
>>> case would be memcpy for DMA right or mem to device (vice versa) transfers?
>>
>> The device is memory mapped so it would be only memcopy.
>>>
>>> Btw many driver in sdhci do use dmaengine APIs and yes we are missing
>>> support in framework than individual drivers
>>
>> I think dmaengine APIs is used only when the platform uses system DMA and not
>> ADMA within the SDHCI controller. IOW there is no dma_async_device_register()
>> to register ADMA in SDHCI with DMA subsystem.
> 
> We are looking it from the different point of view. You are looking for
> dmaengine drivers in that (which would be in drivers/dma/) and I am
> pointing to users of dmaengine in that.
> 
> So the users in mmc would be ones using dmaengine APIs:
> $git grep -l dmaengine_prep_* drivers/mmc/
> 
> which tells me 17 drivers!

right. For the endpoint case, drivers/pci/controller should register with the
dmaengine i.e if the controller has aN embedded DMA (I think it should be okay
to keep that in drivers/pci/controller itself instead of drivers/dma) and
drivers/pci/endpoint/functions/ should use dmaengine API's (Depending on the
platform, this will either use system DMA or DMA within the PCI controller).

Thanks
Kishon

Kishon Vijay Abraham I June 3, 2019, 4:29 a.m. UTC | #14

Hi,

On 31/05/19 1:19 PM, Arnd Bergmann wrote:
> On Fri, May 31, 2019 at 8:32 AM Vinod Koul <[e-mail]> wrote:
>> On 31-05-19, 10:50, Kishon Vijay Abraham I wrote:
>>> On 31/05/19 10:37 AM, Vinod Koul wrote:
>>>> On 30-05-19, 11:16, Kishon Vijay Abraham I wrote:
>>>>>
>>>>> right, my initial thought process was to use only dmaengine APIs in
>>>>> pci-epf-test so that the system DMA or DMA within the PCIe controller can be
>>>>> used transparently. But can we register DMA within the PCIe controller to the
>>>>> DMA subsystem? AFAIK only system DMA should register with the DMA subsystem.
>>>>> (ADMA in SDHCI doesn't use dmaengine). Vinod Koul can confirm.
>>>>
>>>> So would this DMA be dedicated for PCI and all PCI devices on the bus?
>>>
>>> Yes, this DMA will be used only by PCI ($patch is w.r.t PCIe device mode. So
>>> all endpoint functions both physical and virtual functions will use the DMA in
>>> the controller).
>>>> If so I do not see a reason why this cannot be using dmaengine. The use
>>>
>>> Thanks for clarifying. I was under the impression any DMA within a peripheral
>>> controller shouldn't use DMAengine.
>>
>> That is indeed a correct assumption. The dmaengine helps in cases where
>> we have a dma controller with multiple users, for a single user case it
>> might be overhead to setup dma driver and then use it thru framework.
>>
>> Someone needs to see the benefit and cost of using the framework and
>> decide.
> 
> I think the main question is about how generalized we want this to be.
> There are lots of difference PCIe endpoint implementations, and in
> case of some licensable IP cores like the designware PCIe there are
> many variants, as each SoC will do the implementation in a slightly
> different way.
> 
> If we can have a single endpoint driver than can either have an
> integrated DMA engine or use an external one, then abstracting that
> DMA engine helps make the driver work more readily either way.
> 
> Similarly, there may be PCIe endpoint implementations that have
> a dedicated DMA engine in them that is not usable for anything else,
> but that is closely related to an IP core we already have a dmaengine
> driver for. In this case, we can avoid duplication.

right. Either way it makes more sense to register DMA embedded within the PCIe
endpoint controller instead of creating epc_ops for DMA transfers.

Thanks
Kishon

Kishon Vijay Abraham I June 3, 2019, 4:41 a.m. UTC | #15

Hi Alan,

On 31/05/19 11:46 PM, Alan Mikhak wrote:
> On Thu, May 30, 2019 at 10:08 PM Kishon Vijay Abraham I <[e-mail]> wrote:
>> Hi Alan,
>>>
>>> Hi Kishon,
>>>
>>> I have some improvements in mind for a v2 patch in response to
>>> feedback from Gustavo Pimentel that the current implementation is HW
>>> specific. I hesitate from submitting a v2 patch because it seems best
>>> to seek comment on possible directions this may be taking.
>>>
>>> One alternative is to wait for or modify test functions in
>>> pci-epf-test.c to call DMAengine client APIs, if possible. I imagine
>>> pci-epf-test.c test functions would still allocate the necessary local
>>> buffer on the endpoint side for the same canned tests for everyone to
>>> use. They would prepare the buffer in the existing manner by filling
>>> it with random bytes and calculate CRC in the case of a write test.
>>> However, they would then initiate DMA operations by using DMAengine
>>> client APIs in a generic way instead of calling memcpy_toio() and
>>> memcpy_fromio(). They would post-process the buffer in the existing
>>
>> No, you can't remove memcpy_toio/memcpy_fromio APIs. There could be platforms
>> without system DMA or they could have system DMA but without MEMCOPY channels
>> or without DMA in their PCI controller.
> 
> I agree. I wouldn't remove memcpy_toio/fromio. That is the reason this
> patch introduces the '-d' flag for pcitest to communicate that user
> intent across the PCIe bus to pci-epf-test so the endpoint can
> initiate the transfer using either memcpy_toio/fromio or DMA.
> 
>>> manner such as the checking for CRC in the case of a read test.
>>> Finally, they would release the resources and report results back to
>>> the user of pcitest across the PCIe bus through the existing methods.
>>>
>>> Another alternative I have in mind for v2 is to change the struct
>>> pci_epc_dma that this patch added to pci-epc.h from the following:
>>>
>>> struct pci_epc_dma {
>>>         u32     control;
>>>         u32     size;
>>>         u64     sar;
>>>         u64     dar;
>>> };
>>>
>>> to something similar to the following:
>>>
>>> struct pci_epc_dma {
>>>         size_t  size;
>>>         void *buffer;
>>>         int flags;
>>> };
>>>
>>> The 'flags' field can be a bit field or separate boolean values to
>>> specify such things as linked-list mode vs single-block, etc.
>>> Associated #defines would be removed from pci-epc.h to be replaced if
>>> needed with something generic. The 'size' field specifies the size of
>>> DMA transfer that can fit in the buffer.
>>
>> I still have to look closer into your DMA patch but linked-list mode or single
>> block mode shouldn't be an user select-able option but should be determined by
>> the size of transfer.
> 
> Please consider the following when taking a closer look at this patch.

After seeing comments from Vinod and Arnd, it looks like the better way of
adding DMA support would be to register DMA within PCI endpoint controller to
DMA subsystem (as dmaengine) and use only dmaengine APIs in pci_epf_test.
> 
> In my specific use case, I need to verify that any valid block size,
> including a one byte transfer, can be transferred across the PCIe bus
> by memcpy_toio/fromio() or by DMA either as a single block or as
> linked-list. That is why, instead of deciding based on transfer size,
> this patch introduces the '-L' flag for pcitest to communicate the
> user intent across the PCIe bus to pci-epf-test so the endpoint can
> initiate the DMA transfer using a single block or in linked-list mode.
The -L option seems to select an internal DMA configuration which might be
specific to one implementation. As Gustavo already pointed, we should have only
generic options in pcitest. This would no longer be applicable when we move to
dmaengine.

Thanks
Kishon

Vinod Koul June 3, 2019, 4:42 a.m. UTC | #16

Hi Kishon,

On 03-06-19, 09:54, Kishon Vijay Abraham I wrote:

> right. For the endpoint case, drivers/pci/controller should register with the
> dmaengine i.e if the controller has aN embedded DMA (I think it should be okay
> to keep that in drivers/pci/controller itself instead of drivers/dma) and
> drivers/pci/endpoint/functions/ should use dmaengine API's (Depending on the
> platform, this will either use system DMA or DMA within the PCI controller).

Typically I would prefer the driver to be part of drivers/dma.
Would this be a standalone driver or part of the endpoint driver. In
former case we can move to dmaengine for latter i guess it makes sense
to stay in PCI

Thanks

Alan Mikhak June 3, 2019, 5:42 p.m. UTC | #17

On Sun, Jun 2, 2019 at 9:43 PM Kishon Vijay Abraham I <[e-mail]> wrote:
> Hi Alan,
> On 31/05/19 11:46 PM, Alan Mikhak wrote:
> > On Thu, May 30, 2019 at 10:08 PM Kishon Vijay Abraham I <[e-mail]> wrote:
> >> Hi Alan,
> >>> Hi Kishon,
> >>
> >> I still have to look closer into your DMA patch but linked-list mode or single
> >> block mode shouldn't be an user select-able option but should be determined by
> >> the size of transfer.
> >
> > Please consider the following when taking a closer look at this patch.
>
> After seeing comments from Vinod and Arnd, it looks like the better way of
> adding DMA support would be to register DMA within PCI endpoint controller to
> DMA subsystem (as dmaengine) and use only dmaengine APIs in pci_epf_test.

Thanks Kishon. That makes it clear where these pieces should go.

> > In my specific use case, I need to verify that any valid block size,
> > including a one byte transfer, can be transferred across the PCIe bus
> > by memcpy_toio/fromio() or by DMA either as a single block or as
> > linked-list. That is why, instead of deciding based on transfer size,
> > this patch introduces the '-L' flag for pcitest to communicate the
> > user intent across the PCIe bus to pci-epf-test so the endpoint can
> > initiate the DMA transfer using a single block or in linked-list mode.
> The -L option seems to select an internal DMA configuration which might be
> specific to one implementation. As Gustavo already pointed, we should have only
> generic options in pcitest. This would no longer be applicable when we move to
> dmaengine.

Single-block DMA seemed as generic as linked-list DMA and
memcpy_toio/fromio. It remains unclear how else to communicate that
intent to pci_epf_test each time I invoke pcitest.

Regards,
Alan

Kishon Vijay Abraham I Sept. 13, 2019, 12:11 p.m. UTC | #18

+ Haotian Wang

On 03/06/19 11:12 PM, Alan Mikhak wrote:
> On Sun, Jun 2, 2019 at 9:43 PM Kishon Vijay Abraham I <[e-mail]> wrote:
>> Hi Alan,
>> On 31/05/19 11:46 PM, Alan Mikhak wrote:
>>> On Thu, May 30, 2019 at 10:08 PM Kishon Vijay Abraham I <[e-mail]> wrote:
>>>> Hi Alan,
>>>>> Hi Kishon,
>>>>
>>>> I still have to look closer into your DMA patch but linked-list mode or single
>>>> block mode shouldn't be an user select-able option but should be determined by
>>>> the size of transfer.
>>>
>>> Please consider the following when taking a closer look at this patch.
>>
>> After seeing comments from Vinod and Arnd, it looks like the better way of
>> adding DMA support would be to register DMA within PCI endpoint controller to
>> DMA subsystem (as dmaengine) and use only dmaengine APIs in pci_epf_test.
> 
> Thanks Kishon. That makes it clear where these pieces should go.
> 
>>> In my specific use case, I need to verify that any valid block size,
>>> including a one byte transfer, can be transferred across the PCIe bus
>>> by memcpy_toio/fromio() or by DMA either as a single block or as
>>> linked-list. That is why, instead of deciding based on transfer size,
>>> this patch introduces the '-L' flag for pcitest to communicate the
>>> user intent across the PCIe bus to pci-epf-test so the endpoint can
>>> initiate the DMA transfer using a single block or in linked-list mode.
>> The -L option seems to select an internal DMA configuration which might be
>> specific to one implementation. As Gustavo already pointed, we should have only
>> generic options in pcitest. This would no longer be applicable when we move to
>> dmaengine.
> 
> Single-block DMA seemed as generic as linked-list DMA and
> memcpy_toio/fromio. It remains unclear how else to communicate that
> intent to pci_epf_test each time I invoke pcitest.
> 
> Regards,
> Alan
>

Alan Mikhak Sept. 13, 2019, 5:39 p.m. UTC | #19

On Fri, Sep 13, 2019 at 5:11 AM Kishon Vijay Abraham I <[e-mail]> wrote:
>
> + Haotian Wang
>
> On 03/06/19 11:12 PM, Alan Mikhak wrote:
> > On Sun, Jun 2, 2019 at 9:43 PM Kishon Vijay Abraham I <[e-mail]> wrote:
> >> Hi Alan,
> >> On 31/05/19 11:46 PM, Alan Mikhak wrote:
> >>> On Thu, May 30, 2019 at 10:08 PM Kishon Vijay Abraham I <[e-mail]> wrote:
> >>>> Hi Alan,
> >>>>> Hi Kishon,
> >>>>
> >>>> I still have to look closer into your DMA patch but linked-list mode or single
> >>>> block mode shouldn't be an user select-able option but should be determined by
> >>>> the size of transfer.
> >>>
> >>> Please consider the following when taking a closer look at this patch.
> >>
> >> After seeing comments from Vinod and Arnd, it looks like the better way of
> >> adding DMA support would be to register DMA within PCI endpoint controller to
> >> DMA subsystem (as dmaengine) and use only dmaengine APIs in pci_epf_test.
> >
> > Thanks Kishon. That makes it clear where these pieces should go.
> >
> >>> In my specific use case, I need to verify that any valid block size,
> >>> including a one byte transfer, can be transferred across the PCIe bus
> >>> by memcpy_toio/fromio() or by DMA either as a single block or as
> >>> linked-list. That is why, instead of deciding based on transfer size,
> >>> this patch introduces the '-L' flag for pcitest to communicate the
> >>> user intent across the PCIe bus to pci-epf-test so the endpoint can
> >>> initiate the DMA transfer using a single block or in linked-list mode.
> >> The -L option seems to select an internal DMA configuration which might be
> >> specific to one implementation. As Gustavo already pointed, we should have only
> >> generic options in pcitest. This would no longer be applicable when we move to
> >> dmaengine.
> >
> > Single-block DMA seemed as generic as linked-list DMA and
> > memcpy_toio/fromio. It remains unclear how else to communicate that
> > intent to pci_epf_test each time I invoke pcitest.
> >
> > Regards,
> > Alan
> >

Hi Kishon,

FYI, I integrated your changes for DMAengine client support to PCI
endpoint framework into my development branch. The following is the
link you provided earlier as reference. I have been using it with good
results. Haotian Wang also used it in a recent patch for PCI endpoint
function for virtnet. Would you be able to comment on if and when your
DMAengine client support may be submitted upstream?

http://git.ti.com/cgit/cgit.cgi/ti-linux-kernel/ti-linux-kernel.git/tree/drivers/pci/endpoint/pci-epf-core.c?h=ti-linux-4.19.y

Regards,
Alan Mikhak

Patch

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 7b015f2a1c6f..63b86d81a6b5 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -34,6 +34,7 @@ 
 #include <linux/pci_regs.h>
 
 #include <uapi/linux/pcitest.h>
+#include <linux/uaccess.h>
 
 #define DRV_MODULE_NAME				"pci-endpoint-test"
 
@@ -51,6 +52,25 @@ 
 #define COMMAND_READ				BIT(3)
 #define COMMAND_WRITE				BIT(4)
 #define COMMAND_COPY				BIT(5)
+#define COMMAND_FLAG2				BIT(30)
+#define COMMAND_FLAG1				BIT(31)
+
+#define COMMAND_FLAGS				(COMMAND_FLAG1 | \
+						 COMMAND_FLAG2)
+
+#define COMMAND_FLAG_NONE			0
+#define COMMAND_FLAG_DMA			COMMAND_FLAG1
+#define COMMAND_FLAG_DMA_LIST			COMMAND_FLAG2
+
+#define COMMAND_READ_DMA			(COMMAND_READ | \
+						 COMMAND_FLAG_DMA)
+#define COMMAND_READ_DMA_LIST			(COMMAND_READ_DMA | \
+						 COMMAND_FLAG_DMA_LIST)
+
+#define COMMAND_WRITE_DMA			(COMMAND_WRITE | \
+						 COMMAND_FLAG_DMA)
+#define COMMAND_WRITE_DMA_LIST			(COMMAND_WRITE_DMA | \
+						 COMMAND_FLAG_DMA_LIST)
 
 #define PCI_ENDPOINT_TEST_STATUS		0x8
 #define STATUS_READ_SUCCESS			BIT(0)
@@ -425,7 +445,9 @@  static bool pci_endpoint_test_copy(struct pci_endpoint_test *test, size_t size)
 	return ret;
 }
 
-static bool pci_endpoint_test_write(struct pci_endpoint_test *test, size_t size)
+static bool pci_endpoint_test_write(struct pci_endpoint_test *test,
+				    size_t size,
+				    u32 flags)
 {
 	bool ret = false;
 	u32 reg;
@@ -480,7 +502,7 @@  static bool pci_endpoint_test_write(struct pci_endpoint_test *test, size_t size)
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
-				 COMMAND_READ);
+				 COMMAND_READ | flags);
 
 	wait_for_completion(&test->irq_raised);
 
@@ -494,7 +516,24 @@  static bool pci_endpoint_test_write(struct pci_endpoint_test *test, size_t size)
 	return ret;
 }
 
-static bool pci_endpoint_test_read(struct pci_endpoint_test *test, size_t size)
+static bool pci_endpoint_test_write_dma(struct pci_endpoint_test *test,
+					unsigned long arg)
+{
+	u32 flags = COMMAND_FLAG_DMA;
+	struct pcitest_dma dma;
+
+	if (copy_from_user(&dma, (void *)arg, sizeof(struct pcitest_dma)))
+		return -EACCES;
+
+	if (dma.list)
+		flags |= COMMAND_FLAG_DMA_LIST;
+
+	return pci_endpoint_test_write(test, dma.size, flags);
+}
+
+static bool pci_endpoint_test_read(struct pci_endpoint_test *test,
+				   size_t size,
+				   u32 flags)
 {
 	bool ret = false;
 	void *addr;
@@ -542,7 +581,7 @@  static bool pci_endpoint_test_read(struct pci_endpoint_test *test, size_t size)
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, irq_type);
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, 1);
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
-				 COMMAND_WRITE);
+				 COMMAND_WRITE | flags);
 
 	wait_for_completion(&test->irq_raised);
 
@@ -555,6 +594,21 @@  static bool pci_endpoint_test_read(struct pci_endpoint_test *test, size_t size)
 	return ret;
 }
 
+static bool pci_endpoint_test_read_dma(struct pci_endpoint_test *test,
+				       unsigned long arg)
+{
+	u32 flags = COMMAND_FLAG_DMA;
+	struct pcitest_dma dma;
+
+	if (copy_from_user(&dma, (void *)arg, sizeof(struct pcitest_dma)))
+		return -EACCES;
+
+	if (dma.list)
+		flags |= COMMAND_FLAG_DMA_LIST;
+
+	return pci_endpoint_test_read(test, dma.size, flags);
+}
+
 static bool pci_endpoint_test_set_irq(struct pci_endpoint_test *test,
 				      int req_irq_type)
 {
@@ -612,11 +666,17 @@  static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd,
 	case PCITEST_MSIX:
 		ret = pci_endpoint_test_msi_irq(test, arg, cmd == PCITEST_MSIX);
 		break;
+	case PCITEST_WRITE_DMA:
+		ret = pci_endpoint_test_write_dma(test, arg);
+		break;
+	case PCITEST_READ_DMA:
+		ret = pci_endpoint_test_read_dma(test, arg);
+		break;
 	case PCITEST_WRITE:
-		ret = pci_endpoint_test_write(test, arg);
+		ret = pci_endpoint_test_write(test, arg, COMMAND_FLAG_NONE);
 		break;
 	case PCITEST_READ:
-		ret = pci_endpoint_test_read(test, arg);
+		ret = pci_endpoint_test_read(test, arg, COMMAND_FLAG_NONE);
 		break;
 	case PCITEST_COPY:
 		ret = pci_endpoint_test_copy(test, arg);
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 2bf5a35c0570..7e25c0f5edf1 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -366,6 +366,26 @@  dw_pcie_ep_get_features(struct pci_epc *epc, u8 func_no)
 	return ep->ops->get_features(ep);
 }
 
+static int dw_pcie_ep_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma)
+{
+	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+
+	if (!ep->ops->dma_read)
+		return -EINVAL;
+
+	return ep->ops->dma_read(ep, dma);
+}
+
+static int dw_pcie_ep_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma)
+{
+	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+
+	if (!ep->ops->dma_write)
+		return -EINVAL;
+
+	return ep->ops->dma_write(ep, dma);
+}
+
 static const struct pci_epc_ops epc_ops = {
 	.write_header		= dw_pcie_ep_write_header,
 	.set_bar		= dw_pcie_ep_set_bar,
@@ -380,6 +400,8 @@  static const struct pci_epc_ops epc_ops = {
 	.start			= dw_pcie_ep_start,
 	.stop			= dw_pcie_ep_stop,
 	.get_features		= dw_pcie_ep_get_features,
+	.dma_read		= dw_pcie_ep_dma_read,
+	.dma_write		= dw_pcie_ep_dma_write
 };
 
 int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index b8993f2b78df..11d44ec8acc7 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -197,6 +197,8 @@  struct dw_pcie_ep_ops {
 	int	(*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
 			     enum pci_epc_irq_type type, u16 interrupt_num);
 	const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
+	int	(*dma_read)(struct dw_pcie_ep *ep, struct pci_epc_dma *dma);
+	int	(*dma_write)(struct dw_pcie_ep *ep, struct pci_epc_dma *dma);
 };
 
 struct dw_pcie_ep {
@@ -238,6 +240,7 @@  struct dw_pcie {
 	void __iomem		*dbi_base2;
 	/* Used when iatu_unroll_enabled is true */
 	void __iomem		*atu_base;
+	void __iomem		*dma_base;
 	u32			num_viewport;
 	u8			iatu_unroll_enabled;
 	struct pcie_port	pp;
@@ -323,6 +326,16 @@  static inline u32 dw_pcie_readl_atu(struct dw_pcie *pci, u32 reg)
 	return __dw_pcie_read_dbi(pci, pci->atu_base, reg, 0x4);
 }
 
+static inline void dw_pcie_writel_dma(struct dw_pcie *pci, u32 reg, u32 val)
+{
+	__dw_pcie_write_dbi(pci, pci->dma_base, reg, 0x4, val);
+}
+
+static inline u32 dw_pcie_readl_dma(struct dw_pcie *pci, u32 reg)
+{
+	return __dw_pcie_read_dbi(pci, pci->dma_base, reg, 0x4);
+}
+
 static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
 {
 	u32 reg;
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 27806987e93b..3910073712e9 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -28,6 +28,25 @@ 
 #define COMMAND_READ			BIT(3)
 #define COMMAND_WRITE			BIT(4)
 #define COMMAND_COPY			BIT(5)
+#define COMMAND_FLAG2			BIT(30)
+#define COMMAND_FLAG1			BIT(31)
+
+#define COMMAND_FLAGS			(COMMAND_FLAG1 | \
+					 COMMAND_FLAG2)
+
+#define COMMAND_FLAG_NONE		0
+#define COMMAND_FLAG_DMA		COMMAND_FLAG1
+#define COMMAND_FLAG_DMA_LIST		COMMAND_FLAG2
+
+#define COMMAND_READ_DMA		(COMMAND_READ | \
+					 COMMAND_FLAG_DMA)
+#define COMMAND_READ_DMA_LIST		(COMMAND_READ_DMA | \
+					 COMMAND_FLAG_DMA_LIST)
+
+#define COMMAND_WRITE_DMA		(COMMAND_WRITE | \
+					 COMMAND_FLAG_DMA)
+#define COMMAND_WRITE_DMA_LIST		(COMMAND_WRITE_DMA | \
+					 COMMAND_FLAG_DMA_LIST)
 
 #define STATUS_READ_SUCCESS		BIT(0)
 #define STATUS_READ_FAIL		BIT(1)
@@ -187,6 +206,93 @@  static int pci_epf_test_read(struct pci_epf_test *epf_test)
 	return ret;
 }
 
+static int pci_epf_test_read_dma(struct pci_epf_test *epf_test)
+{
+	int ret = -ENOMEM;
+	struct pci_epf *epf = epf_test->epf;
+	struct device *dev = &epf->dev;
+	struct pci_epc *epc = epf->epc;
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+	struct pci_epc_dma dma;
+	u32 crc32;
+	void *buf;
+
+	buf = kzalloc(reg->size, GFP_KERNEL);
+	if (buf) {
+		dma.control = PCI_EPC_DMA_CONTROL_LIE;
+		dma.size = reg->size;
+		dma.sar = reg->src_addr;
+		dma.dar = virt_to_phys(buf);
+
+		ret = pci_epc_dma_read(epc, &dma);
+		if (ret) {
+			dev_err(dev, "pci_epc_dma_read %d\n", ret);
+		} else {
+			crc32 = crc32_le(~0, buf, reg->size);
+			if (crc32 != reg->checksum)
+				ret = -EIO;
+		}
+
+		kfree(buf);
+	}
+
+	return ret;
+}
+
+static int pci_epf_test_read_dma_list(struct pci_epf_test *epf_test)
+{
+	int ret = -ENOMEM;
+	struct pci_epf *epf = epf_test->epf;
+	struct device *dev = &epf->dev;
+	struct pci_epc *epc = epf->epc;
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+	struct pci_epc_dma *dma;
+	u32 crc32;
+	void *buf;
+
+	dma = kcalloc(3, sizeof(*dma), GFP_KERNEL);
+	if (dma) {
+		buf = kzalloc(reg->size, GFP_KERNEL);
+		if (buf) {
+			int half_size = reg->size >> 1;
+			phys_addr_t phys_addr = virt_to_phys(buf);
+
+			dma[0].control = PCI_EPC_DMA_CONTROL_CB;
+			dma[0].size = half_size ? half_size : 1;
+			dma[0].sar = reg->src_addr;
+			dma[0].dar = phys_addr;
+
+			dma[1].control = PCI_EPC_DMA_CONTROL_CB |
+					 PCI_EPC_DMA_CONTROL_LIE;
+			dma[1].size = reg->size - half_size;
+			dma[1].sar = reg->src_addr + half_size;
+			dma[1].dar = phys_addr + half_size;
+
+			dma[2].control = PCI_EPC_DMA_CONTROL_EOL;
+			dma[2].size = 0;
+			dma[2].sar = virt_to_phys(dma);
+			dma[2].dar = 0;
+
+			ret = pci_epc_dma_read(epc, dma);
+			if (ret) {
+				dev_err(dev, "pci_epc_dma_read %d\n", ret);
+			} else {
+				crc32 = crc32_le(~0, buf, reg->size);
+				if (crc32 != reg->checksum)
+					ret = -EIO;
+			}
+
+			kfree(buf);
+		}
+
+		kfree(dma);
+	}
+
+	return ret;
+}
+
 static int pci_epf_test_write(struct pci_epf_test *epf_test)
 {
 	int ret;
@@ -244,6 +350,87 @@  static int pci_epf_test_write(struct pci_epf_test *epf_test)
 	return ret;
 }
 
+static int pci_epf_test_write_dma(struct pci_epf_test *epf_test)
+{
+	int ret = -ENOMEM;
+	struct pci_epf *epf = epf_test->epf;
+	struct device *dev = &epf->dev;
+	struct pci_epc *epc = epf->epc;
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+	struct pci_epc_dma dma;
+	void *buf;
+
+	buf = kzalloc(reg->size, GFP_KERNEL);
+	if (buf) {
+		get_random_bytes(buf, reg->size);
+		reg->checksum = crc32_le(~0, buf, reg->size);
+
+		dma.control = PCI_EPC_DMA_CONTROL_LIE;
+		dma.size = reg->size;
+		dma.sar = virt_to_phys(buf);
+		dma.dar = reg->dst_addr;
+
+		ret = pci_epc_dma_write(epc, &dma);
+		if (ret)
+			dev_err(dev, "pci_epc_dma_write %d\n", ret);
+
+		kfree(buf);
+	}
+
+	return ret;
+}
+
+static int pci_epf_test_write_dma_list(struct pci_epf_test *epf_test)
+{
+	int ret = -ENOMEM;
+	struct pci_epf *epf = epf_test->epf;
+	struct device *dev = &epf->dev;
+	struct pci_epc *epc = epf->epc;
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+	struct pci_epc_dma *dma;
+	void *buf;
+
+	dma = kcalloc(3, sizeof(*dma), GFP_KERNEL);
+	if (dma) {
+		buf = kzalloc(reg->size, GFP_KERNEL);
+		if (buf) {
+			int half_size = reg->size >> 1;
+			phys_addr_t phys_addr = virt_to_phys(buf);
+
+			get_random_bytes(buf, reg->size);
+			reg->checksum = crc32_le(~0, buf, reg->size);
+
+			dma[0].control = PCI_EPC_DMA_CONTROL_CB;
+			dma[0].size = half_size ? half_size : 1;
+			dma[0].sar = phys_addr;
+			dma[0].dar = reg->dst_addr;
+
+			dma[1].control = PCI_EPC_DMA_CONTROL_CB |
+					 PCI_EPC_DMA_CONTROL_LIE;
+			dma[1].size = reg->size - half_size;
+			dma[1].sar = phys_addr + half_size;
+			dma[1].dar = reg->dst_addr + half_size;
+
+			dma[2].control = PCI_EPC_DMA_CONTROL_EOL;
+			dma[2].size = 0;
+			dma[2].sar = virt_to_phys(dma);
+			dma[2].dar = 0;
+
+			ret = pci_epc_dma_write(epc, dma);
+			if (ret)
+				dev_err(dev, "pci_epc_dma_write %d\n", ret);
+
+			kfree(buf);
+		}
+
+		kfree(dma);
+	}
+
+	return ret;
+}
+
 static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test, u8 irq_type,
 				   u16 irq)
 {
@@ -303,18 +490,34 @@  static void pci_epf_test_cmd_handler(struct work_struct *work)
 	}
 
 	if (command & COMMAND_WRITE) {
-		ret = pci_epf_test_write(epf_test);
-		if (ret)
-			reg->status |= STATUS_WRITE_FAIL;
+		command &= (COMMAND_WRITE | COMMAND_FLAGS);
+		if (command == COMMAND_WRITE)
+			ret = pci_epf_test_write(epf_test);
+		else if (command == COMMAND_WRITE_DMA)
+			ret = pci_epf_test_write_dma(epf_test);
+		else if (command == COMMAND_WRITE_DMA_LIST)
+			ret = pci_epf_test_write_dma_list(epf_test);
 		else
+			ret = -EINVAL;
+		if (!ret)
 			reg->status |= STATUS_WRITE_SUCCESS;
+		else
+			reg->status |= STATUS_WRITE_FAIL;
 		pci_epf_test_raise_irq(epf_test, reg->irq_type,
 				       reg->irq_number);
 		goto reset_handler;
 	}
 
 	if (command & COMMAND_READ) {
-		ret = pci_epf_test_read(epf_test);
+		command &= (COMMAND_READ | COMMAND_FLAGS);
+		if (command == COMMAND_READ)
+			ret = pci_epf_test_read(epf_test);
+		else if (command == COMMAND_READ_DMA)
+			ret = pci_epf_test_read_dma(epf_test);
+		else if (command == COMMAND_READ_DMA_LIST)
+			ret = pci_epf_test_read_dma_list(epf_test);
+		else
+			ret = -EINVAL;
 		if (!ret)
 			reg->status |= STATUS_READ_SUCCESS;
 		else
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index e4712a0f249c..a57e501d4abc 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -107,6 +107,52 @@  unsigned int pci_epc_get_first_free_bar(const struct pci_epc_features
 EXPORT_SYMBOL_GPL(pci_epc_get_first_free_bar);
 
 /**
+ * pci_epc_dma_write() - DMA a block of memory to remote address
+ * @epc: the EPC device on which to perform DMA transfer
+ * @dma: DMA descriptors array
+ *
+ * Write contents of local memory to remote memory by DMA.
+ */
+int pci_epc_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma)
+{
+	int ret;
+	unsigned long flags;
+
+	if (IS_ERR(epc) || !epc->ops->dma_write)
+		return -EINVAL;
+
+	spin_lock_irqsave(&epc->lock, flags);
+	ret = epc->ops->dma_write(epc, dma);
+	spin_unlock_irqrestore(&epc->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_epc_dma_write);
+
+/**
+ * pci_epc_dma_read() - DMA a block of memory from remote address
+ * @epc: the EPC device on which to perform DMA transfer
+ * @dma: DMA descriptors array
+ *
+ * Read contents of remote memory into local memory by DMA.
+ */
+int pci_epc_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma)
+{
+	int ret;
+	unsigned long flags;
+
+	if (IS_ERR(epc) || !epc->ops->dma_read)
+		return -EINVAL;
+
+	spin_lock_irqsave(&epc->lock, flags);
+	ret = epc->ops->dma_read(epc, dma);
+	spin_unlock_irqrestore(&epc->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_epc_dma_read);
+
+/**
  * pci_epc_get_features() - get the features supported by EPC
  * @epc: the features supported by *this* EPC device will be returned
  * @func_no: the features supported by the EPC device specific to the
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index f641badc2c61..d845f13d0baf 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -21,6 +21,45 @@  enum pci_epc_irq_type {
 };
 
 /**
+ * struct pci_epc_dma - descriptor for a DMA transfer element
+ * @control: DMA channel control bits for read or write transfer
+ * @size: size of DMA transfer element
+ * @sar: source addrees for DMA transfer element
+ * @dar: destination address for DMA transfer element
+ */
+
+struct pci_epc_dma {
+	u32	control;
+	u32	size;
+	u64	sar;
+	u64	dar;
+};
+
+#define PCI_EPC_DMA_CONTROL_CB          (BIT(0))
+#define PCI_EPC_DMA_CONTROL_TCB         (BIT(1))
+#define PCI_EPC_DMA_CONTROL_LLP         (BIT(2))
+#define PCI_EPC_DMA_CONTROL_LIE         (BIT(3))
+#define PCI_EPC_DMA_CONTROL_RIE         (BIT(4))
+#define PCI_EPC_DMA_CONTROL_CS          (BIT(5) | BIT(6))
+#define PCI_EPC_DMA_CONTROL_CCS         (BIT(8))
+#define PCI_EPC_DMA_CONTROL_LLE         (BIT(9))
+#define PCI_EPC_DMA_CONTROL_FUNC        (BIT(12) | BIT(13) | BIT(14) | \
+					 BIT(15) | BIT(16))
+#define PCI_EPC_DMA_CONTROL_NS_DST      (BIT(23))
+#define PCI_EPC_DMA_CONTROL_NS_SRC      (BIT(24))
+#define PCI_EPC_DMA_CONTROL_RO          (BIT(25))
+#define PCI_EPC_DMA_CONTROL_TC          (BIT(27) | BIT(28) | BIT(29))
+#define PCI_EPC_DMA_CONTROL_AT          (BIT(30) | BIT(31))
+
+#define PCI_EPC_DMA_CONTROL_EOL         (PCI_EPC_DMA_CONTROL_TCB | \
+					 PCI_EPC_DMA_CONTROL_LLP)
+
+#define PCI_EPC_DMA_CONTROL_LIST        (PCI_EPC_DMA_CONTROL_CB | \
+					 PCI_EPC_DMA_CONTROL_EOL| \
+					 PCI_EPC_DMA_CONTROL_CCS | \
+					 PCI_EPC_DMA_CONTROL_LLE)
+
+/**
  * struct pci_epc_ops - set of function pointers for performing EPC operations
  * @write_header: ops to populate configuration space header
  * @set_bar: ops to configure the BAR
@@ -38,6 +77,8 @@  enum pci_epc_irq_type {
  * @raise_irq: ops to raise a legacy, MSI or MSI-X interrupt
  * @start: ops to start the PCI link
  * @stop: ops to stop the PCI link
+ * @dma_read: ops to read remote memory into local memory by DMA
+ * @dma_write: ops to write local memory to remote memory by DMA
  * @owner: the module owner containing the ops
  */
 struct pci_epc_ops {
@@ -61,6 +102,8 @@  struct pci_epc_ops {
 	void	(*stop)(struct pci_epc *epc);
 	const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
 						       u8 func_no);
+	int	(*dma_read)(struct pci_epc *epc, struct pci_epc_dma *dma);
+	int	(*dma_write)(struct pci_epc *epc, struct pci_epc_dma *dma);
 	struct module *owner;
 };
 
@@ -152,6 +195,8 @@  void pci_epc_destroy(struct pci_epc *epc);
 int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf);
 void pci_epc_linkup(struct pci_epc *epc);
 void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf);
+int pci_epc_dma_read(struct pci_epc *epc, struct pci_epc_dma *dma);
+int pci_epc_dma_write(struct pci_epc *epc, struct pci_epc_dma *dma);
 int pci_epc_write_header(struct pci_epc *epc, u8 func_no,
 			 struct pci_epf_header *hdr);
 int pci_epc_set_bar(struct pci_epc *epc, u8 func_no,
diff --git a/include/uapi/linux/pcitest.h b/include/uapi/linux/pcitest.h
index cbf422e56696..505f4a3811c2 100644
--- a/include/uapi/linux/pcitest.h
+++ b/include/uapi/linux/pcitest.h
@@ -10,11 +10,18 @@ 
 #ifndef __UAPI_LINUX_PCITEST_H
 #define __UAPI_LINUX_PCITEST_H
 
+struct pcitest_dma {
+	size_t	size;
+	bool	list;
+};
+
 #define PCITEST_BAR		_IO('P', 0x1)
 #define PCITEST_LEGACY_IRQ	_IO('P', 0x2)
 #define PCITEST_MSI		_IOW('P', 0x3, int)
 #define PCITEST_WRITE		_IOW('P', 0x4, unsigned long)
+#define PCITEST_WRITE_DMA	_IOW('P', 0x4, struct pcitest_dma)
 #define PCITEST_READ		_IOW('P', 0x5, unsigned long)
+#define PCITEST_READ_DMA	_IOW('P', 0x5, struct pcitest_dma)
 #define PCITEST_COPY		_IOW('P', 0x6, unsigned long)
 #define PCITEST_MSIX		_IOW('P', 0x7, int)
 #define PCITEST_SET_IRQTYPE	_IOW('P', 0x8, int)
diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
index 6f1303104d84..66cd19acf18c 100644
--- a/tools/pci/pcitest.c
+++ b/tools/pci/pcitest.c
@@ -44,11 +44,14 @@  struct pci_test {
 	bool		read;
 	bool		write;
 	bool		copy;
+	bool		dma;
+	bool		dma_list;
 	unsigned long	size;
 };
 
 static int run_test(struct pci_test *test)
 {
+	struct pcitest_dma dma;
 	int ret = -EINVAL;
 	int fd;
 
@@ -113,7 +116,13 @@  static int run_test(struct pci_test *test)
 	}
 
 	if (test->write) {
-		ret = ioctl(fd, PCITEST_WRITE, test->size);
+		if (test->dma) {
+			dma.size = test->size;
+			dma.list = test->dma_list;
+			ret = ioctl(fd, PCITEST_WRITE_DMA, &dma);
+		} else {
+			ret = ioctl(fd, PCITEST_WRITE, test->size);
+		}
 		fprintf(stdout, "WRITE (%7ld bytes):\t\t", test->size);
 		if (ret < 0)
 			fprintf(stdout, "TEST FAILED\n");
@@ -122,7 +131,13 @@  static int run_test(struct pci_test *test)
 	}
 
 	if (test->read) {
-		ret = ioctl(fd, PCITEST_READ, test->size);
+		if (test->dma) {
+			dma.size = test->size;
+			dma.list = test->dma_list;
+			ret = ioctl(fd, PCITEST_READ_DMA, &dma);
+		} else {
+			ret = ioctl(fd, PCITEST_READ, test->size);
+		}
 		fprintf(stdout, "READ (%7ld bytes):\t\t", test->size);
 		if (ret < 0)
 			fprintf(stdout, "TEST FAILED\n");
@@ -163,7 +178,7 @@  int main(int argc, char **argv)
 	/* set default endpoint device */
 	test->device = "/dev/pci-endpoint-test.0";
 
-	while ((c = getopt(argc, argv, "D:b:m:x:i:Ilhrwcs:")) != EOF)
+	while ((c = getopt(argc, argv, "D:b:m:x:i:IlhrwcdLs:")) != EOF)
 	switch (c) {
 	case 'D':
 		test->device = optarg;
@@ -204,6 +219,12 @@  int main(int argc, char **argv)
 	case 'c':
 		test->copy = true;
 		continue;
+	case 'd':
+		test->dma = true;
+		continue;
+	case 'L':
+		test->dma_list = true;
+		continue;
 	case 's':
 		test->size = strtoul(optarg, NULL, 0);
 		continue;
@@ -223,6 +244,8 @@  int main(int argc, char **argv)
 			"\t-r			Read buffer test\n"
 			"\t-w			Write buffer test\n"
 			"\t-c			Copy buffer test\n"
+			"\t-d			DMA mode for read or write test\n"
+			"\t-L			Linked-List DMA flag for DMA mode\n"
 			"\t-s <size>		Size of buffer {default: 100KB}\n"
 			"\t-h			Print this help message\n",
 			argv[0]);