[PATCH v1 3/5] drm/panthor: Track VM faults

Lukas Zapolskas posted 5 patches 11 hours ago
[PATCH v1 3/5] drm/panthor: Track VM faults
Posted by Lukas Zapolskas 11 hours ago
Faults reported via the MMU_CONTROL register block will result in fatal
faults for running groups on that AS, which will also be useful to know
for the user.

Signed-off-by: Lukas Zapolskas <lukas.zapolskas@arm.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c  | 16 ++++++++++++++--
 drivers/gpu/drm/panthor/panthor_mmu.h  | 20 ++++++++++++++++++++
 drivers/gpu/drm/panthor/panthor_regs.h |  3 +++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 473a8bebd61e..10a7418eecda 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -384,6 +384,9 @@ struct panthor_vm {
 		/** @locked_region.size: Size of the locked region. */
 		u64 size;
 	} locked_region;
+
+	/** @fault: Fault information (if any) for this VM. */
+	struct panthor_vm_fault fault;
 };
 
 /**
@@ -741,6 +744,7 @@ int panthor_vm_active(struct panthor_vm *vm)
 
 	/* If the VM is re-activated, we clear the fault. */
 	vm->unhandled_fault = false;
+	vm->fault = (struct panthor_vm_fault){ 0 };
 
 	/* Unhandled pagefault on this AS, clear the fault and re-enable interrupts
 	 * before enabling the AS.
@@ -1744,8 +1748,16 @@ static void panthor_mmu_irq_handler(struct panthor_device *ptdev, u32 status)
 		 */
 		ptdev->mmu->irq.mask = new_int_mask;
 
-		if (ptdev->mmu->as.slots[as].vm)
-			ptdev->mmu->as.slots[as].vm->unhandled_fault = true;
+		if (ptdev->mmu->as.slots[as].vm) {
+			struct panthor_vm *vm = ptdev->mmu->as.slots[as].vm;
+
+			vm->unhandled_fault = true;
+			vm->fault.exception_type = AS_FAULTSTATUS_EXCEPTION_TYPE(status);
+			vm->fault.access_type = AS_FAULTSTATUS_ACCESS_TYPE(status);
+			vm->fault.source_id = AS_FAULTSTATUS_SOURCE_ID(status);
+			vm->fault.valid_address = true;
+			vm->fault.address = addr;
+		}
 
 		/* Disable the MMU to kill jobs on this AS. */
 		panthor_mmu_as_disable(ptdev, as, false);
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.h b/drivers/gpu/drm/panthor/panthor_mmu.h
index 0e268fdfdb2f..023fdc79c231 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.h
+++ b/drivers/gpu/drm/panthor/panthor_mmu.h
@@ -16,6 +16,26 @@ struct panthor_vm;
 struct panthor_vma;
 struct panthor_mmu;
 
+/**
+ * struct panthor_vm_fault - Tracking information for VM-level faults.
+ */
+struct panthor_vm_fault {
+	/** @address: Virtual address of the faulting access. */
+	u64 address;
+
+	/** @exception_type: The type of exception that caused the fault. */
+	u32 exception_type;
+
+	/** @access_type: The direction of data transfer that caused the fault. */
+	u32 access_type;
+
+	/** @source_id: ID supplying further data about the source of the fault. */
+	u32 source_id;
+
+	/** @valid_address: Whether the virtual address is valid. */
+	bool valid_address;
+};
+
 int panthor_mmu_init(struct panthor_device *ptdev);
 void panthor_mmu_unplug(struct panthor_device *ptdev);
 void panthor_mmu_pre_reset(struct panthor_device *ptdev);
diff --git a/drivers/gpu/drm/panthor/panthor_regs.h b/drivers/gpu/drm/panthor/panthor_regs.h
index 08bf06c452d6..5aa5e37d29c9 100644
--- a/drivers/gpu/drm/panthor/panthor_regs.h
+++ b/drivers/gpu/drm/panthor/panthor_regs.h
@@ -178,10 +178,13 @@
 #define   AS_LOCK_REGION_MIN_SIZE			(1ULL << 15)
 #define AS_FAULTSTATUS(as)				(MMU_AS(as) + 0x1C)
 #define  AS_FAULTSTATUS_ACCESS_TYPE_MASK		(0x3 << 8)
+#define  AS_FAULTSTATUS_ACCESS_TYPE(x)			(((x) >> 8) & GENMASK(2, 0))
 #define  AS_FAULTSTATUS_ACCESS_TYPE_ATOMIC		(0x0 << 8)
 #define  AS_FAULTSTATUS_ACCESS_TYPE_EX			(0x1 << 8)
 #define  AS_FAULTSTATUS_ACCESS_TYPE_READ		(0x2 << 8)
 #define  AS_FAULTSTATUS_ACCESS_TYPE_WRITE		(0x3 << 8)
+#define  AS_FAULTSTATUS_EXCEPTION_TYPE(x)		((x) & GENMASK(7, 0))
+#define  AS_FAULTSTATUS_SOURCE_ID(x)			(((x) >> 16) & GENMASK(16, 0))
 #define AS_FAULTADDRESS(as)				(MMU_AS(as) + 0x20)
 #define AS_STATUS(as)					(MMU_AS(as) + 0x28)
 #define   AS_STATUS_AS_ACTIVE				BIT(0)
-- 
2.33.0.dirty
Re: [PATCH v1 3/5] drm/panthor: Track VM faults
Posted by Boris Brezillon 10 hours ago
On Mon, 15 Dec 2025 11:54:55 +0000
Lukas Zapolskas <lukas.zapolskas@arm.com> wrote:

