Re: [Nouveau] [PATCH v9 03/10] mm/rmap: Split try_to_munlock from try_to_unmap

2021-05-25 Thread Shakeel Butt
On Tue, May 25, 2021 at 11:40 AM Liam Howlett  wrote:
>
[...]
> >
> > +/*
> > + * Walks the vma's mapping a page and mlocks the page if any locked vma's 
> > are
> > + * found. Once one is found the page is locked and the scan can be 
> > terminated.
> > + */
>
> Can you please add that this requires the mmap_sem() lock to the
> comments?
>

Why does this require mmap_sem() lock? Also mmap_sem() lock of which mm_struct?

> > +static bool page_mlock_one(struct page *page, struct vm_area_struct *vma,
> > +  unsigned long address, void *unused)
> > +{
> > + struct page_vma_mapped_walk pvmw = {
> > + .page = page,
> > + .vma = vma,
> > + .address = address,
> > + };
> > +
> > + /* An un-locked vma doesn't have any pages to lock, continue the scan 
> > */
> > + if (!(vma->vm_flags & VM_LOCKED))
> > + return true;
> > +
> > + while (page_vma_mapped_walk()) {
> > + /* PTE-mapped THP are never mlocked */
> > + if (!PageTransCompound(page))
> > + mlock_vma_page(page);
> > + page_vma_mapped_walk_done();
> > +
> > + /*
> > +  * no need to continue scanning other vma's if the page has
> > +  * been locked.
> > +  */
> > + return false;
> > + }
> > +
> > + return true;
> > +}
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v9 03/10] mm/rmap: Split try_to_munlock from try_to_unmap

2021-05-25 Thread Liam Howlett
* Alistair Popple  [210524 09:29]:
> The behaviour of try_to_unmap_one() is difficult to follow because it
> performs different operations based on a fairly large set of flags used
> in different combinations.
> 
> TTU_MUNLOCK is one such flag. However it is exclusively used by
> try_to_munlock() which specifies no other flags. Therefore rather than
> overload try_to_unmap_one() with unrelated behaviour split this out into
> it's own function and remove the flag.
> 
> Signed-off-by: Alistair Popple 
> Reviewed-by: Ralph Campbell 
> Reviewed-by: Christoph Hellwig 
> 
> ---
> 
> v9:
> * Improved comments
> 
> v8:
> * Renamed try_to_munlock to page_mlock to better reflect what the
>   function actually does.
> * Removed the TODO from the documentation that this patch addresses.
> 
> v7:
> * Added Christoph's Reviewed-by
> 
> v4:
> * Removed redundant check for VM_LOCKED
> ---
>  Documentation/vm/unevictable-lru.rst | 33 ++-
>  include/linux/rmap.h |  3 +-
>  mm/mlock.c   | 10 ++---
>  mm/rmap.c| 61 
>  4 files changed, 63 insertions(+), 44 deletions(-)
> 
> diff --git a/Documentation/vm/unevictable-lru.rst 
> b/Documentation/vm/unevictable-lru.rst
> index 0e1490524f53..eae3af17f2d9 100644
> --- a/Documentation/vm/unevictable-lru.rst
> +++ b/Documentation/vm/unevictable-lru.rst
> @@ -389,14 +389,14 @@ mlocked, munlock_vma_page() updates that zone 
> statistics for the number of
>  mlocked pages.  Note, however, that at this point we haven't checked whether
>  the page is mapped by other VM_LOCKED VMAs.
>  
> -We can't call try_to_munlock(), the function that walks the reverse map to
> +We can't call page_mlock(), the function that walks the reverse map to
>  check for other VM_LOCKED VMAs, without first isolating the page from the 
> LRU.
> -try_to_munlock() is a variant of try_to_unmap() and thus requires that the 
> page
> +page_mlock() is a variant of try_to_unmap() and thus requires that the page
>  not be on an LRU list [more on these below].  However, the call to
> -isolate_lru_page() could fail, in which case we couldn't try_to_munlock().  
> So,
> +isolate_lru_page() could fail, in which case we can't call page_mlock().  So,
>  we go ahead and clear PG_mlocked up front, as this might be the only chance 
> we
> -have.  If we can successfully isolate the page, we go ahead and
> -try_to_munlock(), which will restore the PG_mlocked flag and update the zone
> +have.  If we can successfully isolate the page, we go ahead and call
> +page_mlock(), which will restore the PG_mlocked flag and update the zone
>  page statistics if it finds another VMA holding the page mlocked.  If we fail
>  to isolate the page, we'll have left a potentially mlocked page on the LRU.
>  This is fine, because we'll catch it later if and if vmscan tries to reclaim
> @@ -545,31 +545,24 @@ munlock or munmap system calls, mm teardown 
> (munlock_vma_pages_all), reclaim,
>  holepunching, and truncation of file pages and their anonymous COWed pages.
>  
>  
> -try_to_munlock() Reverse Map Scan
> +page_mlock() Reverse Map Scan
>  -
>  
> -.. warning::
> -   [!] TODO/FIXME: a better name might be page_mlocked() - analogous to the
> -   page_referenced() reverse map walker.
> -
>  When munlock_vma_page() [see section :ref:`munlock()/munlockall() System Call
>  Handling ` above] tries to munlock a
>  page, it needs to determine whether or not the page is mapped by any
>  VM_LOCKED VMA without actually attempting to unmap all PTEs from the
>  page.  For this purpose, the unevictable/mlock infrastructure
> -introduced a variant of try_to_unmap() called try_to_munlock().
> +introduced a variant of try_to_unmap() called page_mlock().
>  
> -try_to_munlock() calls the same functions as try_to_unmap() for anonymous and
> -mapped file and KSM pages with a flag argument specifying unlock versus unmap
> -processing.  Again, these functions walk the respective reverse maps looking
> -for VM_LOCKED VMAs.  When such a VMA is found, as in the try_to_unmap() case,
> -the functions mlock the page via mlock_vma_page() and return SWAP_MLOCK.  
> This
> -undoes the pre-clearing of the page's PG_mlocked done by munlock_vma_page.
> +page_mlock() walks the respective reverse maps looking for VM_LOCKED VMAs. 
> When
> +such a VMA is found the page is mlocked via mlock_vma_page(). This undoes the
> +pre-clearing of the page's PG_mlocked done by munlock_vma_page.
>  
> -Note that try_to_munlock()'s reverse map walk must visit every VMA in a 
> page's
> +Note that page_mlock()'s reverse map walk must visit every VMA in a page's
>  reverse map to determine that a page is NOT mapped into any VM_LOCKED VMA.
>  However, the scan can terminate when it encounters a VM_LOCKED VMA.
> -Although try_to_munlock() might be called a great many times when munlocking 
> a
> +Although page_mlock() might be called a great many times when 

