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) +{ +} + 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) +{ + 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)) { + id++; + continue; + } + pdd = kfd_create_process_device_data(dev, process); if (!pdd) { dev_err(dev->adev->dev, -- 2.49.0
