AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Lijo Lazar <lijo.lazar@amd.com>,
	Hawking Zhang <Hawking.Zhang@amd.com>,
	Asad Kamal <asad.kamal@amd.com>,
	Alex Deucher <alexander.deucher@amd.com>,
	Sasha Levin <sashal@kernel.org>,
	christian.koenig@amd.com, airlied@gmail.com, simona@ffwll.ch,
	srinivasan.shanmugam@amd.com, Jack.Xiao@amd.com,
	Jesse.zhang@amd.com, siqueira@igalia.com,
	rajneesh.bhardwaj@amd.com, Yunxiang.Li@amd.com,
	tim.huang@amd.com, linux@treblig.org,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.15 056/110] drm/amdgpu: Disallow partition query during reset
Date: Sun,  1 Jun 2025 19:23:38 -0400	[thread overview]
Message-ID: <20250601232435.3507697-56-sashal@kernel.org> (raw)
In-Reply-To: <20250601232435.3507697-1-sashal@kernel.org>

From: Lijo Lazar <lijo.lazar@amd.com>

[ Upstream commit 75f138db48c5c493f0ac198c2579d52fc6a4c4a0 ]

Reject queries to get current partition modes during reset. Also, don't
accept sysfs interface requests to switch compute partition mode while
in reset.

Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>
Reviewed-by: Asad Kamal <asad.kamal@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Based on my analysis of the commit and examination of the kernel tree,
my assessment is: **YES** - This commit should be backported to stable
kernel trees. ## Extensive Analysis ### Code Changes Analysis The commit
adds critical race condition protection in two key areas: **1. Compute
Partition Query Protection (`amdgpu_gfx.c`)** ```c static ssize_t
amdgpu_gfx_get_current_compute_partition(...) { + /bin /bin.usr-is-
merged /boot /dev /etc /home /init /lib /lib.usr-is-merged /lib64
/lost+found /media /mnt /opt /proc /root /run /sbin /sbin.usr-is-merged
/snap /srv /sys /tmp /usr /var Only minimal precaution taken to reject
requests while in reset.*/ + if (amdgpu_in_reset(adev)) + return -EPERM;
mode = amdgpu_xcp_query_partition_mode(adev->xcp_mgr,
AMDGPU_XCP_FL_NONE); return sysfs_emit(buf, "%s\n",
amdgpu_gfx_compute_mode_desc(mode)); } ``` **2. Memory Partition Query
Protection (`amdgpu_gmc.c`)** ```c static ssize_t
current_memory_partition_show(...) { + /bin /bin.usr-is-merged /boot
/dev /etc /home /init /lib /lib.usr-is-merged /lib64 /lost+found /media
/mnt /opt /proc /root /run /sbin /sbin.usr-is-merged /snap /srv /sys
/tmp /usr /var Only minimal precaution taken to reject requests while in
reset model/ prompt/ src/ target/ + if (amdgpu_in_reset(adev)) + return
-EPERM; mode = adev->gmc.gmc_funcs->query_mem_partition_mode(adev);
return sysfs_emit(buf, "%s\n", nps_desc[mode]); } ``` **3. Partition
Switch Protection During Reset** ```c static ssize_t
amdgpu_gfx_set_compute_partition(...) { + /bin /bin.usr-is-merged /boot
/dev /etc /home /init /lib /lib.usr-is-merged /lib64 /lost+found /media
/mnt /opt /proc /root /run /sbin /sbin.usr-is-merged /snap /srv /sys
/tmp /usr /var Don't allow a switch while under reset model/ prompt/
src/ target/ + if (!down_read_trylock(&adev->reset_domain->sem)) +
return -EPERM; ret = amdgpu_xcp_switch_partition_mode(adev->xcp_mgr,
mode); + up_read(&adev->reset_domain->sem); return ret ? ret : count; }
``` ### Why This Should Be Backported **1. Fixes Critical Race
Conditions** - Prevents hardware access during GPU reset when device
state is undefined - Eliminates potential system hangs when userspace
queries partition state during reset - Protects against reading
corrupted/uninitialized hardware registers **2. Follows Established
Kernel Patterns** - My kernel tree analysis shows this matches the
protection pattern used throughout amdgpu - Similar protections exist in
RAS sysfs, shader operations, and other hardware access functions - Uses
the standard `amdgpu_in_reset()` check and reset domain semaphore
locking **3. Low Risk, High Value Change** - **Minimal code impact**:
Only adds early return checks, no functional changes to normal operation
- **No behavioral changes**: When not in reset, functions work exactly
as before - **Clear error handling**: Returns appropriate -EPERM error
during reset - **No architectural modifications**: Uses existing reset
infrastructure **4. Addresses User-Facing Stability Issues** - Partition
modes are commonly queried by system monitoring tools - Race conditions
can cause system instability during GPU recovery scenarios - Fix
prevents potential deadlocks in userspace applications **5. Consistent
with Similar Historical Backports** - The referenced similar commits
show that partition mode and reset-related fixes have been consistently
marked as unsuitable for backporting only when they introduce new
features - This commit purely fixes a race condition without introducing
new functionality - Similar race condition fixes in GPU drivers are
typically high priority for stable trees ### Stability Tree Criteria
Assessment ✅ **Fixes important bug**: Race conditions during reset can
cause system instability ✅ **Small and contained**: Only adds reset
state checks, no complex logic changes ✅ **Minimal side effects**: No
functional changes during normal operation ✅ **No architectural
changes**: Uses existing reset infrastructure ✅ **Critical subsystem**:
GPU reset handling affects system stability ✅ **Follows stable rules**:
Important bugfix with minimal regression risk ### Risk Assessment
**Regression Risk: MINIMAL** - Only adds protective checks that prevent
unsafe operations - No changes to core partition switching or query
logic - Uses well-tested `amdgpu_in_reset()` and semaphore mechanisms
**Impact if Not Backported: HIGH** - Users may experience system hangs
during GPU reset scenarios - Monitoring tools querying partition state
can trigger race conditions - Potential data corruption from reading
undefined hardware state This commit represents a textbook example of a
stability fix that should be backported: it fixes a clear race condition
vulnerability with minimal code changes and zero risk of regression
while addressing a real user-facing problem.

 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 10 ++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c |  4 ++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index cf2df7790077d..1dc06e4ab4970 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -1351,6 +1351,10 @@ static ssize_t amdgpu_gfx_get_current_compute_partition(struct device *dev,
 	struct amdgpu_device *adev = drm_to_adev(ddev);
 	int mode;
 
+	/* Only minimal precaution taken to reject requests while in reset.*/
+	if (amdgpu_in_reset(adev))
+		return -EPERM;
+
 	mode = amdgpu_xcp_query_partition_mode(adev->xcp_mgr,
 					       AMDGPU_XCP_FL_NONE);
 
@@ -1394,8 +1398,14 @@ static ssize_t amdgpu_gfx_set_compute_partition(struct device *dev,
 		return -EINVAL;
 	}
 
+	/* Don't allow a switch while under reset */
+	if (!down_read_trylock(&adev->reset_domain->sem))
+		return -EPERM;
+
 	ret = amdgpu_xcp_switch_partition_mode(adev->xcp_mgr, mode);
 
+	up_read(&adev->reset_domain->sem);
+
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index ecb74ccf1d908..6b0fbbb91e579 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -1230,6 +1230,10 @@ static ssize_t current_memory_partition_show(
 	struct amdgpu_device *adev = drm_to_adev(ddev);
 	enum amdgpu_memory_partition mode;
 
+	/* Only minimal precaution taken to reject requests while in reset */
+	if (amdgpu_in_reset(adev))
+		return -EPERM;
+
 	mode = adev->gmc.gmc_funcs->query_mem_partition_mode(adev);
 	if ((mode >= ARRAY_SIZE(nps_desc)) ||
 	    (BIT(mode) & AMDGPU_ALL_NPS_MASK) != BIT(mode))
-- 
2.39.5


  parent reply	other threads:[~2025-06-01 23:27 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-01 23:22 [PATCH AUTOSEL 6.15 001/110] drm/amd/display: disable DPP RCG before DPP CLK enable Sasha Levin
2025-06-01 23:22 ` [PATCH AUTOSEL 6.15 003/110] drm/amdgpu/gfx6: fix CSIB handling Sasha Levin
2025-06-01 23:22 ` [PATCH AUTOSEL 6.15 008/110] drm/amdgpu: Fix API status offset for MES queue reset Sasha Levin
2025-06-01 23:22 ` [PATCH AUTOSEL 6.15 009/110] drm/amd/display: DCN32 null data check Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 020/110] drm/amdkfd: Drop workaround for GC v9.4.3 revID 0 Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 021/110] drm/amdgpu/gfx11: fix CSIB handling Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 025/110] drm/amd/display: Avoid divide by zero by initializing dummy pitch to 1 Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 029/110] drm/amd/display: Add NULL pointer checks in dm_force_atomic_commit() Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 031/110] drm/amd/display: Skip to enable dsc if it has been off Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 032/110] drm/amdgpu: Add basic validation for RAS header Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 035/110] drm/amd/display: Do Not Consider DSC if Valid Config Not Found Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 037/110] drm/amdgpu/gfx10: fix CSIB handling Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 041/110] drm/amd/display: fix zero value for APU watermark_c Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 043/110] drm/amdgpu/gfx7: fix CSIB handling Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 049/110] drm/amd/display: Update IPS sequential_ono requirement checks Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 050/110] drm/amd/display: Correct SSC enable detection for DCN351 Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 051/110] drm/amd/display: Fix Vertical Interrupt definitions for dcn32, dcn401 Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 055/110] drm/amdgpu: fix MES GFX mask Sasha Levin
2025-06-01 23:23 ` Sasha Levin [this message]
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 059/110] drm/amdgpu/gfx8: fix CSIB handling Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 060/110] drm/amd/display: disable EASF narrow filter sharpening Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 061/110] drm/amdgpu/gfx9: fix CSIB handling Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 062/110] drm/amd/display: Fix VUpdate offset calculations for dcn401 Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 064/110] drm/amd/pm: Reset SMU v13.0.x custom settings Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 065/110] drm/amd/display: Correct prefetch calculation Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 066/110] drm/amd/display: Restructure DMI quirks Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 069/110] drm/amdkfd: Set SDMA_RLCx_IB_CNTL/SWITCH_INSIDE_IB Sasha Levin
2025-06-01 23:23 ` [PATCH AUTOSEL 6.15 076/110] drm/amdgpu: Add indirect L1_TLB_CNTL reg programming for VFs Sasha Levin

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=20250601232435.3507697-56-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=Hawking.Zhang@amd.com \
    --cc=Jack.Xiao@amd.com \
    --cc=Jesse.zhang@amd.com \
    --cc=Yunxiang.Li@amd.com \
    --cc=airlied@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=asad.kamal@amd.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=lijo.lazar@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@treblig.org \
    --cc=patches@lists.linux.dev \
    --cc=rajneesh.bhardwaj@amd.com \
    --cc=simona@ffwll.ch \
    --cc=siqueira@igalia.com \
    --cc=srinivasan.shanmugam@amd.com \
    --cc=stable@vger.kernel.org \
    --cc=tim.huang@amd.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