linux-kernel - Re: [PATCH v2 2/2] drm/etnaviv: print MMU exception cause

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a4f36fed24c8c2a1e9228ee178fe16f2957b3ffb.camel@pengutronix.de>
Date:   Fri, 13 Sep 2019 10:53:25 +0200
From:   Lucas Stach <l.stach@...gutronix.de>
To:     Christian Gmeiner <christian.gmeiner@...il.com>,
        linux-kernel@...r.kernel.org
Cc:     David Airlie <airlied@...ux.ie>, etnaviv@...ts.freedesktop.org,
        dri-devel@...ts.freedesktop.org, Daniel Vetter <daniel@...ll.ch>,
        Russell King <linux+etnaviv@...linux.org.uk>
Subject: Re: [PATCH v2 2/2] drm/etnaviv: print MMU exception cause

On Fr, 2019-09-13 at 07:50 +0200, Christian Gmeiner wrote:
> Might be useful when debugging MMU exceptions.
> 
> Changes in V2:
>  - Use a static array of string for error message as suggested
>    by Lucas Stach.

Please move those changelogs below the 3 dashes, so they don't end up
in the commit message. They don't really add any value to the
persistent kernel history.

> Signed-off-by: Christian Gmeiner <christian.gmeiner@...il.com>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index d47d1a8e0219..b8cd85153fe0 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1351,6 +1351,15 @@ static void sync_point_worker(struct work_struct *work)
>  
>  static void dump_mmu_fault(struct etnaviv_gpu *gpu)
>  {
> +	static const char *errors[] = {
> +		"slave not present",
> +		"page not present",
> +		"write violation",
> +		"out of bound",
> +		"read security violation",
> +		"write security violation",
> +	};
> +
>  	u32 status_reg, status;
>  	int i;
>  
> @@ -1364,10 +1373,16 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu)
>  
>  	for (i = 0; i < 4; i++) {
>  		u32 address_reg;
> +		const char *error = "unknown state";
>  
>  		if (!(status & (VIVS_MMUv2_STATUS_EXCEPTION0__MASK << (i * 4))))
>  			continue;
>  
> +		if (status < ARRAY_SIZE(errors))
> +			error = errors[status];

Huh? This won't work. The status register is a bitfield, not an integer
so you need to map the bit position to the array index via ffs() or
something like that.

Regards,
Lucas

> +
> +		dev_err_ratelimited(gpu->dev, "MMU %d %s\n", i, error);
> +
>  		if (gpu->sec_mode == ETNA_SEC_NONE)
>  			address_reg = VIVS_MMUv2_EXCEPTION_ADDR(i);
>  		else