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

[Ghimiray, Himal Prasad]

On 09-08-2025 18:53, Danilo Krummrich wrote:
> 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?

True>
>> 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?

Possible scenarios for ops functionality based on input start and end 
address from user:

a) User-provided range is a subset of an existing drm_gpuva
Expected Result: Same behavior as the default sm_map logic.
Reference: Case 1 from [1].

b) Either start or end (but not both) is not aligned with a drm_gpuva 
boundary
Expected Result: One REMAP and one MAP operation.
Reference: Case 3 from [1].

Existing GPUVMAs:

          drm_gpuva1                        drm_gpuva2
	[a----------------------------b-1][b-------------------c-1]

User Input to ops:
   start = inside drm_gpuva1
   end   = exactly at c-1 (end of drm_gpuva2)

Resulting Mapping:
	drm_gpuva1:pre       drm_gpuva:New map     drm_gpuva2
	[a---------start-1][start------- b-1] [b------------c-1]

Ops Created:
   REMAP:UNMAP drm_gpuva1 a to b
   REMAP:PREV a to start - 1
   MAP: start to b-1

Note: No unmap of drm_gpuvma2 and no merging of New map and drm_gpuva2.

c) Both start and end are not aligned with drm_gpuva boundaries, and 
they fall within different drm_gpuva regions
Expected Result: Two REMAP operations and two MAP operations.
Reference: Case 2 from [1].


d) User-provided range does not overlap with any existing drm_gpuva
Expected Result: No operations.
start and end exactly match the boundaries of one or more existing 
drm_gpuva regions

e) This includes cases where start is at the beginning of drm_gpuva1 and 
end is at the end of drm_gpuva2 (drm_gpuva1 and drm_gpuva2 can be same 
or different).
Expected Result: No operations

[1] 
https://lore.kernel.org/intel-xe/4203f450-4b49-401d-81a8-cdcca02035f9@intel.com/ 


> 
> 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.
> 

I’ve tried to explain the behavior/usecase with madvise and expected 
outcomes of the ops logic in detail in [1]. Could you please take a 
moment to review that and let me know if the explanation is sufficient 
or if any part needs further clarification?

>> 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. :)

It should be. :)

> 
>> 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.

Sure will do that.

> 
>>
>> 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.

Sure