Re: [PATCH v5 00/32] Introduce GPU SVM and Xe SVM implementation

[Demi Marie Obenour <demi@invisiblethingslab.com>]

[-- Attachment #1: Type: text/plain, Size: 6712 bytes --]

On Wed, Feb 12, 2025 at 06:10:40PM -0800, Matthew Brost wrote:
> Version 5 of GPU SVM. Thanks to everyone (especially Sima, Thomas,
> Alistair, Himal) for their numerous reviews on revision 1, 2, 3  and for
> helping to address many design issues.
> 
> This version has been tested with IGT [1] on PVC, BMG, and LNL. Also
> tested with level0 (UMD) PR [2].

What is the plan to deal with not being able to preempt while a page
fault is pending?  This seems like an easy DoS vector.  My understanding
is that SVM is mostly used by compute workloads on headless systems.
Recent AMD client GPUs don't support SVM, so programs that want to run
on client systems should not require SVM if they wish to be portable.

Given the potential for abuse, I think it would be best to require
explicit administrator opt-in to enable SVM, along with possibly having
a timeout to resolve a page fault (after which the context is killed).
Since I expect most uses of SVM to be in the datacenter space (for the
reasons mentioned above), I don't believe this will be a major
limitation in practice.  Programs that wish to run on client systems
already need to use explicit memory transfer or pinned userptr, and
administrators of compute clusters should be willing to enable this
feature because only one workload will be using a GPU at a time.

> Major changes in v2:
> - Dropped mmap write abuse
> - core MM locking and retry loops instead of driver locking to avoid races
> - Removed physical to virtual references
> - Embedded structure/ops for drm_gpusvm_devmem
> - Fixed mremap and fork issues
> - Added DRM pagemap
> - Included RFC documentation in the kernel doc
> 
> Major changes in v3:
> - Move GPU SVM and DRM pagemap to DRM level
> - Mostly addresses Thomas's feedback, lots of small changes documented
>   in each individual patch change log
> 
> Major changes in v4:
> - Pull documentation patch in
> - Fix Kconfig / VRAM migration issue
> - Address feedback which came out of internal multi-GPU implementation
> 
> Major changes in v5:
> - Rebase on s/xe_mem_region/xe_vram_region
> - Bit for uAPI has changed given PXP has landed
> 
> Known issues in v5:
> - Check pages still exists, changed to threshold in this version which
>   is better but still need to root cause cross process page finding on
>   small user allocations.
> 
> Matt
> 
> [1] https://patchwork.freedesktop.org/series/137545/#rev3
> [2] https://github.com/intel/compute-runtime/pull/782
> 
> Matthew Brost (28):
>   drm/xe: Retry BO allocation
>   mm/migrate: Add migrate_device_pfns
>   mm/migrate: Trylock device page in do_swap_page
>   drm/gpusvm: Add support for GPU Shared Virtual Memory
>   drm/xe: Select DRM_GPUSVM Kconfig
>   drm/xe/uapi: Add DRM_XE_VM_BIND_FLAG_CPU_ADDR_MIRROR
>   drm/xe: Add SVM init / close / fini to faulting VMs
>   drm/xe: Nuke VM's mapping upon close
>   drm/xe: Add SVM range invalidation and page fault
>   drm/gpuvm: Add DRM_GPUVA_OP_DRIVER
>   drm/xe: Add (re)bind to SVM page fault handler
>   drm/xe: Add SVM garbage collector
>   drm/xe: Add unbind to SVM garbage collector
>   drm/xe: Do not allow CPU address mirror VMA unbind if the GPU has
>     bindings
>   drm/xe: Enable CPU address mirror uAPI
>   drm/xe/uapi: Add DRM_XE_QUERY_CONFIG_FLAG_HAS_CPU_ADDR_MIRROR
>   drm/xe: Add migrate layer functions for SVM support
>   drm/xe: Add SVM device memory mirroring
>   drm/xe: Add drm_gpusvm_devmem to xe_bo
>   drm/xe: Add GPUSVM device memory copy vfunc functions
>   drm/xe: Add Xe SVM populate_devmem_pfn GPU SVM vfunc
>   drm/xe: Add Xe SVM devmem_release GPU SVM vfunc
>   drm/xe: Add SVM VRAM migration
>   drm/xe: Basic SVM BO eviction
>   drm/xe: Add SVM debug
>   drm/xe: Add modparam for SVM notifier size
>   drm/xe: Add always_migrate_to_vram modparam
>   drm/doc: gpusvm: Add GPU SVM documentation
> 
> Thomas Hellström (4):
>   drm/pagemap: Add DRM pagemap
>   drm/xe/bo: Introduce xe_bo_put_async
>   drm/xe: Add dma_addr res cursor
>   drm/xe: Add drm_pagemap ops to SVM
> 
>  Documentation/gpu/rfc/gpusvm.rst            |   84 +
>  Documentation/gpu/rfc/index.rst             |    4 +
>  drivers/gpu/drm/Kconfig                     |    9 +
>  drivers/gpu/drm/Makefile                    |    1 +
>  drivers/gpu/drm/drm_gpusvm.c                | 2230 +++++++++++++++++++
>  drivers/gpu/drm/xe/Kconfig                  |   10 +
>  drivers/gpu/drm/xe/Makefile                 |    1 +
>  drivers/gpu/drm/xe/xe_bo.c                  |   54 +
>  drivers/gpu/drm/xe/xe_bo.h                  |   20 +
>  drivers/gpu/drm/xe/xe_bo_types.h            |    4 +
>  drivers/gpu/drm/xe/xe_device.c              |    3 +
>  drivers/gpu/drm/xe/xe_device_types.h        |   22 +
>  drivers/gpu/drm/xe/xe_gt_pagefault.c        |   18 +-
>  drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c |   22 +
>  drivers/gpu/drm/xe/xe_gt_tlb_invalidation.h |    2 +
>  drivers/gpu/drm/xe/xe_migrate.c             |  175 ++
>  drivers/gpu/drm/xe/xe_migrate.h             |   10 +
>  drivers/gpu/drm/xe/xe_module.c              |    7 +
>  drivers/gpu/drm/xe/xe_module.h              |    2 +
>  drivers/gpu/drm/xe/xe_pt.c                  |  393 +++-
>  drivers/gpu/drm/xe/xe_pt.h                  |    5 +
>  drivers/gpu/drm/xe/xe_pt_types.h            |    2 +
>  drivers/gpu/drm/xe/xe_query.c               |    5 +-
>  drivers/gpu/drm/xe/xe_res_cursor.h          |  116 +-
>  drivers/gpu/drm/xe/xe_svm.c                 |  964 ++++++++
>  drivers/gpu/drm/xe/xe_svm.h                 |   96 +
>  drivers/gpu/drm/xe/xe_tile.c                |    5 +
>  drivers/gpu/drm/xe/xe_vm.c                  |  374 +++-
>  drivers/gpu/drm/xe/xe_vm.h                  |   15 +-
>  drivers/gpu/drm/xe/xe_vm_types.h            |   57 +
>  include/drm/drm_gpusvm.h                    |  507 +++++
>  include/drm/drm_gpuvm.h                     |    5 +
>  include/drm/drm_pagemap.h                   |  105 +
>  include/linux/migrate.h                     |    1 +
>  include/uapi/drm/xe_drm.h                   |   22 +-
>  mm/memory.c                                 |   13 +-
>  mm/migrate_device.c                         |  116 +-
>  37 files changed, 5326 insertions(+), 153 deletions(-)
>  create mode 100644 Documentation/gpu/rfc/gpusvm.rst
>  create mode 100644 drivers/gpu/drm/drm_gpusvm.c
>  create mode 100644 drivers/gpu/drm/xe/xe_svm.c
>  create mode 100644 drivers/gpu/drm/xe/xe_svm.h
>  create mode 100644 include/drm/drm_gpusvm.h
>  create mode 100644 include/drm/drm_pagemap.h
> 
> -- 
> 2.34.1
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]