Re: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

[Kuehling, Felix]

[+Mukul]

On 2026-09-30 05:02, Christian König wrote:
@Felix and @Philip any objections to this patch?
It is actually a bug fix for the NPA support.

As I understand it, this fixes the flags used when unmapping memory from 
the NPA VM. We passed adev->gmc.noretry_flags from 
amdgpu_ualink_unmap_npa_addr to amdgpu_vm_update_range explicitly in the 
flags parameter. I guess that's no longer needed with your fix. But I 
think the end result would be the same, right?

Regards,
  Felix



Regards,
Christian.

On 9/28/26 21:08, Timur Kristóf wrote:
On 2026. szeptember 28., hétfő 11:10:34 keleti államokbeli nyári idő Christian
König wrote:
That was broken since adding the NPA support.

Signed-off-by: Christian König <christian.koe...@amd.com>
Reviewed-by: Timur Kristóf <timur.kris...@gmail.com>

---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64 ++++++++++++-----------
  1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
c03327f1242d3..e8f441e018839 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device
*adev, amdgpu_vm_pt_continue_dfs((start), (entry));                     
\
             (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev),
&(cursor)))
+/* Return the flags used for cleared PDEs/PTES */
+static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
+                                       struct amdgpu_vm *vm,
+                                       unsigned int level)
+{
+       uint64_t flags;
+
+       if (adev->asic_type < CHIP_VEGA10)
+               return 0;
+
+       if (level != AMDGPU_VM_PTB) {
+               uint64_t value = 0;
+
+               flags = AMDGPU_PDE_PTE_FLAG(adev);
+               if (vm->is_npa)
+                       flags |= adev->gmc.noretry_flags;
+               amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
+       } else if (vm->is_npa) {
+               flags = adev->gmc.noretry_flags;
+       } else {
+               /* Workaround for fault priority problem on GMC9 */
+               flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
+       }
+
+       return flags;
+}
+
  /**
   * amdgpu_vm_pt_clear - initially clear the PDs/PTs
   *
@@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
struct amdgpu_vm *vm, struct ttm_operation_ctx ctx = { true, false };
        struct amdgpu_vm_update_params params;
        struct amdgpu_bo *ancestor = &vmbo->bo;
-       unsigned int entries;
        struct amdgpu_bo *bo = &vmbo->bo;
-       uint64_t value = 0, flags = 0;
-       uint64_t addr;
+       unsigned int entries;
+       uint64_t flags;
        int r, idx;

        /* Figure out our place in the hierarchy */
@@ -404,26 +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
struct amdgpu_vm *vm, if (r)
                goto exit;

-       addr = 0;
-
-       if (adev->asic_type >= CHIP_VEGA10) {
-               if (level != AMDGPU_VM_PTB) {
-                       if (vm->is_npa)
-                               flags = adev->gmc.noretry_flags;
-                       /* Handle leaf PDEs as PTEs */
-                       flags |= AMDGPU_PDE_PTE_FLAG(adev);
-                       amdgpu_gmc_get_vm_pde(adev, level,
-                                             &value, &flags);
-               } else if (vm->is_npa) {
-                       flags = adev->gmc.noretry_flags;
-               } else {
-                       /* Workaround for fault priority problem on
GMC9 */
-                       flags = AMDGPU_PTE_EXECUTABLE | adev-
gmc.init_pte_flags;
-               }
-       }
-
-       r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
-                                    value, flags);
+       flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
+       r = vm->update_funcs->update(&params, vmbo, 0, 0, entries, 0,
flags);
        if (r)
                goto exit;

@@ -712,15 +720,9 @@ static void amdgpu_vm_pte_update_flags(struct
amdgpu_vm_update_params *params, flags |=
AMDGPU_PDE_PTE_FLAG(params->adev);
                amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);

-       } else if (adev->asic_type >= CHIP_VEGA10 &&
-                  !(flags & AMDGPU_PTE_VALID) &&
+       } else if (!(flags & AMDGPU_PTE_VALID) &&
                   !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
-
-               /* Workaround for fault priority problem on GMC9 and
GFX12,
-                * EXECUTABLE for GMC9 fault priority and init_pte_flags
-                * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
-                */
-               flags |= AMDGPU_PTE_EXECUTABLE | adev-
gmc.init_pte_flags;
+               flags |= amdgpu_vm_pt_clear_flags(adev, params->vm,
level);
        }

        /*