From 6b2e8aa45126161135fb4a88870c9526fd8319f8 Mon Sep 17 00:00:00 2001 From: Zack Rusin Date: Mon, 30 Jan 2023 22:35:36 -0500 Subject: drm/vmwgfx: Remove the duplicate bo_free function Remove the explicit bo_free parameter which was switching between vmw_bo_bo_free and vmw_gem_destroy which had exactly the same implementation. It makes no sense to keep parameter which is always the same, remove it and all code referencing it. Instead use the vmw_bo_bo_free directly. Signed-off-by: Zack Rusin Reviewed-by: Martin Krastev Reviewed-by: Maaz Mombasawala Acked-by: Thomas Zimmermann Link: https://patchwork.freedesktop.org/patch/msgid/20230131033542.953249-3-zack@kde.org --- drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/vmwgfx/vmwgfx_shader.c') diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c index 108a496b5d18..93b1400aed4a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c @@ -893,7 +893,7 @@ int vmw_compat_shader_add(struct vmw_private *dev_priv, return -EINVAL; ret = vmw_bo_create(dev_priv, size, &vmw_sys_placement, - true, true, vmw_bo_bo_free, &buf); + true, true, &buf); if (unlikely(ret != 0)) goto out; -- cgit From 09881d2940bbd641f27f9ae7907e8a1893bc54b2 Mon Sep 17 00:00:00 2001 From: Zack Rusin Date: Mon, 30 Jan 2023 22:35:37 -0500 Subject: drm/vmwgfx: Rename vmw_buffer_object to vmw_bo The rest of the drivers which are using ttm have mostly standardized on driver_prefix_bo as the name for subclasses of the TTM buffer object. Make vmwgfx match the rest of the drivers and follow the same naming semantics. This is especially clear given that the name of the file in which the object was defined is vmw_bo.c. Signed-off-by: Zack Rusin Reviewed-by: Martin Krastev Reviewed-by: Maaz Mombasawala Acked-by: Thomas Zimmermann Link: https://patchwork.freedesktop.org/patch/msgid/20230131033542.953249-4-zack@kde.org --- drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/vmwgfx/vmwgfx_shader.c') diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c index 93b1400aed4a..b186d0993d83 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 OR MIT /************************************************************************** * - * Copyright 2009-2015 VMware, Inc., Palo Alto, CA., USA + * Copyright 2009-2023 VMware, Inc., Palo Alto, CA., USA * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the @@ -27,9 +27,10 @@ #include +#include "vmwgfx_binding.h" +#include "vmwgfx_bo.h" #include "vmwgfx_drv.h" #include "vmwgfx_resource_priv.h" -#include "vmwgfx_binding.h" struct vmw_shader { struct vmw_resource res; @@ -158,7 +159,7 @@ static int vmw_gb_shader_init(struct vmw_private *dev_priv, SVGA3dShaderType type, uint8_t num_input_sig, uint8_t num_output_sig, - struct vmw_buffer_object *byte_code, + struct vmw_bo *byte_code, void (*res_free) (struct vmw_resource *res)) { struct vmw_shader *shader = vmw_res_to_shader(res); @@ -680,7 +681,7 @@ int vmw_shader_destroy_ioctl(struct drm_device *dev, void *data, } static int vmw_user_shader_alloc(struct vmw_private *dev_priv, - struct vmw_buffer_object *buffer, + struct vmw_bo *buffer, size_t shader_size, size_t offset, SVGA3dShaderType shader_type, @@ -734,7 +735,7 @@ out: static struct vmw_resource *vmw_shader_alloc(struct vmw_private *dev_priv, - struct vmw_buffer_object *buffer, + struct vmw_bo *buffer, size_t shader_size, size_t offset, SVGA3dShaderType shader_type) @@ -771,7 +772,7 @@ static int vmw_shader_define(struct drm_device *dev, struct drm_file *file_priv, { struct vmw_private *dev_priv = vmw_priv(dev); struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile; - struct vmw_buffer_object *buffer = NULL; + struct vmw_bo *buffer = NULL; SVGA3dShaderType shader_type; int ret; @@ -883,7 +884,7 @@ int vmw_compat_shader_add(struct vmw_private *dev_priv, struct list_head *list) { struct ttm_operation_ctx ctx = { false, true }; - struct vmw_buffer_object *buf; + struct vmw_bo *buf; struct ttm_bo_kmap_obj map; bool is_iomem; int ret; -- cgit From 39985eea5a6dd1e844f216028252870e980b9e7f Mon Sep 17 00:00:00 2001 From: Zack Rusin Date: Mon, 30 Jan 2023 22:35:41 -0500 Subject: drm/vmwgfx: Abstract placement selection Problem with explicit placement selection in vmwgfx is that by the time the buffer object needs to be validated the information about which placement was supposed to be used is lost. To workaround this the driver had a bunch of state in various places e.g. as_mob or cpu_blit to somehow convey the information on which placement was intended. Fix it properly by allowing the buffer objects to hold their preferred placement so it can be reused whenever needed. This makes the entire validation pipeline a lot easier both to understand and maintain. Signed-off-by: Zack Rusin Reviewed-by: Martin Krastev Reviewed-by: Maaz Mombasawala Acked-by: Thomas Zimmermann Link: https://patchwork.freedesktop.org/patch/msgid/20230131033542.953249-8-zack@kde.org --- drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/vmwgfx/vmwgfx_shader.c') diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c index b186d0993d83..9920c103bffb 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c @@ -94,7 +94,8 @@ static const struct vmw_res_func vmw_gb_shader_func = { .prio = 3, .dirty_prio = 3, .type_name = "guest backed shaders", - .backup_placement = &vmw_mob_placement, + .domain = VMW_BO_DOMAIN_MOB, + .busy_domain = VMW_BO_DOMAIN_MOB, .create = vmw_gb_shader_create, .destroy = vmw_gb_shader_destroy, .bind = vmw_gb_shader_bind, @@ -108,7 +109,8 @@ static const struct vmw_res_func vmw_dx_shader_func = { .prio = 3, .dirty_prio = 3, .type_name = "dx shaders", - .backup_placement = &vmw_mob_placement, + .domain = VMW_BO_DOMAIN_MOB, + .busy_domain = VMW_BO_DOMAIN_MOB, .create = vmw_dx_shader_create, /* * The destroy callback is only called with a committed resource on @@ -893,7 +895,9 @@ int vmw_compat_shader_add(struct vmw_private *dev_priv, if (!vmw_shader_id_ok(user_key, shader_type)) return -EINVAL; - ret = vmw_bo_create(dev_priv, size, &vmw_sys_placement, + ret = vmw_bo_create(dev_priv, size, + VMW_BO_DOMAIN_SYS, + VMW_BO_DOMAIN_SYS, true, true, &buf); if (unlikely(ret != 0)) goto out; @@ -913,7 +917,10 @@ int vmw_compat_shader_add(struct vmw_private *dev_priv, WARN_ON(is_iomem); ttm_bo_kunmap(&map); - ret = ttm_bo_validate(&buf->base, &vmw_sys_placement, &ctx); + vmw_bo_placement_set(buf, + VMW_BO_DOMAIN_SYS, + VMW_BO_DOMAIN_SYS); + ret = ttm_bo_validate(&buf->base, &buf->placement, &ctx); WARN_ON(ret != 0); ttm_bo_unreserve(&buf->base); -- cgit From 668b206601c5f5063e03b76784a0d3024fa2b249 Mon Sep 17 00:00:00 2001 From: Zack Rusin Date: Mon, 30 Jan 2023 22:35:42 -0500 Subject: drm/vmwgfx: Stop using raw ttm_buffer_object's Various bits of the driver used raw ttm_buffer_object instead of the driver specific vmw_bo object. All those places used to duplicate the mapped bo caching policy of vmw_bo. Instead of duplicating all of that code and special casing various functions to work both with vmw_bo and raw ttm_buffer_object's unify the buffer object handling code. As part of that work fix the naming of bo's, e.g. insted of generic backup use 'guest_memory' because that's what it really is. All of it makes the driver easier to maintain and the code easier to read. Saves 100+ loc as well. Signed-off-by: Zack Rusin Reviewed-by: Martin Krastev Reviewed-by: Maaz Mombasawala Acked-by: Thomas Zimmermann Link: https://patchwork.freedesktop.org/patch/msgid/20230131033542.953249-9-zack@kde.org --- drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 49 +++++++++++++++++----------------- 1 file changed, 25 insertions(+), 24 deletions(-) (limited to 'drivers/gpu/drm/vmwgfx/vmwgfx_shader.c') diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c index 9920c103bffb..6b8e984695ed 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c @@ -89,7 +89,7 @@ const struct vmw_user_resource_conv *user_shader_converter = static const struct vmw_res_func vmw_gb_shader_func = { .res_type = vmw_res_shader, - .needs_backup = true, + .needs_guest_memory = true, .may_evict = true, .prio = 3, .dirty_prio = 3, @@ -104,7 +104,7 @@ static const struct vmw_res_func vmw_gb_shader_func = { static const struct vmw_res_func vmw_dx_shader_func = { .res_type = vmw_res_shader, - .needs_backup = true, + .needs_guest_memory = true, .may_evict = true, .prio = 3, .dirty_prio = 3, @@ -178,10 +178,10 @@ static int vmw_gb_shader_init(struct vmw_private *dev_priv, return ret; } - res->backup_size = size; + res->guest_memory_size = size; if (byte_code) { - res->backup = vmw_bo_reference(byte_code); - res->backup_offset = offset; + res->guest_memory_bo = vmw_bo_reference(byte_code); + res->guest_memory_offset = offset; } shader->size = size; shader->type = type; @@ -262,8 +262,8 @@ static int vmw_gb_shader_bind(struct vmw_resource *res, cmd->header.size = sizeof(cmd->body); cmd->body.shid = res->id; cmd->body.mobid = bo->resource->start; - cmd->body.offsetInBytes = res->backup_offset; - res->backup_dirty = false; + cmd->body.offsetInBytes = res->guest_memory_offset; + res->guest_memory_dirty = false; vmw_cmd_commit(dev_priv, sizeof(*cmd)); return 0; @@ -280,7 +280,7 @@ static int vmw_gb_shader_unbind(struct vmw_resource *res, } *cmd; struct vmw_fence_obj *fence; - BUG_ON(res->backup->base.resource->mem_type != VMW_PL_MOB); + BUG_ON(res->guest_memory_bo->tbo.resource->mem_type != VMW_PL_MOB); cmd = VMW_CMD_RESERVE(dev_priv, sizeof(*cmd)); if (unlikely(cmd == NULL)) @@ -400,8 +400,8 @@ static int vmw_dx_shader_unscrub(struct vmw_resource *res) cmd->header.size = sizeof(cmd->body); cmd->body.cid = shader->ctx->id; cmd->body.shid = shader->id; - cmd->body.mobid = res->backup->base.resource->start; - cmd->body.offsetInBytes = res->backup_offset; + cmd->body.mobid = res->guest_memory_bo->tbo.resource->start; + cmd->body.offsetInBytes = res->guest_memory_offset; vmw_cmd_commit(dev_priv, sizeof(*cmd)); vmw_cotable_add_resource(shader->cotable, &shader->cotable_head); @@ -511,7 +511,7 @@ static int vmw_dx_shader_unbind(struct vmw_resource *res, struct vmw_fence_obj *fence; int ret; - BUG_ON(res->backup->base.resource->mem_type != VMW_PL_MOB); + BUG_ON(res->guest_memory_bo->tbo.resource->mem_type != VMW_PL_MOB); mutex_lock(&dev_priv->binding_mutex); ret = vmw_dx_shader_scrub(res); @@ -785,7 +785,7 @@ static int vmw_shader_define(struct drm_device *dev, struct drm_file *file_priv, return ret; } - if ((u64)buffer->base.base.size < (u64)size + (u64)offset) { + if ((u64)buffer->tbo.base.size < (u64)size + (u64)offset) { VMW_DEBUG_USER("Illegal buffer- or shader size.\n"); ret = -EINVAL; goto out_bad_arg; @@ -891,25 +891,29 @@ int vmw_compat_shader_add(struct vmw_private *dev_priv, bool is_iomem; int ret; struct vmw_resource *res; + struct vmw_bo_params bo_params = { + .domain = VMW_BO_DOMAIN_SYS, + .busy_domain = VMW_BO_DOMAIN_SYS, + .bo_type = ttm_bo_type_device, + .size = size, + .pin = true + }; if (!vmw_shader_id_ok(user_key, shader_type)) return -EINVAL; - ret = vmw_bo_create(dev_priv, size, - VMW_BO_DOMAIN_SYS, - VMW_BO_DOMAIN_SYS, - true, true, &buf); + ret = vmw_bo_create(dev_priv, &bo_params, &buf); if (unlikely(ret != 0)) goto out; - ret = ttm_bo_reserve(&buf->base, false, true, NULL); + ret = ttm_bo_reserve(&buf->tbo, false, true, NULL); if (unlikely(ret != 0)) goto no_reserve; /* Map and copy shader bytecode. */ - ret = ttm_bo_kmap(&buf->base, 0, PFN_UP(size), &map); + ret = ttm_bo_kmap(&buf->tbo, 0, PFN_UP(size), &map); if (unlikely(ret != 0)) { - ttm_bo_unreserve(&buf->base); + ttm_bo_unreserve(&buf->tbo); goto no_reserve; } @@ -917,12 +921,9 @@ int vmw_compat_shader_add(struct vmw_private *dev_priv, WARN_ON(is_iomem); ttm_bo_kunmap(&map); - vmw_bo_placement_set(buf, - VMW_BO_DOMAIN_SYS, - VMW_BO_DOMAIN_SYS); - ret = ttm_bo_validate(&buf->base, &buf->placement, &ctx); + ret = ttm_bo_validate(&buf->tbo, &buf->placement, &ctx); WARN_ON(ret != 0); - ttm_bo_unreserve(&buf->base); + ttm_bo_unreserve(&buf->tbo); res = vmw_shader_alloc(dev_priv, buf, size, 0, shader_type); if (unlikely(ret != 0)) -- cgit From 9ef8d83e8e25d5f1811b3a38eb1484f85f64296c Mon Sep 17 00:00:00 2001 From: Zack Rusin Date: Sat, 11 Feb 2023 00:05:14 -0500 Subject: drm/vmwgfx: Do not drop the reference to the handle too soon v3: Fix vmw_user_bo_lookup which was also dropping the gem reference before the kernel was done with buffer depending on userspace doing the right thing. Same bug, different spot. It is possible for userspace to predict the next buffer handle and to destroy the buffer while it's still used by the kernel. Delay dropping the internal reference on the buffers until kernel is done with them. Instead of immediately dropping the gem reference in vmw_user_bo_lookup and vmw_gem_object_create_with_handle let the callers decide when they're ready give the control back to userspace. Also fixes the second usage of vmw_gem_object_create_with_handle in vmwgfx_surface.c which wasn't grabbing an explicit reference to the gem object which could have been destroyed by the userspace on the owning surface at any point. Signed-off-by: Zack Rusin Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM") Reviewed-by: Martin Krastev Reviewed-by: Maaz Mombasawala Link: https://patchwork.freedesktop.org/patch/msgid/20230211050514.2431155-1-zack@kde.org --- drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/gpu/drm/vmwgfx/vmwgfx_shader.c') diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c index 6b8e984695ed..e7226db8b242 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c @@ -810,6 +810,7 @@ static int vmw_shader_define(struct drm_device *dev, struct drm_file *file_priv, num_output_sig, tfile, shader_handle); out_bad_arg: vmw_bo_unreference(&buffer); + drm_gem_object_put(&buffer->tbo.base); return ret; } -- cgit