* Fix hardware accelerated video playback with amdgpu on 32Bit system @ 2017-03-29 9:18 Jan Burgmeier 2017-03-29 9:18 ` [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems Jan Burgmeier 0 siblings, 1 reply; 11+ messages in thread From: Jan Burgmeier @ 2017-03-29 9:18 UTC (permalink / raw) To: amd-gfx, dri-devel, linux-kernel; +Cc: airlied Hi, on 32Bit systems hardware accelerated video playback with amdgpu always errors out with the following message: "[drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* IB va_start+ib_bytes is invalid." Attached you find a patch witch fixes the problem. The patch was made against the staging-next branch. The used hardware is: HP T630 Thin Client CPU: AMD Embedded G-Series GX-420GI Radeon R7E GPU: Advanced Micro Devices, Inc. [AMD/ATI] Carrizo (rev 88) Regards, Jan Burgmeier ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems 2017-03-29 9:18 Fix hardware accelerated video playback with amdgpu on 32Bit system Jan Burgmeier @ 2017-03-29 9:18 ` Jan Burgmeier 2017-03-29 13:22 ` Christian König 0 siblings, 1 reply; 11+ messages in thread From: Jan Burgmeier @ 2017-03-29 9:18 UTC (permalink / raw) To: amd-gfx, dri-devel, linux-kernel; +Cc: airlied Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 99424cb8020b..583d22974e14 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev, struct amdgpu_bo *aobj = NULL; uint64_t offset; uint8_t *kptr; + uint64_t it_last; m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start, &aobj); @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev, return -EINVAL; } + it_last = m->it.last; if ((chunk_ib->va_start + chunk_ib->ib_bytes) > - (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { + (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) { DRM_ERROR("IB va_start+ib_bytes is invalid\n"); return -EINVAL; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems @ 2017-03-29 13:22 ` Christian König 0 siblings, 0 replies; 11+ messages in thread From: Christian König @ 2017-03-29 13:22 UTC (permalink / raw) To: Jan Burgmeier, amd-gfx, dri-devel, linux-kernel; +Cc: airlied Am 29.03.2017 um 11:18 schrieb Jan Burgmeier: > Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 99424cb8020b..583d22974e14 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev, > struct amdgpu_bo *aobj = NULL; > uint64_t offset; > uint8_t *kptr; > + uint64_t it_last; > > m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start, > &aobj); > @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev, > return -EINVAL; > } > > + it_last = m->it.last; > if ((chunk_ib->va_start + chunk_ib->ib_bytes) > > - (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { > + (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) { Nice catch, but just adding a u64 case should do here as well. E.g: if ((chunk_ib->va_start + chunk_ib->ib_bytes) > (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { With that fixed the patch is Reviewed-by: Christian König <christian.koenig@amd.com>. Regards, Christian. > DRM_ERROR("IB va_start+ib_bytes is invalid\n"); > return -EINVAL; > } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems @ 2017-03-29 13:22 ` Christian König 0 siblings, 0 replies; 11+ messages in thread From: Christian König @ 2017-03-29 13:22 UTC (permalink / raw) To: Jan Burgmeier, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: airlied-cv59FeDIM0c Am 29.03.2017 um 11:18 schrieb Jan Burgmeier: > Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 99424cb8020b..583d22974e14 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev, > struct amdgpu_bo *aobj = NULL; > uint64_t offset; > uint8_t *kptr; > + uint64_t it_last; > > m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start, > &aobj); > @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev, > return -EINVAL; > } > > + it_last = m->it.last; > if ((chunk_ib->va_start + chunk_ib->ib_bytes) > > - (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { > + (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) { Nice catch, but just adding a u64 case should do here as well. E.g: if ((chunk_ib->va_start + chunk_ib->ib_bytes) > (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { With that fixed the patch is Reviewed-by: Christian König <christian.koenig@amd.com>. Regards, Christian. > DRM_ERROR("IB va_start+ib_bytes is invalid\n"); > return -EINVAL; > } _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems @ 2017-03-29 14:54 ` Michel Dänzer 0 siblings, 0 replies; 11+ messages in thread From: Michel Dänzer @ 2017-03-29 14:54 UTC (permalink / raw) To: Christian König, Jan Burgmeier Cc: amd-gfx, dri-devel, linux-kernel, airlied On 29/03/17 10:22 PM, Christian König wrote: > Am 29.03.2017 um 11:18 schrieb Jan Burgmeier: >> Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> index 99424cb8020b..583d22974e14 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >> *adev, >> struct amdgpu_bo *aobj = NULL; >> uint64_t offset; >> uint8_t *kptr; >> + uint64_t it_last; >> m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start, >> &aobj); >> @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >> *adev, >> return -EINVAL; >> } >> + it_last = m->it.last; >> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >> - (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { >> + (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) { > > Nice catch, but just adding a u64 case should do here as well. E.g: > > if ((chunk_ib->va_start + chunk_ib->ib_bytes) > > (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { That won't work correctly if m->it.last == 0xffffffff ? Or is that not possible? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems @ 2017-03-29 14:54 ` Michel Dänzer 0 siblings, 0 replies; 11+ messages in thread From: Michel Dänzer @ 2017-03-29 14:54 UTC (permalink / raw) To: Christian König, Jan Burgmeier Cc: airlied-cv59FeDIM0c, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 29/03/17 10:22 PM, Christian König wrote: > Am 29.03.2017 um 11:18 schrieb Jan Burgmeier: >> Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> index 99424cb8020b..583d22974e14 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >> *adev, >> struct amdgpu_bo *aobj = NULL; >> uint64_t offset; >> uint8_t *kptr; >> + uint64_t it_last; >> m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start, >> &aobj); >> @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >> *adev, >> return -EINVAL; >> } >> + it_last = m->it.last; >> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >> - (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { >> + (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) { > > Nice catch, but just adding a u64 case should do here as well. E.g: > > if ((chunk_ib->va_start + chunk_ib->ib_bytes) > > (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { That won't work correctly if m->it.last == 0xffffffff ? Or is that not possible? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems @ 2017-03-29 15:18 ` Christian König 0 siblings, 0 replies; 11+ messages in thread From: Christian König @ 2017-03-29 15:18 UTC (permalink / raw) To: Michel Dänzer, Jan Burgmeier Cc: amd-gfx, dri-devel, linux-kernel, airlied Am 29.03.2017 um 16:54 schrieb Michel Dänzer: > On 29/03/17 10:22 PM, Christian König wrote: >> Am 29.03.2017 um 11:18 schrieb Jan Burgmeier: >>> Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> index 99424cb8020b..583d22974e14 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >>> *adev, >>> struct amdgpu_bo *aobj = NULL; >>> uint64_t offset; >>> uint8_t *kptr; >>> + uint64_t it_last; >>> m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start, >>> &aobj); >>> @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >>> *adev, >>> return -EINVAL; >>> } >>> + it_last = m->it.last; >>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >>> - (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { >>> + (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) { >> Nice catch, but just adding a u64 case should do here as well. E.g: >> >> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >> (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { > That won't work correctly if m->it.last == 0xffffffff ? Or is that not > possible? Hui, why? is it.last signed? And even then m->it.last probably won't ever become 0xffffffff on a 32bit system. BTW: We need to fix using the 64bit R/B tree instead of the long sized tree for Vega10 here anyway. Regards, Christian. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems @ 2017-03-29 15:18 ` Christian König 0 siblings, 0 replies; 11+ messages in thread From: Christian König @ 2017-03-29 15:18 UTC (permalink / raw) To: Michel Dänzer, Jan Burgmeier Cc: airlied-cv59FeDIM0c, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-kernel-u79uwXL29TY76Z2rM5mHXA Am 29.03.2017 um 16:54 schrieb Michel Dänzer: > On 29/03/17 10:22 PM, Christian König wrote: >> Am 29.03.2017 um 11:18 schrieb Jan Burgmeier: >>> Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> index 99424cb8020b..583d22974e14 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >>> *adev, >>> struct amdgpu_bo *aobj = NULL; >>> uint64_t offset; >>> uint8_t *kptr; >>> + uint64_t it_last; >>> m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start, >>> &aobj); >>> @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >>> *adev, >>> return -EINVAL; >>> } >>> + it_last = m->it.last; >>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >>> - (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { >>> + (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) { >> Nice catch, but just adding a u64 case should do here as well. E.g: >> >> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >> (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { > That won't work correctly if m->it.last == 0xffffffff ? Or is that not > possible? Hui, why? is it.last signed? And even then m->it.last probably won't ever become 0xffffffff on a 32bit system. BTW: We need to fix using the 64bit R/B tree instead of the long sized tree for Vega10 here anyway. Regards, Christian. _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems 2017-03-29 15:18 ` Christian König (?) @ 2017-03-30 1:41 ` Michel Dänzer 2017-03-30 8:36 ` Christian König -1 siblings, 1 reply; 11+ messages in thread From: Michel Dänzer @ 2017-03-30 1:41 UTC (permalink / raw) To: Christian König, Jan Burgmeier Cc: airlied, dri-devel, amd-gfx, linux-kernel On 30/03/17 12:18 AM, Christian König wrote: > Am 29.03.2017 um 16:54 schrieb Michel Dänzer: >> On 29/03/17 10:22 PM, Christian König wrote: >>> Am 29.03.2017 um 11:18 schrieb Jan Burgmeier: >>>> Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com> >>>> --- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>> index 99424cb8020b..583d22974e14 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>> @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >>>> *adev, >>>> struct amdgpu_bo *aobj = NULL; >>>> uint64_t offset; >>>> uint8_t *kptr; >>>> + uint64_t it_last; >>>> m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start, >>>> &aobj); >>>> @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >>>> *adev, >>>> return -EINVAL; >>>> } >>>> + it_last = m->it.last; >>>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >>>> - (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { >>>> + (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) { >>> Nice catch, but just adding a u64 case should do here as well. E.g: >>> >>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >>> (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { >> That won't work correctly if m->it.last == 0xffffffff ? Or is that not >> possible? > Hui, why? is it.last signed? No. If m->it.last == 0xffffffff, (m->it.last + 1) == 0, the u64 cast won't change that. I thought that would be bad, but maybe not? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems @ 2017-03-30 8:36 ` Christian König 0 siblings, 0 replies; 11+ messages in thread From: Christian König @ 2017-03-30 8:36 UTC (permalink / raw) To: Michel Dänzer, Jan Burgmeier Cc: airlied, dri-devel, amd-gfx, linux-kernel Am 30.03.2017 um 03:41 schrieb Michel Dänzer: > On 30/03/17 12:18 AM, Christian König wrote: >> Am 29.03.2017 um 16:54 schrieb Michel Dänzer: >>> On 29/03/17 10:22 PM, Christian König wrote: >>>> Am 29.03.2017 um 11:18 schrieb Jan Burgmeier: >>>>> Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com> >>>>> --- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- >>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>>> index 99424cb8020b..583d22974e14 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>>> @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >>>>> *adev, >>>>> struct amdgpu_bo *aobj = NULL; >>>>> uint64_t offset; >>>>> uint8_t *kptr; >>>>> + uint64_t it_last; >>>>> m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start, >>>>> &aobj); >>>>> @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >>>>> *adev, >>>>> return -EINVAL; >>>>> } >>>>> + it_last = m->it.last; >>>>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >>>>> - (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { >>>>> + (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) { >>>> Nice catch, but just adding a u64 case should do here as well. E.g: >>>> >>>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >>>> (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { >>> That won't work correctly if m->it.last == 0xffffffff ? Or is that not >>> possible? >> Hui, why? is it.last signed? > No. If m->it.last == 0xffffffff, (m->it.last + 1) == 0, the u64 cast > won't change that. I thought that would be bad, but maybe not? Ah, good catch. Yes the u64 cast needs to be inside the (), not outside. Christian. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems @ 2017-03-30 8:36 ` Christian König 0 siblings, 0 replies; 11+ messages in thread From: Christian König @ 2017-03-30 8:36 UTC (permalink / raw) To: Michel Dänzer, Jan Burgmeier Cc: airlied-cv59FeDIM0c, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, linux-kernel-u79uwXL29TY76Z2rM5mHXA Am 30.03.2017 um 03:41 schrieb Michel Dänzer: > On 30/03/17 12:18 AM, Christian König wrote: >> Am 29.03.2017 um 16:54 schrieb Michel Dänzer: >>> On 29/03/17 10:22 PM, Christian König wrote: >>>> Am 29.03.2017 um 11:18 schrieb Jan Burgmeier: >>>>> Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com> >>>>> --- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++- >>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>>> index 99424cb8020b..583d22974e14 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>>> @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >>>>> *adev, >>>>> struct amdgpu_bo *aobj = NULL; >>>>> uint64_t offset; >>>>> uint8_t *kptr; >>>>> + uint64_t it_last; >>>>> m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start, >>>>> &aobj); >>>>> @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device >>>>> *adev, >>>>> return -EINVAL; >>>>> } >>>>> + it_last = m->it.last; >>>>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >>>>> - (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { >>>>> + (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) { >>>> Nice catch, but just adding a u64 case should do here as well. E.g: >>>> >>>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) > >>>> (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) { >>> That won't work correctly if m->it.last == 0xffffffff ? Or is that not >>> possible? >> Hui, why? is it.last signed? > No. If m->it.last == 0xffffffff, (m->it.last + 1) == 0, the u64 cast > won't change that. I thought that would be bad, but maybe not? Ah, good catch. Yes the u64 cast needs to be inside the (), not outside. Christian. _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-03-30 8:36 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-29 9:18 Fix hardware accelerated video playback with amdgpu on 32Bit system Jan Burgmeier 2017-03-29 9:18 ` [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems Jan Burgmeier 2017-03-29 13:22 ` Christian König 2017-03-29 13:22 ` Christian König 2017-03-29 14:54 ` Michel Dänzer 2017-03-29 14:54 ` Michel Dänzer 2017-03-29 15:18 ` Christian König 2017-03-29 15:18 ` Christian König 2017-03-30 1:41 ` Michel Dänzer 2017-03-30 8:36 ` Christian König 2017-03-30 8:36 ` Christian König
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.