public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"Dean Luick" <dean.luick@cornelisnetworks.com>,
	"Jonas Dreßler" <verdre@v0yd.nl>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: [PATCH v5 00/11] PCI: Improve PCIe Capability RMW concurrency control
Date: Mon, 17 Jul 2023 15:04:52 +0300	[thread overview]
Message-ID: <20230717120503.15276-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.

There's also a related series which improves ASPM service driver and
device driver coordination by removing out-of-band ASPM state
management from device drivers (which will remove some of the code
fragments changed by this series but it has higher regression
potential which is why it seems prudent to do these changes in two
steps):
  https://lore.kernel.org/linux-pci/20230602114751.19671-1-ilpo.jarvinen@linux.intel.com/T/#t

v5:
- Remove reversed logic from a conditional
- Use a variable for CCC setup

v4:
- Rebased on top of pci/main
- Added patch to update documentation

v3:
- Split link retraining change off from ASPM patch & reorder it earlier
- Adjust changelog to take into account the move of link retraining
  code into PCI core and no longer refer to ASPM (currently in
  pci/enumeration branch)
- based on top of pci/main

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 (11):
  PCI: Add locking to RMW PCI Express Capability Register accessors
  PCI: Make link retraining use RMW accessors for changing LNKCTL
  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
  PCI: Document the Capability accessor RMW improvements

 Documentation/PCI/pciebus-howto.rst           | 14 ++++---
 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/pci.c                             |  8 +---
 drivers/pci/pcie/aspm.c                       | 30 +++++++--------
 drivers/pci/probe.c                           |  1 +
 include/linux/pci.h                           | 34 ++++++++++++++++-
 15 files changed, 136 insertions(+), 166 deletions(-)

-- 
2.30.2


             reply	other threads:[~2023-07-17 12:05 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-17 12:04 Ilpo Järvinen [this message]
2023-07-17 12:04 ` [PATCH v5 01/11] PCI: Add locking to RMW PCI Express Capability Register accessors Ilpo Järvinen
2023-07-17 12:04 ` [PATCH v5 02/11] PCI: Make link retraining use RMW accessors for changing LNKCTL Ilpo Järvinen
2023-07-17 12:04 ` [PATCH v5 03/11] PCI: pciehp: Use " Ilpo Järvinen
2023-07-17 12:04 ` [PATCH v5 04/11] PCI/ASPM: " Ilpo Järvinen
2023-07-17 12:04 ` [PATCH v5 05/11] drm/amdgpu: " Ilpo Järvinen
2023-07-20 21:55   ` Bjorn Helgaas
2023-07-21  8:07     ` Ilpo Järvinen
2023-07-21 14:52       ` Alex Deucher
2023-08-03 14:12         ` Ilpo Järvinen
2023-07-17 12:04 ` [PATCH v5 06/11] drm/radeon: " Ilpo Järvinen
2023-08-18 16:12   ` Deucher, Alexander
2023-08-21  9:57     ` Ilpo Järvinen
2023-08-21 19:12     ` Bjorn Helgaas
2023-07-17 12:04 ` [PATCH v5 07/11] net/mlx5: " Ilpo Järvinen
2023-07-17 12:05 ` [PATCH v5 08/11] wifi: ath11k: " Ilpo Järvinen
2023-07-17 12:05 ` [PATCH v5 09/11] wifi: ath12k: " Ilpo Järvinen
2023-07-17 12:05 ` [PATCH v5 10/11] wifi: ath10k: " Ilpo Järvinen
2023-07-17 12:05 ` [PATCH v5 11/11] PCI: Document the Capability accessor RMW improvements Ilpo Järvinen
2023-08-10 16:17 ` [PATCH v5 00/11] PCI: Improve PCIe Capability RMW concurrency control 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=20230717120503.15276-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 \
    --cc=verdre@v0yd.nl \
    /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