From efe2bf965522bf0796d413b47a2abbf81d471d6f Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Mon, 29 Apr 2019 15:08:23 -0700 Subject: drm/virtio: set seqno for dma-fence This is motivated by having meaningful ftrace events, but it also fixes use cases where dma_fence_is_later is called, such as in sync_file_merge. In other drivers, fence creation and cmdbuf submission normally happen atomically, mutex_lock(); fence = dma_fence_create(..., ++timeline->seqno); submit_cmdbuf(); mutex_unlock(); and have no such issue. But in our driver, because most ioctls queue commands into ctrlq, we do not want to grab a lock. Instead, we set seqno to 0 when a fence is created, and update it when the command is finally queued and the seqno is known. Signed-off-by: Chia-I Wu Reviewed-by: Emil Velikov Link: http://patchwork.freedesktop.org/patch/msgid/20190429220825.156644-1-olvaffe@gmail.com Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h | 1 - drivers/gpu/drm/virtio/virtgpu_fence.c | 17 ++++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 491dec0712b3..90461feeafdb 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -102,7 +102,6 @@ struct virtio_gpu_fence { struct dma_fence f; struct virtio_gpu_fence_driver *drv; struct list_head node; - uint64_t seq; }; #define to_virtio_fence(x) \ container_of(x, struct virtio_gpu_fence, f) diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index 87d1966192f4..72b4f7561432 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -40,16 +40,14 @@ bool virtio_fence_signaled(struct dma_fence *f) { struct virtio_gpu_fence *fence = to_virtio_fence(f); - if (atomic64_read(&fence->drv->last_seq) >= fence->seq) + if (atomic64_read(&fence->drv->last_seq) >= fence->f.seqno) return true; return false; } static void virtio_fence_value_str(struct dma_fence *f, char *str, int size) { - struct virtio_gpu_fence *fence = to_virtio_fence(f); - - snprintf(str, size, "%llu", fence->seq); + snprintf(str, size, "%llu", f->seqno); } static void virtio_timeline_value_str(struct dma_fence *f, char *str, int size) @@ -76,6 +74,11 @@ struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev) return fence; fence->drv = drv; + + /* This only partially initializes the fence because the seqno is + * unknown yet. The fence must not be used outside of the driver + * until virtio_gpu_fence_emit is called. + */ dma_fence_init(&fence->f, &virtio_fence_ops, &drv->lock, drv->context, 0); return fence; @@ -89,13 +92,13 @@ int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, unsigned long irq_flags; spin_lock_irqsave(&drv->lock, irq_flags); - fence->seq = ++drv->sync_seq; + fence->f.seqno = ++drv->sync_seq; dma_fence_get(&fence->f); list_add_tail(&fence->node, &drv->fences); spin_unlock_irqrestore(&drv->lock, irq_flags); cmd_hdr->flags |= cpu_to_le32(VIRTIO_GPU_FLAG_FENCE); - cmd_hdr->fence_id = cpu_to_le64(fence->seq); + cmd_hdr->fence_id = cpu_to_le64(fence->f.seqno); return 0; } @@ -109,7 +112,7 @@ void virtio_gpu_fence_event_process(struct virtio_gpu_device *vgdev, spin_lock_irqsave(&drv->lock, irq_flags); atomic64_set(&vgdev->fence_drv.last_seq, last_seq); list_for_each_entry_safe(fence, tmp, &drv->fences, node) { - if (last_seq < fence->seq) + if (last_seq < fence->f.seqno) continue; dma_fence_signal_locked(&fence->f); list_del(&fence->node); -- cgit v1.3.1 From 48ad7751dbd1facd5500c4718698aec550aaea6e Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Mon, 29 Apr 2019 15:08:24 -0700 Subject: drm/virtio: trace drm_fence_emit For most drivers, drm_fence_init is followed by drm_fence_emit immediately. But for our driver, they are done separately. We also don't know the fence seqno until drm_fence_emit. Signed-off-by: Chia-I Wu Link: http://patchwork.freedesktop.org/patch/msgid/20190429220825.156644-2-olvaffe@gmail.com Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_fence.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index 72b4f7561432..e22918ca621b 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -24,6 +24,7 @@ */ #include +#include #include "virtgpu_drv.h" static const char *virtio_get_driver_name(struct dma_fence *f) @@ -97,6 +98,8 @@ int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, list_add_tail(&fence->node, &drv->fences); spin_unlock_irqrestore(&drv->lock, irq_flags); + trace_dma_fence_emit(&fence->f); + cmd_hdr->flags |= cpu_to_le32(VIRTIO_GPU_FLAG_FENCE); cmd_hdr->fence_id = cpu_to_le64(fence->f.seqno); return 0; -- cgit v1.3.1 From 5daf8857c9909903348a1559586be8a09564089e Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Mon, 29 Apr 2019 15:08:25 -0700 Subject: drm/virtio: add trace events for commands Trace when commands are queued for both ctrlq and cursorq. Trace when responses are received for ctrlq. Signed-off-by: Chia-I Wu Link: http://patchwork.freedesktop.org/patch/msgid/20190429220825.156644-3-olvaffe@gmail.com Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/Makefile | 2 +- drivers/gpu/drm/virtio/virtgpu_trace.h | 52 +++++++++++++++++++++++++++ drivers/gpu/drm/virtio/virtgpu_trace_points.c | 5 +++ drivers/gpu/drm/virtio/virtgpu_vq.c | 10 ++++++ 4 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/virtio/virtgpu_trace.h create mode 100644 drivers/gpu/drm/virtio/virtgpu_trace_points.c (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile index 4e90cc8fa651..42949a17ff70 100644 --- a/drivers/gpu/drm/virtio/Makefile +++ b/drivers/gpu/drm/virtio/Makefile @@ -6,6 +6,6 @@ virtio-gpu-y := virtgpu_drv.o virtgpu_kms.o virtgpu_gem.o \ virtgpu_fb.o virtgpu_display.o virtgpu_vq.o virtgpu_ttm.o \ virtgpu_fence.o virtgpu_object.o virtgpu_debugfs.o virtgpu_plane.o \ - virtgpu_ioctl.o virtgpu_prime.o + virtgpu_ioctl.o virtgpu_prime.o virtgpu_trace_points.o obj-$(CONFIG_DRM_VIRTIO_GPU) += virtio-gpu.o diff --git a/drivers/gpu/drm/virtio/virtgpu_trace.h b/drivers/gpu/drm/virtio/virtgpu_trace.h new file mode 100644 index 000000000000..711ecc2bd241 --- /dev/null +++ b/drivers/gpu/drm/virtio/virtgpu_trace.h @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#if !defined(_VIRTGPU_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ) +#define _VIRTGPU_TRACE_H_ + +#include + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM virtio_gpu +#define TRACE_INCLUDE_FILE virtgpu_trace + +DECLARE_EVENT_CLASS(virtio_gpu_cmd, + TP_PROTO(struct virtqueue *vq, struct virtio_gpu_ctrl_hdr *hdr), + TP_ARGS(vq, hdr), + TP_STRUCT__entry( + __field(int, dev) + __field(unsigned int, vq) + __field(const char *, name) + __field(u32, type) + __field(u32, flags) + __field(u64, fence_id) + __field(u32, ctx_id) + ), + TP_fast_assign( + __entry->dev = vq->vdev->index; + __entry->vq = vq->index; + __entry->name = vq->name; + __entry->type = le32_to_cpu(hdr->type); + __entry->flags = le32_to_cpu(hdr->flags); + __entry->fence_id = le64_to_cpu(hdr->fence_id); + __entry->ctx_id = le32_to_cpu(hdr->ctx_id); + ), + TP_printk("vdev=%d vq=%u name=%s type=0x%x flags=0x%x fence_id=%llu ctx_id=%u", + __entry->dev, __entry->vq, __entry->name, + __entry->type, __entry->flags, __entry->fence_id, + __entry->ctx_id) +); + +DEFINE_EVENT(virtio_gpu_cmd, virtio_gpu_cmd_queue, + TP_PROTO(struct virtqueue *vq, struct virtio_gpu_ctrl_hdr *hdr), + TP_ARGS(vq, hdr) +); + +DEFINE_EVENT(virtio_gpu_cmd, virtio_gpu_cmd_response, + TP_PROTO(struct virtqueue *vq, struct virtio_gpu_ctrl_hdr *hdr), + TP_ARGS(vq, hdr) +); + +#endif + +#undef TRACE_INCLUDE_PATH +#define TRACE_INCLUDE_PATH ../../drivers/gpu/drm/virtio +#include diff --git a/drivers/gpu/drm/virtio/virtgpu_trace_points.c b/drivers/gpu/drm/virtio/virtgpu_trace_points.c new file mode 100644 index 000000000000..1970cb6f24ef --- /dev/null +++ b/drivers/gpu/drm/virtio/virtgpu_trace_points.c @@ -0,0 +1,5 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "virtgpu_drv.h" + +#define CREATE_TRACE_POINTS +#include "virtgpu_trace.h" diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index e62fe24b1a2e..2c5eeccb88c0 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -28,6 +28,7 @@ #include #include "virtgpu_drv.h" +#include "virtgpu_trace.h" #include #include #include @@ -192,6 +193,9 @@ void virtio_gpu_dequeue_ctrl_func(struct work_struct *work) list_for_each_entry_safe(entry, tmp, &reclaim_list, list) { resp = (struct virtio_gpu_ctrl_hdr *)entry->resp_buf; + + trace_virtio_gpu_cmd_response(vgdev->ctrlq.vq, resp); + if (resp->type != cpu_to_le32(VIRTIO_GPU_RESP_OK_NODATA)) { if (resp->type >= cpu_to_le32(VIRTIO_GPU_RESP_ERR_UNSPEC)) { struct virtio_gpu_ctrl_hdr *cmd; @@ -284,6 +288,9 @@ retry: spin_lock(&vgdev->ctrlq.qlock); goto retry; } else { + trace_virtio_gpu_cmd_queue(vq, + (struct virtio_gpu_ctrl_hdr *)vbuf->buf); + virtqueue_kick(vq); } @@ -359,6 +366,9 @@ retry: spin_lock(&vgdev->cursorq.qlock); goto retry; } else { + trace_virtio_gpu_cmd_queue(vq, + (struct virtio_gpu_ctrl_hdr *)vbuf->buf); + virtqueue_kick(vq); } -- cgit v1.3.1 From 4d8979b3a6966e8d4d600f1783b58e77c2dbc0ff Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Mon, 29 Apr 2019 15:10:21 -0700 Subject: drm/virtio: allocate fences with GFP_KERNEL It was changed to GFP_ATOMIC in commit ec2f0577c (add & use virtio_gpu_queue_fenced_ctrl_buffer) because the allocation happened with a spinlock held. That was no longer true after commit 9fdd90c0f (add virtio_gpu_alloc_fence()). Signed-off-by: Chia-I Wu Reviewed-by: Robert Foss Reviewed-by: Emil Velikov Link: http://patchwork.freedesktop.org/patch/msgid/20190429221021.159784-1-olvaffe@gmail.com Cc: Gerd Hoffmann Cc: Gustavo Padovan Cc: Robert Foss Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_fence.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index e22918ca621b..8aecc36a0417 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -70,7 +70,7 @@ struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev) { struct virtio_gpu_fence_driver *drv = &vgdev->fence_drv; struct virtio_gpu_fence *fence = kzalloc(sizeof(struct virtio_gpu_fence), - GFP_ATOMIC); + GFP_KERNEL); if (!fence) return fence; -- cgit v1.3.1 From fa2b7c21d9b974b5b1b4f63a02edeaf049054dba Mon Sep 17 00:00:00 2001 From: Robert Foss Date: Mon, 6 May 2019 11:10:34 +0200 Subject: drm/virtio: Remove redundant return type virtio_gpu_fence_emit() always returns 0, since it has no error paths. Consequently no calls for virtio_gpu_fence_emit() use the return value, and it can be removed. Signed-off-by: Robert Foss Reviewed-by: Chia-I Wu Link: http://patchwork.freedesktop.org/patch/msgid/20190506091034.30289-1-robert.foss@collabora.com Suggested-by: Emil Velikov Reviewed-by: Chia-I Wu Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +- drivers/gpu/drm/virtio/virtgpu_fence.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 90461feeafdb..2d3e5b1debb3 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -355,7 +355,7 @@ int virtio_gpu_mmap(struct file *filp, struct vm_area_struct *vma); bool virtio_fence_signaled(struct dma_fence *f); struct virtio_gpu_fence *virtio_gpu_fence_alloc( struct virtio_gpu_device *vgdev); -int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, +void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, struct virtio_gpu_ctrl_hdr *cmd_hdr, struct virtio_gpu_fence *fence); void virtio_gpu_fence_event_process(struct virtio_gpu_device *vdev, diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index 8aecc36a0417..70d6c4329778 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -85,7 +85,7 @@ struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev) return fence; } -int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, +void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, struct virtio_gpu_ctrl_hdr *cmd_hdr, struct virtio_gpu_fence *fence) { @@ -102,7 +102,6 @@ int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, cmd_hdr->flags |= cpu_to_le32(VIRTIO_GPU_FLAG_FENCE); cmd_hdr->fence_id = cpu_to_le64(fence->f.seqno); - return 0; } void virtio_gpu_fence_event_process(struct virtio_gpu_device *vgdev, -- cgit v1.3.1 From cbb56814fbe7fedd71cd6b29dc83bab2915a5b74 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Wed, 22 May 2019 16:47:00 +0100 Subject: drm/virtio: remove irrelevant DRM_UNLOCKED flag DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it. Cc: David Airlie Cc: Gerd Hoffmann Cc: virtualization@lists.linux-foundation.org Cc: Daniel Vetter Signed-off-by: Emil Velikov Link: http://patchwork.freedesktop.org/patch/msgid/20190522154702.16269-2-emil.l.velikov@gmail.com Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 949a264985fc..b7f9dfe61d1c 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -553,34 +553,34 @@ copy_exit: struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS] = { DRM_IOCTL_DEF_DRV(VIRTGPU_MAP, virtio_gpu_map_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_EXECBUFFER, virtio_gpu_execbuffer_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_GETPARAM, virtio_gpu_getparam_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_RESOURCE_CREATE, virtio_gpu_resource_create_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_RESOURCE_INFO, virtio_gpu_resource_info_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), /* make transfer async to the main ring? - no sure, can we * thread these in the underlying GL */ DRM_IOCTL_DEF_DRV(VIRTGPU_TRANSFER_FROM_HOST, virtio_gpu_transfer_from_host_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_TRANSFER_TO_HOST, virtio_gpu_transfer_to_host_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_WAIT, virtio_gpu_wait_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_GET_CAPS, virtio_gpu_get_caps_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_AUTH | DRM_RENDER_ALLOW), }; -- cgit v1.3.1 From 6e58ab7ac7fac61acd7705a8abf1632462c1512a Mon Sep 17 00:00:00 2001 From: Christian König Date: Fri, 10 May 2019 14:15:08 +0200 Subject: drm/ttm: Make LRU removal optional v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are already doing this for DMA-buf imports and also for amdgpu VM BOs for quite a while now. If this doesn't run into any problems we are probably going to stop removing BOs from the LRU altogether. v2: drop BUG_ON from ttm_bo_add_to_lru Signed-off-by: Christian König Reviewed-by: Chunming Zhou Tested-by: Pierre-Eric Pelloux-Prayer Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 +++++---- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++-- drivers/gpu/drm/qxl/qxl_release.c | 2 +- drivers/gpu/drm/radeon/radeon_gem.c | 2 +- drivers/gpu/drm/radeon/radeon_object.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 23 ++++++++++++----------- drivers/gpu/drm/ttm/ttm_execbuf_util.c | 20 ++++++++++++-------- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 3 ++- drivers/gpu/drm/vmwgfx/vmwgfx_validation.h | 2 +- include/drm/ttm/ttm_bo_driver.h | 5 ++++- include/drm/ttm/ttm_execbuf_util.h | 3 ++- 14 files changed, 46 insertions(+), 35 deletions(-) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index e30427161295..81e0e758cc54 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -585,7 +585,7 @@ static int reserve_bo_and_vm(struct kgd_mem *mem, amdgpu_vm_get_pd_bo(vm, &ctx->list, &ctx->vm_pd[0]); ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list, - false, &ctx->duplicates); + false, &ctx->duplicates, true); if (!ret) ctx->reserved = true; else { @@ -658,7 +658,7 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem, } ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list, - false, &ctx->duplicates); + false, &ctx->duplicates, true); if (!ret) ctx->reserved = true; else @@ -1808,7 +1808,8 @@ static int validate_invalid_user_pages(struct amdkfd_process_info *process_info) } /* Reserve all BOs and page tables for validation */ - ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates); + ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates, + true); WARN(!list_empty(&duplicates), "Duplicates should be empty"); if (ret) goto out_free; @@ -2014,7 +2015,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct dma_fence **ef) } ret = ttm_eu_reserve_buffers(&ctx.ticket, &ctx.list, - false, &duplicate_save); + false, &duplicate_save, true); if (ret) { pr_debug("Memory eviction: TTM Reserve Failed. Try again\n"); goto ttm_reserve_fail; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index d72cc583ebd1..fff558cf385b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, } r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true, - &duplicates); + &duplicates, true); if (unlikely(r != 0)) { if (r != -ERESTARTSYS) DRM_ERROR("ttm_eu_reserve_buffers failed.\n"); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c index 54dd02a898b9..06f83cac0d3a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c @@ -79,7 +79,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm, list_add(&csa_tv.head, &list); amdgpu_vm_get_pd_bo(vm, &list, &pd); - r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL); + r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true); if (r) { DRM_ERROR("failed to reserve CSA,PD BOs: err=%d\n", r); return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 7b840367004c..d513a5ad03dd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -171,7 +171,7 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj, amdgpu_vm_get_pd_bo(vm, &list, &vm_pd); - r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates); + r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, true); if (r) { dev_err(adev->dev, "leaking bo va because " "we fail to reserve bo (%d)\n", r); @@ -608,7 +608,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data, amdgpu_vm_get_pd_bo(&fpriv->vm, &list, &vm_pd); - r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates); + r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates, true); if (r) goto error_unref; diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c index 30f85f0130cb..49f9a9385393 100644 --- a/drivers/gpu/drm/qxl/qxl_release.c +++ b/drivers/gpu/drm/qxl/qxl_release.c @@ -256,7 +256,7 @@ int qxl_release_reserve_list(struct qxl_release *release, bool no_intr) return 0; ret = ttm_eu_reserve_buffers(&release->ticket, &release->bos, - !no_intr, NULL); + !no_intr, NULL, true); if (ret) return ret; diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index 44617dec8183..7411e69e2712 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -559,7 +559,7 @@ static void radeon_gem_va_update_vm(struct radeon_device *rdev, if (!vm_bos) return; - r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL); + r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true); if (r) goto error_free; diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 833e909706a9..36683de0300b 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -539,7 +539,7 @@ int radeon_bo_list_validate(struct radeon_device *rdev, u64 bytes_moved_threshold = radeon_bo_get_threshold_for_moves(rdev); INIT_LIST_HEAD(&duplicates); - r = ttm_eu_reserve_buffers(ticket, head, true, &duplicates); + r = ttm_eu_reserve_buffers(ticket, head, true, &duplicates, true); if (unlikely(r != 0)) { return r; } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 2845fceb2fbd..06bbcd2679b2 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -173,19 +173,20 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo) reservation_object_assert_held(bo->resv); - if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) { - BUG_ON(!list_empty(&bo->lru)); + if (!list_empty(&bo->lru)) + return; - man = &bdev->man[bo->mem.mem_type]; - list_add_tail(&bo->lru, &man->lru[bo->priority]); - kref_get(&bo->list_kref); + if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) + return; - if (bo->ttm && !(bo->ttm->page_flags & - (TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED))) { - list_add_tail(&bo->swap, - &bdev->glob->swap_lru[bo->priority]); - kref_get(&bo->list_kref); - } + man = &bdev->man[bo->mem.mem_type]; + list_add_tail(&bo->lru, &man->lru[bo->priority]); + kref_get(&bo->list_kref); + + if (bo->ttm && !(bo->ttm->page_flags & + (TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED))) { + list_add_tail(&bo->swap, &bdev->glob->swap_lru[bo->priority]); + kref_get(&bo->list_kref); } } EXPORT_SYMBOL(ttm_bo_add_to_lru); diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c index 0075eb9a0b52..957ec375a4ba 100644 --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c @@ -69,7 +69,8 @@ void ttm_eu_backoff_reservation(struct ww_acquire_ctx *ticket, list_for_each_entry(entry, list, head) { struct ttm_buffer_object *bo = entry->bo; - ttm_bo_add_to_lru(bo); + if (list_empty(&bo->lru)) + ttm_bo_add_to_lru(bo); reservation_object_unlock(bo->resv); } spin_unlock(&glob->lru_lock); @@ -93,7 +94,7 @@ EXPORT_SYMBOL(ttm_eu_backoff_reservation); int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, struct list_head *list, bool intr, - struct list_head *dups) + struct list_head *dups, bool del_lru) { struct ttm_bo_global *glob; struct ttm_validate_buffer *entry; @@ -172,11 +173,11 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, list_add(&entry->head, list); } - if (ticket) - ww_acquire_done(ticket); - spin_lock(&glob->lru_lock); - ttm_eu_del_from_lru_locked(list); - spin_unlock(&glob->lru_lock); + if (del_lru) { + spin_lock(&glob->lru_lock); + ttm_eu_del_from_lru_locked(list); + spin_unlock(&glob->lru_lock); + } return 0; } EXPORT_SYMBOL(ttm_eu_reserve_buffers); @@ -203,7 +204,10 @@ void ttm_eu_fence_buffer_objects(struct ww_acquire_ctx *ticket, reservation_object_add_shared_fence(bo->resv, fence); else reservation_object_add_excl_fence(bo->resv, fence); - ttm_bo_add_to_lru(bo); + if (list_empty(&bo->lru)) + ttm_bo_add_to_lru(bo); + else + ttm_bo_move_to_lru_tail(bo, NULL); reservation_object_unlock(bo->resv); } spin_unlock(&glob->lru_lock); diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index b7f9dfe61d1c..fe0bd6274db6 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -63,7 +63,7 @@ int virtio_gpu_object_list_validate(struct ww_acquire_ctx *ticket, struct virtio_gpu_object *qobj; int ret; - ret = ttm_eu_reserve_buffers(ticket, head, true, NULL); + ret = ttm_eu_reserve_buffers(ticket, head, true, NULL, true); if (ret != 0) return ret; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c index 711f8fd0dd45..1d38a8b2f2ec 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c @@ -464,7 +464,8 @@ vmw_resource_check_buffer(struct ww_acquire_ctx *ticket, val_buf->bo = &res->backup->base; val_buf->num_shared = 0; list_add_tail(&val_buf->head, &val_list); - ret = ttm_eu_reserve_buffers(ticket, &val_list, interruptible, NULL); + ret = ttm_eu_reserve_buffers(ticket, &val_list, interruptible, NULL, + true); if (unlikely(ret != 0)) goto out_no_reserve; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h index 523f6ac5c335..1d2322ad6fd5 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h @@ -169,7 +169,7 @@ vmw_validation_bo_reserve(struct vmw_validation_context *ctx, bool intr) { return ttm_eu_reserve_buffers(&ctx->ticket, &ctx->bo_list, intr, - NULL); + NULL, true); } /** diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index 129dabbc002d..9f54cf9c60df 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -769,7 +769,10 @@ static inline void ttm_bo_unreserve(struct ttm_buffer_object *bo) { if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) { spin_lock(&bo->bdev->glob->lru_lock); - ttm_bo_add_to_lru(bo); + if (list_empty(&bo->lru)) + ttm_bo_add_to_lru(bo); + else + ttm_bo_move_to_lru_tail(bo, NULL); spin_unlock(&bo->bdev->glob->lru_lock); } reservation_object_unlock(bo->resv); diff --git a/include/drm/ttm/ttm_execbuf_util.h b/include/drm/ttm/ttm_execbuf_util.h index 621615fa7728..7e46cc678e7e 100644 --- a/include/drm/ttm/ttm_execbuf_util.h +++ b/include/drm/ttm/ttm_execbuf_util.h @@ -70,6 +70,7 @@ extern void ttm_eu_backoff_reservation(struct ww_acquire_ctx *ticket, * @list: thread private list of ttm_validate_buffer structs. * @intr: should the wait be interruptible * @dups: [out] optional list of duplicates. + * @del_lru: true if BOs should be removed from the LRU. * * Tries to reserve bos pointed to by the list entries for validation. * If the function returns 0, all buffers are marked as "unfenced", @@ -98,7 +99,7 @@ extern void ttm_eu_backoff_reservation(struct ww_acquire_ctx *ticket, extern int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, struct list_head *list, bool intr, - struct list_head *dups); + struct list_head *dups, bool del_lru); /** * function ttm_eu_fence_buffer_objects. -- cgit v1.3.1 From 4b013bb2d3a4a3104360746d2326d1408d179e72 Mon Sep 17 00:00:00 2001 From: Gurchetan Singh Date: Tue, 4 Jun 2019 16:44:28 -0700 Subject: drm/virtio: use u64_to_user_ptr macro Other DRM drivers use it too. Signed-off-by: Gurchetan Singh Link: http://patchwork.freedesktop.org/patch/msgid/20190604234428.23252-1-gurchetansingh@chromium.org Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index b7f9dfe61d1c..8a17ef97aada 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -168,7 +168,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, goto out_unused_fd; } - user_bo_handles = (void __user *)(uintptr_t)exbuf->bo_handles; + user_bo_handles = u64_to_user_ptr(exbuf->bo_handles); if (copy_from_user(bo_handles, user_bo_handles, exbuf->num_bo_handles * sizeof(uint32_t))) { ret = -EFAULT; @@ -195,8 +195,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, if (ret) goto out_free; - buf = memdup_user((void __user *)(uintptr_t)exbuf->command, - exbuf->size); + buf = memdup_user(u64_to_user_ptr(exbuf->command), exbuf->size); if (IS_ERR(buf)) { ret = PTR_ERR(buf); goto out_unresv; @@ -263,10 +262,9 @@ static int virtio_gpu_getparam_ioctl(struct drm_device *dev, void *data, default: return -EINVAL; } - if (copy_to_user((void __user *)(unsigned long)param->value, - &value, sizeof(int))) { + if (copy_to_user(u64_to_user_ptr(param->value), &value, sizeof(int))) return -EFAULT; - } + return 0; } @@ -545,7 +543,7 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev, ptr = cache_ent->caps_cache; copy_exit: - if (copy_to_user((void __user *)(unsigned long)args->addr, ptr, size)) + if (copy_to_user(u64_to_user_ptr(args->addr), ptr, size)) return -EFAULT; return 0; -- cgit v1.3.1 From 7fdf478a43869bee27e1b50955830f6ebc730b67 Mon Sep 17 00:00:00 2001 From: David Riley Date: Wed, 5 Jun 2019 16:44:20 -0700 Subject: drm/virtio: Ensure cached capset entries are valid before copying. virtio_gpu_get_caps_ioctl could return success with invalid data if a second caller to the function occurred after the entry was created in virtio_gpu_cmd_get_capset but prior to the virtio_gpu_cmd_capset_cb callback being called. This could leak contents of memory as well since the caps_cache allocation is done without zeroing. Signed-off-by: David Riley Link: http://patchwork.freedesktop.org/patch/msgid/20190605234423.11348-1-davidriley@chromium.org Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 8a17ef97aada..9f43b160e1bb 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -524,7 +524,6 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev, list_for_each_entry(cache_ent, &vgdev->cap_cache, head) { if (cache_ent->id == args->cap_set_id && cache_ent->version == args->cap_set_ver) { - ptr = cache_ent->caps_cache; spin_unlock(&vgdev->display_info_lock); goto copy_exit; } @@ -535,6 +534,7 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev, virtio_gpu_cmd_get_capset(vgdev, found_valid, args->cap_set_ver, &cache_ent); +copy_exit: ret = wait_event_timeout(vgdev->resp_wq, atomic_read(&cache_ent->is_valid), 5 * HZ); if (!ret) @@ -542,7 +542,6 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev, ptr = cache_ent->caps_cache; -copy_exit: if (copy_to_user(u64_to_user_ptr(args->addr), ptr, size)) return -EFAULT; -- cgit v1.3.1 From 676a905b09d533ab3a30da753dba4f2daee97402 Mon Sep 17 00:00:00 2001 From: David Riley Date: Wed, 5 Jun 2019 16:44:21 -0700 Subject: drm/virtio: Wake up all waiters when capset response comes in. If multiple callers occur simultaneously, wake them all up. Signed-off-by: David Riley Link: http://patchwork.freedesktop.org/patch/msgid/20190605234423.11348-2-davidriley@chromium.org Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_vq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 2c5eeccb88c0..2bac1cbc5ec8 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -598,7 +598,7 @@ static void virtio_gpu_cmd_capset_cb(struct virtio_gpu_device *vgdev, } } spin_unlock(&vgdev->display_info_lock); - wake_up(&vgdev->resp_wq); + wake_up_all(&vgdev->resp_wq); } static int virtio_get_edid_block(void *data, u8 *buf, -- cgit v1.3.1 From 5934ce998faf33277cc19522609577be835e80b1 Mon Sep 17 00:00:00 2001 From: David Riley Date: Wed, 5 Jun 2019 16:44:22 -0700 Subject: drm/virtio: Fix cache entry creation race. virtio_gpu_cmd_get_capset would check for the existence of an entry under lock. If it was not found, it would unlock and call virtio_gpu_cmd_get_capset to create a new entry. The new entry would be added it to the list without checking if it was added by another task during the period where the lock was not held resulting in duplicate entries. Compounding this issue, virtio_gpu_cmd_capset_cb would stop iterating after find the first matching entry. Multiple callbacks would modify the first entry, but any subsequent entries and their associated waiters would eventually timeout since they don't become valid, also wasting memory along the way. Signed-off-by: David Riley Link: http://patchwork.freedesktop.org/patch/msgid/20190605234423.11348-3-davidriley@chromium.org Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_vq.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 2bac1cbc5ec8..5de38b2ee9ac 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -694,8 +694,11 @@ int virtio_gpu_cmd_get_capset(struct virtio_gpu_device *vgdev, struct virtio_gpu_vbuffer *vbuf; int max_size; struct virtio_gpu_drv_cap_cache *cache_ent; + struct virtio_gpu_drv_cap_cache *search_ent; void *resp_buf; + *cache_p = NULL; + if (idx >= vgdev->num_capsets) return -EINVAL; @@ -726,9 +729,26 @@ int virtio_gpu_cmd_get_capset(struct virtio_gpu_device *vgdev, atomic_set(&cache_ent->is_valid, 0); cache_ent->size = max_size; spin_lock(&vgdev->display_info_lock); - list_add_tail(&cache_ent->head, &vgdev->cap_cache); + /* Search while under lock in case it was added by another task. */ + list_for_each_entry(search_ent, &vgdev->cap_cache, head) { + if (search_ent->id == vgdev->capsets[idx].id && + search_ent->version == version) { + *cache_p = search_ent; + break; + } + } + if (!*cache_p) + list_add_tail(&cache_ent->head, &vgdev->cap_cache); spin_unlock(&vgdev->display_info_lock); + if (*cache_p) { + /* Entry was found, so free everything that was just created. */ + kfree(resp_buf); + kfree(cache_ent->caps_cache); + kfree(cache_ent); + return 0; + } + cmd_p = virtio_gpu_alloc_cmd_resp (vgdev, &virtio_gpu_cmd_capset_cb, &vbuf, sizeof(*cmd_p), sizeof(struct virtio_gpu_resp_capset) + max_size, -- cgit v1.3.1 From 9ff3a5c88e1f1ab17a31402b96d45abe14aab9d7 Mon Sep 17 00:00:00 2001 From: David Riley Date: Mon, 10 Jun 2019 14:18:10 -0700 Subject: drm/virtio: Add memory barriers for capset cache. After data is copied to the cache entry, atomic_set is used indicate that the data is the entry is valid without appropriate memory barriers. Similarly the read side was missing the corresponding memory barriers. Signed-off-by: David Riley Link: http://patchwork.freedesktop.org/patch/msgid/20190610211810.253227-5-davidriley@chromium.org Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 3 +++ drivers/gpu/drm/virtio/virtgpu_vq.c | 2 ++ 2 files changed, 5 insertions(+) (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 9f43b160e1bb..c0ba1ead740f 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -540,6 +540,9 @@ copy_exit: if (!ret) return -EBUSY; + /* is_valid check must proceed before copy of the cache entry. */ + smp_rmb(); + ptr = cache_ent->caps_cache; if (copy_to_user(u64_to_user_ptr(args->addr), ptr, size)) diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 5de38b2ee9ac..6c1a90717535 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -593,6 +593,8 @@ static void virtio_gpu_cmd_capset_cb(struct virtio_gpu_device *vgdev, cache_ent->id == le32_to_cpu(cmd->capset_id)) { memcpy(cache_ent->caps_cache, resp->capset_data, cache_ent->size); + /* Copy must occur before is_valid is signalled. */ + smp_wmb(); atomic_set(&cache_ent->is_valid, 1); break; } -- cgit v1.3.1 From b07baa0393d09fb8ade7d10269d43ed12f517b6e Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 11 Jun 2019 14:54:07 +0200 Subject: drm/virtio: drop framebuffer dirty tracking code No need to have our own implementation, atomic helpers can do it for us. Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20190611125408.29421-1-kraxel@redhat.com --- drivers/gpu/drm/virtio/Makefile | 2 +- drivers/gpu/drm/virtio/virtgpu_display.c | 20 +---- drivers/gpu/drm/virtio/virtgpu_drv.h | 7 -- drivers/gpu/drm/virtio/virtgpu_fb.c | 150 ------------------------------- 4 files changed, 3 insertions(+), 176 deletions(-) delete mode 100644 drivers/gpu/drm/virtio/virtgpu_fb.c (limited to 'drivers/gpu/drm/virtio') diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile index 42949a17ff70..458e606a936f 100644 --- a/drivers/gpu/drm/virtio/Makefile +++ b/drivers/gpu/drm/virtio/Makefile @@ -4,7 +4,7 @@ # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. virtio-gpu-y := virtgpu_drv.o virtgpu_kms.o virtgpu_gem.o \ - virtgpu_fb.o virtgpu_display.o virtgpu_vq.o virtgpu_ttm.o \ + virtgpu_display.o virtgpu_vq.o virtgpu_ttm.o \ virtgpu_fence.o virtgpu_object.o virtgpu_debugfs.o virtgpu_plane.o \ virtgpu_ioctl.o virtgpu_prime.o virtgpu_trace_points.o diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c index 86843a4d6102..ba16e8cb7124 100644 --- a/drivers/gpu/drm/virtio/virtgpu_display.c +++ b/drivers/gpu/drm/virtio/virtgpu_display.c @@ -29,6 +29,7 @@ #include #include #include +#include #define XRES_MIN 32 #define YRES_MIN 32 @@ -49,23 +50,10 @@ static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = { .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, }; -static int -virtio_gpu_framebuffer_surface_dirty(struct drm_framebuffer *fb, - struct drm_file *file_priv, - unsigned int flags, unsigned int color, - struct drm_clip_rect *clips, - unsigned int num_clips) -{ - struct virtio_gpu_framebuffer *virtio_gpu_fb - = to_virtio_gpu_framebuffer(fb); - - return virtio_gpu_surface_dirty(virtio_gpu_fb, clips, num_clips); -} - static const struct drm_framebuffer_funcs virtio_gpu_fb_funcs = { .create_handle = drm_gem_fb_create_handle, .destroy = drm_gem_fb_destroy, - .dirty = virtio_gpu_framebuffer_surface_dirty, + .dirty = drm_atomic_helper_dirtyfb, }; int @@ -85,10 +73,6 @@ virtio_gpu_framebuffer_init(struct drm_device *dev, vgfb->base.obj[0] = NULL; return ret; } - - spin_lock_init(&vgfb->dirty_lock); - vgfb->x1 = vgfb->y1 = INT_MAX; - vgfb->x2 = vgfb->y2 = 0; return 0; } diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 5faccf92aa15..9e2d3062b01d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -142,9 +142,6 @@ struct virtio_gpu_output { struct virtio_gpu_framebuffer { struct drm_framebuffer base; - int x1, y1, x2, y2; /* dirty rect */ - spinlock_t dirty_lock; - uint32_t hw_res_handle; struct virtio_gpu_fence *fence; }; #define to_virtio_gpu_framebuffer(x) \ @@ -254,10 +251,6 @@ int virtio_gpu_mode_dumb_mmap(struct drm_file *file_priv, struct drm_device *dev, uint32_t handle, uint64_t *offset_p); -/* virtio_fb */ -int virtio_gpu_surface_dirty(struct virtio_gpu_framebuffer *qfb, - struct drm_clip_rect *clips, - unsigned int num_clips); /* virtio vg */ int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev); void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev); diff --git a/drivers/gpu/drm/virtio/virtgpu_fb.c b/drivers/gpu/drm/virtio/virtgpu_fb.c deleted file mode 100644 index b07584b1c2bf..000000000000 --- a/drivers/gpu/drm/virtio/virtgpu_fb.c +++ /dev/null @@ -1,150 +0,0 @@ -/* - * Copyright (C) 2015 Red Hat, Inc. - * All Rights Reserved. - * - * Permission is hereby granted, free of charge, to any person obtaining - * a copy of this software and associated documentation files (the - * "Software"), to deal in the Software without restriction, including - * without limitation the rights to use, copy, modify, merge, publish, - * distribute, sublicense, and/or sell copies of the Software, and to - * permit persons to whom the Software is furnished to do so, subject to - * the following conditions: - * - * The above copyright notice and this permission notice (including the - * next paragraph) shall be included in all copies or substantial - * portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. - * IN NO EVENT SHALL THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE - * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION - * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ - -#include -#include -#include "virtgpu_drv.h" - -static int virtio_gpu_dirty_update(struct virtio_gpu_framebuffer *fb, - bool store, int x, int y, - int width, int height) -{ - struct drm_device *dev = fb->base.dev; - struct virtio_gpu_device *vgdev = dev->dev_private; - bool store_for_later = false; - int bpp = fb->base.format->cpp[0]; - int x2, y2; - unsigned long flags; - struct virtio_gpu_object *obj = gem_to_virtio_gpu_obj(fb->base.obj[0]); - - if ((width <= 0) || - (x + width > fb->base.width) || - (y + height > fb->base.height)) { - DRM_DEBUG("values out of range %dx%d+%d+%d, fb %dx%d\n", - width, height, x, y, - fb->base.width, fb->base.height); - return -EINVAL; - } - - /* - * Can be called with pretty much any context (console output - * path). If we are in atomic just store the dirty rect info - * to send out the update later. - * - * Can't test inside spin lock. - */ - if (in_atomic() || store) - store_for_later = true; - - x2 = x + width - 1; - y2 = y + height - 1; - - spin_lock_irqsave(&fb->dirty_lock, flags); - - if (fb->y1 < y) - y = fb->y1; - if (fb->y2 > y2) - y2 = fb->y2; - if (fb->x1 < x) - x = fb->x1; - if (fb->x2 > x2) - x2 = fb->x2; - - if (store_for_later) { - fb->x1 = x; - fb->x2 = x2; - fb->y1 = y; - fb->y2 = y2; - spin_unlock_irqrestore(&fb->dirty_lock, flags); - return 0; - } - - fb->x1 = fb->y1 = INT_MAX; - fb->x2 = fb->y2 = 0; - - spin_unlock_irqrestore(&fb->dirty_lock, flags); - - { - uint32_t offset; - uint32_t w = x2 - x + 1; - uint32_t h = y2 - y + 1; - - offset = (y * fb->base.pitches[0]) + x * bpp; - - virtio_gpu_cmd_transfer_to_host_2d(vgdev, obj, - offset, - cpu_to_le32(w), - cpu_to_le32(h), - cpu_to_le32(x), - cpu_to_le32(y), - NULL); - - } - virtio_gpu_cmd_resource_flush(vgdev, obj->hw_res_handle, - x, y, x2 - x + 1, y2 - y + 1); - return 0; -} - -int virtio_gpu_surface_dirty(struct virtio_gpu_framebuffer *vgfb, - struct drm_clip_rect *clips, - unsigned int num_clips) -{ - struct virtio_gpu_device *vgdev = vgfb->base.dev->dev_private; - struct virtio_gpu_object *obj = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); - struct drm_clip_rect norect; - struct drm_clip_rect *clips_ptr; - int left, right, top, bottom; - int i; - int inc = 1; - - if (!num_clips) { - num_clips = 1; - clips = &norect; - norect.x1 = norect.y1 = 0; - norect.x2 = vgfb->base.width; - norect.y2 = vgfb->base.height; - } - left = clips->x1; - right = clips->x2; - top = clips->y1; - bottom = clips->y2; - - /* skip the first clip rect */ - for (i = 1, clips_ptr = clips + inc; - i < num_clips; i++, clips_ptr += inc) { - left = min_t(int, left, (int)clips_ptr->x1); - right = max_t(int, right, (int)clips_ptr->x2); - top = min_t(int, top, (int)clips_ptr->y1); - bottom = max_t(int, bottom, (int)clips_ptr->y2); - } - - if (obj->dumb) - return virtio_gpu_dirty_update(vgfb, false, left, top, - right - left, bottom - top); - - virtio_gpu_cmd_resource_flush(vgdev, obj->hw_res_handle, - left, top, right - left, bottom - top); - return 0; -} -- cgit v1.3.1