Re: [PATCH 3/5] drm/amdgpu: allow higher level PD invalidations

[Felix Kuehling <felix.kuehling@amd.com>]

On 2020-01-30 7:49, Christian König wrote:
> Allow partial invalidation on unallocated PDs. This is useful when we
> need to silence faults to stop interrupt floods on Vega.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Tested-by: Tom St Denis <tom.stdenis@amd.com>

I already reviewed this a week ago. With two style nit-picks fixed, this 
patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 9705c961405b..6038b3c89633 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1467,9 +1467,8 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
>   			 * smaller than the address shift. Go to the next
>   			 * child entry and try again.
>   			 */
> -			if (!amdgpu_vm_pt_descendant(adev, &cursor))
> -				return -ENOENT;
> -			continue;
> +			if (amdgpu_vm_pt_descendant(adev, &cursor))
> +				continue;
>   		} else if (frag >= parent_shift) {
>   			/* If the fragment size is even larger than the parent
>   			 * shift we should go up one level and check it again.
> @@ -1480,8 +1479,19 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
>   		}
>   
>   		pt = cursor.entry->base.bo;
> -		if (!pt)
> -			return -ENOENT;
> +		if (!pt) {
> +			/* We need all PDs and PTs for mapping something, */
> +			if (flags & AMDGPU_PTE_VALID)
> +				return -ENOENT;
> +
> +			/* but unmapping something can happen at a higher
> +			 * level. */

checkpatch.pl complains about multi-line comments with the */ not on its 
own line.


> +			if (!amdgpu_vm_pt_ancestor(&cursor))
> +				return -EINVAL;
> +
> +			pt = cursor.entry->base.bo;
> +			shift = parent_shift;
> +		}
>   
>   		/* Looks good so far, calculate parameters for the update */
>   		incr = (uint64_t)AMDGPU_GPU_PAGE_SIZE << shift;
> @@ -1495,6 +1505,9 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
>   			uint64_t upd_end = min(entry_end, frag_end);
>   			unsigned nptes = (upd_end - frag_start) >> shift;
>   
> +			/* This can happen when we set higher level PDs to
> +			 * silent to stop fault floods. */

Same as above.


> +			nptes = max(nptes, 1u);
>   			amdgpu_vm_update_flags(params, pt, cursor.level,
>   					       pe_start, dst, nptes, incr,
>   					       flags | AMDGPU_PTE_FRAG(frag));
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx