[RFC PATCH 0/6] drm/gma500: prepare GEM and MMU for 3D rendering and hybrid 2D

[RFC PATCH 0/6] drm/gma500: prepare GEM and MMU for 3D rendering and hybrid 2D

[Gabriel Gama]

Hi,

These six patches prepare the existing gma500 GEM/MMU code for future
SGX535 3D and hybrid CPU/GPU 2D. They target Linux 7.4-rc2
(7475ea8b58616b33c04afb976cf0fbaad9dd63fb), enable neither acceleration path
and add no rendering ABI or command submission.

Display GTT and SGX page tables keep their existing, separate roles. The
series fixes sizing, unpin underflow and page-table cache-flush lifetime,
makes MMU insertion transactional and propagates setup/pin failures.
Review of the rollback and ownership assumptions would be appreciated.

Offline validation ran in Debian AArch64 userspace under PRoot: i386 LLVM19
W=1 full kernel link with HIGHMEM4G, plus native/UBSan CPU tests. All nine
MMU KUnit cases passed in a separate i386 QEMU TCG guest. Checkpatch reports
zero errors and one MAINTAINERS advisory on the already-covered test file.
Physical Mini12 testing has NOT been performed; display, suspend/removal
and SGX cache visibility remain unqualified. Historical GPU results do
not qualify this series.

Patch list:
  1. Use the common dumb-buffer size helper.
  2. Prevent GEM unpin underflow.
  3. Fix SGX page-table cache-flush mapping lifetime.
  4. Make MMU insertion checked and transactional.
  5. Propagate MMU setup and GEM pin failures.
  6. Add software-only MMU KUnit coverage.

Thanks,
Gabriel Gama

[RFC PATCH 1/6] drm/gma500: use the common dumb-buffer size helper

[Gabriel Gama]

Use drm_mode_size_dumb() instead of unchecked size_t arithmetic when
calculating dumb-buffer pitch and size. Preserve 64-byte pitch alignment
and page-aligned allocations through the common DRM helper.

Signed-off-by: Gabriel Gama <operatingsystemsdepression@gmail.com>

---
 drivers/gpu/drm/gma500/gem.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 2708e8c68..7250bf660 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -16,6 +16,7 @@
 #include <asm/set_memory.h>
 
 #include <drm/drm.h>
+#include <drm/drm_dumb_buffers.h>
 #include <drm/drm_print.h>
 #include <drm/drm_vma_manager.h>
 
