From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: linux-pci@vger.kernel.org, "Bjorn Helgaas" <helgaas@kernel.org>,
"Rob Herring" <robh@kernel.org>,
"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
"Krzysztof Wilczyński" <kw@linux.com>,
"Lukas Wunner" <lukas@wunner.de>,
"Dennis Dalessandro" <dennis.dalessandro@cornelisnetworks.com>,
"Jason Gunthorpe" <jgg@ziepe.ca>,
"Leon Romanovsky" <leon@kernel.org>,
linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: [PATCH 06/17] IB/hfi1: Use pcie_lnkctl{,2}_clear_and_set() for changing LNKCTL{,2}
Date: Thu, 11 May 2023 16:14:30 +0300 [thread overview]
Message-ID: <20230511131441.45704-7-ilpo.jarvinen@linux.intel.com> (raw)
In-Reply-To: <20230511131441.45704-1-ilpo.jarvinen@linux.intel.com>
Don't assume that only the driver would be accessing LNKCTL/LNKCTL2.
ASPM policy changes can trigger write to LNKCTL outside of driver's
control. And in the case of upstream (parent), the driver does not even
own the device it's changing the registers for.
Use pcie_lnkctl_clear_and_set() and pcie_lnkctl2_clear_and_set() which
do proper locking to avoid losing concurrent updates to the register
value.
Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/infiniband/hw/hfi1/aspm.c | 16 ++++++----------
drivers/infiniband/hw/hfi1/pcie.c | 28 ++++++----------------------
2 files changed, 12 insertions(+), 32 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/aspm.c b/drivers/infiniband/hw/hfi1/aspm.c
index a3c53be4072c..d3f3b7e9b833 100644
--- a/drivers/infiniband/hw/hfi1/aspm.c
+++ b/drivers/infiniband/hw/hfi1/aspm.c
@@ -66,12 +66,10 @@ static void aspm_hw_enable_l1(struct hfi1_devdata *dd)
return;
/* Enable ASPM L1 first in upstream component and then downstream */
- pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC,
- PCI_EXP_LNKCTL_ASPM_L1);
- pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC,
- PCI_EXP_LNKCTL_ASPM_L1);
+ pcie_lnkctl_clear_and_set(parent, PCI_EXP_LNKCTL_ASPMC,
+ PCI_EXP_LNKCTL_ASPM_L1);
+ pcie_lnkctl_clear_and_set(dd->pcidev, PCI_EXP_LNKCTL_ASPMC,
+ PCI_EXP_LNKCTL_ASPM_L1);
}
void aspm_hw_disable_l1(struct hfi1_devdata *dd)
@@ -79,11 +77,9 @@ void aspm_hw_disable_l1(struct hfi1_devdata *dd)
struct pci_dev *parent = dd->pcidev->bus->self;
/* Disable ASPM L1 first in downstream component and then upstream */
- pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC, 0x0);
+ pcie_lnkctl_clear_and_set(dd->pcidev, PCI_EXP_LNKCTL_ASPMC, 0);
if (parent)
- pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC, 0x0);
+ pcie_lnkctl_clear_and_set(parent, PCI_EXP_LNKCTL_ASPMC, 0);
}
static void aspm_enable(struct hfi1_devdata *dd)
diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
index 08732e1ac966..fe7324d38d64 100644
--- a/drivers/infiniband/hw/hfi1/pcie.c
+++ b/drivers/infiniband/hw/hfi1/pcie.c
@@ -1212,14 +1212,10 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd)
(u32)lnkctl2);
/* only write to parent if target is not as high as ours */
if ((lnkctl2 & PCI_EXP_LNKCTL2_TLS) < target_vector) {
- lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS;
- lnkctl2 |= target_vector;
- dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__,
- (u32)lnkctl2);
- ret = pcie_capability_write_word(parent,
- PCI_EXP_LNKCTL2, lnkctl2);
+ pcie_lnkctl2_clear_and_set(parent, PCI_EXP_LNKCTL2_TLS,
+ target_vector);
if (ret) {
- dd_dev_err(dd, "Unable to write to PCI config\n");
+ dd_dev_err(dd, "Unable to change PCI target speed\n");
return_error = 1;
goto done;
}
@@ -1228,22 +1224,10 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd)
}
dd_dev_info(dd, "%s: setting target link speed\n", __func__);
- ret = pcie_capability_read_word(dd->pcidev, PCI_EXP_LNKCTL2, &lnkctl2);
+ ret = pcie_lnkctl2_clear_and_set(dd->pcidev, PCI_EXP_LNKCTL2_TLS,
+ target_vector);
if (ret) {
- dd_dev_err(dd, "Unable to read from PCI config\n");
- return_error = 1;
- goto done;
- }
-
- dd_dev_info(dd, "%s: ..old link control2: 0x%x\n", __func__,
- (u32)lnkctl2);
- lnkctl2 &= ~PCI_EXP_LNKCTL2_TLS;
- lnkctl2 |= target_vector;
- dd_dev_info(dd, "%s: ..new link control2: 0x%x\n", __func__,
- (u32)lnkctl2);
- ret = pcie_capability_write_word(dd->pcidev, PCI_EXP_LNKCTL2, lnkctl2);
- if (ret) {
- dd_dev_err(dd, "Unable to write to PCI config\n");
+ dd_dev_err(dd, "Unable to change PCI link control2\n");
return_error = 1;
goto done;
}
--
2.30.2
next prev parent reply other threads:[~2023-05-11 13:16 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-11 13:14 [PATCH 00/17] PCI: Improve LNKCTL & LNKCTL2 concurrency control Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 01/17] PCI: Add concurrency safe clear_and_set variants for LNKCTL{,2} Ilpo Järvinen
2023-05-11 15:55 ` Bjorn Helgaas
2023-05-11 17:35 ` Ilpo Järvinen
2023-05-11 19:22 ` Bjorn Helgaas
2023-05-11 19:58 ` Ilpo Järvinen
2023-05-11 20:07 ` Lukas Wunner
2023-05-11 20:28 ` Ilpo Järvinen
2023-05-11 22:21 ` Bjorn Helgaas
2023-05-11 21:27 ` Bjorn Helgaas
2023-05-11 20:23 ` Lukas Wunner
2023-05-12 8:25 ` Ilpo Järvinen
2023-05-14 10:10 ` Lukas Wunner
2023-05-15 11:59 ` Ilpo Järvinen
2023-05-15 18:28 ` Bjorn Helgaas
2023-05-15 22:12 ` Lukas Wunner
2023-05-11 13:14 ` [PATCH 02/17] PCI: pciehp: Protect LNKCTL changes Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 03/17] PCI/ASPM: Use pcie_lnkctl_clear_and_set() Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 04/17] drm/amdgpu: Use pcie_lnkctl{,2}_clear_and_set() for changing LNKCTL{,2} Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 05/17] drm/radeon: " Ilpo Järvinen
2023-05-11 13:14 ` Ilpo Järvinen [this message]
2023-05-11 15:19 ` [PATCH 06/17] IB/hfi1: " Dean Luick
2023-05-11 20:02 ` Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 07/17] e1000e: Use pcie_lnkctl_clear_and_set() for changing LNKCTL Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 08/17] net/mlx5: " Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 09/17] wifi: ath9k: " Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 10/17] mt76: " Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 11/17] Bluetooth: hci_bcm4377: " Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 12/17] misc: rtsx: " Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 13/17] net/tg3: " Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 14/17] r8169: " Ilpo Järvinen
2023-05-11 19:49 ` Heiner Kallweit
2023-05-11 20:00 ` Ilpo Järvinen
2023-05-11 20:10 ` Lukas Wunner
2023-05-11 20:11 ` Heiner Kallweit
2023-05-11 20:02 ` Lukas Wunner
2023-05-11 20:17 ` Heiner Kallweit
2023-05-11 13:14 ` [PATCH 15/17] wifi: ath11k: " Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 16/17] wifi: ath12k: " Ilpo Järvinen
2023-05-11 13:14 ` [PATCH 17/17] wifi: ath10k: " Ilpo Järvinen
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=20230511131441.45704-7-ilpo.jarvinen@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=dennis.dalessandro@cornelisnetworks.com \
--cc=helgaas@kernel.org \
--cc=jgg@ziepe.ca \
--cc=kw@linux.com \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=lukas@wunner.de \
--cc=robh@kernel.org \
/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
all inboxes | Powered by JetHome®