Re: [PATCH] drm/vmwgfx: Keep a gem reference to user bos in surfaces
From: Martin Krastev
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
On 28.09.23 г. 7:13 ч., Zack Rusin wrote:
From: Zack Rusin
Surfaces can be backed (i.e. stored in) memory objects (mob's) which
are created and managed by the userspace as GEM buffers. Surfaces
grab only a ttm reference which means that the gem object can
be deleted underneath us, especially in cases where prime buffer
export is used.
Make sure that all userspace surfaces which are backed by gem objects
hold a gem reference to make sure they're not deleted before vmw
surfaces are done with them, which fixes:
[ cut here ]
refcount_t: underflow; use-after-free.
WARNING: CPU: 2 PID: 2632 at lib/refcount.c:28 refcount_warn_saturate+0xfb/0x150
Modules linked in: overlay vsock_loopback vmw_vsock_virtio_transport_common
vmw_vsock_vmci_transport vsock snd_ens1371 snd_ac97_codec ac97_bus snd_pcm
gameport>
CPU: 2 PID: 2632 Comm: vmw_ref_count Not tainted 6.5.0-rc2-vmwgfx #1
Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference
Platform, BIOS 6.00 11/12/2020
RIP: 0010:refcount_warn_saturate+0xfb/0x150
Code: eb 9e 0f b6 1d 8b 5b a6 01 80 fb 01 0f 87 ba e4 80 00 83 e3 01 75 89 48 c7 c7 c0
3c f9 a3 c6 05 6f 5b a6 01 01 e8 15 81 98 ff <0f> 0b e9 6f ff ff ff 0f b>
RSP: 0018:bdc34344bba0 EFLAGS: 00010286
RAX: RBX: RCX: 0027
RDX: 960475ea1548 RSI: 0001 RDI: 960475ea1540
RBP: bdc34344bba8 R08: 0003 R09: 65646e75203a745f
R10: a5b32b20 R11: 72657466612d6573 R12: 96037d6a6400
R13: 9603484805b0 R14: 000b R15: 9603bed06060
FS: 7f5fd8520c40() GS:960475e8() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 7f5fda755000 CR3: 00010d012005 CR4: 003706e0
Call Trace:
? show_regs+0x6e/0x80
? refcount_warn_saturate+0xfb/0x150
? __warn+0x91/0x150
? refcount_warn_saturate+0xfb/0x150
? report_bug+0x19d/0x1b0
? handle_bug+0x46/0x80
? exc_invalid_op+0x1d/0x80
? asm_exc_invalid_op+0x1f/0x30
? refcount_warn_saturate+0xfb/0x150
drm_gem_object_handle_put_unlocked+0xba/0x110 [drm]
drm_gem_object_release_handle+0x6e/0x80 [drm]
drm_gem_handle_delete+0x6a/0xc0 [drm]
? __pfx_vmw_bo_unref_ioctl+0x10/0x10 [vmwgfx]
vmw_bo_unref_ioctl+0x33/0x40 [vmwgfx]
drm_ioctl_kernel+0xbc/0x160 [drm]
drm_ioctl+0x2d2/0x580 [drm]
? __pfx_vmw_bo_unref_ioctl+0x10/0x10 [vmwgfx]
? do_vmi_munmap+0xee/0x180
vmw_generic_ioctl+0xbd/0x180 [vmwgfx]
vmw_unlocked_ioctl+0x19/0x20 [vmwgfx]
__x64_sys_ioctl+0x99/0xd0
do_syscall_64+0x5d/0x90
? syscall_exit_to_user_mode+0x2a/0x50
? do_syscall_64+0x6d/0x90
? handle_mm_fault+0x16e/0x2f0
? exit_to_user_mode_prepare+0x34/0x170
? irqentry_exit_to_user_mode+0xd/0x20
? irqentry_exit+0x3f/0x50
? exc_page_fault+0x8e/0x190
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
RIP: 0033:0x7f5fda51aaff
Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d
44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <41> 89 c0 3d 00 f0 ff ff 7>
RSP: 002b:7ffd536a4d30 EFLAGS: 0246 ORIG_RAX: 0010
RAX: ffda RBX: 7ffd536a4de0 RCX: 7f5fda51aaff
RDX: 7ffd536a4de0 RSI: 40086442 RDI: 0003
RBP: 40086442 R08: 55fa603ada50 R09:
R10: 0001 R11: 0246 R12: 7ffd536a51b8
R13: 0003 R14: 55fa5ebb4c80 R15: 7f5fda90f040
---[ end trace ]---
A lot of the analyis on the bug was done by Murray McAllister and
Ian Forbes.
Reported-by: Murray McAllister
Cc: Ian Forbes
Signed-off-by: Zack Rusin
Fixes: a950b989ea29 ("drm/vmwgfx: Do not drop the reference to the handle too
soon")
Cc: # v6.2+
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 7 +++---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 17 +
drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 6 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 4 +++
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 10 +---
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 18 +++---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 12 -
drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 4 +--
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 31 +---
11 files changed, 68 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index c43853597776..2bfac3aad7b7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -34,6 +34,8 @@
static void vmw_bo_release(struct vmw_bo *vbo)
{
+ WARN_ON(vbo->tbo.base.funcs &&
+ kref_read(>tbo.base.refcount) != 0);
vmw_bo_unmap(vbo);
drm_gem_object_release(>tbo.base);
}
@@ -497,7 +499,7 @@ static
Re: [PATCH] drm/vmwgfx: Fix possible invalid drm gem put calls
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 18 Aug 2023 04:13:14, Zack Rusin wrote:
>From: Zack Rusin
>
>vmw_bo_unreference sets the input buffer to null on exit, resulting in
>null ptr deref's on the subsequent drm gem put calls.
>
>This went unnoticed because only very old userspace would be exercising
>those paths but it wouldn't be hard to hit on old distros with brand
>new kernels.
>
>Introduce a new function that abstracts unrefing of user bo's to make
>the code cleaner and more explicit.
>
>Signed-off-by: Zack Rusin
>Reported-by: Ian Forbes
>Fixes: 9ef8d83e8e25 ("drm/vmwgfx: Do not drop the reference to the
handle too soon")
>Cc: # v6.4+
>---
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 6 ++
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 8
> drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 6 ++
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 ++
> drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c | 3 +--
> drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 3 +--
> 6 files changed, 16 insertions(+), 16 deletions(-)
>
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
>index 82094c137855..c43853597776 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
>@@ -497,10 +497,9 @@ static int vmw_user_bo_synccpu_release(struct
drm_file *filp,
> if (!(flags & drm_vmw_synccpu_allow_cs)) {
> atomic_dec(_bo->cpu_writers);
> }
>- ttm_bo_put(_bo->tbo);
>+ vmw_user_bo_unref(vmw_bo);
> }
>
>- drm_gem_object_put(_bo->tbo.base);
> return ret;
> }
>
>@@ -540,8 +539,7 @@ int vmw_user_bo_synccpu_ioctl(struct drm_device
*dev, void *data,
> return ret;
>
> ret = vmw_user_bo_synccpu_grab(vbo, arg->flags);
>- vmw_bo_unreference();
>- drm_gem_object_put(>tbo.base);
>+ vmw_user_bo_unref(vbo);
> if (unlikely(ret != 0)) {
> if (ret == -ERESTARTSYS || ret == -EBUSY)
> return -EBUSY;
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
>index 50a836e70994..1d433fceed3d 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
>@@ -195,6 +195,14 @@ static inline struct vmw_bo
*vmw_bo_reference(struct vmw_bo *buf)
> return buf;
> }
>
>+static inline void vmw_user_bo_unref(struct vmw_bo *vbo)
>+{
>+ if (vbo) {
>+ ttm_bo_put(>tbo);
>+ drm_gem_object_put(>tbo.base);
>+ }
>+}
>+
> static inline struct vmw_bo *to_vmw_bo(struct drm_gem_object *gobj)
> {
> return container_of((gobj), struct vmw_bo, tbo.base);
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
>index 6b9aa2b4ef54..25b96821df0f 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
>@@ -1164,8 +1164,7 @@ static int vmw_translate_mob_ptr(struct
vmw_private *dev_priv,
> }
> vmw_bo_placement_set(vmw_bo, VMW_BO_DOMAIN_MOB,
VMW_BO_DOMAIN_MOB);
> ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
>- ttm_bo_put(_bo->tbo);
>- drm_gem_object_put(_bo->tbo.base);
>+ vmw_user_bo_unref(vmw_bo);
> if (unlikely(ret != 0))
> return ret;
>
>@@ -1221,8 +1220,7 @@ static int vmw_translate_guest_ptr(struct
vmw_private *dev_priv,
> vmw_bo_placement_set(vmw_bo, VMW_BO_DOMAIN_GMR |
VMW_BO_DOMAIN_VRAM,
> VMW_BO_DOMAIN_GMR | VMW_BO_DOMAIN_VRAM);
> ret = vmw_validation_add_bo(sw_context->ctx, vmw_bo);
>- ttm_bo_put(_bo->tbo);
>- drm_gem_object_put(_bo->tbo.base);
>+ vmw_user_bo_unref(vmw_bo);
> if (unlikely(ret != 0))
> return ret;
>
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>index b62207be3363..1489ad73c103 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
>@@ -1665,10 +1665,8 @@ static struct drm_framebuffer
*vmw_kms_fb_create(struct drm_device *dev,
>
> err_out:
> /* vmw_user_lookup_handle takes one ref so does new_fb */
>- if (bo) {
>- vmw_bo_unreference();
>- drm_gem_object_put(>tbo.base);
>- }
>+ if (bo)
>+ vmw_user_bo_unref(bo);
> if (surface)
> vmw_surface_unreference();
>
>diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
>index 7e112319a23c..fb85f244c3d0 100644
>--- a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
>+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
>@@ -451,8 +451,7 @@ int vmw_overlay_ioctl(struct drm_device *dev, void
*data,
>
> ret = vmw_overlay_update_stream(dev_priv, buf, arg, true);
>
>- vmw_bo_unreference();
>-
Re: [PATCH v3 3/8] drm/vmwgfx: Use the hotspot properties from cursor planes
From: Martin Krastev
Looks good.
Reviewed-by: Martin Krastev
Regards,
Martin
On 27.06.23 г. 6:58 ч., Zack Rusin wrote:
From: Zack Rusin
Atomic modesetting got support for mouse hotspots via the hotspot
properties. Port the legacy kms hotspot handling to the new properties
on cursor planes.
Signed-off-by: Zack Rusin
Cc: Martin Krastev
Cc: Maaz Mombasawala
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index b62207be3363..de294dfe05d0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -768,13 +768,8 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
struct vmw_plane_state *old_vps = vmw_plane_state_to_vps(old_state);
s32 hotspot_x, hotspot_y;
- hotspot_x = du->hotspot_x;
- hotspot_y = du->hotspot_y;
-
- if (new_state->fb) {
- hotspot_x += new_state->fb->hot_x;
- hotspot_y += new_state->fb->hot_y;
- }
+ hotspot_x = du->hotspot_x + new_state->hotspot_x;
+ hotspot_y = du->hotspot_y + new_state->hotspot_y;
du->cursor_surface = vps->surf;
du->cursor_bo = vps->bo;
Re: [PATCH] drm/vmwgfx: Fix shader stage validation
From: Martin Krastev
Looks good!
Reviewed-by: Martin Krastev
Regards,
Martin
On 16.06.23 г. 22:09 ч., Zack Rusin wrote:
From: Zack Rusin
For multiple commands the driver was not correctly validating the shader
stages resulting in possible kernel oopses. The validation code was only.
if ever, checking the upper bound on the shader stages but never a lower
bound (valid shader stages start at 1 not 0).
Fixes kernel oopses ending up in vmw_binding_add, e.g.:
Oops: [#1] PREEMPT SMP PTI
CPU: 1 PID: 2443 Comm: testcase Not tainted 6.3.0-rc4-vmwgfx #1
Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference
Platform, BIOS 6.00 11/12/2020
RIP: 0010:vmw_binding_add+0x4c/0x140 [vmwgfx]
Code: 7e 30 49 83 ff 0e 0f 87 ea 00 00 00 4b 8d 04 7f 89 d2 89 cb 48 c1 e0 03 4c 8b b0
40 3d 93 c0 48 8b 80 48 3d 93 c0 49 0f af de <48> 03 1c d0 4c 01 e3 49 8>
RSP: 0018:b8014416b968 EFLAGS: 00010206
RAX: c0933ec0 RBX: RCX:
RDX: RSI: b8014416b9c0 RDI: b8014316f000
RBP: b8014416b998 R08: 0003 R09: 746f6c735f726564
R10: aaf2bda0 R11: 732e676e69646e69 R12: b8014316f000
R13: b8014416b9c0 R14: 0040 R15: 0006
FS: 7fba8c0af740() GS:8a1277c8() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 0007c0933eb8 CR3: 000118244001 CR4: 003706e0
Call Trace:
vmw_view_bindings_add+0xf5/0x1b0 [vmwgfx]
? ___drm_dbg+0x8a/0xb0 [drm]
vmw_cmd_dx_set_shader_res+0x8f/0xc0 [vmwgfx]
vmw_execbuf_process+0x590/0x1360 [vmwgfx]
vmw_execbuf_ioctl+0x173/0x370 [vmwgfx]
? __drm_dev_dbg+0xb4/0xe0 [drm]
? __pfx_vmw_execbuf_ioctl+0x10/0x10 [vmwgfx]
drm_ioctl_kernel+0xbc/0x160 [drm]
drm_ioctl+0x2d2/0x580 [drm]
? __pfx_vmw_execbuf_ioctl+0x10/0x10 [vmwgfx]
? do_fault+0x1a6/0x420
vmw_generic_ioctl+0xbd/0x180 [vmwgfx]
vmw_unlocked_ioctl+0x19/0x20 [vmwgfx]
__x64_sys_ioctl+0x96/0xd0
do_syscall_64+0x5d/0x90
? handle_mm_fault+0xe4/0x2f0
? debug_smp_processor_id+0x1b/0x30
? fpregs_assert_state_consistent+0x2e/0x50
? exit_to_user_mode_prepare+0x40/0x180
? irqentry_exit_to_user_mode+0xd/0x20
? irqentry_exit+0x3f/0x50
? exc_page_fault+0x8b/0x180
entry_SYSCALL_64_after_hwframe+0x72/0xdc
Signed-off-by: Zack Rusin
Cc: secur...@openanolis.org
Reported-by: Ziming Zhang
Testcase-found-by: Niels De Graef
Fixes: d80efd5cb3de ("drm/vmwgfx: Initial DX support")
Cc: # v4.3+
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 12 ++
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 29 ++---
2 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 3810a9984a7f..58bfdf203eca 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1513,4 +1513,16 @@ static inline bool vmw_has_fences(struct vmw_private
*vmw)
return (vmw_fifo_caps(vmw) & SVGA_FIFO_CAP_FENCE) != 0;
}
+static inline bool vmw_shadertype_is_valid(enum vmw_sm_type shader_model,
+ u32 shader_type)
+{
+ SVGA3dShaderType max_allowed = SVGA3D_SHADERTYPE_PREDX_MAX;
+
+ if (shader_model >= VMW_SM_5)
+ max_allowed = SVGA3D_SHADERTYPE_MAX;
+ else if (shader_model >= VMW_SM_4)
+ max_allowed = SVGA3D_SHADERTYPE_DX10_MAX;
+ return shader_type >= SVGA3D_SHADERTYPE_MIN && shader_type <
max_allowed;
+}
+
#endif
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index 6b9aa2b4ef54..d30c0e3d3ab7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -1992,7 +1992,7 @@ static int vmw_cmd_set_shader(struct vmw_private
*dev_priv,
cmd = container_of(header, typeof(*cmd), header);
- if (cmd->body.type >= SVGA3D_SHADERTYPE_PREDX_MAX) {
+ if (!vmw_shadertype_is_valid(VMW_SM_LEGACY, cmd->body.type)) {
VMW_DEBUG_USER("Illegal shader type %u.\n",
(unsigned int) cmd->body.type);
return -EINVAL;
@@ -2115,8 +2115,6 @@ vmw_cmd_dx_set_single_constant_buffer(struct vmw_private
*dev_priv,
SVGA3dCmdHeader *header)
{
VMW_DECLARE_CMD_VAR(*cmd, SVGA3dCmdDXSetSingleConstantBuffer);
- SVGA3dShaderType max_shader_num = has_sm5_context(dev_priv) ?
- SVGA3D_NUM_SHADERTYPE : SVGA3D_NUM_SHADERTYPE_DX10;
struct vmw_resource *res = NULL;
struct vmw_ctx_validation_info *ctx_node = VMW_GET_CTX_NODE(sw_context);
@@ -2133,6 +2131,14 @@ vmw_cmd_dx_set_single_constant_buffer(struct vmw_private
*dev_priv,
if (unlikely(ret != 0))
return ret;
+ if (!vmw_shadertype_is_valid(dev_priv->sm_type, cmd->body.type) ||
+ cmd->body.slot >= SVGA3D_DX_MAX_CONSTBUFFERS) {
+
Re: [PATCH] drm/atomic-helper: Do not assume vblank is always present
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 5.04.23 г. 7:56 ч., Zack Rusin wrote:
From: Zack Rusin
Many drivers (in particular all of the virtualized drivers) do not
implement vblank handling. Assuming vblank is always present
leads to crashes.
Fix the crashes by making sure the device supports vblank before using
it.
Fixes crashes on boot, as in:
Oops: [#1] PREEMPT SMP PTI
CPU: 0 PID: 377 Comm: systemd-modules Not tainted 6.3.0-rc4-vmwgfx #1
Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference
Platform, BIOS 6.00 11/12/2020
RIP: 0010:drm_crtc_next_vblank_start+0x2c/0x80 [drm]
Code: 1e fa 0f 1f 44 00 00 8b 87 90 00 00 00 48 8b 17 55 48 8d 0c c0 48 89 e5 41 54 53
48 8d 1c 48 48 c1 e3 04 48 03 9a 40 01 00 00 <8b> 53 74 85 d2 74 3f 8b 4>
RSP: 0018:b825004073c8 EFLAGS: 00010246
RAX: RBX: RCX:
RDX: 9b18cf8d RSI: b825004073e8 RDI: 9b18d0f4
RBP: b825004073d8 R08: 9b18cf8d R09:
R10: 9b18c57ef6e8 R11: 9b18c2d59e00 R12:
R13: 9b18cfa99280 R14: R15: 9b18cf8d
FS: 7f2b82538900() GS:9b19f7c0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 0074 CR3: 0001060a6003 CR4: 003706f0
Call Trace:
drm_atomic_helper_wait_for_fences+0x87/0x1e0 [drm_kms_helper]
drm_atomic_helper_commit+0xa1/0x160 [drm_kms_helper]
drm_atomic_commit+0x9a/0xd0 [drm]
? __pfx___drm_printfn_info+0x10/0x10 [drm]
drm_client_modeset_commit_atomic+0x1e9/0x230 [drm]
drm_client_modeset_commit_locked+0x5f/0x160 [drm]
? mutex_lock+0x17/0x50
drm_client_modeset_commit+0x2b/0x50 [drm]
__drm_fb_helper_restore_fbdev_mode_unlocked+0xca/0x100 [drm_kms_helper]
drm_fb_helper_set_par+0x40/0x80 [drm_kms_helper]
fbcon_init+0x2c5/0x690
visual_init+0xee/0x190
do_bind_con_driver.isra.0+0x1ce/0x4b0
do_take_over_console+0x136/0x220
? vprintk_default+0x21/0x30
do_fbcon_takeover+0x78/0x130
do_fb_registered+0x139/0x270
fbcon_fb_registered+0x3b/0x90
? fb_add_videomode+0x81/0xf0
register_framebuffer+0x22f/0x330
__drm_fb_helper_initial_config_and_unlock+0x349/0x520 [drm_kms_helper]
? kmalloc_large+0x25/0xc0
drm_fb_helper_initial_config+0x3f/0x50 [drm_kms_helper]
drm_fbdev_generic_client_hotplug+0x73/0xc0 [drm_kms_helper]
drm_fbdev_generic_setup+0x99/0x170 [drm_kms_helper]
Signed-off-by: Zack Rusin
Fixes: d39e48ca80c0 ("drm/atomic-helper: Set fence deadline for vblank")
Cc: Rob Clark
Cc: Daniel Vetter
---
drivers/gpu/drm/drm_vblank.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 299fa2a19a90..6438aeb1c65f 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -997,12 +997,16 @@ int drm_crtc_next_vblank_start(struct drm_crtc *crtc,
ktime_t *vblanktime)
{
unsigned int pipe = drm_crtc_index(crtc);
struct drm_vblank_crtc *vblank = >dev->vblank[pipe];
- struct drm_display_mode *mode = >hwmode;
+ struct drm_display_mode *mode;
u64 vblank_start;
+ if (!drm_dev_has_vblank(crtc->dev))
+ return -EINVAL;
+
if (!vblank->framedur_ns || !vblank->linedur_ns)
return -EINVAL;
+ mode = >hwmode;
if (!drm_crtc_get_last_vbltimestamp(crtc, vblanktime, false))
return -EINVAL;
Re: [PATCH 2/3] drm/vmwgfx: Print errors when running on broken/unsupported configs
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 21.03.23 г. 4:09 ч., Zack Rusin wrote:
From: Zack Rusin
virtualbox implemented an incomplete version of the svga device which
they decided to drop soon after the initial release. The device was
always broken in various ways and never supported by vmwgfx.
vmwgfx should refuse to load on those configurations but currently
drm has no way of reloading fbdev when the specific pci driver refuses
to load, which would leave users without a usable fb. Instead of
refusing to load print an error and disable a bunch of functionality
that virtualbox never implemented to at least get fb to work on their
setup.
Signed-off-by: Zack Rusin
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 29 +
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 ++
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 9 +
3 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 2588615a2a38..8b24ecf60e3e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -45,6 +45,9 @@
#include
#include
+#ifdef CONFIG_X86
+#include
+#endif
#include
#include
#include
@@ -897,6 +900,16 @@ static int vmw_driver_load(struct vmw_private *dev_priv,
u32 pci_id)
cap2_names, ARRAY_SIZE(cap2_names));
}
+ if (!vmwgfx_supported(dev_priv)) {
+ vmw_disable_backdoor();
+ drm_err_once(_priv->drm,
+"vmwgfx seems to be running on an unsupported
hypervisor.");
+ drm_err_once(_priv->drm,
+"This configuration is likely broken.");
+ drm_err_once(_priv->drm,
+"Please switch to a supported graphics device to avoid
problems.");
+ }
+
ret = vmw_dma_select_mode(dev_priv);
if (unlikely(ret != 0)) {
drm_info(_priv->drm,
@@ -1320,6 +1333,22 @@ static void vmw_master_drop(struct drm_device *dev,
vmw_kms_legacy_hotspot_clear(dev_priv);
}
+bool vmwgfx_supported(struct vmw_private *vmw)
+{
+#if defined(CONFIG_X86)
+ return hypervisor_is_type(X86_HYPER_VMWARE);
+#elif defined(CONFIG_ARM64)
+ /*
+* On aarch64 only svga3 is supported
+*/
+ return vmw->pci_id == VMWGFX_PCI_ID_SVGA3;
+#else
+ drm_warn_once(>drm,
+ "vmwgfx is running on an unknown architecture.");
+ return false;
+#endif
+}
+
/**
* __vmw_svga_enable - Enable SVGA mode, FIFO and use of VRAM.
*
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index fb8f0c0642c0..3810a9984a7f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -773,6 +773,7 @@ static inline u32 vmw_max_num_uavs(struct vmw_private
*dev_priv)
extern void vmw_svga_enable(struct vmw_private *dev_priv);
extern void vmw_svga_disable(struct vmw_private *dev_priv);
+bool vmwgfx_supported(struct vmw_private *vmw);
/**
@@ -1358,6 +1359,7 @@ int vmw_bo_cpu_blit(struct ttm_buffer_object *dst,
struct vmw_diff_cpy *diff);
/* Host messaging -vmwgfx_msg.c: */
+void vmw_disable_backdoor(void);
int vmw_host_get_guestinfo(const char *guest_info_param,
char *buffer, size_t *length);
__printf(1, 2) int vmw_host_printf(const char *fmt, ...);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index ca1a3fe44fa5..2651fe0ef518 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -1179,3 +1179,12 @@ int vmw_mksstat_remove_ioctl(struct drm_device *dev,
void *data,
return -EAGAIN;
}
+
+/**
+ * vmw_disable_backdoor: Disables all backdoor communication
+ * with the hypervisor.
+ */
+void vmw_disable_backdoor(void)
+{
+ vmw_msg_enabled = 0;
+}
Re: [PATCH] drm/vmwgfx: Fix src/dst_pitch confusion
From: Martin Krastev
We reviewers botched that one.
Reviewed-by: Martin Krastev
Regards,
Martin
On 14.03.23 г. 23:14 ч., Zack Rusin wrote:
From: Zack Rusin
The src/dst_pitch got mixed up during the rework of the function, make
sure the offset's refer to the correct one.
Spotted by clang:
Clang warns (or errors with CONFIG_WERROR):
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:509:29: error: variable 'dst_pitch' is
uninitialized when used here [-Werror,-Wuninitialized]
src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp;
^
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:492:26: note: initialize the variable
'dst_pitch' to silence this warning
s32 src_pitch, dst_pitch;
^
= 0
1 error generated.
Signed-off-by: Zack Rusin
Link: https://github.com/ClangBuiltLinux/linux/issues/1811
Reported-by: Nathan Chancellor
Reported-by: Dave Airlie
Fixes: 39985eea5a6d ("drm/vmwgfx: Abstract placement selection")
---
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index d79a6eccfaa4..ba0c0e12cfe9 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -506,11 +506,11 @@ static void vmw_stdu_bo_cpu_commit(struct vmw_kms_dirty
*dirty)
/* Assume we are blitting from Guest (bo) to Host (display_srf) */
src_pitch = stdu->display_srf->metadata.base_size.width * stdu->cpp;
src_bo = >display_srf->res.guest_memory_bo->tbo;
- src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp;
+ src_offset = ddirty->top * src_pitch + ddirty->left * stdu->cpp;
dst_pitch = ddirty->pitch;
dst_bo = >buf->tbo;
- dst_offset = ddirty->fb_top * src_pitch + ddirty->fb_left * stdu->cpp;
+ dst_offset = ddirty->fb_top * dst_pitch + ddirty->fb_left * stdu->cpp;
(void) vmw_bo_cpu_blit(dst_bo, dst_offset, dst_pitch,
src_bo, src_offset, src_pitch,
Re: [PATCH] drm/vmwgfx: Stop accessing buffer objects which failed init
From: Martin Krastev
Much nicer now.
Reviewed-by: Martin Krastev
Regards,
Martin
On 8.02.23 г. 20:00 ч., Zack Rusin wrote:
From: Zack Rusin
ttm_bo_init_reserved on failure puts the buffer object back which
causes it to be deleted, but kfree was still being called on the same
buffer in vmw_bo_create leading to a double free.
After the double free the vmw_gem_object_create_with_handle was
setting the gem function objects before checking the return status
of vmw_bo_create leading to null pointer access.
Fix the entire path by relaying on ttm_bo_init_reserved to delete the
buffer objects on failure and making sure the return status is checked
before setting the gem function objects on the buffer object.
Signed-off-by: Zack Rusin
Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM")
Cc: # v5.17+
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +++-
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 63486802c8fd..43ffa5c7acbd 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -431,13 +431,15 @@ int vmw_bo_create(struct vmw_private *vmw,
return -ENOMEM;
}
+ /*
+* vmw_bo_init will delete the *p_bo object if it fails
+*/
ret = vmw_bo_init(vmw, *p_bo, params, vmw_bo_free);
if (unlikely(ret != 0))
goto out_error;
return ret;
out_error:
- kfree(*p_bo);
*p_bo = NULL;
return ret;
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index f042e22b8b59..51bd1f8c5cc4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -127,11 +127,11 @@ int vmw_gem_object_create_with_handle(struct vmw_private
*dev_priv,
};
ret = vmw_bo_create(dev_priv, , p_vbo);
-
- (*p_vbo)->tbo.base.funcs = _gem_object_funcs;
if (ret != 0)
goto out_no_bo;
+ (*p_vbo)->tbo.base.funcs = _gem_object_funcs;
+
ret = drm_gem_handle_create(filp, &(*p_vbo)->tbo.base, handle);
/* drop reference from allocate - handle holds it now */
drm_gem_object_put(&(*p_vbo)->tbo.base);
Re: [PATCH v2] drm/vmwgfx: Do not drop the reference to the handle too soon
From: Martin Krastev
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
On 9.02.23 г. 14:37 ч., Zack Rusin wrote:
From: Zack Rusin
v2: Update the commit message to include note describing why the second
usag of vmw_gem_object_create_with_handle in vmwgfx_surface.c wasn't
changed
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.
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")
Cc: # v5.17+
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 1 -
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 43ffa5c7acbd..65bd88c8fef9 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -708,7 +708,8 @@ int vmw_dumb_create(struct drm_file *file_priv,
ret = vmw_gem_object_create_with_handle(dev_priv, file_priv,
args->size, >handle,
);
-
+ /* drop reference from allocate - handle holds it now */
+ drm_gem_object_put(>tbo.base);
return ret;
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index 51bd1f8c5cc4..d6baf73a6458 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -133,8 +133,6 @@ int vmw_gem_object_create_with_handle(struct vmw_private
*dev_priv,
(*p_vbo)->tbo.base.funcs = _gem_object_funcs;
ret = drm_gem_handle_create(filp, &(*p_vbo)->tbo.base, handle);
- /* drop reference from allocate - handle holds it now */
- drm_gem_object_put(&(*p_vbo)->tbo.base);
out_no_bo:
return ret;
}
@@ -161,6 +159,8 @@ int vmw_gem_object_create_ioctl(struct drm_device *dev,
void *data,
rep->map_handle = drm_vma_node_offset_addr(>tbo.base.vma_node);
rep->cur_gmr_id = handle;
rep->cur_gmr_offset = 0;
+ /* drop reference from allocate - handle holds it now */
+ drm_gem_object_put(>tbo.base);
out_no_bo:
return ret;
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 9d4ae9623a00..d18fec953fa7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -867,7 +867,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void
*data,
goto out_unlock;
}
vmw_bo_reference(res->guest_memory_bo);
- drm_gem_object_get(>guest_memory_bo->tbo.base);
}
tmp = vmw_resource_reference(>res);
Re: [PATCH] drm/vmwgfx: Do not drop the reference to the handle too soon
From: Martin Krastev
Looks good to me.
Reviewed-by: Martin Krastev
Regards,
Martin
On 8.02.23 г. 23:53 ч., Zack Rusin wrote:
From: Zack Rusin
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.
Signed-off-by: Zack Rusin
Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM")
Cc: # v5.17+
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 1 -
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 43ffa5c7acbd..65bd88c8fef9 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -708,7 +708,8 @@ int vmw_dumb_create(struct drm_file *file_priv,
ret = vmw_gem_object_create_with_handle(dev_priv, file_priv,
args->size, >handle,
);
-
+ /* drop reference from allocate - handle holds it now */
+ drm_gem_object_put(>tbo.base);
return ret;
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index 51bd1f8c5cc4..d6baf73a6458 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -133,8 +133,6 @@ int vmw_gem_object_create_with_handle(struct vmw_private
*dev_priv,
(*p_vbo)->tbo.base.funcs = _gem_object_funcs;
ret = drm_gem_handle_create(filp, &(*p_vbo)->tbo.base, handle);
- /* drop reference from allocate - handle holds it now */
- drm_gem_object_put(&(*p_vbo)->tbo.base);
out_no_bo:
return ret;
}
@@ -161,6 +159,8 @@ int vmw_gem_object_create_ioctl(struct drm_device *dev,
void *data,
rep->map_handle = drm_vma_node_offset_addr(>tbo.base.vma_node);
rep->cur_gmr_id = handle;
rep->cur_gmr_offset = 0;
+ /* drop reference from allocate - handle holds it now */
+ drm_gem_object_put(>tbo.base);
out_no_bo:
return ret;
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 9d4ae9623a00..d18fec953fa7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -867,7 +867,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void
*data,
goto out_unlock;
}
vmw_bo_reference(res->guest_memory_bo);
- drm_gem_object_get(>guest_memory_bo->tbo.base);
}
tmp = vmw_resource_reference(>res);
Re: [PATCH v2 8/8] drm/vmwgfx: Stop using raw ttm_buffer_object's
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 31.01.23 г. 5:35 ч., Zack Rusin wrote:
From: Zack Rusin
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
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c| 204 +---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.h| 60 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c| 44 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_context.c | 16 +-
drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 51 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 17 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 53 +++--
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 14 +-
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 37 +--
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 105 -
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 6 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_mob.c | 38 +--
drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c| 51 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 220 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource_priv.h | 7 +-
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 29 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_shader.c| 49 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_so.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 8 +-
drivers/gpu/drm/vmwgfx/vmwgfx_streamoutput.c | 8 +-
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 98
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c| 66 ++
drivers/gpu/drm/vmwgfx/vmwgfx_va.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_validation.c| 62 +++--
27 files changed, 566 insertions(+), 691 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index d8f6ccecf4bf..63486802c8fd 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -32,6 +32,12 @@
#include
+static void vmw_bo_release(struct vmw_bo *vbo)
+{
+ vmw_bo_unmap(vbo);
+ drm_gem_object_release(>tbo.base);
+}
+
/**
* vmw_bo_free - vmw_bo destructor
*
@@ -43,26 +49,10 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
WARN_ON(vbo->dirty);
WARN_ON(!RB_EMPTY_ROOT(>res_tree));
- vmw_bo_unmap(vbo);
- drm_gem_object_release(>base);
+ vmw_bo_release(vbo);
kfree(vbo);
}
-/**
- * bo_is_vmw - check if the buffer object is a _bo
- * @bo: ttm buffer object to be checked
- *
- * Uses destroy function associated with the object to determine if this is
- * a _bo.
- *
- * Returns:
- * true if the object is of _bo type, false if not.
- */
-static bool bo_is_vmw(struct ttm_buffer_object *bo)
-{
- return bo->destroy == _bo_free;
-}
-
/**
* vmw_bo_pin_in_placement - Validate a buffer to placement.
*
@@ -79,7 +69,7 @@ static int vmw_bo_pin_in_placement(struct vmw_private
*dev_priv,
bool interruptible)
{
struct ttm_operation_ctx ctx = {interruptible, false };
- struct ttm_buffer_object *bo = >base;
+ struct ttm_buffer_object *bo = >tbo;
int ret;
vmw_execbuf_release_pinned_bo(dev_priv);
@@ -88,7 +78,7 @@ static int vmw_bo_pin_in_placement(struct vmw_private
*dev_priv,
if (unlikely(ret != 0))
goto err;
- if (buf->base.pin_count > 0)
+ if (buf->tbo.pin_count > 0)
ret = ttm_resource_compat(bo->resource, placement)
? 0 : -EINVAL;
else
@@ -120,7 +110,7 @@ int vmw_bo_pin_in_vram_or_gmr(struct vmw_private *dev_priv,
bool interruptible)
{
struct ttm_operation_ctx ctx = {interruptible, false };
- struct ttm_buffer_object *bo = >base;
+ struct ttm_buffer_object *bo = >tbo;
int ret;
vmw_execbuf_release_pinned_bo(dev_priv);
@@ -129,7 +119,7 @@ int vmw_bo_pin_in_vram_or_gmr(struct vmw_private *dev_priv,
if (unlikely(ret != 0))
goto err;
- if (buf->base.pin_count > 0) {
+ if (buf->tbo.pin_count > 0) {
ret = ttm_resource_compat(bo->resource, _vram_gmr_placement)
? 0 : -EINVAL;
goto out_unreserve;
@@ -195,7 +185,7 @@ int
Re: [PATCH v2 6/8] drm/vmwgfx: Rename dummy to is_iomem
From: Martin Krastev
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
On 31.01.23 г. 5:35 ч., Zack Rusin wrote:
From: Zack Rusin
Rename dummy to is_iomem because that's what it is even if we're not
activelly using it. Makes the code easier to read.
Signed-off-by: Zack Rusin
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 1082218a1cfc..e83286e08837 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -153,9 +153,9 @@ static void vmw_cursor_update_mob(struct vmw_private
*dev_priv,
SVGAGBCursorHeader *header;
SVGAGBAlphaCursorHeader *alpha_header;
const u32 image_size = width * height * sizeof(*image);
- bool dummy;
+ bool is_iomem;
- header = ttm_kmap_obj_virtual(>cursor.map, );
+ header = ttm_kmap_obj_virtual(>cursor.map, _iomem);
alpha_header = >header.alphaHeader;
memset(header, 0, sizeof(*header));
@@ -185,13 +185,13 @@ static u32 vmw_du_cursor_mob_size(u32 w, u32 h)
*/
static u32 *vmw_du_cursor_plane_acquire_image(struct vmw_plane_state *vps)
{
- bool dummy;
+ bool is_iomem;
if (vps->surf) {
if (vps->surf_mapped)
return vmw_bo_map_and_cache(vps->surf->res.backup);
return vps->surf->snooper.image;
} else if (vps->bo)
- return ttm_kmap_obj_virtual(>bo->map, );
+ return ttm_kmap_obj_virtual(>bo->map, _iomem);
return NULL;
}
@@ -364,7 +364,7 @@ void vmw_kms_cursor_snoop(struct vmw_surface *srf,
SVGA3dCopyBox *box;
unsigned box_count;
void *virtual;
- bool dummy;
+ bool is_iomem;
struct vmw_dma_cmd {
SVGA3dCmdHeader header;
SVGA3dCmdSurfaceDMA dma;
@@ -424,7 +424,7 @@ void vmw_kms_cursor_snoop(struct vmw_surface *srf,
if (unlikely(ret != 0))
goto err_unreserve;
- virtual = ttm_kmap_obj_virtual(, );
+ virtual = ttm_kmap_obj_virtual(, _iomem);
if (box->w == VMW_CURSOR_SNOOP_WIDTH && cmd->dma.guest.pitch == image_pitch) {
memcpy(srf->snooper.image, virtual,
@@ -658,14 +658,14 @@ vmw_du_cursor_plane_cleanup_fb(struct drm_plane *plane,
{
struct vmw_cursor_plane *vcp = vmw_plane_to_vcp(plane);
struct vmw_plane_state *vps = vmw_plane_state_to_vps(old_state);
- bool dummy;
+ bool is_iomem;
if (vps->surf_mapped) {
vmw_bo_unmap(vps->surf->res.backup);
vps->surf_mapped = false;
}
- if (vps->bo && ttm_kmap_obj_virtual(>bo->map, )) {
+ if (vps->bo && ttm_kmap_obj_virtual(>bo->map, _iomem)) {
const int ret = ttm_bo_reserve(>bo->base, true, false,
NULL);
if (likely(ret == 0)) {
Re: [PATCH v2 7/8] drm/vmwgfx: Abstract placement selection
From: Martin Krastev
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
On 31.01.23 г. 5:35 ч., Zack Rusin wrote:
From: Zack Rusin
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
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c| 145 +++--
drivers/gpu/drm/vmwgfx/vmwgfx_bo.h| 25 +-
drivers/gpu/drm/vmwgfx/vmwgfx_context.c | 9 +-
drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 11 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 3 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 -
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 35 +-
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 5 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 21 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 11 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource_priv.h | 3 +-
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 13 +-
drivers/gpu/drm/vmwgfx/vmwgfx_shader.c| 15 +-
drivers/gpu/drm/vmwgfx/vmwgfx_so.c| 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 304 ++
drivers/gpu/drm/vmwgfx/vmwgfx_streamoutput.c | 3 +-
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 6 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c| 47 ---
drivers/gpu/drm/vmwgfx/vmwgfx_va.c| 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_validation.c| 74 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h| 6 +-
22 files changed, 312 insertions(+), 456 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index c6dc733f6d45..d8f6ccecf4bf 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -135,11 +135,17 @@ int vmw_bo_pin_in_vram_or_gmr(struct vmw_private
*dev_priv,
goto out_unreserve;
}
- ret = ttm_bo_validate(bo, _vram_gmr_placement, );
+ vmw_bo_placement_set(buf,
+VMW_BO_DOMAIN_GMR | VMW_BO_DOMAIN_VRAM,
+VMW_BO_DOMAIN_GMR);
+ ret = ttm_bo_validate(bo, >placement, );
if (likely(ret == 0) || ret == -ERESTARTSYS)
goto out_unreserve;
- ret = ttm_bo_validate(bo, _vram_placement, );
+ vmw_bo_placement_set(buf,
+VMW_BO_DOMAIN_VRAM,
+VMW_BO_DOMAIN_VRAM);
+ ret = ttm_bo_validate(bo, >placement, );
out_unreserve:
if (!ret)
@@ -190,17 +196,8 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private
*dev_priv,
{
struct ttm_operation_ctx ctx = {interruptible, false };
struct ttm_buffer_object *bo = >base;
- struct ttm_placement placement;
- struct ttm_place place;
int ret = 0;
- place = vmw_vram_placement.placement[0];
- place.lpfn = PFN_UP(bo->resource->size);
- placement.num_placement = 1;
- placement.placement =
- placement.num_busy_placement = 1;
- placement.busy_placement =
-
vmw_execbuf_release_pinned_bo(dev_priv);
ret = ttm_bo_reserve(bo, interruptible, false, NULL);
if (unlikely(ret != 0))
@@ -216,14 +213,21 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private
*dev_priv,
bo->resource->start > 0 &&
buf->base.pin_count == 0) {
ctx.interruptible = false;
- (void) ttm_bo_validate(bo, _sys_placement, );
+ vmw_bo_placement_set(buf,
+VMW_BO_DOMAIN_SYS,
+VMW_BO_DOMAIN_SYS);
+ (void)ttm_bo_validate(bo, >placement, );
}
+ vmw_bo_placement_set(buf,
+VMW_BO_DOMAIN_VRAM,
+VMW_BO_DOMAIN_VRAM);
+ buf->places[0].lpfn = PFN_UP(bo->resource->size);
if (buf->base.pin_count > 0)
- ret = ttm_resource_compat(bo->resource, )
+ ret = ttm_resource_compat(bo->resource, >placement)
? 0 : -EINVAL;
else
- ret = ttm_bo_validate(bo, , );
+ ret = ttm_bo_validate(bo, >placement, );
/* For some reason we didn't end up at the start of vram */
WARN_ON(ret == 0 && bo->resource->start != 0);
@@ -431,7 +435,7 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv,
unsigned long size,
}
int vmw_bo_create(struct vmw_private *vmw,
- size_t size,
Re: [PATCH v2 1/8] drm/vmwgfx: Use the common gem mmap instead of the custom code
From: Martin Krastev
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
On 31.01.23 г. 5:35 ч., Zack Rusin wrote:
From: Zack Rusin
Before vmwgfx supported gem it needed to implement the entire mmap logic
explicitly. With GEM support that's not needed and the generic code
can be used by simply setting the vm_ops to vmwgfx specific ones on the
gem object itself.
Removes a lot of code from vmwgfx without any functional difference.
Signed-off-by: Zack Rusin
Reviewed-by: Thomas Zimmermann
Reviewed-by: Martin Krastev
---
drivers/gpu/drm/vmwgfx/Makefile | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 6 --
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 8 ++
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 110 ---
5 files changed, 10 insertions(+), 118 deletions(-)
delete mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
index 2a644f035597..e94479d9cd5b 100644
--- a/drivers/gpu/drm/vmwgfx/Makefile
+++ b/drivers/gpu/drm/vmwgfx/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
- vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
+ vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o \
vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
vmwgfx_surface.o vmwgfx_prime.o vmwgfx_mob.o vmwgfx_shader.o \
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index bd02cb0e6837..e0c2e3748015 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1566,7 +1566,7 @@ static const struct file_operations vmwgfx_driver_fops = {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = vmw_unlocked_ioctl,
- .mmap = vmw_mmap,
+ .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
#if defined(CONFIG_COMPAT)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 5acbf5849b27..4dfa5044a9e7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1053,12 +1053,6 @@ vmw_is_cursor_bypass3_enabled(const struct vmw_private
*dev_priv)
return (vmw_fifo_caps(dev_priv) & SVGA_FIFO_CAP_CURSOR_BYPASS_3) != 0;
}
-/**
- * TTM glue - vmwgfx_ttm_glue.c
- */
-
-extern int vmw_mmap(struct file *filp, struct vm_area_struct *vma);
-
/**
* TTM buffer object driver - vmwgfx_ttm_buffer.c
*/
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index ce609e7d758f..ba4ddd9f7a7e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -103,6 +103,13 @@ static struct sg_table *vmw_gem_object_get_sg_table(struct
drm_gem_object *obj)
return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages,
vmw_tt->dma_ttm.num_pages);
}
+static const struct vm_operations_struct vmw_vm_ops = {
+ .pfn_mkwrite = vmw_bo_vm_mkwrite,
+ .page_mkwrite = vmw_bo_vm_mkwrite,
+ .fault = vmw_bo_vm_fault,
+ .open = ttm_bo_vm_open,
+ .close = ttm_bo_vm_close,
+};
static const struct drm_gem_object_funcs vmw_gem_object_funcs = {
.free = vmw_gem_object_free,
@@ -115,6 +122,7 @@ static const struct drm_gem_object_funcs
vmw_gem_object_funcs = {
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
.mmap = drm_gem_ttm_mmap,
+ .vm_ops = _vm_ops,
};
/**
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
deleted file mode 100644
index 265f7c48d856..
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ /dev/null
@@ -1,110 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0 OR MIT
-/**
- *
- * Copyright 2009-2011 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
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, 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
Re: [PATCH 5/7] drm/vmwgfx: Cleanup the vmw bo usage in the cursor paths
From: Martin Krastev
Ah, good!
Reviewed-by: Martin Krastev
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
From: Zack Rusin
Base mapped count is useless because the ttm unmap functions handle
null maps just fine so completely remove all the code related to it.
Rename dummy to is_iomem because that's what it is even if we're not
activelly using it. Makes the code easier to read.
Signed-off-by: Zack Rusin
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 4
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 28 +---
2 files changed, 9 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
index db85609ec01c..4dcf37235cb0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
@@ -44,7 +44,6 @@ struct vmw_resource;
* struct vmw_bo - TTM buffer object with vmwgfx additions
* @base: The TTM buffer object
* @res_tree: RB tree of resources using this buffer object as a backing MOB
- * @base_mapped_count: ttm BO mapping count; used by KMS atomic helpers.
* @cpu_writers: Number of synccpu write grabs. Protected by reservation when
* increased. May be decreased without reservation.
* @dx_query_ctx: DX context if this buffer object is used as a DX query MOB
@@ -55,8 +54,6 @@ struct vmw_resource;
struct vmw_bo {
struct ttm_buffer_object base;
struct rb_root res_tree;
- /* For KMS atomic helpers: ttm bo mapping count */
- atomic_t base_mapped_count;
atomic_t cpu_writers;
/* Not ref-counted. Protected by binding_mutex */
@@ -67,7 +64,6 @@ struct vmw_bo {
struct vmw_bo_dirty *dirty;
};
-
int vmw_bo_create_kernel(struct vmw_private *dev_priv,
unsigned long size,
struct ttm_placement *placement,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 6780391c57ea..e83286e08837 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -153,9 +153,9 @@ static void vmw_cursor_update_mob(struct vmw_private
*dev_priv,
SVGAGBCursorHeader *header;
SVGAGBAlphaCursorHeader *alpha_header;
const u32 image_size = width * height * sizeof(*image);
- bool dummy;
+ bool is_iomem;
- header = ttm_kmap_obj_virtual(>cursor.map, );
+ header = ttm_kmap_obj_virtual(>cursor.map, _iomem);
alpha_header = >header.alphaHeader;
memset(header, 0, sizeof(*header));
@@ -185,13 +185,13 @@ static u32 vmw_du_cursor_mob_size(u32 w, u32 h)
*/
static u32 *vmw_du_cursor_plane_acquire_image(struct vmw_plane_state *vps)
{
- bool dummy;
+ bool is_iomem;
if (vps->surf) {
if (vps->surf_mapped)
return vmw_bo_map_and_cache(vps->surf->res.backup);
return vps->surf->snooper.image;
} else if (vps->bo)
- return ttm_kmap_obj_virtual(>bo->map, );
+ return ttm_kmap_obj_virtual(>bo->map, _iomem);
return NULL;
}
@@ -364,7 +364,7 @@ void vmw_kms_cursor_snoop(struct vmw_surface *srf,
SVGA3dCopyBox *box;
unsigned box_count;
void *virtual;
- bool dummy;
+ bool is_iomem;
struct vmw_dma_cmd {
SVGA3dCmdHeader header;
SVGA3dCmdSurfaceDMA dma;
@@ -424,7 +424,7 @@ void vmw_kms_cursor_snoop(struct vmw_surface *srf,
if (unlikely(ret != 0))
goto err_unreserve;
- virtual = ttm_kmap_obj_virtual(, );
+ virtual = ttm_kmap_obj_virtual(, _iomem);
if (box->w == VMW_CURSOR_SNOOP_WIDTH && cmd->dma.guest.pitch == image_pitch) {
memcpy(srf->snooper.image, virtual,
@@ -658,19 +658,18 @@ vmw_du_cursor_plane_cleanup_fb(struct drm_plane *plane,
{
struct vmw_cursor_plane *vcp = vmw_plane_to_vcp(plane);
struct vmw_plane_state *vps = vmw_plane_state_to_vps(old_state);
- bool dummy;
+ bool is_iomem;
if (vps->surf_mapped) {
vmw_bo_unmap(vps->surf->res.backup);
vps->surf_mapped = false;
}
- if (vps->bo && ttm_kmap_obj_virtual(>bo->map, )) {
+ if (vps->bo && ttm_kmap_obj_virtual(>bo->map, _iomem)) {
const int ret = ttm_bo_reserve(>bo->base, true, false,
NULL);
if (likely(ret == 0)) {
- if (atomic_read(>bo->base_mapped_count) == 0)
- ttm_bo_kunmap(>bo->map);
+ ttm_bo_kunmap(>bo->map);
ttm_bo_unreserve(>bo->base);
}
}
@@ -744,9 +743,6 @@ vmw_du_cursor_plane_prepare_fb(struct drm_plane *plane,
ret = ttm_bo_kmap(>bo->base, 0, PFN_UP(size), >bo->map);
- if (likely(ret == 0))
- atomic_inc(>bo->base_mapped_count);
-
ttm_bo_unreserve(>bo->base);
if (unlikely(ret != 0))
@@ -786,7
Re: [PATCH 4/7] drm/vmwgfx: Simplify fb pinning
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
From: Zack Rusin
Only the legacy display unit requires pinning of the fb memory in vram.
Both the screen objects and screen targets can present from any buffer.
That makes the pinning abstraction pointless. Simplify all of the code
and move it to the legacy display unit, the only place that needs it.
Signed-off-by: Zack Rusin
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 8 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 4 --
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 66 -
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 57 +
5 files changed, 54 insertions(+), 85 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 586e1f1e9e49..fa289e67143d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -86,10 +86,10 @@ static bool bo_is_vmw(struct ttm_buffer_object *bo)
* Return: Zero on success, Negative error code on failure. In particular
* -ERESTARTSYS if interrupted by a signal
*/
-int vmw_bo_pin_in_placement(struct vmw_private *dev_priv,
- struct vmw_bo *buf,
- struct ttm_placement *placement,
- bool interruptible)
+static int vmw_bo_pin_in_placement(struct vmw_private *dev_priv,
+ struct vmw_bo *buf,
+ struct ttm_placement *placement,
+ bool interruptible)
{
struct ttm_operation_ctx ctx = {interruptible, false };
struct ttm_buffer_object *bo = >base;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
index 298406da1d79..db85609ec01c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
@@ -83,10 +83,6 @@ int vmw_bo_init(struct vmw_private *dev_priv,
int vmw_bo_unref_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
-int vmw_bo_pin_in_placement(struct vmw_private *vmw_priv,
- struct vmw_bo *bo,
- struct ttm_placement *placement,
- bool interruptible);
int vmw_bo_pin_in_vram(struct vmw_private *dev_priv,
struct vmw_bo *buf,
bool interruptible);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index ad41396c0a5d..6780391c57ea 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1487,69 +1487,6 @@ static const struct drm_framebuffer_funcs
vmw_framebuffer_bo_funcs = {
.dirty = vmw_framebuffer_bo_dirty_ext,
};
-/*
- * Pin the bofer in a location suitable for access by the
- * display system.
- */
-static int vmw_framebuffer_pin(struct vmw_framebuffer *vfb)
-{
- struct vmw_private *dev_priv = vmw_priv(vfb->base.dev);
- struct vmw_bo *buf;
- struct ttm_placement *placement;
- int ret;
-
- buf = vfb->bo ? vmw_framebuffer_to_vfbd(>base)->buffer :
- vmw_framebuffer_to_vfbs(>base)->surface->res.backup;
-
- if (!buf)
- return 0;
-
- switch (dev_priv->active_display_unit) {
- case vmw_du_legacy:
- vmw_overlay_pause_all(dev_priv);
- ret = vmw_bo_pin_in_start_of_vram(dev_priv, buf, false);
- vmw_overlay_resume_all(dev_priv);
- break;
- case vmw_du_screen_object:
- case vmw_du_screen_target:
- if (vfb->bo) {
- if (dev_priv->capabilities & SVGA_CAP_3D) {
- /*
-* Use surface DMA to get content to
-* sreen target surface.
-*/
- placement = _vram_gmr_placement;
- } else {
- /* Use CPU blit. */
- placement = _sys_placement;
- }
- } else {
- /* Use surface / image update */
- placement = _mob_placement;
- }
-
- return vmw_bo_pin_in_placement(dev_priv, buf, placement, false);
- default:
- return -EINVAL;
- }
-
- return ret;
-}
-
-static int vmw_framebuffer_unpin(struct vmw_framebuffer *vfb)
-{
- struct vmw_private *dev_priv = vmw_priv(vfb->base.dev);
- struct vmw_bo *buf;
-
- buf = vfb->bo ? vmw_framebuffer_to_vfbd(>base)->buffer :
- vmw_framebuffer_to_vfbs(>base)->surface->res.backup;
-
- if (WARN_ON(!buf))
- return 0;
-
- return vmw_bo_unpin(dev_priv, buf, false);
-}
-
/**
*
Re: [PATCH 3/7] drm/vmwgfx: Rename vmw_buffer_object to vmw_bo
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
From: Zack Rusin
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
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 91 +
drivers/gpu/drm/vmwgfx/vmwgfx_bo.h | 191 +++
drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c | 10 +-
drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c | 9 +-
drivers/gpu/drm/vmwgfx/vmwgfx_context.c | 11 +-
drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 9 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 7 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 182 ++
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 53 +++--
drivers/gpu/drm/vmwgfx/vmwgfx_fence.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 26 +--
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 17 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 12 +-
drivers/gpu/drm/vmwgfx/vmwgfx_mob.c | 7 +-
drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c | 18 +-
drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 27 +--
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 29 +--
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 15 +-
drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 15 +-
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 13 +-
drivers/gpu/drm/vmwgfx/vmwgfx_streamoutput.c | 9 +-
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 9 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 3 +-
drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 30 +--
drivers/gpu/drm/vmwgfx/vmwgfx_validation.h | 6 +-
25 files changed, 431 insertions(+), 370 deletions(-)
create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 8aaeeecd2016..586e1f1e9e49 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0 OR MIT
/**
*
- * Copyright © 2011-2018 VMware, Inc., Palo Alto, CA., USA
+ * Copyright © 2011-2023 VMware, Inc., Palo Alto, CA., USA
* All Rights Reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a
@@ -26,55 +26,54 @@
*
**/
-#include
-
+#include "vmwgfx_bo.h"
#include "vmwgfx_drv.h"
-#include "ttm_object.h"
+#include
+
/**
- * vmw_buffer_object - Convert a struct ttm_buffer_object to a struct
- * vmw_buffer_object.
+ * vmw_bo - Convert a struct ttm_buffer_object to a struct vmw_bo.
*
* @bo: Pointer to the TTM buffer object.
- * Return: Pointer to the struct vmw_buffer_object embedding the
+ * Return: Pointer to the struct vmw_bo embedding the
* TTM buffer object.
*/
-static struct vmw_buffer_object *
-vmw_buffer_object(struct ttm_buffer_object *bo)
+static struct vmw_bo *
+vmw_bo(struct ttm_buffer_object *bo)
{
- return container_of(bo, struct vmw_buffer_object, base);
+ return container_of(bo, struct vmw_bo, base);
}
/**
- * vmw_bo_bo_free - vmw buffer object destructor
+ * vmw_bo_free - vmw_bo destructor
*
* @bo: Pointer to the embedded struct ttm_buffer_object
*/
-static void vmw_bo_bo_free(struct ttm_buffer_object *bo)
+static void vmw_bo_free(struct ttm_buffer_object *bo)
{
- struct vmw_buffer_object *vmw_bo = vmw_buffer_object(bo);
+ struct vmw_bo *vbo = vmw_bo(bo);
- WARN_ON(vmw_bo->dirty);
- WARN_ON(!RB_EMPTY_ROOT(_bo->res_tree));
- vmw_bo_unmap(vmw_bo);
+ WARN_ON(vbo->dirty);
+ WARN_ON(!RB_EMPTY_ROOT(>res_tree));
+ vmw_bo_unmap(vbo);
drm_gem_object_release(>base);
- kfree(vmw_bo);
+ kfree(vbo);
}
/**
- * bo_is_vmw - check if the buffer object is a _buffer_object
+ * bo_is_vmw - check if the buffer object is a _bo
* @bo: ttm buffer object to be checked
*
* Uses destroy function associated with the object to determine if this is
- * a _buffer_object.
+ * a _bo.
*
* Returns:
- * true if the object is of _buffer_object type, false if not.
+ * true if the object is of _bo type, false if not.
*/
static bool bo_is_vmw(struct ttm_buffer_object *bo)
{
- return bo->destroy == _bo_bo_free;
+ return bo->destroy == _bo_free;
}
/**
@@ -88,7 +87,7 @@ static bool bo_is_vmw(struct ttm_buffer_object *bo)
* -ERESTARTSYS if interrupted by a signal
*/
int vmw_bo_pin_in_placement(struct vmw_private *dev_priv,
- struct vmw_buffer_object
Re: [PATCH 2/7] drm/vmwgfx: Remove the duplicate bo_free function
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
From: Zack Rusin
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
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 49 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 3 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 6 +--
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 18 +
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 3 +-
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 2 +-
8 files changed, 27 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index aa1cd5126a32..8aaeeecd2016 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -46,6 +46,22 @@ vmw_buffer_object(struct ttm_buffer_object *bo)
return container_of(bo, struct vmw_buffer_object, base);
}
+/**
+ * vmw_bo_bo_free - vmw buffer object destructor
+ *
+ * @bo: Pointer to the embedded struct ttm_buffer_object
+ */
+static void vmw_bo_bo_free(struct ttm_buffer_object *bo)
+{
+ struct vmw_buffer_object *vmw_bo = vmw_buffer_object(bo);
+
+ WARN_ON(vmw_bo->dirty);
+ WARN_ON(!RB_EMPTY_ROOT(_bo->res_tree));
+ vmw_bo_unmap(vmw_bo);
+ drm_gem_object_release(>base);
+ kfree(vmw_bo);
+}
+
/**
* bo_is_vmw - check if the buffer object is a _buffer_object
* @bo: ttm buffer object to be checked
@@ -58,8 +74,7 @@ vmw_buffer_object(struct ttm_buffer_object *bo)
*/
static bool bo_is_vmw(struct ttm_buffer_object *bo)
{
- return bo->destroy == _bo_bo_free ||
- bo->destroy == _gem_destroy;
+ return bo->destroy == _bo_bo_free;
}
/**
@@ -376,23 +391,6 @@ void vmw_bo_unmap(struct vmw_buffer_object *vbo)
ttm_bo_kunmap(>map);
}
-
-/**
- * vmw_bo_bo_free - vmw buffer object destructor
- *
- * @bo: Pointer to the embedded struct ttm_buffer_object
- */
-void vmw_bo_bo_free(struct ttm_buffer_object *bo)
-{
- struct vmw_buffer_object *vmw_bo = vmw_buffer_object(bo);
-
- WARN_ON(vmw_bo->dirty);
- WARN_ON(!RB_EMPTY_ROOT(_bo->res_tree));
- vmw_bo_unmap(vmw_bo);
- drm_gem_object_release(>base);
- kfree(vmw_bo);
-}
-
/* default destructor */
static void vmw_bo_default_destroy(struct ttm_buffer_object *bo)
{
@@ -449,13 +447,10 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv,
unsigned long size,
int vmw_bo_create(struct vmw_private *vmw,
size_t size, struct ttm_placement *placement,
bool interruptible, bool pin,
- void (*bo_free)(struct ttm_buffer_object *bo),
struct vmw_buffer_object **p_bo)
{
int ret;
- BUG_ON(!bo_free);
-
*p_bo = kmalloc(sizeof(**p_bo), GFP_KERNEL);
if (unlikely(!*p_bo)) {
DRM_ERROR("Failed to allocate a buffer.\n");
@@ -463,8 +458,7 @@ int vmw_bo_create(struct vmw_private *vmw,
}
ret = vmw_bo_init(vmw, *p_bo, size,
- placement, interruptible, pin,
- bo_free);
+ placement, interruptible, pin);
if (unlikely(ret != 0))
goto out_error;
@@ -484,7 +478,6 @@ int vmw_bo_create(struct vmw_private *vmw,
* @placement: Initial placement.
* @interruptible: Whether waits should be performed interruptible.
* @pin: If the BO should be created pinned at a fixed location.
- * @bo_free: The buffer object destructor.
* Returns: Zero on success, negative error code on error.
*
* Note that on error, the code will free the buffer object.
@@ -492,8 +485,7 @@ int vmw_bo_create(struct vmw_private *vmw,
int vmw_bo_init(struct vmw_private *dev_priv,
struct vmw_buffer_object *vmw_bo,
size_t size, struct ttm_placement *placement,
- bool interruptible, bool pin,
- void (*bo_free)(struct ttm_buffer_object *bo))
+ bool interruptible, bool pin)
{
struct ttm_operation_ctx ctx = {
.interruptible = interruptible,
@@ -503,7 +495,6 @@ int vmw_bo_init(struct vmw_private *dev_priv,
struct drm_device *vdev = _priv->drm;
int ret;
- WARN_ON_ONCE(!bo_free);
memset(vmw_bo, 0, sizeof(*vmw_bo));
BUILD_BUG_ON(TTM_MAX_BO_PRIORITY <= 3);
vmw_bo->base.priority = 3;
@@ -513,7 +504,7 @@ int vmw_bo_init(struct vmw_private *dev_priv,
drm_gem_private_object_init(vdev, _bo->base.base, size);
ret = ttm_bo_init_reserved(bdev, _bo->base,
Re: [PATCH 1/7] drm/vmwgfx: Use the common gem mmap instead of the custom code
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
From: Zack Rusin
Before vmwgfx supported gem it needed to implement the entire mmap logic
explicitly. With GEM support that's not needed and the generic code
can be used by simply setting the vm_ops to vmwgfx specific ones on the
gem object itself.
Removes a lot of code from vmwgfx without any functional difference.
Signed-off-by: Zack Rusin
---
drivers/gpu/drm/vmwgfx/Makefile | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 6 --
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 8 ++
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 110 ---
5 files changed, 10 insertions(+), 118 deletions(-)
delete mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
index 2a644f035597..e94479d9cd5b 100644
--- a/drivers/gpu/drm/vmwgfx/Makefile
+++ b/drivers/gpu/drm/vmwgfx/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
- vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
+ vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o \
vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
vmwgfx_surface.o vmwgfx_prime.o vmwgfx_mob.o vmwgfx_shader.o \
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index bd02cb0e6837..e0c2e3748015 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1566,7 +1566,7 @@ static const struct file_operations vmwgfx_driver_fops = {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = vmw_unlocked_ioctl,
- .mmap = vmw_mmap,
+ .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
#if defined(CONFIG_COMPAT)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 5acbf5849b27..4dfa5044a9e7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1053,12 +1053,6 @@ vmw_is_cursor_bypass3_enabled(const struct vmw_private
*dev_priv)
return (vmw_fifo_caps(dev_priv) & SVGA_FIFO_CAP_CURSOR_BYPASS_3) != 0;
}
-/**
- * TTM glue - vmwgfx_ttm_glue.c
- */
-
-extern int vmw_mmap(struct file *filp, struct vm_area_struct *vma);
-
/**
* TTM buffer object driver - vmwgfx_ttm_buffer.c
*/
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index ce609e7d758f..ba4ddd9f7a7e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -103,6 +103,13 @@ static struct sg_table *vmw_gem_object_get_sg_table(struct
drm_gem_object *obj)
return drm_prime_pages_to_sg(obj->dev, vmw_tt->dma_ttm.pages,
vmw_tt->dma_ttm.num_pages);
}
+static const struct vm_operations_struct vmw_vm_ops = {
+ .pfn_mkwrite = vmw_bo_vm_mkwrite,
+ .page_mkwrite = vmw_bo_vm_mkwrite,
+ .fault = vmw_bo_vm_fault,
+ .open = ttm_bo_vm_open,
+ .close = ttm_bo_vm_close,
+};
static const struct drm_gem_object_funcs vmw_gem_object_funcs = {
.free = vmw_gem_object_free,
@@ -115,6 +122,7 @@ static const struct drm_gem_object_funcs
vmw_gem_object_funcs = {
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
.mmap = drm_gem_ttm_mmap,
+ .vm_ops = _vm_ops,
};
/**
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
deleted file mode 100644
index 265f7c48d856..
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ /dev/null
@@ -1,110 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0 OR MIT
-/**
- *
- * Copyright 2009-2011 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
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, 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 NON-INFRINGEMENT. IN NO EVENT SHALL
- *
Re: [PATCH] drm/ttm: Fix a regression causing kernel oops'es
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 11.01.23 г. 19:50 ч., Zack Rusin wrote:
From: Zack Rusin
The branch is explicitly taken if ttm == NULL which means that to avoid
a null pointer reference the ttm object can not be used inside. Switch
back to dst_mem to avoid kernel oops'es.
This fixes kernel oops'es with any buffer objects which don't have ttm_tt,
e.g. with vram based screen objects on vmwgfx.
Signed-off-by: Zack Rusin
Fixes: e3c92eb4a84f ("drm/ttm: rework on ttm_resource to use size_t type")
Cc: Somalapuram Amaranath
Cc: Christian König
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index fee7c20775c0..12017ec24d9f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -183,7 +183,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
clear = src_iter->ops->maps_tt && (!ttm || !ttm_tt_is_populated(ttm));
if (!(clear && ttm && !(ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC)))
- ttm_move_memcpy(clear, ttm->num_pages, dst_iter, src_iter);
+ ttm_move_memcpy(clear, PFN_UP(dst_mem->size), dst_iter,
src_iter);
if (!src_iter->ops->maps_tt)
ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, src_mem);
Re: [PATCH] drm/vmwgfx: Remove rcu locks from user resources
From: Martin Krastev
Looks good!
Reviewed-by: Martin Krastev
Regards,
Martin
On 7.12.22 г. 19:29 ч., Zack Rusin wrote:
From: Zack Rusin
User resource lookups used rcu to avoid two extra atomics. Unfortunately
the rcu paths were buggy and it was easy to make the driver crash by
submitting command buffers from two different threads. Because the
lookups never show up in performance profiles replace them with a
regular spin lock which fixes the races in accesses to those shared
resources.
Fixes kernel oops'es in IGT's vmwgfx execution_buffer stress test and
seen crashes with apps using shared resources.
Fixes: e14c02e6b699 ("drm/vmwgfx: Look up objects without taking a reference")
Signed-off-by: Zack Rusin
---
drivers/gpu/drm/vmwgfx/ttm_object.c | 41 +-
drivers/gpu/drm/vmwgfx/ttm_object.h | 14 --
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 38 -
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 18 +--
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 176 +++
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 33 -
6 files changed, 87 insertions(+), 233 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.c
b/drivers/gpu/drm/vmwgfx/ttm_object.c
index 932b125ebf3d..ddf8373c1d77 100644
--- a/drivers/gpu/drm/vmwgfx/ttm_object.c
+++ b/drivers/gpu/drm/vmwgfx/ttm_object.c
@@ -254,40 +254,6 @@ void ttm_base_object_unref(struct ttm_base_object **p_base)
kref_put(>refcount, ttm_release_base);
}
-/**
- * ttm_base_object_noref_lookup - look up a base object without reference
- * @tfile: The struct ttm_object_file the object is registered with.
- * @key: The object handle.
- *
- * This function looks up a ttm base object and returns a pointer to it
- * without refcounting the pointer. The returned pointer is only valid
- * until ttm_base_object_noref_release() is called, and the object
- * pointed to by the returned pointer may be doomed. Any persistent usage
- * of the object requires a refcount to be taken using kref_get_unless_zero().
- * Iff this function returns successfully it needs to be paired with
- * ttm_base_object_noref_release() and no sleeping- or scheduling functions
- * may be called inbetween these function callse.
- *
- * Return: A pointer to the object if successful or NULL otherwise.
- */
-struct ttm_base_object *
-ttm_base_object_noref_lookup(struct ttm_object_file *tfile, uint64_t key)
-{
- struct vmwgfx_hash_item *hash;
- int ret;
-
- rcu_read_lock();
- ret = ttm_tfile_find_ref_rcu(tfile, key, );
- if (ret) {
- rcu_read_unlock();
- return NULL;
- }
-
- __release(RCU);
- return hlist_entry(hash, struct ttm_ref_object, hash)->obj;
-}
-EXPORT_SYMBOL(ttm_base_object_noref_lookup);
-
struct ttm_base_object *ttm_base_object_lookup(struct ttm_object_file *tfile,
uint64_t key)
{
@@ -295,15 +261,16 @@ struct ttm_base_object *ttm_base_object_lookup(struct
ttm_object_file *tfile,
struct vmwgfx_hash_item *hash;
int ret;
- rcu_read_lock();
- ret = ttm_tfile_find_ref_rcu(tfile, key, );
+ spin_lock(>lock);
+ ret = ttm_tfile_find_ref(tfile, key, );
if (likely(ret == 0)) {
base = hlist_entry(hash, struct ttm_ref_object, hash)->obj;
if (!kref_get_unless_zero(>refcount))
base = NULL;
}
- rcu_read_unlock();
+ spin_unlock(>lock);
+
return base;
}
diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.h
b/drivers/gpu/drm/vmwgfx/ttm_object.h
index f0ebbe340ad6..8098a3846bae 100644
--- a/drivers/gpu/drm/vmwgfx/ttm_object.h
+++ b/drivers/gpu/drm/vmwgfx/ttm_object.h
@@ -307,18 +307,4 @@ extern int ttm_prime_handle_to_fd(struct ttm_object_file
*tfile,
#define ttm_prime_object_kfree(__obj, __prime)\
kfree_rcu(__obj, __prime.base.rhead)
-struct ttm_base_object *
-ttm_base_object_noref_lookup(struct ttm_object_file *tfile, uint64_t key);
-
-/**
- * ttm_base_object_noref_release - release a base object pointer looked up
- * without reference
- *
- * Releases a base object pointer looked up with
ttm_base_object_noref_lookup().
- */
-static inline void ttm_base_object_noref_release(void)
-{
- __acquire(RCU);
- rcu_read_unlock();
-}
#endif
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index d218b15953e0..d579f3eee9af 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -715,44 +715,6 @@ int vmw_user_bo_lookup(struct drm_file *filp,
return 0;
}
-/**
- * vmw_user_bo_noref_lookup - Look up a vmw user buffer object without
reference
- * @filp: The TTM object file the handle is registered with.
- * @handle: The user buffer object handle.
- *
- * This function looks up a struct vmw_bo and returns a pointer to the
- * struct vmw_buffer_object it derives from without refcounting the
Re: [PATCH] drm/vmwgfx: Don't use screen objects when SEV is active
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 1.12.22 г. 19:53 ч., Zack Rusin wrote:
From: Zack Rusin
When SEV is enabled gmr's and mob's are explicitly disabled because
the encrypted system memory can not be used by the hypervisor.
The driver was disabling GMR's but the presentation code, which depends
on GMR's, wasn't honoring it which lead to black screen on hosts
with SEV enabled.
Make sure screen objects presentation is not used when guest memory
regions have been disabled to fix presentation on SEV enabled hosts.
Fixes: 3b0d6458c705 ("drm/vmwgfx: Refuse DMA operation when SEV encryption is
active")
Cc: # v5.7+
Signed-off-by: Zack Rusin
Reported-by: Nicholas Hunt
---
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index 8db61c541a80..e1f36a09c59c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -926,6 +926,10 @@ int vmw_kms_sou_init_display(struct vmw_private *dev_priv)
struct drm_device *dev = _priv->drm;
int i;
+ /* Screen objects won't work if GMR's aren't available */
+ if (!dev_priv->has_gmr)
+ return -ENOSYS;
+
if (!(dev_priv->capabilities & SVGA_CAP_SCREEN_OBJECT_2)) {
return -ENOSYS;
}
Re: [PATCH v3] drm/vmwgfx: Fix race issue calling pin_user_pages
From: Martin Krastev
Looks great!
Reviewed-by: Martin Krastev
Regards,
Martin
On 9.11.22 г. 17:37 ч., Dawei Li wrote:
pin_user_pages() is unsafe without protection of mmap_lock,
fix it by calling pin_user_pages_fast().
Fixes: 7a7a933edd6c ("drm/vmwgfx: Introduce VMware mks-guest-stats")
Signed-off-by: Dawei Li
---
v1:
https://lore.kernel.org/all/tycp286mb23235c9a9fcf85c045f95ea7ca...@tycp286mb2323.jpnp286.prod.outlook.com/
v1->v2:
Rebased to latest vmwgfx/drm-misc-fixes.
v2->v3
Replace pin_user_pages() with pin_user_pages_fast().
---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 089046fa21be..50fa3df0bc0c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -1085,21 +1085,21 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
reset_ppn_array(pdesc->strsPPNs, ARRAY_SIZE(pdesc->strsPPNs));
/* Pin mksGuestStat user pages and store those in the instance descriptor */
- nr_pinned_stat = pin_user_pages(arg->stat, num_pages_stat,
FOLL_LONGTERM, pages_stat, NULL);
+ nr_pinned_stat = pin_user_pages_fast(arg->stat, num_pages_stat,
FOLL_LONGTERM, pages_stat);
if (num_pages_stat != nr_pinned_stat)
goto err_pin_stat;
for (i = 0; i < num_pages_stat; ++i)
pdesc->statPPNs[i] = page_to_pfn(pages_stat[i]);
- nr_pinned_info = pin_user_pages(arg->info, num_pages_info, FOLL_LONGTERM, pages_info, NULL);
+ nr_pinned_info = pin_user_pages_fast(arg->info, num_pages_info,
FOLL_LONGTERM, pages_info);
if (num_pages_info != nr_pinned_info)
goto err_pin_info;
for (i = 0; i < num_pages_info; ++i)
pdesc->infoPPNs[i] = page_to_pfn(pages_info[i]);
- nr_pinned_strs = pin_user_pages(arg->strs, num_pages_strs, FOLL_LONGTERM, pages_strs, NULL);
+ nr_pinned_strs = pin_user_pages_fast(arg->strs, num_pages_strs,
FOLL_LONGTERM, pages_strs);
if (num_pages_strs != nr_pinned_strs)
goto err_pin_strs;
Re: [PATCH v2] drm/vmwgfx: Protect pin_user_pages with mmap_lock
From: Martin Krastev
Thanks for the catch. LGTM, with the following remarks:
1) Original design used erroneously pin_user_pages() in place of
pin_user_pages_fast(); you could just substitute pin_user_pages for
pin_user_pages_fast and call it a day, Please, consider that option
after reading (2) below.
2) Re exception handling in vmw_mksstat_add_ioctl(), the 'incorrect
exception handling' would be incorrect in the context of the new
refactor, i.e. with a common entry point to all pin_user_pages()
exceptions; it was correct originally, with dedicated entry points, as
all nr_pinned_* were used only after being assigned.
Basically, you could keep everything as it was and just do the
substitution suggested in (1) and the patch would be as good.
Regards,
Martin
On 6.11.22 г. 17:47 ч., Dawei Li wrote:
This patch includes changes below:
1) pin_user_pages() is unsafe without protection of mmap_lock,
fix it by calling mmap_read_lock() & mmap_read_unlock().
2) fix & refactor the incorrect exception handling procedure in
vmw_mksstat_add_ioctl().
Fixes: 7a7a933edd6c ("drm/vmwgfx: Introduce VMware mks-guest-stats")
Signed-off-by: Dawei Li
---
v1:
https://lore.kernel.org/all/tycp286mb23235c9a9fcf85c045f95ea7ca...@tycp286mb2323.jpnp286.prod.outlook.com/
v1->v2:
Rebased to latest vmwgfx/drm-misc-fixes
---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 089046fa21be..ec40a3364e0a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -1020,9 +1020,9 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
const size_t num_pages_info = PFN_UP(arg->info_len);
const size_t num_pages_strs = PFN_UP(arg->strs_len);
long desc_len;
- long nr_pinned_stat;
- long nr_pinned_info;
- long nr_pinned_strs;
+ long nr_pinned_stat = 0;
+ long nr_pinned_info = 0;
+ long nr_pinned_strs = 0;
struct page *pages_stat[ARRAY_SIZE(pdesc->statPPNs)];
struct page *pages_info[ARRAY_SIZE(pdesc->infoPPNs)];
struct page *pages_strs[ARRAY_SIZE(pdesc->strsPPNs)];
@@ -1084,28 +1084,33 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
reset_ppn_array(pdesc->infoPPNs, ARRAY_SIZE(pdesc->infoPPNs));
reset_ppn_array(pdesc->strsPPNs, ARRAY_SIZE(pdesc->strsPPNs));
+ /* pin_user_pages() needs protection of mmap_lock */
+ mmap_read_lock(current->mm);
+
/* Pin mksGuestStat user pages and store those in the instance
descriptor */
nr_pinned_stat = pin_user_pages(arg->stat, num_pages_stat,
FOLL_LONGTERM, pages_stat, NULL);
if (num_pages_stat != nr_pinned_stat)
- goto err_pin_stat;
+ goto __err_pin_pages;
for (i = 0; i < num_pages_stat; ++i)
pdesc->statPPNs[i] = page_to_pfn(pages_stat[i]);
nr_pinned_info = pin_user_pages(arg->info, num_pages_info, FOLL_LONGTERM, pages_info, NULL);
if (num_pages_info != nr_pinned_info)
- goto err_pin_info;
+ goto __err_pin_pages;
for (i = 0; i < num_pages_info; ++i)
pdesc->infoPPNs[i] = page_to_pfn(pages_info[i]);
nr_pinned_strs = pin_user_pages(arg->strs, num_pages_strs, FOLL_LONGTERM, pages_strs, NULL);
if (num_pages_strs != nr_pinned_strs)
- goto err_pin_strs;
+ goto __err_pin_pages;
for (i = 0; i < num_pages_strs; ++i)
pdesc->strsPPNs[i] = page_to_pfn(pages_strs[i]);
+ mmap_read_unlock(current->mm);
+
/* Send the descriptor to the host via a hypervisor call. The
mksGuestStat
pages will remain in use until the user requests a matching remove
stats
or a stats reset occurs. */
@@ -1120,15 +1125,15 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
return 0;
-err_pin_strs:
+__err_pin_pages:
+ mmap_read_unlock(current->mm);
+
if (nr_pinned_strs > 0)
unpin_user_pages(pages_strs, nr_pinned_strs);
-err_pin_info:
if (nr_pinned_info > 0)
unpin_user_pages(pages_info, nr_pinned_info);
-err_pin_stat:
if (nr_pinned_stat > 0)
unpin_user_pages(pages_stat, nr_pinned_stat);
Re: [PATCH v3 17/17] drm/vmwgfx: Fix a sparse warning in kernel docs
From: Martin Krastev
On 21.10.22 г. 6:44 ч., Zack Rusin wrote:
From: Zack Rusin
Fixes a warning about extra docs about a function argument that has been
removed a while back:
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c:3888: warning: Excess function
parameter 'sync_file' description in 'vmw_execbuf_copy_fence_user'
Fixes: a0f90c881570 ("drm/vmwgfx: Fix stale file descriptors on failed
usercopy")
Signed-off-by: Zack Rusin
---
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index c943ab801ca7..f16fc489d725 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -3869,7 +3869,6 @@ int vmw_execbuf_fence_commands(struct drm_file *file_priv,
* @fence: Pointer to the fenc object.
* @fence_handle: User-space fence handle.
* @out_fence_fd: exported file descriptor for the fence. -1 if not used
- * @sync_file: Only used to clean up in case of an error in this function.
*
* This function copies fence information to user-space. If copying fails, the
* user-space struct drm_vmw_fence_rep::error member is hopefully left
Phantom arg doc goodbye!
Reviewed-by: Martin Krastev
Regards,
Martin
Re: [PATCH] drm/vmwgfx: Fix memory leak in vmw_mksstat_add_ioctl()
Thank you for the catch!
Reviewed-by: Martin Krastev
Regards,
Martin
On 16.09.22 г. 23:47 ч., Rafael Mendonca wrote:
If the copy of the description string from userspace fails, then the page
for the instance descriptor doesn't get freed before returning -EFAULT,
which leads to a memleak.
Fixes: 7a7a933edd6c ("drm/vmwgfx: Introduce VMware mks-guest-stats")
Signed-off-by: Rafael Mendonca
---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 2aceac7856e2..089046fa21be 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -1076,6 +1076,7 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void
*data,
if (desc_len < 0) {
atomic_set(_priv->mksstat_user_pids[slot], 0);
+ __free_page(page);
return -EFAULT;
}
Re: [PATCH v2 3/8] drm/vmwgfx: Use the hotspot properties from cursor planes
From: Martin Krastev
On 12.07.22 г. 6:32 ч., Zack Rusin wrote:
From: Zack Rusin
Atomic modesetting got support for mouse hotspots via the hotspot
properties. Port the legacy kms hotspot handling to the new properties
on cursor planes.
Signed-off-by: Zack Rusin
Cc: Martin Krastev
Cc: Maaz Mombasawala
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index ff2f735bbe7a..3d3f73109199 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -652,13 +652,8 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
s32 hotspot_x, hotspot_y;
- hotspot_x = du->hotspot_x;
- hotspot_y = du->hotspot_y;
-
- if (new_state->fb) {
- hotspot_x += new_state->fb->hot_x;
- hotspot_y += new_state->fb->hot_y;
- }
+ hotspot_x = du->hotspot_x + new_state->hotspot_x;
+ hotspot_y = du->hotspot_y + new_state->hotspot_y;
du->cursor_surface = vps->surf;
du->cursor_bo = vps->bo;
LGTM.
Reviewed-by: Martin Krastev
Regards,
Martin
Re: [PATCH 2/6] drm/vmwgfx: Create mouse hotspot properties on cursor planes
From: Martin Krastev
On 2.06.22 г. 18:42 ч., Zack Rusin wrote:
From: Zack Rusin
Atomic modesetting got support for mouse hotspots via the hotspot
properties. Drivers need to create those properties on cursor planes
which require the mouse hotspot coordinates.
Add the code creating hotspot properties and port away from old legacy
hotspot API. The legacy hotspot paths have an implementation that works
with new atomic properties so there's no reason to keep them and it
makes sense to unify both paths.
Signed-off-by: Zack Rusin
Cc: Martin Krastev
Cc: Maaz Mombasawala
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 11 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 2 ++
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 1 +
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 ++
4 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 693028c31b6b..a4cd312fee46 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -652,13 +652,8 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
s32 hotspot_x, hotspot_y;
- hotspot_x = du->hotspot_x;
- hotspot_y = du->hotspot_y;
-
- if (new_state->fb) {
- hotspot_x += new_state->fb->hot_x;
- hotspot_y += new_state->fb->hot_y;
- }
+ hotspot_x = du->hotspot_x + new_state->hotspot_x;
+ hotspot_y = du->hotspot_y + new_state->hotspot_y;
du->cursor_surface = vps->surf;
du->cursor_bo = vps->bo;
@@ -2270,8 +2265,6 @@ int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
int i;
for (i = 0; i < size; i++) {
- DRM_DEBUG("%d r/g/b = 0x%04x / 0x%04x / 0x%04x\n", i,
- r[i], g[i], b[i]);
vmw_write(dev_priv, SVGA_PALETTE_BASE + i * 3 + 0, r[i] >> 8);
vmw_write(dev_priv, SVGA_PALETTE_BASE + i * 3 + 1, g[i] >> 8);
vmw_write(dev_priv, SVGA_PALETTE_BASE + i * 3 + 2, b[i] >> 8);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
index e4347faccee0..43e89c6755b2 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
@@ -474,6 +474,8 @@ static int vmw_ldu_init(struct vmw_private *dev_priv,
unsigned unit)
(>base,
dev_priv->implicit_placement_property,
1);
+ if (vmw_cmd_supported(dev_priv))
+ drm_plane_create_hotspot_properties(>base);
return 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index c89ad3a2d141..8d46b0cbe640 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -932,6 +932,7 @@ static int vmw_sou_init(struct vmw_private *dev_priv,
unsigned unit)
dev->mode_config.suggested_x_property, 0);
drm_object_attach_property(>base,
dev->mode_config.suggested_y_property, 0);
+ drm_plane_create_hotspot_properties(>base);
return 0;
err_free_unregister:
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index eb014b97d156..d940b9a525e7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1822,6 +1822,8 @@ static int vmw_stdu_init(struct vmw_private *dev_priv,
unsigned unit)
dev->mode_config.suggested_x_property, 0);
drm_object_attach_property(>base,
dev->mode_config.suggested_y_property, 0);
+ drm_plane_create_hotspot_properties(>base);
+
return 0;
err_free_unregister:
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
Re: [PATCH v2] drm/vmwgfx: Fix gem refcounting and memory evictions
From: Martin Krastev
From: Zack Rusin
Date: Wed, 20 Apr 2022 at 07:03
Subject: [PATCH v2] drm/vmwgfx: Fix gem refcounting and memory evictions
To:
Cc: ,
From: Zack Rusin
v2: Add the last part of the ref count fix which was spotted by
Philipp Sieweck where the ref count of cpu writers is off due to
ERESTARTSYS or EBUSY during bo waits.
The initial GEM port broke refcounting on shareable (prime) surfaces and
memory evictions. The prime surfaces broke because the parent surfaces
weren't increasing the ref count on GEM surfaces, which meant that
the memory backing textures could have been deleted while the texture
was still accessible. The evictions broke due to a typo, the code was
supposed to exit if the passed buffers were not vmw_buffer_object
not if they were. They're tied because the evictions depend on having
memory to actually evict.
This fixes crashes with XA state tracker which is used for xrender
acceleration on xf86-video-vmware, apps/tests which use a lot of
memory (a good test being the piglit's streaming-texture-leak) and
desktops.
Signed-off-by: Zack Rusin
Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM")
Reported-by: Philipp Sieweck
Cc: # v5.17+
Reviewed-by: Maaz Mombasawala
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 43 -
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 8 ++---
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 7 +++-
3 files changed, 28 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 31aecc46624b..04c8a378aeed 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -46,6 +46,21 @@ vmw_buffer_object(struct ttm_buffer_object *bo)
return container_of(bo, struct vmw_buffer_object, base);
}
+/**
+ * bo_is_vmw - check if the buffer object is a _buffer_object
+ * @bo: ttm buffer object to be checked
+ *
+ * Uses destroy function associated with the object to determine if this is
+ * a _buffer_object.
+ *
+ * Returns:
+ * true if the object is of _buffer_object type, false if not.
+ */
+static bool bo_is_vmw(struct ttm_buffer_object *bo)
+{
+ return bo->destroy == _bo_bo_free ||
+ bo->destroy == _gem_destroy;
+}
/**
* vmw_bo_pin_in_placement - Validate a buffer to placement.
@@ -615,8 +630,9 @@ int vmw_user_bo_synccpu_ioctl(struct drm_device
*dev, void *data,
ret = vmw_user_bo_synccpu_grab(vbo, arg->flags);
vmw_bo_unreference();
- if (unlikely(ret != 0 && ret != -ERESTARTSYS &&
-ret != -EBUSY)) {
+ if (unlikely(ret != 0)) {
+ if (ret == -ERESTARTSYS || ret == -EBUSY)
+ return -EBUSY;
DRM_ERROR("Failed synccpu grab on handle 0x%08x.\n",
(unsigned int) arg->handle);
return ret;
@@ -798,7 +814,7 @@ int vmw_dumb_create(struct drm_file *file_priv,
void vmw_bo_swap_notify(struct ttm_buffer_object *bo)
{
/* Is @bo embedded in a struct vmw_buffer_object? */
- if (vmw_bo_is_vmw_bo(bo))
+ if (!bo_is_vmw(bo))
return;
/* Kill any cached kernel maps before swapout */
@@ -822,7 +838,7 @@ void vmw_bo_move_notify(struct ttm_buffer_object *bo,
struct vmw_buffer_object *vbo;
/* Make sure @bo is embedded in a struct vmw_buffer_object? */
- if (vmw_bo_is_vmw_bo(bo))
+ if (!bo_is_vmw(bo))
return;
vbo = container_of(bo, struct vmw_buffer_object, base);
@@ -843,22 +859,3 @@ void vmw_bo_move_notify(struct ttm_buffer_object *bo,
if (mem->mem_type != VMW_PL_MOB && bo->resource->mem_type ==
VMW_PL_MOB)
vmw_resource_unbind_list(vbo);
}
-
-/**
- * vmw_bo_is_vmw_bo - check if the buffer object is a _buffer_object
- * @bo: buffer object to be checked
- *
- * Uses destroy function associated with the object to determine if this is
- * a _buffer_object.
- *
- * Returns:
- * true if the object is of _buffer_object type, false if not.
- */
-bool vmw_bo_is_vmw_bo(struct ttm_buffer_object *bo)
-{
- if (bo->destroy == _bo_bo_free ||
- bo->destroy == _gem_destroy)
- return true;
-
- return false;
-}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 72a17618ba0a..0c12faa4e533 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1018,13 +1018,10 @@ static int vmw_driver_load(struct vmw_private
*dev_priv, u32 pci_id)
goto out_no_fman;
}
- drm_vma_offset_manager_init(_priv->vma_manager,
- DRM_FILE_PAGE_OFFSET_START,
- DRM_FILE_PAGE_OFFSET_SIZE);
ret = ttm_device_init(_priv->bdev, _bo_driver,
dev_priv->drm.dev,