@@ -200,21 +201,16 @@ psb_gem_create(struct drm_device *dev, u64 size, const char *name, bool stolen,
 int psb_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
 			struct drm_mode_create_dumb *args)
 {
-	size_t pitch, size;
 	struct psb_gem_object *pobj;
 	struct drm_gem_object *obj;
 	u32 handle;
 	int ret;
 
-	pitch = args->width * DIV_ROUND_UP(args->bpp, 8);
-	pitch = ALIGN(pitch, 64);
-
-	size = pitch * args->height;
-	size = roundup(size, PAGE_SIZE);
-	if (!size)
-		return -EINVAL;
+	ret = drm_mode_size_dumb(dev, args, 64, 0);
+	if (ret)
+		return ret;
 
-	pobj = psb_gem_create(dev, size, "gem", false, PAGE_SIZE);
+	pobj = psb_gem_create(dev, args->size, "gem", false, PAGE_SIZE);
 	if (IS_ERR(pobj))
 		return PTR_ERR(pobj);
 	obj = &pobj->base;
@@ -225,8 +221,6 @@ int psb_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
 
 	drm_gem_object_put(obj);
 
-	args->pitch = pitch;
-	args->size = size;
 	args->handle = handle;
 
 	return 0;

[RFC PATCH 2/6] drm/gma500: do not underflow an unpinned GEM object

[Gabriel Gama]

Return after warning if psb_gem_unpin() is called without a pin.
Decrementing zero makes in_gart negative, so later pins incorrectly skip
backing-page and mapping setup. Release the reservation lock normally.

Signed-off-by: Gabriel Gama <operatingsystemsdepression@gmail.com>

---
 drivers/gpu/drm/gma500/gem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 7250bf660..fa5d3f137 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -85,7 +85,8 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
 	if (drm_WARN_ONCE(dev, ret, "dma_resv_lock() failed, ret=%d\n", ret))
 		return;
 
-	WARN_ON(!pobj->in_gart);
+	if (WARN_ON(!pobj->in_gart))
+		goto out;
 
 	--pobj->in_gart;
 

[RFC PATCH 3/6] drm/gma500: Fix SGX page-table cache-flush mapping lifetime

[Gabriel Gama]

Flush the modified directory entry before unmapping it. The old order
uses a kmap_atomic pointer outside its mapping lifetime, which DMA32
direct mappings can mask. Use kmap_local_page under the existing spinlock.

Also calculate the CLFLUSH iteration count only on the path that uses it,
avoiding division by an uninitialized cache-line size when CLFLUSH
is absent.

Signed-off-by: Gabriel Gama <operatingsystemsdepression@gmail.com>

---
 drivers/gpu/drm/gma500/mmu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/gma500/mmu.c b/drivers/gpu/drm/gma500/mmu.c
index 4fbc22a59..34ae0f125 100644
--- a/drivers/gpu/drm/gma500/mmu.c
+++ b/drivers/gpu/drm/gma500/mmu.c
@@ -265,7 +265,7 @@ static struct psb_mmu_pt *psb_mmu_alloc_pt(struct psb_mmu_pd *pd)
 	struct psb_mmu_pt *pt = kmalloc_obj(*pt);
 	void *v;
 	uint32_t clflush_add = pd->driver->clflush_add >> PAGE_SHIFT;
-	uint32_t clflush_count = PAGE_SIZE / clflush_add;
+	u32 clflush_count;
 	spinlock_t *lock = &pd->driver->lock;
 	uint8_t *clf;
 	uint32_t *ptes;
@@ -289,6 +289,7 @@ static struct psb_mmu_pt *psb_mmu_alloc_pt(struct psb_mmu_pd *pd)
 		*ptes++ = pd->invalid_pte;
 
 	if (pd->driver->has_clflush && pd->hw_context != -1) {
+		clflush_count = PAGE_SIZE / clflush_add;
 		mb();
 		for (i = 0; i < clflush_count; ++i) {
 			psb_clflush(clf);
@@ -331,16 +332,15 @@ static struct psb_mmu_pt *psb_mmu_pt_alloc_map_lock(struct psb_mmu_pd *pd,
 			continue;
 		}
 
-		v = kmap_atomic(pd->p);
+		v = kmap_local_page(pd->p);
 		pd->tables[index] = pt;
 		v[index] = (page_to_pfn(pt->p) << 12) | pd->pd_mask;
 		pt->index = index;
-		kunmap_atomic((void *) v);
-
 		if (pd->hw_context != -1) {
 			psb_mmu_clflush(pd->driver, (void *)&v[index]);
 			atomic_set(&pd->driver->needs_tlbflush, 1);
 		}
+		kunmap_local(v);
 	}
 	pt->v = kmap_atomic(pt->p);
 	return pt;

[RFC PATCH 4/6] drm/gma500: Make SGX MMU insertion checked and transactional

[Gabriel Gama]

Validate 32-bit mapping extents, PFNs and tiled geometry before changing
PTEs. Serialize changes with the write semaphore and undo only the
inserted prefix on allocation failure or collision, preserving existing
entries. Do not allocate tables or decrement counts when removing holes.

Use 64-bit exclusive endpoints so mapping the last GPU page does not wrap
on i386. Zero-page requests remain no-ops.

Signed-off-by: Gabriel Gama <operatingsystemsdepression@gmail.com>

---
 drivers/gpu/drm/gma500/mmu.c | 211 +++++++++++++++++++++++++----------
 drivers/gpu/drm/gma500/mmu.h |   5 +-
 2 files changed, 158 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/gma500/mmu.c b/drivers/gpu/drm/gma500/mmu.c
index 34ae0f125..0d98c7870 100644
--- a/drivers/gpu/drm/gma500/mmu.c
+++ b/drivers/gpu/drm/gma500/mmu.c
@@ -136,10 +136,9 @@ void psb_mmu_set_pd_context(struct psb_mmu_pd *pd, int hw_context)
 
 }
 
-static inline unsigned long psb_pd_addr_end(unsigned long addr,
-					    unsigned long end)
+static inline u64 psb_pd_addr_end(u64 addr, u64 end)
 {
-	addr = (addr + PSB_PDE_MASK + 1) & ~PSB_PDE_MASK;
+	addr = (addr + PSB_PDE_MASK + 1) & ~(u64)PSB_PDE_MASK;
 	return (addr < end) ? addr : end;
 }
 
@@ -484,13 +483,13 @@ static void psb_mmu_flush_ptes(struct psb_mmu_pd *pd, unsigned long address,
 	struct psb_mmu_pt *pt;
 	uint32_t rows = 1;
 	uint32_t i;
-	unsigned long addr;
-	unsigned long end;
-	unsigned long next;
-	unsigned long add;
-	unsigned long row_add;
+	u64 addr;
+	u64 end;
+	u64 next;
+	u64 add;
+	u64 row_add;
 	unsigned long clflush_add = pd->driver->clflush_add;
-	unsigned long clflush_mask = pd->driver->clflush_mask;
+	u64 clflush_mask = ~((u64)clflush_add - 1);
 
 	if (!pd->driver->has_clflush)
 		return;
@@ -500,8 +499,8 @@ static void psb_mmu_flush_ptes(struct psb_mmu_pd *pd, unsigned long address,
 	else
 		desired_tile_stride = num_pages;
 
-	add = desired_tile_stride << PAGE_SHIFT;
-	row_add = hw_tile_stride << PAGE_SHIFT;
+	add = (u64)desired_tile_stride << PAGE_SHIFT;
+	row_add = (u64)hw_tile_stride << PAGE_SHIFT;
 	mb();
 	for (i = 0; i < rows; ++i) {
 
@@ -525,38 +524,97 @@ static void psb_mmu_flush_ptes(struct psb_mmu_pd *pd, unsigned long address,
 	mb();
 }
 
+/* Validate the complete mapping before touching page tables. */
+static int psb_mmu_check_range(unsigned long address, u32 num_pages,
+			       u32 desired_tile_stride, u32 hw_tile_stride)
+{
+	u64 span;
+	u32 rows;
+
+	if (!IS_ALIGNED(address, PAGE_SIZE))
+		return -EINVAL;
+	if (((u64)address >> 32) || num_pages > BIT(20))
+		return -ERANGE;
+	if (!num_pages)
+		return 0;
+
+	if (hw_tile_stride) {
+		if (!desired_tile_stride ||
+		    num_pages % desired_tile_stride ||
+		    hw_tile_stride < desired_tile_stride)
+			return -EINVAL;
+		rows = num_pages / desired_tile_stride;
+		span = (u64)(rows - 1) * hw_tile_stride + desired_tile_stride;
+	} else {
+		span = num_pages;
+	}
+
+	if (span > (BIT_ULL(32) - address) >> PAGE_SHIFT)
+		return -ERANGE;
+
+	return 0;
+}
+
+/* The caller holds driver->sem for write; only its inserted prefix is undone. */
+static void psb_mmu_rollback_pages(struct psb_mmu_pd *pd,
+				   unsigned long address, u32 inserted,
+				   u32 desired_tile_stride, u32 hw_tile_stride)
+{
+	struct psb_mmu_pt *pt;
+	u64 offset;
+	u32 i;
+
+	for (i = 0; i < inserted; i++) {
+		if (hw_tile_stride)
+			offset = (u64)(i / desired_tile_stride) * hw_tile_stride +
+				 i % desired_tile_stride;
+		else
+			offset = i;
+		pt = psb_mmu_pt_map_lock(pd, address + (offset << PAGE_SHIFT));
+		if (WARN_ON_ONCE(!pt))
+			continue;
+		psb_mmu_invalidate_pte(pt, address + (offset << PAGE_SHIFT));
+		pt->count--;
+		psb_mmu_pt_unmap_unlock(pt);
+	}
+}
+
 void psb_mmu_remove_pfn_sequence(struct psb_mmu_pd *pd,
 				 unsigned long address, uint32_t num_pages)
 {
 	struct psb_mmu_pt *pt;
-	unsigned long addr;
-	unsigned long end;
-	unsigned long next;
+	u64 addr;
+	u64 end;
+	u64 next;
 	unsigned long f_address = address;
 
-	down_read(&pd->driver->sem);
+	if (!num_pages || psb_mmu_check_range(address, num_pages, 0, 0))
+		return;
+
+	down_write(&pd->driver->sem);
 
 	addr = address;
-	end = addr + (num_pages << PAGE_SHIFT);
+	end = addr + ((u64)num_pages << PAGE_SHIFT);
 
 	do {
 		next = psb_pd_addr_end(addr, end);
-		pt = psb_mmu_pt_alloc_map_lock(pd, addr);
+		pt = psb_mmu_pt_map_lock(pd, addr);
 		if (!pt)
-			goto out;
+			continue;
 		do {
-			psb_mmu_invalidate_pte(pt, addr);
-			--pt->count;
+			if (pt->v[psb_mmu_pt_index(addr)] != pd->invalid_pte) {
+				psb_mmu_invalidate_pte(pt, addr);
+				--pt->count;
+			}
 		} while (addr += PAGE_SIZE, addr < next);
 		psb_mmu_pt_unmap_unlock(pt);
 
 	} while (addr = next, next != end);
 
-out:
 	if (pd->hw_context != -1)
 		psb_mmu_flush_ptes(pd, f_address, num_pages, 1, 1);
 
-	up_read(&pd->driver->sem);
+	up_write(&pd->driver->sem);
 
 	if (pd->hw_context != -1)
 		psb_mmu_flush(pd->driver);
@@ -571,22 +629,26 @@ void psb_mmu_remove_pages(struct psb_mmu_pd *pd, unsigned long address,
 	struct psb_mmu_pt *pt;
 	uint32_t rows = 1;
 	uint32_t i;
-	unsigned long addr;
-	unsigned long end;
-	unsigned long next;
-	unsigned long add;
-	unsigned long row_add;
+	u64 addr;
+	u64 end;
+	u64 next;
+	u64 add;
+	u64 row_add;
 	unsigned long f_address = address;
 
+	if (!num_pages || psb_mmu_check_range(address, num_pages,
+					      desired_tile_stride, hw_tile_stride))
+		return;
+
 	if (hw_tile_stride)
 		rows = num_pages / desired_tile_stride;
 	else
 		desired_tile_stride = num_pages;
 
-	add = desired_tile_stride << PAGE_SHIFT;
-	row_add = hw_tile_stride << PAGE_SHIFT;
+	add = (u64)desired_tile_stride << PAGE_SHIFT;
+	row_add = (u64)hw_tile_stride << PAGE_SHIFT;
 
-	down_read(&pd->driver->sem);
+	down_write(&pd->driver->sem);
 
 	/* Make sure we only need to flush this processor's cache */
 
@@ -601,8 +663,10 @@ void psb_mmu_remove_pages(struct psb_mmu_pd *pd, unsigned long address,
 			if (!pt)
 				continue;
 			do {
-				psb_mmu_invalidate_pte(pt, addr);
-				--pt->count;
+				if (pt->v[psb_mmu_pt_index(addr)] != pd->invalid_pte) {
+					psb_mmu_invalidate_pte(pt, addr);
+					--pt->count;
+				}
 
 			} while (addr += PAGE_SIZE, addr < next);
 			psb_mmu_pt_unmap_unlock(pt);
@@ -614,7 +678,7 @@ void psb_mmu_remove_pages(struct psb_mmu_pd *pd, unsigned long address,
 		psb_mmu_flush_ptes(pd, f_address, num_pages,
 				   desired_tile_stride, hw_tile_stride);
 
-	up_read(&pd->driver->sem);
+	up_write(&pd->driver->sem);
 
 	if (pd->hw_context != -1)
 		psb_mmu_flush(pd->driver);
@@ -626,16 +690,23 @@ int psb_mmu_insert_pfn_sequence(struct psb_mmu_pd *pd, uint32_t start_pfn,
 {
 	struct psb_mmu_pt *pt;
 	uint32_t pte;
-	unsigned long addr;
-	unsigned long end;
-	unsigned long next;
+	u64 addr;
+	u64 end;
+	u64 next;
 	unsigned long f_address = address;
-	int ret = -ENOMEM;
+	u32 inserted = 0;
+	int ret;
+
+	ret = psb_mmu_check_range(address, num_pages, 0, 0);
+	if (ret || !num_pages)
+		return ret;
+	if ((u64)start_pfn + num_pages > BIT_ULL(20))
+		return -ERANGE;
 
-	down_read(&pd->driver->sem);
+	down_write(&pd->driver->sem);
 
 	addr = address;
-	end = addr + (num_pages << PAGE_SHIFT);
+	end = addr + ((u64)num_pages << PAGE_SHIFT);
 
 	do {
 		next = psb_pd_addr_end(addr, end);
@@ -645,9 +716,15 @@ int psb_mmu_insert_pfn_sequence(struct psb_mmu_pd *pd, uint32_t start_pfn,
 			goto out;
 		}
 		do {
+			if (pt->v[psb_mmu_pt_index(addr)] != pd->invalid_pte) {
+				psb_mmu_pt_unmap_unlock(pt);
+				ret = -EBUSY;
+				goto out;
+			}
 			pte = psb_mmu_mask_pte(start_pfn++, type);
 			psb_mmu_set_pte(pt, addr, pte);
 			pt->count++;
+			inserted++;
 		} while (addr += PAGE_SIZE, addr < next);
 		psb_mmu_pt_unmap_unlock(pt);
 
@@ -655,10 +732,12 @@ int psb_mmu_insert_pfn_sequence(struct psb_mmu_pd *pd, uint32_t start_pfn,
 	ret = 0;
 
 out:
+	if (ret)
+		psb_mmu_rollback_pages(pd, f_address, inserted, 0, 0);
 	if (pd->hw_context != -1)
 		psb_mmu_flush_ptes(pd, f_address, num_pages, 1, 1);
 
-	up_read(&pd->driver->sem);
+	up_write(&pd->driver->sem);
 
 	if (pd->hw_context != -1)
 		psb_mmu_flush(pd->driver);
@@ -675,26 +754,35 @@ int psb_mmu_insert_pages(struct psb_mmu_pd *pd, struct page **pages,
 	uint32_t rows = 1;
 	uint32_t i;
 	uint32_t pte;
-	unsigned long addr;
-	unsigned long end;
-	unsigned long next;
-	unsigned long add;
-	unsigned long row_add;
+	u64 addr;
+	u64 end;
+	u64 next;
+	u64 add;
+	u64 row_add;
 	unsigned long f_address = address;
-	int ret = -ENOMEM;
+	u32 inserted = 0;
+	int ret;
+
+	ret = psb_mmu_check_range(address, num_pages,
+				  desired_tile_stride, hw_tile_stride);
+	if (ret || !num_pages)
+		return ret;
+	if (!pages)
+		return -EINVAL;
+	for (i = 0; i < num_pages; i++) {
+		if (!pages[i] || page_to_pfn(pages[i]) >= BIT_ULL(20))
+			return -ERANGE;
+	}
 
-	if (hw_tile_stride) {
-		if (num_pages % desired_tile_stride != 0)
-			return -EINVAL;
+	if (hw_tile_stride)
 		rows = num_pages / desired_tile_stride;
-	} else {
+	else
 		desired_tile_stride = num_pages;
-	}
 
-	add = desired_tile_stride << PAGE_SHIFT;
-	row_add = hw_tile_stride << PAGE_SHIFT;
+	add = (u64)desired_tile_stride << PAGE_SHIFT;
+	row_add = (u64)hw_tile_stride << PAGE_SHIFT;
 
-	down_read(&pd->driver->sem);
+	down_write(&pd->driver->sem);
 
 	for (i = 0; i < rows; ++i) {
 
@@ -704,13 +792,21 @@ int psb_mmu_insert_pages(struct psb_mmu_pd *pd, struct page **pages,
 		do {
 			next = psb_pd_addr_end(addr, end);
 			pt = psb_mmu_pt_alloc_map_lock(pd, addr);
-			if (!pt)
+			if (!pt) {
+				ret = -ENOMEM;
 				goto out;
+			}
 			do {
+				if (pt->v[psb_mmu_pt_index(addr)] != pd->invalid_pte) {
+					psb_mmu_pt_unmap_unlock(pt);
+					ret = -EBUSY;
+					goto out;
+				}
 				pte = psb_mmu_mask_pte(page_to_pfn(*pages++),
 						       type);
 				psb_mmu_set_pte(pt, addr, pte);
 				pt->count++;
+				inserted++;
 			} while (addr += PAGE_SIZE, addr < next);
 			psb_mmu_pt_unmap_unlock(pt);
 
@@ -721,11 +817,14 @@ int psb_mmu_insert_pages(struct psb_mmu_pd *pd, struct page **pages,
 
 	ret = 0;
 out:
+	if (ret)
+		psb_mmu_rollback_pages(pd, f_address, inserted, desired_tile_stride,
+				       hw_tile_stride);
 	if (pd->hw_context != -1)
 		psb_mmu_flush_ptes(pd, f_address, num_pages,
 				   desired_tile_stride, hw_tile_stride);
 
-	up_read(&pd->driver->sem);
+	up_write(&pd->driver->sem);
 
 	if (pd->hw_context != -1)
 		psb_mmu_flush(pd->driver);
diff --git a/drivers/gpu/drm/gma500/mmu.h b/drivers/gpu/drm/gma500/mmu.h
index e6d397037..4474718aa 100644
--- a/drivers/gpu/drm/gma500/mmu.h
+++ b/drivers/gpu/drm/gma500/mmu.h
@@ -9,8 +9,9 @@
 #define __MMU_H
 
 struct psb_mmu_driver {
-	/* protects driver- and pd structures. Always take in read mode
-	 * before taking the page table spinlock.
+	/*
+	 * Protects driver and page-directory structures. Mapping changes take
+	 * this semaphore for write, before the page-table spinlock.
 	 */
 	struct rw_semaphore sem;
 

[RFC PATCH 5/6] drm/gma500: Propagate SGX MMU setup and GEM pin failures

[Gabriel Gama]

Check the GEM GPU address and perform transactional MMU insertion before
changing cache attributes or publishing the GTT mapping. On failure,
release temporary pages without publishing a pin and return the error.

Propagate stolen-memory mapping failures through probe cleanup before
installing a hardware context. Existing cache-attribute error handling
is unchanged. This depends on the preceding rollback fix.

Signed-off-by: Gabriel Gama <operatingsystemsdepression@gmail.com>

---
 drivers/gpu/drm/gma500/gem.c     | 19 ++++++++++++++-----
 drivers/gpu/drm/gma500/psb_drv.c |  2 ++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index fa5d3f137..c79713e26 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -32,7 +32,7 @@ int psb_gem_pin(struct psb_gem_object *pobj)
 	struct drm_gem_object *obj = &pobj->base;
 	struct drm_device *dev = obj->dev;
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	u32 gpu_base = dev_priv->gtt.gatt_start;
+	u64 gpu_address = (u64)dev_priv->gtt.gatt_start + pobj->offset;
 	struct page **pages;
 	unsigned int npages;
 	int ret;
@@ -44,6 +44,11 @@ int psb_gem_pin(struct psb_gem_object *pobj)
 	if (pobj->in_gart || pobj->stolen)
 		goto out; /* already mapped */
 
+	if (gpu_address >= BIT_ULL(32)) {
+		ret = -ERANGE;
+		goto err_dma_resv_unlock;
+	}
+
 	pages = drm_gem_get_pages(obj);
 	if (IS_ERR(pages)) {
 		ret = PTR_ERR(pages);
@@ -52,12 +57,14 @@ int psb_gem_pin(struct psb_gem_object *pobj)
 
 	npages = obj->size / PAGE_SIZE;
 
-	set_pages_array_wc(pages, npages);
+	ret = psb_mmu_insert_pages(psb_mmu_get_default_pd(dev_priv->mmu),
+				   pages, gpu_address, npages, 0, 0,
+				   PSB_MMU_CACHED_MEMORY);
+	if (ret)
+		goto err_put_pages;
 
+	set_pages_array_wc(pages, npages);
 	psb_gtt_insert_pages(dev_priv, &pobj->resource, pages);
-	psb_mmu_insert_pages(psb_mmu_get_default_pd(dev_priv->mmu), pages,
-			     (gpu_base + pobj->offset), npages, 0, 0,
-			     PSB_MMU_CACHED_MEMORY);
 
 	pobj->pages = pages;
 
@@ -67,6 +74,8 @@ int psb_gem_pin(struct psb_gem_object *pobj)
 
 	return 0;
 
+err_put_pages:
+	drm_gem_put_pages(obj, pages, false, false);
 err_dma_resv_unlock:
 	dma_resv_unlock(obj->resv);
 	return ret;
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index d17cb5b4a..d942f7264 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -354,6 +354,8 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 					  dev_priv->stolen_base >> PAGE_SHIFT,
 					  pg->gatt_start,
 					  pg->stolen_size >> PAGE_SHIFT, 0);
+	if (ret)
+		goto out_err;
 
 	psb_mmu_set_pd_context(psb_mmu_get_default_pd(dev_priv->mmu), 0);
 	psb_mmu_set_pd_context(dev_priv->pf_pd, 1);

[RFC PATCH 6/6] drm/gma500: Add software-only KUnit coverage for SGX page tables

[Gabriel Gama]

Add KUnit cases for invalid ranges and tiles, the final GPU page,
directory crossings, PFN limits, collision rollback and removal holes.

The optional built-in fixture uses a software page directory with no DRM
device and hw_context=-1. It performs no MMIO or GPU submission.

Signed-off-by: Gabriel Gama <operatingsystemsdepression@gmail.com>

---
 drivers/gpu/drm/gma500/Kconfig    |  10 ++
 drivers/gpu/drm/gma500/Makefile   |   2 +
 drivers/gpu/drm/gma500/mmu_test.c | 222 ++++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 drivers/gpu/drm/gma500/mmu_test.c

diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
index dc4c56638..b6e2dda13 100644
--- a/drivers/gpu/drm/gma500/Kconfig
+++ b/drivers/gpu/drm/gma500/Kconfig
@@ -16,3 +16,13 @@ config DRM_GMA500
 	  Say yes for an experimental 2D KMS framebuffer driver for the
 	  Intel GMA500 (Poulsbo), Intel GMA600 (Moorestown/Oak Trail) and
 	  Intel GMA3600/3650 (Cedar Trail).
+
+config DRM_GMA500_KUNIT_TEST
+	bool "KUnit tests for the GMA500 SGX page tables" if !KUNIT_ALL_TESTS
+	depends on DRM_GMA500=y && KUNIT=y
+	default KUNIT_ALL_TESTS
+	help
+	  Run software-only tests for SGX page-table range validation,
+	  rollback, mapping collisions and removal. The tests use a software
+	  page directory with no device or hardware context, and never
+	  submit commands to the GPU or access its registers.
diff --git a/drivers/gpu/drm/gma500/Makefile b/drivers/gpu/drm/gma500/Makefile
index 58fed80c7..069400d70 100644
--- a/drivers/gpu/drm/gma500/Makefile
+++ b/drivers/gpu/drm/gma500/Makefile
@@ -39,4 +39,6 @@ gma500_gfx-y += \
 gma500_gfx-$(CONFIG_ACPI) +=  opregion.o
 gma500_gfx-$(CONFIG_DRM_FBDEV_EMULATION) +=  fbdev.o
 
+gma500_gfx-$(CONFIG_DRM_GMA500_KUNIT_TEST) += mmu_test.o
+
 obj-$(CONFIG_DRM_GMA500) += gma500_gfx.o
diff --git a/drivers/gpu/drm/gma500/mmu_test.c b/drivers/gpu/drm/gma500/mmu_test.c
new file mode 100644
index 000000000..0581c7d9a
--- /dev/null
+++ b/drivers/gpu/drm/gma500/mmu_test.c
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* SGX page-table tests. No device, hardware context, or MMIO is required. */
+#include <kunit/test.h>
+#include <linux/highmem.h>
+
+#include "mmu.h"
+#include "psb_drv.h"
+
+struct psb_mmu_test_context {
+	struct psb_mmu_driver driver;
+	struct psb_mmu_pd *pd;
+	struct page *pages[4];
+};
+
+static int psb_mmu_test_init(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx;
+	int i;
+
+	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	test->priv = ctx;
+	spin_lock_init(&ctx->driver.lock);
+	init_rwsem(&ctx->driver.sem);
+	ctx->pd = psb_mmu_alloc_pd(&ctx->driver, 1, 0);
+	if (!ctx->pd)
+		return -ENOMEM;
+	for (i = 0; i < ARRAY_SIZE(ctx->pages); i++) {
+		ctx->pages[i] = alloc_page(GFP_DMA32);
+		if (!ctx->pages[i]) {
+			while (i--)
+				__free_page(ctx->pages[i]);
+			psb_mmu_free_pagedir(ctx->pd);
+			return -ENOMEM;
+		}
+	}
+	return 0;
+}
+
+static void psb_mmu_test_exit(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->pages); i++)
+		__free_page(ctx->pages[i]);
+	psb_mmu_free_pagedir(ctx->pd);
+}
+
+static u32 psb_mmu_test_pte(struct psb_mmu_pd *pd, u32 address)
+{
+	struct psb_mmu_pt *pt = pd->tables[address >> PSB_PDE_SHIFT];
+	u32 *entries;
+	u32 value;
+
+	if (!pt)
+		return pd->invalid_pte;
+	entries = kmap_local_page(pt->p);
+	value = entries[(address >> PAGE_SHIFT) & 0x3ff];
+	kunmap_local(entries);
+	return value;
+}
+
+static void psb_mmu_zero_pages_test(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pages(ctx->pd, NULL, 0, 0,
+						   0, 0, 0), 0);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pfn_sequence(ctx->pd, 0,
+							  0, 0, 0), 0);
+	psb_mmu_remove_pages(ctx->pd, 0, 0, 0, 0);
+	psb_mmu_remove_pfn_sequence(ctx->pd, 0, 0);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->pd->tables[0], NULL);
+}
+
+static void psb_mmu_range_test(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   1, 1, 0, 0, 0), -EINVAL);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   0xfffff000UL, 2, 0, 0, 0),
+			-ERANGE);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   0, 4, 0, 4, 0), -EINVAL);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   0, 4, 3, 4, 0), -EINVAL);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   0, 4, 2, 1, 0), -EINVAL);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   0, 4, 2, U32_MAX, 0), -ERANGE);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pages(ctx->pd, NULL,
+						   0, 1, 0, 0, 0), -EINVAL);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->pd->tables[0], NULL);
+}
+
+static void psb_mmu_last_page_test(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+
+	KUNIT_ASSERT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   0xfffff000UL, 1, 0, 0, 0), 0);
+	KUNIT_EXPECT_EQ(test, psb_mmu_test_pte(ctx->pd, 0xfffff000),
+			(u32)(page_to_pfn(ctx->pages[0]) << PAGE_SHIFT) |
+			PSB_PTE_VALID);
+	psb_mmu_remove_pages(ctx->pd, 0xfffff000UL, 1, 0, 0);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->pd->tables[1023], NULL);
+}
+
+static void psb_mmu_directory_boundary_test(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+
+	KUNIT_ASSERT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   0x3ff000, 2, 0, 0, 0), 0);
+	KUNIT_EXPECT_EQ(test, ctx->pd->tables[0]->count, 1U);
+	KUNIT_EXPECT_EQ(test, ctx->pd->tables[1]->count, 1U);
+	psb_mmu_remove_pages(ctx->pd, 0x3ff000, 2, 0, 0);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->pd->tables[0], NULL);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->pd->tables[1], NULL);
+}
+
+static void psb_mmu_collision_rollback_test(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+	u32 retained;
+
+	KUNIT_ASSERT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages + 3,
+						   0x400000, 1, 0, 0, 0), 0);
+	retained = psb_mmu_test_pte(ctx->pd, 0x400000);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   0x3ff000, 2, 0, 0, 0), -EBUSY);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->pd->tables[0], NULL);
+	KUNIT_EXPECT_EQ(test, psb_mmu_test_pte(ctx->pd, 0x400000), retained);
+	KUNIT_EXPECT_EQ(test, ctx->pd->tables[1]->count, 1U);
+}
+
+static void psb_mmu_tiled_rollback_test(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+	u32 retained;
+
+	KUNIT_ASSERT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages + 3,
+						   0x8000, 1, 0, 0, 0), 0);
+	retained = psb_mmu_test_pte(ctx->pd, 0x8000);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   0x3000, 4, 2, 4, 0), -EBUSY);
+	KUNIT_EXPECT_EQ(test, psb_mmu_test_pte(ctx->pd, 0x3000), 0U);
+	KUNIT_EXPECT_EQ(test, psb_mmu_test_pte(ctx->pd, 0x4000), 0U);
+	KUNIT_EXPECT_EQ(test, psb_mmu_test_pte(ctx->pd, 0x7000), 0U);
+	KUNIT_EXPECT_EQ(test, psb_mmu_test_pte(ctx->pd, 0x8000), retained);
+	KUNIT_EXPECT_EQ(test, ctx->pd->tables[0]->count, 1U);
+}
+
+static void psb_mmu_pfn_range_test(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pfn_sequence(ctx->pd, 0x100000,
+							  0, 1, 0), -ERANGE);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pfn_sequence(ctx->pd, 0xfffff,
+							  0, 2, 0), -ERANGE);
+	KUNIT_ASSERT_EQ(test, psb_mmu_insert_pfn_sequence(ctx->pd, 0xfffff,
+							  0, 1, 0), 0);
+	KUNIT_EXPECT_EQ(test, psb_mmu_test_pte(ctx->pd, 0), 0xfffff001U);
+	psb_mmu_remove_pfn_sequence(ctx->pd, 0, 1);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->pd->tables[0], NULL);
+}
+
+static void psb_mmu_pfn_rollback_test(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+	u32 retained;
+
+	KUNIT_ASSERT_EQ(test, psb_mmu_insert_pfn_sequence(ctx->pd, 0x100,
+							  0x400000, 1, 0), 0);
+	retained = psb_mmu_test_pte(ctx->pd, 0x400000);
+	KUNIT_EXPECT_EQ(test, psb_mmu_insert_pfn_sequence(ctx->pd, 0x200,
+							  0x3ff000, 2, 0), -EBUSY);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->pd->tables[0], NULL);
+	KUNIT_EXPECT_EQ(test, psb_mmu_test_pte(ctx->pd, 0x400000), retained);
+	KUNIT_EXPECT_EQ(test, ctx->pd->tables[1]->count, 1U);
+}
+
+static void psb_mmu_remove_hole_test(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+
+	KUNIT_ASSERT_EQ(test, psb_mmu_insert_pages(ctx->pd, ctx->pages,
+						   0x1000, 1, 0, 0, 0), 0);
+	psb_mmu_remove_pages(ctx->pd, 0x2000, 1, 0, 0);
+	KUNIT_EXPECT_EQ(test, ctx->pd->tables[0]->count, 1U);
+	psb_mmu_remove_pages(ctx->pd, 0x1000, 1, 0, 0);
+	psb_mmu_remove_pfn_sequence(ctx->pd, 0x400000, 1);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->pd->tables[0], NULL);
+	KUNIT_EXPECT_PTR_EQ(test, ctx->pd->tables[1], NULL);
+}
+
+static struct kunit_case psb_mmu_test_cases[] = {
+	KUNIT_CASE(psb_mmu_zero_pages_test),
+	KUNIT_CASE(psb_mmu_range_test),
+	KUNIT_CASE(psb_mmu_last_page_test),
+	KUNIT_CASE(psb_mmu_directory_boundary_test),
+	KUNIT_CASE(psb_mmu_collision_rollback_test),
+	KUNIT_CASE(psb_mmu_tiled_rollback_test),
+	KUNIT_CASE(psb_mmu_pfn_range_test),
+	KUNIT_CASE(psb_mmu_pfn_rollback_test),
+	KUNIT_CASE(psb_mmu_remove_hole_test),
+	{}
+};
+
+static struct kunit_suite psb_mmu_test_suite = {
+	.name = "psb_mmu",
+	.init = psb_mmu_test_init,
+	.exit = psb_mmu_test_exit,
+	.test_cases = psb_mmu_test_cases,
+};
+
+kunit_test_suite(psb_mmu_test_suite);

