Don't assume that only the driver would be accessing LNKCTL. ASPM policy changes can trigger write to LNKCTL outside of driver's control. And in the case of upstream bridge, the driver does not even own the device it's changing the registers for.
Use RMW capability accessors which do proper locking to avoid losing concurrent updates to the register value.
Fixes: 8a7cd27679d0 ("drm/radeon/cik: add support for pcie gen1/2/3 switching") Fixes: b9d305dfb66c ("drm/radeon: implement pcie gen2/3 support for SI") Suggested-by: Lukas Wunner lukas@wunner.de Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Cc: stable@vger.kernel.org --- drivers/gpu/drm/radeon/cik.c | 36 ++++++++++------------------------- drivers/gpu/drm/radeon/si.c | 37 ++++++++++-------------------------- 2 files changed, 20 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 5819737c21c6..a6f3c811ceb8 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -9534,17 +9534,8 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp;
- pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &bridge_cfg); - pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL, - &gpu_cfg); - - tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16); - - tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL, - tmp16); + pcie_capability_set_word(root, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD); + pcie_capability_set_word(rdev->pdev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD);
tmp = RREG32_PCIE_PORT(PCIE_LC_STATUS1); max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK) >> LC_DETECTED_LINK_WIDTH_SHIFT; @@ -9591,21 +9582,14 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) msleep(100);
/* linkctl */ - pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (bridge_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(root, PCI_EXP_LNKCTL, - tmp16); - - pcie_capability_read_word(rdev->pdev, - PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (gpu_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(rdev->pdev, - PCI_EXP_LNKCTL, - tmp16); + pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + bridge_cfg & + PCI_EXP_LNKCTL_HAWD); + pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + gpu_cfg & + PCI_EXP_LNKCTL_HAWD);
/* linkctl2 */ pcie_capability_read_word(root, PCI_EXP_LNKCTL2, diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index 8d5e4b25609d..a91012447b56 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -7131,17 +7131,8 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp;
- pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &bridge_cfg); - pcie_capability_read_word(rdev->pdev, PCI_EXP_LNKCTL, - &gpu_cfg); - - tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(root, PCI_EXP_LNKCTL, tmp16); - - tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD; - pcie_capability_write_word(rdev->pdev, PCI_EXP_LNKCTL, - tmp16); + pcie_capability_set_word(root, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD); + pcie_capability_set_word(rdev->pdev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_HAWD);
tmp = RREG32_PCIE(PCIE_LC_STATUS1); max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK) >> LC_DETECTED_LINK_WIDTH_SHIFT; @@ -7188,22 +7179,14 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) msleep(100);
/* linkctl */ - pcie_capability_read_word(root, PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (bridge_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(root, - PCI_EXP_LNKCTL, - tmp16); - - pcie_capability_read_word(rdev->pdev, - PCI_EXP_LNKCTL, - &tmp16); - tmp16 &= ~PCI_EXP_LNKCTL_HAWD; - tmp16 |= (gpu_cfg & PCI_EXP_LNKCTL_HAWD); - pcie_capability_write_word(rdev->pdev, - PCI_EXP_LNKCTL, - tmp16); + pcie_capability_clear_and_set_word(root, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + bridge_cfg & + PCI_EXP_LNKCTL_HAWD); + pcie_capability_clear_and_set_word(rdev->pdev, PCI_EXP_LNKCTL, + PCI_EXP_LNKCTL_HAWD, + gpu_cfg & + PCI_EXP_LNKCTL_HAWD);
/* linkctl2 */ pcie_capability_read_word(root, PCI_EXP_LNKCTL2,
[Public]
-----Original Message----- From: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Sent: Monday, July 17, 2023 8:05 AM 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; Deucher, Alexander Alexander.Deucher@amd.com; Koenig, Christian Christian.Koenig@amd.com; Pan, Xinhui Xinhui.Pan@amd.com; David Airlie airlied@gmail.com; Daniel Vetter daniel@ffwll.ch; amd- gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- kernel@vger.kernel.org Cc: Dean Luick dean.luick@cornelisnetworks.com; Jonas Dreßler verdre@v0yd.nl; Ilpo Järvinen ilpo.jarvinen@linux.intel.com; stable@vger.kernel.org Subject: [PATCH v5 06/11] drm/radeon: Use RMW accessors for changing LNKCTL
Don't assume that only the driver would be accessing LNKCTL. ASPM policy changes can trigger write to LNKCTL outside of driver's control. And in the case of upstream bridge, the driver does not even own the device it's changing the registers for.
Use RMW capability accessors which do proper locking to avoid losing concurrent updates to the register value.
Fixes: 8a7cd27679d0 ("drm/radeon/cik: add support for pcie gen1/2/3 switching") Fixes: b9d305dfb66c ("drm/radeon: implement pcie gen2/3 support for SI") Suggested-by: Lukas Wunner lukas@wunner.de Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Cc: stable@vger.kernel.org
For this and the amdgpu patch: Acked-by: Alex Deucher alexander.deucher@amd.com I'm not sure if this is stable material however. Is there some issue today?
drivers/gpu/drm/radeon/cik.c | 36 ++++++++++------------------------- drivers/gpu/drm/radeon/si.c | 37 ++++++++++-------------------------- 2 files changed, 20 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 5819737c21c6..a6f3c811ceb8 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -9534,17 +9534,8 @@ static void cik_pcie_gen3_enable(struct radeon_device *rdev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp;
pcie_capability_read_word(root, PCI_EXP_LNKCTL,&bridge_cfg);pcie_capability_read_word(rdev->pdev,PCI_EXP_LNKCTL,
&gpu_cfg);tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD;pcie_capability_write_word(root, PCI_EXP_LNKCTL,tmp16);
tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD;pcie_capability_write_word(rdev->pdev,PCI_EXP_LNKCTL,
tmp16);
pcie_capability_set_word(root, PCI_EXP_LNKCTL,PCI_EXP_LNKCTL_HAWD);
pcie_capability_set_word(rdev->pdev,PCI_EXP_LNKCTL, +PCI_EXP_LNKCTL_HAWD);
tmp = RREG32_PCIE_PORT(PCIE_LC_STATUS1); max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK)LC_DETECTED_LINK_WIDTH_SHIFT; @@ -9591,21 +9582,14 @@ static
void cik_pcie_gen3_enable(struct radeon_device *rdev) msleep(100);
/* linkctl */
pcie_capability_read_word(root,PCI_EXP_LNKCTL,
&tmp16);tmp16 &= ~PCI_EXP_LNKCTL_HAWD;tmp16 |= (bridge_cfg &PCI_EXP_LNKCTL_HAWD);
pcie_capability_write_word(root,PCI_EXP_LNKCTL,
tmp16);pcie_capability_read_word(rdev->pdev,PCI_EXP_LNKCTL,&tmp16);tmp16 &= ~PCI_EXP_LNKCTL_HAWD;tmp16 |= (gpu_cfg &PCI_EXP_LNKCTL_HAWD);
pcie_capability_write_word(rdev->pdev,PCI_EXP_LNKCTL,tmp16);
pcie_capability_clear_and_set_word(root,PCI_EXP_LNKCTL,
PCI_EXP_LNKCTL_HAWD,
bridge_cfg &PCI_EXP_LNKCTL_HAWD);
pcie_capability_clear_and_set_word(rdev-pdev, PCI_EXP_LNKCTL,
PCI_EXP_LNKCTL_HAWD,
gpu_cfg &PCI_EXP_LNKCTL_HAWD);
/* linkctl2 */ pcie_capability_read_word(root,PCI_EXP_LNKCTL2, diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c index 8d5e4b25609d..a91012447b56 100644 --- a/drivers/gpu/drm/radeon/si.c +++ b/drivers/gpu/drm/radeon/si.c @@ -7131,17 +7131,8 @@ static void si_pcie_gen3_enable(struct radeon_device *rdev) u16 bridge_cfg2, gpu_cfg2; u32 max_lw, current_lw, tmp;
pcie_capability_read_word(root, PCI_EXP_LNKCTL,&bridge_cfg);pcie_capability_read_word(rdev->pdev,PCI_EXP_LNKCTL,
&gpu_cfg);tmp16 = bridge_cfg | PCI_EXP_LNKCTL_HAWD;pcie_capability_write_word(root, PCI_EXP_LNKCTL,tmp16);
tmp16 = gpu_cfg | PCI_EXP_LNKCTL_HAWD;pcie_capability_write_word(rdev->pdev,PCI_EXP_LNKCTL,
tmp16);
pcie_capability_set_word(root, PCI_EXP_LNKCTL,PCI_EXP_LNKCTL_HAWD);
pcie_capability_set_word(rdev->pdev,PCI_EXP_LNKCTL, +PCI_EXP_LNKCTL_HAWD);
tmp = RREG32_PCIE(PCIE_LC_STATUS1); max_lw = (tmp & LC_DETECTED_LINK_WIDTH_MASK)LC_DETECTED_LINK_WIDTH_SHIFT; @@ -7188,22 +7179,14 @@ static
void si_pcie_gen3_enable(struct radeon_device *rdev) msleep(100);
/* linkctl */
pcie_capability_read_word(root,PCI_EXP_LNKCTL,
&tmp16);tmp16 &= ~PCI_EXP_LNKCTL_HAWD;tmp16 |= (bridge_cfg &PCI_EXP_LNKCTL_HAWD);
pcie_capability_write_word(root,PCI_EXP_LNKCTL,tmp16);pcie_capability_read_word(rdev->pdev,PCI_EXP_LNKCTL,&tmp16);tmp16 &= ~PCI_EXP_LNKCTL_HAWD;tmp16 |= (gpu_cfg &PCI_EXP_LNKCTL_HAWD);
pcie_capability_write_word(rdev->pdev,PCI_EXP_LNKCTL,tmp16);
pcie_capability_clear_and_set_word(root,PCI_EXP_LNKCTL,
PCI_EXP_LNKCTL_HAWD,
bridge_cfg &PCI_EXP_LNKCTL_HAWD);
pcie_capability_clear_and_set_word(rdev-pdev, PCI_EXP_LNKCTL,
PCI_EXP_LNKCTL_HAWD,
gpu_cfg &PCI_EXP_LNKCTL_HAWD);
/* linkctl2 */ pcie_capability_read_word(root,PCI_EXP_LNKCTL2,
2.30.2
On Fri, 18 Aug 2023, Deucher, Alexander wrote:
[Public]
-----Original Message----- From: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Sent: Monday, July 17, 2023 8:05 AM 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; Deucher, Alexander Alexander.Deucher@amd.com; Koenig, Christian Christian.Koenig@amd.com; Pan, Xinhui Xinhui.Pan@amd.com; David Airlie airlied@gmail.com; Daniel Vetter daniel@ffwll.ch; amd- gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- kernel@vger.kernel.org Cc: Dean Luick dean.luick@cornelisnetworks.com; Jonas Dreßler verdre@v0yd.nl; Ilpo Järvinen ilpo.jarvinen@linux.intel.com; stable@vger.kernel.org Subject: [PATCH v5 06/11] drm/radeon: Use RMW accessors for changing LNKCTL
Don't assume that only the driver would be accessing LNKCTL. ASPM policy changes can trigger write to LNKCTL outside of driver's control. And in the case of upstream bridge, the driver does not even own the device it's changing the registers for.
Use RMW capability accessors which do proper locking to avoid losing concurrent updates to the register value.
Fixes: 8a7cd27679d0 ("drm/radeon/cik: add support for pcie gen1/2/3 switching") Fixes: b9d305dfb66c ("drm/radeon: implement pcie gen2/3 support for SI") Suggested-by: Lukas Wunner lukas@wunner.de Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Cc: stable@vger.kernel.org
For this and the amdgpu patch: Acked-by: Alex Deucher alexander.deucher@amd.com I'm not sure if this is stable material however. Is there some issue today?
These were added without Cc stable into pci.git/pcie-rmw.
On Fri, Aug 18, 2023 at 04:12:57PM +0000, Deucher, Alexander wrote:
-----Original Message----- From: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Sent: Monday, July 17, 2023 8:05 AM 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; Deucher, Alexander Alexander.Deucher@amd.com; Koenig, Christian Christian.Koenig@amd.com; Pan, Xinhui Xinhui.Pan@amd.com; David Airlie airlied@gmail.com; Daniel Vetter daniel@ffwll.ch; amd- gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- kernel@vger.kernel.org Cc: Dean Luick dean.luick@cornelisnetworks.com; Jonas Dreßler verdre@v0yd.nl; Ilpo Järvinen ilpo.jarvinen@linux.intel.com; stable@vger.kernel.org Subject: [PATCH v5 06/11] drm/radeon: Use RMW accessors for changing LNKCTL
Don't assume that only the driver would be accessing LNKCTL. ASPM policy changes can trigger write to LNKCTL outside of driver's control. And in the case of upstream bridge, the driver does not even own the device it's changing the registers for.
Use RMW capability accessors which do proper locking to avoid losing concurrent updates to the register value.
Fixes: 8a7cd27679d0 ("drm/radeon/cik: add support for pcie gen1/2/3 switching") Fixes: b9d305dfb66c ("drm/radeon: implement pcie gen2/3 support for SI") Suggested-by: Lukas Wunner lukas@wunner.de Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Cc: stable@vger.kernel.org
For this and the amdgpu patch: Acked-by: Alex Deucher alexander.deucher@amd.com I'm not sure if this is stable material however. Is there some issue today?
Added your ack, thanks! I dropped the stable tag on the whole series.
Bjorn
linux-stable-mirror@lists.linaro.org
-
Bjorn Helgaas -
Deucher, Alexander -
Ilpo Järvinen