Re: [Nouveau] [PATCH v9 07/10] mm: Device exclusive memory access

2021-05-25 Thread Balbir Singh
On Mon, May 24, 2021 at 03:11:57PM -0700, Andrew Morton wrote:
> On Mon, 24 May 2021 23:27:22 +1000 Alistair Popple  wrote:
> 
> > Some devices require exclusive write access to shared virtual
> > memory (SVM) ranges to perform atomic operations on that memory. This
> > requires CPU page tables to be updated to deny access whilst atomic
> > operations are occurring.
> > 
> > In order to do this introduce a new swap entry
> > type (SWP_DEVICE_EXCLUSIVE). When a SVM range needs to be marked for
> > exclusive access by a device all page table mappings for the particular
> > range are replaced with device exclusive swap entries. This causes any
> > CPU access to the page to result in a fault.
> > 
> > Faults are resovled by replacing the faulting entry with the original
> > mapping. This results in MMU notifiers being called which a driver uses
> > to update access permissions such as revoking atomic access. After
> > notifiers have been called the device will no longer have exclusive
> > access to the region.
> > 
> > Walking of the page tables to find the target pages is handled by
> > get_user_pages() rather than a direct page table walk. A direct page
> > table walk similar to what migrate_vma_collect()/unmap() does could also
> > have been utilised. However this resulted in more code similar in
> > functionality to what get_user_pages() provides as page faulting is
> > required to make the PTEs present and to break COW.
> > 
> > ...
> >
> >  Documentation/vm/hmm.rst |  17 
> >  include/linux/mmu_notifier.h |   6 ++
> >  include/linux/rmap.h |   4 +
> >  include/linux/swap.h |   7 +-
> >  include/linux/swapops.h  |  44 -
> >  mm/hmm.c |   5 +
> >  mm/memory.c  | 128 +++-
> >  mm/mprotect.c|   8 ++
> >  mm/page_vma_mapped.c |   9 +-
> >  mm/rmap.c| 186 +++
> >  10 files changed, 405 insertions(+), 9 deletions(-)
> > 
> 
> This is quite a lot of code added to core MM for a single driver.
> 
> Is there any expectation that other drivers will use this code?
> 
> Is there a way of reducing the impact (code size, at least) for systems
> which don't need this code?
>
> How beneficial is this code to nouveau users?  I see that it permits a
> part of OpenCL to be implemented, but how useful/important is this in
> the real world?

That is a very good question! I've not reviewed the code, but a sample
program with the described use case would make things easy to parse.
I suspect that is not easy to build at the moment?

I wonder how we co-ordinate all the work the mm is doing, page migration,
reclaim with device exclusive access? Do we have any numbers for the worst
case page fault latency when something is marked away for exclusive access?
I presume for now this is anonymous memory only? SWP_DEVICE_EXCLUSIVE would
only impact the address space of programs using the GPU. Should the exclusively
marked range live in the unreclaimable list and recycled back to 
active/in-active
to account for the fact that

