[PATCH 0/3] Enable device atomics with a VM bind flag

[Zeng, Oak]

> -----Original Message-----
> From: Nirmoy Das <nirmoy.das at linux.intel.com>
> Sent: Thursday, April 11, 2024 1:01 PM
> To: Zeng, Oak <oak.zeng at intel.com>; Das, Nirmoy <nirmoy.das at intel.com>;
> intel-xe at lists.freedesktop.org
> Cc: Vivekanandan, Balasubramani <balasubramani.vivekanandan at intel.com>;
> Welty, Brian <brian.welty at intel.com>; Yang, Fei <fei.yang at intel.com>;
> Landwerlin, Lionel G <lionel.g.landwerlin at intel.com>; Roper, Matthew D
> <matthew.d.roper at intel.com>; Brost, Matthew <matthew.brost at intel.com>;
> Mrozek, Michal <michal.mrozek at intel.com>; Thomas Hellstr_m
> <thomas.hellstrom at linux.intel.com>
> Subject: Re: [PATCH 0/3] Enable device atomics with a VM bind flag
> 
> Hi Oak,
> 
> On 4/11/2024 6:22 PM, Zeng, Oak wrote:
> >
> >> -----Original Message-----
> >> From: Das, Nirmoy <nirmoy.das at intel.com>
> >> Sent: Wednesday, April 10, 2024 1:03 PM
> >> To: intel-xe at lists.freedesktop.org
> >> Cc: Das, Nirmoy <nirmoy.das at intel.com>; Vivekanandan, Balasubramani
> >> <balasubramani.vivekanandan at intel.com>; Welty, Brian
> >> <brian.welty at intel.com>; Yang, Fei <fei.yang at intel.com>; Landwerlin,
> Lionel G
> >> <lionel.g.landwerlin at intel.com>; Roper, Matthew D
> >> <matthew.d.roper at intel.com>; Brost, Matthew
> <matthew.brost at intel.com>;
> >> Mrozek, Michal <michal.mrozek at intel.com>; Zeng, Oak
> <oak.zeng at intel.com>;
> >> Thomas Hellstr_m <thomas.hellstrom at linux.intel.com>
> >> Subject: [PATCH 0/3] Enable device atomics with a VM bind flag
> >>
> >> Currently device atomics in SMEM only buffer is not supported and
> >> given that simultaneous usage of device atomics and CPU atomics on
> >> the same SMEM buffer is not guaranteed to function without migration,
> >> and UMD expects no migration for SMEM-only buffer objects, so this
> >> provide a way to set device atomics when UMD is certain to use the
> >> buffer only for device atomics.
> > If I understand it correctly, per umd-kmd contract, kmd should assume all
> shared allocations support system-wide atomic by default
> 
> Do we have any reference to this ? I wish this is documented somewhere.

I don't know such document either... I had that impression when I discussed with UMD previously


> 
> For Level0, the system-wide atomics is default expectation only when
> there is KMD migration and on buffer with SMEM+LMEM placements. This is
> my understanding
> 
> after long discussion that I had with Michal, Matt, and Lionel.
> 
> This patch is needed because a vulkan application can always try to do
> cpu atomics  on SMEM only BO and we can't allow device atomics on that
> by default.
> 
> >   (i.e. even if there is not any user settings). So if we want to introduce API to
> let user to say "only device atomic is expected on this allocation", the API would
> be "No system-wide atomics on this allocation".
> I think the flag name suites well with existing
> "ZE_MEMORY_ATOMIC_ATTR_EXP_FLAG_DEVICE_ATOMICS".

Yah, your flag matches above L0 definition. 

What is not clear to me is, what is the default behavior of an allocation? If the default behavior is system-wide atomics (means simultaneous host and device atomic operations are possible), then what is the point of SET_DEVICE_ATOMIC... 

I think the uAPI format depends on the default behavior:
	If default is system-wide atomic, then uAPI should be NO_SYSTEM_ATOMIC, NO_DEVICE_ATOMIC...
	If default is no atomic, then uAPI should be SET_DEVICE_ATOMIC, SET_SYSTEM_ATOMIC...

@Mrozek, Michal can you comment?

Oak


> >
> > Similarly, we can introduce API to let user say:
> > 	there is no atomics expected to this allocation
> > 	no device atomic or
> > 	no host atomic
> 
> We can add those once we have such requirement from UMD.
> 
> 
> Regards,
> 
> Nirmoy
> 
> >
> > But the above assumption (all allocation by default support system-wide
> atomics) doesn't work for device or host allocations on some platform, i.e., as
> explained in the commit message...
> >
> > So the API format is still an open to me. Should we define thing similar to
> umd api here: https://spec.oneapi.io/level-zero/latest/core/api.html#ze-
> memory-atomic-attr-exp-flags-t
> >
> > Oak
> >
> >
> >> Test-with: 20240410170041.24963-1-nirmoy.das at intel.com
> >> Cc: Balasubramani Vivekanandan <balasubramani.vivekanandan at intel.com>
> >> Cc: Brian Welty <brian.welty at intel.com>
> >> Cc: Fei Yang <fei.yang at intel.com>
> >> Cc: Lionel G Landwerlin <lionel.g.landwerlin at intel.com>
> >> Cc: Matt Roper <matthew.d.roper at intel.com>
> >> Cc: Matthew Brost <matthew.brost at intel.com>
> >> Cc: Michal Mrozek <michal.mrozek at intel.com>
> >> Cc: Oak Zeng <oak.zeng at intel.com>
> >> Cc: Thomas Hellstr_m <thomas.hellstrom at linux.intel.com>
> >>
> >> Nirmoy Das (3):
> >>    drm/xe: Consolidate setting PTE_AE into one place
> >>    drm/xe: Add function to check if BO has single placement
> >>    drm/xe/uapi: Introduce VMA bind flag for device atomics
> >>
> >>   drivers/gpu/drm/xe/xe_bo.c       | 14 ++++++++++++++
> >>   drivers/gpu/drm/xe/xe_bo.h       |  1 +
> >>   drivers/gpu/drm/xe/xe_pt.c       |  4 +---
> >>   drivers/gpu/drm/xe/xe_vm.c       | 32 ++++++++++++++++++++++++++++---
> -
> >>   drivers/gpu/drm/xe/xe_vm_types.h |  2 ++
> >>   include/uapi/drm/xe_drm.h        |  9 +++++----
> >>   6 files changed, 51 insertions(+), 11 deletions(-)
> >>
> >> --
> >> 2.42.0