* Re: drm/i915/gen8: page directories rework allocation
@ 2015-04-30 10:47 Dan Carpenter
2015-04-30 12:27 ` Dave Gordon
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Dan Carpenter @ 2015-04-30 10:47 UTC (permalink / raw)
To: michel.thierry; +Cc: intel-gfx
Hi Michel,
The patch 69876bed7e00: "drm/i915/gen8: page directories rework
allocation" from Apr 8, 2015, has the following issue:
drivers/gpu/drm/i915/i915_gem_gtt.c:760
warn: too many zeroes
drivers/gpu/drm/i915/i915_gem_gtt.c
746 static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
747 struct i915_page_directory_pointer *pdp,
748 uint64_t start,
749 uint64_t length,
750 unsigned long *new_pds)
751 {
752 struct drm_device *dev = ppgtt->base.dev;
753 struct i915_page_directory *pd;
754 uint64_t temp;
755 uint32_t pdpe;
756
757 WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
758
759 /* FIXME: PPGTT container_of won't work for 64b */
760 WARN_ON((start + length) > 0x800000000ULL);
^
This last zero was not intended.
761
762 gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
763 if (pd)
764 continue;
765
766 pd = alloc_pd_single(dev);
767 if (IS_ERR(pd))
768 goto unwind_out;
769
770 gen8_initialize_pd(&ppgtt->base, pd);
771 pdp->page_directory[pdpe] = pd;
772 set_bit(pdpe, new_pds);
773 }
774
775 return 0;
776
777 unwind_out:
778 for_each_set_bit(pdpe, new_pds, GEN8_LEGACY_PDPES)
779 unmap_and_free_pd(pdp->page_directory[pdpe], dev);
780
781 return -ENOMEM;
782 }
regards,
dan carpenter
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: drm/i915/gen8: page directories rework allocation
2015-04-30 10:47 drm/i915/gen8: page directories rework allocation Dan Carpenter
@ 2015-04-30 12:27 ` Dave Gordon
2015-04-30 13:59 ` [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories Michel Thierry
2015-04-30 15:06 ` [PATCH v2] " Michel Thierry
2 siblings, 0 replies; 13+ messages in thread
From: Dave Gordon @ 2015-04-30 12:27 UTC (permalink / raw)
To: michel.thierry; +Cc: intel-gfx, Dan Carpenter
On 30/04/15 11:47, Dan Carpenter wrote:
> Hi Michel,
>
> The patch 69876bed7e00: "drm/i915/gen8: page directories rework
> allocation" from Apr 8, 2015, has the following issue:
>
> drivers/gpu/drm/i915/i915_gem_gtt.c:760
> warn: too many zeroes
>
> drivers/gpu/drm/i915/i915_gem_gtt.c
> 746 static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
> 747 struct i915_page_directory_pointer *pdp,
> 748 uint64_t start,
> 749 uint64_t length,
> 750 unsigned long *new_pds)
> 751 {
> 752 struct drm_device *dev = ppgtt->base.dev;
> 753 struct i915_page_directory *pd;
> 754 uint64_t temp;
> 755 uint32_t pdpe;
> 756
> 757 WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
> 758
> 759 /* FIXME: PPGTT container_of won't work for 64b */
> 760 WARN_ON((start + length) > 0x800000000ULL);
> ^
> This last zero was not intended.
Wouldn't (1ULL << 31) be clearer?
And shouldn't it be >= not > ?
Or perhaps,
/* FIXME: upper bound must not overflow 31 bits */
WARN_ON((start + length) & (~0ULL << 31));
Pick whichever you think is most comprehensible and least typo-prone!
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 10:47 drm/i915/gen8: page directories rework allocation Dan Carpenter
2015-04-30 12:27 ` Dave Gordon
@ 2015-04-30 13:59 ` Michel Thierry
2015-04-30 14:22 ` Ville Syrjälä
` (2 more replies)
2015-04-30 15:06 ` [PATCH v2] " Michel Thierry
2 siblings, 3 replies; 13+ messages in thread
From: Michel Thierry @ 2015-04-30 13:59 UTC (permalink / raw)
To: intel-gfx
The patch 69876bed7e008f5fe01538a2d47c09f2862129d0: "drm/i915/gen8:
page directories rework allocation" added an overflow warning, but the
mask had an extra 0. Use typo-prone option suggested by Dave instead.
This check will be unnecessary after gen8_alloc_va_range handles more
than 4 PDPs (48b addressing).
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6fae6bd..6d894fc 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -756,8 +756,8 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
- /* FIXME: PPGTT container_of won't work for 64b */
- WARN_ON((start + length) > 0x800000000ULL);
+ /* FIXME: upper bound must not overflow 31 bits */
+ WARN_ON((start + length) & (~0ULL << 31));
gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
if (pd)
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 13:59 ` [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories Michel Thierry
@ 2015-04-30 14:22 ` Ville Syrjälä
2015-04-30 14:33 ` Michel Thierry
2015-05-02 0:38 ` shuang.he
2015-05-04 12:57 ` David Weinehall
2 siblings, 1 reply; 13+ messages in thread
From: Ville Syrjälä @ 2015-04-30 14:22 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
On Thu, Apr 30, 2015 at 02:59:34PM +0100, Michel Thierry wrote:
> The patch 69876bed7e008f5fe01538a2d47c09f2862129d0: "drm/i915/gen8:
> page directories rework allocation" added an overflow warning, but the
> mask had an extra 0. Use typo-prone option suggested by Dave instead.
>
> This check will be unnecessary after gen8_alloc_va_range handles more
> than 4 PDPs (48b addressing).
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 6fae6bd..6d894fc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -756,8 +756,8 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
>
> WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
>
> - /* FIXME: PPGTT container_of won't work for 64b */
> - WARN_ON((start + length) > 0x800000000ULL);
> + /* FIXME: upper bound must not overflow 31 bits */
> + WARN_ON((start + length) & (~0ULL << 31));
Why is it 31 and not 32?
>
> gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
> if (pd)
> --
> 2.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 14:22 ` Ville Syrjälä
@ 2015-04-30 14:33 ` Michel Thierry
2015-04-30 14:53 ` Dave Gordon
0 siblings, 1 reply; 13+ messages in thread
From: Michel Thierry @ 2015-04-30 14:33 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On 4/30/2015 3:22 PM, Ville Syrjälä wrote:
> On Thu, Apr 30, 2015 at 02:59:34PM +0100, Michel Thierry wrote:
>> The patch 69876bed7e008f5fe01538a2d47c09f2862129d0: "drm/i915/gen8:
>> page directories rework allocation" added an overflow warning, but the
>> mask had an extra 0. Use typo-prone option suggested by Dave instead.
>>
>> This check will be unnecessary after gen8_alloc_va_range handles more
>> than 4 PDPs (48b addressing).
>>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Cc: Dave Gordon <david.s.gordon@intel.com>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 6fae6bd..6d894fc 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -756,8 +756,8 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
>>
>> WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
>>
>> - /* FIXME: PPGTT container_of won't work for 64b */
>> - WARN_ON((start + length) > 0x800000000ULL);
>> + /* FIXME: upper bound must not overflow 31 bits */
>> + WARN_ON((start + length) & (~0ULL << 31));
>
> Why is it 31 and not 32?
>
Right, the check really should be (start + length) >= 0x100000000ULL.
>>
>> gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
>> if (pd)
>> --
>> 2.1.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 14:33 ` Michel Thierry
@ 2015-04-30 14:53 ` Dave Gordon
2015-04-30 14:56 ` Michel Thierry
2015-04-30 14:58 ` Ville Syrjälä
0 siblings, 2 replies; 13+ messages in thread
From: Dave Gordon @ 2015-04-30 14:53 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
On 30/04/15 15:33, Michel Thierry wrote:
> On 4/30/2015 3:22 PM, Ville Syrjälä wrote:
>> On Thu, Apr 30, 2015 at 02:59:34PM +0100, Michel Thierry wrote:
>>> The patch 69876bed7e008f5fe01538a2d47c09f2862129d0: "drm/i915/gen8:
>>> page directories rework allocation" added an overflow warning, but the
>>> mask had an extra 0. Use typo-prone option suggested by Dave instead.
>>>
>>> This check will be unnecessary after gen8_alloc_va_range handles more
>>> than 4 PDPs (48b addressing).
>>>
>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>> Cc: Dave Gordon <david.s.gordon@intel.com>
>>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>> index 6fae6bd..6d894fc 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>> @@ -756,8 +756,8 @@ static int
>>> gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
>>>
>>> WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
>>>
>>> - /* FIXME: PPGTT container_of won't work for 64b */
>>> - WARN_ON((start + length) > 0x800000000ULL);
>>> + /* FIXME: upper bound must not overflow 31 bits */
>>> + WARN_ON((start + length) & (~0ULL << 31));
>>
>> Why is it 31 and not 32?
>
> Right, the check really should be (start + length) >= 0x100000000ULL.
Something with '32' in it might be more obvious and save anyone having
to count the zeroes ... and a comment that also mentioned the limit:
/* FIXME: for now, upper bound must fit in 32 bits */
WARN_ON((start + length) >= (1ULL << 32))
WARN_ON((start + length) & (~0ULL << 32))
WARN_ON((start + length) >> 32) != 0)
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 14:53 ` Dave Gordon
@ 2015-04-30 14:56 ` Michel Thierry
2015-04-30 14:58 ` Ville Syrjälä
1 sibling, 0 replies; 13+ messages in thread
From: Michel Thierry @ 2015-04-30 14:56 UTC (permalink / raw)
To: Dave Gordon; +Cc: intel-gfx
On 4/30/2015 3:53 PM, Dave Gordon wrote:
> On 30/04/15 15:33, Michel Thierry wrote:
>> On 4/30/2015 3:22 PM, Ville Syrjälä wrote:
>>> On Thu, Apr 30, 2015 at 02:59:34PM +0100, Michel Thierry wrote:
>>>> The patch 69876bed7e008f5fe01538a2d47c09f2862129d0: "drm/i915/gen8:
>>>> page directories rework allocation" added an overflow warning, but the
>>>> mask had an extra 0. Use typo-prone option suggested by Dave instead.
>>>>
>>>> This check will be unnecessary after gen8_alloc_va_range handles more
>>>> than 4 PDPs (48b addressing).
>>>>
>>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>> Cc: Dave Gordon <david.s.gordon@intel.com>
>>>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>> index 6fae6bd..6d894fc 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>> @@ -756,8 +756,8 @@ static int
>>>> gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
>>>>
>>>> WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
>>>>
>>>> - /* FIXME: PPGTT container_of won't work for 64b */
>>>> - WARN_ON((start + length) > 0x800000000ULL);
>>>> + /* FIXME: upper bound must not overflow 31 bits */
>>>> + WARN_ON((start + length) & (~0ULL << 31));
>>>
>>> Why is it 31 and not 32?
>>
>> Right, the check really should be (start + length) >= 0x100000000ULL.
>
> Something with '32' in it might be more obvious and save anyone having
> to count the zeroes ... and a comment that also mentioned the limit:
>
> /* FIXME: for now, upper bound must fit in 32 bits */
>
> WARN_ON((start + length) >= (1ULL << 32))
> WARN_ON((start + length) & (~0ULL << 32))
> WARN_ON((start + length) >> 32) != 0)
Yes, I was planning to just replace _31_ with _32_ and avoid all those
zeroes...
>
> .Dave.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 14:53 ` Dave Gordon
2015-04-30 14:56 ` Michel Thierry
@ 2015-04-30 14:58 ` Ville Syrjälä
1 sibling, 0 replies; 13+ messages in thread
From: Ville Syrjälä @ 2015-04-30 14:58 UTC (permalink / raw)
To: Dave Gordon; +Cc: intel-gfx
On Thu, Apr 30, 2015 at 03:53:29PM +0100, Dave Gordon wrote:
> On 30/04/15 15:33, Michel Thierry wrote:
> > On 4/30/2015 3:22 PM, Ville Syrjälä wrote:
> >> On Thu, Apr 30, 2015 at 02:59:34PM +0100, Michel Thierry wrote:
> >>> The patch 69876bed7e008f5fe01538a2d47c09f2862129d0: "drm/i915/gen8:
> >>> page directories rework allocation" added an overflow warning, but the
> >>> mask had an extra 0. Use typo-prone option suggested by Dave instead.
> >>>
> >>> This check will be unnecessary after gen8_alloc_va_range handles more
> >>> than 4 PDPs (48b addressing).
> >>>
> >>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >>> Cc: Dave Gordon <david.s.gordon@intel.com>
> >>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> >>> ---
> >>> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>> index 6fae6bd..6d894fc 100644
> >>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>> @@ -756,8 +756,8 @@ static int
> >>> gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
> >>>
> >>> WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
> >>>
> >>> - /* FIXME: PPGTT container_of won't work for 64b */
> >>> - WARN_ON((start + length) > 0x800000000ULL);
> >>> + /* FIXME: upper bound must not overflow 31 bits */
> >>> + WARN_ON((start + length) & (~0ULL << 31));
> >>
> >> Why is it 31 and not 32?
> >
> > Right, the check really should be (start + length) >= 0x100000000ULL.
>
> Something with '32' in it might be more obvious and save anyone having
> to count the zeroes ... and a comment that also mentioned the limit:
>
> /* FIXME: for now, upper bound must fit in 32 bits */
>
> WARN_ON((start + length) >= (1ULL << 32))
This would match the '1ULL<<32' used in the ppgtt init.
> WARN_ON((start + length) & (~0ULL << 32))
> WARN_ON((start + length) >> 32) != 0)
>
> .Dave.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 13:59 ` [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories Michel Thierry
2015-04-30 14:22 ` Ville Syrjälä
@ 2015-05-02 0:38 ` shuang.he
2015-05-04 12:57 ` David Weinehall
2 siblings, 0 replies; 13+ messages in thread
From: shuang.he @ 2015-05-02 0:38 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, michel.thierry
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6299
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 302/302 302/302
SNB 316/316 316/316
IVB 264/264 264/264
BYT -3 227/227 224/227
BDW 318/318 318/318
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@gem_dummy_reloc_loop@render FAIL(1)PASS(18) TIMEOUT(1)PASS(1)
*BYT igt@gem_exec_parse@bitmasks FAIL(1)PASS(7) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
BYT igt@gem_pipe_control_store_loop@fresh-buffer FAIL(1)TIMEOUT(10)PASS(9) TIMEOUT(2)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 13:59 ` [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories Michel Thierry
2015-04-30 14:22 ` Ville Syrjälä
2015-05-02 0:38 ` shuang.he
@ 2015-05-04 12:57 ` David Weinehall
2 siblings, 0 replies; 13+ messages in thread
From: David Weinehall @ 2015-05-04 12:57 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
On Thu, Apr 30, 2015 at 02:59:34PM +0100, Michel Thierry wrote:
> The patch 69876bed7e008f5fe01538a2d47c09f2862129d0: "drm/i915/gen8:
> page directories rework allocation" added an overflow warning, but the
> mask had an extra 0. Use typo-prone option suggested by Dave instead.
I think you mean "less typo-prone" here :)
Kind regards, David Weinehall
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 10:47 drm/i915/gen8: page directories rework allocation Dan Carpenter
2015-04-30 12:27 ` Dave Gordon
2015-04-30 13:59 ` [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories Michel Thierry
@ 2015-04-30 15:06 ` Michel Thierry
2015-05-02 4:26 ` shuang.he
2015-05-06 10:14 ` Daniel Vetter
2 siblings, 2 replies; 13+ messages in thread
From: Michel Thierry @ 2015-04-30 15:06 UTC (permalink / raw)
To: intel-gfx
The patch 69876bed7e008f5fe01538a2d47c09f2862129d0: "drm/i915/gen8:
page directories rework allocation" added an overflow warning, but the
mask had an extra 0. Use typo-prone option suggested by Dave instead,
to check for (start + length) >= 0x100000000ULL.
This check will be unnecessary after gen8_alloc_va_range handles more
than 4 PDPs (48b addressing).
v2: Really check for 32b overflow (Ville)
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6fae6bd..3450cb5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -756,8 +756,8 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
- /* FIXME: PPGTT container_of won't work for 64b */
- WARN_ON((start + length) > 0x800000000ULL);
+ /* FIXME: upper bound must not overflow 32 bits */
+ WARN_ON((start + length) >= (1ULL << 32));
gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
if (pd)
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 15:06 ` [PATCH v2] " Michel Thierry
@ 2015-05-02 4:26 ` shuang.he
2015-05-06 10:14 ` Daniel Vetter
1 sibling, 0 replies; 13+ messages in thread
From: shuang.he @ 2015-05-02 4:26 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, michel.thierry
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6300
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 302/302 302/302
SNB 316/316 316/316
IVB 264/264 264/264
BYT -3 227/227 224/227
BDW 318/318 318/318
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@gem_dummy_reloc_loop@render FAIL(1)PASS(18) TIMEOUT(1)PASS(1)
BYT igt@gem_pipe_control_store_loop@fresh-buffer FAIL(1)TIMEOUT(10)PASS(9) TIMEOUT(1)PASS(1)
*BYT igt@gem_threaded_access_tiled FAIL(1)PASS(6) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories
2015-04-30 15:06 ` [PATCH v2] " Michel Thierry
2015-05-02 4:26 ` shuang.he
@ 2015-05-06 10:14 ` Daniel Vetter
1 sibling, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2015-05-06 10:14 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
On Thu, Apr 30, 2015 at 04:06:51PM +0100, Michel Thierry wrote:
> The patch 69876bed7e008f5fe01538a2d47c09f2862129d0: "drm/i915/gen8:
> page directories rework allocation" added an overflow warning, but the
> mask had an extra 0. Use typo-prone option suggested by Dave instead,
I inserted the "less" above while applying.
> to check for (start + length) >= 0x100000000ULL.
>
> This check will be unnecessary after gen8_alloc_va_range handles more
> than 4 PDPs (48b addressing).
>
> v2: Really check for 32b overflow (Ville)
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 6fae6bd..3450cb5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -756,8 +756,8 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
>
> WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
>
> - /* FIXME: PPGTT container_of won't work for 64b */
> - WARN_ON((start + length) > 0x800000000ULL);
> + /* FIXME: upper bound must not overflow 32 bits */
> + WARN_ON((start + length) >= (1ULL << 32));
>
> gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
> if (pd)
> --
> 2.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-05-06 10:12 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-30 10:47 drm/i915/gen8: page directories rework allocation Dan Carpenter
2015-04-30 12:27 ` Dave Gordon
2015-04-30 13:59 ` [PATCH] drm/i915: Fix 32b overflow check in gen8_ppgtt_alloc_page_directories Michel Thierry
2015-04-30 14:22 ` Ville Syrjälä
2015-04-30 14:33 ` Michel Thierry
2015-04-30 14:53 ` Dave Gordon
2015-04-30 14:56 ` Michel Thierry
2015-04-30 14:58 ` Ville Syrjälä
2015-05-02 0:38 ` shuang.he
2015-05-04 12:57 ` David Weinehall
2015-04-30 15:06 ` [PATCH v2] " Michel Thierry
2015-05-02 4:26 ` shuang.he
2015-05-06 10:14 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox