None of the DRM usage sites of temporary mappings requires the side effects of io/kmap_atomic(), i.e. preemption and pagefault disable.
Replace them with the io/kmap_local() variants, simplify the copy_to/from_user() error handling and remove the atomic variants.
Thanks,
tglx --- Documentation/driver-api/io-mapping.rst | 22 +++------- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 7 +-- drivers/gpu/drm/i915/i915_gem.c | 40 ++++++------------- drivers/gpu/drm/i915/selftests/i915_gem.c | 4 - drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 8 +-- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/fbmem.h | 8 +-- drivers/gpu/drm/qxl/qxl_image.c | 18 ++++---- drivers/gpu/drm/qxl/qxl_ioctl.c | 27 ++++++------ drivers/gpu/drm/qxl/qxl_object.c | 12 ++--- drivers/gpu/drm/qxl/qxl_object.h | 4 - drivers/gpu/drm/qxl/qxl_release.c | 4 - drivers/gpu/drm/ttm/ttm_bo_util.c | 20 +++++---- drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 30 +++++--------- include/linux/highmem-internal.h | 14 ------ include/linux/io-mapping.h | 42 -------------------- 15 files changed, 93 insertions(+), 167 deletions(-)
From: Thomas Gleixner tglx@linutronix.de
There is no reason to disable pagefaults and preemption as a side effect of kmap_atomic_prot().
Use kmap_local_page_prot() instead and document the reasoning for the mapping usage with the given pgprot.
Remove the NULL pointer check for the map. These functions return a valid address for valid pages and the return was bogus anyway as it would have left preemption and pagefaults disabled.
Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: Christian Koenig christian.koenig@amd.com Cc: Huang Rui ray.huang@amd.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/ttm/ttm_bo_util.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -181,13 +181,15 @@ static int ttm_copy_io_ttm_page(struct t return -ENOMEM;
src = (void *)((unsigned long)src + (page << PAGE_SHIFT)); - dst = kmap_atomic_prot(d, prot); - if (!dst) - return -ENOMEM; + /* + * Ensure that a highmem page is mapped with the correct + * pgprot. For non highmem the mapping is already there. + */ + dst = kmap_local_page_prot(d, prot);
memcpy_fromio(dst, src, PAGE_SIZE);
- kunmap_atomic(dst); + kunmap_local(dst);
return 0; } @@ -203,13 +205,15 @@ static int ttm_copy_ttm_io_page(struct t return -ENOMEM;
dst = (void *)((unsigned long)dst + (page << PAGE_SHIFT)); - src = kmap_atomic_prot(s, prot); - if (!src) - return -ENOMEM; + /* + * Ensure that a highmem page is mapped with the correct + * pgprot. For non highmem the mapping is already there. + */ + src = kmap_local_page_prot(s, prot);
memcpy_toio(dst, src, PAGE_SIZE);
- kunmap_atomic(src); + kunmap_local(src);
return 0; }
Am 03.03.21 um 14:20 schrieb Thomas Gleixner:
From: Thomas Gleixner tglx@linutronix.de
There is no reason to disable pagefaults and preemption as a side effect of kmap_atomic_prot().
Use kmap_local_page_prot() instead and document the reasoning for the mapping usage with the given pgprot.
Remove the NULL pointer check for the map. These functions return a valid address for valid pages and the return was bogus anyway as it would have left preemption and pagefaults disabled.
Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: Christian Koenig christian.koenig@amd.com Cc: Huang Rui ray.huang@amd.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: dri-devel@lists.freedesktop.org
drivers/gpu/drm/ttm/ttm_bo_util.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -181,13 +181,15 @@ static int ttm_copy_io_ttm_page(struct t return -ENOMEM;
src = (void *)((unsigned long)src + (page << PAGE_SHIFT));
- dst = kmap_atomic_prot(d, prot);
- if (!dst)
return -ENOMEM;
- /*
* Ensure that a highmem page is mapped with the correct* pgprot. For non highmem the mapping is already there.*/
I find the comment a bit misleading. Maybe write:
/* * Locally map highmem pages with the correct pgprot. * Normal memory should already have the correct pgprot in the linear mapping. */
Apart from that looks good to me.
Regards, Christian.
dst = kmap_local_page_prot(d, prot);
memcpy_fromio(dst, src, PAGE_SIZE);
- kunmap_atomic(dst);
kunmap_local(dst);
return 0; }
@@ -203,13 +205,15 @@ static int ttm_copy_ttm_io_page(struct t return -ENOMEM;
dst = (void *)((unsigned long)dst + (page << PAGE_SHIFT));
- src = kmap_atomic_prot(s, prot);
- if (!src)
return -ENOMEM;
/*
* Ensure that a highmem page is mapped with the correct* pgprot. For non highmem the mapping is already there.*/src = kmap_local_page_prot(s, prot);
memcpy_toio(dst, src, PAGE_SIZE);
- kunmap_atomic(src);
kunmap_local(src);
return 0; }
From: Thomas Gleixner tglx@linutronix.de
There is no reason to disable pagefaults and preemption as a side effect of kmap_atomic_prot().
Use kmap_local_page_prot() instead and document the reasoning for the mapping usage with the given pgprot.
Remove the NULL pointer check for the map. These functions return a valid address for valid pages and the return was bogus anyway as it would have left preemption and pagefaults disabled.
Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Roland Scheidegger sroland@vmware.com Cc: Zack Rusin zackr@vmware.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c @@ -375,12 +375,12 @@ static int vmw_bo_cpu_blit_line(struct v copy_size = min_t(u32, copy_size, PAGE_SIZE - src_page_offset);
if (unmap_src) { - kunmap_atomic(d->src_addr); + kunmap_local(d->src_addr); d->src_addr = NULL; }
if (unmap_dst) { - kunmap_atomic(d->dst_addr); + kunmap_local(d->dst_addr); d->dst_addr = NULL; }
@@ -388,12 +388,8 @@ static int vmw_bo_cpu_blit_line(struct v if (WARN_ON_ONCE(dst_page >= d->dst_num_pages)) return -EINVAL;
- d->dst_addr = - kmap_atomic_prot(d->dst_pages[dst_page], - d->dst_prot); - if (!d->dst_addr) - return -ENOMEM; - + d->dst_addr = kmap_local_page_prot(d->dst_pages[dst_page], + d->dst_prot); d->mapped_dst = dst_page; }
@@ -401,12 +397,8 @@ static int vmw_bo_cpu_blit_line(struct v if (WARN_ON_ONCE(src_page >= d->src_num_pages)) return -EINVAL;
- d->src_addr = - kmap_atomic_prot(d->src_pages[src_page], - d->src_prot); - if (!d->src_addr) - return -ENOMEM; - + d->src_addr = kmap_local_page_prot(d->src_pages[src_page], + d->src_prot); d->mapped_src = src_page; } diff->do_cpy(diff, d->dst_addr + dst_page_offset, @@ -436,8 +428,10 @@ static int vmw_bo_cpu_blit_line(struct v * * Performs a CPU blit from one buffer object to another avoiding a full * bo vmap which may exhaust- or fragment vmalloc space. - * On supported architectures (x86), we're using kmap_atomic which avoids - * cross-processor TLB- and cache flushes and may, on non-HIGHMEM systems + * + * On supported architectures (x86), we're using kmap_local_prot() which + * avoids cross-processor TLB- and cache flushes. kmap_local_prot() will + * either map a highmem page with the proper pgprot on HIGHMEM=y systems or * reference already set-up mappings. * * Neither of the buffer objects may be placed in PCI memory @@ -500,9 +494,9 @@ int vmw_bo_cpu_blit(struct ttm_buffer_ob } out: if (d.src_addr) - kunmap_atomic(d.src_addr); + kunmap_local(d.src_addr); if (d.dst_addr) - kunmap_atomic(d.dst_addr); + kunmap_local(d.dst_addr);
return ret; }
On Mar 3, 2021, at 08:20, Thomas Gleixner tglx@linutronix.de wrote:
From: Thomas Gleixner tglx@linutronix.de
There is no reason to disable pagefaults and preemption as a side effect of kmap_atomic_prot().
Use kmap_local_page_prot() instead and document the reasoning for the mapping usage with the given pgprot.
Remove the NULL pointer check for the map. These functions return a valid address for valid pages and the return was bogus anyway as it would have left preemption and pagefaults disabled.
Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Roland Scheidegger sroland@vmware.com Cc: Zack Rusin zackr@vmware.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: dri-devel@lists.freedesktop.org
drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c @@ -375,12 +375,12 @@ static int vmw_bo_cpu_blit_line(struct v copy_size = min_t(u32, copy_size, PAGE_SIZE - src_page_offset);
if (unmap_src) {
kunmap_atomic(d->src_addr);
kunmap_local(d->src_addr); d->src_addr = NULL;}
if (unmap_dst) {
kunmap_atomic(d->dst_addr);
kunmap_local(d->dst_addr); d->dst_addr = NULL;@@ -388,12 +388,8 @@ static int vmw_bo_cpu_blit_line(struct v if (WARN_ON_ONCE(dst_page >= d->dst_num_pages)) return -EINVAL;
d->dst_addr =kmap_atomic_prot(d->dst_pages[dst_page],d->dst_prot);if (!d->dst_addr)return -ENOMEM;
d->dst_addr = kmap_local_page_prot(d->dst_pages[dst_page],d->dst_prot); d->mapped_dst = dst_page;@@ -401,12 +397,8 @@ static int vmw_bo_cpu_blit_line(struct v if (WARN_ON_ONCE(src_page >= d->src_num_pages)) return -EINVAL;
d->src_addr =kmap_atomic_prot(d->src_pages[src_page],d->src_prot);if (!d->src_addr)return -ENOMEM;
d->src_addr = kmap_local_page_prot(d->src_pages[src_page],d->src_prot); d->mapped_src = src_page;@@ -436,8 +428,10 @@ static int vmw_bo_cpu_blit_line(struct v
- Performs a CPU blit from one buffer object to another avoiding a full
- bo vmap which may exhaust- or fragment vmalloc space.
- On supported architectures (x86), we're using kmap_atomic which avoids
- cross-processor TLB- and cache flushes and may, on non-HIGHMEM systems
- On supported architectures (x86), we're using kmap_local_prot() which
- avoids cross-processor TLB- and cache flushes. kmap_local_prot() will
- either map a highmem page with the proper pgprot on HIGHMEM=y systems or
- reference already set-up mappings.
- Neither of the buffer objects may be placed in PCI memory
@@ -500,9 +494,9 @@ int vmw_bo_cpu_blit(struct ttm_buffer_ob } out: if (d.src_addr)
kunmap_atomic(d.src_addr);
kunmap_local(d.src_addr);
kunmap_atomic(d.dst_addr);
kunmap_local(d.dst_addr);return ret;
}
Looks good. Thanks.
Reviewed-by: Zack Rusin zackr@vmware.com
z
On 03.03.21 14:20, Thomas Gleixner wrote:
From: Thomas Gleixner tglx@linutronix.de
There is no reason to disable pagefaults and preemption as a side effect of kmap_atomic_prot().
Use kmap_local_page_prot() instead and document the reasoning for the mapping usage with the given pgprot.
Remove the NULL pointer check for the map. These functions return a valid address for valid pages and the return was bogus anyway as it would have left preemption and pagefaults disabled.
Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: VMware Graphics linux-graphics-maintainer@vmware.com Cc: Roland Scheidegger sroland@vmware.com Cc: Zack Rusin zackr@vmware.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: dri-devel@lists.freedesktop.org
drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-)
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c @@ -375,12 +375,12 @@ static int vmw_bo_cpu_blit_line(struct v copy_size = min_t(u32, copy_size, PAGE_SIZE - src_page_offset);
if (unmap_src) {
kunmap_atomic(d->src_addr);
kunmap_local(d->src_addr); d->src_addr = NULL;}
if (unmap_dst) {
kunmap_atomic(d->dst_addr);
kunmap_local(d->dst_addr); d->dst_addr = NULL;@@ -388,12 +388,8 @@ static int vmw_bo_cpu_blit_line(struct v if (WARN_ON_ONCE(dst_page >= d->dst_num_pages)) return -EINVAL;
d->dst_addr =kmap_atomic_prot(d->dst_pages[dst_page],d->dst_prot);if (!d->dst_addr)return -ENOMEM;
d->dst_addr = kmap_local_page_prot(d->dst_pages[dst_page],d->dst_prot); d->mapped_dst = dst_page;@@ -401,12 +397,8 @@ static int vmw_bo_cpu_blit_line(struct v if (WARN_ON_ONCE(src_page >= d->src_num_pages)) return -EINVAL;
d->src_addr =kmap_atomic_prot(d->src_pages[src_page],d->src_prot);if (!d->src_addr)return -ENOMEM;
d->src_addr = kmap_local_page_prot(d->src_pages[src_page],d->src_prot); d->mapped_src = src_page;@@ -436,8 +428,10 @@ static int vmw_bo_cpu_blit_line(struct v
- Performs a CPU blit from one buffer object to another avoiding a full
- bo vmap which may exhaust- or fragment vmalloc space.
- On supported architectures (x86), we're using kmap_atomic which avoids
- cross-processor TLB- and cache flushes and may, on non-HIGHMEM systems
- On supported architectures (x86), we're using kmap_local_prot() which
- avoids cross-processor TLB- and cache flushes. kmap_local_prot() will
- either map a highmem page with the proper pgprot on HIGHMEM=y systems or
- reference already set-up mappings.
- Neither of the buffer objects may be placed in PCI memory
@@ -500,9 +494,9 @@ int vmw_bo_cpu_blit(struct ttm_buffer_ob } out: if (d.src_addr)
kunmap_atomic(d.src_addr);
kunmap_local(d.src_addr);
kunmap_atomic(d.dst_addr);
kunmap_local(d.dst_addr);return ret;
}
Seems reasonable to me. Reviewed-by: Roland Scheidegger sroland@vmware.com
From: Thomas Gleixner tglx@linutronix.de
No more users.
Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: Andrew Morton akpm@linux-foundation.org Cc: linux-mm@kvack.org --- include/linux/highmem-internal.h | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)
--- a/include/linux/highmem-internal.h +++ b/include/linux/highmem-internal.h @@ -88,16 +88,11 @@ static inline void __kunmap_local(void * kunmap_local_indexed(vaddr); }
-static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot) +static inline void *kmap_atomic(struct page *page) { preempt_disable(); pagefault_disable(); - return __kmap_local_page_prot(page, prot); -} - -static inline void *kmap_atomic(struct page *page) -{ - return kmap_atomic_prot(page, kmap_prot); + return __kmap_local_page_prot(page, kmap_prot); }
static inline void *kmap_atomic_pfn(unsigned long pfn) @@ -184,11 +179,6 @@ static inline void *kmap_atomic(struct p return page_address(page); }
-static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot) -{ - return kmap_atomic(page); -} - static inline void *kmap_atomic_pfn(unsigned long pfn) { return kmap_atomic(pfn_to_page(pfn));
From: Thomas Gleixner tglx@linutronix.de
None of these mapping requires the side effect of disabling pagefaults and preemption.
Use io_mapping_map_local_wc() instead, rename the related functions accordingly and clean up qxl_process_single_command() to use a plain copy_from_user() as the local maps are not disabling pagefaults.
Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: David Airlie airlied@linux.ie Cc: Gerd Hoffmann kraxel@redhat.com Cc: Daniel Vetter daniel@ffwll.ch Cc: virtualization@lists.linux-foundation.org Cc: spice-devel@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/qxl/qxl_image.c | 18 +++++++++--------- drivers/gpu/drm/qxl/qxl_ioctl.c | 27 +++++++++++++-------------- drivers/gpu/drm/qxl/qxl_object.c | 12 ++++++------ drivers/gpu/drm/qxl/qxl_object.h | 4 ++-- drivers/gpu/drm/qxl/qxl_release.c | 4 ++-- 5 files changed, 32 insertions(+), 33 deletions(-)
--- a/drivers/gpu/drm/qxl/qxl_image.c +++ b/drivers/gpu/drm/qxl/qxl_image.c @@ -124,12 +124,12 @@ qxl_image_init_helper(struct qxl_device wrong (check the bitmaps are sent correctly first) */
- ptr = qxl_bo_kmap_atomic_page(qdev, chunk_bo, 0); + ptr = qxl_bo_kmap_local_page(qdev, chunk_bo, 0); chunk = ptr; chunk->data_size = height * chunk_stride; chunk->prev_chunk = 0; chunk->next_chunk = 0; - qxl_bo_kunmap_atomic_page(qdev, chunk_bo, ptr); + qxl_bo_kunmap_local_page(qdev, chunk_bo, ptr);
{ void *k_data, *i_data; @@ -143,7 +143,7 @@ qxl_image_init_helper(struct qxl_device i_data = (void *)data;
while (remain > 0) { - ptr = qxl_bo_kmap_atomic_page(qdev, chunk_bo, page << PAGE_SHIFT); + ptr = qxl_bo_kmap_local_page(qdev, chunk_bo, page << PAGE_SHIFT);
if (page == 0) { chunk = ptr; @@ -157,7 +157,7 @@ qxl_image_init_helper(struct qxl_device
memcpy(k_data, i_data, size);
- qxl_bo_kunmap_atomic_page(qdev, chunk_bo, ptr); + qxl_bo_kunmap_local_page(qdev, chunk_bo, ptr); i_data += size; remain -= size; page++; @@ -175,10 +175,10 @@ qxl_image_init_helper(struct qxl_device page_offset = offset_in_page(out_offset); size = min((int)(PAGE_SIZE - page_offset), remain);
- ptr = qxl_bo_kmap_atomic_page(qdev, chunk_bo, page_base); + ptr = qxl_bo_kmap_local_page(qdev, chunk_bo, page_base); k_data = ptr + page_offset; memcpy(k_data, i_data, size); - qxl_bo_kunmap_atomic_page(qdev, chunk_bo, ptr); + qxl_bo_kunmap_local_page(qdev, chunk_bo, ptr); remain -= size; i_data += size; out_offset += size; @@ -189,7 +189,7 @@ qxl_image_init_helper(struct qxl_device qxl_bo_kunmap(chunk_bo);
image_bo = dimage->bo; - ptr = qxl_bo_kmap_atomic_page(qdev, image_bo, 0); + ptr = qxl_bo_kmap_local_page(qdev, image_bo, 0); image = ptr;
image->descriptor.id = 0; @@ -212,7 +212,7 @@ qxl_image_init_helper(struct qxl_device break; default: DRM_ERROR("unsupported image bit depth\n"); - qxl_bo_kunmap_atomic_page(qdev, image_bo, ptr); + qxl_bo_kunmap_local_page(qdev, image_bo, ptr); return -EINVAL; } image->u.bitmap.flags = QXL_BITMAP_TOP_DOWN; @@ -222,7 +222,7 @@ qxl_image_init_helper(struct qxl_device image->u.bitmap.palette = 0; image->u.bitmap.data = qxl_bo_physical_address(qdev, chunk_bo, 0);
- qxl_bo_kunmap_atomic_page(qdev, image_bo, ptr); + qxl_bo_kunmap_local_page(qdev, image_bo, ptr);
return 0; } --- a/drivers/gpu/drm/qxl/qxl_ioctl.c +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c @@ -89,11 +89,11 @@ apply_reloc(struct qxl_device *qdev, str { void *reloc_page;
- reloc_page = qxl_bo_kmap_atomic_page(qdev, info->dst_bo, info->dst_offset & PAGE_MASK); + reloc_page = qxl_bo_kmap_local_page(qdev, info->dst_bo, info->dst_offset & PAGE_MASK); *(uint64_t *)(reloc_page + (info->dst_offset & ~PAGE_MASK)) = qxl_bo_physical_address(qdev, info->src_bo, info->src_offset); - qxl_bo_kunmap_atomic_page(qdev, info->dst_bo, reloc_page); + qxl_bo_kunmap_local_page(qdev, info->dst_bo, reloc_page); }
static void @@ -105,9 +105,9 @@ apply_surf_reloc(struct qxl_device *qdev if (info->src_bo && !info->src_bo->is_primary) id = info->src_bo->surface_id;
- reloc_page = qxl_bo_kmap_atomic_page(qdev, info->dst_bo, info->dst_offset & PAGE_MASK); + reloc_page = qxl_bo_kmap_local_page(qdev, info->dst_bo, info->dst_offset & PAGE_MASK); *(uint32_t *)(reloc_page + (info->dst_offset & ~PAGE_MASK)) = id; - qxl_bo_kunmap_atomic_page(qdev, info->dst_bo, reloc_page); + qxl_bo_kunmap_local_page(qdev, info->dst_bo, reloc_page); }
/* return holding the reference to this object */ @@ -149,7 +149,6 @@ static int qxl_process_single_command(st struct qxl_bo *cmd_bo; void *fb_cmd; int i, ret, num_relocs; - int unwritten;
switch (cmd->type) { case QXL_CMD_DRAW: @@ -184,21 +183,21 @@ static int qxl_process_single_command(st goto out_free_reloc;
/* TODO copy slow path code from i915 */ - fb_cmd = qxl_bo_kmap_atomic_page(qdev, cmd_bo, (release->release_offset & PAGE_MASK)); - unwritten = __copy_from_user_inatomic_nocache - (fb_cmd + sizeof(union qxl_release_info) + (release->release_offset & ~PAGE_MASK), - u64_to_user_ptr(cmd->command), cmd->command_size); + fb_cmd = qxl_bo_kmap_local_page(qdev, cmd_bo, (release->release_offset & PAGE_MASK));
- { + if (copy_from_user(fb_cmd + sizeof(union qxl_release_info) + + (release->release_offset & ~PAGE_MASK), + u64_to_user_ptr(cmd->command), cmd->command_size)) { + ret = -EFAULT; + } else { struct qxl_drawable *draw = fb_cmd;
draw->mm_time = qdev->rom->mm_clock; }
- qxl_bo_kunmap_atomic_page(qdev, cmd_bo, fb_cmd); - if (unwritten) { - DRM_ERROR("got unwritten %d\n", unwritten); - ret = -EFAULT; + qxl_bo_kunmap_local_page(qdev, cmd_bo, fb_cmd); + if (ret) { + DRM_ERROR("copy from user failed %d\n", ret); goto out_free_release; }
--- a/drivers/gpu/drm/qxl/qxl_object.c +++ b/drivers/gpu/drm/qxl/qxl_object.c @@ -178,8 +178,8 @@ int qxl_bo_kmap(struct qxl_bo *bo, struc return 0; }
-void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, - struct qxl_bo *bo, int page_offset) +void *qxl_bo_kmap_local_page(struct qxl_device *qdev, + struct qxl_bo *bo, int page_offset) { unsigned long offset; void *rptr; @@ -195,7 +195,7 @@ void *qxl_bo_kmap_atomic_page(struct qxl goto fallback;
offset = bo->tbo.mem.start << PAGE_SHIFT; - return io_mapping_map_atomic_wc(map, offset + page_offset); + return io_mapping_map_local_wc(map, offset + page_offset); fallback: if (bo->kptr) { rptr = bo->kptr + (page_offset * PAGE_SIZE); @@ -222,14 +222,14 @@ void qxl_bo_kunmap(struct qxl_bo *bo) ttm_bo_vunmap(&bo->tbo, &bo->map); }
-void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev, - struct qxl_bo *bo, void *pmap) +void qxl_bo_kunmap_local_page(struct qxl_device *qdev, + struct qxl_bo *bo, void *pmap) { if ((bo->tbo.mem.mem_type != TTM_PL_VRAM) && (bo->tbo.mem.mem_type != TTM_PL_PRIV)) goto fallback;
- io_mapping_unmap_atomic(pmap); + io_mapping_unmap_local(pmap); return; fallback: qxl_bo_kunmap(bo); --- a/drivers/gpu/drm/qxl/qxl_object.h +++ b/drivers/gpu/drm/qxl/qxl_object.h @@ -65,8 +65,8 @@ extern int qxl_bo_create(struct qxl_devi struct qxl_bo **bo_ptr); extern int qxl_bo_kmap(struct qxl_bo *bo, struct dma_buf_map *map); extern void qxl_bo_kunmap(struct qxl_bo *bo); -void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, int page_offset); -void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, void *map); +void *qxl_bo_kmap_local_page(struct qxl_device *qdev, struct qxl_bo *bo, int page_offset); +void qxl_bo_kunmap_local_page(struct qxl_device *qdev, struct qxl_bo *bo, void *map); extern struct qxl_bo *qxl_bo_ref(struct qxl_bo *bo); extern void qxl_bo_unref(struct qxl_bo **bo); extern int qxl_bo_pin(struct qxl_bo *bo); --- a/drivers/gpu/drm/qxl/qxl_release.c +++ b/drivers/gpu/drm/qxl/qxl_release.c @@ -408,7 +408,7 @@ union qxl_release_info *qxl_release_map( union qxl_release_info *info; struct qxl_bo *bo = release->release_bo;
- ptr = qxl_bo_kmap_atomic_page(qdev, bo, release->release_offset & PAGE_MASK); + ptr = qxl_bo_kmap_local_page(qdev, bo, release->release_offset & PAGE_MASK); if (!ptr) return NULL; info = ptr + (release->release_offset & ~PAGE_MASK); @@ -423,7 +423,7 @@ void qxl_release_unmap(struct qxl_device void *ptr;
ptr = ((void *)info) - (release->release_offset & ~PAGE_MASK); - qxl_bo_kunmap_atomic_page(qdev, bo, ptr); + qxl_bo_kunmap_local_page(qdev, bo, ptr); }
void qxl_release_fence_buffer_objects(struct qxl_release *release)
From: Thomas Gleixner tglx@linutronix.de
Neither fbmem_peek() nor fbmem_poke() require to disable pagefaults and preemption as a side effect of io_mapping_map_atomic_wc().
Use io_mapping_map_local_wc() instead.
Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: Ben Skeggs bskeggs@redhat.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: nouveau@lists.freedesktop.org --- drivers/gpu/drm/nouveau/nvkm/subdev/devinit/fbmem.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/fbmem.h +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/devinit/fbmem.h @@ -60,19 +60,19 @@ fbmem_fini(struct io_mapping *fb) static inline u32 fbmem_peek(struct io_mapping *fb, u32 off) { - u8 __iomem *p = io_mapping_map_atomic_wc(fb, off & PAGE_MASK); + u8 __iomem *p = io_mapping_map_local_wc(fb, off & PAGE_MASK); u32 val = ioread32(p + (off & ~PAGE_MASK)); - io_mapping_unmap_atomic(p); + io_mapping_unmap_local(p); return val; }
static inline void fbmem_poke(struct io_mapping *fb, u32 off, u32 val) { - u8 __iomem *p = io_mapping_map_atomic_wc(fb, off & PAGE_MASK); + u8 __iomem *p = io_mapping_map_local_wc(fb, off & PAGE_MASK); iowrite32(val, p + (off & ~PAGE_MASK)); wmb(); - io_mapping_unmap_atomic(p); + io_mapping_unmap_local(p); }
static inline bool
From: Thomas Gleixner tglx@linutronix.de
None of these mapping requires the side effect of disabling pagefaults and preemption.
Use io_mapping_map_local_wc() instead, and clean up gtt_user_read() and gtt_user_write() to use a plain copy_from_user() as the local maps are not disabling pagefaults.
Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: Chris Wilson chris@chris-wilson.co.uk Cc: intel-gfx@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 7 +--- drivers/gpu/drm/i915/i915_gem.c | 40 ++++++++----------------- drivers/gpu/drm/i915/selftests/i915_gem.c | 4 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 8 ++--- 4 files changed, 22 insertions(+), 37 deletions(-)
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -1080,7 +1080,7 @@ static void reloc_cache_reset(struct rel struct i915_ggtt *ggtt = cache_to_ggtt(cache);
intel_gt_flush_ggtt_writes(ggtt->vm.gt); - io_mapping_unmap_atomic((void __iomem *)vaddr); + io_mapping_unmap_local((void __iomem *)vaddr);
if (drm_mm_node_allocated(&cache->node)) { ggtt->vm.clear_range(&ggtt->vm, @@ -1146,7 +1146,7 @@ static void *reloc_iomap(struct drm_i915
if (cache->vaddr) { intel_gt_flush_ggtt_writes(ggtt->vm.gt); - io_mapping_unmap_atomic((void __force __iomem *) unmask_page(cache->vaddr)); + io_mapping_unmap_local((void __force __iomem *) unmask_page(cache->vaddr)); } else { struct i915_vma *vma; int err; @@ -1194,8 +1194,7 @@ static void *reloc_iomap(struct drm_i915 offset += page << PAGE_SHIFT; }
- vaddr = (void __force *)io_mapping_map_atomic_wc(&ggtt->iomap, - offset); + vaddr = (void __force *)io_mapping_map_local_wc(&ggtt->iomap, offset); cache->page = page; cache->vaddr = (unsigned long)vaddr;
--- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -253,22 +253,15 @@ gtt_user_read(struct io_mapping *mapping char __user *user_data, int length) { void __iomem *vaddr; - unsigned long unwritten; + bool fail = false;
/* We can use the cpu mem copy function because this is X86. */ - vaddr = io_mapping_map_atomic_wc(mapping, base); - unwritten = __copy_to_user_inatomic(user_data, - (void __force *)vaddr + offset, - length); - io_mapping_unmap_atomic(vaddr); - if (unwritten) { - vaddr = io_mapping_map_wc(mapping, base, PAGE_SIZE); - unwritten = copy_to_user(user_data, - (void __force *)vaddr + offset, - length); - io_mapping_unmap(vaddr); - } - return unwritten; + vaddr = io_mapping_map_local_wc(mapping, base); + if (copy_to_user(user_data, (void __force *)vaddr + offset, length)) + fail = true; + io_mapping_unmap_local(vaddr); + + return fail; }
static int @@ -437,21 +430,14 @@ ggtt_write(struct io_mapping *mapping, char __user *user_data, int length) { void __iomem *vaddr; - unsigned long unwritten; + bool fail = false;
/* We can use the cpu mem copy function because this is X86. */ - vaddr = io_mapping_map_atomic_wc(mapping, base); - unwritten = __copy_from_user_inatomic_nocache((void __force *)vaddr + offset, - user_data, length); - io_mapping_unmap_atomic(vaddr); - if (unwritten) { - vaddr = io_mapping_map_wc(mapping, base, PAGE_SIZE); - unwritten = copy_from_user((void __force *)vaddr + offset, - user_data, length); - io_mapping_unmap(vaddr); - } - - return unwritten; + vaddr = io_mapping_map_local_wc(mapping, base); + if (copy_from_user((void __force *)vaddr + offset, user_data, length)) + fail = true; + io_mapping_unmap_local(vaddr); + return fail; }
/** --- a/drivers/gpu/drm/i915/selftests/i915_gem.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c @@ -58,12 +58,12 @@ static void trash_stolen(struct drm_i915
ggtt->vm.insert_page(&ggtt->vm, dma, slot, I915_CACHE_NONE, 0);
- s = io_mapping_map_atomic_wc(&ggtt->iomap, slot); + s = io_mapping_map_local_wc(&ggtt->iomap, slot); for (x = 0; x < PAGE_SIZE / sizeof(u32); x++) { prng = next_pseudo_random32(prng); iowrite32(prng, &s[x]); } - io_mapping_unmap_atomic(s); + io_mapping_unmap_local(s); }
ggtt->vm.clear_range(&ggtt->vm, slot, PAGE_SIZE); --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -1201,9 +1201,9 @@ static int igt_ggtt_page(void *arg) u64 offset = tmp.start + order[n] * PAGE_SIZE; u32 __iomem *vaddr;
- vaddr = io_mapping_map_atomic_wc(&ggtt->iomap, offset); + vaddr = io_mapping_map_local_wc(&ggtt->iomap, offset); iowrite32(n, vaddr + n); - io_mapping_unmap_atomic(vaddr); + io_mapping_unmap_local(vaddr); } intel_gt_flush_ggtt_writes(ggtt->vm.gt);
@@ -1213,9 +1213,9 @@ static int igt_ggtt_page(void *arg) u32 __iomem *vaddr; u32 val;
- vaddr = io_mapping_map_atomic_wc(&ggtt->iomap, offset); + vaddr = io_mapping_map_local_wc(&ggtt->iomap, offset); val = ioread32(vaddr + n); - io_mapping_unmap_atomic(vaddr); + io_mapping_unmap_local(vaddr);
if (val != n) { pr_err("insert page failed: found %d, expected %d\n",
From: Thomas Gleixner tglx@linutronix.de
No more users. Get rid of it and remove the traces in documentation.
Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: Andrew Morton akpm@linux-foundation.org Cc: linux-mm@kvack.org --- Documentation/driver-api/io-mapping.rst | 22 +++++----------- include/linux/io-mapping.h | 42 +------------------------------- 2 files changed, 9 insertions(+), 55 deletions(-)
--- a/Documentation/driver-api/io-mapping.rst +++ b/Documentation/driver-api/io-mapping.rst @@ -21,19 +21,15 @@ mappable, while 'size' indicates how lar enable. Both are in bytes.
This _wc variant provides a mapping which may only be used with -io_mapping_map_atomic_wc(), io_mapping_map_local_wc() or -io_mapping_map_wc(). +io_mapping_map_local_wc() or io_mapping_map_wc().
With this mapping object, individual pages can be mapped either temporarily or long term, depending on the requirements. Of course, temporary maps are -more efficient. They come in two flavours:: +more efficient.
void *io_mapping_map_local_wc(struct io_mapping *mapping, unsigned long offset)
- void *io_mapping_map_atomic_wc(struct io_mapping *mapping, - unsigned long offset) - 'offset' is the offset within the defined mapping region. Accessing addresses beyond the region specified in the creation function yields undefined results. Using an offset which is not page aligned yields an @@ -50,9 +46,6 @@ io_mapping_map_local_wc() has a side eff migration to make the mapping code work. No caller can rely on this side effect.
-io_mapping_map_atomic_wc() has the side effect of disabling preemption and -pagefaults. Don't use in new code. Use io_mapping_map_local_wc() instead. - Nested mappings need to be undone in reverse order because the mapping code uses a stack for keeping track of them::
@@ -65,11 +58,10 @@ Nested mappings need to be undone in rev The mappings are released with::
void io_mapping_unmap_local(void *vaddr) - void io_mapping_unmap_atomic(void *vaddr)
-'vaddr' must be the value returned by the last io_mapping_map_local_wc() or -io_mapping_map_atomic_wc() call. This unmaps the specified mapping and -undoes the side effects of the mapping functions. +'vaddr' must be the value returned by the last io_mapping_map_local_wc() +call. This unmaps the specified mapping and undoes eventual side effects of +the mapping function.
If you need to sleep while holding a mapping, you can use the regular variant, although this may be significantly slower:: @@ -77,8 +69,8 @@ If you need to sleep while holding a map void *io_mapping_map_wc(struct io_mapping *mapping, unsigned long offset)
-This works like io_mapping_map_atomic/local_wc() except it has no side -effects and the pointer is globaly visible. +This works like io_mapping_map_local_wc() except it has no side effects and +the pointer is globaly visible.
The mappings are released with::
--- a/include/linux/io-mapping.h +++ b/include/linux/io-mapping.h @@ -60,28 +60,7 @@ io_mapping_fini(struct io_mapping *mappi iomap_free(mapping->base, mapping->size); }
-/* Atomic map/unmap */ -static inline void __iomem * -io_mapping_map_atomic_wc(struct io_mapping *mapping, - unsigned long offset) -{ - resource_size_t phys_addr; - - BUG_ON(offset >= mapping->size); - phys_addr = mapping->base + offset; - preempt_disable(); - pagefault_disable(); - return __iomap_local_pfn_prot(PHYS_PFN(phys_addr), mapping->prot); -} - -static inline void -io_mapping_unmap_atomic(void __iomem *vaddr) -{ - kunmap_local_indexed((void __force *)vaddr); - pagefault_enable(); - preempt_enable(); -} - +/* Temporary mappings which are only valid in the current context */ static inline void __iomem * io_mapping_map_local_wc(struct io_mapping *mapping, unsigned long offset) { @@ -163,24 +142,7 @@ io_mapping_unmap(void __iomem *vaddr) { }
-/* Atomic map/unmap */ -static inline void __iomem * -io_mapping_map_atomic_wc(struct io_mapping *mapping, - unsigned long offset) -{ - preempt_disable(); - pagefault_disable(); - return io_mapping_map_wc(mapping, offset, PAGE_SIZE); -} - -static inline void -io_mapping_unmap_atomic(void __iomem *vaddr) -{ - io_mapping_unmap(vaddr); - pagefault_enable(); - preempt_enable(); -} - +/* Temporary mappings which are only valid in the current context */ static inline void __iomem * io_mapping_map_local_wc(struct io_mapping *mapping, unsigned long offset) {
dri-devel@lists.freedesktop.org
-
Christian König -
Roland Scheidegger -
Thomas Gleixner -
Zack Rusin