Skip to content

Commit b134f7a

Browse files
committed
drm/vmwgfx: Abstract placement selection
jira VULN-8161 cve CVE-2023-5633 commit-author Zack Rusin <zackr@vmware.com> commit 39985ee upstream-diff Complications from not having the upstream patchset containing f87c1f0 ("drm/ttm: prevent moving of pinned BOs"), which we cannot backport without breaking kABI. 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 <zackr@vmware.com> Reviewed-by: Martin Krastev <krastevm@vmware.com> Reviewed-by: Maaz Mombasawala <mombasawalam@vmware.com> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Link: https://patchwork.freedesktop.org/patch/msgid/20230131033542.953249-8-zack@kde.org (cherry picked from commit 39985ee) Signed-off-by: Sultan Alsawaf <sultan@ciq.com>
1 parent 89edd70 commit b134f7a

22 files changed

+314
-460
lines changed

drivers/gpu/drm/vmwgfx/vmwgfx_bo.c

Lines changed: 128 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -129,17 +129,23 @@ int vmw_bo_pin_in_vram_or_gmr(struct vmw_private *dev_priv,
129129
if (unlikely(ret != 0))
130130
goto err;
131131

132+
vmw_bo_placement_set(buf,
133+
VMW_BO_DOMAIN_GMR | VMW_BO_DOMAIN_VRAM,
134+
VMW_BO_DOMAIN_GMR);
132135
if (buf->base.pin_count > 0) {
133-
ret = ttm_resource_compat(bo->resource, &vmw_vram_gmr_placement)
136+
ret = ttm_resource_compat(bo->resource, &buf->placement)
134137
? 0 : -EINVAL;
135138
goto out_unreserve;
136139
}
137140

138-
ret = ttm_bo_validate(bo, &vmw_vram_gmr_placement, &ctx);
141+
ret = ttm_bo_validate(bo, &buf->placement, &ctx);
139142
if (likely(ret == 0) || ret == -ERESTARTSYS)
140143
goto out_unreserve;
141144

142-
ret = ttm_bo_validate(bo, &vmw_vram_placement, &ctx);
145+
vmw_bo_placement_set(buf,
146+
VMW_BO_DOMAIN_VRAM,
147+
VMW_BO_DOMAIN_VRAM);
148+
ret = ttm_bo_validate(bo, &buf->placement, &ctx);
143149

144150
out_unreserve:
145151
if (!ret)
@@ -190,17 +196,8 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private *dev_priv,
190196
{
191197
struct ttm_operation_ctx ctx = {interruptible, false };
192198
struct ttm_buffer_object *bo = &buf->base;
193-
struct ttm_placement placement;
194-
struct ttm_place place;
195199
int ret = 0;
196200

197-
place = vmw_vram_placement.placement[0];
198-
place.lpfn = bo->resource->num_pages;
199-
placement.num_placement = 1;
200-
placement.placement = &place;
201-
placement.num_busy_placement = 1;
202-
placement.busy_placement = &place;
203-
204201
vmw_execbuf_release_pinned_bo(dev_priv);
205202
ret = ttm_bo_reserve(bo, interruptible, false, NULL);
206203
if (unlikely(ret != 0))
@@ -216,14 +213,21 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private *dev_priv,
216213
bo->resource->start > 0 &&
217214
buf->base.pin_count == 0) {
218215
ctx.interruptible = false;
219-
(void) ttm_bo_validate(bo, &vmw_sys_placement, &ctx);
216+
vmw_bo_placement_set(buf,
217+
VMW_BO_DOMAIN_SYS,
218+
VMW_BO_DOMAIN_SYS);
219+
(void)ttm_bo_validate(bo, &buf->placement, &ctx);
220220
}
221221

222+
vmw_bo_placement_set(buf,
223+
VMW_BO_DOMAIN_VRAM,
224+
VMW_BO_DOMAIN_VRAM);
225+
buf->places[0].lpfn = bo->resource->num_pages;
222226
if (buf->base.pin_count > 0)
223-
ret = ttm_resource_compat(bo->resource, &placement)
227+
ret = ttm_resource_compat(bo->resource, &buf->placement)
224228
? 0 : -EINVAL;
225229
else
226-
ret = ttm_bo_validate(bo, &placement, &ctx);
230+
ret = ttm_bo_validate(bo, &buf->placement, &ctx);
227231

228232
/* For some reason we didn't end up at the start of vram */
229233
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,
431435
}
432436

