Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
To: Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org
Cc: "Rick Wertenbroek" <rick.wertenbroek@gmail.com>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Subject: [PATCH 03/12] pci: epf-test: Fix DMA transfer completion detection
Date: Wed, 15 Feb 2023 12:21:46 +0900	[thread overview]
Message-ID: <20230215032155.74993-4-damien.lemoal@opensource.wdc.com> (raw)
In-Reply-To: <20230215032155.74993-1-damien.lemoal@opensource.wdc.com>

pci_epf_test_data_transfer() and pci_epf_test_dma_callback() are not
handling DMA transfer completion correctly, leading to completion
notifications to the RC side that are too early. This problem can be
detected when the RC side is running an IOMMU with messages such as:

pci-endpoint-test 0000:0b:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT
domain=0x001c address=0xfff00000 flags=0x0000]

When running the pcitest.sh tests: the address used for a previous
test transfer generates the above error while the next test transfer is
running.

Fix this by testing the dma transfer status in
pci_epf_test_dma_callback() and notifying the completion only when the
transfer status is DMA_COMPLETE or DMA_ERROR. Furthermore, in
pci_epf_test_data_transfer(), be paranoid and check again the transfer
status and always call dmaengine_terminate_sync() before returning.

While at it, also modify the channel tx submit call to use
dmaengine_submit() instead of the hard coded call to the tx_submit()
operation.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 42 +++++++++++++------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 55283d2379a6..030769893efb 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -54,6 +54,9 @@ struct pci_epf_test {
 	struct delayed_work	cmd_handler;
 	struct dma_chan		*dma_chan_tx;
 	struct dma_chan		*dma_chan_rx;
+	struct dma_chan		*transfer_chan;
+	dma_cookie_t		transfer_cookie;
+	enum dma_status		transfer_status;
 	struct completion	transfer_complete;
 	bool			dma_supported;
 	bool			dma_private;
@@ -85,8 +88,14 @@ static size_t bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
 static void pci_epf_test_dma_callback(void *param)
 {
 	struct pci_epf_test *epf_test = param;
-
-	complete(&epf_test->transfer_complete);
+	struct dma_tx_state state;
+
+	epf_test->transfer_status =
+		dmaengine_tx_status(epf_test->transfer_chan,
+				    epf_test->transfer_cookie, &state);
+	if (epf_test->transfer_status == DMA_COMPLETE ||
+	    epf_test->transfer_status == DMA_ERROR)
+		complete(&epf_test->transfer_complete);
 }
 
 /**
@@ -120,7 +129,6 @@ static int pci_epf_test_data_transfer(struct pci_epf_test *epf_test,
 	struct dma_async_tx_descriptor *tx;
 	struct dma_slave_config sconf = {};
 	struct device *dev = &epf->dev;
-	dma_cookie_t cookie;
 	int ret;
 
 	if (IS_ERR_OR_NULL(chan)) {
@@ -151,26 +159,36 @@ static int pci_epf_test_data_transfer(struct pci_epf_test *epf_test,
 		return -EIO;
 	}
 
+	reinit_completion(&epf_test->transfer_complete);
+	epf_test->transfer_chan = chan;
 	tx->callback = pci_epf_test_dma_callback;
 	tx->callback_param = epf_test;
-	cookie = tx->tx_submit(tx);
-	reinit_completion(&epf_test->transfer_complete);
+	epf_test->transfer_cookie = dmaengine_submit(tx);
 
-	ret = dma_submit_error(cookie);
+	ret = dma_submit_error(epf_test->transfer_cookie);
 	if (ret) {
-		dev_err(dev, "Failed to do DMA tx_submit %d\n", cookie);
-		return -EIO;
+		dev_err(dev, "Failed to do DMA tx_submit %d\n", ret);
+		goto terminate;
 	}
 
 	dma_async_issue_pending(chan);
 	ret = wait_for_completion_interruptible(&epf_test->transfer_complete);
 	if (ret < 0) {
-		dmaengine_terminate_sync(chan);
-		dev_err(dev, "DMA wait_for_completion_timeout\n");
-		return -ETIMEDOUT;
+		dev_err(dev, "DMA wait_for_completion interrupted\n");
+		goto terminate;
 	}
 
-	return 0;
+	if (epf_test->transfer_status == DMA_ERROR) {
+		dev_err(dev, "DMA transfer failed\n");
+		ret = -EIO;
+	}
+
+	WARN_ON(epf_test->transfer_status != DMA_COMPLETE);
+
+terminate:
+	dmaengine_terminate_sync(chan);
+
+	return ret;
 }
 
 struct epf_dma_filter {
-- 
2.39.1


  parent reply	other threads:[~2023-02-15  3:22 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-15  3:21 [PATCH 00/12] PCI endpoint fixes and improvements Damien Le Moal
2023-02-15  3:21 ` [PATCH 01/12] pci: endpoint: Automatically create a function type attributes group Damien Le Moal
2023-02-16 10:04   ` Manivannan Sadhasivam
2023-02-16 12:31     ` Damien Le Moal
2023-02-15  3:21 ` [PATCH 02/12] pci: endpoint: do not export pci_epf_type_add_cfs() Damien Le Moal
2023-02-16 10:15   ` Manivannan Sadhasivam
2023-02-16 12:33     ` Damien Le Moal
2023-02-15  3:21 ` Damien Le Moal [this message]
2023-02-16 10:18   ` [PATCH 03/12] pci: epf-test: Fix DMA transfer completion detection Manivannan Sadhasivam
2023-02-15  3:21 ` [PATCH 04/12] pci: epf-test: Use driver registers as volatile Damien Le Moal
2023-02-16 10:23   ` Manivannan Sadhasivam
2023-02-15  3:21 ` [PATCH 05/12] pci: epf-test: Simplify dma support checks Damien Le Moal
2023-02-16 10:27   ` Manivannan Sadhasivam
2023-02-15  3:21 ` [PATCH 06/12] pci: epf-test: Simplify transfers result print Damien Le Moal
2023-02-16 10:39   ` Manivannan Sadhasivam
2023-02-15  3:21 ` [PATCH 07/12] pci: epf-test: Add debug and error messages Damien Le Moal
2023-02-15 11:34   ` Greg Kroah-Hartman
2023-02-15 11:44     ` Damien Le Moal
2023-02-15 11:34   ` Greg Kroah-Hartman
2023-02-15 11:45     ` Damien Le Moal
2023-02-15 12:01       ` Greg Kroah-Hartman
2023-02-15 12:18         ` Damien Le Moal
2023-02-15 13:24           ` Greg Kroah-Hartman
2023-02-15 13:49             ` Arnd Bergmann
2023-02-15 22:55               ` Damien Le Moal
2023-02-15  3:21 ` [PATCH 08/12] misc: pci_endpoint_test: Free IRQs before removing the device Damien Le Moal
2023-02-16 10:46   ` Manivannan Sadhasivam
2023-02-15  3:21 ` [PATCH 09/12] misc: pci_endpoint_test: Do not write status in IRQ handler Damien Le Moal
2023-02-16 10:51   ` Manivannan Sadhasivam
2023-02-15  3:21 ` [PATCH 10/12] misc: pci_endpoint_test: Re-init completion for every test Damien Le Moal
2023-02-16 10:55   ` Manivannan Sadhasivam
2023-02-16 12:35     ` Damien Le Moal
2023-02-15  3:21 ` [PATCH 11/12] misc: pci_endpoint_test: Simplify pci_endpoint_test_msi_irq() Damien Le Moal
2023-02-16 10:57   ` Manivannan Sadhasivam
2023-02-15  3:21 ` [PATCH 12/12] misc: pci_endpoint_test: Add debug and error messages Damien Le Moal
2023-02-15 11:34   ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230215032155.74993-4-damien.lemoal@opensource.wdc.com \
    --to=damien.lemoal@opensource.wdc.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kishon@kernel.org \
    --cc=kw@linux.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=mani@kernel.org \
    --cc=rick.wertenbroek@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox