Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Am 2021-04-14 um 3:44 a.m. schrieb Thomas Zimmermann: > Hi > > Am 07.04.21 um 21:49 schrieb Felix Kuehling: >> On 2021-04-07 3:34 p.m., Felix Kuehling wrote: >>> On 2021-04-07 7:25 a.m., Christian König wrote: + /* + * Don't verify access for KFD BOs. They > don't have a GEM + * object associated with them. + */ + if (bo->kfd_bo) + goto out; >>> Who does the access verification now? >> This is somewhat confusing. >> >> I took this check as-is, including the comment, from amdgpu's >> verify_access function. The verify_access function was called by >> ttm_bo_mmap. It returned 0 and ttm_bo_mmap did the mapping. > This is probably a left-over from when we mapped BOs using /dev/kfd. > We > changed this to use /dev/dri/renderD* a long time ago to fix CPU > mapping > invalidations on memory evictions. I think we can let GEM do the > access > check. Ok, good to know. Thomas can you remove the extra handling in a separate prerequisite patch? If anybody then bisects to this patch we at least know what to do to get it working again. >>> >>> FWIW, I ran KFDTest test with this shortcut removed on current >>> amd-staging-drm-next + my HMM patch series, and it didn't seem to >>> cause any issues. >> >> Wait, I celebrated too soon. I was running the wrong kernel. I do see >> some failures where access is being denied. I need to do more debugging > >> to figure out what's causing that. > > Any news here? I saw the patch at [1], which removes the kfd_bo test. > Can I assume that the series addresses the issue? Yes, that series fixes the problem. I need to pester more people to review it. Regards, Felix > > Best regards > Thomas > > [1] https://patchwork.freedesktop.org/patch/427516/?series=88822=1 > >> >> Regards, >> Felix >> >> >>> >>> Regards, >>> Felix >>> >>> Regards, Christian. >> ___ >> dri-devel mailing list >> dri-de...@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Hi Am 07.04.21 um 21:49 schrieb Felix Kuehling: On 2021-04-07 3:34 p.m., Felix Kuehling wrote: On 2021-04-07 7:25 a.m., Christian König wrote: + /* + * Don't verify access for KFD BOs. They don't have a GEM + * object associated with them. + */ + if (bo->kfd_bo) + goto out; Who does the access verification now? This is somewhat confusing. I took this check as-is, including the comment, from amdgpu's verify_access function. The verify_access function was called by ttm_bo_mmap. It returned 0 and ttm_bo_mmap did the mapping. This is probably a left-over from when we mapped BOs using /dev/kfd. We changed this to use /dev/dri/renderD* a long time ago to fix CPU mapping invalidations on memory evictions. I think we can let GEM do the access check. Ok, good to know. Thomas can you remove the extra handling in a separate prerequisite patch? If anybody then bisects to this patch we at least know what to do to get it working again. FWIW, I ran KFDTest test with this shortcut removed on current amd-staging-drm-next + my HMM patch series, and it didn't seem to cause any issues. Wait, I celebrated too soon. I was running the wrong kernel. I do see some failures where access is being denied. I need to do more debugging to figure out what's causing that. Any news here? I saw the patch at [1], which removes the kfd_bo test. Can I assume that the series addresses the issue? Best regards Thomas [1] https://patchwork.freedesktop.org/patch/427516/?series=88822=1 Regards, Felix Regards, Felix Regards, Christian. ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Hi Am 07.04.21 um 21:49 schrieb Felix Kuehling: On 2021-04-07 3:34 p.m., Felix Kuehling wrote: On 2021-04-07 7:25 a.m., Christian König wrote: + /* + * Don't verify access for KFD BOs. They don't have a GEM + * object associated with them. + */ + if (bo->kfd_bo) + goto out; Who does the access verification now? This is somewhat confusing. I took this check as-is, including the comment, from amdgpu's verify_access function. The verify_access function was called by ttm_bo_mmap. It returned 0 and ttm_bo_mmap did the mapping. This is probably a left-over from when we mapped BOs using /dev/kfd. We changed this to use /dev/dri/renderD* a long time ago to fix CPU mapping invalidations on memory evictions. I think we can let GEM do the access check. Ok, good to know. Thomas can you remove the extra handling in a separate prerequisite patch? If anybody then bisects to this patch we at least know what to do to get it working again. FWIW, I ran KFDTest test with this shortcut removed on current amd-staging-drm-next + my HMM patch series, and it didn't seem to cause any issues. Wait, I celebrated too soon. I was running the wrong kernel. I do see some failures where access is being denied. I need to do more debugging to figure out what's causing that. OK, thanks for looking into this. I'll wait a bit before sending out the new patchset. Best regards Thomas Regards, Felix Regards, Felix Regards, Christian. -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
On 2021-04-07 3:34 p.m., Felix Kuehling wrote: On 2021-04-07 7:25 a.m., Christian König wrote: + /* + * Don't verify access for KFD BOs. They don't have a GEM + * object associated with them. + */ + if (bo->kfd_bo) + goto out; Who does the access verification now? This is somewhat confusing. I took this check as-is, including the comment, from amdgpu's verify_access function. The verify_access function was called by ttm_bo_mmap. It returned 0 and ttm_bo_mmap did the mapping. This is probably a left-over from when we mapped BOs using /dev/kfd. We changed this to use /dev/dri/renderD* a long time ago to fix CPU mapping invalidations on memory evictions. I think we can let GEM do the access check. Ok, good to know. Thomas can you remove the extra handling in a separate prerequisite patch? If anybody then bisects to this patch we at least know what to do to get it working again. FWIW, I ran KFDTest test with this shortcut removed on current amd-staging-drm-next + my HMM patch series, and it didn't seem to cause any issues. Wait, I celebrated too soon. I was running the wrong kernel. I do see some failures where access is being denied. I need to do more debugging to figure out what's causing that. Regards, Felix Regards, Felix Regards, Christian. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
On 2021-04-07 7:25 a.m., Christian König wrote: + /* + * Don't verify access for KFD BOs. They don't have a GEM + * object associated with them. + */ + if (bo->kfd_bo) + goto out; Who does the access verification now? This is somewhat confusing. I took this check as-is, including the comment, from amdgpu's verify_access function. The verify_access function was called by ttm_bo_mmap. It returned 0 and ttm_bo_mmap did the mapping. This is probably a left-over from when we mapped BOs using /dev/kfd. We changed this to use /dev/dri/renderD* a long time ago to fix CPU mapping invalidations on memory evictions. I think we can let GEM do the access check. Ok, good to know. Thomas can you remove the extra handling in a separate prerequisite patch? If anybody then bisects to this patch we at least know what to do to get it working again. FWIW, I ran KFDTest test with this shortcut removed on current amd-staging-drm-next + my HMM patch series, and it didn't seem to cause any issues. Regards, Felix Regards, Christian. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Hi,
Am 06.04.21 um 17:27 schrieb Felix Kuehling:
Am 2021-04-06 um 6:38 a.m. schrieb Thomas Zimmermann:
Hi
Am 06.04.21 um 11:35 schrieb Christian König:
Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.
This change resolves several inconsistencies between regular mmap and
prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
areas. Previously it way only set for regular mmap calls, prime-based
mmap used TTM's default vm_ops. The check for kfd_bo has been taken
from amdgpu_verify_access(), which is not called any longer and has
been removed.
As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71
-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 -
6 files changed, 66 insertions(+), 122 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index e0c4f7c7f1b9..19c5ab08d9ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -42,52 +42,6 @@
#include
#include
-/**
- * amdgpu_gem_prime_mmap - _driver.gem_prime_mmap implementation
- * @obj: GEM BO
- * @vma: Virtual memory area
- *
- * Sets up a userspace mapping of the BO's memory in the given
- * virtual memory area.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- */
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma)
-{
- struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
- struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
- unsigned asize = amdgpu_bo_size(bo);
- int ret;
-
- if (!vma->vm_file)
- return -ENODEV;
-
- if (adev == NULL)
- return -ENODEV;
-
- /* Check for valid size. */
- if (asize < vma->vm_end - vma->vm_start)
- return -EINVAL;
-
- if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
- (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
- return -EPERM;
- }
- vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
-
- /* prime mmap does not need to check access, so allow here */
- ret = drm_vma_node_allow(>vma_node,
vma->vm_file->private_data);
- if (ret)
- return ret;
-
- ret = ttm_bo_mmap(vma->vm_file, vma, >mman.bdev);
- drm_vma_node_revoke(>vma_node, vma->vm_file->private_data);
-
- return ret;
-}
-
static int
__dma_resv_make_exclusive(struct dma_resv *obj)
{
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 39b5b9616fd8..3e93b9b407a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,6 @@ struct drm_gem_object
*amdgpu_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf);
bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
struct amdgpu_bo *bo);
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma);
extern const struct dma_buf_ops amdgpu_dmabuf_ops;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 76f48f79c70b..e96d2758f4bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1656,7 +1656,7 @@ static const struct file_operations
amdgpu_driver_kms_fops = {
.flush = amdgpu_flush,
.release = drm_release,
.unlocked_ioctl = amdgpu_drm_ioctl,
- .mmap = amdgpu_mmap,
+ .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
#ifdef CONFIG_COMPAT
@@ -1719,7 +1719,7 @@ static const struct drm_driver
amdgpu_kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import = amdgpu_gem_prime_import,
- .gem_prime_mmap = amdgpu_gem_prime_mmap,
+ .gem_prime_mmap = drm_gem_prime_mmap,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index fb7171e5507c..fe93faad05f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -41,6 +41,36 @@
static const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
+static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
Please name that function amdgpu_gem_fault or amdgpu_gem_object_fault
+{
+ struct ttm_buffer_object *bo =
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Am 2021-04-06 um 6:38 a.m. schrieb Thomas Zimmermann:
> Hi
>
> Am 06.04.21 um 11:35 schrieb Christian König:
>> Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:
>>> Moving the driver-specific mmap code into a GEM object function allows
>>> for using DRM helpers for various mmap callbacks.
>>>
>>> This change resolves several inconsistencies between regular mmap and
>>> prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
>>> areas. Previously it way only set for regular mmap calls, prime-based
>>> mmap used TTM's default vm_ops. The check for kfd_bo has been taken
>>> from amdgpu_verify_access(), which is not called any longer and has
>>> been removed.
>>>
>>> As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
>>> implemented in amdgpu's GEM code.
>>>
>>> Signed-off-by: Thomas Zimmermann
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 2 -
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +-
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71
>>> -
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 -
>>> 6 files changed, 66 insertions(+), 122 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> index e0c4f7c7f1b9..19c5ab08d9ec 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> @@ -42,52 +42,6 @@
>>> #include
>>> #include
>>> -/**
>>> - * amdgpu_gem_prime_mmap - _driver.gem_prime_mmap implementation
>>> - * @obj: GEM BO
>>> - * @vma: Virtual memory area
>>> - *
>>> - * Sets up a userspace mapping of the BO's memory in the given
>>> - * virtual memory area.
>>> - *
>>> - * Returns:
>>> - * 0 on success or a negative error code on failure.
>>> - */
>>> -int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
>>> - struct vm_area_struct *vma)
>>> -{
>>> - struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>>> - struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>> - unsigned asize = amdgpu_bo_size(bo);
>>> - int ret;
>>> -
>>> - if (!vma->vm_file)
>>> - return -ENODEV;
>>> -
>>> - if (adev == NULL)
>>> - return -ENODEV;
>>> -
>>> - /* Check for valid size. */
>>> - if (asize < vma->vm_end - vma->vm_start)
>>> - return -EINVAL;
>>> -
>>> - if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
>>> - (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
>>> - return -EPERM;
>>> - }
>>> - vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
>>> -
>>> - /* prime mmap does not need to check access, so allow here */
>>> - ret = drm_vma_node_allow(>vma_node,
>>> vma->vm_file->private_data);
>>> - if (ret)
>>> - return ret;
>>> -
>>> - ret = ttm_bo_mmap(vma->vm_file, vma, >mman.bdev);
>>> - drm_vma_node_revoke(>vma_node, vma->vm_file->private_data);
>>> -
>>> - return ret;
>>> -}
>>> -
>>> static int
>>> __dma_resv_make_exclusive(struct dma_resv *obj)
>>> {
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
>>> index 39b5b9616fd8..3e93b9b407a9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
>>> @@ -31,8 +31,6 @@ struct drm_gem_object
>>> *amdgpu_gem_prime_import(struct drm_device *dev,
>>> struct dma_buf *dma_buf);
>>> bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
>>> struct amdgpu_bo *bo);
>>> -int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
>>> - struct vm_area_struct *vma);
>>> extern const struct dma_buf_ops amdgpu_dmabuf_ops;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index 76f48f79c70b..e96d2758f4bb 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -1656,7 +1656,7 @@ static const struct file_operations
>>> amdgpu_driver_kms_fops = {
>>> .flush = amdgpu_flush,
>>> .release = drm_release,
>>> .unlocked_ioctl = amdgpu_drm_ioctl,
>>> - .mmap = amdgpu_mmap,
>>> + .mmap = drm_gem_mmap,
>>> .poll = drm_poll,
>>> .read = drm_read,
>>> #ifdef CONFIG_COMPAT
>>> @@ -1719,7 +1719,7 @@ static const struct drm_driver
>>> amdgpu_kms_driver = {
>>> .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>>> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>>> .gem_prime_import = amdgpu_gem_prime_import,
>>> - .gem_prime_mmap = amdgpu_gem_prime_mmap,
>>> + .gem_prime_mmap = drm_gem_prime_mmap,
>>> .name = DRIVER_NAME,
>>> .desc = DRIVER_DESC,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Am 2021-04-06 um 9:04 a.m. schrieb Christian König: > Am 06.04.21 um 14:52 schrieb Thomas Zimmermann: >> Hi >> >> Am 06.04.21 um 14:42 schrieb Christian König: >>> Hi Thomas, >>> >>> Am 06.04.21 um 13:55 schrieb Thomas Zimmermann: Hi Am 06.04.21 um 12:56 schrieb Christian König: >> >> In the end I went with the semantics I found in amdgpu_mmap() and >> handled KFD specially. Let me know if this requires to be changed. > > Well the question is where is the call to > drm_vma_node_verify_access() now? Cause that needs to be skipped > for KFD BOs. I see. It's now drm_vma_node_is_allowed(); called by drm_gem_mmap(). [1] So drm_gem_mmap() cannot be used by amdgpu. If I understand the code at [2] correctly, KFD objects don't use the GEM ioctl interfaces, but they still use the internal GEM object that is part of the TTM BO. In this case, amdgpu could have its own version of drm_gem_mmap(), which calls drm_gem_mmap_obj(), [3] which in turn handles the mmap details via GEM object functions. >>> >>> Correct, well we could cleanup the KFD to use the GEM functions as >>> well. >> >> The KFD code already calls amdgpu_gem_object_create(). It should have >> the object-functions pointer set for use with mmap. Not sure what the >> use of drm_vma_node_is_allowed() would involve. > > The KFD allows BOs to be mmaped with different offsets than what's > used in the DRM node. > > So drm_vma_node_is_allowed() would return false as far as I know. We used to mmap KFD BOs through the /dev/kfd file descriptor. We moved that to using the /dev/dri/renderD* file descriptors a long time ago. If there is some KFD special casing left in the code for BO mmap, it's probably an oversight and we should be able to remove it. We still have a few special mmaps in /dev/kfd, but they are for things that don't involve GEM BOs that could be mmapped through the render node: doorbells, MMIO pages and CWSR trap-handler mappings for APUs. Regards, Felix > > Regards, > Christian. > >> >> Best regards >> Thomas >> >>> >>> Felix what exactly was your objections to using this? >>> >>> Regards, >>> Christian. >>> drm_gem_prime_mmap() doesn't do any additional verification. Best regards Thomas [1] https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/drm_gem.c#L1156 [2] https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c#L1224 [3] https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/gpu/drm/drm_gem.c#L1053 > > Regards, > Christian. > >> >> Best regards >> Thomas >> - int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset, uint64_t dst_offset, uint32_t byte_count, struct dma_resv *resv, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index dec0db8b0b13..6e51faad7371 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo, struct dma_resv *resv, struct dma_fence **fence); -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma); int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo); int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo); uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type); >>> >>> ___ >>> dri-devel mailing list >>> dri-de...@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >>> >>> ___ >>> dri-devel mailing list >>> dri-de...@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> > ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Am 06.04.21 um 14:52 schrieb Thomas Zimmermann: Hi Am 06.04.21 um 14:42 schrieb Christian König: Hi Thomas, Am 06.04.21 um 13:55 schrieb Thomas Zimmermann: Hi Am 06.04.21 um 12:56 schrieb Christian König: In the end I went with the semantics I found in amdgpu_mmap() and handled KFD specially. Let me know if this requires to be changed. Well the question is where is the call to drm_vma_node_verify_access() now? Cause that needs to be skipped for KFD BOs. I see. It's now drm_vma_node_is_allowed(); called by drm_gem_mmap(). [1] So drm_gem_mmap() cannot be used by amdgpu. If I understand the code at [2] correctly, KFD objects don't use the GEM ioctl interfaces, but they still use the internal GEM object that is part of the TTM BO. In this case, amdgpu could have its own version of drm_gem_mmap(), which calls drm_gem_mmap_obj(), [3] which in turn handles the mmap details via GEM object functions. Correct, well we could cleanup the KFD to use the GEM functions as well. The KFD code already calls amdgpu_gem_object_create(). It should have the object-functions pointer set for use with mmap. Not sure what the use of drm_vma_node_is_allowed() would involve. The KFD allows BOs to be mmaped with different offsets than what's used in the DRM node. So drm_vma_node_is_allowed() would return false as far as I know. Regards, Christian. Best regards Thomas Felix what exactly was your objections to using this? Regards, Christian. drm_gem_prime_mmap() doesn't do any additional verification. Best regards Thomas [1] https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/drm_gem.c#L1156 [2] https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c#L1224 [3] https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/gpu/drm/drm_gem.c#L1053 Regards, Christian. Best regards Thomas - int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset, uint64_t dst_offset, uint32_t byte_count, struct dma_resv *resv, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index dec0db8b0b13..6e51faad7371 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo, struct dma_resv *resv, struct dma_fence **fence); -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma); int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo); int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo); uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type); ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Hi Am 06.04.21 um 14:42 schrieb Christian König: Hi Thomas, Am 06.04.21 um 13:55 schrieb Thomas Zimmermann: Hi Am 06.04.21 um 12:56 schrieb Christian König: In the end I went with the semantics I found in amdgpu_mmap() and handled KFD specially. Let me know if this requires to be changed. Well the question is where is the call to drm_vma_node_verify_access() now? Cause that needs to be skipped for KFD BOs. I see. It's now drm_vma_node_is_allowed(); called by drm_gem_mmap(). [1] So drm_gem_mmap() cannot be used by amdgpu. If I understand the code at [2] correctly, KFD objects don't use the GEM ioctl interfaces, but they still use the internal GEM object that is part of the TTM BO. In this case, amdgpu could have its own version of drm_gem_mmap(), which calls drm_gem_mmap_obj(), [3] which in turn handles the mmap details via GEM object functions. Correct, well we could cleanup the KFD to use the GEM functions as well. The KFD code already calls amdgpu_gem_object_create(). It should have the object-functions pointer set for use with mmap. Not sure what the use of drm_vma_node_is_allowed() would involve. Best regards Thomas Felix what exactly was your objections to using this? Regards, Christian. drm_gem_prime_mmap() doesn't do any additional verification. Best regards Thomas [1] https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/drm_gem.c#L1156 [2] https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c#L1224 [3] https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/gpu/drm/drm_gem.c#L1053 Regards, Christian. Best regards Thomas - int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset, uint64_t dst_offset, uint32_t byte_count, struct dma_resv *resv, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index dec0db8b0b13..6e51faad7371 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo, struct dma_resv *resv, struct dma_fence **fence); -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma); int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo); int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo); uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type); ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Hi Thomas, Am 06.04.21 um 13:55 schrieb Thomas Zimmermann: Hi Am 06.04.21 um 12:56 schrieb Christian König: In the end I went with the semantics I found in amdgpu_mmap() and handled KFD specially. Let me know if this requires to be changed. Well the question is where is the call to drm_vma_node_verify_access() now? Cause that needs to be skipped for KFD BOs. I see. It's now drm_vma_node_is_allowed(); called by drm_gem_mmap(). [1] So drm_gem_mmap() cannot be used by amdgpu. If I understand the code at [2] correctly, KFD objects don't use the GEM ioctl interfaces, but they still use the internal GEM object that is part of the TTM BO. In this case, amdgpu could have its own version of drm_gem_mmap(), which calls drm_gem_mmap_obj(), [3] which in turn handles the mmap details via GEM object functions. Correct, well we could cleanup the KFD to use the GEM functions as well. Felix what exactly was your objections to using this? Regards, Christian. drm_gem_prime_mmap() doesn't do any additional verification. Best regards Thomas [1] https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/drm_gem.c#L1156 [2] https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c#L1224 [3] https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/gpu/drm/drm_gem.c#L1053 Regards, Christian. Best regards Thomas - int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset, uint64_t dst_offset, uint32_t byte_count, struct dma_resv *resv, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index dec0db8b0b13..6e51faad7371 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo, struct dma_resv *resv, struct dma_fence **fence); -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma); int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo); int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo); uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type); ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Hi Am 06.04.21 um 12:56 schrieb Christian König: In the end I went with the semantics I found in amdgpu_mmap() and handled KFD specially. Let me know if this requires to be changed. Well the question is where is the call to drm_vma_node_verify_access() now? Cause that needs to be skipped for KFD BOs. I see. It's now drm_vma_node_is_allowed(); called by drm_gem_mmap(). [1] So drm_gem_mmap() cannot be used by amdgpu. If I understand the code at [2] correctly, KFD objects don't use the GEM ioctl interfaces, but they still use the internal GEM object that is part of the TTM BO. In this case, amdgpu could have its own version of drm_gem_mmap(), which calls drm_gem_mmap_obj(), [3] which in turn handles the mmap details via GEM object functions. drm_gem_prime_mmap() doesn't do any additional verification. Best regards Thomas [1] https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/drm_gem.c#L1156 [2] https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c#L1224 [3] https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/gpu/drm/drm_gem.c#L1053 Regards, Christian. Best regards Thomas - int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset, uint64_t dst_offset, uint32_t byte_count, struct dma_resv *resv, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index dec0db8b0b13..6e51faad7371 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo, struct dma_resv *resv, struct dma_fence **fence); -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma); int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo); int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo); uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type); ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_signature Description: OpenPGP digital signature ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Hi Thomas,
Am 06.04.21 um 12:38 schrieb Thomas Zimmermann:
Hi
Am 06.04.21 um 11:35 schrieb Christian König:
Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.
This change resolves several inconsistencies between regular mmap and
prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
areas. Previously it way only set for regular mmap calls, prime-based
mmap used TTM's default vm_ops. The check for kfd_bo has been taken
from amdgpu_verify_access(), which is not called any longer and has
been removed.
As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71
-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 -
6 files changed, 66 insertions(+), 122 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index e0c4f7c7f1b9..19c5ab08d9ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -42,52 +42,6 @@
#include
#include
-/**
- * amdgpu_gem_prime_mmap - _driver.gem_prime_mmap implementation
- * @obj: GEM BO
- * @vma: Virtual memory area
- *
- * Sets up a userspace mapping of the BO's memory in the given
- * virtual memory area.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- */
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma)
-{
- struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
- struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
- unsigned asize = amdgpu_bo_size(bo);
- int ret;
-
- if (!vma->vm_file)
- return -ENODEV;
-
- if (adev == NULL)
- return -ENODEV;
-
- /* Check for valid size. */
- if (asize < vma->vm_end - vma->vm_start)
- return -EINVAL;
-
- if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
- (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
- return -EPERM;
- }
- vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
-
- /* prime mmap does not need to check access, so allow here */
- ret = drm_vma_node_allow(>vma_node,
vma->vm_file->private_data);
- if (ret)
- return ret;
-
- ret = ttm_bo_mmap(vma->vm_file, vma, >mman.bdev);
- drm_vma_node_revoke(>vma_node, vma->vm_file->private_data);
-
- return ret;
-}
-
static int
__dma_resv_make_exclusive(struct dma_resv *obj)
{
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 39b5b9616fd8..3e93b9b407a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,6 @@ struct drm_gem_object
*amdgpu_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf);
bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
struct amdgpu_bo *bo);
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma);
extern const struct dma_buf_ops amdgpu_dmabuf_ops;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 76f48f79c70b..e96d2758f4bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1656,7 +1656,7 @@ static const struct file_operations
amdgpu_driver_kms_fops = {
.flush = amdgpu_flush,
.release = drm_release,
.unlocked_ioctl = amdgpu_drm_ioctl,
- .mmap = amdgpu_mmap,
+ .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
#ifdef CONFIG_COMPAT
@@ -1719,7 +1719,7 @@ static const struct drm_driver
amdgpu_kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import = amdgpu_gem_prime_import,
- .gem_prime_mmap = amdgpu_gem_prime_mmap,
+ .gem_prime_mmap = drm_gem_prime_mmap,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index fb7171e5507c..fe93faad05f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -41,6 +41,36 @@
static const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
+static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
Please name that function amdgpu_gem_fault or amdgpu_gem_object_fault
+{
+ struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+ vm_fault_t ret;
+
+ ret =
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Hi
Am 06.04.21 um 11:35 schrieb Christian König:
Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.
This change resolves several inconsistencies between regular mmap and
prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
areas. Previously it way only set for regular mmap calls, prime-based
mmap used TTM's default vm_ops. The check for kfd_bo has been taken
from amdgpu_verify_access(), which is not called any longer and has
been removed.
As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 -
6 files changed, 66 insertions(+), 122 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index e0c4f7c7f1b9..19c5ab08d9ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -42,52 +42,6 @@
#include
#include
-/**
- * amdgpu_gem_prime_mmap - _driver.gem_prime_mmap implementation
- * @obj: GEM BO
- * @vma: Virtual memory area
- *
- * Sets up a userspace mapping of the BO's memory in the given
- * virtual memory area.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- */
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma)
-{
- struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
- struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
- unsigned asize = amdgpu_bo_size(bo);
- int ret;
-
- if (!vma->vm_file)
- return -ENODEV;
-
- if (adev == NULL)
- return -ENODEV;
-
- /* Check for valid size. */
- if (asize < vma->vm_end - vma->vm_start)
- return -EINVAL;
-
- if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
- (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
- return -EPERM;
- }
- vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
-
- /* prime mmap does not need to check access, so allow here */
- ret = drm_vma_node_allow(>vma_node,
vma->vm_file->private_data);
- if (ret)
- return ret;
-
- ret = ttm_bo_mmap(vma->vm_file, vma, >mman.bdev);
- drm_vma_node_revoke(>vma_node, vma->vm_file->private_data);
-
- return ret;
-}
-
static int
__dma_resv_make_exclusive(struct dma_resv *obj)
{
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 39b5b9616fd8..3e93b9b407a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,6 @@ struct drm_gem_object
*amdgpu_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf);
bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
struct amdgpu_bo *bo);
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma);
extern const struct dma_buf_ops amdgpu_dmabuf_ops;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 76f48f79c70b..e96d2758f4bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1656,7 +1656,7 @@ static const struct file_operations
amdgpu_driver_kms_fops = {
.flush = amdgpu_flush,
.release = drm_release,
.unlocked_ioctl = amdgpu_drm_ioctl,
- .mmap = amdgpu_mmap,
+ .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
#ifdef CONFIG_COMPAT
@@ -1719,7 +1719,7 @@ static const struct drm_driver amdgpu_kms_driver
= {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import = amdgpu_gem_prime_import,
- .gem_prime_mmap = amdgpu_gem_prime_mmap,
+ .gem_prime_mmap = drm_gem_prime_mmap,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index fb7171e5507c..fe93faad05f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -41,6 +41,36 @@
static const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
+static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
Please name that function amdgpu_gem_fault or amdgpu_gem_object_fault
+{
+ struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+ vm_fault_t ret;
+
+ ret = ttm_bo_vm_reserve(bo, vmf);
+ if (ret)
+ return ret;
+
+ ret =
Re: [Nouveau] [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function
Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.
This change resolves several inconsistencies between regular mmap and
prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
areas. Previously it way only set for regular mmap calls, prime-based
mmap used TTM's default vm_ops. The check for kfd_bo has been taken
from amdgpu_verify_access(), which is not called any longer and has
been removed.
As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71 -
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 -
6 files changed, 66 insertions(+), 122 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index e0c4f7c7f1b9..19c5ab08d9ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -42,52 +42,6 @@
#include
#include
-/**
- * amdgpu_gem_prime_mmap - _driver.gem_prime_mmap implementation
- * @obj: GEM BO
- * @vma: Virtual memory area
- *
- * Sets up a userspace mapping of the BO's memory in the given
- * virtual memory area.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- */
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma)
-{
- struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
- struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
- unsigned asize = amdgpu_bo_size(bo);
- int ret;
-
- if (!vma->vm_file)
- return -ENODEV;
-
- if (adev == NULL)
- return -ENODEV;
-
- /* Check for valid size. */
- if (asize < vma->vm_end - vma->vm_start)
- return -EINVAL;
-
- if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
- (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
- return -EPERM;
- }
- vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
-
- /* prime mmap does not need to check access, so allow here */
- ret = drm_vma_node_allow(>vma_node, vma->vm_file->private_data);
- if (ret)
- return ret;
-
- ret = ttm_bo_mmap(vma->vm_file, vma, >mman.bdev);
- drm_vma_node_revoke(>vma_node, vma->vm_file->private_data);
-
- return ret;
-}
-
static int
__dma_resv_make_exclusive(struct dma_resv *obj)
{
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 39b5b9616fd8..3e93b9b407a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,6 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct
drm_device *dev,
struct dma_buf *dma_buf);
bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
struct amdgpu_bo *bo);
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma);
extern const struct dma_buf_ops amdgpu_dmabuf_ops;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 76f48f79c70b..e96d2758f4bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1656,7 +1656,7 @@ static const struct file_operations
amdgpu_driver_kms_fops = {
.flush = amdgpu_flush,
.release = drm_release,
.unlocked_ioctl = amdgpu_drm_ioctl,
- .mmap = amdgpu_mmap,
+ .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
#ifdef CONFIG_COMPAT
@@ -1719,7 +1719,7 @@ static const struct drm_driver amdgpu_kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import = amdgpu_gem_prime_import,
- .gem_prime_mmap = amdgpu_gem_prime_mmap,
+ .gem_prime_mmap = drm_gem_prime_mmap,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index fb7171e5507c..fe93faad05f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -41,6 +41,36 @@
static const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
+static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
Please name that function amdgpu_gem_fault or amdgpu_gem_object_fault
+{
+ struct ttm_buffer_object *bo =
