[Intel-gfx] [PATCH 1/2] drm/i915: Remove guard page insertion around unevictable nodes

[Intel-gfx] [PATCH 1/2] drm/i915: Remove guard page insertion around unevictable nodes

[Chris Wilson]

Assume that unevictable nodes are not in the GTT and so we can ignore
page boundary concerns, and so allow regular nodes to abutt against
irregular unevictable nodes.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h       | 2 --
 drivers/gpu/drm/i915/i915_gem_evict.c | 6 ++++--
 drivers/gpu/drm/i915/i915_vma.h       | 9 ++++++++-
 drivers/gpu/drm/i915/i915_vma_types.h | 2 ++
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 99cf861df92d..69c5a185ecff 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -357,8 +357,6 @@ enum i915_cache_level {
 	I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */
 };
 
-#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
-
 struct intel_fbc {
 	/* This is always the inner lock when overlapping with struct_mutex and
 	 * it's the outer lock when overlapping with stolen_lock. */
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 4d2d59a9942b..aef88fdb9f66 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -313,11 +313,13 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
 		 */
 		if (i915_vm_has_cache_coloring(vm)) {
 			if (node->start + node->size == target->start) {
-				if (node->color == target->color)
+				if (i915_node_color_matches(node,
+							    target->color))
 					continue;
 			}
 			if (node->start == target->start + target->size) {
-				if (node->color == target->color)
+				if (i915_node_color_matches(node,
+							    target->color))
 					continue;
 			}
 		}
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index a64adc8c883b..609e8be142e6 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -283,10 +283,17 @@ static inline bool i915_vma_is_bound(const struct i915_vma *vma,
 	return atomic_read(&vma->flags) & where;
 }
 
+static inline bool i915_node_color_matches(const struct drm_mm_node *node,
+					   unsigned long color)
+{
+	return color == I915_COLOR_UNEVICTABLE || node->color == color;
+}
+
 static inline bool i915_node_color_differs(const struct drm_mm_node *node,
 					   unsigned long color)
 {
-	return drm_mm_node_allocated(node) && node->color != color;
+	return drm_mm_node_allocated(node) &&
+		!i915_node_color_matches(node, color);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
index f5cb848b7a7e..e72a07692a64 100644
--- a/drivers/gpu/drm/i915/i915_vma_types.h
+++ b/drivers/gpu/drm/i915/i915_vma_types.h
@@ -95,6 +95,8 @@ enum i915_cache_level;
  *
  */
 
+#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
+
 struct intel_remapped_plane_info {
 	/* in gtt pages */
 	unsigned int width, height, stride, offset;
-- 
2.20.1

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

[Intel-gfx] [PATCH 2/2] drm/i915/gt: Always try to reserve GGTT address 0x0

[Chris Wilson]

Since writing to address 0 is a very common mistake, let's try to avoid
putting anything sensitive there.

References: https://gitlab.freedesktop.org/drm/intel/-/issues/2989
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_ggtt.c | 40 +++++++++++++++++++---------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index dac07d66f658..3a737d4fbc3c 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -535,16 +535,32 @@ static int init_ggtt(struct i915_ggtt *ggtt)
 
 	mutex_init(&ggtt->error_mutex);
 	if (ggtt->mappable_end) {
-		/* Reserve a mappable slot for our lockless error capture */
-		ret = drm_mm_insert_node_in_range(&ggtt->vm.mm,
-						  &ggtt->error_capture,
-						  PAGE_SIZE, 0,
-						  I915_COLOR_UNEVICTABLE,
-						  0, ggtt->mappable_end,
-						  DRM_MM_INSERT_LOW);
-		if (ret)
-			return ret;
+		/*
+		 * Reserve a mappable slot for our lockless error capture.
+		 *
+		 * We strongly prefer taking address 0x0 in order to protect
+		 * other critical buffers against accidental overwrites,
+		 * as writing to address 0 is a very common mistake.
+		 *
+		 * Since 0 may already be in use by the system (e.g. the BIOS
+		 * framebuffer), we let the reservation fail quietly and hope
+		 * 0 remains reserved always.
+		 */
+		ggtt->error_capture.size = I915_GTT_PAGE_SIZE;
+		ggtt->error_capture.color = I915_COLOR_UNEVICTABLE;
+		if (drm_mm_reserve_node(&ggtt->vm.mm, &ggtt->error_capture))
+			drm_mm_insert_node_in_range(&ggtt->vm.mm,
+						    &ggtt->error_capture,
+						    ggtt->error_capture.size, 0,
+						    ggtt->error_capture.color,
+						    0, ggtt->mappable_end,
+						    DRM_MM_INSERT_LOW);
 	}
+	if (drm_mm_node_allocated(&ggtt->error_capture))
+		drm_dbg(&ggtt->vm.i915->drm,
+			"Reserved GGTT:[%llx, %llx] for use by error capture\n",
+			ggtt->error_capture.start,
+			ggtt->error_capture.start + ggtt->error_capture.size);
 
 	/*
 	 * The upper portion of the GuC address space has a sizeable hole
@@ -557,9 +573,9 @@ static int init_ggtt(struct i915_ggtt *ggtt)
 
 	/* Clear any non-preallocated blocks */
 	drm_mm_for_each_hole(entry, &ggtt->vm.mm, hole_start, hole_end) {
-		drm_dbg_kms(&ggtt->vm.i915->drm,
-			    "clearing unused GTT space: [%lx, %lx]\n",
-			    hole_start, hole_end);
+		drm_dbg(&ggtt->vm.i915->drm,
+			"clearing unused GTT space: [%lx, %lx]\n",
+			hole_start, hole_end);
 		ggtt->vm.clear_range(&ggtt->vm, hole_start,
 				     hole_end - hole_start);
 	}
-- 
2.20.1

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

[Intel-gfx] [PATCH] drm/i915: Remove guard page insertion around unevictable nodes

[Chris Wilson]

Assume that unevictable nodes are not in the GTT and so we can ignore
page boundary concerns, and so allow regular nodes to abutt against
irregular unevictable nodes.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h       |  2 --
 drivers/gpu/drm/i915/i915_gem_evict.c |  6 ++++--
 drivers/gpu/drm/i915/i915_vma.h       | 10 +++++++++-
 drivers/gpu/drm/i915/i915_vma_types.h |  2 ++
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 99cf861df92d..69c5a185ecff 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -357,8 +357,6 @@ enum i915_cache_level {
 	I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */
 };
 
-#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
-
 struct intel_fbc {
 	/* This is always the inner lock when overlapping with struct_mutex and
 	 * it's the outer lock when overlapping with stolen_lock. */
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 4d2d59a9942b..aef88fdb9f66 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -313,11 +313,13 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
 		 */
 		if (i915_vm_has_cache_coloring(vm)) {
 			if (node->start + node->size == target->start) {
-				if (node->color == target->color)
+				if (i915_node_color_matches(node,
+							    target->color))
 					continue;
 			}
 			if (node->start == target->start + target->size) {
-				if (node->color == target->color)
+				if (i915_node_color_matches(node,
+							    target->color))
 					continue;
 			}
 		}
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index a64adc8c883b..dac953815118 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -283,10 +283,18 @@ static inline bool i915_vma_is_bound(const struct i915_vma *vma,
 	return atomic_read(&vma->flags) & where;
 }
 
+static inline bool i915_node_color_matches(const struct drm_mm_node *node,
+					   unsigned long color)
+{
+	return (node->color | color) == I915_COLOR_UNEVICTABLE ||
+	       	node->color == color;
+}
+
 static inline bool i915_node_color_differs(const struct drm_mm_node *node,
 					   unsigned long color)
 {
-	return drm_mm_node_allocated(node) && node->color != color;
+	return drm_mm_node_allocated(node) &&
+		!i915_node_color_matches(node, color);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
index f5cb848b7a7e..e72a07692a64 100644
--- a/drivers/gpu/drm/i915/i915_vma_types.h
+++ b/drivers/gpu/drm/i915/i915_vma_types.h
@@ -95,6 +95,8 @@ enum i915_cache_level;
  *
  */
 
+#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
+
 struct intel_remapped_plane_info {
 	/* in gtt pages */
 	unsigned int width, height, stride, offset;
-- 
2.20.1

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with drm/i915: Remove guard page insertion around unevictable nodes (rev2)

[Patchwork]

== Series Details ==

Series: series starting with drm/i915: Remove guard page insertion around unevictable nodes (rev2)
URL   : https://patchwork.freedesktop.org/series/86220/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
d8da1202f742 drm/i915: Remove guard page insertion around unevictable nodes
-:58: ERROR:CODE_INDENT: code indent should use tabs where possible
#58: FILE: drivers/gpu/drm/i915/i915_vma.h:290:
+^I       ^Inode->color == color;$

-:58: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#58: FILE: drivers/gpu/drm/i915/i915_vma.h:290:
+^I       ^Inode->color == color;$

total: 1 errors, 1 warnings, 0 checks, 50 lines checked
ef937ce200af drm/i915/gt: Always try to reserve GGTT address 0x0


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

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with drm/i915: Remove guard page insertion around unevictable nodes (rev2)

[Patchwork]

== Series Details ==

Series: series starting with drm/i915: Remove guard page insertion around unevictable nodes (rev2)
URL   : https://patchwork.freedesktop.org/series/86220/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_reset.c:1327:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gvt/mmio.c:295:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1450:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1504:15: warning: memset with byte count of 16777216
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block


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

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with drm/i915: Remove guard page insertion around unevictable nodes (rev2)

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 2598 bytes --]

== Series Details ==

Series: series starting with drm/i915: Remove guard page insertion around unevictable nodes (rev2)
URL   : https://patchwork.freedesktop.org/series/86220/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9675 -> Patchwork_19474
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/index.html

Known issues
------------

  Here are the changes found in Patchwork_19474 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@read_all_entries:
    - fi-tgl-y:           [PASS][1] -> [DMESG-WARN][2] ([i915#402]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/fi-tgl-y/igt@debugfs_test@read_all_entries.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/fi-tgl-y/igt@debugfs_test@read_all_entries.html

  * igt@i915_selftest@live@blt:
    - fi-snb-2600:        [PASS][3] -> [DMESG-FAIL][4] ([i915#1409])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/fi-snb-2600/igt@i915_selftest@live@blt.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/fi-snb-2600/igt@i915_selftest@live@blt.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - fi-tgl-y:           [DMESG-WARN][5] ([i915#402]) -> [PASS][6] +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/fi-tgl-y/igt@gem_close_race@basic-threads.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/fi-tgl-y/igt@gem_close_race@basic-threads.html

  
  [i915#1409]: https://gitlab.freedesktop.org/drm/intel/issues/1409
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402


Participating hosts (41 -> 37)
------------------------------

  Missing    (4): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u 


Build changes
-------------

  * Linux: CI_DRM_9675 -> Patchwork_19474

  CI-20190529: 20190529
  CI_DRM_9675: f84d1efce492ef0f46cf4fa1bf01b17ce1627fe4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5968: 267fa33c0947fbbb642098098ff6e5902f9a3d94 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19474: ef937ce200af05b71504476de427b0b4f054f4b9 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ef937ce200af drm/i915/gt: Always try to reserve GGTT address 0x0
d8da1202f742 drm/i915: Remove guard page insertion around unevictable nodes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/index.html

[-- Attachment #1.2: Type: text/html, Size: 3328 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with drm/i915: Remove guard page insertion around unevictable nodes (rev2)

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 14312 bytes --]

== Series Details ==

Series: series starting with drm/i915: Remove guard page insertion around unevictable nodes (rev2)
URL   : https://patchwork.freedesktop.org/series/86220/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9675_full -> Patchwork_19474_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_19474_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-none-vip@rcs0:
    - shard-glk:          [PASS][1] -> [FAIL][2] ([i915#2842])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-glk5/igt@gem_exec_fair@basic-none-vip@rcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-glk9/igt@gem_exec_fair@basic-none-vip@rcs0.html

  * igt@gem_exec_fair@basic-pace@vcs1:
    - shard-iclb:         NOTRUN -> [FAIL][3] ([i915#2842]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-iclb1/igt@gem_exec_fair@basic-pace@vcs1.html

  * igt@gen7_exec_parse@basic-offset:
    - shard-skl:          NOTRUN -> [SKIP][4] ([fdo#109271]) +15 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl3/igt@gen7_exec_parse@basic-offset.html

  * igt@kms_color@pipe-a-ctm-0-75:
    - shard-skl:          [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-skl6/igt@kms_color@pipe-a-ctm-0-75.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl3/igt@kms_color@pipe-a-ctm-0-75.html

  * igt@kms_color_chamelium@pipe-b-gamma:
    - shard-kbl:          NOTRUN -> [SKIP][7] ([fdo#109271] / [fdo#111827]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-kbl1/igt@kms_color_chamelium@pipe-b-gamma.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x85-offscreen:
    - shard-skl:          [PASS][8] -> [FAIL][9] ([i915#54]) +6 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-skl3/igt@kms_cursor_crc@pipe-a-cursor-256x85-offscreen.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl9/igt@kms_cursor_crc@pipe-a-cursor-256x85-offscreen.html

  * igt@kms_cursor_legacy@flip-vs-cursor-legacy:
    - shard-skl:          [PASS][10] -> [FAIL][11] ([i915#2346])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl3/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1:
    - shard-skl:          [PASS][12] -> [FAIL][13] ([i915#79])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html

  * igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2:
    - shard-glk:          [PASS][14] -> [FAIL][15] ([i915#79])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-glk9/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-glk9/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2.html

  * igt@kms_flip@flip-vs-suspend@b-vga1:
    - shard-snb:          [PASS][16] -> [DMESG-WARN][17] ([i915#42])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-snb2/igt@kms_flip@flip-vs-suspend@b-vga1.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-snb6/igt@kms_flip@flip-vs-suspend@b-vga1.html

  * igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-16bpp-ytile:
    - shard-skl:          NOTRUN -> [FAIL][18] ([i915#2628])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl8/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-16bpp-ytile.html

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-gtt:
    - shard-kbl:          NOTRUN -> [SKIP][19] ([fdo#109271]) +40 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-kbl1/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-gtt.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-skl:          NOTRUN -> [FAIL][20] ([fdo#108145] / [i915#265])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#108145] / [i915#265]) +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109441]) +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-iclb1/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@sysfs_heartbeat_interval@mixed@rcs0:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([i915#1731])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-skl1/igt@sysfs_heartbeat_interval@mixed@rcs0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl8/igt@sysfs_heartbeat_interval@mixed@rcs0.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-iclb:         [FAIL][27] ([i915#2842]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-iclb7/igt@gem_exec_fair@basic-none-share@rcs0.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-iclb7/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-none@vcs0:
    - shard-kbl:          [FAIL][29] ([i915#2842]) -> [PASS][30] +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-kbl1/igt@gem_exec_fair@basic-none@vcs0.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-kbl3/igt@gem_exec_fair@basic-none@vcs0.html

  * igt@gem_exec_fair@basic-pace@vcs1:
    - shard-tglb:         [FAIL][31] ([i915#2842]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-tglb7/igt@gem_exec_fair@basic-pace@vcs1.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-tglb7/igt@gem_exec_fair@basic-pace@vcs1.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-glk:          [FAIL][33] ([i915#2842]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-glk6/igt@gem_exec_fair@basic-throttle@rcs0.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-glk3/igt@gem_exec_fair@basic-throttle@rcs0.html
    - shard-iclb:         [FAIL][35] ([i915#2849]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-iclb6/igt@gem_exec_fair@basic-throttle@rcs0.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-iclb7/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_exec_whisper@basic-forked-all:
    - shard-glk:          [DMESG-WARN][37] ([i915#118] / [i915#95]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-glk3/igt@gem_exec_whisper@basic-forked-all.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-glk6/igt@gem_exec_whisper@basic-forked-all.html

  * igt@gem_huc_copy@huc-copy:
    - shard-tglb:         [SKIP][39] ([i915#2190]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-tglb6/igt@gem_huc_copy@huc-copy.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-tglb2/igt@gem_huc_copy@huc-copy.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x128-random:
    - shard-skl:          [FAIL][41] ([i915#54]) -> [PASS][42] +4 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-skl3/igt@kms_cursor_crc@pipe-a-cursor-128x128-random.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl9/igt@kms_cursor_crc@pipe-a-cursor-128x128-random.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [FAIL][43] ([i915#1188]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-skl6/igt@kms_hdr@bpc-switch-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl3/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][45] ([fdo#108145] / [i915#265]) -> [PASS][46] +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][47] ([fdo#109642] / [fdo#111068] / [i915#658]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-iclb1/igt@kms_psr2_su@page_flip.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-iclb2/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [SKIP][49] ([fdo#109441]) -> [PASS][50] +2 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-iclb7/igt@kms_psr@psr2_sprite_plane_move.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_vblank@pipe-c-wait-forked:
    - shard-skl:          [DMESG-WARN][51] ([i915#1982]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-skl3/igt@kms_vblank@pipe-c-wait-forked.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-skl10/igt@kms_vblank@pipe-c-wait-forked.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][53] ([i915#588]) -> [SKIP][54] ([i915#658])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-iclb8/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-iclb:         [WARN][55] ([i915#2684]) -> [WARN][56] ([i915#2681] / [i915#2684])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-iclb5/igt@i915_pm_rc6_residency@rc6-idle.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-iclb8/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@kms_psr2_sf@cursor-plane-update-sf:
    - shard-iclb:         [SKIP][57] ([i915#658]) -> [SKIP][58] ([i915#2920])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-iclb7/igt@kms_psr2_sf@cursor-plane-update-sf.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-iclb2/igt@kms_psr2_sf@cursor-plane-update-sf.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-0:
    - shard-iclb:         [SKIP][59] ([i915#2920]) -> [SKIP][60] ([i915#658]) +2 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9675/shard-iclb2/igt@kms_psr2_sf@plane-move-sf-dmg-area-0.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/shard-iclb8/igt@kms_psr2_sf@plane-move-sf-dmg-area-0.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1731]: https://gitlab.freedesktop.org/drm/intel/issues/1731
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2628]: https://gitlab.freedesktop.org/drm/intel/issues/2628
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2684]: https://gitlab.freedesktop.org/drm/intel/issues/2684
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2849]: https://gitlab.freedesktop.org/drm/intel/issues/2849
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#42]: https://gitlab.freedesktop.org/drm/intel/issues/42
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_9675 -> Patchwork_19474

  CI-20190529: 20190529
  CI_DRM_9675: f84d1efce492ef0f46cf4fa1bf01b17ce1627fe4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5968: 267fa33c0947fbbb642098098ff6e5902f9a3d94 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19474: ef937ce200af05b71504476de427b0b4f054f4b9 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19474/index.html

[-- Attachment #1.2: Type: text/html, Size: 17052 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH] drm/i915: Remove guard page insertion around unevictable nodes

[Matthew Auld]

On Sun, 24 Jan 2021 at 13:57, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Assume that unevictable nodes are not in the GTT and so we can ignore
> page boundary concerns, and so allow regular nodes to abutt against
> irregular unevictable nodes.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  2 --
>  drivers/gpu/drm/i915/i915_gem_evict.c |  6 ++++--
>  drivers/gpu/drm/i915/i915_vma.h       | 10 +++++++++-
>  drivers/gpu/drm/i915/i915_vma_types.h |  2 ++
>  4 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 99cf861df92d..69c5a185ecff 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -357,8 +357,6 @@ enum i915_cache_level {
>         I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */
>  };
>
> -#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
> -
>  struct intel_fbc {
>         /* This is always the inner lock when overlapping with struct_mutex and
>          * it's the outer lock when overlapping with stolen_lock. */
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index 4d2d59a9942b..aef88fdb9f66 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -313,11 +313,13 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
>                  */
>                 if (i915_vm_has_cache_coloring(vm)) {
>                         if (node->start + node->size == target->start) {
> -                               if (node->color == target->color)
> +                               if (i915_node_color_matches(node,
> +                                                           target->color))
>                                         continue;
>                         }
>                         if (node->start == target->start + target->size) {
> -                               if (node->color == target->color)
> +                               if (i915_node_color_matches(node,
> +                                                           target->color))
>                                         continue;
>                         }
>                 }

Since we bail early on seeing COLOR_UNEVICTABLE, and since we have to
enlarge our search space by a page on both ends, do we need something
like:

@@ -291,18 +291,22 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,

                /* Always look at the page afterwards to avoid the end-of-GTT */
                end += I915_GTT_PAGE_SIZE;
+
+               /*
+                * The special head node is marked as I915_COLOR_UNEVICTABLE to
+                * prevent normal vma from using the last page, in order to
+                * prevent prefetching over the boundary. If we are dealing with
+                * a normal vma and we abutt with the head node we will always
+                * get -ENOSPC as per the color adjust later, so just bail
+                * early???
+                */
+               if (target->color != I915_COLOR_UNEVICTABLE && end >= vm->total)
+                       return -ENOSPC;
        }
        GEM_BUG_ON(start >= end);

        drm_mm_for_each_node_in_range(node, &vm->mm, start, end) {
-               /* If we find any non-objects (!vma), we cannot evict them */
-               if (node->color == I915_COLOR_UNEVICTABLE) {
-                       ret = -ENOSPC;
-                       break;
-               }
-
                GEM_BUG_ON(!drm_mm_node_allocated(node));
-               vma = container_of(node, typeof(*vma), node);

                /*
                 * If we are using coloring to insert guard pages between
@@ -310,6 +314,9 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
                 * to check whether the objects on either side of our range
                 * abutt and conflict. If they are in conflict, then we evict
                 * those as well to make room for our guard pages.
+                *
+                * Also consider I915_COLOR_UNEVICTABLE nodes, since we consider
+                * these special and permit abutting on either end.
                 */
                if (i915_vm_has_cache_coloring(vm)) {
                        if (node->start + node->size == target->start) {
@@ -324,6 +331,14 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
                        }
                }

+               /* If we find any non-objects (!vma), we cannot evict them */
+               if (node->color == I915_COLOR_UNEVICTABLE) {
+                       ret = -ENOSPC;
+                       break;
+               }
+
+               vma = container_of(node, typeof(*vma), node);
+
                if (i915_vma_is_pinned(vma)) {
                        ret = -ENOSPC;
                        break;


> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index a64adc8c883b..dac953815118 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -283,10 +283,18 @@ static inline bool i915_vma_is_bound(const struct i915_vma *vma,
>         return atomic_read(&vma->flags) & where;
>  }
>
> +static inline bool i915_node_color_matches(const struct drm_mm_node *node,
> +                                          unsigned long color)
> +{
> +       return (node->color | color) == I915_COLOR_UNEVICTABLE ||
> +               node->color == color;
> +}
> +
>  static inline bool i915_node_color_differs(const struct drm_mm_node *node,
>                                            unsigned long color)
>  {
> -       return drm_mm_node_allocated(node) && node->color != color;
> +       return drm_mm_node_allocated(node) &&
> +               !i915_node_color_matches(node, color);
>  }
>
>  /**
> diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
> index f5cb848b7a7e..e72a07692a64 100644
> --- a/drivers/gpu/drm/i915/i915_vma_types.h
> +++ b/drivers/gpu/drm/i915/i915_vma_types.h
> @@ -95,6 +95,8 @@ enum i915_cache_level;
>   *
>   */
>
> +#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
> +
>  struct intel_remapped_plane_info {
>         /* in gtt pages */
>         unsigned int width, height, stride, offset;
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Remove guard page insertion around unevictable nodes

[Chris Wilson]

Quoting Matthew Auld (2021-01-25 11:16:13)
> On Sun, 24 Jan 2021 at 13:57, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Assume that unevictable nodes are not in the GTT and so we can ignore
> > page boundary concerns, and so allow regular nodes to abutt against
> > irregular unevictable nodes.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h       |  2 --
> >  drivers/gpu/drm/i915/i915_gem_evict.c |  6 ++++--
> >  drivers/gpu/drm/i915/i915_vma.h       | 10 +++++++++-
> >  drivers/gpu/drm/i915/i915_vma_types.h |  2 ++
> >  4 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 99cf861df92d..69c5a185ecff 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -357,8 +357,6 @@ enum i915_cache_level {
> >         I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */
> >  };
> >
> > -#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
> > -
> >  struct intel_fbc {
> >         /* This is always the inner lock when overlapping with struct_mutex and
> >          * it's the outer lock when overlapping with stolen_lock. */
> > diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> > index 4d2d59a9942b..aef88fdb9f66 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> > @@ -313,11 +313,13 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
> >                  */
> >                 if (i915_vm_has_cache_coloring(vm)) {
> >                         if (node->start + node->size == target->start) {
> > -                               if (node->color == target->color)
> > +                               if (i915_node_color_matches(node,
> > +                                                           target->color))
> >                                         continue;
> >                         }
> >                         if (node->start == target->start + target->size) {
> > -                               if (node->color == target->color)
> > +                               if (i915_node_color_matches(node,
> > +                                                           target->color))
> >                                         continue;
> >                         }
> >                 }
> 
> Since we bail early on seeing COLOR_UNEVICTABLE, and since we have to
> enlarge our search space by a page on both ends, do we need something
> like:

Are we not doing the opposite here, and skipping the evict if either
node/target is unevictable?

So we always expand the search by a page if the vm has coloring enabled,
but then skip the guard page eviction if either side says no.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Remove guard page insertion around unevictable nodes

[Chris Wilson]

Quoting Chris Wilson (2021-01-25 11:24:22)
> Quoting Matthew Auld (2021-01-25 11:16:13)
> > On Sun, 24 Jan 2021 at 13:57, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > >
> > > Assume that unevictable nodes are not in the GTT and so we can ignore
> > > page boundary concerns, and so allow regular nodes to abutt against
> > > irregular unevictable nodes.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h       |  2 --
> > >  drivers/gpu/drm/i915/i915_gem_evict.c |  6 ++++--
> > >  drivers/gpu/drm/i915/i915_vma.h       | 10 +++++++++-
> > >  drivers/gpu/drm/i915/i915_vma_types.h |  2 ++
> > >  4 files changed, 15 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 99cf861df92d..69c5a185ecff 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -357,8 +357,6 @@ enum i915_cache_level {
> > >         I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */
> > >  };
> > >
> > > -#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
> > > -
> > >  struct intel_fbc {
> > >         /* This is always the inner lock when overlapping with struct_mutex and
> > >          * it's the outer lock when overlapping with stolen_lock. */
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> > > index 4d2d59a9942b..aef88fdb9f66 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> > > @@ -313,11 +313,13 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
> > >                  */
> > >                 if (i915_vm_has_cache_coloring(vm)) {
> > >                         if (node->start + node->size == target->start) {
> > > -                               if (node->color == target->color)
> > > +                               if (i915_node_color_matches(node,
> > > +                                                           target->color))
> > >                                         continue;
> > >                         }
> > >                         if (node->start == target->start + target->size) {
> > > -                               if (node->color == target->color)
> > > +                               if (i915_node_color_matches(node,
> > > +                                                           target->color))
> > >                                         continue;
> > >                         }
> > >                 }
> > 
> > Since we bail early on seeing COLOR_UNEVICTABLE, and since we have to
> > enlarge our search space by a page on both ends, do we need something
> > like:
> 
> Are we not doing the opposite here, and skipping the evict if either
> node/target is unevictable?

Oh, you mean the earlier abort if we try to evict an unevictable node
inside the target range.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/gt: Always try to reserve GGTT address 0x0

[Matthew Auld]

On Sun, 24 Jan 2021 at 13:54, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Since writing to address 0 is a very common mistake, let's try to avoid
> putting anything sensitive there.
>
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/2989
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_ggtt.c | 40 +++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index dac07d66f658..3a737d4fbc3c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -535,16 +535,32 @@ static int init_ggtt(struct i915_ggtt *ggtt)
>
>         mutex_init(&ggtt->error_mutex);
>         if (ggtt->mappable_end) {
> -               /* Reserve a mappable slot for our lockless error capture */
> -               ret = drm_mm_insert_node_in_range(&ggtt->vm.mm,
> -                                                 &ggtt->error_capture,
> -                                                 PAGE_SIZE, 0,
> -                                                 I915_COLOR_UNEVICTABLE,
> -                                                 0, ggtt->mappable_end,
> -                                                 DRM_MM_INSERT_LOW);
> -               if (ret)
> -                       return ret;
> +               /*
> +                * Reserve a mappable slot for our lockless error capture.
> +                *
> +                * We strongly prefer taking address 0x0 in order to protect
> +                * other critical buffers against accidental overwrites,
> +                * as writing to address 0 is a very common mistake.
> +                *
> +                * Since 0 may already be in use by the system (e.g. the BIOS
> +                * framebuffer), we let the reservation fail quietly and hope
> +                * 0 remains reserved always.
> +                */
> +               ggtt->error_capture.size = I915_GTT_PAGE_SIZE;
> +               ggtt->error_capture.color = I915_COLOR_UNEVICTABLE;
> +               if (drm_mm_reserve_node(&ggtt->vm.mm, &ggtt->error_capture))
> +                       drm_mm_insert_node_in_range(&ggtt->vm.mm,
> +                                                   &ggtt->error_capture,
> +                                                   ggtt->error_capture.size, 0,
> +                                                   ggtt->error_capture.color,
> +                                                   0, ggtt->mappable_end,
> +                                                   DRM_MM_INSERT_LOW);

We don't want to check the in_range for an error? It should be
impossible I guess?

Reviewed-by: Matthew Auld <matthew.auld@intel.com>

>         }
> +       if (drm_mm_node_allocated(&ggtt->error_capture))
> +               drm_dbg(&ggtt->vm.i915->drm,
> +                       "Reserved GGTT:[%llx, %llx] for use by error capture\n",
> +                       ggtt->error_capture.start,
> +                       ggtt->error_capture.start + ggtt->error_capture.size);
>
>         /*
>          * The upper portion of the GuC address space has a sizeable hole
> @@ -557,9 +573,9 @@ static int init_ggtt(struct i915_ggtt *ggtt)
>
>         /* Clear any non-preallocated blocks */
>         drm_mm_for_each_hole(entry, &ggtt->vm.mm, hole_start, hole_end) {
> -               drm_dbg_kms(&ggtt->vm.i915->drm,
> -                           "clearing unused GTT space: [%lx, %lx]\n",
> -                           hole_start, hole_end);
> +               drm_dbg(&ggtt->vm.i915->drm,
> +                       "clearing unused GTT space: [%lx, %lx]\n",
> +                       hole_start, hole_end);
>                 ggtt->vm.clear_range(&ggtt->vm, hole_start,
>                                      hole_end - hole_start);
>         }
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/gt: Always try to reserve GGTT address 0x0

[Chris Wilson]

Quoting Matthew Auld (2021-01-25 11:28:16)
> On Sun, 24 Jan 2021 at 13:54, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Since writing to address 0 is a very common mistake, let's try to avoid
> > putting anything sensitive there.
> >
> > References: https://gitlab.freedesktop.org/drm/intel/-/issues/2989
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_ggtt.c | 40 +++++++++++++++++++---------
> >  1 file changed, 28 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > index dac07d66f658..3a737d4fbc3c 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> > @@ -535,16 +535,32 @@ static int init_ggtt(struct i915_ggtt *ggtt)
> >
> >         mutex_init(&ggtt->error_mutex);
> >         if (ggtt->mappable_end) {
> > -               /* Reserve a mappable slot for our lockless error capture */
> > -               ret = drm_mm_insert_node_in_range(&ggtt->vm.mm,
> > -                                                 &ggtt->error_capture,
> > -                                                 PAGE_SIZE, 0,
> > -                                                 I915_COLOR_UNEVICTABLE,
> > -                                                 0, ggtt->mappable_end,
> > -                                                 DRM_MM_INSERT_LOW);
> > -               if (ret)
> > -                       return ret;
> > +               /*
> > +                * Reserve a mappable slot for our lockless error capture.
> > +                *
> > +                * We strongly prefer taking address 0x0 in order to protect
> > +                * other critical buffers against accidental overwrites,
> > +                * as writing to address 0 is a very common mistake.
> > +                *
> > +                * Since 0 may already be in use by the system (e.g. the BIOS
> > +                * framebuffer), we let the reservation fail quietly and hope
> > +                * 0 remains reserved always.
> > +                */
> > +               ggtt->error_capture.size = I915_GTT_PAGE_SIZE;
> > +               ggtt->error_capture.color = I915_COLOR_UNEVICTABLE;
> > +               if (drm_mm_reserve_node(&ggtt->vm.mm, &ggtt->error_capture))
> > +                       drm_mm_insert_node_in_range(&ggtt->vm.mm,
> > +                                                   &ggtt->error_capture,
> > +                                                   ggtt->error_capture.size, 0,
> > +                                                   ggtt->error_capture.color,
> > +                                                   0, ggtt->mappable_end,
> > +                                                   DRM_MM_INSERT_LOW);
> 
> We don't want to check the in_range for an error? It should be
> impossible I guess?

For the purpose of error-capture, we will survive without. (I guess we
should include a note in the error capture as to which path was used to
retrieve the data.) Far better to risk error capture not being fully
functional than aborting the driver load. (Although what could go wrong
here... A driver bug?)

For the secondary purpose of preventing use of 0x0, if the reservation
fails, then we know something is at 0x0 and we have to trust that it
remains in place preventing our user.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Remove guard page insertion around unevictable nodes

[Matthew Auld]

On Mon, 25 Jan 2021 at 11:28, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Chris Wilson (2021-01-25 11:24:22)
> > Quoting Matthew Auld (2021-01-25 11:16:13)
> > > On Sun, 24 Jan 2021 at 13:57, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > >
> > > > Assume that unevictable nodes are not in the GTT and so we can ignore
> > > > page boundary concerns, and so allow regular nodes to abutt against
> > > > irregular unevictable nodes.
> > > >
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.h       |  2 --
> > > >  drivers/gpu/drm/i915/i915_gem_evict.c |  6 ++++--
> > > >  drivers/gpu/drm/i915/i915_vma.h       | 10 +++++++++-
> > > >  drivers/gpu/drm/i915/i915_vma_types.h |  2 ++
> > > >  4 files changed, 15 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index 99cf861df92d..69c5a185ecff 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -357,8 +357,6 @@ enum i915_cache_level {
> > > >         I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */
> > > >  };
> > > >
> > > > -#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
> > > > -
> > > >  struct intel_fbc {
> > > >         /* This is always the inner lock when overlapping with struct_mutex and
> > > >          * it's the outer lock when overlapping with stolen_lock. */
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> > > > index 4d2d59a9942b..aef88fdb9f66 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> > > > @@ -313,11 +313,13 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
> > > >                  */
> > > >                 if (i915_vm_has_cache_coloring(vm)) {
> > > >                         if (node->start + node->size == target->start) {
> > > > -                               if (node->color == target->color)
> > > > +                               if (i915_node_color_matches(node,
> > > > +                                                           target->color))
> > > >                                         continue;
> > > >                         }
> > > >                         if (node->start == target->start + target->size) {
> > > > -                               if (node->color == target->color)
> > > > +                               if (i915_node_color_matches(node,
> > > > +                                                           target->color))
> > > >                                         continue;
> > > >                         }
> > > >                 }
> > >
> > > Since we bail early on seeing COLOR_UNEVICTABLE, and since we have to
> > > enlarge our search space by a page on both ends, do we need something
> > > like:
> >
> > Are we not doing the opposite here, and skipping the evict if either
> > node/target is unevictable?
>
> Oh, you mean the earlier abort if we try to evict an unevictable node
> inside the target range.

Yeah, if it only abuts and is COLOR_UNEVICTABLE we can ignore the node
now, but if it's actually within our range then we abort like before.
And then there is some strangeness with the head node.

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

Re: [Intel-gfx] [PATCH] drm/i915: Remove guard page insertion around unevictable nodes

[Chris Wilson]

Quoting Matthew Auld (2021-01-25 11:35:22)
> On Mon, 25 Jan 2021 at 11:28, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Quoting Chris Wilson (2021-01-25 11:24:22)
> > > Quoting Matthew Auld (2021-01-25 11:16:13)
> > > > On Sun, 24 Jan 2021 at 13:57, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > > >
> > > > > Assume that unevictable nodes are not in the GTT and so we can ignore
> > > > > page boundary concerns, and so allow regular nodes to abutt against
> > > > > irregular unevictable nodes.
> > > > >
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_drv.h       |  2 --
> > > > >  drivers/gpu/drm/i915/i915_gem_evict.c |  6 ++++--
> > > > >  drivers/gpu/drm/i915/i915_vma.h       | 10 +++++++++-
> > > > >  drivers/gpu/drm/i915/i915_vma_types.h |  2 ++
> > > > >  4 files changed, 15 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > > index 99cf861df92d..69c5a185ecff 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > @@ -357,8 +357,6 @@ enum i915_cache_level {
> > > > >         I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */
> > > > >  };
> > > > >
> > > > > -#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
> > > > > -
> > > > >  struct intel_fbc {
> > > > >         /* This is always the inner lock when overlapping with struct_mutex and
> > > > >          * it's the outer lock when overlapping with stolen_lock. */
> > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> > > > > index 4d2d59a9942b..aef88fdb9f66 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> > > > > @@ -313,11 +313,13 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
> > > > >                  */
> > > > >                 if (i915_vm_has_cache_coloring(vm)) {
> > > > >                         if (node->start + node->size == target->start) {
> > > > > -                               if (node->color == target->color)
> > > > > +                               if (i915_node_color_matches(node,
> > > > > +                                                           target->color))
> > > > >                                         continue;
> > > > >                         }
> > > > >                         if (node->start == target->start + target->size) {
> > > > > -                               if (node->color == target->color)
> > > > > +                               if (i915_node_color_matches(node,
> > > > > +                                                           target->color))
> > > > >                                         continue;
> > > > >                         }
> > > > >                 }
> > > >
> > > > Since we bail early on seeing COLOR_UNEVICTABLE, and since we have to
> > > > enlarge our search space by a page on both ends, do we need something
> > > > like:
> > >
> > > Are we not doing the opposite here, and skipping the evict if either
> > > node/target is unevictable?
> >
> > Oh, you mean the earlier abort if we try to evict an unevictable node
> > inside the target range.
> 
> Yeah, if it only abuts and is COLOR_UNEVICTABLE we can ignore the node
> now, but if it's actually within our range then we abort like before.
> And then there is some strangeness with the head node.

Hmm. On second thought, the reservation is using the direct reserve now
and not entering i915_gem_evict_for_now() so for the moment we don't
have to worry about any changes here. We can ponder whether we can
remove guard pages around foreign nodes later.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx