[PATCH] drm/ttm/tests: add a test to exercise ttm_bo_swapout

[PATCH] drm/ttm/tests: add a test to exercise ttm_bo_swapout

[Thadeu Lima de Souza Cascardo]

Currently, ttm_bo_swapout is not exercised by the TTM KUnit tests.

It used to be exercised until commit 76689eb52667 ("drm/ttm: remove
ttm_bo_validate_swapout test"), but that test was removed as it was
unreliable due to requiring to allocate half of the system memory.

Calling ttm_bo_swapout directly with a single allocated BO, however, does
not suffer from that problem, and was able to detect a UAF introduced by
commit c06da4b3573a ("drm/ttm: Tidy usage of local variables a little
bit"), when built with KASAN.

When applying a fix to that UAF, the test passed without any issues.

Cc: Karolina Stolarek <karolina.stolarek@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
 drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c | 42 ++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
index 6d95447a989d20d60227025be874265b2b491f59..9848c008a443d70ad16c6e018381878f0898e487 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
@@ -759,6 +759,47 @@ static void ttm_bo_validate_move_fence_not_signaled(struct kunit *test)
 	ttm_mock_manager_fini(priv->ttm_dev, snd_mem);
 }
 
+static void ttm_bo_validate_swapout(struct kunit *test)
+{
+	u32 mem_type = TTM_PL_TT;
+	struct ttm_test_devices *priv = test->priv;
+	struct ttm_operation_ctx ctx_init = { };
+	enum ttm_bo_type bo_type = ttm_bo_type_device;
+	struct ttm_resource_manager *man;
+	struct ttm_placement *placement;
+	struct ttm_buffer_object *bo;
+	struct ttm_place *place;
+	int err;
+	s64 swapped;
+
+	ttm_mock_manager_init(priv->ttm_dev, mem_type, MANAGER_SIZE);
+	man = ttm_manager_type(priv->ttm_dev, mem_type);
+	KUNIT_ASSERT_NOT_NULL(test, man);
+
+	place = ttm_place_kunit_init(test, mem_type, 0);
+	KUNIT_ASSERT_NOT_NULL(test, place);
+	placement = ttm_placement_kunit_init(test, place, 1);
+	KUNIT_ASSERT_NOT_NULL(test, placement);
+
+	bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, bo);
+
+	drm_gem_private_object_init(priv->drm, &bo->base, MANAGER_SIZE);
+	err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement,
+				   PAGE_SIZE, &ctx_init, NULL, NULL,
+				   &dummy_ttm_bo_destroy);
+	KUNIT_EXPECT_EQ(test, err, 0);
+	dma_resv_unlock(bo->base.resv);
+
+	swapped = ttm_bo_swapout(priv->ttm_dev, &ctx_init, man, GFP_KERNEL, 1);
+	KUNIT_EXPECT_EQ(test, swapped, MANAGER_SIZE / PAGE_SIZE);
+	KUNIT_EXPECT_EQ(test, bo->resource->mem_type, TTM_PL_SYSTEM);
+	KUNIT_EXPECT_TRUE(test, bo->ttm->page_flags & TTM_TT_FLAG_SWAPPED);
+
+	ttm_bo_fini(bo);
+	ttm_mock_manager_fini(priv->ttm_dev, mem_type);
+}
+
 static void ttm_bo_validate_happy_evict(struct kunit *test)
 {
 	u32 mem_type = TTM_PL_VRAM, mem_multihop = TTM_PL_TT,
@@ -1153,6 +1194,7 @@ static struct kunit_case ttm_bo_validate_test_cases[] = {
 	KUNIT_CASE(ttm_bo_validate_move_fence_signaled),
 	KUNIT_CASE_PARAM(ttm_bo_validate_move_fence_not_signaled,
 			 ttm_bo_validate_wait_gen_params),
+	KUNIT_CASE(ttm_bo_validate_swapout),
 	KUNIT_CASE(ttm_bo_validate_happy_evict),
 	KUNIT_CASE(ttm_bo_validate_all_pinned_evict),
 	KUNIT_CASE(ttm_bo_validate_allowed_only_evict),

---
base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
change-id: 20260306-ttm_bo_swapout_test-ad130eac0bcf

Best regards,
-- 
Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

Claude review: drm/ttm/tests: add a test to exercise ttm_bo_swapout

[Claude Code Review Bot]

Overall Series Review

Subject: drm/ttm/tests: add a test to exercise ttm_bo_swapout
Author: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
Patches: 1
Reviewed: 2026-03-09T08:23:09.150307

---

This is a single-patch series that adds a KUnit test for `ttm_bo_swapout`. The test is well-motivated: the commit message explains that a previous swapout test was removed (commit 76689eb52667) because it was unreliable, and this new test takes a simpler, more direct approach by calling `ttm_bo_swapout` on a single BO rather than trying to trigger it indirectly through memory pressure. The test was validated by detecting a real UAF bug. The patch is clean and follows the existing test patterns in the file.

**Verdict: Looks good overall, with a couple of minor observations.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/ttm/tests: add a test to exercise ttm_bo_swapout

[Claude Code Review Bot]

Patch Review

**Positive aspects:**
- Well-written commit message with clear motivation and historical context.
- The test follows the established patterns in `ttm_bo_validate_test.c` (same allocation style, same cleanup pattern with `ttm_bo_fini`/`ttm_mock_manager_fini`).
- Good validation: checks the return value, the resulting mem_type, and the swapped flag.

**Minor observations:**

1. **`KUNIT_EXPECT_EQ` vs `KUNIT_ASSERT_EQ` for `ttm_bo_init_reserved` failure** (line 791):
   ```c
   KUNIT_EXPECT_EQ(test, err, 0);
   ```
   If `ttm_bo_init_reserved` fails, the test continues and will likely crash on the subsequent `dma_resv_unlock` and `ttm_bo_swapout` calls with an uninitialized BO. Using `KUNIT_ASSERT_EQ` would abort the test on failure, preventing cascading issues. That said, the existing tests in this file all use `KUNIT_EXPECT_EQ` for this same check (e.g., lines 129, 178, 220, 252), so this is consistent with the existing code style. It could be argued the existing tests all have this same weakness, but that's a separate cleanup.

2. **Potential NULL dereference in assertions after swapout** (lines 796-797):
   ```c
   KUNIT_EXPECT_EQ(test, bo->resource->mem_type, TTM_PL_SYSTEM);
   KUNIT_EXPECT_TRUE(test, bo->ttm->page_flags & TTM_TT_FLAG_SWAPPED);
   ```
   If `ttm_bo_swapout` fails or doesn't behave as expected, `bo->resource` or `bo->ttm` could potentially be NULL. A defensive check or using `KUNIT_ASSERT_NOT_NULL` before dereferencing would be safer. However, in practice, a failed swapout would likely leave these pointers valid (just not in the expected state), so this is a minor robustness concern rather than a real bug.

3. **Checking swapped return value** (line 794-795):
   ```c
   swapped = ttm_bo_swapout(priv->ttm_dev, &ctx_init, man, GFP_KERNEL, 1);
   KUNIT_EXPECT_EQ(test, swapped, MANAGER_SIZE / PAGE_SIZE);
   ```
   The `MANAGER_SIZE / PAGE_SIZE` expectation is consistent with having initialized the GEM object with size `MANAGER_SIZE` at line 787. This looks correct.

4. **Cleanup ordering**: The test calls `ttm_bo_fini(bo)` before `ttm_mock_manager_fini(priv->ttm_dev, mem_type)`, which is the correct order (destroy BO before tearing down the manager). This matches the pattern in other tests like `ttm_bo_validate_move_fence_not_signaled` at lines 757-759.

**Overall:** The patch is straightforward, well-structured, and follows the existing conventions. The observations above are all minor. The test provides useful coverage for `ttm_bo_swapout` that was previously missing.

**Reviewed-by worthy** with no blocking issues.

---
Generated by Claude Code Patch Reviewer