Message-ID: <Y9MQCfCo9F7dwl33@DUT025-TGLU.fm.intel.com>
Date:   Thu, 26 Jan 2023 23:43:05 +0000
From:   Matthew Brost <matthew.brost@...el.com>
To:     Danilo Krummrich <dakr@...hat.com>
CC:     <daniel@...ll.ch>, <airlied@...hat.com>,
        <christian.koenig@....com>, <bskeggs@...hat.com>,
        <jason@...kstrand.net>, <tzimmermann@...e.de>,
        <mripard@...nel.org>, <corbet@....net>,
        <nouveau@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
        <dri-devel@...ts.freedesktop.org>, <linux-doc@...r.kernel.org>
Subject: Re: [PATCH drm-next 03/14] drm: manager to keep track of GPUs VA
 mappings

On Wed, Jan 18, 2023 at 07:12:45AM +0100, Danilo Krummrich wrote:
> This adds the infrastructure for a manager implementation to keep track
> of GPU virtual address (VA) mappings.
> 
> New UAPIs, motivated by Vulkan sparse memory bindings graphics drivers
> start implementing, allow userspace applications to request multiple and
> arbitrary GPU VA mappings of buffer objects. The DRM GPU VA manager is
> intended to serve the following purposes in this context.
> 
> 1) Provide a dedicated range allocator to track GPU VA allocations and
>    mappings, making use of the drm_mm range allocator.
> 
> 2) Generically connect GPU VA mappings to their backing buffers, in
>    particular DRM GEM objects.
> 
> 3) Provide a common implementation to perform more complex mapping
>    operations on the GPU VA space. In particular splitting and merging
>    of GPU VA mappings, e.g. for intersecting mapping requests or partial
>    unmap requests.
> 
> Idea-suggested-by: Dave Airlie <airlied@...hat.com>
> Signed-off-by: Danilo Krummrich <dakr@...hat.com>

<snip>