Re: [RFC PATCH 3/6] drm/gma500: Fix SGX page-table cache-flush mapping lifetime

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] drm/gma500: missing Fixes: trailer for SGX MMU cache-flush fixes
--

commit c92354a62278204d941ae70ac0f3b2869cb66190
Author: Gabriel Gama <operatingsystemsdepression@gmail.com>

> drm/gma500: Fix SGX page-table cache-flush mapping lifetime
>
> Flush the modified directory entry before unmapping it. The old order
> uses a kmap_atomic pointer outside its mapping lifetime, which DMA32
> direct mappings can mask. Use kmap_local_page under the existing spinlock.
>
> Also calculate the CLFLUSH iteration count only on the path that uses it,
> avoiding division by an uninitialized cache-line size when CLFLUSH
> is absent.
>
> Signed-off-by: Gabriel Gama <operatingsystemsdepression@gmail.com>

[Severity: Low]
Both the post-unmap cache flush in psb_mmu_pt_alloc_map_lock() and the
unconditional division by clflush_add in psb_mmu_alloc_pt() were
introduced in 8c8f1c958ab5 ("gma500: introduce the GTT and MMU handling
logic").  Should this include a Fixes: tag?

  Fixes: 8c8f1c958ab5 ("gma500: introduce the GTT and MMU handling logic")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109055451.17458-1-operatingsystemsdepression@gmail.com?part=3

Re: [RFC PATCH 5/6] drm/gma500: Propagate SGX MMU setup and GEM pin failures

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] drm/gma500: missing Fixes trailer for MMU error checks
--

commit 781534e30915fea9a7d73d13c97f11a95323ad77
Author: Gabriel Gama <operatingsystemsdepression@gmail.com>

> drm/gma500: Propagate SGX MMU setup and GEM pin failures
>
> Check the GEM GPU address and perform transactional MMU insertion before
> changing cache attributes or publishing the GTT mapping. On failure,
> release temporary pages without publishing a pin and return the error.
>
> Propagate stolen-memory mapping failures through probe cleanup before
> installing a hardware context. Existing cache-attribute error handling
> is unchanged. This depends on the preceding rollback fix.
>
> Signed-off-by: Gabriel Gama <operatingsystemsdepression@gmail.com>

[Severity: Low]
Since this fixes the unchecked psb_mmu_insert_pages() and
psb_mmu_insert_pfn_sequence() calls, should the commit message include a
Fixes tag?

  Fixes: ae012bdc5799 ("drm/gma500: Hook up the MMU")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109055451.17458-1-operatingsystemsdepression@gmail.com?part=5

Re: [RFC PATCH 6/6] drm/gma500: Add software-only KUnit coverage for SGX page tables

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/gma500: NULL deref and double free in psb_mmu_test_exit()
  on init error
--

commit 0ae7cbd175ba45e2df744da8590157d933a675ae
Author: Gabriel Gama <operatingsystemsdepression@gmail.com>

drm/gma500: Add software-only KUnit coverage for SGX page tables

This commit adds a built-in KUnit test suite for GMA500 SGX page-table
operations using a software page directory without hardware access.

> diff --git a/drivers/gpu/drm/gma500/mmu_test.c b/drivers/gpu/drm/gma500/mmu_test.c
> new file mode 100644
> index 0000000000000..0581c7d9ac4ac
> --- /dev/null
> +++ b/drivers/gpu/drm/gma500/mmu_test.c
> @@ -0,0 +1,222 @@
[ ... ]
> +static int psb_mmu_test_init(struct kunit *test)
> +{
> +	struct psb_mmu_test_context *ctx;
> +	int i;
> +
> +	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +	test->priv = ctx;
> +	spin_lock_init(&ctx->driver.lock);
> +	init_rwsem(&ctx->driver.sem);
> +	ctx->pd = psb_mmu_alloc_pd(&ctx->driver, 1, 0);
> +	if (!ctx->pd)
> +		return -ENOMEM;
> +	for (i = 0; i < ARRAY_SIZE(ctx->pages); i++) {
> +		ctx->pages[i] = alloc_page(GFP_DMA32);
> +		if (!ctx->pages[i]) {
> +			while (i--)
> +				__free_page(ctx->pages[i]);
> +			psb_mmu_free_pagedir(ctx->pd);
> +			return -ENOMEM;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static void psb_mmu_test_exit(struct kunit *test)
> +{
> +	struct psb_mmu_test_context *ctx = test->priv;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->pages); i++)
> +		__free_page(ctx->pages[i]);
> +	psb_mmu_free_pagedir(ctx->pd);
> +}

[Severity: Medium]
Can psb_mmu_test_exit() cause a NULL pointer dereference or double free
when psb_mmu_test_init() fails?

KUnit runs suite->exit(test) via kunit_run_case_cleanup() even when
suite->init(test) returns a non-zero error.  If kunit_kzalloc() or
psb_mmu_alloc_pd() fails, psb_mmu_test_exit() dereferences a NULL ctx or
ctx->pd; if alloc_page() fails, psb_mmu_test_init() frees ctx->pages[] and
ctx->pd without clearing the pointers, and psb_mmu_test_exit() frees them
again while passing NULL for the unallocated pages.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109055451.17458-1-operatingsystemsdepression@gmail.com?part=6

Re: [RFC PATCH 6/6] drm/gma500: Add software-only KUnit coverage for SGX page tables

[Operating System's Depression]

[-- Attachment #1: Type: text/plain, Size: 34 bytes --]

fixing,  sending v2 in ˜50 mins

[-- Attachment #2: Type: text/html, Size: 59 bytes --]