Re: [Intel-gfx] [PATCH 5/5] drm/i915/gtt: Setup vm callbacks late

2017-02-28 Thread Chris Wilson 
On Tue, Feb 28, 2017 at 05:28:11PM +0200, Mika Kuoppala wrote:
> If we manage to tangle errorpaths and get call to callbacks,
> it is better to defensively keep them as null until object init is
> finished so that we get clean null deref on callsite,
> instead of more cryptic wreckage with partly initialized vm objects.

I wouldn't go so far as saying clean; it's a jump to the NULL function
pointer, which can be quite confusing until you realise why you have
such an odd stack frame.

> Signed-off-by: Mika Kuoppala 

Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 5/5] drm/i915/gtt: Setup vm callbacks late

2017-02-28 Thread Mika Kuoppala 
If we manage to tangle errorpaths and get call to callbacks,
it is better to defensively keep them as null until object init is
finished so that we get clean null deref on callsite,
instead of more cryptic wreckage with partly initialized vm objects.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9c9a03ee..cee9c4f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1338,11 +1338,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
return ret;
}
 
-   ppgtt->base.cleanup = gen8_ppgtt_cleanup;
-   ppgtt->base.unbind_vma = ppgtt_unbind_vma;
-   ppgtt->base.bind_vma = ppgtt_bind_vma;
-   ppgtt->debug_dump = gen8_dump_ppgtt;
-
/* There are only few exceptions for gen >=6. chv and bxt.
 * And we are not sure about the latter so play safe for now.
 */
@@ -1382,6 +1377,11 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
if (intel_vgpu_active(dev_priv))
gen8_ppgtt_notify_vgt(ppgtt, true);
 
+   ppgtt->base.cleanup = gen8_ppgtt_cleanup;
+   ppgtt->base.unbind_vma = ppgtt_unbind_vma;
+   ppgtt->base.bind_vma = ppgtt_bind_vma;
+   ppgtt->debug_dump = gen8_dump_ppgtt;
+
return 0;
 
 free_scratch:
@@ -1808,13 +1808,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
if (ret)
return ret;
 
-   ppgtt->base.clear_range = gen6_ppgtt_clear_range;
-   ppgtt->base.insert_entries = gen6_ppgtt_insert_entries;
-   ppgtt->base.unbind_vma = ppgtt_unbind_vma;
-   ppgtt->base.bind_vma = ppgtt_bind_vma;
-   ppgtt->base.cleanup = gen6_ppgtt_cleanup;
ppgtt->base.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
-   ppgtt->debug_dump = gen6_dump_ppgtt;
 
gen6_scratch_va_range(ppgtt, 0, ppgtt->base.total);
gen6_write_page_range(ppgtt, 0, ppgtt->base.total);
@@ -1825,6 +1819,13 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
return ret;
}
 
+   ppgtt->base.clear_range = gen6_ppgtt_clear_range;
+   ppgtt->base.insert_entries = gen6_ppgtt_insert_entries;
+   ppgtt->base.unbind_vma = ppgtt_unbind_vma;
+   ppgtt->base.bind_vma = ppgtt_bind_vma;
+   ppgtt->base.cleanup = gen6_ppgtt_cleanup;
+   ppgtt->debug_dump = gen6_dump_ppgtt;
+
DRM_DEBUG_DRIVER("Allocated pde space (%lldM) at GTT entry: %llx\n",
 ppgtt->node.size >> 20,
 ppgtt->node.start / PAGE_SIZE);
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/i915/gtt: Setup vm callbacks late

2017-02-22 Thread Chris Wilson 
On Wed, Feb 22, 2017 at 06:54:25PM +0200, Mika Kuoppala wrote:
> If we manage to tangle errorpaths and get call to callbacks,
> it is better to defensively keep them as null until object init is
> finished so that we get clean null deref on callsite,
> instead of more cryptic wreckage with partly initialized vm objects.
> 
> Signed-off-by: Mika Kuoppala 

