[PATCH] drm/vmwgfx: Stop using dev_private to store driver data.

2024-05-01 Thread Maaz Mombasawala
Currently vmwgfx uses the dev_private opaque pointer in drm_device to store
driver data in vmw_private struct. Using dev_private is deprecated, and the
recommendation is to embed struct drm_device in the larger per-device
structure.

The vmwgfx driver already embeds struct drm_device in its struct
vmw_private, so switch to using that exclusively and stop using
dev_private.

Signed-off-by: Maaz Mombasawala 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 --
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 ++--
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index bdad93864b98..97e48e93dbbf 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -858,8 +858,6 @@ static int vmw_driver_load(struct vmw_private *dev_priv, 
u32 pci_id)
bool refuse_dma = false;
struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
 
-   dev_priv->drm.dev_private = dev_priv;
-
vmw_sw_context_init(dev_priv);
 
mutex_init(_priv->cmdbuf_mutex);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 4ecaea0026fc..df89e468a1fc 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -638,7 +638,7 @@ static inline struct vmw_surface *vmw_res_to_srf(struct 
vmw_resource *res)
 
 static inline struct vmw_private *vmw_priv(struct drm_device *dev)
 {
-   return (struct vmw_private *)dev->dev_private;
+   return container_of(dev, struct vmw_private, drm);
 }
 
 static inline struct vmw_private *vmw_priv_from_ttm(struct ttm_device *bdev)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 13b2820cae51..b3f0fb6828de 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -276,7 +276,7 @@ static void vmw_du_put_cursor_mob(struct vmw_cursor_plane 
*vcp,
 static int vmw_du_get_cursor_mob(struct vmw_cursor_plane *vcp,
 struct vmw_plane_state *vps)
 {
-   struct vmw_private *dev_priv = vcp->base.dev->dev_private;
+   struct vmw_private *dev_priv = vmw_priv(vcp->base.dev);
u32 size = vmw_du_cursor_mob_size(vps->base.crtc_w, vps->base.crtc_h);
u32 i;
u32 cursor_max_dim, mob_max_size;
@@ -515,7 +515,7 @@ void vmw_du_cursor_plane_destroy(struct drm_plane *plane)
struct vmw_cursor_plane *vcp = vmw_plane_to_vcp(plane);
u32 i;
 
-   vmw_cursor_update_position(plane->dev->dev_private, false, 0, 0);
+   vmw_cursor_update_position(vmw_priv(plane->dev), false, 0, 0);
 
for (i = 0; i < ARRAY_SIZE(vcp->cursor_mobs); i++)
vmw_du_destroy_cursor_mob(>cursor_mobs[i]);
-- 
2.34.1



Re: [PATCH] drm/vmwgfx: Fix invalid reads in fence signaled events

2024-04-25 Thread Maaz Mombasawala
On 4/25/24 12:27, Zack Rusin wrote:
> Correctly set the length of the drm_event to the size of the structure
> that's actually used.
> 
> The length of the drm_event was set to the parent structure instead of
> to the drm_vmw_event_fence which is supposed to be read. drm_read
> uses the length parameter to copy the event to the user space thus
> resuling in oob reads.
> 
> Signed-off-by: Zack Rusin 
> Fixes: 8b7de6aa8468 ("vmwgfx: Rework fence event action")
> Reported-by: zdi-disclosu...@trendmicro.com # ZDI-CAN-23566
> Cc: David Airlie 
> CC: Daniel Vetter 
> Cc: Zack Rusin 
> Cc: Broadcom internal kernel review list 
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Cc:  # v3.4+
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> index 2a0cda324703..5efc6a766f64 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
> @@ -991,7 +991,7 @@ static int vmw_event_fence_action_create(struct drm_file 
> *file_priv,
>   }
>  
>   event->event.base.type = DRM_VMW_EVENT_FENCE_SIGNALED;
> - event->event.base.length = sizeof(*event);
> + event->event.base.length = sizeof(event->event);
>   event->event.user_data = user_data;
>  
>   ret = drm_event_reserve_init(dev, file_priv, >base, 
> >event.base);

LGTM!

Reviewed-by: Maaz Mombasawala 

Thanks,

Maaz Mombasawala 


Re: [PATCH 1/5] drm/vmwgfx: Implement virtual kms

2024-04-05 Thread Maaz Mombasawala
On 4/2/24 16:28, Zack Rusin wrote:
>  
> @@ -541,6 +518,8 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, 
> unsigned unit)
>dev_priv->implicit_placement_property,
>1);
>  
> + vmw_du_init(>base);
> +
>   return 0;
>  
>  err_free_unregister:

> @@ -905,6 +900,9 @@ 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);
> +
> + vmw_du_init(>base);
> +
>   return 0;
>  
>  err_free_unregister:

