Claude review: drm/vmwgfx: Reserve ttm object before resv usage

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**vmw_bo_free reservation concern:** The added `ttm_bo_reserve`/`ttm_bo_unreserve` pair wraps `vmw_resource_mob_detach` in the buffer object destructor:

```c
+		ttm_bo_reserve(bo, false, false, NULL);
 		vmw_resource_mob_detach(res);
+		ttm_bo_unreserve(bo);
```

This is inside `vmw_bo_free`, which is the TTM buffer object destruction callback. At this point the buffer object refcount is already 0 (confirmed by the `WARN_ON(kref_read(&vbo->tbo.base.refcount) != 0)` at the top of `vmw_bo_free`). Calling `ttm_bo_reserve` on an object with refcount 0 is potentially problematic — `ttm_bo_reserve` calls `__ttm_bo_reserve` which does `ww_mutex_lock` on the reservation object. If another thread is concurrently trying to lock this (e.g., during eviction), the object could be in an inconsistent state. The return value of `ttm_bo_reserve` is also not checked here (the `(void)` cast on `vmw_resource_reserve` above suggests ignoring errors is intentional for this path, but the reserve call itself could fail).

Compare with `vmw_resource_release` (vmwgfx_resource.c:129) which does `ttm_bo_reserve` but has `BUG_ON(ret)` — at least there the BO has a valid refcount.

Consider whether `dma_resv_assert_held` is the right thing to assert in `vmw_resource_mob_detach`, or whether the assertion should be conditionally relaxed for the destructor path.

**vkms CRC worker reservation — reasonable fix:**

```c
+		ret = ttm_bo_reserve(&surf->res.guest_memory_bo->tbo, false, false, NULL);
+		if (ret != 0) {
+			drm_warn(&vmw->drm, "%s: failed reserve\n", __func__);
+			goto done;
+		}
+
 		compute_crc(crtc, surf, &crc32);
+
+		ttm_bo_unreserve(&surf->res.guest_memory_bo->tbo);
```

This is a valid fix since `compute_crc` calls `vmw_bo_map_and_cache`, which likely needs the reservation held. However, looking at the `goto done` path:

```c
+done:
+
 		vmw_surface_unreference(&surf);
```

There's a blank line after the `done:` label — minor style nit. More importantly, when the reservation fails the CRC is silently skipped with only a warning. This is acceptable for a CRC worker (best-effort), but the CRC subsystem will get stale/missing CRC entries for the affected frames.

---
Generated by Claude Code Patch Reviewer