On 12-08-2025 21:36, Danilo Krummrich wrote:
On Mon Aug 11, 2025 at 8:52 AM CEST, Himal Prasad Ghimiray wrote:
On 09-08-2025 18:53, Danilo Krummrich wrote:
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-cdcca0203...@intel.com/
<snip>
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?
Thanks a lot for writing this up!
I think this clarifies everything, the examples from [1] are good (sorry that
your reply from the RFC got lost somehow on my end).
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.
Great, this will help users (as well as reviewers) a lot. Please also add your
examples from [1] to this entry, similar to the existing examples for sm_map.
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.
Having the correct understanding of how this is supposed to work (and seeing how
the code turns out) I think it's still OK to integrate it into sm_map().
However, it probably makes sense to factor out the code into a common function
and then build the madvise() and sm_map() functions on top of it.
__drm_gpuvm_sm_map is that common function, and does
drm_gpuvm_madvise_ops_create sound OK? With separate functions for
sm_map and madvise, I see there's no need to add a flag to
drm_gpuvm_map_req at this moment. I will drop [1] in the next version.
[1] https://patchwork.freedesktop.org/patch/667561/?series=149550&rev=6
Thanks
Please also find some more comments on the patch itself.
Again, I really think this needs some proper documentation like in the
"DOC: Split and Merge" documentation section.
Sure
Thanks!
