Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 17 Jan 2019 15:55:52 +0100
From:   Jonathan =?utf-8?Q?Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>
To:     Christophe Leroy <christophe.leroy@c-s.fr>
Cc:     Jonathan =?utf-8?Q?Neusch=C3=A4fer?= <j.neuschaefer@gmx.net>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Paul Mackerras <paulus@samba.org>,
        Michael Ellerman <mpe@ellerman.id.au>,
        linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
        stable@vger.kernel.org
Subject: Re: [RESENDING PATCH] powerpc/wii: properly disable use of BATs when
 requested.
Message-ID: <20190117145552.GA2200@latitude>
References: <7e6748349978f4f177b6a1f3f1c773da98ae3b59.1547570012.git.christophe.leroy@c-s.fr>
 <20190117010509.GH22334@latitude>
 <19fd4dd4-ece2-d508-1ed5-5a9ed2aa84fe@c-s.fr>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="6TrnltStXW4iwmi0"
Content-Disposition: inline
In-Reply-To: <19fd4dd4-ece2-d508-1ed5-5a9ed2aa84fe@c-s.fr>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--6TrnltStXW4iwmi0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jan 17, 2019 at 11:29:06AM +0100, Christophe Leroy wrote:
[...]
> > >   	/* MEM2 64MB@0x10000000 */
> > >   	delta =3D wii_hole_start + wii_hole_size;
> > > +	if (__map_without_bats)
> > > +		return delta;
> > > +
> >=20
> > Nothing is visibly broken without this patch, even with
> > CONFIG_DEBUG_PAGEALLOC (tested on top of v5.0-rc2), but the patch still
> > looks correct.
>=20
> Obviously, CONFIG_DEBUG_PAGEALLOC cannot work without this patch.
> The purpose of CONFIG_DEBUG_PAGEALLOC is to unmap unused parts of memory =
so
> that any access to them will pagefault.
>=20
> As this function inconditionnaly sets a BAT for the second block of RAM, =
any
> access to free area in the upper block will be granted without a fault.
>=20
> I think you can test it by doing a kmalloc() then a kfree(), then try to
> read in that area (hopefully kmalloc() allocates memory from the top so it
> should go in the upper block). Maybe there is an LKDTM test for that.

Ah, makes sense, thanks for the explanation.

>=20
> >=20
> > I'd prefer the 'if' block to be before the whole delta/size calculation,
> > to make the code slightly more readable because the delta and size
> > calculations stay in one visual block. It doesn't need to happen after
> > delta is calculated.
>=20
> Euh ... the function has to return 'delta', so if I put the if block befo=
re
> the calculation of delta, it means we have to calculate delta twice:

Oh, sorry, I misread the code, but you're completely right (I shouldn't
answer mails while tired).

>=20
> 	if (__map_without_bats)
> 		return wii_hole_start + wii_hole_size;
>=20
> 	delta =3D wii_hole_start + wii_hole_size;
>=20
> My eyes don't really like it, so if we want to keep delta and size
> calculation together, the 'if' will go after calculation of size.

I agree.

> In anycase, this change is only really for fixing stable releases because
> this function will go away with my other serie.

ACK


Thanks,
Jonathan

--6TrnltStXW4iwmi0
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEvHAHGBBjQPVy+qvDCDBEmo7zX9sFAlxAl3AACgkQCDBEmo7z
X9sekw//ae7XPjO55eotK1OhX+5W0gcbkzrIUV14TGSXYWeXO8LhFtt7W3tFjJky
6mAjVmQu0wGSz/9EqKxh1p7GW9/3xWqK6vL6PCG8/9L6s39hQE6BZepE4zUL7SZO
l/WW2RDHfgsUYcTSwHUE3Ph9R+7u33+4IJqEogW8m0HORL+D4aMaeAnRbxS1c1wu
GvUyD99gOOIEVcmKzDzoC5MqTNpQzsPNIB1p+6O/Y+GwFUK0ZQF1YkbaGRLJSbY5
peAp3iGwzamqLumKAI48DYp/46HQK1HsTXBLHd1ykOfYRWWcnjFsbxmzo/vn+vRA
rpWh3TSvEnNqU5RyES8dpcov1hdIVi+yVhGOahbR6Y156+RC241voO8ppPJpQtZS
rQ4jcc/01OExU4yM0eh4OpCx2XBvzv7Se7alH2vjJfSDLcOWFICAyw6g2KlJtIPf
+rhpoCmAN4tpOzKio88iViAfDBivRvqqKvdDNUxfTym/14Z6JMg0Xt9WDKesVcwa
IzCLE8bAFOv2GE8FW3KPvUQRHYoupjCVoFYREbXM8cAIfNmYC2+zqzkcIDeAehXB
9SMOxQGkfCwPgX1rtP+kiOPrJNnMB8F/CZpWekMPAaGgYLdIfGBwLzJ3ox5wveHc
qpKEpg0THoBIbJMcFCEy3kHI+qv4fG4jwmY8YDC8sJ1Jk6WIuSc=
=4s0A
-----END PGP SIGNATURE-----

--6TrnltStXW4iwmi0--