[PATCH 1/6] drm/vmwgfx: keep cursor mappings and pins with each plane state

[PATCH 1/6] drm/vmwgfx: keep cursor mappings and pins with each plane state

[Zack Rusin]

Cursor plane states duplicate the user object reference without acquiring
its mapping or pin. Preparing the duplicated state nevertheless unmaps
that object, and only pins the new buffer if it is not already pinned.
Cleanup of an older state can therefore remove a mapping or pin still
needed by another cursor commit. Repeated cursor updates from KMS clients
can then access an unmapped or movable buffer.

Give each prepared state its own mapping, buffer reference and pin, and
reset the mapping when duplicating a state. Release only the resources
owned by that state in cleanup_fb(). Map the required image extent for
every state so resizing a cursor on the same buffer cannot reuse a
smaller cached mapping. Unwind the pin and reservation if preparation
fails.

Fixes: 965544150d1c ("drm/vmwgfx: Refactor cursor handling")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 69 ++++++++++++--------
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h |  4 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c          |  1 +
 3 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index 5a69606a0593..942d07079ef7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -112,8 +112,7 @@ static void vmw_cursor_update_mob(struct vmw_private *vmw,
 {
 	SVGAGBCursorHeader *header;
 	SVGAGBAlphaCursorHeader *alpha_header;
-	struct vmw_bo *bo = vmw_user_object_buffer(&vps->uo);
-	u32 *image = vmw_bo_map_and_cache(bo);
+	u32 *image = vps->cursor.map.virtual;
 	const u32 image_size = vps->base.crtc_w * vps->base.crtc_h * sizeof(*image);
 
 	header = vmw_bo_map_and_cache(vps->cursor.mob);
@@ -132,7 +131,6 @@ static void vmw_cursor_update_mob(struct vmw_private *vmw,
 	memcpy(header + 1, image, image_size);
 	vmw_write(vmw, SVGA_REG_CURSOR_MOBID, vmw_bo_mobid(vps->cursor.mob));
 
-	vmw_bo_unmap(bo);
 	vmw_bo_unmap(vps->cursor.mob);
 }
 
@@ -490,9 +488,21 @@ vmw_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);
+	struct vmw_bo *bo;
+	int ret;
 
-	if (!vmw_user_object_is_null(&vps->uo))
-		vmw_user_object_unmap(&vps->uo);
+	if (vps->cursor.map.bo) {
+		bo = to_vmw_bo(&vps->cursor.map.bo->base);
+		ret = ttm_bo_reserve(&bo->tbo, false, false, NULL);
+		if (WARN_ON_ONCE(ret))
+			return;
+
+		ttm_bo_kunmap(&vps->cursor.map);
+		vps->cursor.map.bo = NULL;
+		ttm_bo_unpin(&bo->tbo);
+		ttm_bo_unreserve(&bo->tbo);
+		vmw_bo_unreference(&bo);
+	}
 
 	vmw_cursor_mob_unmap(vps);
 	vmw_cursor_mob_put(vcp, vps);
@@ -625,10 +635,12 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
 	struct vmw_surface *surface;
 	int ret = 0;
 
-	if (!vmw_user_object_is_null(&vps->uo)) {
-		vmw_user_object_unmap(&vps->uo);
+	/*
+	 * A duplicated state only owns the user object reference. Its mappings
+	 * and pins belong to the old state until cleanup_fb().
+	 */
+	if (!vmw_user_object_is_null(&vps->uo))
 		vmw_user_object_unref(&vps->uo);
-	}
 
 	if (fb) {
 		if (vmw_framebuffer_to_vfb(fb)->bo) {
@@ -652,31 +664,34 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
 		bo = vmw_user_object_buffer(&vps->uo);
 		if (bo) {
 			struct ttm_operation_ctx ctx = { false, false };
+			size_t size = (size_t)new_state->crtc_w *
+				      new_state->crtc_h * sizeof(u32);
 
 			ret = ttm_bo_reserve(&bo->tbo, true, false, NULL);
 			if (ret != 0)
-				return -ENOMEM;
+				return ret;
 
 			ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
 			if (ret != 0)
-				return -ENOMEM;
+				goto out_unreserve;
 
 			/*
-			 * vmw_bo_pin_reserved also validates, so to skip
-			 * the extra validation use ttm_bo_pin directly
+			 * vmw_bo_pin_reserved() also validates, so to skip
+			 * the extra validation use ttm_bo_pin directly.
+			 * Every state takes its own pin, the buffer can
+			 * still be in use by an older state whose commit
+			 * hasn't been cleaned up yet.
 			 */
-			if (!bo->tbo.pin_count)
-				ttm_bo_pin(&bo->tbo);
+			ttm_bo_pin(&bo->tbo);
 
-			if (vmw_framebuffer_to_vfb(fb)->bo) {
-				const u32 size = new_state->crtc_w *
-						 new_state->crtc_h *
-						 sizeof(u32);
-
-				(void)vmw_bo_map_and_cache_size(bo, size);
-			} else {
-				vmw_bo_map_and_cache(bo);
+			ret = ttm_bo_kmap(&bo->tbo, 0, PFN_UP(size),
+					  &vps->cursor.map);
+			if (ret) {
+				vps->cursor.map.bo = NULL;
+				ttm_bo_unpin(&bo->tbo);
+				goto out_unreserve;
 			}
+			vmw_bo_reference(bo);
 			ttm_bo_unreserve(&bo->tbo);
 		}
 		if (!vmw_user_object_is_null(&vps->uo)) {
@@ -703,6 +718,10 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
 	}
 
 	return 0;
+
+out_unreserve:
+	ttm_bo_unreserve(&bo->tbo);
+	return ret;
 }
 
 /**
@@ -796,7 +815,6 @@ void
 vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 			       struct drm_atomic_commit *state)
 {
-	struct vmw_bo *bo;
 	struct drm_plane_state *new_state =
 		drm_atomic_get_new_plane_state(state, plane);
 	struct drm_plane_state *old_state =
@@ -823,9 +841,8 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 		vmw_cursor_update_mob(dev_priv, vps);
 		break;
 	case VMW_CURSOR_UPDATE_GB_ONLY:
-		bo = vmw_user_object_buffer(&vps->uo);
-		if (bo)
-			vmw_send_define_cursor_cmd(dev_priv, bo->map.virtual,
+		if (vps->cursor.map.virtual)
+			vmw_send_define_cursor_cmd(dev_priv, vps->cursor.map.virtual,
 						   vps->base.crtc_w,
 						   vps->base.crtc_h,
 						   vps->base.hotspot_x,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h
index d32cf17d013b..9b50f80f9635 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h
@@ -14,6 +14,8 @@
 #include "drm/drm_fourcc.h"
 #include "drm/drm_plane.h"
 
+#include <drm/ttm/ttm_bo.h>
+
 #include <linux/types.h>
 
 struct SVGA3dCmdHeader;
@@ -41,6 +43,8 @@ struct vmw_cursor_plane_state {
 	enum vmw_cursor_update_type update_type;
 	bool changed;
 	bool surface_changed;
+	/* The mapped buffer is referenced and pinned by this state. */
+	struct ttm_bo_kmap_obj map;
 	struct vmw_bo *mob;
 	struct {
 		s32 hotspot_x;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 31d3ee0825ca..85228eb9c3e6 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -283,6 +283,7 @@ vmw_du_plane_duplicate_state(struct drm_plane *plane)
 	vps->cpp = 0;
 
 	vps->cursor.mob = NULL;
+	memset(&vps->cursor.map, 0, sizeof(vps->cursor.map));
 
 	/* Each ref counted resource needs to be acquired again */
 	vmw_user_object_ref(&vps->uo);
-- 
2.53.0

[PATCH 2/6] drm/vmwgfx: validate GB-only cursor command limits

[Zack Rusin]

Guest-backed devices without cursor MOB support define cursor images with
SVGA_CMD_DEFINE_ALPHA_CURSOR. SVGA_REG_CURSOR_MAX_DIMENSION belongs to the
cursor MOB register class and reads as zero on devices that do not expose
that class. Applying this register to GB-only cursors rejects every
nonempty image submitted by KMS clients, leaving Xorg without a hardware
cursor.

The submission transport also limits the command size. A 96x96 image fits
the 40 KiB FIFO image limit, but exceeds the 32 KiB command-buffer limit
and causes a device command error when submitted through that transport.

Validate GB-only images against SVGA_MAX_CURSOR_CMD_DIMENSION and
SVGA_MAX_CURSOR_CMD_BYTES. For command-buffer submission, also bound the
whole command by SVGA_CB_MAX_COMMAND_SIZE, including its header. Calculate
the image byte count in 64 bits and keep the register-based dimension and
MOB size checks for MOB cursors.

Fixes: d5ed8749168a ("drm/vmwgfx: enforce cursor size limits for MOB cursors")
Cc: stable@vger.kernel.org
Reported-by: Yunfeng Li <yunfeng.li.nb@gmail.com>
Closes: https://patch.msgid.link/20260830151619.2452307-1-yunfeng.li.nb@gmail.com
Assisted-by: LLM
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 23 ++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index 942d07079ef7..14490ead99cd 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -778,8 +778,27 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
 				 "surface not suitable for cursor\n");
 			return -EINVAL;
 		}
-	} else if (update_type == VMW_CURSOR_UPDATE_GB_ONLY ||
-		   update_type == VMW_CURSOR_UPDATE_MOB) {
+	} else if (update_type == VMW_CURSOR_UPDATE_GB_ONLY) {
+		u64 image_size = (u64)new_state->crtc_w *
+				 new_state->crtc_h * sizeof(u32);
+		u32 max_size = SVGA_MAX_CURSOR_CMD_BYTES;
+
+		/* The command-buffer limit includes the define-cursor header. */
+		if (vmw->cman)
+			max_size = min_t(u32, max_size,
+					 SVGA_CB_MAX_COMMAND_SIZE -
+					 sizeof(struct vmw_svga_fifo_cmd_define_cursor));
+
+		if (!new_state->crtc_w || !new_state->crtc_h ||
+		    new_state->crtc_w > SVGA_MAX_CURSOR_CMD_DIMENSION ||
+		    new_state->crtc_h > SVGA_MAX_CURSOR_CMD_DIMENSION ||
+		    image_size > max_size) {
+			drm_dbg_kms(&vmw->drm,
+				    "Cursor dimensions (%u, %u) violate command limits\n",
+				    new_state->crtc_w, new_state->crtc_h);
+			return -EINVAL;
+		}
+	} else if (update_type == VMW_CURSOR_UPDATE_MOB) {
 		u32 cursor_max_dim =
 			vmw_read(vmw, SVGA_REG_CURSOR_MAX_DIMENSION);
 
-- 
2.53.0

[PATCH 3/6] drm/vmwgfx: include legacy hotspots in GB-only cursor images

[Zack Rusin]

From: Tomas M <tomas@slax.org>

The guest-backed cursor path without cursor MOB support defines the image
using only the hotspot from the plane state. Its position calculation
also includes the legacy hotspot supplied through DRM_VMW_CURSOR_BYPASS.

Xorg's xf86-video-vmware driver supplies its hotspot through that ioctl
and loads the image with drmModeSetCursor(), which does not set a plane
hotspot. The mismatched image and position hotspots displace the visible
cursor by the legacy offset.

Use the combined hotspot for both the GB-only image command and cursor
position, matching the legacy and MOB image paths.

Fixes: eef295a85082 ("drm/vmwgfx: Restore Guest-Backed only cursor plane support")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Tomas M <tomas@slax.org>
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index 14490ead99cd..f0661e5effeb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -852,6 +852,10 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 		return;
 	}
 
+	/* Include legacy Xorg cursor-bypass offsets in the image and position. */
+	hotspot_x = vps->cursor.legacy.hotspot_x + new_state->hotspot_x;
+	hotspot_y = vps->cursor.legacy.hotspot_y + new_state->hotspot_y;
+
 	switch (vps->cursor.update_type) {
 	case VMW_CURSOR_UPDATE_LEGACY:
 		vmw_cursor_plane_update_legacy(dev_priv, vps);
@@ -864,8 +868,7 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 			vmw_send_define_cursor_cmd(dev_priv, vps->cursor.map.virtual,
 						   vps->base.crtc_w,
 						   vps->base.crtc_h,
-						   vps->base.hotspot_x,
-						   vps->base.hotspot_y);
+						   hotspot_x, hotspot_y);
 		break;
 	case VMW_CURSOR_UPDATE_NONE:
 		/* do nothing */
@@ -878,9 +881,6 @@ vmw_cursor_plane_atomic_update(struct drm_plane *plane,
 	cursor_x = new_state->crtc_x + du->set_gui_x;
 	cursor_y = new_state->crtc_y + du->set_gui_y;
 
-	hotspot_x = vps->cursor.legacy.hotspot_x + new_state->hotspot_x;
-	hotspot_y = vps->cursor.legacy.hotspot_y + new_state->hotspot_y;
-
 	vmw_cursor_update_position(dev_priv, true, cursor_x + hotspot_x,
 				   cursor_y + hotspot_y);
 }
-- 
2.53.0

[PATCH 4/6] drm/vmwgfx: reserve dumb buffers while detaching their surfaces

[Zack Rusin]

vmw_bo_free() detaches the surface associated with a dumb buffer and
clears its guest memory binding. It reserves the resource, but does not
hold the buffer reservation required by vmw_resource_mob_detach(). TTM
releases that reservation before invoking the buffer destructor.

KMS clients destroying dumb buffers can therefore trigger the reservation
assertion from vmw_resource_mob_detach(). GNOME reaches this path through
DRM_IOCTL_MODE_DESTROY_DUMB during cursor and display updates.

Reacquire the buffer reservation while detaching the surface and clearing
its dirty tracking and guest memory binding. Acquire it after
cmdbuf_mutex, following the command validation lock order, and release it
before releasing the resource and GEM object.

Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 9c7a73c0b0dc..a6d7156e4281 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -55,6 +55,8 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 			/* Reserve and switch the backing mob. */
 			mutex_lock(&res->dev_priv->cmdbuf_mutex);
 			(void)vmw_resource_reserve(res, false, true);
+			/* TTM drops the reservation before calling the destructor. */
+			dma_resv_lock(bo->base.resv, NULL);
 			vmw_resource_mob_detach(res);
 			if (res->dirty)
 				res->func->dirty_free(res);
@@ -62,6 +64,7 @@ static void vmw_bo_free(struct ttm_buffer_object *bo)
 				vmw_bo_dirty_release(res->guest_memory_bo);
 			res->guest_memory_bo = NULL;
 			res->guest_memory_offset = 0;
+			dma_resv_unlock(bo->base.resv);
 			vmw_resource_unreserve(res, true, false, false, NULL,
 					       0);
 			mutex_unlock(&res->dev_priv->cmdbuf_mutex);
-- 
2.53.0

[PATCH 5/6] drm/vmwgfx: validate the framebuffer selected for cursor updates

[Zack Rusin]

Cursor atomic checks select an update path and look up surfaces through
vps->uo. That object is copied from the previous plane state and is only
replaced with the new framebuffer's object in prepare_fb(). Switching
between snooped surfaces and buffer objects therefore applies the old
framebuffer's cursor limits to the new image.

KMS clients can also submit a guest-backed surface without a backing BO.
The check accepts it, but the MOB update later copies from a NULL source.
Zero-sized MOB images pass the check as well, then fail preparation when
mapping zero pages, so TEST_ONLY accepts a commit that cannot succeed.

Resolve the user object directly from the selected framebuffer in both
validation and preparation. Apply the matching cursor limits, reject
zero dimensions and require a backing BO for GB-only and MOB images.
Recheck the backing BO in preparation before mapping it, while keeping
snooped surfaces independent of a BO mapping.

Fixes: 965544150d1c ("drm/vmwgfx: Refactor cursor handling")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 61 ++++++++++++++------
 1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index f0661e5effeb..9e8ea9e07f40 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -89,10 +89,26 @@ vmw_cursor_plane_update_legacy(struct vmw_private *vmw,
 	}
 }
 
