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?
With the minor nit-picks above fixed, this series is:
Reviewed-by: Mukul Joshi <[email protected]>
> + id++;
> + continue;
> + }
> +
> pdd = kfd_create_process_device_data(dev, process);
> if (!pdd) {
> dev_err(dev->adev->dev,
> --
> 2.49.0
