Re: [RESENDING PATCH] powerpc/wii: properly disable use of BATs when requested.
From: Jonathan NeuschÃfer
Date: Thu Jan 17 2019 - 09:56:05 EST
On Thu, Jan 17, 2019 at 11:29:06AM +0100, Christophe Leroy wrote:
> > > /* 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.
> > 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.
> In anycase, this change is only really for fixing stable releases because
> this function will go away with my other serie.
Description: PGP signature