-static enum vmw_cursor_update_type
-vmw_cursor_update_type(struct vmw_private *vmw, struct vmw_plane_state *vps)
+static struct vmw_user_object
+vmw_cursor_plane_user_object(struct drm_framebuffer *fb)
 {
-	struct vmw_surface *surface = vmw_user_object_surface(&vps->uo);
+	struct vmw_user_object uo = { 0 };
+
+	if (!fb)
+		return uo;
+
+	if (vmw_framebuffer_to_vfb(fb)->bo)
+		uo.buffer = vmw_framebuffer_to_vfbd(fb)->buffer;
+	else
+		uo = vmw_framebuffer_to_vfbs(fb)->uo;
+
+	return uo;
+}
+
+static enum vmw_cursor_update_type
+vmw_cursor_update_type(struct vmw_private *vmw, struct vmw_user_object *uo)
+{
+	struct vmw_surface *surface = vmw_user_object_surface(uo);
 
 	if (surface && surface->snooper.image)
 		return VMW_CURSOR_UPDATE_LEGACY;
@@ -642,17 +658,10 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
 	if (!vmw_user_object_is_null(&vps->uo))
 		vmw_user_object_unref(&vps->uo);
 
-	if (fb) {
-		if (vmw_framebuffer_to_vfb(fb)->bo) {
-			vps->uo.buffer = vmw_framebuffer_to_vfbd(fb)->buffer;
-			vps->uo.surface = NULL;
-		} else {
-			memcpy(&vps->uo, &vmw_framebuffer_to_vfbs(fb)->uo, sizeof(vps->uo));
-		}
-		vmw_user_object_ref(&vps->uo);
-	}
+	vps->uo = vmw_cursor_plane_user_object(fb);
+	vmw_user_object_ref(&vps->uo);
 
-	vps->cursor.update_type = vmw_cursor_update_type(vmw, vps);
+	vps->cursor.update_type = vmw_cursor_update_type(vmw, &vps->uo);
 	switch (vps->cursor.update_type) {
 	case VMW_CURSOR_UPDATE_LEGACY:
 		surface = vmw_user_object_surface(&vps->uo);
@@ -662,6 +671,9 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
 	case VMW_CURSOR_UPDATE_GB_ONLY:
 	case VMW_CURSOR_UPDATE_MOB: {
 		bo = vmw_user_object_buffer(&vps->uo);
+		if (fb && !bo)
+			return -EINVAL;
+
 		if (bo) {
 			struct ttm_operation_ctx ctx = { false, false };
 			size_t size = (size_t)new_state->crtc_w *
@@ -744,7 +756,7 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
 	int ret = 0;
 	struct drm_crtc_state *crtc_state = NULL;
 	struct vmw_surface *surface = NULL;
-	struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
+	struct vmw_user_object uo;
 	enum vmw_cursor_update_type update_type;
 	struct drm_framebuffer *fb = new_state->fb;
 
@@ -763,7 +775,21 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
 	if (!fb)
 		return 0;
 
-	update_type = vmw_cursor_update_type(vmw, vps);
+	if (!new_state->crtc_w || !new_state->crtc_h) {
+		drm_dbg_kms(&vmw->drm,
+			    "Cursor dimensions (%u, %u) must be nonzero\n",
+			    new_state->crtc_w, new_state->crtc_h);
+		return -EINVAL;
+	}
+
+	/* The duplicated plane state's user object still belongs to the old FB. */
+	uo = vmw_cursor_plane_user_object(fb);
+	update_type = vmw_cursor_update_type(vmw, &uo);
+	if ((update_type == VMW_CURSOR_UPDATE_GB_ONLY ||
+	     update_type == VMW_CURSOR_UPDATE_MOB) &&
+	    !vmw_user_object_buffer(&uo))
+		return -EINVAL;
+
 	if (update_type == VMW_CURSOR_UPDATE_LEGACY) {
 		if (new_state->crtc_w != VMW_CURSOR_SNOOP_WIDTH ||
 		    new_state->crtc_h != VMW_CURSOR_SNOOP_HEIGHT) {
@@ -772,7 +798,7 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
 				 new_state->crtc_w, new_state->crtc_h);
 			return -EINVAL;
 		}
-		surface = vmw_user_object_surface(&vps->uo);
+		surface = vmw_user_object_surface(&uo);
 		if (!surface || !surface->snooper.image) {
 			drm_warn(&vmw->drm,
 				 "surface not suitable for cursor\n");
@@ -789,8 +815,7 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
 					 SVGA_CB_MAX_COMMAND_SIZE -
 					 sizeof(struct vmw_svga_fifo_cmd_define_cursor));
 
-		if (!new_state->crtc_w || !new_state->crtc_h ||
-		    new_state->crtc_w > SVGA_MAX_CURSOR_CMD_DIMENSION ||
+		if (new_state->crtc_w > SVGA_MAX_CURSOR_CMD_DIMENSION ||
 		    new_state->crtc_h > SVGA_MAX_CURSOR_CMD_DIMENSION ||
 		    image_size > max_size) {
 			drm_dbg_kms(&vmw->drm,
-- 
2.53.0

[PATCH 6/6] drm/vmwgfx: reserve cursor MOBs before unpinning them

[Zack Rusin]

vmw_cursor_mob_destroy() unpins cursor MOBs when discarding an image from
the plane's cache or destroying the cursor plane. It calls ttm_bo_unpin()
without reserving the BO, although TTM requires its reservation to be held
when changing pin counts and LRU membership.

KMS clients issuing repeated nonblocking cursor updates can exhaust the
cache and reach this path. On a lockdep-enabled kernel this triggers
reservation assertions in ttm_bo_unpin() and ttm_resource_unevictable().

Reserve the MOB around ttm_bo_unpin(), then release the reservation before
dropping the BO reference.

Fixes: 485d98d472d5 ("drm/vmwgfx: Add support for CursorMob and CursorBypass 4")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
index 9e8ea9e07f40..b678e5b68db7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
@@ -166,10 +166,17 @@ static u32 vmw_cursor_mob_size(enum vmw_cursor_update_type update_type,
 
 static void vmw_cursor_mob_destroy(struct vmw_bo **vbo)
 {
+	int ret;
+
 	if (!(*vbo))
 		return;
 
+	ret = ttm_bo_reserve(&(*vbo)->tbo, false, false, NULL);
+	if (WARN_ON_ONCE(ret))
+		return;
+
 	ttm_bo_unpin(&(*vbo)->tbo);
+	ttm_bo_unreserve(&(*vbo)->tbo);
 	vmw_bo_unreference(vbo);
 }
 
-- 
2.53.0

Re: [PATCH 5/6] drm/vmwgfx: validate the framebuffer selected for cursor updates

[Maaz Mombasawala <maaz.mombasawala@broadcom.com>]

On Tue Oct 27, 2026 at 2:17 PM PDT, Zack Rusin wrote:
> Cursor atomic checks select an update path and look up surfaces through
> vps->uo. That object is copied from the previous plane state and is only
> replaced with the new framebuffer's object in prepare_fb(). Switching
> between snooped surfaces and buffer objects therefore applies the old
> framebuffer's cursor limits to the new image.
>
> KMS clients can also submit a guest-backed surface without a backing BO.
> The check accepts it, but the MOB update later copies from a NULL source.
> Zero-sized MOB images pass the check as well, then fail preparation when
> mapping zero pages, so TEST_ONLY accepts a commit that cannot succeed.
>
> Resolve the user object directly from the selected framebuffer in both
> validation and preparation. Apply the matching cursor limits, reject
> zero dimensions and require a backing BO for GB-only and MOB images.
> Recheck the backing BO in preparation before mapping it, while keeping
> snooped surfaces independent of a BO mapping.
>
> Fixes: 965544150d1c ("drm/vmwgfx: Refactor cursor handling")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 61 ++++++++++++++------
>  1 file changed, 43 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> index f0661e5effeb..9e8ea9e07f40 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> @@ -89,10 +89,26 @@ vmw_cursor_plane_update_legacy(struct vmw_private *vmw,
>  	}
>  }
>  
> -static enum vmw_cursor_update_type
> -vmw_cursor_update_type(struct vmw_private *vmw, struct vmw_plane_state *vps)
> +static struct vmw_user_object
> +vmw_cursor_plane_user_object(struct drm_framebuffer *fb)
>  {
> -	struct vmw_surface *surface = vmw_user_object_surface(&vps->uo);
> +	struct vmw_user_object uo = { 0 };
> +
> +	if (!fb)
> +		return uo;
> +
> +	if (vmw_framebuffer_to_vfb(fb)->bo)
> +		uo.buffer = vmw_framebuffer_to_vfbd(fb)->buffer;
> +	else
> +		uo = vmw_framebuffer_to_vfbs(fb)->uo;
> +
> +	return uo;
> +}
> +
> +static enum vmw_cursor_update_type
> +vmw_cursor_update_type(struct vmw_private *vmw, struct vmw_user_object *uo)
> +{
> +	struct vmw_surface *surface = vmw_user_object_surface(uo);
>  
>  	if (surface && surface->snooper.image)
>  		return VMW_CURSOR_UPDATE_LEGACY;
> @@ -642,17 +658,10 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
>  	if (!vmw_user_object_is_null(&vps->uo))
>  		vmw_user_object_unref(&vps->uo);
>  
> -	if (fb) {
> -		if (vmw_framebuffer_to_vfb(fb)->bo) {
> -			vps->uo.buffer = vmw_framebuffer_to_vfbd(fb)->buffer;
> -			vps->uo.surface = NULL;
> -		} else {
> -			memcpy(&vps->uo, &vmw_framebuffer_to_vfbs(fb)->uo, sizeof(vps->uo));
> -		}
> -		vmw_user_object_ref(&vps->uo);
> -	}
> +	vps->uo = vmw_cursor_plane_user_object(fb);
> +	vmw_user_object_ref(&vps->uo);

If fb == NULL then uo = 0 and that would cause a null pointer dereference in vmw_user_object_ref
since it's not doing a null check over there.

>  
> -	vps->cursor.update_type = vmw_cursor_update_type(vmw, vps);
> +	vps->cursor.update_type = vmw_cursor_update_type(vmw, &vps->uo);
>  	switch (vps->cursor.update_type) {
>  	case VMW_CURSOR_UPDATE_LEGACY:
>  		surface = vmw_user_object_surface(&vps->uo);
> @@ -662,6 +671,9 @@ int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
>  	case VMW_CURSOR_UPDATE_GB_ONLY:
>  	case VMW_CURSOR_UPDATE_MOB: {
>  		bo = vmw_user_object_buffer(&vps->uo);
> +		if (fb && !bo)
> +			return -EINVAL;
> +
>  		if (bo) {
>  			struct ttm_operation_ctx ctx = { false, false };
>  			size_t size = (size_t)new_state->crtc_w *
> @@ -744,7 +756,7 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
>  	int ret = 0;
>  	struct drm_crtc_state *crtc_state = NULL;
>  	struct vmw_surface *surface = NULL;
> -	struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
> +	struct vmw_user_object uo;
>  	enum vmw_cursor_update_type update_type;
>  	struct drm_framebuffer *fb = new_state->fb;
>  
> @@ -763,7 +775,21 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
>  	if (!fb)
>  		return 0;
>  
> -	update_type = vmw_cursor_update_type(vmw, vps);
> +	if (!new_state->crtc_w || !new_state->crtc_h) {
> +		drm_dbg_kms(&vmw->drm,
> +			    "Cursor dimensions (%u, %u) must be nonzero\n",
> +			    new_state->crtc_w, new_state->crtc_h);
> +		return -EINVAL;
> +	}
> +
> +	/* The duplicated plane state's user object still belongs to the old FB. */
> +	uo = vmw_cursor_plane_user_object(fb);
> +	update_type = vmw_cursor_update_type(vmw, &uo);
> +	if ((update_type == VMW_CURSOR_UPDATE_GB_ONLY ||
> +	     update_type == VMW_CURSOR_UPDATE_MOB) &&
> +	    !vmw_user_object_buffer(&uo))
> +		return -EINVAL;
> +
>  	if (update_type == VMW_CURSOR_UPDATE_LEGACY) {
>  		if (new_state->crtc_w != VMW_CURSOR_SNOOP_WIDTH ||
>  		    new_state->crtc_h != VMW_CURSOR_SNOOP_HEIGHT) {
> @@ -772,7 +798,7 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
>  				 new_state->crtc_w, new_state->crtc_h);
>  			return -EINVAL;
>  		}
> -		surface = vmw_user_object_surface(&vps->uo);
> +		surface = vmw_user_object_surface(&uo);
>  		if (!surface || !surface->snooper.image) {
>  			drm_warn(&vmw->drm,
>  				 "surface not suitable for cursor\n");
> @@ -789,8 +815,7 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
>  					 SVGA_CB_MAX_COMMAND_SIZE -
>  					 sizeof(struct vmw_svga_fifo_cmd_define_cursor));
>  
> -		if (!new_state->crtc_w || !new_state->crtc_h ||
> -		    new_state->crtc_w > SVGA_MAX_CURSOR_CMD_DIMENSION ||

Why remove these checks?

> +		if (new_state->crtc_w > SVGA_MAX_CURSOR_CMD_DIMENSION ||
>  		    new_state->crtc_h > SVGA_MAX_CURSOR_CMD_DIMENSION ||
>  		    image_size > max_size) {
>  			drm_dbg_kms(&vmw->drm,


-- 
Maaz Mombasawala <maaz.mombasawala@broadcom.com> 

Re: [PATCH 2/6] drm/vmwgfx: validate GB-only cursor command limits

[Maaz Mombasawala <maaz.mombasawala@broadcom.com>]

On Tue Oct 27, 2026 at 2:17 PM PDT, Zack Rusin wrote:
> Guest-backed devices without cursor MOB support define cursor images with
> SVGA_CMD_DEFINE_ALPHA_CURSOR. SVGA_REG_CURSOR_MAX_DIMENSION belongs to the
> cursor MOB register class and reads as zero on devices that do not expose
> that class. Applying this register to GB-only cursors rejects every
> nonempty image submitted by KMS clients, leaving Xorg without a hardware
> cursor.
>
> The submission transport also limits the command size. A 96x96 image fits
> the 40 KiB FIFO image limit, but exceeds the 32 KiB command-buffer limit
> and causes a device command error when submitted through that transport.
>
> Validate GB-only images against SVGA_MAX_CURSOR_CMD_DIMENSION and
> SVGA_MAX_CURSOR_CMD_BYTES. For command-buffer submission, also bound the
> whole command by SVGA_CB_MAX_COMMAND_SIZE, including its header. Calculate
> the image byte count in 64 bits and keep the register-based dimension and
> MOB size checks for MOB cursors.
>
> Fixes: d5ed8749168a ("drm/vmwgfx: enforce cursor size limits for MOB cursors")
> Cc: stable@vger.kernel.org
> Reported-by: Yunfeng Li <yunfeng.li.nb@gmail.com>
> Closes: https://patch.msgid.link/20260830151619.2452307-1-yunfeng.li.nb@gmail.com
> Assisted-by: LLM
> Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 23 ++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> index 942d07079ef7..14490ead99cd 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> @@ -778,8 +778,27 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
>  				 "surface not suitable for cursor\n");
>  			return -EINVAL;
>  		}
> -	} else if (update_type == VMW_CURSOR_UPDATE_GB_ONLY ||
> -		   update_type == VMW_CURSOR_UPDATE_MOB) {
> +	} else if (update_type == VMW_CURSOR_UPDATE_GB_ONLY) {
> +		u64 image_size = (u64)new_state->crtc_w *
> +				 new_state->crtc_h * sizeof(u32);
> +		u32 max_size = SVGA_MAX_CURSOR_CMD_BYTES;
> +
> +		/* The command-buffer limit includes the define-cursor header. */
> +		if (vmw->cman)
> +			max_size = min_t(u32, max_size,
> +					 SVGA_CB_MAX_COMMAND_SIZE -
> +					 sizeof(struct vmw_svga_fifo_cmd_define_cursor));
> +
> +		if (!new_state->crtc_w || !new_state->crtc_h ||
> +		    new_state->crtc_w > SVGA_MAX_CURSOR_CMD_DIMENSION ||
> +		    new_state->crtc_h > SVGA_MAX_CURSOR_CMD_DIMENSION ||
> +		    image_size > max_size) {

We should have a separate message for image_size > max_size.

> +			drm_dbg_kms(&vmw->drm,
> +				    "Cursor dimensions (%u, %u) violate command limits\n",
> +				    new_state->crtc_w, new_state->crtc_h);
> +			return -EINVAL;
> +		}
> +	} else if (update_type == VMW_CURSOR_UPDATE_MOB) {
>  		u32 cursor_max_dim =
>  			vmw_read(vmw, SVGA_REG_CURSOR_MAX_DIMENSION);
>  


-- 
Maaz Mombasawala <maaz.mombasawala@broadcom.com> 

Re: [PATCH 6/6] drm/vmwgfx: reserve cursor MOBs before unpinning them

[Maaz Mombasawala <maaz.mombasawala@broadcom.com>]

LGTM!

Reviewed-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
-- 
Maaz Mombasawala <maaz.mombasawala@broadcom.com> 

Re: [PATCH 1/6] drm/vmwgfx: keep cursor mappings and pins with each plane state

[Maaz Mombasawala <maaz.mombasawala@broadcom.com>]

LGTM!

Reviewed-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>

-- 
Maaz Mombasawala <maaz.mombasawala@broadcom.com> 

Re: [PATCH 3/6] drm/vmwgfx: include legacy hotspots in GB-only cursor images

[Maaz Mombasawala <maaz.mombasawala@broadcom.com>]

LGTM!

Reviewed-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>

-- 
Maaz Mombasawala <maaz.mombasawala@broadcom.com> 

Re: [PATCH 4/6] drm/vmwgfx: reserve dumb buffers while detaching their surfaces

[Maaz Mombasawala <maaz.mombasawala@broadcom.com>]

LGTM!

Reviewed-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>

-- 
Maaz Mombasawala <maaz.mombasawala@broadcom.com> 

Re: [PATCH 1/6] drm/vmwgfx: keep cursor mappings and pins with each plane state

[Ian Forbes]

[-- Attachment #1: Type: text/plain, Size: 480 bytes --]

This is pulling in peices of
https://patchwork.kernel.org/project/dri-devel/patch/20260414200819.88506-1-ian.forbes@broadcom.com/
I'd prefer dropping this patch from this series and completing that series.


On Wed, Nov 4, 2026 at 8:39 PM Maaz Mombasawala
<maaz.mombasawala@broadcom.com> <maaz.mombasawala@broadcom.com> wrote:
>
> LGTM!
>
> Reviewed-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
>
> --
> Maaz Mombasawala <maaz.mombasawala@broadcom.com>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]

Re: [PATCH 1/6] drm/vmwgfx: keep cursor mappings and pins with each plane state

[Zack Rusin]

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]

On Thu, Nov 5, 2026 at 2:05 PM Ian Forbes <ian.forbes@broadcom.com> wrote:
>
> This is pulling in peices of
> https://patchwork.kernel.org/project/dri-devel/patch/20260414200819.88506-1-ian.forbes@broadcom.com/
> I'd prefer dropping this patch from this series and completing that series.

Looking at the changes I think we probably want to do the opposite
because this one also handles bo's lifetime and preparation failures.
Wouldn't it make more sense to rebase your series on top of this?

z

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]

Re: [PATCH 2/6] drm/vmwgfx: validate GB-only cursor command limits

[Zack Rusin]

[-- Attachment #1: Type: text/plain, Size: 3684 bytes --]

On Wed, Nov 4, 2026 at 9:36 PM Maaz Mombasawala
<maaz.mombasawala@broadcom.com> <maaz.mombasawala@broadcom.com> wrote:
>
> On Tue Oct 27, 2026 at 2:17 PM PDT, Zack Rusin wrote:
> > Guest-backed devices without cursor MOB support define cursor images with
> > SVGA_CMD_DEFINE_ALPHA_CURSOR. SVGA_REG_CURSOR_MAX_DIMENSION belongs to the
> > cursor MOB register class and reads as zero on devices that do not expose
> > that class. Applying this register to GB-only cursors rejects every
> > nonempty image submitted by KMS clients, leaving Xorg without a hardware
> > cursor.
> >
> > The submission transport also limits the command size. A 96x96 image fits
> > the 40 KiB FIFO image limit, but exceeds the 32 KiB command-buffer limit
> > and causes a device command error when submitted through that transport.
> >
> > Validate GB-only images against SVGA_MAX_CURSOR_CMD_DIMENSION and
> > SVGA_MAX_CURSOR_CMD_BYTES. For command-buffer submission, also bound the
> > whole command by SVGA_CB_MAX_COMMAND_SIZE, including its header. Calculate
> > the image byte count in 64 bits and keep the register-based dimension and
> > MOB size checks for MOB cursors.
> >
> > Fixes: d5ed8749168a ("drm/vmwgfx: enforce cursor size limits for MOB cursors")
> > Cc: stable@vger.kernel.org
> > Reported-by: Yunfeng Li <yunfeng.li.nb@gmail.com>
> > Closes: https://patch.msgid.link/20260830151619.2452307-1-yunfeng.li.nb@gmail.com
> > Assisted-by: LLM
> > Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c | 23 ++++++++++++++++++--
> >  1 file changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> > index 942d07079ef7..14490ead99cd 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> > @@ -778,8 +778,27 @@ int vmw_cursor_plane_atomic_check(struct drm_plane *plane,
> >                                "surface not suitable for cursor\n");
> >                       return -EINVAL;
> >               }
> > -     } else if (update_type == VMW_CURSOR_UPDATE_GB_ONLY ||
> > -                update_type == VMW_CURSOR_UPDATE_MOB) {
> > +     } else if (update_type == VMW_CURSOR_UPDATE_GB_ONLY) {
> > +             u64 image_size = (u64)new_state->crtc_w *
> > +                              new_state->crtc_h * sizeof(u32);
> > +             u32 max_size = SVGA_MAX_CURSOR_CMD_BYTES;
> > +
> > +             /* The command-buffer limit includes the define-cursor header. */
> > +             if (vmw->cman)
> > +                     max_size = min_t(u32, max_size,
> > +                                      SVGA_CB_MAX_COMMAND_SIZE -
> > +                                      sizeof(struct vmw_svga_fifo_cmd_define_cursor));
> > +
> > +             if (!new_state->crtc_w || !new_state->crtc_h ||
> > +                 new_state->crtc_w > SVGA_MAX_CURSOR_CMD_DIMENSION ||
> > +                 new_state->crtc_h > SVGA_MAX_CURSOR_CMD_DIMENSION ||
> > +                 image_size > max_size) {
>
> We should have a separate message for image_size > max_size.

How come? And what kind of a message? I'm not sure if we want to be
printing all the limits here because it's not like those are forwarded
to the users apps, or that the userspace would adjust its code based
on kernel warnings and for the gui apps that respect the drm api
cursor already has the max size set. Therefore, I'm unsure if printing
the exact case (width, height, total...) that caused the request to
error out would help anyone.

z

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]