linux-pci.vger.kernel.org archive mirror
 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 v2 09/16] PCI: epf-test: Improve handling of command and status registers
Date: Wed,  8 Mar 2023 18:03:06 +0900	[thread overview]
Message-ID: <20230308090313.1653-10-damien.lemoal@opensource.wdc.com> (raw)
In-Reply-To: <20230308090313.1653-1-damien.lemoal@opensource.wdc.com>

The pci-epf-test driver uses the test register bar memory directly
to get and execute a test registers set by the RC side and defined
using a struct pci_epf_test_reg. This direct use relies on a casts of
the register bar to get a pointer to a struct pci_epf_test_reg to
execute the test case and sending back the test result through the
status field of struct pci_epf_test_reg. In practice, the status field
is always updated before an interrupt is raised in
pci_epf_test_raise_irq(), to ensure that the RC side sees the updated
status when receiving the interrupts.

However, such cast-based direct access does not ensure that changes to
the status register make it to memory, and so visible to the host,
before an interrupt is raised, thus potentially resulting in the RC host
not seeing the correct status result for a test.

Avoid this potential problem by using READ_ONCE()/WRITE_ONCE() when
accessing the command and status fields of a pci_epf_test_reg structure.
This ensure that a test start (pci_epf_test_cmd_handler() function) and
completion (with the function pci_epf_test_raise_irq()) achive a correct
synchronization with the host side mmio register accesses.

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

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 43d623682850..e0cf8c2bf6db 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -615,9 +615,14 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
 	struct pci_epf *epf = epf_test->epf;
 	struct device *dev = &epf->dev;
 	struct pci_epc *epc = epf->epc;
+	u32 status = reg->status | STATUS_IRQ_RAISED;
 	int count;
 
-	reg->status |= STATUS_IRQ_RAISED;
+	/*
+	 * Set the status before raising the IRQ to ensure that the host sees
+	 * the updated value when it gets the IRQ.
+	 */
+	WRITE_ONCE(reg->status, status);
 
 	switch (reg->irq_type) {
 	case IRQ_TYPE_LEGACY:
@@ -661,12 +666,12 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
 	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
 	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
 
-	command = reg->command;
+	command = READ_ONCE(reg->command);
 	if (!command)
 		goto reset_handler;
 
-	reg->command = 0;
-	reg->status = 0;
+	WRITE_ONCE(reg->command, 0);
+	WRITE_ONCE(reg->status, 0);
 
 	if (reg->irq_type > IRQ_TYPE_MSIX) {
 		dev_err(dev, "Failed to detect IRQ type\n");
-- 
2.39.2


  parent reply	other threads:[~2023-03-08  9:04 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-08  9:02 [PATCH v2 00/16] PCI endpoint fixes and improvements Damien Le Moal
2023-03-08  9:02 ` [PATCH v2 01/16] PCI: endpoint: Automatically create a function specific attributes group Damien Le Moal
2023-03-15 14:27   ` Manivannan Sadhasivam
2023-03-08  9:02 ` [PATCH v2 02/16] PCI: endpoint: Move pci_epf_type_add_cfs() code Damien Le Moal
2023-03-15 15:01   ` Manivannan Sadhasivam
2023-03-08  9:03 ` [PATCH v2 03/16] PCI: epf-test: Fix DMA transfer completion initialization Damien Le Moal
2023-03-15 15:03   ` Manivannan Sadhasivam
2023-03-08  9:03 ` [PATCH v2 04/16] PCI: epf-test: Fix DMA transfer completion detection Damien Le Moal
2023-03-15 15:20   ` Manivannan Sadhasivam
2023-03-15 23:46     ` Damien Le Moal
2023-03-08  9:03 ` [PATCH v2 05/16] PCI: epf-test: Use dmaengine_submit() to initiate DMA transfer Damien Le Moal
2023-03-15 15:21   ` Manivannan Sadhasivam
2023-03-08  9:03 ` [PATCH v2 06/16] PCI: epf-test: Simplify read/write/copy test functions Damien Le Moal
2023-03-15 15:24   ` Manivannan Sadhasivam
2023-03-08  9:03 ` [PATCH v2 07/16] PCI: epf-test: Simply pci_epf_test_raise_irq() Damien Le Moal
2023-03-15 15:27   ` Manivannan Sadhasivam
2023-03-08  9:03 ` [PATCH v2 08/16] PCI: epf-test: Simplify IRQ test commands execution Damien Le Moal
2023-03-15 15:37   ` Manivannan Sadhasivam
2023-03-08  9:03 ` Damien Le Moal [this message]
2023-03-15 15:51   ` [PATCH v2 09/16] PCI: epf-test: Improve handling of command and status registers Manivannan Sadhasivam
2023-03-15 23:49     ` Damien Le Moal
2023-03-16 15:25     ` Arnd Bergmann
2023-03-16 16:31       ` Manivannan Sadhasivam
2023-03-16 16:32   ` Manivannan Sadhasivam
2023-03-08  9:03 ` [PATCH v2 10/16] PCI: epf-test: Cleanup pci_epf_test_cmd_handler() Damien Le Moal
2023-03-15 15:52   ` Manivannan Sadhasivam
2023-03-08  9:03 ` [PATCH v2 11/16] PCI: epf-test: Simplify dma support checks Damien Le Moal
2023-03-15 15:57   ` Manivannan Sadhasivam
2023-03-08  9:03 ` [PATCH v2 12/16] PCI: epf-test: Simplify transfers result print Damien Le Moal
2023-03-08  9:03 ` [PATCH v2 13/16] misc: pci_endpoint_test: Free IRQs before removing the device Damien Le Moal
2023-03-08  9:03 ` [PATCH v2 14/16] misc: pci_endpoint_test: Re-init completion for every test Damien Le Moal
2023-03-15 15:55   ` Manivannan Sadhasivam
2023-03-08  9:03 ` [PATCH v2 15/16] misc: pci_endpoint_test: Do not write status in IRQ handler Damien Le Moal
2023-03-08  9:03 ` [PATCH v2 16/16] misc: pci_endpoint_test: Simplify pci_endpoint_test_msi_irq() Damien Le Moal

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=20230308090313.1653-10-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;
as well as URLs for NNTP newsgroup(s).