Re: [v2 13/31] drm/xe/svm: Introduce DRM_XE_SVM kernel config

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Apr 09, 2024 at 04:17:24PM -0400, Oak Zeng wrote:
> Introduce a DRM_XE_SVM kernel config entry for

Maybe consider another name for this? I could see use cases for non-SVM
where we still want private pages mapped (e.g. VRAM userptrs on
non-faulting devices). Don't really have suggestion but worth
considering.

> xe svm feature. xe svm feature allows share
> virtual address space between CPU and GPU program.
> 
> v1: Improve commit message (Thomas)
>     Avoid using #if directive (Thomas)
> 
> Signed-off-by: Oak Zeng <oak.zeng@intel.com>
> Co-developed-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@intel.com>
> Cc: Brian Welty <brian.welty@intel.com>
> ---
>  drivers/gpu/drm/xe/Kconfig   | 21 +++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_tile.c |  7 +++++--
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
> index 449a1ecbc92a..0accb2cb81d6 100644
> --- a/drivers/gpu/drm/xe/Kconfig
> +++ b/drivers/gpu/drm/xe/Kconfig
> @@ -84,6 +84,27 @@ config DRM_XE_FORCE_PROBE
>  	  4571.
>  
>  	  Use "!*" to block the probe of the driver for all known devices.
> +config DRM_XE_SVM
> +	bool "Enable Shared Virtual Memory support in xe"
> +	depends on DRM_XE
> +	depends on ARCH_ENABLE_MEMORY_HOTPLUG
> +	depends on ARCH_ENABLE_MEMORY_HOTREMOVE
> +	depends on MEMORY_HOTPLUG
> +	depends on MEMORY_HOTREMOVE
> +	depends on ARCH_HAS_PTE_DEVMAP
> +	depends on SPARSEMEM_VMEMMAP
> +	depends on ZONE_DEVICE
> +	depends on DEVICE_PRIVATE
> +	depends on MMU
> +	select HMM_MIRROR
> +	select MMU_NOTIFIER
> +	default y
> +	help
> +	  Choose this option if you want Shared Virtual Memory (SVM)
> +	  support in xe. With SVM, virtual address space is shared
> +	  between CPU and GPU. This means any virtual address such
> +	  as malloc or mmap returns, variables on stack, or global
> +	  memory pointers, can be used for GPU transparently.
>  
>  menu "drm/Xe Debugging"
>  depends on DRM_XE
> diff --git a/drivers/gpu/drm/xe/xe_tile.c b/drivers/gpu/drm/xe/xe_tile.c
> index f1c4f9de51df..a1a436912fe3 100644
> --- a/drivers/gpu/drm/xe/xe_tile.c
> +++ b/drivers/gpu/drm/xe/xe_tile.c
> @@ -159,9 +159,12 @@ static int tile_ttm_mgr_init(struct xe_tile *tile)
>   */
>  int xe_tile_init_noalloc(struct xe_tile *tile)
>  {
> -	struct xe_device *xe = tile_to_xe(tile);
> +	struct xe_device __maybe_unused *xe;

Just assign this here blindly? The __maybe_unused should suppress the
warning CONFIG_DRM_XE_SVM is false and should just compile out if it is.

Matt 

>  	int err;
>  
> +	if (IS_ENABLED(CONFIG_DRM_XE_SVM))
> +		xe = tile_to_xe(tile);
> +
>  	xe_device_mem_access_get(tile_to_xe(tile));
>  
>  	err = tile_ttm_mgr_init(tile);
> @@ -177,7 +180,7 @@ int xe_tile_init_noalloc(struct xe_tile *tile)
>  
>  	xe_tile_sysfs_init(tile);
>  
> -	if (xe->info.has_usm)
> +	if (IS_ENABLED(CONFIG_DRM_XE_SVM) && xe->info.has_usm)
>  		xe_devm_add(tile, &tile->mem.vram);
>  err_mem_access:
>  	xe_device_mem_access_put(tile_to_xe(tile));
> -- 
> 2.26.3
>