From: Philip Yang <[email protected]>

ualink_ppod_setup_commit_store() updated accel_state via
check_ppod_state() but never called deactivate_local_vpod() when the
accelerator was already in READY state. A subsequent vpod config commit
would then call activate_local_vpod() again, resulting in
activate_accelerator() calling amdgpu_ualink_manager_start() a second
time on top of already-initialized NPA VM, causing a NULL pointer
dereference in rb_insert_color when drm_sched_entity nodes already in
the SDMA scheduler FIFO rb-tree were double-initialized.

Fix: call deactivate_accelerator() in ualink_ppod_setup_commit_store()
guarded by accel_state >= READY, under mgpu_info.mutex to serialize with
activate_local_vpod() in ualink_vpod_config_commit_store().

Signed-off-by: Philip Yang <[email protected]>
Reviewed-by: Felix Kuehling <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 6329fc8e217e7..c0478087b95d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -398,6 +398,8 @@ static const struct kobj_type ualink_info_ktype = {
 };
 
 #ifdef UALINK_ENABLE_DEPRECATED_CONFIG_SYSFS
+static void deactivate_accelerator(struct amdgpu_device *adev);
+
 UALINK_VALUE_SHOW(ppod_setup, accel_id,  ppod.accel_id,  "%u");
 UALINK_VALUE_SHOW(ppod_setup, bandwidth, ppod.bandwidth, "%u");
 UALINK_VALUE_SHOW(ppod_setup, latency,   ppod.latency,   "%u");
@@ -452,7 +454,18 @@ static ssize_t ualink_ppod_setup_commit_store(struct 
kobject *kobj,
        if (r)
                return r;
 
+       /*
+        * Hold mgpu_info.mutex to serialize with activate_local_vpod() in
+        * vpod_config_commit_store which also holds this lock. Without it,
+        * deactivate_accelerator() can race with activate_accelerator(),
+        * causing concurrent vm_fini / vm_init on the same NPA VM.
+        */
+       mutex_lock(&mgpu_info.mutex);
+       if (info->accel_state >= AMDGPU_UALINK_ACCEL_STATE_READY)
+               deactivate_accelerator(adev);
+
        info->accel_state = check_ppod_state(adev, setup);
+       mutex_unlock(&mgpu_info.mutex);
 
        /* TODO: If accel_state was ACTIVE, reset all connections */
 
-- 
2.55.0

Reply via email to