lkml.org 
[lkml]   [2019]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RESENDING PATCH] powerpc/wii: properly disable use of BATs when requested.
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.

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
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2019-01-17 15:56    [W:0.052 / U:32.872 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site