From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: linux-pci@vger.kernel.org, "Bjorn Helgaas" <bhelgaas@google.com>,
"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Wilczyński" <kw@linux.com>,
"Emmanuel Grumbach" <emmanuel.grumbach@intel.com>,
"Rafael J . Wysocki" <rafael@kernel.org>,
"Heiner Kallweit" <hkallweit1@gmail.com>,
"Lukas Wunner" <lukas@wunner.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
"Dean Luick" <dean.luick@cornelisnetworks.com>,
"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: [PATCH v2 0/9] PCI: Improve PCIe Capability RMW concurrency control
Date: Wed, 17 May 2023 13:52:26 +0300 [thread overview]
Message-ID: <20230517105235.29176-1-ilpo.jarvinen@linux.intel.com> (raw)
PCI Express Capability RMW accessors don't properly protect against
concurrent access. Link Control Register is written by a number of
things in the kernel in a RMW fashion without any concurrency control.
This could in the unlucky case lead to losing one of the updates. One
of the most obvious path which can race with most of the other LNKCTL
RMW operations seems to be ASPM policy sysfs write which triggers
LNKCTL update. Similarly, Root Control Register can be concurrently
accessed by AER and PME.
Make pcie_capability_clear_and_set_word() (and other RMW accessors that
call it) to use a per device spinlock to protect the RMW operations to
the Capability Registers that require locking. Convert open-coded
LNKCTL RMW operations to use pcie_capability_clear_and_set_word() to
benefit from the locking.
As for LNKCTL2, all current users are safe so I left those changes to
be part of PCIe bandwidth control work I'll send separately later. It
will add LNKCTL2 RMW ops that require locking.
I went with the wrapper approach I suggested to Lukas even if I
personally feel it's over-engineering for a case that doesn't current
exist in the kernel code. That, however, is contained in the first
patch so it can be easily changed without affecting the other patches.
I felt better to do these as fixes first before altering code related
to duplicated ASPM code in drivers. Replacing the custom ASPM code in
drivers with calls to core will not necessarily yield the same states
as previously even if disabling ASPM becomes always possible because
re-enabling ASPM would still be prohibited (some drivers do disable and
enable ASPM back later). Thus, the ASPM "cleanups" would have higher
regression potential.
The series is based on top of the "PCI/ASPM: Handle link retraining
race" patch I sent earlier but is not yet applied.
v2:
- Keep the RMW ops caller API the same
- Make pcie_capability_clear_and_set_word() a wrapper that uses
locked/unlocked variant based on the capability reg
- Extracted LNKCTL2 changes out from this series to keep this purely
a series which fixes something (LNKCTL2 RMW lock is necessary only
when PCIe BW control is introduced).
- Added Fixes tags (it's a bit rathole but yeah, they're there now).
- Renamed cap_lock to pcie_cap_lock
- Changed ath1* to clear the ASPMC field before setting it
Ilpo Järvinen (9):
PCI: Add locking to RMW PCI Express Capability Register accessors
PCI: pciehp: Use RMW accessors for changing LNKCTL
PCI/ASPM: Use RMW accessors for changing LNKCTL
drm/amdgpu: Use RMW accessors for changing LNKCTL
drm/radeon: Use RMW accessors for changing LNKCTL
net/mlx5: Use RMW accessors for changing LNKCTL
wifi: ath11k: Use RMW accessors for changing LNKCTL
wifi: ath12k: Use RMW accessors for changing LNKCTL
wifi: ath10k: Use RMW accessors for changing LNKCTL
drivers/gpu/drm/amd/amdgpu/cik.c | 36 +++++------------
drivers/gpu/drm/amd/amdgpu/si.c | 36 +++++------------
drivers/gpu/drm/radeon/cik.c | 36 +++++------------
drivers/gpu/drm/radeon/si.c | 37 +++++-------------
.../ethernet/mellanox/mlx5/core/fw_reset.c | 9 +----
drivers/net/wireless/ath/ath10k/pci.c | 9 +++--
drivers/net/wireless/ath/ath11k/pci.c | 10 +++--
drivers/net/wireless/ath/ath12k/pci.c | 10 +++--
drivers/pci/access.c | 20 ++++++++--
drivers/pci/hotplug/pciehp_hpc.c | 12 ++----
drivers/pci/pcie/aspm.c | 39 ++++++++-----------
drivers/pci/probe.c | 1 +
include/linux/pci.h | 34 +++++++++++++++-
13 files changed, 128 insertions(+), 161 deletions(-)
--
2.30.2
next reply other threads:[~2023-05-17 10:56 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-17 10:52 Ilpo Järvinen [this message]
2023-05-17 10:52 ` [PATCH v2 1/9] PCI: Add locking to RMW PCI Express Capability Register accessors Ilpo Järvinen
2023-05-17 11:32 ` Rafael J. Wysocki
2023-05-17 10:52 ` [PATCH v2 2/9] PCI: pciehp: Use RMW accessors for changing LNKCTL Ilpo Järvinen
2023-05-17 11:32 ` Rafael J. Wysocki
2023-05-17 10:52 ` [PATCH v2 3/9] PCI/ASPM: " Ilpo Järvinen
2023-05-17 11:33 ` Rafael J. Wysocki
2023-06-16 19:10 ` Lukas Wunner
2023-06-19 14:45 ` Ilpo Järvinen
2023-06-19 15:09 ` Bjorn Helgaas
2023-06-19 16:06 ` Ilpo Järvinen
2023-05-17 10:52 ` [PATCH v2 4/9] drm/amdgpu: " Ilpo Järvinen
2023-05-17 10:52 ` [PATCH v2 5/9] drm/radeon: " Ilpo Järvinen
2023-05-17 10:52 ` [PATCH v2 6/9] net/mlx5: " Ilpo Järvinen
2023-05-17 11:18 ` Moshe Shemesh
2023-05-17 10:52 ` [PATCH v2 7/9] wifi: ath11k: " Ilpo Järvinen
2023-05-17 11:04 ` Kalle Valo
2023-05-17 10:52 ` [PATCH v2 8/9] wifi: ath12k: " Ilpo Järvinen
2023-05-17 11:03 ` Kalle Valo
2023-05-17 10:52 ` [PATCH v2 9/9] wifi: ath10k: " Ilpo Järvinen
2023-05-17 11:05 ` Kalle Valo
2023-05-24 15:10 ` Bjorn Helgaas
2023-05-25 10:11 ` Ilpo Järvinen
2023-05-26 11:48 ` Ilpo Järvinen
2023-05-26 22:26 ` Bjorn Helgaas
2023-05-26 22:20 ` Bjorn Helgaas
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=20230517105235.29176-1-ilpo.jarvinen@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=dean.luick@cornelisnetworks.com \
--cc=emmanuel.grumbach@intel.com \
--cc=hkallweit1@gmail.com \
--cc=kw@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=lukas@wunner.de \
--cc=rafael@kernel.org \
--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®