AMD General
-----Original Message-----
From: Lazar, Lijo <[email protected]>
Sent: Wednesday, September 9, 2026 3:51 AM
To: [email protected]
Cc: Zhang, Hawking <[email protected]>; Deucher, Alexander
<[email protected]>; Kuehling, Felix <[email protected]>;
Joshi, Mukul <[email protected]>; Liu, Xiang(Dean)
<[email protected]>
Subject: [PATCH 1/2] drm/amdgpu: Fix kfd device lock during partition
The compute partition switch locked the kfd device, freed it during the pre-
switch teardown, and then unlocked at the end - by which point
adev->kfd.dev referenced the newly reinitialized device, so the unlock
landed on the wrong object and the per-device lock was never balanced.
Add a dedicated prepare function to unwind and clean up kfd device.
Don't use the global lock during teardown. A new device is created after
partition switch; hence there is no need to do 'unlock' on a different object
after the switch.
User is not supposed to run any process on the device during partition
switch. Keep acquire/release semantics for init_complete so that the device
remains unavailable for a process till its reinitialization is complete.
Signed-off-by: Lijo Lazar <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 19
+++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 6 ++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c | 6 ++++--
drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 11 +----------
drivers/gpu/drm/amd/amdgpu/soc_v1_0.c | 11 +----------
drivers/gpu/drm/amd/amdkfd/kfd_device.c | 10 ++++++++--
drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c | 6 ++++++
7 files changed, 45 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 09feb45584ba..a2a368f1e290 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -245,6 +245,25 @@ void amdgpu_amdkfd_device_fini_sw(struct
amdgpu_device *adev)
}
}
+int amdgpu_amdkfd_prepare_partition_switch(struct amdgpu_device
*adev)
+{
+ struct kfd_dev *kfd = adev->kfd.dev;
+ int r;
+
+ if (!kfd)
+ return 0;
+ r = kgd2kfd_check_and_lock_kfd(kfd);
+ if (r)
+ return r;
+
+ adev->kfd.init_complete = false;
+ kgd2kfd_device_fini(kfd);
+ adev->kfd.dev = NULL;
+ amdgpu_amdkfd_total_mem_size -= adev->gmc.real_vram_size;
+
+ return 0;
+}
+
void amdgpu_amdkfd_interrupt(struct amdgpu_device *adev,
const void *ih_ring_entry)
{
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 1b7dc0d3963b..36e222f3ac5b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -170,6 +170,7 @@ void amdgpu_amdkfd_interrupt(struct
amdgpu_device *adev, void amdgpu_amdkfd_device_probe(struct
amdgpu_device *adev); void amdgpu_amdkfd_device_init(struct
amdgpu_device *adev); void amdgpu_amdkfd_device_fini_sw(struct
amdgpu_device *adev);
+int amdgpu_amdkfd_prepare_partition_switch(struct amdgpu_device
*adev);
int amdgpu_amdkfd_check_and_lock_kfd(struct amdgpu_device *adev);
void amdgpu_amdkfd_unlock_kfd(struct amdgpu_device *adev); int
amdgpu_amdkfd_submit_ib(struct amdgpu_device *adev, @@ -431,6 +432,7
@@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf);
bool kgd2kfd_device_init(struct kfd_dev *kfd,
const struct kgd2kfd_shared_resources
*gpu_resources); void kgd2kfd_device_exit(struct kfd_dev *kfd);
+void kgd2kfd_device_fini(struct kfd_dev *kfd);
void kgd2kfd_suspend(struct kfd_dev *kfd, bool suspend_proc); int
kgd2kfd_resume(struct kfd_dev *kfd, bool resume_proc); void
kgd2kfd_suspend_process(struct kfd_dev *kfd); @@ -485,6 +487,10 @@
static inline void kgd2kfd_device_exit(struct kfd_dev *kfd) { }
+static inline void kgd2kfd_device_fini(struct kfd_dev *kfd) { }
+
Maybe follow the pattern of having open and close branch on new lines as done
for
the other functions.
static inline void kgd2kfd_suspend(struct kfd_dev *kfd, bool suspend_proc) {
} diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
index a8c79a66df1d..92fa15ace438 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
@@ -694,14 +694,16 @@ void
amdgpu_xcp_update_supported_modes(struct amdgpu_xcp_mgr *xcp_mgr)
int amdgpu_xcp_pre_partition_switch(struct amdgpu_xcp_mgr *xcp_mgr,
u32 flags) {
+ int ret = 0;
+
/* TODO:
* Stop user queues and threads, and make sure GPU is empty of
work.
*/
if (flags & AMDGPU_XCP_OPS_KFD)
- amdgpu_amdkfd_device_fini_sw(xcp_mgr->adev);
+ ret = amdgpu_amdkfd_prepare_partition_switch(xcp_mgr-
adev);
- return 0;
+ return ret;
}
int amdgpu_xcp_post_partition_switch(struct amdgpu_xcp_mgr *xcp_mgr,
u32 flags) diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
index cddfe4015f53..699e797679ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
+++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
@@ -391,15 +391,9 @@ static int
aqua_vanjaram_switch_partition_mode(struct amdgpu_xcp_mgr *xcp_mgr,
!adev->in_suspend)
flags |= AMDGPU_XCP_OPS_KFD;
- if (flags & AMDGPU_XCP_OPS_KFD) {
- ret = amdgpu_amdkfd_check_and_lock_kfd(adev);
- if (ret)
- goto out;
- }
-
ret = amdgpu_xcp_pre_partition_switch(xcp_mgr, flags);
if (ret)
- goto unlock;
+ goto out;
num_xcc_per_xcp = __aqua_vanjaram_get_xcc_per_xcp(xcp_mgr,
mode);
if (adev->gfx.funcs->switch_partition_mode)
@@ -413,9 +407,6 @@ static int
aqua_vanjaram_switch_partition_mode(struct amdgpu_xcp_mgr *xcp_mgr,
ret = amdgpu_xcp_post_partition_switch(xcp_mgr, flags);
if (!ret)
__aqua_vanjaram_update_available_partition_mode(xcp_mgr);
-unlock:
- if (flags & AMDGPU_XCP_OPS_KFD)
- amdgpu_amdkfd_unlock_kfd(adev);
out:
return ret;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
index 757da0a30fdc..28eca48c6596 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc_v1_0.c
@@ -1097,15 +1097,9 @@ static int soc_v1_0_switch_partition_mode(struct
amdgpu_xcp_mgr *xcp_mgr,
if (adev->kfd.init_complete && !amdgpu_in_reset(adev))
flags |= AMDGPU_XCP_OPS_KFD;
- if (flags & AMDGPU_XCP_OPS_KFD) {
- ret = amdgpu_amdkfd_check_and_lock_kfd(adev);
- if (ret)
- goto out;
- }
-
ret = amdgpu_xcp_pre_partition_switch(xcp_mgr, flags);
if (ret)
- goto unlock;
+ goto out;
num_xcc_per_xcp = __soc_v1_0_get_xcc_per_xcp(xcp_mgr, mode);
if (adev->gfx.imu.funcs &&
@@ -1126,9 +1120,6 @@ static int soc_v1_0_switch_partition_mode(struct
amdgpu_xcp_mgr *xcp_mgr,
ret = amdgpu_xcp_post_partition_switch(xcp_mgr, flags);
if (!ret)
__soc_v1_0_update_available_partition_mode(xcp_mgr);
-unlock:
- if (flags & AMDGPU_XCP_OPS_KFD)
- amdgpu_amdkfd_unlock_kfd(adev);
out:
return ret;
}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 1c1e7878a2a7..76b308286095 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -952,7 +952,8 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
svm_range_set_max_pages(kfd->adev);
- kfd->init_complete = true;
+ /* Release pairs with the acquire in kfd_init_apertures(). */
+ smp_store_release(&kfd->init_complete, true);
dev_info(kfd_device, "added device %x:%x\n", kfd->adev->pdev-
vendor,
kfd->adev->pdev->device);
@@ -977,7 +978,7 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
return kfd->init_complete;
}
-void kgd2kfd_device_exit(struct kfd_dev *kfd)
+void kgd2kfd_device_fini(struct kfd_dev *kfd)
{
if (kfd->init_complete) {
/* Cleanup KFD nodes */
@@ -991,6 +992,11 @@ void kgd2kfd_device_exit(struct kfd_dev *kfd)
}
kfree(kfd);
+}
+
+void kgd2kfd_device_exit(struct kfd_dev *kfd) {
Open brace should be on the next line.
Maybe run it through checkpatch once if not already done.
+ kgd2kfd_device_fini(kfd);
/* after remove a kfd device unlock kfd driver */
kgd2kfd_unlock_kfd(NULL);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
b/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
index 04c5e26f01ed..da61d939b35c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
@@ -396,6 +396,12 @@ int kfd_init_apertures(struct kfd_process *process)
continue;
}
+ /* acquire pairs with the release in kgd2kfd_device_init() */
+ if (!smp_load_acquire(&dev->kfd->init_complete)) {
Do we need similar smp_load_acquire() at other call sites of this
kfd->init_complete?