> diff --git a/include/drm/drm_gpuva_mgr.h b/include/drm/drm_gpuva_mgr.h
> new file mode 100644
> index 000000000000..adeb0c916e91
> --- /dev/null
> +++ b/include/drm/drm_gpuva_mgr.h
> @@ -0,0 +1,527 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#ifndef __DRM_GPUVA_MGR_H__
> +#define __DRM_GPUVA_MGR_H__
> +
> +/*
> + * Copyright (c) 2022 Red Hat.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <drm/drm_mm.h>
> +#include <linux/mm.h>
> +#include <linux/rbtree.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +struct drm_gpuva_region;
> +struct drm_gpuva;
> +struct drm_gpuva_ops;
> +
> +/**
> + * struct drm_gpuva_manager - DRM GPU VA Manager
> + *
> + * The DRM GPU VA Manager keeps track of a GPU's virtual address space by using
> + * the &drm_mm range allocator. Typically, this structure is embedded in bigger
> + * driver structures.
> + *
> + * Drivers can pass addresses and ranges in an arbitrary unit, e.g. bytes or
> + * pages.
> + *
> + * There should be one manager instance per GPU virtual address space.
> + */
> +struct drm_gpuva_manager {
> +	/**
> +	 * @name: the name of the DRM GPU VA space
> +	 */
> +	const char *name;
> +
> +	/**
> +	 * @mm_start: start of the VA space
> +	 */
> +	u64 mm_start;
> +
> +	/**
> +	 * @mm_range: length of the VA space
> +	 */
> +	u64 mm_range;
> +
> +	/**
> +	 * @region_mm: the &drm_mm range allocator to track GPU VA regions
> +	 */
> +	struct drm_mm region_mm;
> +

I'd suggest using a rb_tree rather than drm_mm, it should be quite a bit
more light weight - that is what we currently use in Xe for VM / VMA
management.

See lines 994-1056 in the following file:
https://cgit.freedesktop.org/drm/drm-xe/tree/drivers/gpu/drm/xe/xe_vm.c?h=drm-xe-next

I'm pretty sure all of your magic marcos (drm_gpuva_for_each*) should be
easily implemented using a rb_tree too.

Matt

> +	/**
> +	 * @va_mm: the &drm_mm range allocator to track GPU VA mappings
> +	 */
> +	struct drm_mm va_mm;
> +
> +	/**
> +	 * @kernel_alloc_node:
> +	 *
> +	 * &drm_mm_node representing the address space cutout reserved for
> +	 * the kernel
> +	 */
> +	struct drm_mm_node kernel_alloc_node;
> +};
> +
> +void drm_gpuva_manager_init(struct drm_gpuva_manager *mgr,
> +			    const char *name,
> +			    u64 start_offset, u64 range,
> +			    u64 reserve_offset, u64 reserve_range);
> +void drm_gpuva_manager_destroy(struct drm_gpuva_manager *mgr);
> +
> +/**
> + * struct drm_gpuva_region - structure to track a portion of GPU VA space
> + *
> + * This structure represents a portion of a GPUs VA space and is associated
> + * with a &drm_gpuva_manager. Internally it is based on a &drm_mm_node.
> + *
> + * GPU VA mappings, represented by &drm_gpuva objects, are restricted to be
> + * placed within a &drm_gpuva_region.
> + */
> +struct drm_gpuva_region {
> +	/**
> +	 * @node: the &drm_mm_node to track the GPU VA region
> +	 */
> +	struct drm_mm_node node;
> +
> +	/**
> +	 * @mgr: the &drm_gpuva_manager this object is associated with
> +	 */
> +	struct drm_gpuva_manager *mgr;
> +
> +	/**
> +	 * @sparse: indicates whether this region is sparse
> +	 */
> +	bool sparse;
> +};
> +
> +struct drm_gpuva_region *
> +drm_gpuva_region_find(struct drm_gpuva_manager *mgr,
> +		      u64 addr, u64 range);
> +int drm_gpuva_region_insert(struct drm_gpuva_manager *mgr,
> +			    struct drm_gpuva_region *reg,
> +			    u64 addr, u64 range);
> +void drm_gpuva_region_destroy(struct drm_gpuva_manager *mgr,
> +			      struct drm_gpuva_region *reg);
> +
> +int drm_gpuva_insert(struct drm_gpuva_manager *mgr,
> +		     struct drm_gpuva *va,
> +		     u64 addr, u64 range);
> +/**
> + * drm_gpuva_for_each_region_in_range - iternator to walk over a range of nodes
> + * @node__: &drm_gpuva_region structure to assign to in each iteration step
> + * @gpuva__: &drm_gpuva_manager structure to walk
> + * @start__: starting offset, the first node will overlap this
> + * @end__: ending offset, the last node will start before this (but may overlap)
> + *
> + * This iterator walks over all nodes in the range allocator that lie
> + * between @start and @end. It is implemented similarly to list_for_each(),
> + * but is using &drm_mm's internal interval tree to accelerate the search for
> + * the starting node, and hence isn't safe against removal of elements. It
> + * assumes that @end is within (or is the upper limit of) the &drm_gpuva_manager.
> + * If [@start, @end] are beyond the range of the &drm_gpuva_manager, the
> + * iterator may walk over the special _unallocated_ &drm_mm.head_node of the
> + * backing &drm_mm, and may even continue indefinitely.
> + */
> +#define drm_gpuva_for_each_region_in_range(node__, gpuva__, start__, end__) \
> +	for (node__ = (struct drm_gpuva_region *)__drm_mm_interval_first(&(gpuva__)->region_mm, \
> +									 (start__), (end__)-1); \
> +	     node__->node.start < (end__); \
> +	     node__ = (struct drm_gpuva_region *)list_next_entry(&node__->node, node_list))
> +
> +/**
> + * drm_gpuva_for_each_region - iternator to walk over a range of nodes
> + * @entry: &drm_gpuva_region structure to assign to in each iteration step
> + * @gpuva: &drm_gpuva_manager structure to walk
> + *
> + * This iterator walks over all &drm_gpuva_region structures associated with the
> + * &drm_gpuva_manager.
> + */
> +#define drm_gpuva_for_each_region(entry, gpuva) \
> +	list_for_each_entry(entry, drm_mm_nodes(&(gpuva)->region_mm), node.node_list)
> +
> +/**
> + * drm_gpuva_for_each_region_safe - iternator to safely walk over a range of
> + * nodes
> + * @entry: &drm_gpuva_region structure to assign to in each iteration step
> + * @next: &next &drm_gpuva_region to store the next step
> + * @gpuva: &drm_gpuva_manager structure to walk
> + *
> + * This iterator walks over all &drm_gpuva_region structures associated with the
> + * &drm_gpuva_manager. It is implemented with list_for_each_safe(), so save
> + * against removal of elements.
> + */
> +#define drm_gpuva_for_each_region_safe(entry, next, gpuva) \
> +	list_for_each_entry_safe(entry, next, drm_mm_nodes(&(gpuva)->region_mm), node.node_list)
> +
> +
> +/**
> + * enum drm_gpuva_flags - flags for struct drm_gpuva
> + */
> +enum drm_gpuva_flags {
> +	/**
> +	 * @DRM_GPUVA_SWAPPED: flag indicating that the &drm_gpuva is swapped
> +	 */
> +	DRM_GPUVA_SWAPPED = (1 << 0),
> +};
> +
> +/**
> + * struct drm_gpuva - structure to track a GPU VA mapping
> + *
> + * This structure represents a GPU VA mapping and is associated with a
> + * &drm_gpuva_manager. Internally it is based on a &drm_mm_node.
> + *
> + * Typically, this structure is embedded in bigger driver structures.
> + */
> +struct drm_gpuva {
> +	/**
> +	 * @node: the &drm_mm_node to track the GPU VA mapping
> +	 */
> +	struct drm_mm_node node;
> +
> +	/**
> +	 * @mgr: the &drm_gpuva_manager this object is associated with
> +	 */
> +	struct drm_gpuva_manager *mgr;
> +
> +	/**
> +	 * @region: the &drm_gpuva_region the &drm_gpuva is mapped in
> +	 */
> +	struct drm_gpuva_region *region;
> +
> +	/**
> +	 * @head: the &list_head to attach this object to a &drm_gem_object
> +	 */
> +	struct list_head head;
> +
> +	/**
> +	 * @flags: the &drm_gpuva_flags for this mapping
> +	 */
> +	enum drm_gpuva_flags flags;
> +
> +	/**
> +	 * @gem: structure containing the &drm_gem_object and it's offset
> +	 */
> +	struct {
> +		/**
> +		 * @offset: the offset within the &drm_gem_object
> +		 */
> +		u64 offset;
> +
> +		/**
> +		 * @obj: the mapped &drm_gem_object
> +		 */
> +		struct drm_gem_object *obj;
> +	} gem;
> +};
> +
> +void drm_gpuva_link_locked(struct drm_gpuva *va);
> +void drm_gpuva_link_unlocked(struct drm_gpuva *va);
> +void drm_gpuva_unlink_locked(struct drm_gpuva *va);
> +void drm_gpuva_unlink_unlocked(struct drm_gpuva *va);
> +
> +void drm_gpuva_destroy_locked(struct drm_gpuva *va);
> +void drm_gpuva_destroy_unlocked(struct drm_gpuva *va);
> +
> +struct drm_gpuva *drm_gpuva_find(struct drm_gpuva_manager *mgr,
> +				 u64 addr, u64 range);
> +struct drm_gpuva *drm_gpuva_find_prev(struct drm_gpuva_manager *mgr, u64 start);
> +struct drm_gpuva *drm_gpuva_find_next(struct drm_gpuva_manager *mgr, u64 end);
> +
> +/**
> + * drm_gpuva_swap - sets whether the backing BO of this &drm_gpuva is swapped
> + * @va: the &drm_gpuva to set the swap flag of
> + * @swap: indicates whether the &drm_gpuva is swapped
> + */
> +static inline void drm_gpuva_swap(struct drm_gpuva *va, bool swap)
> +{
> +	if (swap)
> +		va->flags |= DRM_GPUVA_SWAPPED;
> +	else
> +		va->flags &= ~DRM_GPUVA_SWAPPED;
> +}
> +
> +/**
> + * drm_gpuva_swapped - indicates whether the backing BO of this &drm_gpuva
> + * is swapped
> + * @va: the &drm_gpuva to check
> + */
> +static inline bool drm_gpuva_swapped(struct drm_gpuva *va)
> +{
> +	return va->flags & DRM_GPUVA_SWAPPED;
> +}
> +
> +/**
> + * drm_gpuva_for_each_va_in_range - iternator to walk over a range of nodes
> + * @node__: &drm_gpuva structure to assign to in each iteration step
> + * @gpuva__: &drm_gpuva_manager structure to walk
> + * @start__: starting offset, the first node will overlap this
> + * @end__: ending offset, the last node will start before this (but may overlap)
> + *
> + * This iterator walks over all nodes in the range allocator that lie
> + * between @start and @end. It is implemented similarly to list_for_each(),
> + * but is using &drm_mm's internal interval tree to accelerate the search for
> + * the starting node, and hence isn't safe against removal of elements. It
> + * assumes that @end is within (or is the upper limit of) the &drm_gpuva_manager.
> + * If [@start, @end] are beyond the range of the &drm_gpuva_manager, the
> + * iterator may walk over the special _unallocated_ &drm_mm.head_node of the
> + * backing &drm_mm, and may even continue indefinitely.
> + */
> +#define drm_gpuva_for_each_va_in_range(node__, gpuva__, start__, end__) \
> +	for (node__ = (struct drm_gpuva *)__drm_mm_interval_first(&(gpuva__)->va_mm, \
> +								  (start__), (end__)-1); \
> +	     node__->node.start < (end__); \
> +	     node__ = (struct drm_gpuva *)list_next_entry(&node__->node, node_list))
> +
> +/**
> + * drm_gpuva_for_each_va - iternator to walk over a range of nodes
> + * @entry: &drm_gpuva structure to assign to in each iteration step
> + * @gpuva: &drm_gpuva_manager structure to walk
> + *
> + * This iterator walks over all &drm_gpuva structures associated with the
> + * &drm_gpuva_manager.
> + */
> +#define drm_gpuva_for_each_va(entry, gpuva) \
> +	list_for_each_entry(entry, drm_mm_nodes(&(gpuva)->va_mm), node.node_list)
> +
> +/**
> + * drm_gpuva_for_each_va_safe - iternator to safely walk over a range of
> + * nodes
> + * @entry: &drm_gpuva structure to assign to in each iteration step
> + * @next: &next &drm_gpuva to store the next step
> + * @gpuva: &drm_gpuva_manager structure to walk
> + *
> + * This iterator walks over all &drm_gpuva structures associated with the
> + * &drm_gpuva_manager. It is implemented with list_for_each_safe(), so save
> + * against removal of elements.
> + */
> +#define drm_gpuva_for_each_va_safe(entry, next, gpuva) \
> +	list_for_each_entry_safe(entry, next, drm_mm_nodes(&(gpuva)->va_mm), node.node_list)
> +
> +/**
> + * enum drm_gpuva_op_type - GPU VA operation type
> + *
> + * Operations to alter the GPU VA mappings tracked by the &drm_gpuva_manager
> + * can be map, remap or unmap operations.
> + */
> +enum drm_gpuva_op_type {
> +	/**
> +	 * @DRM_GPUVA_OP_MAP: the map op type
> +	 */
> +	DRM_GPUVA_OP_MAP,
> +
> +	/**
> +	 * @DRM_GPUVA_OP_REMAP: the remap op type
> +	 */
> +	DRM_GPUVA_OP_REMAP,
> +
> +	/**
> +	 * @DRM_GPUVA_OP_UNMAP: the unmap op type
> +	 */
> +	DRM_GPUVA_OP_UNMAP,
> +};
> +
> +/**
> + * struct drm_gpuva_op_map - GPU VA map operation
> + *
> + * This structure represents a single map operation generated by the
> + * DRM GPU VA manager.
> + */
> +struct drm_gpuva_op_map {
> +	/**
> +	 * @va: structure containing address and range of a map
> +	 * operation
> +	 */
> +	struct {
> +		/**
> +		 * @addr: the base address of the new mapping
> +		 */
> +		u64 addr;
> +
> +		/**
> +		 * @range: the range of the new mapping
> +		 */
> +		u64 range;
> +	} va;
> +
> +	/**
> +	 * @gem: structure containing the &drm_gem_object and it's offset
> +	 */
> +	struct {
> +		/**
> +		 * @offset: the offset within the &drm_gem_object
> +		 */
> +		u64 offset;
> +
> +		/**
> +		 * @obj: the &drm_gem_object to map
> +		 */
> +		struct drm_gem_object *obj;
> +	} gem;
> +};
> +
> +/**
> + * struct drm_gpuva_op_unmap - GPU VA unmap operation
> + *
> + * This structure represents a single unmap operation generated by the
> + * DRM GPU VA manager.
> + */
> +struct drm_gpuva_op_unmap {
> +	/**
> +	 * @va: the &drm_gpuva to unmap
> +	 */
> +	struct drm_gpuva *va;
> +
> +	/**
> +	 * @keep:
> +	 *
> +	 * Indicates whether this &drm_gpuva is physically contiguous with the
> +	 * original mapping request.
> +	 *
> +	 * Optionally, if &keep is set, drivers may keep the actual page table
> +	 * mappings for this &drm_gpuva, adding the missing page table entries
> +	 * only and update the &drm_gpuva_manager accordingly.
> +	 */
> +	bool keep;
> +};
> +
> +/**
> + * struct drm_gpuva_op_remap - GPU VA remap operation
> + *
> + * This represents a single remap operation generated by the DRM GPU VA manager.
> + *
> + * A remap operation is generated when an existing GPU VA mmapping is split up
> + * by inserting a new GPU VA mapping or by partially unmapping existent
> + * mapping(s), hence it consists of a maximum of two map and one unmap
> + * operation.
> + *
> + * The @unmap operation takes care of removing the original existing mapping.
> + * @prev is used to remap the preceding part, @next the subsequent part.
> + *
> + * If either a new mapping's start address is aligned with the start address
> + * of the old mapping or the new mapping's end address is aligned with the
> + * end address of the old mapping, either @prev or @next is NULL.
> + *
> + * Note, the reason for a dedicated remap operation, rather than arbitrary
> + * unmap and map operations, is to give drivers the chance of extracting driver
> + * specific data for creating the new mappings from the unmap operations's
> + * &drm_gpuva structure which typically is embedded in larger driver specific
> + * structures.
> + */
> +struct drm_gpuva_op_remap {
> +	/**
> +	 * @prev: the preceding part of a split mapping
> +	 */
> +	struct drm_gpuva_op_map *prev;
> +
> +	/**
> +	 * @next: the subsequent part of a split mapping
> +	 */
> +	struct drm_gpuva_op_map *next;
> +
> +	/**
> +	 * @unmap: the unmap operation for the original existing mapping
> +	 */
> +	struct drm_gpuva_op_unmap *unmap;
> +};
> +
> +/**
> + * struct drm_gpuva_op - GPU VA operation
> + *
> + * This structure represents a single generic operation, which can be either
> + * map, unmap or remap.
> + *
> + * The particular type of the operation is defined by @op.
> + */
> +struct drm_gpuva_op {
> +	/**
> +	 * @entry:
> +	 *
> +	 * The &list_head used to distribute instances of this struct within
> +	 * &drm_gpuva_ops.
> +	 */
> +	struct list_head entry;
> +
> +	/**
> +	 * @op: the type of the operation
> +	 */
> +	enum drm_gpuva_op_type op;
> +
> +	union {
> +		/**
> +		 * @map: the map operation
> +		 */
> +		struct drm_gpuva_op_map map;
> +
> +		/**
> +		 * @unmap: the unmap operation
> +		 */
> +		struct drm_gpuva_op_unmap unmap;
> +
> +		/**
> +		 * @remap: the remap operation
> +		 */
> +		struct drm_gpuva_op_remap remap;
> +	};
> +};
> +
> +/**
> + * struct drm_gpuva_ops - wraps a list of &drm_gpuva_op
> + */
> +struct drm_gpuva_ops {
> +	/**
> +	 * @list: the &list_head
> +	 */
> +	struct list_head list;
> +};
> +
> +/**
> + * drm_gpuva_for_each_op - iterator to walk over all ops
> + * @op: &drm_gpuva_op to assign in each iteration step
> + * @ops: &drm_gpuva_ops to walk
> + *
> + * This iterator walks over all ops within a given list of operations.
> + */
> +#define drm_gpuva_for_each_op(op, ops) list_for_each_entry(op, &(ops)->list, entry)
> +
> +/**
> + * drm_gpuva_for_each_op_safe - iterator to safely walk over all ops
> + * @op: &drm_gpuva_op to assign in each iteration step
> + * @next: &next &drm_gpuva_op to store the next step
> + * @ops: &drm_gpuva_ops to walk
> + *
> + * This iterator walks over all ops within a given list of operations. It is
> + * implemented with list_for_each_safe(), so save against removal of elements.
> + */
> +#define drm_gpuva_for_each_op_safe(op, next, ops) \
> +	list_for_each_entry_safe(op, next, &(ops)->list, entry)
> +
> +struct drm_gpuva_ops *
> +drm_gpuva_sm_map_ops_create(struct drm_gpuva_manager *mgr,
> +			    u64 addr, u64 range,
> +			    struct drm_gem_object *obj, u64 offset);
> +struct drm_gpuva_ops *
> +drm_gpuva_sm_unmap_ops_create(struct drm_gpuva_manager *mgr,
> +			      u64 addr, u64 range);
> +void drm_gpuva_ops_free(struct drm_gpuva_ops *ops);
> +
> +#endif /* __DRM_GPUVA_MGR_H__ */
> -- 
> 2.39.0
> 

Powered by blists - more mailing lists