linux-kernel - Re: [PATCH 04/12] drm/nouveau/bar/nvc0: support chips without BAR3

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140324221051.GC17218@mithrandir>
Date:	Mon, 24 Mar 2014 23:10:53 +0100
From:	Thierry Reding <thierry.reding@...il.com>
To:	Alexandre Courbot <acourbot@...dia.com>
Cc:	Ben Skeggs <bskeggs@...hat.com>, nouveau@...ts.freedesktop.org,
	dri-devel@...ts.freedesktop.org, linux-tegra@...r.kernel.org,
	linux-kernel@...r.kernel.org, gnurou@...il.com
Subject: Re: [PATCH 04/12] drm/nouveau/bar/nvc0: support chips without BAR3

On Mon, Mar 24, 2014 at 05:42:26PM +0900, Alexandre Courbot wrote:
[...]
> diff --git a/drivers/gpu/drm/nouveau/core/subdev/bar/nvc0.c b/drivers/gpu/drm/nouveau/core/subdev/bar/nvc0.c
[...]
>  static int
> -nvc0_bar_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
> -	      struct nouveau_oclass *oclass, void *data, u32 size,
> -	      struct nouveau_object **pobject)
> +nvc0_bar_init_vm(struct nvc0_bar_priv *priv, int nr, int bar)
>  {
[...]
> -	/* BAR3 */
>  	ret = nouveau_gpuobj_new(nv_object(priv), NULL, 0x1000, 0, 0,
> -				&priv->bar[0].mem);
> -	mem = priv->bar[0].mem;
> +				&priv->bar[nr].mem);
> +	mem = priv->bar[nr].mem;
>  	if (ret)
>  		return ret;
>  
>  	ret = nouveau_gpuobj_new(nv_object(priv), NULL, 0x8000, 0, 0,
> -				&priv->bar[0].pgd);
> +				&priv->bar[nr].pgd);
>  	if (ret)
>  		return ret;
[...]
> +static int
> +nvc0_bar_ctor(struct nouveau_object *parent, struct nouveau_object *engine,
> +	      struct nouveau_oclass *oclass, void *data, u32 size,
> +	      struct nouveau_object **pobject)
> +{
[...]
> +	/* BAR3 */
> +	if (has_bar3) {
> +		ret = nvc0_bar_init_vm(priv, 0, 3);
[...]
> +	/* BAR1 */
> +	ret = nvc0_bar_init_vm(priv, 1, 1);
>  	if (ret)
>  		return ret;

The calls to nvc0_bar_init_vm() are somewhat confusing in my opinion. It
is hard to see from the invocation what these numbers mean and therefore
distinguish which parameter is which.

Perhaps a slightly more readable way would be to pass in a pointer to a
structure as second parameter instead of the index into an array. So
it'd look somewhat like this:

	if (has_bar3) {
		ret = nvc0_bar_init_vm(priv, &priv->bar[0], 3);
		...
	}
	...
	ret = nvc0_bar_init_vm(priv, &priv->bar[1], 1);
	...

Unfortunately that would require a new type to be created for the bar[]
structures, so it'd be slightly more intrusive.

Thierry

Content of type "application/pgp-signature" skipped