Re: [RESENDING PATCH] powerpc/wii: properly disable use of BATs when requested.

On Thu, Jan 17, 2019 at 11:29:06AM +0100, Christophe Leroy wrote:
[...]

quoted

quoted

  	/* MEM2 64MB@0x10000000 */
  	delta = wii_hole_start + wii_hole_size;
+	if (__map_without_bats)
+		return delta;
+

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.

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.

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.

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.

quoted

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.

Euh ... the function has to return 'delta', so if I put the if block before
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).

	if (__map_without_bats)
		return wii_hole_start + wii_hole_size;

	delta = wii_hole_start + wii_hole_size;

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