1. It is not reclaimable and reclaim will only hurt via page faults?
2. It ages the page correctly or at-least allows for that possibility when the
   page is used by the GPU.

Balbir Singh.
 
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [PATCH -next] drm/nouveau: Remove set but not used variable 'dev'

2021-05-25 Thread Baokun Li
Fixes gcc '-Wunused-but-set-variable' warning:

drivers/gpu/drm/nouveau/nouveau_bo.c: In function 'nouveau_ttm_tt_populate':
drivers/gpu/drm/nouveau/nouveau_bo.c:1258:17: warning:
 variable ‘dev’ set but not used [-Wunused-but-set-variable]

drivers/gpu/drm/nouveau/nouveau_bo.c: In function 'nouveau_ttm_tt_unpopulate':
drivers/gpu/drm/nouveau/nouveau_bo.c:1281:17: warning:
 variable ‘dev’ set but not used [-Wunused-but-set-variable]

It never used since introduction.

Signed-off-by: Baokun Li 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 7a2624c0ba4c..51f9a2e6532e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1254,7 +1254,6 @@ nouveau_ttm_tt_populate(struct ttm_device *bdev,
 {
struct ttm_tt *ttm_dma = (void *)ttm;
struct nouveau_drm *drm;
-   struct device *dev;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
 
if (ttm_tt_is_populated(ttm))
@@ -1267,7 +1266,6 @@ nouveau_ttm_tt_populate(struct ttm_device *bdev,
}
 
drm = nouveau_bdev(bdev);
-   dev = drm->dev->dev;
 
return ttm_pool_alloc(>ttm.bdev.pool, ttm, ctx);
 }
@@ -1277,14 +1275,12 @@ nouveau_ttm_tt_unpopulate(struct ttm_device *bdev,
  struct ttm_tt *ttm)
 {
struct nouveau_drm *drm;
-   struct device *dev;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
 
if (slave)
return;
 
drm = nouveau_bdev(bdev);
-   dev = drm->dev->dev;
 
return ttm_pool_free(>ttm.bdev.pool, ttm);
 }
-- 
2.25.4

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v4 0/7] drm: Clean up mmap for TTM-based GEM drivers

2021-05-25 Thread Thomas Zimmermann

Something I forgot to add is that patches 1 to 4 already have a

  Reviewed-by: Christian König 



Am 25.05.21 um 17:10 schrieb Thomas Zimmermann:

Implement mmap via struct drm_gem_object_functions.mmap in amdgpu,
radeon and nouveau. This allows for using common DRM helpers for
the mmap-related callbacks in struct file_operations and struct
drm_driver. The drivers have their own vm_ops, which are now set
automatically by the DRM core functions. The code in each driver's
verify_access becomes part of the driver's new mmap implementation.

With the GEM drivers converted, vmwgfx is the only user of
ttm_bo_mmap() and related infrastructure. So move everything into
vmwgfx and delete the rsp code from TTM.

This touches several drivers. Preferably everything would be merged
at once via drm-misc-next.

v4:
* rebase on top of amdgpu hot-unplug changes
v3:
* tidy up the new mmap functions in amdgpu and radeon (Christian)
v2:
* removal of amdgpu fbdev mmap already merged (Christian)
* rebase on top of amdgpu fixes [1] (Felix)
* replace pr_err() with drm_err() in vmwgfx patch (Zack)
* several typos

[1] https://patchwork.freedesktop.org/series/88822/

Thomas Zimmermann (7):
   drm/ttm: Don't override vm_ops callbacks, if set
   drm/amdgpu: Implement mmap as GEM object function
   drm/radeon: Implement mmap as GEM object function
   drm/nouveau: Implement mmap as GEM object function
   drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
   drm/vmwgfx: Inline vmw_verify_access()
   drm/ttm: Remove ttm_bo_mmap() and friends

  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 | 55 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 75 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  1 -
  drivers/gpu/drm/nouveau/nouveau_bo.c| 10 ---
  drivers/gpu/drm/nouveau/nouveau_drm.c   |  3 +-
  drivers/gpu/drm/nouveau/nouveau_gem.c   | 36 ++
  drivers/gpu/drm/nouveau/nouveau_ttm.c   | 49 --
  drivers/gpu/drm/nouveau/nouveau_ttm.h   |  1 -
  drivers/gpu/drm/radeon/radeon_drv.c |  3 +-
  drivers/gpu/drm/radeon/radeon_gem.c | 49 ++
  drivers/gpu/drm/radeon/radeon_ttm.c | 65 --
  drivers/gpu/drm/radeon/radeon_ttm.h |  1 -
  drivers/gpu/drm/ttm/ttm_bo_vm.c | 60 ++---
  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  9 ---
  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c| 53 ++-
  include/drm/ttm/ttm_bo_api.h| 13 
  include/drm/ttm/ttm_device.h| 15 -
  20 files changed, 202 insertions(+), 348 deletions(-)


