linux-kernel - Re: drm/i915: avoid "may be used uninitialized" warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 18 Jan 2017 10:31:09 +0200
From:   Jani Nikula <jani.nikula@...ux.intel.com>
To:     Sreedhar Donelli <sreedhar.donelli@....com>,
        daniel.vetter@...el.com, linux-kernel@...r.kernel.org
Cc:     airlied@...ux.ie, intel-gfx@...ts.freedesktop.org,
        dri-devel@...ts.freedesktop.org
Subject: Re: drm/i915: avoid "may be used uninitialized" warnings

On Wed, 18 Jan 2017, Sreedhar Donelli <sreedhar.donelli@....com> wrote:
> compilation issue on kernel-4.10-rc4 is resolved and i created a patch file.
> please find the attachment for patch file.

There are several issues with this contribution. Please learn to use git
send-email to send your patches. Don't attach them. You need to write a
proper commit message. That commit message needs to include a
Signed-off-by tag. You absolutely can't have a disclaimer about
confidential/privileged with the patch. See
Documentation/process/submitting-patches.rst.

On the content, there are no issues in the code, the compiler just seems
unable to figure it out. If we were to "fix" this, for sure we don't
need the superfluous changes included in this patch.

BR,
Jani.

>
> I have taken this issue from below link
>
> https://lkml.org/lkml/2017/1/16/89
>
>
>
> Thanks & Regards
>  Sreedhar Donelli
>  Tata Consultancy Services
>  Ph:-   +91 8019039399
>  Cell:- +91 8019039399
>  Mailto: sreedhar.donelli@....com
>  Website: http://www.tcs.com
>  ____________________________________________
>  Experience certainty.	IT Services
>  			Business Solutions
>  			Consulting
>  ____________________________________________
>  
> =====-----=====-----=====
> Notice: The information contained in this e-mail
> message and/or attachments to it may contain 
> confidential or privileged information. If you are 
> not the intended recipient, any dissemination, use, 
> review, distribution, printing or copying of the 
> information contained in this e-mail message 
> and/or attachments to it are strictly prohibited. If 
> you have received this communication in error, 
> please notify us by reply e-mail or telephone and 
> immediately and permanently delete the message 
> and any attachments. Thank you
>
>
> diff -uNr a/linux/drivers/gpu/drm/i915/i915_gem_gtt.c b/linux/drivers/gpu/drm/i915/i915_gem_gtt.c
> --- a/linux/drivers/gpu/drm/i915/i915_gem_gtt.c	2017-01-17 18:18:56.564887392 +0530
> +++ b/linux/drivers/gpu/drm/i915/i915_gem_gtt.c	2017-01-17 19:18:50.524978315 +0530
> @@ -2364,7 +2364,7 @@
>  	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;
> @@ -2385,8 +2385,10 @@
>  	 * 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)
> +	if (i != 0){
> +		gen8_pte_t last_gtt_entry = readq(&gtt_entries[i-1]);
>  		WARN_ON(readq(&gtt_entries[i-1]) != 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
> @@ -2439,7 +2441,7 @@
>  	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;
> @@ -2453,14 +2455,17 @@
>  		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)
> +	if (i != 0) {
> +		gen8_pte_t last_gtt_entry = readl(&gtt_entries[i-1]);
>  		WARN_ON(readl(&gtt_entries[i-1]) != 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 -uNr a/linux/drivers/gpu/drm/i915/i915_gem_gtt.h b/linux/drivers/gpu/drm/i915/i915_gem_gtt.h
> --- a/linux/drivers/gpu/drm/i915/i915_gem_gtt.h	2017-01-17 18:18:56.564887392 +0530
> +++ b/linux/drivers/gpu/drm/i915/i915_gem_gtt.h	2017-01-17 19:19:57.196980002 +0530
> @@ -54,6 +54,7 @@
>  #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

-- 
Jani Nikula, Intel Open Source Technology Center