* [PATCH 1/2] drm/amdgpu: return EINVAL instead of ENOENT in the VM code @ 2020-01-22 14:03 Christian König 2020-01-22 14:03 ` [PATCH 2/2] drm/amdgpu: allow higher level PD invalidations Christian König 2020-01-22 18:11 ` [PATCH 1/2] drm/amdgpu: return EINVAL instead of ENOENT in the VM code Luben Tuikov 0 siblings, 2 replies; 5+ messages in thread From: Christian König @ 2020-01-22 14:03 UTC (permalink / raw) To: tom.stdenis, amd-gfx That we can't find a PD above the root is expected can only happen if we try to update a larger range than actually managed by the VM. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 5cb182231f5d..8119f32ca94d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -1475,7 +1475,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params, * shift we should go up one level and check it again. */ if (!amdgpu_vm_pt_ancestor(&cursor)) - return -ENOENT; + return -EINVAL; continue; } -- 2.14.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/amdgpu: allow higher level PD invalidations 2020-01-22 14:03 [PATCH 1/2] drm/amdgpu: return EINVAL instead of ENOENT in the VM code Christian König @ 2020-01-22 14:03 ` Christian König 2020-01-22 22:22 ` Felix Kuehling 2020-01-22 18:11 ` [PATCH 1/2] drm/amdgpu: return EINVAL instead of ENOENT in the VM code Luben Tuikov 1 sibling, 1 reply; 5+ messages in thread From: Christian König @ 2020-01-22 14:03 UTC (permalink / raw) To: tom.stdenis, amd-gfx 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> --- 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 8119f32ca94d..0f79c17118bf 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. */ + 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. */ + nptes = max(nptes, 1u); amdgpu_vm_update_flags(params, pt, cursor.level, pe_start, dst, nptes, incr, flags | AMDGPU_PTE_FRAG(frag)); -- 2.14.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: allow higher level PD invalidations 2020-01-22 14:03 ` [PATCH 2/2] drm/amdgpu: allow higher level PD invalidations Christian König @ 2020-01-22 22:22 ` Felix Kuehling 0 siblings, 0 replies; 5+ messages in thread From: Felix Kuehling @ 2020-01-22 22:22 UTC (permalink / raw) To: Christian König, tom.stdenis, amd-gfx Two style nit-picks inline. Otherwise the series is Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com> On 2020-01-22 9:03 a.m., 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> > --- > 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 8119f32ca94d..0f79c17118bf 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. */ Nit-pick: This comment would upset checkpatch.pl. > + 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: return EINVAL instead of ENOENT in the VM code 2020-01-22 14:03 [PATCH 1/2] drm/amdgpu: return EINVAL instead of ENOENT in the VM code Christian König 2020-01-22 14:03 ` [PATCH 2/2] drm/amdgpu: allow higher level PD invalidations Christian König @ 2020-01-22 18:11 ` Luben Tuikov 2020-01-22 18:52 ` Christian König 1 sibling, 1 reply; 5+ messages in thread From: Luben Tuikov @ 2020-01-22 18:11 UTC (permalink / raw) To: Christian König, tom.stdenis, amd-gfx Do we have user-space/libs rely on this errno in particular? On 2020-01-22 9:03 a.m., Christian König wrote: > That we can't find a PD above the root is expected can only happen if > we try to update a larger range than actually managed by the VM. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 5cb182231f5d..8119f32ca94d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1475,7 +1475,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params, > * shift we should go up one level and check it again. > */ > if (!amdgpu_vm_pt_ancestor(&cursor)) > - return -ENOENT; > + return -EINVAL; > continue; > } > > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: return EINVAL instead of ENOENT in the VM code 2020-01-22 18:11 ` [PATCH 1/2] drm/amdgpu: return EINVAL instead of ENOENT in the VM code Luben Tuikov @ 2020-01-22 18:52 ` Christian König 0 siblings, 0 replies; 5+ messages in thread From: Christian König @ 2020-01-22 18:52 UTC (permalink / raw) To: Luben Tuikov, tom.stdenis, amd-gfx Am 22.01.20 um 19:11 schrieb Luben Tuikov: > Do we have user-space/libs rely on this errno in particular? No, userspace can't even directly call this function. The problem was rather that during debugging I called the function with fixed parameters and wondered for quite a while why it can't find the table in the tree structure. Turned out that the table was perfectly fine, but my parameters had a 0 to much and so was outside of the valid range. Regards, Christian. > > On 2020-01-22 9:03 a.m., Christian König wrote: >> That we can't find a PD above the root is expected can only happen if >> we try to update a larger range than actually managed by the VM. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 5cb182231f5d..8119f32ca94d 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1475,7 +1475,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params, >> * shift we should go up one level and check it again. >> */ >> if (!amdgpu_vm_pt_ancestor(&cursor)) >> - return -ENOENT; >> + return -EINVAL; >> continue; >> } >> >> _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-01-22 22:22 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-01-22 14:03 [PATCH 1/2] drm/amdgpu: return EINVAL instead of ENOENT in the VM code Christian König 2020-01-22 14:03 ` [PATCH 2/2] drm/amdgpu: allow higher level PD invalidations Christian König 2020-01-22 22:22 ` Felix Kuehling 2020-01-22 18:11 ` [PATCH 1/2] drm/amdgpu: return EINVAL instead of ENOENT in the VM code Luben Tuikov 2020-01-22 18:52 ` Christian König
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox