* simple_alloc space tramples initrd
@ 2016-01-08 17:45 dwalker
2016-01-11 3:09 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: dwalker @ 2016-01-08 17:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Paul Mackerras, Michael Ellerman, linuxppc-dev
Hi,
A powerpc machine I'm working on has this problem where the simple_alloc_init() area is trampling the
initrd. The two are placed fairly close together.
I have a fix for this proposed to add a section into arch/powerpc/boot/zImage.lds.S called "mallocarea"
to account for this space, but not all powerpc platforms use simple_alloc_init(). So for those platforms
it's a waste.
Another alternative is to alter the bootloader to place more space between the kernel image and initrd image.
I wanted to get some feedback on the right way to fix this. It seems like it could be a generic issue on powerpc,
or it's possibly already fixed someplace and I just haven't noticed.
Any feedback appreciated.
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: simple_alloc space tramples initrd
2016-01-08 17:45 simple_alloc space tramples initrd dwalker
@ 2016-01-11 3:09 ` Michael Ellerman
2016-01-11 16:49 ` dwalker
0 siblings, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2016-01-11 3:09 UTC (permalink / raw)
To: dwalker, Benjamin Herrenschmidt; +Cc: Paul Mackerras, linuxppc-dev
On Fri, 2016-01-08 at 09:45 -0800, dwalker@fifo99.com wrote:
> Hi,
>
> A powerpc machine I'm working on has this problem where the
> simple_alloc_init() area is trampling the initrd. The two are placed fairly
> close together.
Which machine / platform?
> I have a fix for this proposed to add a section into
> arch/powerpc/boot/zImage.lds.S called "mallocarea" to account for this space,
> but not all powerpc platforms use simple_alloc_init(). So for those platforms
> it's a waste.
Yeah I don't really like the sound of that. We could do it if it was behind a
CONFIG option, but hopefully there is a better solution.
> Another alternative is to alter the bootloader to place more space between
> the kernel image and initrd image.
>
> I wanted to get some feedback on the right way to fix this. It seems like it
> could be a generic issue on powerpc, or it's possibly already fixed someplace
> and I just haven't noticed.
I don't really know that code very well. But ideally either the boot loader
gives you space, or the platform boot code is smart enough to detect that there
is insufficient room and puts the heap somewhere else.
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: simple_alloc space tramples initrd
2016-01-11 3:09 ` Michael Ellerman
@ 2016-01-11 16:49 ` dwalker
2016-01-11 22:17 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: dwalker @ 2016-01-11 16:49 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev
On Mon, Jan 11, 2016 at 02:09:34PM +1100, Michael Ellerman wrote:
> On Fri, 2016-01-08 at 09:45 -0800, dwalker@fifo99.com wrote:
> > Hi,
> >
> > A powerpc machine I'm working on has this problem where the
> > simple_alloc_init() area is trampling the initrd. The two are placed fairly
> > close together.
>
> Which machine / platform?
It's not upstream yet. I'm still putting the patches together, that's when this
issue came up. I can send an RFC if you want to look at the patches.
> > I have a fix for this proposed to add a section into
> > arch/powerpc/boot/zImage.lds.S called "mallocarea" to account for this space,
> > but not all powerpc platforms use simple_alloc_init(). So for those platforms
> > it's a waste.
>
> Yeah I don't really like the sound of that. We could do it if it was behind a
> CONFIG option, but hopefully there is a better solution.
>
> > Another alternative is to alter the bootloader to place more space between
> > the kernel image and initrd image.
> >
> > I wanted to get some feedback on the right way to fix this. It seems like it
> > could be a generic issue on powerpc, or it's possibly already fixed someplace
> > and I just haven't noticed.
>
> I don't really know that code very well. But ideally either the boot loader
> gives you space, or the platform boot code is smart enough to detect that there
> is insufficient room and puts the heap somewhere else.
It seems like the kernel should be able to handle it. I believe the bootloader passes
the initrd location , but I don't think it's evaluated till later in the boot up. For
simple_alloc_init() it seems all platforms just assume the space is empty without checking.
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: simple_alloc space tramples initrd
2016-01-11 16:49 ` dwalker
@ 2016-01-11 22:17 ` Michael Ellerman
2016-01-11 23:07 ` dwalker
0 siblings, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2016-01-11 22:17 UTC (permalink / raw)
To: dwalker; +Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev
On Mon, 2016-01-11 at 08:49 -0800, dwalker@fifo99.com wrote:
> On Mon, Jan 11, 2016 at 02:09:34PM +1100, Michael Ellerman wrote:
> > On Fri, 2016-01-08 at 09:45 -0800, dwalker@fifo99.com wrote:
> > > Hi,
> > >
> > > A powerpc machine I'm working on has this problem where the
> > > simple_alloc_init() area is trampling the initrd. The two are placed fairly
> > > close together.
> >
> > Which machine / platform?
>
> It's not upstream yet. I'm still putting the patches together, that's when this
> issue came up. I can send an RFC if you want to look at the patches.
OK. Thanks but I don't need more patches to look at :)
I was just trying to narrow down which code you were talking about.
> > I don't really know that code very well. But ideally either the boot loader
> > gives you space, or the platform boot code is smart enough to detect that there
> > is insufficient room and puts the heap somewhere else.
>
> It seems like the kernel should be able to handle it. I believe the bootloader passes
> the initrd location , but I don't think it's evaluated till later in the boot up. For
> simple_alloc_init() it seems all platforms just assume the space is empty without checking.
Yeah that's what I see too, which seems like it's liable to break, but
obviously hasn't for anyone else yet.
The bootloader must pass the initrd location, otherwise the kernel can't use
it, so it seems like the kernel should be able to notice when they are too
close. But it may be complicated by the sequencing of the code.
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: simple_alloc space tramples initrd
2016-01-11 22:17 ` Michael Ellerman
@ 2016-01-11 23:07 ` dwalker
2016-01-11 23:37 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: dwalker @ 2016-01-11 23:07 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev
On Tue, Jan 12, 2016 at 09:17:53AM +1100, Michael Ellerman wrote:
> On Mon, 2016-01-11 at 08:49 -0800, dwalker@fifo99.com wrote:
> > On Mon, Jan 11, 2016 at 02:09:34PM +1100, Michael Ellerman wrote:
> > > On Fri, 2016-01-08 at 09:45 -0800, dwalker@fifo99.com wrote:
> > > > Hi,
> > > >
> > > > A powerpc machine I'm working on has this problem where the
> > > > simple_alloc_init() area is trampling the initrd. The two are placed fairly
> > > > close together.
> > >
> > > Which machine / platform?
> >
> > It's not upstream yet. I'm still putting the patches together, that's when this
> > issue came up. I can send an RFC if you want to look at the patches.
>
> OK. Thanks but I don't need more patches to look at :)
>
> I was just trying to narrow down which code you were talking about.
It's coming eventually anyways ;) ..
> > > I don't really know that code very well. But ideally either the boot loader
> > > gives you space, or the platform boot code is smart enough to detect that there
> > > is insufficient room and puts the heap somewhere else.
> >
> > It seems like the kernel should be able to handle it. I believe the bootloader passes
> > the initrd location , but I don't think it's evaluated till later in the boot up. For
> > simple_alloc_init() it seems all platforms just assume the space is empty without checking.
>
> Yeah that's what I see too, which seems like it's liable to break, but
> obviously hasn't for anyone else yet.
>
> The bootloader must pass the initrd location, otherwise the kernel can't use
> it, so it seems like the kernel should be able to notice when they are too
> close. But it may be complicated by the sequencing of the code.
I found a similar one,
arch/powerpc/boot/ps3.c:platform_init()
I realized that in platform_init() your discovering the initrd location, so you do have
access to the values. In ps3 you can see how if the initrd is placed in the 16megs after
the kernel image then the simple_alloc code could corrupt it.
I think it would be appropriate to check the initrd location in that function (since it's available)
and make a choice to put the simple_alloc area after the initrd if the areas overlap. Does that make
sense ?
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: simple_alloc space tramples initrd
2016-01-11 23:07 ` dwalker
@ 2016-01-11 23:37 ` Michael Ellerman
0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2016-01-11 23:37 UTC (permalink / raw)
To: dwalker; +Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev
On Mon, 2016-01-11 at 15:07 -0800, dwalker@fifo99.com wrote:
> On Tue, Jan 12, 2016 at 09:17:53AM +1100, Michael Ellerman wrote:
> > On Mon, 2016-01-11 at 08:49 -0800, dwalker@fifo99.com wrote:
> > > On Mon, Jan 11, 2016 at 02:09:34PM +1100, Michael Ellerman wrote:
> > > > On Fri, 2016-01-08 at 09:45 -0800, dwalker@fifo99.com wrote:
> > > > > A powerpc machine I'm working on has this problem where the
> > > > > simple_alloc_init() area is trampling the initrd. The two are placed fairly
> > > > > close together.
> > > >
> > > > Which machine / platform?
> > >
> > > It's not upstream yet. I'm still putting the patches together, that's when this
> > > issue came up. I can send an RFC if you want to look at the patches.
> >
> > OK. Thanks but I don't need more patches to look at :)
> >
> > I was just trying to narrow down which code you were talking about.
>
> It's coming eventually anyways ;) ..
Hah, yeah I know :)
> > > > I don't really know that code very well. But ideally either the boot loader
> > > > gives you space, or the platform boot code is smart enough to detect that there
> > > > is insufficient room and puts the heap somewhere else.
> > >
> > > It seems like the kernel should be able to handle it. I believe the bootloader passes
> > > the initrd location , but I don't think it's evaluated till later in the boot up. For
> > > simple_alloc_init() it seems all platforms just assume the space is empty without checking.
> >
> > Yeah that's what I see too, which seems like it's liable to break, but
> > obviously hasn't for anyone else yet.
> >
> > The bootloader must pass the initrd location, otherwise the kernel can't use
> > it, so it seems like the kernel should be able to notice when they are too
> > close. But it may be complicated by the sequencing of the code.
>
> I found a similar one,
>
> arch/powerpc/boot/ps3.c:platform_init()
>
> I realized that in platform_init() your discovering the initrd location, so you do have
> access to the values. In ps3 you can see how if the initrd is placed in the 16megs after
> the kernel image then the simple_alloc code could corrupt it.
>
> I think it would be appropriate to check the initrd location in that function
> (since it's available) and make a choice to put the simple_alloc area after
> the initrd if the areas overlap. Does that make sense ?
Hmm, maybe. I think the ps3 code knows at link time where the initrd is, so
that's kind of cheating.
But if on your platform you can find out the initrd location early enough, then
yes ideally you take that into account when initialising the allocator.
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-11 23:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-08 17:45 simple_alloc space tramples initrd dwalker
2016-01-11 3:09 ` Michael Ellerman
2016-01-11 16:49 ` dwalker
2016-01-11 22:17 ` Michael Ellerman
2016-01-11 23:07 ` dwalker
2016-01-11 23:37 ` Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).