base-commit: 28dddc0c90bc6464be4c5e3224a293c022564a4e
prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
--
2.31.1



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


[Nouveau] [PATCH v4 6/7] drm/vmwgfx: Inline vmw_verify_access()

2021-05-25 Thread Thomas Zimmermann
Vmwgfx is the only user of the TTM's verify_access callback. Inline
the call and avoid the indirection through the function pointer.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Zack Rusin 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 -
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c   | 7 ++-
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 7bfe83c936ff..35b03fe21161 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -661,14 +661,6 @@ static void vmw_evict_flags(struct ttm_buffer_object *bo,
*placement = vmw_sys_placement;
 }
 
-static int vmw_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-   struct ttm_object_file *tfile =
-   vmw_fpriv((struct drm_file *)filp->private_data)->tfile;
-
-   return vmw_user_bo_verify_access(bo, tfile);
-}
-
 static int vmw_ttm_io_mem_reserve(struct ttm_device *bdev, struct ttm_resource 
*mem)
 {
struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, 
bdev);
@@ -771,7 +763,6 @@ struct ttm_device_funcs vmw_bo_driver = {
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = vmw_evict_flags,
.move = vmw_move,
-   .verify_access = vmw_verify_access,
.swap_notify = vmw_swap_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
 };
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index c8b6543b4e39..e6b1f98ec99f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@ -67,6 +67,7 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
};
struct drm_file *file_priv = filp->private_data;
struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
+   struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
struct ttm_device *bdev = _priv->bdev;
struct ttm_buffer_object *bo;
int ret;
@@ -78,11 +79,7 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
if (unlikely(!bo))
return -EINVAL;
 
-   if (unlikely(!bo->bdev->funcs->verify_access)) {
-   ret = -EPERM;
-   goto out_unref;
-   }
-   ret = bo->bdev->funcs->verify_access(bo, filp);
+   ret = vmw_user_bo_verify_access(bo, tfile);
if (unlikely(ret != 0))
goto out_unref;
 
-- 
2.31.1

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [PATCH v4 7/7] drm/ttm: Remove ttm_bo_mmap() and friends

2021-05-25 Thread Thomas Zimmermann
The function ttm_bo_mmap is unused. Remove it and it's helpers; including
the verify_access callback in struct ttm_device_funcs.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 53 -
 include/drm/ttm/ttm_bo_api.h| 13 
 include/drm/ttm/ttm_device.h| 15 --
 3 files changed, 81 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index ff07dbc91c03..9bd15cb39145 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -560,30 +560,6 @@ static const struct vm_operations_struct ttm_bo_vm_ops = {
.access = ttm_bo_vm_access,
 };
 
-static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_device *bdev,
- unsigned long offset,
- unsigned long pages)
-{
-   struct drm_vma_offset_node *node;
-   struct ttm_buffer_object *bo = NULL;
-
-   drm_vma_offset_lock_lookup(bdev->vma_manager);
-
-   node = drm_vma_offset_lookup_locked(bdev->vma_manager, offset, pages);
-   if (likely(node)) {
-   bo = container_of(node, struct ttm_buffer_object,
- base.vma_node);
-   bo = ttm_bo_get_unless_zero(bo);
-   }
-
-   drm_vma_offset_unlock_lookup(bdev->vma_manager);
-
-   if (!bo)
-   pr_err("Could not find buffer object to map\n");
-
-   return bo;
-}
-
 static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct 
vm_area_struct *vma)
 {
/*
@@ -611,35 +587,6 @@ static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object 
*bo, struct vm_area_s
vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
 }
 
-int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
-   struct ttm_device *bdev)
-{
-   struct ttm_buffer_object *bo;
-   int ret;
-
-   if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
-   return -EINVAL;
-
-   bo = ttm_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
-   if (unlikely(!bo))
-   return -EINVAL;
-
-   if (unlikely(!bo->bdev->funcs->verify_access)) {
-   ret = -EPERM;
-   goto out_unref;
-   }
-   ret = bo->bdev->funcs->verify_access(bo, filp);
-   if (unlikely(ret != 0))
-   goto out_unref;
-
-   ttm_bo_mmap_vma_setup(bo, vma);
-   return 0;
-out_unref:
-   ttm_bo_put(bo);
-   return ret;
-}
-EXPORT_SYMBOL(ttm_bo_mmap);
-
 int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
 {
ttm_bo_get(bo);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 254ede97f8e3..f2a5f37c61b7 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -524,19 +524,6 @@ void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct 
dma_buf_map *map);
  */
 int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo);
 