> @@ -1575,6 +1576,9 @@ 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);
> +
> + vmw_du_init(>base);
> +
>   return 0;
>  
>  err_free_unregister:

Shouldn't calls to vmw_du_init() be behind an if(vkms_enabled) condition?

Thanks,

Maaz Mombasawala 



Re: [PATCH] drm/vmwgfx: Fix possible null pointer derefence with invalid contexts

2024-01-10 Thread Maaz Mombasawala
On 1/10/24 12:03, Zack Rusin wrote:
> vmw_context_cotable can return either an error or a null pointer and its
> usage sometimes went unchecked. Subsequent code would then try to access
> either a null pointer or an error value.
> 
> The invalid dereferences were only possible with malformed userspace
> apps which never properly initialized the rendering contexts.
> 
> Check the results of vmw_context_cotable to fix the invalid derefs.
> 
> Thanks:
> ziming zhang(@ezrak1e) from Ant Group Light-Year Security Lab
> who was the first person to discover it.
> Niels De Graef who reported it and helped to track down the poc.
> 
> Fixes: 9c079b8ce8bf ("drm/vmwgfx: Adapt execbuf to the new validation api")
> Cc:  # v4.20+
> Reported-by: Niels De Graef  
> Signed-off-by: Zack Rusin 
> Cc: Martin Krastev 
> Cc: Maaz Mombasawala 
> Cc: Ian Forbes 
> Cc: Broadcom internal kernel review list 
> 
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> index 272141b6164c..4f09959d27ba 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> @@ -447,7 +447,7 @@ static int vmw_resource_context_res_add(struct 
> vmw_private *dev_priv,
>   vmw_res_type(ctx) == vmw_res_dx_context) {
>   for (i = 0; i < cotable_max; ++i) {
>   res = vmw_context_cotable(ctx, i);
> - if (IS_ERR(res))
> + if (IS_ERR_OR_NULL(res))
>   continue;
>  
>   ret = vmw_execbuf_res_val_add(sw_context, res,
> @@ -1266,6 +1266,8 @@ static int vmw_cmd_dx_define_query(struct vmw_private 
> *dev_priv,
>   return -EINVAL;
>  
>   cotable_res = vmw_context_cotable(ctx_node->ctx, SVGA_COTABLE_DXQUERY);
> + if (IS_ERR_OR_NULL(cotable_res))
> + return cotable_res ? PTR_ERR(cotable_res) : -EINVAL;
>   ret = vmw_cotable_notify(cotable_res, cmd->body.queryId);
>  
>   return ret;
> @@ -2484,6 +2486,8 @@ static int vmw_cmd_dx_view_define(struct vmw_private 
> *dev_priv,
>   return ret;
>  
>   res = vmw_context_cotable(ctx_node->ctx, vmw_view_cotables[view_type]);
> + if (IS_ERR_OR_NULL(res))
> + return res ? PTR_ERR(res) : -EINVAL;
>   ret = vmw_cotable_notify(res, cmd->defined_id);
>   if (unlikely(ret != 0))
>   return ret;
> @@ -2569,8 +2573,8 @@ static int vmw_cmd_dx_so_define(struct vmw_private 
> *dev_priv,
>  
>   so_type = vmw_so_cmd_to_type(header->id);
>   res = vmw_context_cotable(ctx_node->ctx, vmw_so_cotables[so_type]);
> - if (IS_ERR(res))
> - return PTR_ERR(res);
> + if (IS_ERR_OR_NULL(res))
> + return res ? PTR_ERR(res) : -EINVAL;
>   cmd = container_of(header, typeof(*cmd), header);
>   ret = vmw_cotable_notify(res, cmd->defined_id);
>  
> @@ -2689,6 +2693,8 @@ static int vmw_cmd_dx_define_shader(struct vmw_private 
> *dev_priv,
>   return -EINVAL;
>  
>   res = vmw_context_cotable(ctx_node->ctx, SVGA_COTABLE_DXSHADER);
> + if (IS_ERR_OR_NULL(res))
> + return res ? PTR_ERR(res) : -EINVAL;
>   ret = vmw_cotable_notify(res, cmd->body.shaderId);
>   if (ret)
>   return ret;
> @@ -3010,6 +3016,8 @@ static int vmw_cmd_dx_define_streamoutput(struct 
> vmw_private *dev_priv,
>   }
>  
>   res = vmw_context_cotable(ctx_node->ctx, SVGA_COTABLE_STREAMOUTPUT);
> + if (IS_ERR_OR_NULL(res))
> + return res ? PTR_ERR(res) : -EINVAL;
>   ret = vmw_cotable_notify(res, cmd->body.soid);
>   if (ret)
>   return ret;

LGTM!

Reviewed-by: Maaz Mombasawala 

Thanks,

Maaz Mombasawala 



Re: [PATCH] drm/vmwgfx: Unmap the surface before resetting it on a plane state

2024-01-03 Thread Maaz Mombasawala
On 12/23/23 21:25, Zack Rusin wrote:
> Switch to a new plane state requires unreferencing of all held surfaces.
> In the work required for mob cursors the mapped surfaces started being
> cached but the variable indicating whether the surface is currently
> mapped was not being reset. This leads to crashes as the duplicated
> state, incorrectly, indicates the that surface is mapped even when
> no surface is present. That's because after unreferencing the surface
> it's perfectly possible for the plane to be backed by a bo instead of a
> surface.
>
> Reset the surface mapped flag when unreferencing the plane state surface
> to fix null derefs in cleanup. Fixes crashes in KDE KWin 6.0 on Wayland:
>
> Oops:  [#1] PREEMPT SMP PTI
> CPU: 4 PID: 2533 Comm: kwin_wayland Not tainted 6.7.0-rc3-vmwgfx #2
> Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference 
> Platform, BIOS 6.00 11/12/2020
> RIP: 0010:vmw_du_cursor_plane_cleanup_fb+0x124/0x140 [vmwgfx]
> Code: 00 00 00 75 3a 48 83 c4 10 5b 5d c3 cc cc cc cc 48 8b b3 a8 00 00 00 48 
> c7 c7 99 90 43 c0 e8 93 c5 db ca 48 8b 83 a8 00 00 00 <48> 8b 78 28 e8 e3 f>
> RSP: 0018:b6b98216fa80 EFLAGS: 00010246
> RAX:  RBX: 969d84cdcb00 RCX: 0027
> RDX:  RSI: 0001 RDI: 969e75f21600
> RBP: 969d4143dc50 R08:  R09: b6b98216f920
> R10: 0003 R11: 969e7feb3b10 R12: 
> R13:  R14: 027b R15: 969d49c9fc00
> FS:  7f1e8f1b4180() GS:969e75f0() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 0028 CR3: 000104006004 CR4: 003706f0
> Call Trace:
>  
>  ? __die+0x23/0x70
>  ? page_fault_oops+0x171/0x4e0
>  ? exc_page_fault+0x7f/0x180
>  ? asm_exc_page_fault+0x26/0x30
>  ? vmw_du_cursor_plane_cleanup_fb+0x124/0x140 [vmwgfx]
>  drm_atomic_helper_cleanup_planes+0x9b/0xc0
>  commit_tail+0xd1/0x130
>  drm_atomic_helper_commit+0x11a/0x140
>  drm_atomic_commit+0x97/0xd0
>  ? __pfx___drm_printfn_info+0x10/0x10
>  drm_atomic_helper_update_plane+0xf5/0x160
>  drm_mode_cursor_universal+0x10e/0x270
>  drm_mode_cursor_common+0x102/0x230
>  ? __pfx_drm_mode_cursor2_ioctl+0x10/0x10
>  drm_ioctl_kernel+0xb2/0x110
>  drm_ioctl+0x26d/0x4b0
>  ? __pfx_drm_mode_cursor2_ioctl+0x10/0x10
>  ? __pfx_drm_ioctl+0x10/0x10
>  vmw_generic_ioctl+0xa4/0x110 [vmwgfx]
>  __x64_sys_ioctl+0x94/0xd0
>  do_syscall_64+0x61/0xe0
>  ? __x64_sys_ioctl+0xaf/0xd0
>  ? syscall_exit_to_user_mode+0x2b/0x40
>  ? do_syscall_64+0x70/0xe0
>  ? __x64_sys_ioctl+0xaf/0xd0
>  ? syscall_exit_to_user_mode+0x2b/0x40
>  ? do_syscall_64+0x70/0xe0
>  ? exc_page_fault+0x7f/0x180
>  entry_SYSCALL_64_after_hwframe+0x6e/0x76
> RIP: 0033:0x7f1e93f279ed
> Code: 04 25 28 00 00 00 48 89 45 c8 31 c0 48 8d 45 10 c7 45 b0 10 00 00 00 48 
> 89 45 b8 48 8d 45 d0 48 89 45 c0 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff f>
> RSP: 002b:7ffca0faf600 EFLAGS: 0246 ORIG_RAX: 0010
> RAX: ffda RBX: 55db876ed2c0 RCX: 7f1e93f279ed
> RDX: 7ffca0faf6c0 RSI: c02464bb RDI: 0015
> RBP: 7ffca0faf650 R08: 55db87184010 R09: 0007
> R10: 55db886471a0 R11: 0246 R12: 7ffca0faf6c0
> R13: c02464bb R14: 0015 R15: 7ffca0faf790
>  
> Modules linked in: snd_seq_dummy snd_hrtimer nf_conntrack_netbios_ns 
> nf_conntrack_broadcast nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib 
> nft_reject_ine>
> CR2: 0028
> ---[ end trace  ]---
> RIP: 0010:vmw_du_cursor_plane_cleanup_fb+0x124/0x140 [vmwgfx]
> Code: 00 00 00 75 3a 48 83 c4 10 5b 5d c3 cc cc cc cc 48 8b b3 a8 00 00 00 48 
> c7 c7 99 90 43 c0 e8 93 c5 db ca 48 8b 83 a8 00 00 00 <48> 8b 78 28 e8 e3 f>
> RSP: 0018:b6b98216fa80 EFLAGS: 00010246
> RAX:  RBX: 969d84cdcb00 RCX: 0027
> RDX:  RSI: 0001 RDI: 969e75f21600
> RBP: 969d4143dc50 R08:  R09: b6b98216f920
> R10: 0003 R11: 969e7feb3b10 R12: 
> R13:  R14: 027b R15: 969d49c9fc00
> FS:  7f1e8f1b4180() GS:969e75f0() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 0028 CR3: 000104006004 CR4: 003706f0
>
> Signed-off-by: Zack Rusin 
> Fixes: 485d98d472d5 ("drm/vmwgfx: Add support for CursorMob and CursorBypass 
> 4")
> Reported-by: Stefan Hoffmeister 
> Closes: https://gitlab.freedesktop.org/drm/misc/-/issues/34
> Cc: Martin Krastev 
> Cc: Maaz 

[PATCH 2/2] drm/vmwgfx: Add SPDX header to vmwgfx_drm.h

2023-10-09 Thread Maaz Mombasawala (VMware)
From: Maaz Mombasawala 

Update vmwgfx_drm.h with SPDX-License-Identifier:
(GPL-2.0 WITH Linux-syscall-note) OR MIT

Signed-off-by: Maaz Mombasawala 
Reviewed-by: Martin Krastev 
Signed-off-by: Maaz Mombasawala (VMware) 
---
 include/uapi/drm/vmwgfx_drm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/drm/vmwgfx_drm.h b/include/uapi/drm/vmwgfx_drm.h
index 26d96fecb902..7d786a0cc835 100644
--- a/include/uapi/drm/vmwgfx_drm.h
+++ b/include/uapi/drm/vmwgfx_drm.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: (GPL-2.0 WITH Linux-syscall-note) OR MIT */
 /**
  *
  * Copyright © 2009-2023 VMware, Inc., Palo Alto, CA., USA
-- 
2.34.1



[PATCH 1/2] drm/vmwgfx: Make all surfaces shareable

2023-10-09 Thread Maaz Mombasawala (VMware)
From: Maaz Mombasawala 

There is no real need to have a separate pool for shareable and
non-shareable surfaces. Make all surfaces shareable, regardless of whether
the drm_vmw_surface_flag_shareable has been specified.

Signed-off-by: Maaz Mombasawala 
Reviewed-by: Martin Krastev 
Signed-off-by: Maaz Mombasawala (VMware) 
---
 drivers/gpu/drm/vmwgfx/ttm_object.c |  6 +++---
 drivers/gpu/drm/vmwgfx/ttm_object.h |  3 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 17 ++---
 include/uapi/drm/vmwgfx_drm.h   |  5 +++--
 4 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.c 
b/drivers/gpu/drm/vmwgfx/ttm_object.c
index ddf8373c1d77..6806c05e57f6 100644
--- a/drivers/gpu/drm/vmwgfx/ttm_object.c
+++ b/drivers/gpu/drm/vmwgfx/ttm_object.c
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 OR MIT */
 /**
  *
- * Copyright (c) 2009-2022 VMware, Inc., Palo Alto, CA., USA
+ * Copyright (c) 2009-2023 VMware, Inc., Palo Alto, CA., USA
  * All Rights Reserved.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
@@ -648,7 +648,6 @@ int ttm_prime_handle_to_fd(struct ttm_object_file *tfile,
  * @tfile: struct ttm_object_file identifying the caller
  * @size: The size of the dma_bufs we export.
  * @prime: The object to be initialized.
- * @shareable: See ttm_base_object_init
  * @type: See ttm_base_object_init
  * @refcount_release: See ttm_base_object_init
  *
@@ -656,10 +655,11 @@ int ttm_prime_handle_to_fd(struct ttm_object_file *tfile,
  * for data sharing between processes and devices.
  */
 int ttm_prime_object_init(struct ttm_object_file *tfile, size_t size,
- struct ttm_prime_object *prime, bool shareable,
+ struct ttm_prime_object *prime,
  enum ttm_object_type type,
  void (*refcount_release) (struct ttm_base_object **))
 {
+   bool shareable = !!(type == VMW_RES_SURFACE);
mutex_init(>mutex);
prime->size = PAGE_ALIGN(size);
prime->real_type = type;
diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.h 
b/drivers/gpu/drm/vmwgfx/ttm_object.h
index e6b77ee33e55..573e038c0fab 100644
--- a/drivers/gpu/drm/vmwgfx/ttm_object.h
+++ b/drivers/gpu/drm/vmwgfx/ttm_object.h
@@ -1,6 +1,6 @@
 /**
  *
- * Copyright (c) 2006-2022 VMware, Inc., Palo Alto, CA., USA
+ * Copyright (c) 2006-2023 VMware, Inc., Palo Alto, CA., USA
  * All Rights Reserved.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
@@ -288,7 +288,6 @@ extern void ttm_object_device_release(struct 
ttm_object_device **p_tdev);
 extern int ttm_prime_object_init(struct ttm_object_file *tfile,
 size_t size,
 struct ttm_prime_object *prime,
-bool shareable,
 enum ttm_object_type type,
 void (*refcount_release)
 (struct ttm_base_object **));
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 5db403ee8261..3156bdd095e9 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -836,8 +836,6 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
srf->snooper.image = NULL;
}
 
-   user_srf->prime.base.shareable = false;
-   user_srf->prime.base.tfile = NULL;
if (drm_is_primary_client(file_priv))
user_srf->master = drm_file_get_master(file_priv);
 
@@ -851,10 +849,10 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
goto out_unlock;
 
/*
-* A gb-aware client referencing a shared surface will
-* expect a backup buffer to be present.
+* A gb-aware client referencing a surface will expect a backup
+* buffer to be present.
 */
-   if (dev_priv->has_mob && req->shareable) {
+   if (dev_priv->has_mob) {
uint32_t backup_handle;
 
ret = vmw_gem_object_create_with_handle(dev_priv,
@@ -875,8 +873,9 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
}
 
tmp = vmw_resource_reference(>res);
-   ret = ttm_prime_object_init(tfile, res->guest_memory_size, 
_srf->prime,
-   req->shareable, VMW_RES_SURFACE,
+   ret = ttm_prime_object_init(tfile, res->guest_memory_size,
+   _srf->prime,
+   VMW_RES_SURFACE,
_user_surface_base_release);
 
if (unlikely(ret != 0)) {
@@ -1557,8 +1556,6 @@ vmw_gb_surfa

Re: [PATCH] drm/vmwgfx: Fix possible invalid drm gem put calls

2023-08-23 Thread Maaz Mombasawala (VMWare)

LGTM!

Reviewed-by: Maaz Mombasawala

Maaz Mombasawala (VMware)

On 8/17/2023 9:13 PM, 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();

-   drm_gem_object_put(>tbo.base);
+   vmw_user_bo_unref(buf);
  
  out_unlock:

  

Re: [PATCH] drm/vmwgfx: Fix shader stage validation

2023-06-16 Thread Maaz Mombasawala (VMWare)

LGTM.

Reviewed-by: Maaz Mombasawala

Maaz Mombasawala (VMware)


On 6/16/2023 12:09 PM, 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->

Re: [PATCH 2/3] drm/vmwgfx: Print errors when running on broken/unsupported configs

2023-03-21 Thread &quot;Maaz Mombasawala (VMware)
On 3/20/23 19: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;
> +}


LGTM

Reviewed-by: Maaz Mombasawala 

-- 
Maaz Mombasawala (VMware) 



Re: [PATCH] drm/vmwgfx: Stop accessing buffer objects which failed init

2023-02-08 Thread &quot;Maaz Mombasawala (VMware)
On 2/8/23 10: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);

LGTM!

Reviewed-by: Maaz Mombasawala 

-- 
Maaz Mombasawala (VMware) 



Re: [PATCH] drm/vmwgfx: Do not drop the reference to the handle too soon

2023-02-08 Thread &quot;Maaz Mombasawala (VMware)
On 2/8/23 13: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);


LGTM!

Reviewed-by: Maaz Mombasawala 

-- 
Maaz Mombasawala (VMware) 



Re: [PATCH v2 8/8] drm/vmwgfx: Stop using raw ttm_buffer_object's

2023-01-31 Thread &quot;Maaz Mombasawala (VMware)
On 1/30/23 19: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 : 

Re: [PATCH v2 7/8] drm/vmwgfx: Abstract placement selection

2023-01-31 Thread &quot;Maaz Mombasawala (VMware)
On 1/30/23 19: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,
> -

Re: [PATCH v2 6/8] drm/vmwgfx: Rename dummy to is_iomem

2023-01-31 Thread &quot;Maaz Mombasawala (VMware)
On 1/30/23 19: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)) {


LGTM!

Reviewed-by: Maaz Mombasawala 
-- 
Maaz Mombasawala (VMware) 



Re: [PATCH v2 5/8] drm/vmwgfx: Cleanup the vmw bo usage in the cursor paths

2023-01-31 Thread &quot;Maaz Mombasawala (VMware)
On 1/30/23 19:35, 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.
> 
> Signed-off-by: Zack Rusin 
> Reviewed-by: Martin Krastev 
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.h  |  3 ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 +---
>  2 files changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> index e2dadd68a16d..2ede1e28d7ce 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 */
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 6780391c57ea..1082218a1cfc 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -669,8 +669,7 @@ vmw_du_cursor_plane_cleanup_fb(struct drm_plane *plane,
>   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 +782,6 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane *plane,
>   struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
>   struct vmw_plane_state *old_vps = vmw_plane_state_to_vps(old_state);
>   s32 hotspot_x, hotspot_y;
> - bool dummy;
>  
>   hotspot_x = du->hotspot_x;
>   hotspot_y = du->hotspot_y;
> @@ -828,11 +823,6 @@ vmw_du_cursor_plane_atomic_update(struct drm_plane 
> *plane,
>   hotspot_x, hotspot_y);
>   }
>  
> - if (vps->bo) {
> -     if (ttm_kmap_obj_virtual(>bo->map, ))
> - atomic_dec(>bo->base_mapped_count);
> - }
> -
>   du->cursor_x = new_state->crtc_x + du->set_gui_x;
>   du->cursor_y = new_state->crtc_y + du->set_gui_y;
>  

LGTM!

Reviewed-by: Maaz Mombasawala 
-- 
Maaz Mombasawala (VMware) 



Re: [PATCH v2 4/8] drm/vmwgfx: Simplify fb pinning

2023-01-31 Thread &quot;Maaz Mombasawala (VMware)
; -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);
> -}
> -
>  /**
>   * vmw_create_bo_proxy - create a proxy surface for the buffer object
>   *
> @@ -1766,9 +1703,6 @@ vmw_kms_new_framebuffer(struct vmw_private *dev_priv,
>   if (ret)
>   return ERR_PTR(ret);
>  
> - vfb->pin = vmw_framebuffer_pin;
> - vfb->unpin = vmw_framebuffer_unpin;
> -
>   return vfb;
>  }
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 2d097ba20ad8..7a97e53e8e51 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -1,7 +1,7 @@
>  /* SPDX-License-Identifier: GPL-2.0 OR MIT */
>  /**
>   *
> - * Copyright 2009-2022 VMware, Inc., Palo Alto, CA., USA
> + * Copyright 2009-2023 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
> @@ -217,8 +217,6 @@ struct vmw_kms_dirty {
>   */
>  struct vmw_framebuffer {
>   struct drm_framebuffer base;
> - int (*pin)(struct vmw_framebuffer *fb);
> - int (*unpin)(struct vmw_framebuffer *fb);
>   bool bo;
>   uint32_t user_handle;
>  };
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> index a56e5d0ca3c6..b77fe0bc18a7 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0 OR MIT
>  /**
>   *
> - * Copyright 2009-2022 VMware, Inc., Palo Alto, CA., USA
> + * Copyright 2009-2023 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
> @@ -25,11 +25,13 @@
>   *
>   **/
>  
> +#include "vmwgfx_bo.h"
> +#include "vmwgfx_kms.h"
> +
>  #include 
>  #include 
>  #include 
>  
> -#include "vmwgfx_kms.h"
>  
>  #define vmw_crtc_to_ldu(x) \
>   container_of(x, struct vmw_legacy_display_unit, base.crtc)
> @@ -134,6 +136,47 @@ static int vmw_ldu_commit_list(struct vmw_private 
> *dev_priv)
>   return 0;
>  }
>  
> +/*
> + * Pin the buffer in a location suitable for access by the
> + * display system.
> + */
> +static int vmw_ldu_fb_pin(struct vmw_framebuffer *vfb)
> +{
> + struct vmw_private *dev_priv = vmw_priv(vfb->base.dev);
> + struct vmw_bo *buf;
> + int ret;
> +
> + buf = vfb->bo ?  vmw_framebuffer_to_vfbd(>base)->buffer :
> + vmw_framebuffer_to_vfbs(>base)->surface->res.backup;
> +
> + if (!buf)
> + return 0;
> + WARN_ON(dev_priv->active_display_unit != vmw_du_legacy);
> +
> + if (dev_priv->active_display_unit == 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);
> + } else
> + ret = -EINVAL;
> +
> + return ret;
> +}
> +
> +static int vmw_ldu_fb_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);
> +}
> +
>  static int vmw_ldu_del_active(struct vmw_private *vmw_priv,
> struct vmw_legacy_display_unit *ldu)
>  {
> @@ -145,8 +188,7 @@ static int vmw_ldu_del_active(struct vmw_private 
> *vmw_priv,
>   list_del_init(>active);
>   if (--(ld->num_active) == 0) {
>   BUG_ON(!ld->fb);
> - if (ld->fb->unpin)
> - ld->fb->unpin(ld->fb);
> + WARN_ON(vmw_ldu_fb_unpin(ld->fb));
>   ld->fb = NULL;
>   }
>  
> @@ -163,11 +205,10 @@ static int vmw_ldu_add_active(struct vmw_private 
> *vmw_priv,
>  
>   BUG_ON(!ld->num_active && ld->fb);
>   if (vfb != ld->fb) {
> - if (ld->fb && ld->fb->unpin)
> - ld->fb->unpin(ld->fb);
> + if (ld->fb)
> + WARN_ON(vmw_ldu_fb_unpin(ld->fb));
>   vmw_svga_enable(vmw_priv);
> - if (vfb->pin)
> - vfb->pin(vfb);
> + WARN_ON(vmw_ldu_fb_pin(vfb));
>   ld->fb = vfb;
>   }
>  

LGTM!

Reviewed-by: Maaz Mombasawala 
-- 
Maaz Mombasawala (VMware) 



Re: [PATCH v2 3/8] drm/vmwgfx: Rename vmw_buffer_object to vmw_bo

2023-01-31 Thread &quot;Maaz Mombasawala (VMware)
On 1/30/23 19:35, 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 
> Reviewed-by: Martin Krastev 
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c   |  94 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.h   | 189 +++
>  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  | 184 ++
>  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c  |  53 +++---
>  drivers/gpu/drm/vmwgfx/vmwgfx_fence.c|   2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_gem.c  |  27 +--
>  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, 419 insertions(+), 386 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..b6dc0baef350 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,41 @@
>   *
>   **/
>  
> -#include 
> -
> +#include "vmwgfx_bo.h"
>  #include "vmwgfx_drv.h"
> -#include "ttm_object.h"
>  
>  
> -/**
> - * vmw_buffer_object - Convert a struct ttm_buffer_object to a struct
> - * vmw_buffer_object.
> - *
> - * @bo: Pointer to the TTM buffer object.
> - * Return: Pointer to the struct vmw_buffer_object embedding the
> - * TTM buffer object.
> - */
> -static struct vmw_buffer_object *
> -vmw_buffer_object(struct ttm_buffer_object *bo)
> -{
> - return container_of(bo, struct vmw_buffer_object, base);
> -}
> +#include 
>  
>  /**
> - * 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 = to_vmw_bo(>base);
>  
> - 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 +74,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 *buf,
> + struct vmw_bo *buf,
>   struct ttm_placement *placement,
>   

Re: [PATCH v2 2/8] drm/vmwgfx: Remove the duplicate bo_free function

2023-01-31 Thread &quot;Maaz Mombasawala (VMware)
*
> - * @bo: Pointer to the embedded struct ttm_buffer_object
> - */
> -void vmw_gem_destroy(struct ttm_buffer_object *bo)
> -{
> - struct vmw_buffer_object *vbo = vmw_buffer_object(bo);
> -
> - WARN_ON(vbo->dirty);
> - WARN_ON(!RB_EMPTY_ROOT(>res_tree));
> - vmw_bo_unmap(vbo);
> - drm_gem_object_release(>base.base);
> - kfree(vbo);
> -}
> -
>  int vmw_gem_object_create_with_handle(struct vmw_private *dev_priv,
> struct drm_file *filp,
> uint32_t size,
> @@ -153,7 +137,7 @@ int vmw_gem_object_create_with_handle(struct vmw_private 
> *dev_priv,
>   (dev_priv->has_mob) ?
>   _sys_placement :
>   _vram_sys_placement,
> - true, false, _gem_destroy, p_vbo);
> + true, false, p_vbo);
>  
>   (*p_vbo)->base.base.funcs = _gem_object_funcs;
>   if (ret != 0)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index c7d645e5ec7b..5879e8b9950a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -332,8 +332,7 @@ static int vmw_resource_buf_alloc(struct vmw_resource 
> *res,
>  
>   ret = vmw_bo_create(res->dev_priv, res->backup_size,
>   res->func->backup_placement,
> - interruptible, false,
> - _bo_bo_free, );
> + interruptible, false, );
>   if (unlikely(ret != 0))
>   goto out_no_bo;
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index e1f36a09c59c..e51a63c05943 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -445,7 +445,7 @@ vmw_sou_primary_plane_prepare_fb(struct drm_plane *plane,
>   vmw_overlay_pause_all(dev_priv);
>   ret = vmw_bo_create(dev_priv, size,
>   _vram_placement,
> - false, true, _bo_bo_free, >bo);
> + false, true, >bo);
>   vmw_overlay_resume_all(dev_priv);
>   if (ret) {
>   vps->bo = NULL; /* vmw_bo_init frees on error */
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c 
> b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> index 108a496b5d18..93b1400aed4a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> @@ -893,7 +893,7 @@ int vmw_compat_shader_add(struct vmw_private *dev_priv,
>   return -EINVAL;
>  
>   ret = vmw_bo_create(dev_priv, size, _sys_placement,
> - true, true, vmw_bo_bo_free, );
> + true, true, );
>   if (unlikely(ret != 0))
>   goto out;
>  

LGTM!

Reviewed-by: Maaz Mombasawala 

-- 
Maaz Mombasawala (VMware) 



Re: [PATCH v2 1/8] drm/vmwgfx: Use the common gem mmap instead of the custom code

2023-01-31 Thread &quot;Maaz Mombasawala (VMware)
- * 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
> - * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY 
> CLAIM,
> - * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> - * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> - * USE OR OTHER DEALINGS IN THE SOFTWARE.
> - *
> - **/
> -
> -#include "vmwgfx_drv.h"
> -
> -static int vmw_bo_vm_lookup(struct ttm_device *bdev,
> -struct drm_file *filp,
> -unsigned long offset,
> -unsigned long pages,
> -struct ttm_buffer_object **p_bo)
> -{
> - struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, 
> bdev);
> - struct drm_device *drm = _priv->drm;
> - struct drm_vma_offset_node *node;
> - int ret;
> -
> - *p_bo = NULL;
> -
> - drm_vma_offset_lock_lookup(bdev->vma_manager);
> -
> - node = drm_vma_offset_lookup_locked(bdev->vma_manager, offset, pages);
> - if (likely(node)) {
> - *p_bo = container_of(node, struct ttm_buffer_object,
> -   base.vma_node);
> - *p_bo = ttm_bo_get_unless_zero(*p_bo);
> - }
> -
> - drm_vma_offset_unlock_lookup(bdev->vma_manager);
> -
> - if (!*p_bo) {
> - drm_err(drm, "Could not find buffer object to map\n");
> - return -EINVAL;
> - }
> -
> - if (!drm_vma_node_is_allowed(node, filp)) {
> - ret = -EACCES;
> - goto out_no_access;
> - }
> -
> - return 0;
> -out_no_access:
> - ttm_bo_put(*p_bo);
> - return ret;
> -}
> -
> -int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
> -{
> - 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,
> - };
> - struct drm_file *file_priv = filp->private_data;
> - struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
> - struct ttm_device *bdev = _priv->bdev;
> - struct ttm_buffer_object *bo;
> - int ret;
> -
> - if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
> - return -EINVAL;
> -
> - ret = vmw_bo_vm_lookup(bdev, file_priv, vma->vm_pgoff, vma_pages(vma), 
> );
> - if (unlikely(ret != 0))
> - return ret;
> -
> - ret = ttm_bo_mmap_obj(vma, bo);
> - if (unlikely(ret != 0))
> - goto out_unref;
> -
> - vma->vm_ops = _vm_ops;
> -
> - /* Use VM_PFNMAP rather than VM_MIXEDMAP if not a COW mapping */
> - if (!is_cow_mapping(vma->vm_flags))
> - vma->vm_flags = (vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP;
> -
> - ttm_bo_put(bo); /* release extra ref taken by ttm_bo_mmap_obj() */
> -
> - return 0;
> -
> -out_unref:
> - ttm_bo_put(bo);
> - return ret;
> -}
> -

LGTM!

Reviewed-by: Maaz Mombasawala 

-- 
Maaz Mombasawala (VMware) 



Re: [PATCH] drm/vmwgfx: Remove rcu locks from user resources

2022-12-07 Thread &quot;Maaz Mombasawala (VMware)
LGTM.

Reviewed-by: Maaz Mombasawala 

On 12/7/22 09: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 

Re: [PATCH] drm/vmwgfx: Don't use screen objects when SEV is active

2022-12-01 Thread &quot;Maaz Mombasawala (VMware)
On 12/1/22 09: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;
>   }

LGTM

-- 
Maaz Mombasawala (VMware) 



Re: [PATCH v3 17/17] drm/vmwgfx: Fix a sparse warning in kernel docs

2022-10-21 Thread &quot;Maaz Mombasawala (VMware)
On 10/20/22 20: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

-- 

Looks good.

Reviewed-by: Maaz Mombasawala 

Maaz Mombasawala (VMware)