Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/qxl: Pin buffer objects for internal mappings

Add qxl_bo_pin_and_vmap() that pins and vmaps a buffer object in one
step. Update callers of the regular qxl_bo_vmap(). Fixes a bug where
qxl accesses an unpinned buffer object while it is being moved; such
as with the monitor-description BO. An typical error is shown below.

[ 4.303586] [drm:drm_atomic_helper_commit_planes] *ERROR* head 1 wrong: 65376256x16777216+0+0
[ 4.586883] [drm:drm_atomic_helper_commit_planes] *ERROR* head 1 wrong: 65376256x16777216+0+0
[ 4.904036] [drm:drm_atomic_helper_commit_planes] *ERROR* head 1 wrong: 65335296x16777216+0+0
[ 5.374347] [drm:qxl_release_from_id_locked] *ERROR* failed to find id in release_idr

Commit b33651a5c98d ("drm/qxl: Do not pin buffer objects for vmap")
removed the implicit pin operation from qxl's vmap code. This is the
correct behavior for GEM and PRIME interfaces, but the pin is still
needed for qxl internal operation.

Also add a corresponding function qxl_bo_vunmap_and_unpin() and remove
the old qxl_bo_vmap() helpers.

Future directions: BOs should not be pinned or vmapped unnecessarily.
The pin-and-vmap operation should be removed from the driver and a
temporary mapping should be established with a vmap_local-like helper.
See the client helper drm_client_buffer_vmap_local() for semantics.

v2:
- unreserve BO on errors in qxl_bo_pin_and_vmap() (Dmitry)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: b33651a5c98d ("drm/qxl: Do not pin buffer objects for vmap")
Reported-by: David Kaplan <david.kaplan@amd.com>
Closes: https://lore.kernel.org/dri-devel/ab0fb17d-0f96-4ee6-8b21-65d02bb02655@suse.de/
Tested-by: David Kaplan <david.kaplan@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Zack Rusin <zack.rusin@broadcom.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux.dev
Cc: spice-devel@lists.freedesktop.org
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Zack Rusin <zack.rusin@broadcom.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240708142208.194361-1-tzimmermann@suse.de

+20 -11
+7 -7
drivers/gpu/drm/qxl/qxl_display.c
··· 584 584 if (ret) 585 585 goto err; 586 586 587 - ret = qxl_bo_vmap(cursor_bo, &cursor_map); 587 + ret = qxl_bo_pin_and_vmap(cursor_bo, &cursor_map); 588 588 if (ret) 589 589 goto err_unref; 590 590 591 - ret = qxl_bo_vmap(user_bo, &user_map); 591 + ret = qxl_bo_pin_and_vmap(user_bo, &user_map); 592 592 if (ret) 593 593 goto err_unmap; 594 594 ··· 614 614 user_map.vaddr, size); 615 615 } 616 616 617 - qxl_bo_vunmap(user_bo); 618 - qxl_bo_vunmap(cursor_bo); 617 + qxl_bo_vunmap_and_unpin(user_bo); 618 + qxl_bo_vunmap_and_unpin(cursor_bo); 619 619 return cursor_bo; 620 620 621 621 err_unmap: 622 - qxl_bo_vunmap(cursor_bo); 622 + qxl_bo_vunmap_and_unpin(cursor_bo); 623 623 err_unref: 624 624 qxl_bo_unpin(cursor_bo); 625 625 qxl_bo_unref(&cursor_bo); ··· 1205 1205 } 1206 1206 qdev->monitors_config_bo = gem_to_qxl_bo(gobj); 1207 1207 1208 - ret = qxl_bo_vmap(qdev->monitors_config_bo, &map); 1208 + ret = qxl_bo_pin_and_vmap(qdev->monitors_config_bo, &map); 1209 1209 if (ret) 1210 1210 return ret; 1211 1211 ··· 1236 1236 qdev->monitors_config = NULL; 1237 1237 qdev->ram_header->monitors_config = 0; 1238 1238 1239 - ret = qxl_bo_vunmap(qdev->monitors_config_bo); 1239 + ret = qxl_bo_vunmap_and_unpin(qdev->monitors_config_bo); 1240 1240 if (ret) 1241 1241 return ret; 1242 1242
+11 -2
drivers/gpu/drm/qxl/qxl_object.c
··· 182 182 return 0; 183 183 } 184 184 185 - int qxl_bo_vmap(struct qxl_bo *bo, struct iosys_map *map) 185 + int qxl_bo_pin_and_vmap(struct qxl_bo *bo, struct iosys_map *map) 186 186 { 187 187 int r; 188 188 ··· 190 190 if (r) 191 191 return r; 192 192 193 + r = qxl_bo_pin_locked(bo); 194 + if (r) { 195 + qxl_bo_unreserve(bo); 196 + return r; 197 + } 198 + 193 199 r = qxl_bo_vmap_locked(bo, map); 200 + if (r) 201 + qxl_bo_unpin_locked(bo); 194 202 qxl_bo_unreserve(bo); 195 203 return r; 196 204 } ··· 249 241 ttm_bo_vunmap(&bo->tbo, &bo->map); 250 242 } 251 243 252 - int qxl_bo_vunmap(struct qxl_bo *bo) 244 + int qxl_bo_vunmap_and_unpin(struct qxl_bo *bo) 253 245 { 254 246 int r; 255 247 ··· 258 250 return r; 259 251 260 252 qxl_bo_vunmap_locked(bo); 253 + qxl_bo_unpin_locked(bo); 261 254 qxl_bo_unreserve(bo); 262 255 return 0; 263 256 }
+2 -2
drivers/gpu/drm/qxl/qxl_object.h
··· 59 59 u32 priority, 60 60 struct qxl_surface *surf, 61 61 struct qxl_bo **bo_ptr); 62 - int qxl_bo_vmap(struct qxl_bo *bo, struct iosys_map *map); 62 + int qxl_bo_pin_and_vmap(struct qxl_bo *bo, struct iosys_map *map); 63 63 int qxl_bo_vmap_locked(struct qxl_bo *bo, struct iosys_map *map); 64 - int qxl_bo_vunmap(struct qxl_bo *bo); 64 + int qxl_bo_vunmap_and_unpin(struct qxl_bo *bo); 65 65 void qxl_bo_vunmap_locked(struct qxl_bo *bo); 66 66 void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, int page_offset); 67 67 void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, void *map);