On Wed, 2025-01-29 at 11:52 -0800, Matthew Brost wrote:
> Add XE_BO_FLAG_CPU_ADDR_MIRROR to indicate BO is tied to SVM range.
> While these BO's are kernel allocations, we need a VM reference in
> this
> case which this flag indicates. In addition, we do not support CCS on
> these BO's either. The later can be revisited later.
>
> v2:
> - Take VM ref for system allocator BOs
> v3:
> - s/XE_BO_FLAG_SYSTEM_ALLOC/XE_BO_FLAG_CPU_ADDR_MIRROR (Thomas)
> - Better commit message (Thomas)
> - Drop XE_BO_FLAG_SKIP_CLEAR for now
> - Add comment about possibly supporting CCS (Thomas)
> v4:
> - Fix alignment issue (Checkpatch)
>
> Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
I was wondering, since the bo might as well be an external bo and
benefit from finer resv granularity on eviction, (multi-device actually
uses this), can't we drop the bo->vm reference? And, assuming tile is
not needed either (is it)? Can we skip the flag altogether?
/Thomas
> ---
> drivers/gpu/drm/xe/xe_bo.c | 12 ++++++++----
> drivers/gpu/drm/xe/xe_bo.h | 1 +
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index e914a60b8afc..20c96709e267 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -1239,7 +1239,7 @@ static void xe_ttm_bo_destroy(struct
> ttm_buffer_object *ttm_bo)
> xe_drm_client_remove_bo(bo);
> #endif
>
> - if (bo->vm && xe_bo_is_user(bo))
> + if (bo->vm && (xe_bo_is_user(bo) || bo->flags &
> XE_BO_FLAG_CPU_ADDR_MIRROR))
> xe_vm_put(bo->vm);
>
> mutex_lock(&xe->mem_access.vram_userfault.lock);
> @@ -1435,7 +1435,8 @@ struct xe_bo *___xe_bo_create_locked(struct
> xe_device *xe, struct xe_bo *bo,
> int err;
>
> /* Only kernel objects should set GT */
> - xe_assert(xe, !tile || type == ttm_bo_type_kernel);
> + xe_assert(xe, !tile || type == ttm_bo_type_kernel ||
> + flags & XE_BO_FLAG_CPU_ADDR_MIRROR);
>
> if (XE_WARN_ON(!size)) {
> xe_bo_free(bo);
> @@ -1631,7 +1632,7 @@ __xe_bo_create_locked(struct xe_device *xe,
> * by having all the vm's bo refereferences released at vm
> close
> * time.
> */
> - if (vm && xe_bo_is_user(bo))
> + if (vm && (xe_bo_is_user(bo) || bo->flags &
> XE_BO_FLAG_CPU_ADDR_MIRROR))
> xe_vm_get(vm);
> bo->vm = vm;
>
> @@ -2503,8 +2504,11 @@ bool xe_bo_needs_ccs_pages(struct xe_bo *bo)
> * system memory (i.e., it allows XE_PL_TT placement),
> FlatCCS
> * can't be used since there's no CCS storage associated
> with
> * non-VRAM addresses.
> + *
> + * XXX: Can we support CCS with CPU address mirroring?
> */
> - if (IS_DGFX(xe) && (bo->flags & XE_BO_FLAG_SYSTEM))
> + if (IS_DGFX(xe) && ((bo->flags & XE_BO_FLAG_SYSTEM) ||
> + (bo->flags &
> XE_BO_FLAG_CPU_ADDR_MIRROR)))
> return false;
>
> return true;
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index ce55a2bb13f6..c01ed535a8c3 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -47,6 +47,7 @@
> XE_BO_FLAG_GGTT1 | \
> XE_BO_FLAG_GGTT2 | \
> XE_BO_FLAG_GGTT3)
> +#define XE_BO_FLAG_CPU_ADDR_MIRROR BIT(22)
>
> /* this one is trigger internally only */
> #define XE_BO_FLAG_INTERNAL_TEST BIT(30)
