Skip to content

Commit

Permalink
drm: Refactor handling of GEM object mappings
Browse files Browse the repository at this point in the history
Each GEM object is backed by a VM object which stores a handle for each page
belonging to the object.  A GEM object can be mapped multiple times.  Panfrost's
buffer objects are backed by physical RAM from the host system.

The old code tried to maintain a global list of VMAs which map a GEM object, and
this used the object pointer as a unique key, which breaks when the same object
is mapped more than once.  The mapping list is also somewhat awkward to
maintain: FreeBSD's vm_fault layer is supposed to hide details relating to
mappings of a given object (for instance, it doesn't pass the fault address to
DRM), but Linux passes such details, so we end up faking them.

Refactor things to handle the possibility of multiply-mapped GEM objects:
- Minimize uses of VMAs.  In the pager populate handler, we don't know which
  mapping the fault came from, so it doesn't make sense to try and look up a
  VMA.  In practice, there is no need to know the origin of the fault, we just
  need a pindex to select the page to map.
- Introduce a struct drm_object_glue, which glues together a GEM object, a VM
  object, and a list of VMAs which map the object.  The list of VMAs is mostly
  useless but I've avoided removing it for now.  Most uses of the VMA are
  replaced by extending the thread-private vm_fault structure.
- Convert the global linked list lock to an sx lock so that it's possible to
  hold it while allocating memory.
- Change the DRM object fault interface to avoid referencing VMAs.  This
  deviates from upstream, but the modified code is all quite FreeBSD-specific
  anyway.

While here, fix a related problem: when mmap()ing a device file,
drm_fstub_do_mmap() tries to infer the VM object type to use.  In particular, it
creates a "managed device" object for panfrost buffer objects, which isn't
correct since the object is backed by host RAM.  The problem is further
illustrated by the need to clear VPO_UNMANAGED and set PG_FICTITIOUS: setting
PG_FICTITIOUS in host RAM pages is incorrect and can trigger assertion failures
in the contig reclaim code.

My view is that it's not possible to correctly map DRM objects without modifying
upstream sources a little bit.  That is, we cannot hide everything in
drmkpi/linuxkpi.  Keeping this in mind, the change fixes the problem described
above by adding a new VM object type to struct vm_operations_struct, used in
drm_fstub_do_mmap() to allocate the correct type of VM object.
  • Loading branch information
markjdb committed Sep 4, 2024
1 parent 9b2bbcd commit a50a81b
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 149 deletions.
3 changes: 3 additions & 0 deletions sys/dev/drm/core/drm_vm.c
Original file line number Diff line number Diff line change
Expand Up @@ -370,20 +370,23 @@ static const struct vm_operations_struct drm_vm_ops = {
.fault = drm_vm_fault,
.open = drm_vm_open,
.close = drm_vm_close,
.objtype = OBJT_MGTDEVICE,
};

/** Shared virtual memory operations */
static const struct vm_operations_struct drm_vm_shm_ops = {
.fault = drm_vm_shm_fault,
.open = drm_vm_open,
.close = drm_vm_shm_close,
.objtype = OBJT_MGTDEVICE,
};

/** DMA virtual memory operations */
static const struct vm_operations_struct drm_vm_dma_ops = {
.fault = drm_vm_dma_fault,
.open = drm_vm_open,
.close = drm_vm_close,
.objtype = OBJT_MGTDEVICE,
};

/** Scatter-gather virtual memory operations */
Expand Down
12 changes: 5 additions & 7 deletions sys/dev/drm/drmkpi/include/linux/mm.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,20 +119,18 @@ struct vm_area_struct {
struct vm_fault {
unsigned int flags;
pgoff_t pgoff;
union {
/* user-space address */
void *virtual_address; /* < 4.11 */
unsigned long address; /* >= 4.11 */
};
vm_object_t object;
vm_pindex_t pindex; /* fault pindex */
int count; /* pages faulted in */
struct page *page;
struct vm_area_struct *vma;
};

struct vm_operations_struct {
void (*open) (struct vm_area_struct *);
void (*close) (struct vm_area_struct *);
int (*fault) (struct vm_area_struct *, struct vm_fault *);
int (*fault) (struct vm_fault *);
int (*access) (struct vm_area_struct *, unsigned long, void *, int, int);
int objtype;
};

struct sysinfo {
Expand Down
16 changes: 7 additions & 9 deletions sys/dev/drm/freebsd/drm_gem_cma_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,25 +156,23 @@ drm_gem_cma_alloc(struct drm_device *drm, struct drm_gem_cma_object *bo)
}

static int
drm_gem_cma_fault(struct vm_area_struct *dummy, struct vm_fault *vmf)
drm_gem_cma_fault(struct vm_fault *vmf)
{
struct vm_area_struct *vma;
struct drm_gem_object *gem_obj;
struct drm_gem_cma_object *bo;
vm_object_t obj;
vm_pindex_t pidx;
struct page *page;
int i;

vma = vmf->vma;
gem_obj = vma->vm_private_data;
obj = vmf->object;
gem_obj = obj->handle;
bo = container_of(gem_obj, struct drm_gem_cma_object, gem_obj);
obj = vma->vm_obj;

if (!bo->m)
return (VM_FAULT_SIGBUS);

pidx = OFF_TO_IDX(vmf->address - vma->vm_start);
pidx = vmf->pindex;
if (pidx >= bo->npages)
return (VM_FAULT_SIGBUS);

Expand All @@ -191,9 +189,8 @@ drm_gem_cma_fault(struct vm_area_struct *dummy, struct vm_fault *vmf)
}
VM_OBJECT_WUNLOCK(obj);

vma->vm_pfn_first = 0;
vma->vm_pfn_count = bo->npages;
DRM_DEBUG("%s: pidx: %llu, start: 0x%08X, addr: 0x%08lX\n", __func__, pidx, vma->vm_start, vmf->address);
vmf->pindex = 0;
vmf->count = bo->npages;

return (VM_FAULT_NOPAGE);

Expand All @@ -209,6 +206,7 @@ const struct vm_operations_struct drm_gem_cma_vm_ops = {
.fault = drm_gem_cma_fault,
.open = drm_gem_vm_open,
.close = drm_gem_vm_close,
.objtype = OBJT_MGTDEVICE,
};

static int
Expand Down
Loading

0 comments on commit a50a81b

Please sign in to comment.