Re: [PATCH] [v2] drm/i915: Paranoia - get zeroed page table pages

[Imre Deak <imre.deak@intel.com>]

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

On Thu, 2014-02-27 at 19:47 -0800, Ben Widawsky wrote:
> We normally clear the page tables as one of the first things during
> initialization. They are however wired up (and potentially valid) before
> we clear them.

I might be missing something, but afaics the page directories/tables are
not in use until after ppgtt->enable()/mm_switch() is called on them,
which is after the clear_range() call.

I'd understand if it's about leaving uninitialized stuff _after_
clear_range() is called. But I think because of the 1G size alignment
for ppgtt that's not possible either.

--Imre

> To prevent the GPU from doing anything we might later regret, simply get
> zeroed pages, which always mean invalid on all GENs.
> 
> NOTE: that a similar paranoia could be applied to GGTT via making sure
> all entries are invalid ASAP. I think the extra work required to fix
> such a BIOS bug is unwarranted until proven necessary.
> 
> v2: Remove useless GFP_ZERO in the kcallocs
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 0c27d8a..5e3957e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -359,7 +359,7 @@ static struct page **__gen8_alloc_page_tables(void)
>  		return ERR_PTR(-ENOMEM);
>  
>  	for (i = 0; i < GEN8_PDES_PER_PAGE; i++) {
> -		pt_pages[i] = alloc_page(GFP_KERNEL);
> +		pt_pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
>  		if (!pt_pages[i])
>  			goto bail;
>  	}
> @@ -421,7 +421,8 @@ static int gen8_ppgtt_allocate_dma(struct i915_hw_ppgtt *ppgtt)
>  static int gen8_ppgtt_allocate_page_directories(struct i915_hw_ppgtt *ppgtt,
>  						const int max_pdp)
>  {
> -	ppgtt->pd_pages = alloc_pages(GFP_KERNEL, get_order(max_pdp << PAGE_SHIFT));
> +	ppgtt->pd_pages = alloc_pages(GFP_KERNEL | __GFP_ZERO,
> +				      get_order(max_pdp << PAGE_SHIFT));
>  	if (!ppgtt->pd_pages)
>  		return -ENOMEM;
>  
> @@ -1021,7 +1022,7 @@ static int gen6_ppgtt_allocate_page_tables(struct i915_hw_ppgtt *ppgtt)
>  		return -ENOMEM;
>  
>  	for (i = 0; i < ppgtt->num_pd_entries; i++) {
> -		ppgtt->pt_pages[i] = alloc_page(GFP_KERNEL);
> +		ppgtt->pt_pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO);
>  		if (!ppgtt->pt_pages[i]) {
>  			gen6_ppgtt_free(ppgtt);
>  			return -ENOMEM;


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

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

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