[Intel-gfx] [PATCH] drm/i915: Add lmem fault handler

[Chris Wilson]

Quoting Matthew Auld (2019-12-12 15:11:02)
> On Thu, 12 Dec 2019 at 14:20, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > Quoting Abdiel Janulgue (2019-12-12 11:34:38)
> > > Fault handler to handle missing pages for lmem objects.
> > >
> > > v3: Add get_vm_cpu_ops, iterate over all memory regions in the
> > >     lmem selftest, use remap_io_mapping.
> > >
> > > Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> > > Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gem/i915_gem_lmem.c      |  40 +++++
> > >  drivers/gpu/drm/i915/gem/i915_gem_lmem.h      |   6 +
> > >  drivers/gpu/drm/i915/gem/i915_gem_mman.c      |  32 +++-
> > >  drivers/gpu/drm/i915/gem/i915_gem_mman.h      |   1 +
> > >  .../drm/i915/gem/selftests/i915_gem_mman.c    | 137 +++++++++++++++---
> > >  5 files changed, 188 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > > index 0e2bf6b7e143..bbe625935005 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> > > @@ -6,8 +6,36 @@
> > >  #include "intel_memory_region.h"
> > >  #include "gem/i915_gem_region.h"
> > >  #include "gem/i915_gem_lmem.h"
> > > +#include "gem/i915_gem_mman.h"
> > >  #include "i915_drv.h"
> > >
> > > +vm_fault_t vm_fault_iomem(struct vm_fault *vmf)
> > > +{
> > > +       struct vm_area_struct *area = vmf->vma;
> > > +       struct i915_mmap_offset *priv = area->vm_private_data;
> > > +       struct drm_i915_gem_object *obj = priv->obj;
> > > +       struct intel_memory_region *mem = obj->mm.region;
> > > +       unsigned long size = area->vm_end - area->vm_start;
> > > +       bool write = area->vm_flags & VM_WRITE;
> > > +       int ret;
> > > +
> > > +       /* Sanity check that we allow writing into this object */
> > > +       if (i915_gem_object_is_readonly(obj) && write)
> > > +               return VM_FAULT_SIGBUS;
> > > +
> > > +       ret = i915_gem_object_pin_pages(obj);
> > > +       if (ret)
> > > +               return i915_error_to_vmf_fault(ret);
> > > +
> > > +       ret = remap_io_mapping(area, area->vm_start,
> > > +                              i915_gem_object_lmem_io_pfn(obj, 0), size,
> > > +                              &mem->iomap);
> >
> > So this implementation only works with contiguous objects, right?
> 
> Hmm can't we go back to what we had before, so support !contiguous also?

The fun part is that you can do both :) Do a discontiguous
remap_io_mapping() The queue part is that remap_io_mapping() avoids the
O(N^2), and should give us O(N) instead.

> Also do we need to reject !(WC | UC) somewhere for local-memory?

We don't strictly need to (caveat emptor), same rules apply for
incoherent device memory, you have clflush around your access. It may
not be advised, but sometimes it is better.
-Chris