[PATCH 1/2] drm/amdgpu: set the executable flag on unused Vega10 PTEs v2

[Kuehling, Felix]

On 2019-01-03 4:59 a.m., Christian König wrote:
> Otherwise we run into a non-retry fault on access.
>
> It seems to be a hardware bug that the executable bit has
> higher priority than the valid bit.
>
> v2: handle clears as well
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 36 ++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index e73d152659a2..1ab83f45f1ec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -799,9 +799,16 @@ static int amdgpu_vm_clear_bo(struct amdgpu_device *adev,
>  		addr += ats_entries * 8;
>  	}
>  
> -	if (entries)
> +	if (entries) {
> +		uint64_t value = 0;
> +
> +		/* Workaround for fault priority problem on GMC9 */
> +		if (level == AMDGPU_VM_PTB && adev->asic_type >= CHIP_VEGA10)
> +			value = AMDGPU_PTE_EXECUTABLE;
> +
>  		amdgpu_vm_set_pte_pde(adev, &job->ibs[0], addr, 0,
> -				      entries, 0, 0);
> +				      entries, 0, value);
> +	}
>  
>  	amdgpu_ring_pad_ib(ring, &job->ibs[0]);
>  
> @@ -1506,20 +1513,27 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>  }
>  
>  /**
> - * amdgpu_vm_update_huge - figure out parameters for PTE updates
> + * amdgpu_vm_update_flags - figure out flags for PTE updates

Looks like this function does more than just update flags. It does the
PTE update and tweaks the flags as it does so. Maybe it would make sense
to call it something like amdgpu_vm_do_update_ptes.

Either way, this patch is Reviewed-by: Felix Kuehling
<Felix.Kuehling at amd.com>.

Regards,
  Felix

>   *
>   * Make sure to set the right flags for the PTEs at the desired level.
>   */
> -static void amdgpu_vm_update_huge(struct amdgpu_pte_update_params *params,
> -				  struct amdgpu_bo *bo, unsigned level,
> -				  uint64_t pe, uint64_t addr,
> -				  unsigned count, uint32_t incr,
> -				  uint64_t flags)
> +static void amdgpu_vm_update_flags(struct amdgpu_pte_update_params *params,
> +				   struct amdgpu_bo *bo, unsigned level,
> +				   uint64_t pe, uint64_t addr,
> +				   unsigned count, uint32_t incr,
> +				   uint64_t flags)
>  
>  {
>  	if (level != AMDGPU_VM_PTB) {
>  		flags |= AMDGPU_PDE_PTE;
>  		amdgpu_gmc_get_vm_pde(params->adev, level, &addr, &flags);
> +
> +	} else if (params->adev->asic_type >= CHIP_VEGA10 &&
> +		   !(flags & AMDGPU_PTE_VALID) &&
> +		   !(flags & AMDGPU_PTE_PRT)) {
> +
> +		/* Workaround for fault priority problem on GMC9 */
> +		flags |= AMDGPU_PTE_EXECUTABLE;
>  	}
>  
>  	amdgpu_vm_update_func(params, bo, pe, addr, count, incr, flags);
> @@ -1676,9 +1690,9 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>  			uint64_t upd_end = min(entry_end, frag_end);
>  			unsigned nptes = (upd_end - frag_start) >> shift;
>  
> -			amdgpu_vm_update_huge(params, pt, cursor.level,
> -					      pe_start, dst, nptes, incr,
> -					      flags | AMDGPU_PTE_FRAG(frag));
> +			amdgpu_vm_update_flags(params, pt, cursor.level,
> +					       pe_start, dst, nptes, incr,
> +					       flags | AMDGPU_PTE_FRAG(frag));
>  
>  			pe_start += nptes * 8;
>  			dst += (uint64_t)nptes * AMDGPU_GPU_PAGE_SIZE << shift;