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

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

[Gabriel Gama]

Hi,

This v2 targets Linux 7.4-rc2 at
7475ea8b58616b33c04afb976cf0fbaad9dd63fb. The six patches improve existing
gma500 GEM and SGX MMU handling as prerequisites for future 3D rendering
and hybrid CPU/GPU 2D. They do not enable either rendering path or add a
userspace ABI. Display GTT and SGX page tables remain separate mappings.

Since v1, I verified the introducing commits for patches 3 and 5 and added
Fixes tags. Patch 6 now handles initialization failure correctly: KUnit
still invokes the per-case exit callback, so partial resources are released
once through a NULL-safe cleanup rather than freed again through stale
pointers. The successful fixture path is unchanged.

Cross-builds and native/UBSan checks ran in Debian AArch64 Linux userspace
under PRoot. The i386 LLVM 19 W=1/HIGHMEM4G kernel linked successfully, and
all nine MMU KUnit cases passed in a QEMU TCG guest. Eleven host allocation
budgets checked fixture init failure and repeated cleanup. Physical Mini12
testing remains pending; no hardware rendering result is claimed.

Changes since v1:
  - Patch 3: Fixes tag for 8c8f1c958ab5.
  - Patch 5: Fixes tag for ae012bdc5799.
  - Patch 6: safe partial-init cleanup, checked with allocation failures.

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.

Previous version: Message-ID
<20261109055451.17458-1-operatingsystemsdepression@gmail.com>

Thanks,
Gabriel Gama

[RFC PATCH v2 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 v2 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 v2 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.

Fixes: 8c8f1c958ab5 ("gma500: introduce the GTT and MMU handling logic")
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 v2 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 v2 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.

Fixes: ae012bdc5799 ("drm/gma500: Hook up the MMU")
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 v2 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.

KUnit calls the per-case exit callback even after init fails. Use one
NULL-safe cleanup for partial initialization and clear released pointers
and test->priv so repeated cleanup cannot release resources twice.

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 | 230 ++++++++++++++++++++++++++++++
 3 files changed, 242 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..f9dbe66cd
--- /dev/null
+++ b/drivers/gpu/drm/gma500/mmu_test.c
@@ -0,0 +1,230 @@
+// 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;
+
+	test->priv = NULL;
+	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])
+			return -ENOMEM;
+	}
+	return 0;
+}
+
+static void psb_mmu_test_exit(struct kunit *test)
+{
+	struct psb_mmu_test_context *ctx = test->priv;
+	int i;
+
+	if (!ctx)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->pages); i++) {
+		if (ctx->pages[i]) {
+			__free_page(ctx->pages[i]);
+			ctx->pages[i] = NULL;
+		}
+	}
+	if (ctx->pd) {
+		psb_mmu_free_pagedir(ctx->pd);
+		ctx->pd = NULL;
+	}
+	test->priv = NULL;
+}
+
+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 v2 4/6] drm/gma500: Make SGX MMU insertion checked and transactional

[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 page-table bug
  fixes
--

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

> drm/gma500: Make SGX MMU insertion checked and transactional
>
> 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>

[Severity: Low]
This fixes several bugs in SGX MMU page-table management, such as
allocating page tables and underflowing pt->count when removing unmapped
holes, wrapping 32-bit endpoints on i386, and taking driver->sem in read
mode during PTE updates.  Should this include a Fixes tag?

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

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

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

[Joseph Telaak]

On Mon, Nov 09, 2026 at 03:32:59AM -0300, Gabriel Gama wrote:
> -	WARN_ON(!pobj->in_gart);
> +	if (WARN_ON(!pobj->in_gart))
> +		goto out;

Looks right to me: in_gart is an int, so an unbalanced unpin took it to
-1 and the next pin skipped the page and mapping setup. goto out still
drops the reservation lock.

Small nit: drm_WARN_ON(dev, ...) would match the drm_WARN_ONCE() just
above.

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

[Joseph Telaak]

On Mon, Nov 09, 2026 at 03:33:00AM -0300, Gabriel Gama wrote:
> -		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);

Looks right. One concrete trigger is a 32-bit HIGHMEM kernel with
CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP: kmap_atomic() then maps even the
lowmem PD page through a fixmap slot, so the clflush after the unmap
hits a cleared PTE. It might be worth naming in the message.

The clflush_count change is a separate fix. I'd split it out, or at
least mention it in the subject.

The Fixes tag is right: 8c8f1c958ab5 added both.

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

[Joseph Telaak]

On Mon, Nov 09, 2026 at 03:33:02AM -0300, Gabriel Gama wrote:
> Fixes: ae012bdc5799 ("drm/gma500: Hook up the MMU")

The code looks right to me, and ae012bdc5799 is where the ignored returns
came from.

My concern is that this patch has a Fixes tag and 4/6 doesn't. If stable
takes this one alone, the old insert still leaves partial PTEs on
-ENOMEM, and this patch then frees the pages under them. I'd either drop
the tag or give 4/6 (or its rollback part) a Fixes tag too, with a stable
dependency note.

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

[Joseph Telaak]

On Mon, Nov 09, 2026 at 03:33:01AM -0300, Gabriel Gama wrote:
> Use 64-bit exclusive endpoints so mapping the last GPU page does not wrap
> on i386. Zero-page requests remain no-ops.

They weren't no-ops before. The inner do/while ran once, so a zero-page
insert or remove touched one PTE. They become no-ops with this patch.

This does a lot in one patch: u64 endpoints, validation, the remove-hole
fix, collision and rollback, and the semaphore change. Could you split
it? The hole fix is reachable at unload: psb_driver_unload() removes
mmu_gatt_start (0xe0000000) for vram_stolen_size, while load maps the
stolen range at gatt_start, so unless the GATT BAR sits at 0xe0000000
the removed range was never mapped. That part could take the Fixes tag
the bot suggested.

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

[Joseph Telaak]

On Mon, Nov 09, 2026 at 03:32:58AM -0300, Gabriel Gama wrote:
> Use drm_mode_size_dumb() instead of unchecked size_t arithmetic when
> calculating dumb-buffer pitch and size.

drm_mode_create_dumb() already checks width * cpp * height before it
calls ->dumb_create. What was unchecked is the product after the
64-byte pitch alignment, and only with a 32-bit size_t. Maybe say that.