[PATCH v2 18/32] drm/xe/vm: Add attributes struct as member of vma
Matthew Brost
matthew.brost at intel.com
Wed May 14 18:36:03 UTC 2025
On Mon, Apr 07, 2025 at 03:47:05PM +0530, Himal Prasad Ghimiray wrote:
> The attribute of xe_vma will determine the migration policy and the
> encoding of the page table entries (PTEs) for that vma.
> This attribute helps manage how memory pages are moved and how their
> addresses are translated. It will be used by madvise to set the
> behavior of the vma.
>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
> ---
> drivers/gpu/drm/xe/xe_vm.c | 6 ++++++
> drivers/gpu/drm/xe/xe_vm_types.h | 20 ++++++++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 27a8dbe709c2..1ff9e477e061 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2470,6 +2470,12 @@ static struct xe_vma *new_vma(struct xe_vm *vm, struct drm_gpuva_op_map *op,
> vma = ERR_PTR(err);
> }
>
> + /*TODO: assign devmem_fd of local vram once multi device
> + * support is added.
> + */
> + vma->attr.preferred_loc.devmem_fd = 1;
Assigning a value of '1' is a bit odd... I'd prefer using a define or
something similar to indicate the intended behavior. I noticed a few
other assignments to '1' in the final result—same comment applies to
those.
> + vma->attr.atomic_access = DRM_XE_VMA_ATOMIC_UNDEFINED;
> +
> return vma;
> }
>
> diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> index d3c1209348e9..5f5feffecb82 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -77,6 +77,19 @@ struct xe_userptr {
> #endif
> };
>
> +/**
> + * struct xe_vma_mem_attr - memory attributes associated with vma
> + */
> +struct xe_vma_mem_attr {
> + /** @preferred_loc: perferred memory_location*/
> + struct {
> + u32 migration_policy; /* represents migration policies */
> + u32 devmem_fd; /* devmem_fd used for determining pagemap_fd requested by user */
> + } preferred_loc;
I'm a little unclear on how these variables work.
In the uAPI for migration_policy, I see MIGRATE_ALL_PAGES and
MIGRATE_ONLY_SYSTEM_PAGES (these should probably be normalized with a
DRM_XE_* prefix, by the way), but it's unclear to me what exactly these
mean or how they're used based on the final result—could you clarify?
Likewise, I'm confused about the devmem_fd usage. It can either be
assigned a devmem_fd from the uAPI, but in some cases, it's interpreted
as a region. I assume this is anticipating multi-GPU support, but again,
the plan isn't clear to me. Could you explain?
In general I agree with the idea of xe_vma_mem_attr though.
Matt
> + /** @atomic_access: The atomic access type for the vma */
> + u32 atomic_access;
> +};
> +
> struct xe_vma {
> /** @gpuva: Base GPUVA object */
> struct drm_gpuva gpuva;
> @@ -128,6 +141,13 @@ struct xe_vma {
> * Needs to be signalled before UNMAP can be processed.
> */
> struct xe_user_fence *ufence;
> +
> + /**
> + * @attr: The attributes of vma which determines the migration policy
> + * and encoding of the PTEs for this vma.
> + */
> + struct xe_vma_mem_attr attr;
> +
> };
>
> /**
> --
> 2.34.1
>
More information about the Intel-xe
mailing list