Re: [Intel-xe] [RFC PATCH] drm/xe/uAPi: Clarify the meaning of min_page_size and MIN_ALIGNMENT

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Oct 13, 2023 at 12:22:26PM +0200, Thomas Hellström wrote:
> In particular, don't *enforce* alignment of buffer object sizes to
> MIN_ALIGNMENT, as enforcing that for imported dma-bufs would not
> really make sense.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Francois Dugast <francois.dugast@intel.com>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  include/uapi/drm/xe_drm.h | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index d48d8e3c898c..5895c95879d1 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -167,11 +167,10 @@ struct drm_xe_query_mem_region {
>  	 *
>  	 * When the kernel allocates memory for this region, the
>  	 * underlying pages will be at least @min_page_size in size.
> -	 *
> -	 * Important note: When userspace allocates a GTT address which
> -	 * can point to memory allocated from this region, it must also
> -	 * respect this minimum alignment. This is enforced by the
> -	 * kernel.

This old statement seems bold on the kernel is enforcing it. If we are
but intend to not enforce anymore we should also change the code in
the same patch, no?!
Also by the commit message I had the impression that this was removing
the enforcement, and not just the documentation about it.

> +	 * Buffer objects with an allowable placement in this region may
> +	 * be created with a smaller size, but for increased performance,
> +	 * user-space drivers should align sizes of such buffer objects
> +	 * to this value whenever possible.
>  	 */
>  	__u32 min_page_size;
>  	/**
> @@ -252,6 +251,14 @@ struct drm_xe_query_config {
>  #define XE_QUERY_CONFIG_REV_AND_DEVICE_ID	0
>  #define XE_QUERY_CONFIG_FLAGS			1
>  	#define XE_QUERY_CONFIG_FLAGS_HAS_VRAM		(0x1 << 0)
> +/*
> + * XE_QUERY_CONFIG_MIN_ALIGNMENT - GPU VM bind address - and size alignment.
> + * GPU virtual address mappings of buffer objects must have their VM addr and
> + * range aligned to this value, with the exception of buffer objects with
> + * allowable placement in system memory only. If this alignment results in
> + * a mapping beyond the end of the buffer object, accesses beyond the
> + * end of the buffer object have undefined reslut.

s/reslut/result.

Should we make this into a kernel doc?

> + */
>  #define XE_QUERY_CONFIG_MIN_ALIGNMENT		2
>  #define XE_QUERY_CONFIG_VA_BITS			3
>  #define XE_QUERY_CONFIG_GT_COUNT		4
> -- 
> 2.41.0
>