-/**
- * ttm_bo_mmap - mmap out of the ttm device address space.
- *
- * @filp:  filp as input from the mmap method.
- * @vma:   vma as input from the mmap method.
- * @bdev:  Pointer to the ttm_device with the address space manager.
- *
- * This function is intended to be called by the device mmap method.
- * if the device address space is to be backed by the bo manager.
- */
-int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
-   struct ttm_device *bdev);
-
 /**
  * ttm_bo_io
  *
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 7c8f87bd52d3..cd592f8e941b 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -161,21 +161,6 @@ struct ttm_device_funcs {
struct ttm_resource *new_mem,
struct ttm_place *hop);
 
-   /**
-* struct ttm_bo_driver_member verify_access
-*
-* @bo: Pointer to a buffer object.
-* @filp: Pointer to a struct file trying to access the object.
-*
-* Called from the map / write / read methods to verify that the
-* caller is permitted to access the buffer object.
-* This member may be set to NULL, which will refuse this kind of
-* access for all buffer objects.
-* This function should return 0 if access is granted, -EPERM otherwise.
-*/
-   int (*verify_access)(struct ttm_buffer_object *bo,
-struct file *filp);
-
/**
 * Hook to notify driver about a resource delete.
 */
-- 
2.31.1

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [PATCH v4 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver

2021-05-25 Thread Thomas Zimmermann
The vmwgfx driver is the only remaining user of ttm_bo_mmap(). Inline
the code. The internal helper ttm_bo_vm_lookup() is now also part of
vmwgfx as vmw_bo_vm_lookup().

v2:
* replace pr_err() with drm_err() (Zack)

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Zack Rusin 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 56 ++--
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index cb9975889e2f..c8b6543b4e39 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@ -27,6 +27,32 @@
 
 #include "vmwgfx_drv.h"
 
+static struct ttm_buffer_object *vmw_bo_vm_lookup(struct ttm_device *bdev,
+ unsigned long offset,
+ unsigned long pages)
+{
+   struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, 
bdev);
+   struct drm_device *drm = _priv->drm;
+   struct drm_vma_offset_node *node;
+   struct ttm_buffer_object *bo = NULL;
+
+   drm_vma_offset_lock_lookup(bdev->vma_manager);
+
+   node = drm_vma_offset_lookup_locked(bdev->vma_manager, offset, pages);
+   if (likely(node)) {
+   bo = container_of(node, struct ttm_buffer_object,
+ base.vma_node);
+   bo = ttm_bo_get_unless_zero(bo);
+   }
+
+   drm_vma_offset_unlock_lookup(bdev->vma_manager);
+
+   if (!bo)
+   drm_err(drm, "Could not find buffer object to map\n");
+
+   return bo;
+}
+
 int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
 {
static const struct vm_operations_struct vmw_vm_ops = {
@@ -41,10 +67,28 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
};
struct drm_file *file_priv = filp->private_data;
struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
-   int ret = ttm_bo_mmap(filp, vma, _priv->bdev);
+   struct ttm_device *bdev = _priv->bdev;
+   struct ttm_buffer_object *bo;
+   int ret;
+
+   if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
+   return -EINVAL;
+
+   bo = vmw_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
+   if (unlikely(!bo))
+   return -EINVAL;
 
-   if (ret)
-   return ret;
+   if (unlikely(!bo->bdev->funcs->verify_access)) {
+   ret = -EPERM;
+   goto out_unref;
+   }
+   ret = bo->bdev->funcs->verify_access(bo, filp);
+   if (unlikely(ret != 0))
+   goto out_unref;
+
+   ret = ttm_bo_mmap_obj(vma, bo);
+   if (unlikely(ret != 0))
+   goto out_unref;
 
vma->vm_ops = _vm_ops;
 
@@ -52,7 +96,13 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
if (!is_cow_mapping(vma->vm_flags))
vma->vm_flags = (vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP;
 
+   ttm_bo_put(bo); /* release extra ref taken by ttm_bo_mmap_obj() */
+
return 0;
+
+out_unref:
+   ttm_bo_put(bo);
+   return ret;
 }
 
 /* struct vmw_validation_mem callback */
-- 
2.31.1

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [PATCH v4 4/7] drm/nouveau: Implement mmap as GEM object function

2021-05-25 Thread Thomas Zimmermann
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

The GEM object function is provided by GEM TTM helpers. Nouveau's
implementation of verify_access is unused and has been removed. Access
permissions are validated by the DRM helpers.

As a side effect, nouveau_ttm_vm_ops and nouveau_ttm_fault() are now
implemented in nouveau's GEM code.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c  | 10 --
 drivers/gpu/drm/nouveau/nouveau_drm.c |  3 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c | 36 
 drivers/gpu/drm/nouveau/nouveau_ttm.c | 49 ---
 drivers/gpu/drm/nouveau/nouveau_ttm.h |  1 -
 5 files changed, 38 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 7a2624c0ba4c..c390f24f25f3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1050,15 +1050,6 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
return ret;
 }
 
