Re: [PATCH v6 04/26] drm/gpuvm: Introduce DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MADVISE flag

["Danilo Krummrich" <dakr@kernel.org>]

On Thu Aug 7, 2025 at 6:43 PM CEST, Himal Prasad Ghimiray wrote:
> - DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MADVISE: This flag is used by
>   drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the
> user-provided range and split the existing non-GEM object VMA if the

What do you mean with non-GEM object VMA? I assume you just mean sparse
mappings?

> start or end of the input range lies within it. The operations can
> create up to 2 REMAPS and 2 MAPs.

Wait -- that doesn't make sense with what I thought how this works.

In the RFC you gave the following example:

	Example:
	Input Range: 0x00007f0a54000000 to 0x00007f0a54400000
	GPU VMA: 0x0000000000000000 to 0x0000800000000000
	Operations Result:
	- REMAP:UNMAP: addr=0x0000000000000000, range=0x0000800000000000
	- REMAP:PREV: addr=0x0000000000000000, range=0x00007f0a54000000
	- REMAP:NEXT: addr=0x00007f0a54400000, range=0x000000f5abc00000
	- MAP: addr=0x00007f0a54000000, range=0x0000000000400000

That's exactly the same what the existing logic does. So in which case do you
have *two* MAP operations?

For completeness, the other example you gave was:

	Example:
	Input Range: 0x00007fc898800000 to 0x00007fc899000000
	GPU VMAs:
	- 0x0000000000000000 to 0x00007fc898800000
	- 0x00007fc898800000 to 0x00007fc898a00000
	- 0x00007fc898a00000 to 0x00007fc898c00000
	- 0x00007fc898c00000 to 0x00007fc899000000
	- 0x00007fc899000000 to 0x00007fc899200000
	Operations Result: None

This just means, if things are split already, at the defined edges, just don't
do anything, which is also conform with the existing logic except for the "no
merge" part, which is obviously fine given that it's explicitly for splitting
things.

Can you please provide some additional *simple* examples, like the documentation
of GPUVM does today for the normal split/merge stuff? I.e. please don't use
complex real addresses, that makes it hard to parse.

Also, can you please provide some information on what this whole thing does
*semantically*? I thought I understood it, but now I'm not so sure anymore.

> The purpose of this operation is to be
> used by the Xe driver to assign attributes to GPUVMA's within the
> user-defined range.

Well, hopefully it's useful to other drivers as well. :)

> Unlike drm_gpuvm_sm_map_ops_flags in default mode,
> the operation with this flag will never have UNMAPs and
> merges, and can be without any final operations.

I really think this is significant enough of a feature to add some proper
documentation about it.

Please add a separate section about madvise operations to the documentation at
the beginning of the drivers/gpu/drm/drm_gpuvm.c file.

>
> v2
> - use drm_gpuvm_sm_map_ops_create with flags instead of defining new
>   ops_create (Danilo)

If this turns out not to be what I thought semantically and we still agree it's
the correct approach, I think I have to take this back and it should indeed be
an entirely separate code path. But let's wait for your answers above.

Again, I really think this needs some proper documentation like in the
"DOC: Split and Merge" documentation section.