On Mon, 15 Dec 2025 11:54:55 +0000 Lukas Zapolskas <[email protected]> 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 <[email protected]> > --- > 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)
