Re: [PATCH v4] drm/vmwgfx: Treat zero SVGA_REG_CURSOR_MAX_DIMENSION as unset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch incorrectly groups the `mob_max_size` validation under the `cursor_max_dim > 0` condition in `vmw_cursor_mob_get()`.
--

commit 4e542ce85fcba71b4d6d0cd77e13a02898b305c1
Author: fausten <yunfeng.li.nb@gmail.com>
Date:   Sun Aug 30 15:16:19 2026 +0000

drm/vmwgfx: Treat zero SVGA_REG_CURSOR_MAX_DIMENSION as unset

This commit modifies the cursor plane validation to treat a reported
SVGA_REG_CURSOR_MAX_DIMENSION of 0 as "not implemented". This restores
the hardware cursor visibility on older SVGA version 2 guests.

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> @@ -237,8 +237,10 @@ static int vmw_cursor_mob_get(struct vmw_cursor_plane *vcp,
>  	mob_max_size = vmw_read(dev_priv, SVGA_REG_MOB_MAX_SIZE);
>  	cursor_max_dim = vmw_read(dev_priv, SVGA_REG_CURSOR_MAX_DIMENSION);
>  
> -	if (size > mob_max_size || vps->base.crtc_w > cursor_max_dim ||
> -	    vps->base.crtc_h > cursor_max_dim)
> +	/* Cursor MOBs, and thus this register, exist only on hw version 18+. */
> +	if (cursor_max_dim > 0 &&
> +	    (size > mob_max_size || vps->base.crtc_w > cursor_max_dim ||
> +	     vps->base.crtc_h > cursor_max_dim))

[Severity: Low]
Does this incorrectly bypass the mob_max_size check when cursor_max_dim
evaluates to 0?

The parentheses group size > mob_max_size with the cursor dimension
checks. If cursor_max_dim is 0, the logical AND fails early, which
completely skips checking if the size exceeds mob_max_size in
vmw_cursor_mob_get().

While this shouldn't cause an immediate out-of-bounds update since
vmw_cursor_plane_atomic_check() independently validates mob_max_size,
should the mob_max_size check remain separate from the cursor_max_dim
validation to ensure it is always evaluated here?

>  		return -EINVAL;
>  
>  	if (vps->cursor.mob) {

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830151619.2452307-1-yunfeng.li.nb@gmail.com?part=1