On 12/5/25 22:49, Philip Yang wrote:
> With this bug MTYPE_UC 0x3 can not update to MTYPE_RW 0x1.
>
> Set AMDGPU_PTE_EXECUTABLE flag if mapping flag set, but should not
> clear it if mapping flag is not set, to only update mtype.
>
> Signed-off-by: Philip Yang <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 97a04e3171f2..d2e8b96c0372 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1194,8 +1194,6 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device
> *adev,
> {
> if (vm_flags & AMDGPU_VM_PAGE_EXECUTABLE)
> *flags |= AMDGPU_PTE_EXECUTABLE;
> - else
> - *flags &= ~AMDGPU_PTE_EXECUTABLE;
That change looks questionable, we said that we always wanted to override the
passed in flags.
Why is that necessary?
>
> switch (vm_flags & AMDGPU_VM_MTYPE_MASK) {
> case AMDGPU_VM_MTYPE_DEFAULT:
> @@ -1204,16 +1202,16 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device
> *adev,
> *flags = AMDGPU_PTE_MTYPE_VG10(*flags, MTYPE_NC);
> break;
> case AMDGPU_VM_MTYPE_WC:
> - *flags |= AMDGPU_PTE_MTYPE_VG10(*flags, MTYPE_WC);
> + *flags = AMDGPU_PTE_MTYPE_VG10(*flags, MTYPE_WC);
> break;
> case AMDGPU_VM_MTYPE_RW:
> - *flags |= AMDGPU_PTE_MTYPE_VG10(*flags, MTYPE_RW);
> + *flags = AMDGPU_PTE_MTYPE_VG10(*flags, MTYPE_RW);
> break;
> case AMDGPU_VM_MTYPE_CC:
> - *flags |= AMDGPU_PTE_MTYPE_VG10(*flags, MTYPE_CC);
> + *flags = AMDGPU_PTE_MTYPE_VG10(*flags, MTYPE_CC);
> break;
> case AMDGPU_VM_MTYPE_UC:
> - *flags |= AMDGPU_PTE_MTYPE_VG10(*flags, MTYPE_UC);
> + *flags = AMDGPU_PTE_MTYPE_VG10(*flags, MTYPE_UC);
> break;
> }
That is a really good catch and should probably get a CC stable as well.
Regards,
Christian.
