Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/amdgpu: use atomic functions with memory barriers for vm fault info

The atomic variable vm_fault_info_updated is used to synchronize access to
adev->gmc.vm_fault_info between the interrupt handler and
get_vm_fault_info().

The default atomic functions like atomic_set() and atomic_read() do not
provide memory barriers. This allows for CPU instruction reordering,
meaning the memory accesses to vm_fault_info and the vm_fault_info_updated
flag are not guaranteed to occur in the intended order. This creates a
race condition that can lead to inconsistent or stale data being used.

The previous implementation, which used an explicit mb(), was incomplete
and inefficient. It failed to account for all potential CPU reorderings,
such as the access of vm_fault_info being reordered before the atomic_read
of the flag. This approach is also more verbose and less performant than
using the proper atomic functions with acquire/release semantics.

Fix this by switching to atomic_set_release() and atomic_read_acquire().
These functions provide the necessary acquire and release semantics,
which act as memory barriers to ensure the correct order of operations.
It is also more efficient and idiomatic than using explicit full memory
barriers.

Fixes: b97dfa27ef3a ("drm/amdgpu: save vm fault information for amdkfd")
Cc: stable@vger.kernel.org
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
Signed-off-by: Felix Kuehling <felix.kuehling@amd.com>
Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

authored by

Gui-Dong Han and committed by
Alex Deucher
6df8e84a ff780f4f

+8 -11
+2 -3
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
··· 2329 2329 int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct amdgpu_device *adev, 2330 2330 struct kfd_vm_fault_info *mem) 2331 2331 { 2332 - if (atomic_read(&adev->gmc.vm_fault_info_updated) == 1) { 2332 + if (atomic_read_acquire(&adev->gmc.vm_fault_info_updated) == 1) { 2333 2333 *mem = *adev->gmc.vm_fault_info; 2334 - mb(); /* make sure read happened */ 2335 - atomic_set(&adev->gmc.vm_fault_info_updated, 0); 2334 + atomic_set_release(&adev->gmc.vm_fault_info_updated, 0); 2336 2335 } 2337 2336 return 0; 2338 2337 }
+3 -4
drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
··· 1068 1068 GFP_KERNEL); 1069 1069 if (!adev->gmc.vm_fault_info) 1070 1070 return -ENOMEM; 1071 - atomic_set(&adev->gmc.vm_fault_info_updated, 0); 1071 + atomic_set_release(&adev->gmc.vm_fault_info_updated, 0); 1072 1072 1073 1073 return 0; 1074 1074 } ··· 1290 1290 vmid = REG_GET_FIELD(status, VM_CONTEXT1_PROTECTION_FAULT_STATUS, 1291 1291 VMID); 1292 1292 if (amdgpu_amdkfd_is_kfd_vmid(adev, vmid) 1293 - && !atomic_read(&adev->gmc.vm_fault_info_updated)) { 1293 + && !atomic_read_acquire(&adev->gmc.vm_fault_info_updated)) { 1294 1294 struct kfd_vm_fault_info *info = adev->gmc.vm_fault_info; 1295 1295 u32 protections = REG_GET_FIELD(status, 1296 1296 VM_CONTEXT1_PROTECTION_FAULT_STATUS, ··· 1306 1306 info->prot_read = protections & 0x8 ? true : false; 1307 1307 info->prot_write = protections & 0x10 ? true : false; 1308 1308 info->prot_exec = protections & 0x20 ? true : false; 1309 - mb(); 1310 - atomic_set(&adev->gmc.vm_fault_info_updated, 1); 1309 + atomic_set_release(&adev->gmc.vm_fault_info_updated, 1); 1311 1310 } 1312 1311 1313 1312 return 0;
+3 -4
drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
··· 1183 1183 GFP_KERNEL); 1184 1184 if (!adev->gmc.vm_fault_info) 1185 1185 return -ENOMEM; 1186 - atomic_set(&adev->gmc.vm_fault_info_updated, 0); 1186 + atomic_set_release(&adev->gmc.vm_fault_info_updated, 0); 1187 1187 1188 1188 return 0; 1189 1189 } ··· 1478 1478 vmid = REG_GET_FIELD(status, VM_CONTEXT1_PROTECTION_FAULT_STATUS, 1479 1479 VMID); 1480 1480 if (amdgpu_amdkfd_is_kfd_vmid(adev, vmid) 1481 - && !atomic_read(&adev->gmc.vm_fault_info_updated)) { 1481 + && !atomic_read_acquire(&adev->gmc.vm_fault_info_updated)) { 1482 1482 struct kfd_vm_fault_info *info = adev->gmc.vm_fault_info; 1483 1483 u32 protections = REG_GET_FIELD(status, 1484 1484 VM_CONTEXT1_PROTECTION_FAULT_STATUS, ··· 1494 1494 info->prot_read = protections & 0x8 ? true : false; 1495 1495 info->prot_write = protections & 0x10 ? true : false; 1496 1496 info->prot_exec = protections & 0x20 ? true : false; 1497 - mb(); 1498 - atomic_set(&adev->gmc.vm_fault_info_updated, 1); 1497 + atomic_set_release(&adev->gmc.vm_fault_info_updated, 1); 1499 1498 } 1500 1499 1501 1500 return 0;