433437
int vmw_bo_create(struct vmw_private *vmw,
434-
size_t size, struct ttm_placement *placement,
438+
size_t size, u32 domain, u32 busy_domain,
435439
bool interruptible, bool pin,
436440
struct vmw_bo **p_bo)
437441
{
@@ -444,7 +448,8 @@ int vmw_bo_create(struct vmw_private *vmw,
444448
}
445449

446450
ret = vmw_bo_init(vmw, *p_bo, size,
447-
placement, interruptible, pin);
451+
domain, busy_domain,
452+
interruptible, pin);
448453
if (unlikely(ret != 0))
449454
goto out_error;
450455

@@ -461,7 +466,8 @@ int vmw_bo_create(struct vmw_private *vmw,
461466
* @dev_priv: Pointer to the device private struct
462467
* @vmw_bo: Pointer to the struct vmw_bo to initialize.
463468
* @size: Buffer object size in bytes.
464-
* @placement: Initial placement.
469+
* @domain: Domain to put the bo in.
470+
* @busy_domain: Domain to put the bo if busy.
465471
* @interruptible: Whether waits should be performed interruptible.
466472
* @pin: If the BO should be created pinned at a fixed location.
467473
* Returns: Zero on success, negative error code on error.
@@ -470,7 +476,9 @@ int vmw_bo_create(struct vmw_private *vmw,
470476
*/
471477
int vmw_bo_init(struct vmw_private *dev_priv,
472478
struct vmw_bo *vmw_bo,
473-
size_t size, struct ttm_placement *placement,
479+
size_t size,
480+
u32 domain,
481+
u32 busy_domain,
474482
bool interruptible, bool pin)
475483
{
476484
struct ttm_operation_ctx ctx = {
@@ -489,10 +497,9 @@ int vmw_bo_init(struct vmw_private *dev_priv,
489497
size = ALIGN(size, PAGE_SIZE);
490498
drm_gem_private_object_init(vdev, &vmw_bo->base.base, size);
491499

492-
ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, size,
493-
ttm_bo_type_device,
494-
placement,
495-
0, &ctx, NULL, NULL, vmw_bo_free);
500+
vmw_bo_placement_set(vmw_bo, domain, busy_domain);
501+
ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, size, ttm_bo_type_device,
502+
&vmw_bo->placement, 0, &ctx, NULL, NULL, vmw_bo_free);
496503
if (unlikely(ret)) {
497504
return ret;
498505
}
@@ -827,3 +834,101 @@ void vmw_bo_move_notify(struct ttm_buffer_object *bo,
827834
if (mem->mem_type != VMW_PL_MOB && bo->resource->mem_type == VMW_PL_MOB)
828835
vmw_resource_unbind_list(vbo);
829836
}
837+
838+
static u32
839+
set_placement_list(struct ttm_place *pl, u32 domain)
840+
{
841+
u32 n = 0;
842+
843+
/*
844+
* The placements are ordered according to our preferences
845+
*/
846+
if (domain & VMW_BO_DOMAIN_MOB) {
847+
pl[n].mem_type = VMW_PL_MOB;
848+
pl[n].flags = 0;
849+
pl[n].fpfn = 0;
850+
pl[n].lpfn = 0;
851+
n++;
852+
}
853+
if (domain & VMW_BO_DOMAIN_GMR) {
854+
pl[n].mem_type = VMW_PL_GMR;
855+
pl[n].flags = 0;
856+
pl[n].fpfn = 0;
857+
pl[n].lpfn = 0;
858+
n++;
859+
}
860+
if (domain & VMW_BO_DOMAIN_VRAM) {
861+
pl[n].mem_type = TTM_PL_VRAM;
862+
pl[n].flags = 0;
863+
pl[n].fpfn = 0;
864+
pl[n].lpfn = 0;
865+
n++;
866+
}
867+
WARN_ON((domain & VMW_BO_DOMAIN_WAITABLE_SYS) != 0);
868+
if (domain & VMW_BO_DOMAIN_WAITABLE_SYS) {
869+
pl[n].mem_type = VMW_PL_SYSTEM;
870+
pl[n].flags = 0;
871+
pl[n].fpfn = 0;
872+
pl[n].lpfn = 0;
873+
n++;
874+
}
875+
if (domain & VMW_BO_DOMAIN_SYS) {
876+
pl[n].mem_type = TTM_PL_SYSTEM;
877+
pl[n].flags = 0;
878+
pl[n].fpfn = 0;
879+
pl[n].lpfn = 0;
880+
n++;
881+
}
882+
883+
WARN_ON(!n);
884+
if (!n) {
885+
pl[n].mem_type = TTM_PL_SYSTEM;
886+
pl[n].flags = 0;
887+
pl[n].fpfn = 0;
888+
pl[n].lpfn = 0;
889+
n++;
890+
}
891+
return n;
892+
}
893+
894+
void vmw_bo_placement_set(struct vmw_bo *bo, u32 domain, u32 busy_domain)
895+
{
896+
struct ttm_device *bdev = bo->base.bdev;
897+
struct vmw_private *vmw =
898+
container_of(bdev, struct vmw_private, bdev);
899+
struct ttm_placement *pl = &bo->placement;
900+
bool mem_compatible = false;
901+
u32 i;
902+
903+
pl->placement = bo->places;
904+
pl->num_placement = set_placement_list(bo->places, domain);
905+
906+
if (drm_debug_enabled(DRM_UT_DRIVER) && bo->base.resource) {
907+
for (i = 0; i < pl->num_placement; ++i) {
908+
if (bo->base.resource->mem_type == TTM_PL_SYSTEM ||
909+
bo->base.resource->mem_type == pl->placement[i].mem_type)
910+
mem_compatible = true;
911+
}
912+
if (!mem_compatible)
913+
drm_warn(&vmw->drm,
914+
"%s: Incompatible transition from "
915+
"bo->base.resource->mem_type = %u to domain = %u\n",
916+
__func__, bo->base.resource->mem_type, domain);
917+
}
918+
919+
pl->busy_placement = bo->busy_places;
920+
pl->num_busy_placement = set_placement_list(bo->busy_places, busy_domain);
921+
}
922+
923+
void vmw_bo_placement_set_default_accelerated(struct vmw_bo *bo)
924+
{
925+
struct ttm_device *bdev = bo->base.bdev;
926+
struct vmw_private *vmw =
927+
container_of(bdev, struct vmw_private, bdev);
928+
u32 domain = VMW_BO_DOMAIN_GMR | VMW_BO_DOMAIN_VRAM;
929+
930+
if (vmw->has_mob)
931+
domain = VMW_BO_DOMAIN_MOB;
932+
933+
vmw_bo_placement_set(bo, domain, domain);
934+
}

drivers/gpu/drm/vmwgfx/vmwgfx_bo.h

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "device_include/svga_reg.h"
3232

3333
#include <drm/ttm/ttm_bo.h>
34+
#include <drm/ttm/ttm_placement.h>
3435

3536
#include <linux/rbtree_types.h>
3637
#include <linux/types.h>
@@ -40,6 +41,14 @@ struct vmw_fence_obj;
4041
struct vmw_private;
4142
struct vmw_resource;
4243

44+
enum vmw_bo_domain {
45+
VMW_BO_DOMAIN_SYS = BIT(0),
46+
VMW_BO_DOMAIN_WAITABLE_SYS = BIT(1),
47+
VMW_BO_DOMAIN_VRAM = BIT(2),
48+
VMW_BO_DOMAIN_GMR = BIT(3),
49+
VMW_BO_DOMAIN_MOB = BIT(4),
50+
};
51+
4352
/**
4453
* struct vmw_bo - TTM buffer object with vmwgfx additions
4554
* @base: The TTM buffer object
@@ -53,6 +62,11 @@ struct vmw_resource;
5362
*/
5463
struct vmw_bo {
5564
struct ttm_buffer_object base;
65+
66+
struct ttm_placement placement;
67+
struct ttm_place places[5];
68+
struct ttm_place busy_places[5];
69+
5670
struct rb_root res_tree;
5771

5872
atomic_t cpu_writers;
@@ -64,17 +78,24 @@ struct vmw_bo {
6478
struct vmw_bo_dirty *dirty;
6579
};
6680

81+
void vmw_bo_placement_set(struct vmw_bo *bo, u32 domain, u32 busy_domain);
82+
void vmw_bo_placement_set_default_accelerated(struct vmw_bo *bo);
83+
6784
int vmw_bo_create_kernel(struct vmw_private *dev_priv,
6885
unsigned long size,
6986
struct ttm_placement *placement,
7087
struct ttm_buffer_object **p_bo);
7188
int vmw_bo_create(struct vmw_private *dev_priv,
72-
size_t size, struct ttm_placement *placement,
89+
size_t size,
90+
u32 domain,
91+
u32 busy_domain,
7392
bool interruptible, bool pin,
7493
struct vmw_bo **p_bo);
7594
int vmw_bo_init(struct vmw_private *dev_priv,
7695
struct vmw_bo *vmw_bo,
77-
size_t size, struct ttm_placement *placement,
96+
size_t size,
97+
u32 domain,
98+
u32 busy_domain,
7899
bool interruptible, bool pin);
79100
int vmw_bo_unref_ioctl(struct drm_device *dev, void *data,
80101
struct drm_file *file_priv);

drivers/gpu/drm/vmwgfx/vmwgfx_context.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ static const struct vmw_res_func vmw_legacy_context_func = {
7676
.needs_backup = false,
7777
.may_evict = false,
7878
.type_name = "legacy contexts",
79-
.backup_placement = NULL,
79+
.domain = VMW_BO_DOMAIN_SYS,
80+
.busy_domain = VMW_BO_DOMAIN_SYS,
8081
.create = NULL,
8182
.destroy = NULL,
8283
.bind = NULL,
@@ -90,7 +91,8 @@ static const struct vmw_res_func vmw_gb_context_func = {
9091
.prio = 3,
9192
.dirty_prio = 3,
9293
.type_name = "guest backed contexts",
93-
.backup_placement = &vmw_mob_placement,
94+
.domain = VMW_BO_DOMAIN_MOB,
95+
.busy_domain = VMW_BO_DOMAIN_MOB,
9496
.create = vmw_gb_context_create,
9597
.destroy = vmw_gb_context_destroy,
9698
.bind = vmw_gb_context_bind,
@@ -104,7 +106,8 @@ static const struct vmw_res_func vmw_dx_context_func = {
104106
.prio = 3,
105107
.dirty_prio = 3,
106108
.type_name = "dx contexts",
107-
.backup_placement = &vmw_mob_placement,
109+
.domain = VMW_BO_DOMAIN_MOB,
110+
.busy_domain = VMW_BO_DOMAIN_MOB,
108111
.create = vmw_dx_context_create,
109112
.destroy = vmw_dx_context_destroy,
110113
.bind = vmw_dx_context_bind,

drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ static const struct vmw_res_func vmw_cotable_func = {
136136
.prio = 3,
137137
.dirty_prio = 3,
138138
.type_name = "context guest backed object tables",
139-
.backup_placement = &vmw_mob_placement,
139+
.domain = VMW_BO_DOMAIN_MOB,
140+
.busy_domain = VMW_BO_DOMAIN_MOB,
140141
.create = vmw_cotable_create,
141142
.destroy = vmw_cotable_destroy,
142143
.bind = vmw_cotable_bind,
@@ -424,7 +425,8 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
424425
* for the new COTable. Initially pin the buffer object to make sure
425426
* we can use tryreserve without failure.
426427
*/
427-
ret = vmw_bo_create(dev_priv, new_size, &vmw_mob_placement,
428+
ret = vmw_bo_create(dev_priv, new_size,
429+
VMW_BO_DOMAIN_MOB, VMW_BO_DOMAIN_MOB,
428430
true, true, &buf);
429431
if (ret) {
430432
DRM_ERROR("Failed initializing new cotable MOB.\n");
@@ -465,7 +467,10 @@ static int vmw_cotable_resize(struct vmw_resource *res, size_t new_size)
465467
}
466468

467469
/* Unpin new buffer, and switch backup buffers. */
468-
ret = ttm_bo_validate(bo, &vmw_mob_placement, &ctx);
470+
vmw_bo_placement_set(buf,
471+
VMW_BO_DOMAIN_MOB,
472+
VMW_BO_DOMAIN_MOB);
473+
ret = ttm_bo_validate(bo, &buf->placement, &ctx);
469474
if (unlikely(ret != 0)) {
470475
DRM_ERROR("Failed validating new COTable backup buffer.\n");
471476
goto out_wait;

drivers/gpu/drm/vmwgfx/vmwgfx_drv.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,8 @@ static int vmw_dummy_query_bo_create(struct vmw_private *dev_priv)
399399
* user of the bo currently.
400400
*/
401401
ret = vmw_bo_create(dev_priv, PAGE_SIZE,
402-
&vmw_sys_placement, false, true, &vbo);
402+
VMW_BO_DOMAIN_SYS, VMW_BO_DOMAIN_SYS,
403+
false, true, &vbo);
403404
if (unlikely(ret != 0))
404405
return ret;
405406

drivers/gpu/drm/vmwgfx/vmwgfx_drv.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -930,9 +930,7 @@ extern struct ttm_placement vmw_vram_placement;
930930
extern struct ttm_placement vmw_vram_sys_placement;
931931
extern struct ttm_placement vmw_vram_gmr_placement;
932932
extern struct ttm_placement vmw_sys_placement;
933-
extern struct ttm_placement vmw_srf_placement;
934933
extern struct ttm_placement vmw_mob_placement;
935-
extern struct ttm_placement vmw_nonfixed_placement;
936934
extern struct ttm_device_funcs vmw_bo_driver;
937935
extern const struct vmw_sg_table *
938936
vmw_bo_sg_table(struct ttm_buffer_object *bo);

0 commit comments

Comments
 (0)