Series looks ok, but you opened a can of bikeshed! :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 5/5] drm/i915/gtt: Setup vm callbacks late

2017-02-22 Thread Mika Kuoppala 
If we manage to tangle errorpaths and get call to callbacks,
it is better to defensively keep them as null until object init is
finished so that we get clean null deref on callsite,
instead of more cryptic wreckage with partly initialized vm objects.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 791cb81..77276c9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1298,11 +1298,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
return ret;
}
 
-   ppgtt->base.cleanup = gen8_ppgtt_cleanup;
-   ppgtt->base.unbind_vma = ppgtt_unbind_vma;
-   ppgtt->base.bind_vma = ppgtt_bind_vma;
-   ppgtt->debug_dump = gen8_dump_ppgtt;
-
/* There are only few exceptions for gen >=6. chv and bxt.
 * And we are not sure about the latter so play safe for now.
 */
@@ -1317,7 +1312,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
gen8_initialize_pml4(>base, >pml4);
 
ppgtt->switch_mm = gen8_mm_switch_48bit;
-
ppgtt->base.allocate_va_range = gen8_ppgtt_alloc_4lvl;
ppgtt->base.insert_entries = gen8_ppgtt_insert_4lvl;
ppgtt->base.clear_range = gen8_ppgtt_clear_4lvl;
@@ -1326,8 +1320,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
if (ret)
goto free_scratch;
 
-   ppgtt->switch_mm = gen8_mm_switch_32bit;
-
if (intel_vgpu_active(dev_priv)) {
ret = gen8_preallocate_top_level_pdp(ppgtt);
if (ret) {
@@ -1336,6 +1328,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
}
}
 
+   ppgtt->switch_mm = gen8_mm_switch_32bit;
ppgtt->base.allocate_va_range = gen8_ppgtt_alloc_3lvl;
ppgtt->base.insert_entries = gen8_ppgtt_insert_3lvl;
ppgtt->base.clear_range = gen8_ppgtt_clear_3lvl;
@@ -1344,6 +1337,11 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
if (intel_vgpu_active(dev_priv))
gen8_ppgtt_notify_vgt(ppgtt, true);
 
+   ppgtt->base.cleanup = gen8_ppgtt_cleanup;
+   ppgtt->base.unbind_vma = ppgtt_unbind_vma;
+   ppgtt->base.bind_vma = ppgtt_bind_vma;
+   ppgtt->debug_dump = gen8_dump_ppgtt;
+
return 0;
 
 free_scratch:
@@ -1787,13 +1785,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
if (ret)
return ret;
 
-   ppgtt->base.clear_range = gen6_ppgtt_clear_range;
-   ppgtt->base.insert_entries = gen6_ppgtt_insert_entries;
-   ppgtt->base.unbind_vma = ppgtt_unbind_vma;
-   ppgtt->base.bind_vma = ppgtt_bind_vma;
-   ppgtt->base.cleanup = gen6_ppgtt_cleanup;
ppgtt->base.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
-   ppgtt->debug_dump = gen6_dump_ppgtt;
 
gen6_scratch_va_range(ppgtt, 0, ppgtt->base.total);
gen6_write_page_range(ppgtt, 0, ppgtt->base.total);
@@ -1804,6 +1796,13 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
return ret;
}
 
+   ppgtt->base.clear_range = gen6_ppgtt_clear_range;
+   ppgtt->base.insert_entries = gen6_ppgtt_insert_entries;
+   ppgtt->base.unbind_vma = ppgtt_unbind_vma;
+   ppgtt->base.bind_vma = ppgtt_bind_vma;
+   ppgtt->base.cleanup = gen6_ppgtt_cleanup;
+   ppgtt->debug_dump = gen6_dump_ppgtt;
+
DRM_DEBUG_DRIVER("Allocated pde space (%lldM) at GTT entry: %llx\n",
 ppgtt->node.size >> 20,
 ppgtt->node.start / PAGE_SIZE);
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx