[PATCH v2] drm/xe: Support for mmap-ing mmio regions

[Matthew Brost]

On Tue, Jun 10, 2025 at 05:25:14PM +0300, Levi, Ilia wrote:
> On 10/06/2025 3:31, Matthew Brost wrote:
> > On Mon, Jun 09, 2025 at 02:58:20PM -0700, Matt Roper wrote:
> >> On Mon, Jun 09, 2025 at 08:46:19AM -0700, Matthew Brost wrote:
> >>> On Mon, Jun 09, 2025 at 12:59:38PM +0300, Ilia Levi wrote:
> >>>> Allow the driver to expose hardware register spaces to userspace
> >>>> through GEM objects with fake mmap offsets. This can be useful
> >>>> for userspace-firmware communication, debugging, etc.
> >>>>
> >>>> v2: Minor doc fix (CI)
> >>>>
> >>>> Signed-off-by: Ilia Levi <ilia.levi at intel.com>
> >>>> ---
> >>>>  drivers/gpu/drm/xe/xe_device_types.h |  14 +++
> >>>>  drivers/gpu/drm/xe/xe_mmio.c         | 142 +++++++++++++++++++++++++++
> >>>>  drivers/gpu/drm/xe/xe_mmio.h         |   4 +
> >>>>  3 files changed, 160 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> >>>> index ac27389ccb8b..78542de0d48d 100644
> >>>> --- a/drivers/gpu/drm/xe/xe_device_types.h
> >>>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> >>>> @@ -10,6 +10,7 @@
> >>>>  
> >>>>  #include <drm/drm_device.h>
> >>>>  #include <drm/drm_file.h>
> >>>> +#include <drm/drm_gem.h>
> >>>>  #include <drm/drm_pagemap.h>
> >>>>  #include <drm/ttm/ttm_device.h>
> >>>>  
> >>>> @@ -161,6 +162,19 @@ struct xe_mmio {
> >>>>  	u32 adj_offset;
> >>>>  };
> >>>>  
> >>>> +/**
> >>>> + * struct xe_mmio_gem - GEM wrapper for xe_mmio
> >>>> + *
> >>>> + * A GEM object for exposing xe_mmio instance to userspace via mmap.
> >>>> + */
> >>>> +struct xe_mmio_gem {
> >>>> +	/** @base: GEM object base */
> >>>> +	struct drm_gem_object base;
> >>>> +
> >>>> +	/** @mmio: The MMIO region to expose */
> >>>> +	struct xe_mmio mmio;
> >>> Any reason not this is not a pointer to xe_mmio?
> >>>
> >>>> +};
> >>>> +
> >>>>  /**
> >>>>   * struct xe_tile - hardware tile structure
> >>>>   *
> >>>> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> >>>> index 7357458bc0d2..6bfa915a9602 100644
> >>>> --- a/drivers/gpu/drm/xe/xe_mmio.c
> >>>> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> >>>> @@ -408,3 +408,145 @@ int xe_mmio_wait32_not(struct xe_mmio *mmio, struct xe_reg reg, u32 mask, u32 va
> >>>>  {
> >>>>  	return __xe_mmio_wait32(mmio, reg, mask, val, timeout_us, out_val, atomic, false);
> >>>>  }
> >>>> +
> >>>> +/**
> >>>> + * DOC: Exposing MMIO regions to userspace
> >>>> + *
> >>>> + * In certain cases, the driver may allow userspace to mmap a portion of the hardware registers.
> >>>> + *
> >>>> + * This can be done as follows:
> >>>> + * 1. Define an xe_mmio instance that represents this portion.
> >>>> + * 2. Call xe_mmio_gem_create() to create a GEM object with an mmap-able fake offset.
> >>>> + * 3. Use drm_vma_node_offset_addr() on the created GEM object to retrieve the fake offset.
> >>>> + * 4. Provide the fake offset to userspace.
> >>>> + * 5. Userspace can call mmap with the fake offset. The length provided to mmap
> >>>> + *    must match the size of the xe_mmio instance.
> >>>> + * 6. When the region is no longer needed, call xe_mmio_gem_destroy() to release the GEM object.
> >>>> + *
> >>>> + * Limitations: The exposed xe_mmio must be page-aligned with regards to its BAR offset and size.
> >>>> + *
> >>>> + * WARNING: Exposing MMIO regions to userspace can have security and stability implications.
> >>>> + * Make sure not to expose any sensitive registers.
> >>> This is no secondary issue, it is a primary one. There is no way we can
> >>> expose entire PCIe MMIO register space to a non-root user (a root user
> >>> itself can just mmap the PCIe MMIO bar bypassing the XeKMD if it wants
> >>> btw).
> >>>
> >>> What is the requirement for this patch? I know on other PCIe devices
> >>> I've worked on we had similar issues where we needed to expose a subset
> >>> of the PCIe MMIO register space to user space for kernel bypass
> >>> submission - everything we needed to expose in that case was 4k aligned
> >>> + tied to a single process via a KMD config before mapping to user
> >>> space. I suspect to do anything safely here, somethine similar needs to
> >>> be done.
> >> I don't think it was the original intent of Ilia's patch, but I can
> > Yea, I think I figured that out after I sent this - the xe_mmio
> > structure through me off here as this structure currently only exists in
> > the xe_tile, xe_gt but I'm assuming now xe_mmio_gem_create is called
> > with a xe_mmio that is subset of either the one of those, which I can't
> > say I love this as a design - I'd rather have xe_mmio_gem_create called
> > with a pointer + size to avoid confusion.
> 
> 
> Actually my initial implementation was with bar offset and size. But we plan to
> use xe_mmio as a "view" on certain portions of the BAR (i.e. hw blocks) in
> upcoming platforms, so it lends itself naturally to be "exposed". See the example
> from my previous mail, and if you still think it's a bad choice - let's discuss.
> - Ilia
> 

It is hard to tell without a complete picture, also a bit of bikeshed.
My preference still would be offset + size, but it makes more sense to
use xe_mmio with upcoming changes, then with it as is.

> 
> >> think of at least one feature expected on an upcoming platform where
> >> there is a page or two of registers that the hardware team designed
> >> under the assumption that they'd get mapped directly to userspace in a
> >> read-only manner (i.e., information read-out only, no control of the
> >> hardware).  I think some of the internal discussion was considering just
> >> going our own direction and exposing the contents of that register block
> >> via an xe_query instead since the contents likely wouldn't be changing
> >> after boot, but if this infrastructure exists it might be a simpler and
> >> more natural match for the hardware design.  +Cc Shuicheng since he
> >> might find this interesting.
> >>
> >> But if we're allowing this to map registers to userspace in a read/write
> >> manner (or map any special registers that have side effects on read)
> >> then we probably need a bit more scrutiny of the specific use cases to
> >> determine what kind of permissions are required, whether they're safe to
> >> access in concurrent / racing manners, etc.
> >>
> > I think rules basically are - cannot read another processes data, cannot
> > issue a write that has any affect on another process. I'd assume if the
> > hardware is designed with mapping MMIO space to user processes in mind,
> > it would be designed in a way to not violate these rules.
> >
> > Matt
> 
> 
> Sounds reasonable, doesn't |VM_DONTCOPY| + drm_vma_node_allow present in this
> patch cover this? Or am I missing something?

As Matt R. suggests, the security here is baked into the hardware in
that we don't expose any problematic registers which would break the
above rules.

Matt

> - Ilia
> 
> 
> >
> >> And of course since this is new uapi, the regular graphics upstreaming
> >> rules apply and we'll need an opensource userspace consumer fully
> >> developed and reviewed before we can merge the kernel changes.
> >>
> >>
> >> Matt
> >>
> >>> Matt
> >>>
> >>>> + */
> >>>> +
> >>>> +static void xe_mmio_gem_free(struct drm_gem_object *);
> >>>> +static int xe_mmio_gem_mmap(struct drm_gem_object *, struct vm_area_struct *);
> >>>> +
> >>>> +static const struct vm_operations_struct vm_ops = {
> >>>> +	.open = drm_gem_vm_open,
> >>>> +	.close = drm_gem_vm_close,
> >>>> +};
> >>>> +
> >>>> +static const struct drm_gem_object_funcs xe_mmio_gem_funcs = {
> >>>> +	.free = xe_mmio_gem_free,
> >>>> +	.mmap = xe_mmio_gem_mmap,
> >>>> +	.vm_ops = &vm_ops,
> >>>> +};
> >>>> +
> >>>> +static inline struct xe_mmio_gem *to_xe_mmio_gem(struct drm_gem_object *obj)
> >>>> +{
> >>>> +	return container_of(obj, struct xe_mmio_gem, base);
> >>>> +}
> >>>> +
> >>>> +static inline phys_addr_t xe_mmio_phys_addr(struct xe_mmio *mmio)
> >>>> +{
> >>>> +	struct xe_device *xe = tile_to_xe(mmio->tile);
> >>>> +
> >>>> +	/*
> >>>> +	 * All MMIO instances are currently on PCI BAR 0, so we can do the trick below.
> >>>> +	 * In the future we may want to store the physical address in struct xe_mmio.
> >>>> +	 */
> >>>> +	return pci_resource_start(to_pci_dev(xe->drm.dev), GTTMMADR_BAR) +
> >>>> +		(uintptr_t)(mmio->regs - xe->mmio.regs);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * xe_mmio_gem_create - Expose an MMIO region to userspace
> >>>> + * @mmio: xe_mmio instance
> >>>> + * @file: DRM file descriptor
> >>>> + *
> >>>> + * This function creates a GEM object with an mmap-able fake offset that wraps
> >>>> + * the provided xe_mmio instance.
> >>>> + *
> >>>> + * See: "Exposing MMIO regions to userspace"
> >>>> + */
> >>>> +struct xe_mmio_gem *
> >>>> +xe_mmio_gem_create(struct xe_mmio *mmio, struct drm_file *file)
> >>>> +{
> >>>> +	struct xe_device *xe = tile_to_xe(mmio->tile);
> >>>> +	size_t size = mmio->regs_size;
> >>>> +	struct xe_mmio_gem *obj;
> >>>> +	struct drm_gem_object *base;
> >>>> +	int err;
> >>>> +
> >>>> +	if ((xe_mmio_phys_addr(mmio) % PAGE_SIZE != 0) || (size % PAGE_SIZE != 0))
> >>>> +		return ERR_PTR(-EINVAL);
> >>>> +
> >>>> +	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> >>>> +	if (!obj)
> >>>> +		return ERR_PTR(-ENOMEM);
> >>>> +
> >>>> +	base = &obj->base;
> >>>> +	base->funcs = &xe_mmio_gem_funcs;
> >>>> +	obj->mmio = *mmio;
> >>>> +
> >>>> +	drm_gem_private_object_init(&xe->drm, base, size);
> >>>> +
> >>>> +	err = drm_gem_create_mmap_offset(base);
> >>>> +	if (err)
> >>>> +		goto free_gem;
> >>>> +
> >>>> +	err = drm_vma_node_allow(&base->vma_node, file);
> >>>> +	if (err)
> >>>> +		goto free_gem;
> >>>> +
> >>>> +	return obj;
> >>>> +
> >>>> +free_gem:
> >>>> +	xe_mmio_gem_free(base);
> >>>> +	return ERR_PTR(err);
> >>>> +}
> >>>> +
> >>>> +static void xe_mmio_gem_free(struct drm_gem_object *base)
> >>>> +{
> >>>> +	struct xe_mmio_gem *obj = to_xe_mmio_gem(base);
> >>>> +
> >>>> +	drm_gem_object_release(base);
> >>>> +	kfree(obj);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>> + * xe_mmio_gem_destroy - Destroy the GEM object wrapping xe_mmio
> >>>> + * @gem: the GEM object to destroy
> >>>> + *
> >>>> + * This function releases resources associated with the GEM object created by
> >>>> + * xe_mmio_gem_create().
> >>>> + *
> >>>> + * See: "Exposing MMIO regions to userspace"
> >>>> + */
> >>>> +void xe_mmio_gem_destroy(struct xe_mmio_gem *gem)
> >>>> +{
> >>>> +	xe_mmio_gem_free(&gem->base);
> >>>> +}
> >>>> +
> >>>> +static int xe_mmio_gem_mmap(struct drm_gem_object *base, struct vm_area_struct *vma)
> >>>> +{
> >>>> +	struct xe_mmio_gem *obj = to_xe_mmio_gem(base);
> >>>> +	struct xe_mmio *mmio = &obj->mmio;
> >>>> +
> >>>> +	if (vma->vm_end - vma->vm_start != base->size)
> >>>> +		return -EINVAL;
> >>>> +
> >>>> +	/*
> >>>> +	 * Set vm_pgoff (used as a fake buffer offset by DRM) to 0 and map the
> >>>> +	 * whole buffer from the start.
> >>>> +	 */
> >>>> +	vma->vm_pgoff = 0;
> >>>> +	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> >>>> +
> >>>> +	vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP |
> >>>> +		     VM_DONTCOPY | VM_NORESERVE);
> >>>> +
> >>>> +	return remap_pfn_range(vma, vma->vm_start, xe_mmio_phys_addr(mmio) >> PAGE_SHIFT,
> >>>> +			       base->size, vma->vm_page_prot);
> >>>> +}
> >>>> diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
> >>>> index c151ba569003..2990bbcef24d 100644
> >>>> --- a/drivers/gpu/drm/xe/xe_mmio.h
> >>>> +++ b/drivers/gpu/drm/xe/xe_mmio.h
> >>>> @@ -8,6 +8,7 @@
> >>>>  
> >>>>  #include "xe_gt_types.h"
> >>>>  
> >>>> +struct drm_file;
> >>>>  struct xe_device;
> >>>>  struct xe_reg;
> >>>>  
> >>>> @@ -42,4 +43,7 @@ static inline struct xe_mmio *xe_root_tile_mmio(struct xe_device *xe)
> >>>>  	return &xe->tiles[0].mmio;
> >>>>  }
> >>>>  
> >>>> +struct xe_mmio_gem *xe_mmio_gem_create(struct xe_mmio *mmio, struct drm_file *file);
> >>>> +void xe_mmio_gem_destroy(struct xe_mmio_gem *gem);
> >>>> +
> >>>>  #endif
> >>>> -- 
> >>>> 2.43.0
> >>>>
> >> -- 
> >> Matt Roper
> >> Graphics Software Engineer
> >> Linux GPU Platform Enablement
> >> Intel Corporation
> 
>