[PATCH] drm/etnaviv: print MMU exception cause

[PATCH] drm/etnaviv: print MMU exception cause

[Lucas Stach]

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;
+		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));
 	}
 }
 
-- 
2.30.2

Re: [PATCH] drm/etnaviv: print MMU exception cause

[Christian Gmeiner]

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!

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

Re: [PATCH] drm/etnaviv: print MMU exception cause

[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];

... 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

Re: [PATCH] drm/etnaviv: print MMU exception cause

[Lucas Stach]

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