drm/i915: avoid "may be used uninitialised" warnings

Commit Message

Dave Gordon July 25, 2016, 4:29 p.m. UTC

gcc is getting false positives in its detection of uninitialised values.
Specifically it thinks 'gtt_entry' can be used in a WARN_ON() macro
without previously being assigned (the assigment is inside a conditional
loop), bu actually the WARN_ON() can only be reached if the assignment
has also been executed at least once.

To avoid the annoying warning, though, this patch reorganises the code a
little and adds an explicit initialisation of the suspect variable.

Signed-off-by: Dave Gordon <[e-mail]>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 19 ++++++++++++-------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
 2 files changed, 13 insertions(+), 7 deletions(-)

Comments

Chris Wilson July 25, 2016, 4:40 p.m. UTC | #1

On Mon, Jul 25, 2016 at 05:29:45PM +0100, Dave Gordon wrote:
> gcc is getting false positives in its detection of uninitialised values.
> Specifically it thinks 'gtt_entry' can be used in a WARN_ON() macro
> without previously being assigned (the assigment is inside a conditional
> loop), bu actually the WARN_ON() can only be reached if the assignment
> has also been executed at least once.

gcc 4.7 through 6.1 doesn't generate the warning for me, and the
kbuilder can happily ignore it.

At any rate the markup is uninitialised_var().
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 30da543..90e1cf3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2368,7 +2368,7 @@  static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
 	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
 	struct sgt_iter sgt_iter;
 	gen8_pte_t __iomem *gtt_entries;
-	gen8_pte_t gtt_entry;
+	gen8_pte_t gtt_entry = I915_NULL_PTE;
 	dma_addr_t addr;
 	int rpm_atomic_seq;
 	int i = 0;
@@ -2389,8 +2389,10 @@  static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
 	 * of NUMA access patterns. Therefore, even with the way we assume
 	 * hardware should work, we must keep this posting read for paranoia.
 	 */
-	if (i != 0)
-		WARN_ON(readq(&gtt_entries[i-1]) != gtt_entry);
+	if (i != 0) {
+		gen8_pte_t last_gtt_entry = readq(&gtt_entries[i-1]);
+		WARN_ON(last_gtt_entry != gtt_entry);
+	}
 
 	/* This next bit makes the above posting read even more important. We
 	 * want to flush the TLBs only after we're certain all the PTE updates
@@ -2465,7 +2467,7 @@  static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
 	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
 	struct sgt_iter sgt_iter;
 	gen6_pte_t __iomem *gtt_entries;
-	gen6_pte_t gtt_entry;
+	gen6_pte_t gtt_entry = I915_NULL_PTE;
 	dma_addr_t addr;
 	int rpm_atomic_seq;
 	int i = 0;
@@ -2479,14 +2481,17 @@  static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
 		iowrite32(gtt_entry, &gtt_entries[i++]);
 	}
 
-	/* XXX: This serves as a posting read to make sure that the PTE has
+	/*
+	 * XXX: This serves as a posting read to make sure that the PTE has
 	 * actually been updated. There is some concern that even though
 	 * registers and PTEs are within the same BAR that they are potentially
 	 * of NUMA access patterns. Therefore, even with the way we assume
 	 * hardware should work, we must keep this posting read for paranoia.
 	 */
-	if (i != 0)
-		WARN_ON(readl(&gtt_entries[i-1]) != gtt_entry);
+	if (i != 0) {
+		gen8_pte_t last_gtt_entry = readl(&gtt_entries[i-1]);
+		WARN_ON(last_gtt_entry != gtt_entry);
+	}
 
 	/* This next bit makes the above posting read even more important. We
 	 * want to flush the TLBs only after we're certain all the PTE updates
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index c4a6579..e088210 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -54,6 +54,7 @@  typedef uint64_t gen8_ppgtt_pml4e_t;
 #define GEN6_PTE_UNCACHED		(1 << 1)
 #define GEN6_PTE_VALID			(1 << 0)
 
+#define	I915_NULL_PTE			0
 #define I915_PTES(pte_len)		(PAGE_SIZE / (pte_len))
 #define I915_PTE_MASK(pte_len)		(I915_PTES(pte_len) - 1)
 #define I915_PDES			512