-static int
-nouveau_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-   struct nouveau_bo *nvbo = nouveau_bo(bo);
-
-   return drm_vma_node_verify_access(>bo.base.vma_node,
- filp->private_data);
-}
-
 static void
 nouveau_ttm_io_mem_free_locked(struct nouveau_drm *drm,
   struct ttm_resource *reg)
@@ -1331,7 +1322,6 @@ struct ttm_device_funcs nouveau_bo_driver = {
.evict_flags = nouveau_bo_evict_flags,
.delete_mem_notify = nouveau_bo_delete_mem_notify,
.move = nouveau_bo_move,
-   .verify_access = nouveau_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
 };
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 3204fc0a90d2..a616cf4573b8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1179,7 +1179,7 @@ nouveau_driver_fops = {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = nouveau_drm_ioctl,
-   .mmap = nouveau_ttm_mmap,
+   .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
 #if defined(CONFIG_COMPAT)
@@ -1212,6 +1212,7 @@ driver_stub = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import_sg_table = nouveau_gem_prime_import_sg_table,
+   .gem_prime_mmap = drm_gem_prime_mmap,
 
.dumb_create = nouveau_display_dumb_create,
.dumb_map_offset = drm_gem_ttm_dumb_map_offset,
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index a70e82413fa7..722e1decc202 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -39,6 +39,40 @@
 #include 
 #include 
 
+static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
+{
+   struct vm_area_struct *vma = vmf->vma;
+   struct ttm_buffer_object *bo = vma->vm_private_data;
+   pgprot_t prot;
+   vm_fault_t ret;
+
+   ret = ttm_bo_vm_reserve(bo, vmf);
+   if (ret)
+   return ret;
+
+   ret = nouveau_ttm_fault_reserve_notify(bo);
+   if (ret)
+   goto error_unlock;
+
+   nouveau_bo_del_io_reserve_lru(bo);
+   prot = vm_get_page_prot(vma->vm_flags);
+   ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1);
+   nouveau_bo_add_io_reserve_lru(bo);
+   if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+   return ret;
+
+error_unlock:
+   dma_resv_unlock(bo->base.resv);
+   return ret;
+}
+
+static const struct vm_operations_struct nouveau_ttm_vm_ops = {
+   .fault = nouveau_ttm_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+   .access = ttm_bo_vm_access
+};
+
 void
 nouveau_gem_object_del(struct drm_gem_object *gem)
 {
@@ -180,6 +214,8 @@ const struct drm_gem_object_funcs nouveau_gem_object_funcs 
= {
.get_sg_table = nouveau_gem_prime_get_sg_table,
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
+   .mmap = drm_gem_ttm_mmap,
+   .vm_ops = _ttm_vm_ops,
 };
 
 int
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c 
b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 468eacb41f05..65430912ff72 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -125,55 +125,6 @@ const struct ttm_resource_manager_func nv04_gart_manager = 
{
.free = nouveau_manager_del,
 };
 
-static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
-{
-   struct vm_area_struct *vma = vmf->vma;
-   struct ttm_buffer_object *bo = vma->vm_private_data;
-   pgprot_t prot;
-   vm_fault_t ret;
-
-   ret = ttm_bo_vm_reserve(bo, vmf);
-   

[Nouveau] [PATCH v4 2/7] drm/amdgpu: Implement mmap as GEM object function

2021-05-25 Thread 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 function amdgpu_verify_access() is
no longer being called and therefore removed by this patch.

As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.

v4:
* rebased
v3:
* rename mmap function to amdgpu_gem_object_mmap() (Christian)
* remove unnecessary checks from mmap (Christian)
v2:
* rename amdgpu_ttm_vm_ops and amdgpu_ttm_fault() to
  amdgpu_gem_vm_ops and amdgpu_gem_fault() (Christian)
* the check for kfd_bo has meanwhile been removed

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 | 55 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 75 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  1 -
 6 files changed, 57 insertions(+), 126 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index baa980a477d9..6ec1312b7389 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 3a0890ca816a..d3b9cde993e6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1691,7 +1691,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
@@ -1758,7 +1758,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 18974bd081f0..73c76a3e2b12 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -32,6 +32,7 @@
 

[Nouveau] [PATCH v4 3/7] drm/radeon: Implement mmap as GEM object function

2021-05-25 Thread Thomas Zimmermann
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

This change also allows to support prime-based mmap via DRM's helper
drm_gem_prime_mmap().

Permission checks are implemented by drm_gem_mmap(), with an additional
check for radeon_ttm_tt_has_userptr() in the GEM object function. The
function radeon_verify_access() is now unused and has thus been removed.

As a side effect, radeon_ttm_vm_ops and radeon_ttm_fault() are now
implemented in amdgpu's GEM code.

v3:
* remove unnecessary checks from mmap (Christian)
v2:
* rename radeon_ttm_vm_ops and radeon_ttm_fault() to
  radeon_gem_vm_ops and radeon_gem_fault() (Christian)
* fix commit description (Alex)

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/radeon/radeon_drv.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c | 49 ++
 drivers/gpu/drm/radeon/radeon_ttm.c | 65 -
 drivers/gpu/drm/radeon/radeon_ttm.h |  1 -
 4 files changed, 51 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index 31d3dd0e5258..8cd135fa6dcd 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -545,7 +545,7 @@ static const struct file_operations radeon_driver_kms_fops 
= {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = radeon_drm_ioctl,
-   .mmap = radeon_mmap,
+   .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
 #ifdef CONFIG_COMPAT
@@ -620,6 +620,7 @@ static const struct drm_driver 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_sg_table = radeon_gem_prime_import_sg_table,
+   .gem_prime_mmap = drm_gem_prime_mmap,
 
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
b/drivers/gpu/drm/radeon/radeon_gem.c
index 05ea2f39f626..ff8849827d61 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -44,6 +44,42 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj);
 
 const struct drm_gem_object_funcs radeon_gem_object_funcs;
 
+static vm_fault_t radeon_gem_fault(struct vm_fault *vmf)
+{
+   struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+   struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
+   vm_fault_t ret;
+
+   down_read(>pm.mclk_lock);
+
+   ret = ttm_bo_vm_reserve(bo, vmf);
+   if (ret)
+   goto unlock_mclk;
+
+   ret = radeon_bo_fault_reserve_notify(bo);
+   if (ret)
+   goto unlock_resv;
+
+   ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
+  TTM_BO_VM_NUM_PREFAULT, 1);
+   if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+   goto unlock_mclk;
+
+unlock_resv:
+   dma_resv_unlock(bo->base.resv);
+
+unlock_mclk:
+   up_read(>pm.mclk_lock);
+   return ret;
+}
+
+static const struct vm_operations_struct radeon_gem_vm_ops = {
+   .fault = radeon_gem_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+   .access = ttm_bo_vm_access
+};
+
 static void radeon_gem_object_free(struct drm_gem_object *gobj)
 {
struct radeon_bo *robj = gem_to_radeon_bo(gobj);
@@ -226,6 +262,17 @@ static int radeon_gem_handle_lockup(struct radeon_device 
*rdev, int r)
return r;
 }
 
+static int radeon_gem_object_mmap(struct drm_gem_object *obj, struct 
vm_area_struct *vma)
+{
+   struct radeon_bo *bo = gem_to_radeon_bo(obj);
+   struct radeon_device *rdev = radeon_get_rdev(bo->tbo.bdev);
+
+   if (radeon_ttm_tt_has_userptr(rdev, bo->tbo.ttm))
+   return -EPERM;
+
+   return drm_gem_ttm_mmap(obj, vma);
+}
+
 const struct drm_gem_object_funcs radeon_gem_object_funcs = {
.free = radeon_gem_object_free,
.open = radeon_gem_object_open,
@@ -236,6 +283,8 @@ const struct drm_gem_object_funcs radeon_gem_object_funcs = 
{
.get_sg_table = radeon_gem_prime_get_sg_table,
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
+   .mmap = radeon_gem_object_mmap,
+   .vm_ops = _gem_vm_ops,
 };
 
 /*
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 3361d11769a2..a71d94f7067b 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -135,17 +135,6 @@ static void radeon_evict_flags(struct ttm_buffer_object 
*bo,
*placement = rbo->placement;
 }
 
-static int radeon_verify_access(struct ttm_buffer_object *bo, struct file 
*filp)
-{
-   struct radeon_bo *rbo = container_of(bo, struct radeon_bo, tbo);
-   struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
-
-   if (radeon_ttm_tt_has_userptr(rdev, 

[Nouveau] [PATCH v4 0/7] drm: Clean up mmap for TTM-based GEM drivers

2021-05-25 Thread Thomas Zimmermann
Implement mmap via struct drm_gem_object_functions.mmap in amdgpu,
radeon and nouveau. This allows for using common DRM helpers for
the mmap-related callbacks in struct file_operations and struct
drm_driver. The drivers have their own vm_ops, which are now set
automatically by the DRM core functions. The code in each driver's
verify_access becomes part of the driver's new mmap implementation.

With the GEM drivers converted, vmwgfx is the only user of
ttm_bo_mmap() and related infrastructure. So move everything into
vmwgfx and delete the rsp code from TTM.

This touches several drivers. Preferably everything would be merged
at once via drm-misc-next.

v4:
* rebase on top of amdgpu hot-unplug changes
v3:
* tidy up the new mmap functions in amdgpu and radeon (Christian)
v2:
* removal of amdgpu fbdev mmap already merged (Christian)
* rebase on top of amdgpu fixes [1] (Felix)
* replace pr_err() with drm_err() in vmwgfx patch (Zack)
* several typos

[1] https://patchwork.freedesktop.org/series/88822/

Thomas Zimmermann (7):
  drm/ttm: Don't override vm_ops callbacks, if set
  drm/amdgpu: Implement mmap as GEM object function
  drm/radeon: Implement mmap as GEM object function
  drm/nouveau: Implement mmap as GEM object function
  drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
  drm/vmwgfx: Inline vmw_verify_access()
  drm/ttm: Remove ttm_bo_mmap() and friends

 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 | 55 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 75 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  1 -
 drivers/gpu/drm/nouveau/nouveau_bo.c| 10 ---
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  3 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c   | 36 ++
 drivers/gpu/drm/nouveau/nouveau_ttm.c   | 49 --
 drivers/gpu/drm/nouveau/nouveau_ttm.h   |  1 -
 drivers/gpu/drm/radeon/radeon_drv.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c | 49 ++
 drivers/gpu/drm/radeon/radeon_ttm.c | 65 --
 drivers/gpu/drm/radeon/radeon_ttm.h |  1 -
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 60 ++---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  9 ---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c| 53 ++-
 include/drm/ttm/ttm_bo_api.h| 13 
 include/drm/ttm/ttm_device.h| 15 -
 20 files changed, 202 insertions(+), 348 deletions(-)


base-commit: 28dddc0c90bc6464be4c5e3224a293c022564a4e
prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
--
2.31.1

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [PATCH v4 1/7] drm/ttm: Don't override vm_ops callbacks, if set

2021-05-25 Thread Thomas Zimmermann
Drivers may want to set their own callbacks for a VM area. Only set
TTM's callbacks if the vm_ops field is clear.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 7ff9fd551357..ff07dbc91c03 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -586,7 +586,12 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct 
ttm_device *bdev,
 
 static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct 
vm_area_struct *vma)
 {
-   vma->vm_ops = _bo_vm_ops;
+   /*
+* Drivers may want to override the vm_ops field. Otherwise we
+* use TTM's default callbacks.
+*/
+   if (!vma->vm_ops)
+   vma->vm_ops = _bo_vm_ops;
 
/*
 * Note: We're transferring the bo reference to
-- 
2.31.1

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] [PATCH v9 07/10] mm: Device exclusive memory access

2021-05-25 Thread Alistair Popple
On Tuesday, 25 May 2021 11:31:17 AM AEST John Hubbard wrote:
> On 5/24/21 3:11 PM, Andrew Morton wrote:
> >> ...
> >> 
> >>   Documentation/vm/hmm.rst |  17 
> >>   include/linux/mmu_notifier.h |   6 ++
> >>   include/linux/rmap.h |   4 +
> >>   include/linux/swap.h |   7 +-
> >>   include/linux/swapops.h  |  44 -
> >>   mm/hmm.c |   5 +
> >>   mm/memory.c  | 128 +++-
> >>   mm/mprotect.c|   8 ++
> >>   mm/page_vma_mapped.c |   9 +-
> >>   mm/rmap.c| 186 +++
> >>   10 files changed, 405 insertions(+), 9 deletions(-)
> > 
> > This is quite a lot of code added to core MM for a single driver.
> > 
> > Is there any expectation that other drivers will use this code?
> 
> Yes! This should work for GPUs (and potentially, other devices) that support
> OpenCL SVM atomic accesses on the device. I haven't looked into how amdgpu
> works in any detail, but that's certainly at the top of the list of likely
> additional callers.
> 
> > Is there a way of reducing the impact (code size, at least) for systems
> > which don't need this code?

All of the code added to mm/rmap.c is specific to implementing this feature 
and not depended on by other core MM code so could be put behind something 
like CONFIG_DEVICE_PRIVATE to reduce the code size impact (I realise now it 
currently isn't but should be).

The impact on compiled code size in mm/memory.c also ends up being minimised 
by the compiler because all of it is of the form:

if (is_device_exclusive_entry(...)) {
[...]
}

Meaning it should get thrown away when the feature is not configured given 
is_device_exclusive_entry() is a static inline always returning false in that 
case.

> I'll leave this question to others for the moment, in order to answer
> the "do we need it at all" points.
> 
> > How beneficial is this code to nouveau users?  I see that it permits a
> > part of OpenCL to be implemented, but how useful/important is this in
> > the real world?
> 
> So this is interesting. Right now, OpenCL support in Nouveau is rather new
> and so probably not a huge impact yet. However, we've built up enough
> experience with CUDA and OpenCL to learn that atomic operations, as part of
> the user space programming model, are a super big deal. Atomic operations
> are so useful and important that I'd expect many OpenCL SVM users to be
> uninterested in programming models that lack atomic operations for GPU
> compute programs.
> 
> Again, this doesn't rule out future, non-GPU accelerator devices that may
> come along.
> 
> Atomic ops are just a really important piece of high-end multi-threaded
> programming, it turns out. So this is the beginning of support for an
> important building block for general purpose programming on devices that
> have GPU-like memory models.
> 
> 
> thanks,




___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau