On Wed, Mar 05, 2014 at 06:32:06PM +0200, Imre Deak wrote:
> 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
The only case I was able to fathom was if we accidentally connect a PDE
before we populate the page table. I felt it was a rather harmless patch
though.
I do agree with the IRC conversation that it shouldn't happen. It was in
lines with the same reason of why we never BUG_ON.
>
> > 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@xxxxxxxxxxxx>
> > ---
> > 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;
>
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
