Message ID | 20221130185303.2025810-1-l.stach@pengutronix.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/etnaviv: print MMU exception cause | expand |
Hi Lucas Am Mi., 30. Nov. 2022 um 19:53 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>: > > From: Christian Gmeiner <christian.gmeiner@gmail.com> > > The MMU tells us the fault status. While the raw register value is > already printed, it's a bit more user friendly to translate the > fault reasons into human readable format. > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > I've rewritten parts of the patch to properly cover multiple > MMUs and squashed the reason into the existing message. Christian, > please tell me if you are fine with having your name attached to > this patch. > --- Uff.. and old patch I forgot to send an updated version - sorry for that. I am happy to finally see this patch with your improvements landing - thanks!
On Mi, 2022-11-30 at 19:53 +0100, Lucas Stach wrote: From: Christian Gmeiner <christian.gmeiner@gmail.com> The MMU tells us the fault status. While the raw register value is already printed, it's a bit more user friendly to translate the fault reasons into human readable format. Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- I've rewritten parts of the patch to properly cover multiple MMUs and squashed the reason into the existing message. Christian, please tell me if you are fine with having your name attached to this patch. --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 37018bc55810..f79203b774d9 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1426,6 +1426,15 @@ static void sync_point_worker(struct work_struct *work) static void dump_mmu_fault(struct etnaviv_gpu *gpu) { + static const char *fault_reasons[] = { + "slave not present", + "page not present", + "write violation", + "out of bounds", + "read security violation", + "write security violation", + }; + u32 status_reg, status; int i; @@ -1438,18 +1447,25 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu) dev_err_ratelimited(gpu->dev, "MMU fault status 0x%08x\n", status); for (i = 0; i < 4; i++) { + const char *reason = "unknown"; u32 address_reg; + u32 mmu_status; - if (!(status & (VIVS_MMUv2_STATUS_EXCEPTION0__MASK << (i * 4)))) + mmu_status = (status >> (i * 4)) & VIVS_MMUv2_STATUS_EXCEPTION0__MASK; VIVS_MMUv2_STATUS_EXCEPTION0__MASK is 0x3 ... + if (!mmu_status) continue; + if ((mmu_status - 1) < ARRAY_SIZE(fault_reasons)) + reason = fault_reasons[mmu_status - 1]; ... so (mmu_status - 1) can be 2 at most. This leaves me wondering how "out of bounds" and the "security violation" errors can be reached. I think this requires the exception bitfield masks to be extended to 0x7. regards Philipp
Hi Philipp, Am Donnerstag, dem 01.12.2022 um 09:40 +0100 schrieb Philipp Zabel: > On Mi, 2022-11-30 at 19:53 +0100, Lucas Stach wrote: > From: Christian Gmeiner <christian.gmeiner@gmail.com> > > The MMU tells us the fault status. While the raw register value is > already printed, it's a bit more user friendly to translate the > fault reasons into human readable format. > > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > I've rewritten parts of the patch to properly cover multiple > MMUs and squashed the reason into the existing message. Christian, > please tell me if you are fine with having your name attached to > this patch. > --- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index 37018bc55810..f79203b774d9 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -1426,6 +1426,15 @@ static void sync_point_worker(struct work_struct *work) > > > static void dump_mmu_fault(struct etnaviv_gpu *gpu) > { > + static const char *fault_reasons[] = { > + "slave not present", > + "page not present", > + "write violation", > + "out of bounds", > + "read security violation", > + "write security violation", > + }; > + > u32 status_reg, status; > int i; > > > @@ -1438,18 +1447,25 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu) > dev_err_ratelimited(gpu->dev, "MMU fault status 0x%08x\n", status); > > > for (i = 0; i < 4; i++) { > + const char *reason = "unknown"; > u32 address_reg; > + u32 mmu_status; > > > - if (!(status & (VIVS_MMUv2_STATUS_EXCEPTION0__MASK << (i * 4)))) > + mmu_status = (status >> (i * 4)) & VIVS_MMUv2_STATUS_EXCEPTION0__MASK; > > VIVS_MMUv2_STATUS_EXCEPTION0__MASK is 0x3 ... > > + if (!mmu_status) > continue; > > > + if ((mmu_status - 1) < ARRAY_SIZE(fault_reasons)) > + reason = fault_reasons[mmu_status - 1]; > Your mail quoting seems to be broken, again. > ... so (mmu_status - 1) can be 2 at most. This leaves me wondering how > "out of bounds" and the "security violation" errors can be reached. I > think this requires the exception bitfield masks to be extended to 0x7. Good catch! That's a inconsistency in rnndb, where we claim to be able to stuff the full exception enum into 2 bits. Will fix! Regards, Lucas
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index 37018bc55810..f79203b774d9 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -1426,6 +1426,15 @@ static void sync_point_worker(struct work_struct *work) static void dump_mmu_fault(struct etnaviv_gpu *gpu) { + static const char *fault_reasons[] = { + "slave not present", + "page not present", + "write violation", + "out of bounds", + "read security violation", + "write security violation", + }; + u32 status_reg, status; int i; @@ -1438,18 +1447,25 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu) dev_err_ratelimited(gpu->dev, "MMU fault status 0x%08x\n", status); for (i = 0; i < 4; i++) { + const char *reason = "unknown"; u32 address_reg; + u32 mmu_status; - if (!(status & (VIVS_MMUv2_STATUS_EXCEPTION0__MASK << (i * 4)))) + mmu_status = (status >> (i * 4)) & VIVS_MMUv2_STATUS_EXCEPTION0__MASK; + if (!mmu_status) continue; + if ((mmu_status - 1) < ARRAY_SIZE(fault_reasons)) + reason = fault_reasons[mmu_status - 1]; + if (gpu->sec_mode == ETNA_SEC_NONE) address_reg = VIVS_MMUv2_EXCEPTION_ADDR(i); else address_reg = VIVS_MMUv2_SEC_EXCEPTION_ADDR; - dev_err_ratelimited(gpu->dev, "MMU %d fault addr 0x%08x\n", i, - gpu_read(gpu, address_reg)); + dev_err_ratelimited(gpu->dev, + "MMU %d fault (%s) addr 0x%08x\n", + i, reason, gpu_read(gpu, address_reg)); } }