> Faults reported via the MMU_CONTROL register block will result in fatal
> faults for running groups on that AS, which will also be useful to know
> for the user.
> 
> Signed-off-by: Lukas Zapolskas <lukas.zapolskas@arm.com>
> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c  | 16 ++++++++++++++--
>  drivers/gpu/drm/panthor/panthor_mmu.h  | 20 ++++++++++++++++++++
>  drivers/gpu/drm/panthor/panthor_regs.h |  3 +++
>  3 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 473a8bebd61e..10a7418eecda 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -384,6 +384,9 @@ struct panthor_vm {
>  		/** @locked_region.size: Size of the locked region. */
>  		u64 size;
>  	} locked_region;
> +
> +	/** @fault: Fault information (if any) for this VM. */
> +	struct panthor_vm_fault fault;
>  };
>  
>  /**
> @@ -741,6 +744,7 @@ int panthor_vm_active(struct panthor_vm *vm)
>  
>  	/* If the VM is re-activated, we clear the fault. */
>  	vm->unhandled_fault = false;
> +	vm->fault = (struct panthor_vm_fault){ 0 };

I'd rather go for a memset() here, to follow the kernel coding style.

	memset(&vm->fault, 0, sizeof(vm->fault));

>  
>  	/* Unhandled pagefault on this AS, clear the fault and re-enable interrupts
>  	 * before enabling the AS.
> @@ -1744,8 +1748,16 @@ static void panthor_mmu_irq_handler(struct panthor_device *ptdev, u32 status)
>  		 */
>  		ptdev->mmu->irq.mask = new_int_mask;
>  
> -		if (ptdev->mmu->as.slots[as].vm)
> -			ptdev->mmu->as.slots[as].vm->unhandled_fault = true;
> +		if (ptdev->mmu->as.slots[as].vm) {
> +			struct panthor_vm *vm = ptdev->mmu->as.slots[as].vm;
> +
> +			vm->unhandled_fault = true;
> +			vm->fault.exception_type = AS_FAULTSTATUS_EXCEPTION_TYPE(status);
> +			vm->fault.access_type = AS_FAULTSTATUS_ACCESS_TYPE(status);
> +			vm->fault.source_id = AS_FAULTSTATUS_SOURCE_ID(status);

I'd be tempted to return the raw value to userspace instead of parsing
it here. This way if the meaning/layout changes, it's the UMD
responsibility to adjust the parsing (we do that with various other
things already, based on the GPU_ID).

> +			vm->fault.valid_address = true;

Do we really need an extra field to report address validity? Can't we
just the UMD check the status and take a decision based on that?

> +			vm->fault.address = addr;
> +		}
>  
>  		/* Disable the MMU to kill jobs on this AS. */
>  		panthor_mmu_as_disable(ptdev, as, false);
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.h b/drivers/gpu/drm/panthor/panthor_mmu.h
> index 0e268fdfdb2f..023fdc79c231 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.h
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.h
> @@ -16,6 +16,26 @@ struct panthor_vm;
>  struct panthor_vma;
>  struct panthor_mmu;
>  
> +/**
> + * struct panthor_vm_fault - Tracking information for VM-level faults.
> + */
> +struct panthor_vm_fault {
> +	/** @address: Virtual address of the faulting access. */
> +	u64 address;
> +
> +	/** @exception_type: The type of exception that caused the fault. */
> +	u32 exception_type;
> +
> +	/** @access_type: The direction of data transfer that caused the fault. */
> +	u32 access_type;
> +
> +	/** @source_id: ID supplying further data about the source of the fault. */
> +	u32 source_id;
> +
> +	/** @valid_address: Whether the virtual address is valid. */
> +	bool valid_address;
> +};
> +
>  int panthor_mmu_init(struct panthor_device *ptdev);
>  void panthor_mmu_unplug(struct panthor_device *ptdev);
>  void panthor_mmu_pre_reset(struct panthor_device *ptdev);
> diff --git a/drivers/gpu/drm/panthor/panthor_regs.h b/drivers/gpu/drm/panthor/panthor_regs.h
> index 08bf06c452d6..5aa5e37d29c9 100644
> --- a/drivers/gpu/drm/panthor/panthor_regs.h
> +++ b/drivers/gpu/drm/panthor/panthor_regs.h
> @@ -178,10 +178,13 @@
>  #define   AS_LOCK_REGION_MIN_SIZE			(1ULL << 15)
>  #define AS_FAULTSTATUS(as)				(MMU_AS(as) + 0x1C)
>  #define  AS_FAULTSTATUS_ACCESS_TYPE_MASK		(0x3 << 8)
> +#define  AS_FAULTSTATUS_ACCESS_TYPE(x)			(((x) >> 8) & GENMASK(2, 0))
>  #define  AS_FAULTSTATUS_ACCESS_TYPE_ATOMIC		(0x0 << 8)
>  #define  AS_FAULTSTATUS_ACCESS_TYPE_EX			(0x1 << 8)
>  #define  AS_FAULTSTATUS_ACCESS_TYPE_READ		(0x2 << 8)
>  #define  AS_FAULTSTATUS_ACCESS_TYPE_WRITE		(0x3 << 8)
> +#define  AS_FAULTSTATUS_EXCEPTION_TYPE(x)		((x) & GENMASK(7, 0))
> +#define  AS_FAULTSTATUS_SOURCE_ID(x)			(((x) >> 16) & GENMASK(16, 0))
>  #define AS_FAULTADDRESS(as)				(MMU_AS(as) + 0x20)
>  #define AS_STATUS(as)					(MMU_AS(as) + 0x28)
>  #define   AS_STATUS_AS_ACTIVE				BIT(0)