• Imre Deak's avatar
    drm/i915: avoid leaking DMA mappings · e2273302
    Imre Deak authored
    We have 3 types of DMA mappings for GEM objects:
    1. physically contiguous for stolen and for objects needing contiguous
       memory
    2. DMA-buf mappings imported via a DMA-buf attach operation
    3. SG DMA mappings for shmem backed and userptr objects
    
    For 1. and 2. the lifetime of the DMA mapping matches the lifetime of the
    corresponding backing pages and so in practice we create/release the
    mapping in the object's get_pages/put_pages callback.
    
    For 3. the lifetime of the mapping matches that of any existing GPU binding
    of the object, so we'll create the mapping when the object is bound to
    the first vma and release the mapping when the object is unbound from its
    last vma.
    
    Since the object can be bound to multiple vmas, we can end up creating a
    new DMA mapping in the 3. case even if the object already had one. This
    is not allowed by the DMA API and can lead to leaked mapping data and
    IOMMU memory space starvation in certain cases. For example HW IOMMU
    drivers (intel_iommu) allocate a new range from their memory space
    whenever a mapping is created, silently overriding a pre-existing
    mapping.
    
    Fix this by moving the creation/removal of DMA mappings to the object's
    get_pages/put_pages callbacks. These callbacks already check for and do
    an early return in case of any nested calls. This way objects of the 3.
    case also become more like the other object types.
    
    I noticed this issue by enabling DMA debugging, which got disabled after
    a while due to its internal mapping tables getting full. It also reported
    errors in connection to random other drivers that did a DMA mapping for
    an address that was previously mapped by i915 but was never released.
    Besides these diagnostic messages and the memory space starvation
    problem for IOMMUs, I'm not aware of this causing a real issue.
    
    The fix is based on a patch from Chris.
    
    v2:
    - move the DMA mapping create/remove calls to the get_pages/put_pages
      callbacks instead of adding new callbacks for these (Chris)
    v3:
    - also fix the get_page cache logic on the userptr async path (Chris)
    Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
    Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
    Cc: stable@vger.kernel.org
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
    e2273302
i915_gem_userptr.c 22.6 KB