[RFC] ELF Relocatable x86 and x86_64 bzImages

Andi Kleen · External Since: Jul 07, 2006 Posts: 1925

> It is probably patch 17:
> "x86_64: Separate normal memory map initialization from the hotplug case"

Ok that messes things up. Actually I think i prefered the previous
code - it was not that bad as you make it. The two variants.
are really doing mostly the same. So best you drop that.

> I don't see any other patches that touch arch/x86_64/mm/init.c
> before that. At least not in 2.6.18-rc3, which is the base of
> my patchset.

I got three patches that touch mm/init.c in my patchkit
(ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/)

BTW I didn't merge any further patches currently, but might
after the next round when the current comments are addressed.

-Andi

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Andi Kleen · External Since: Jul 07, 2006 Posts: 1925

> Yes. ISO C only defines pointer arithmetic with in arrays.
> I believe gnu C makes it a well defined case.

Nope, it doesn't.

There was a miscompilation on PPC some time ago, that is why
HIDE_RELOC() and __pa_symbol() was implemented.

>
> Currently we do not appear to have any problems on i386.
> But I have at least one case of code that is shared between
> i386 and x86_64 and it is appropriate to use __pa_symbol on
> x86_64.
>
> So I added __pa_symbol for that practical reason.
>
> I would have no problems with generalizing this but I wanted to
> at least make it possible to use the concept on i386.

No problem with that, just use HIDE_RELOC

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Eric W. Biederman · External Since: May 19, 2006 Posts: 1134

Andi Kleen <ak DeleteThis @suse.de> writes:

>> > Actually the best way to reuse would be to first do 64bit uncompressor
>> > and linker directly, but short of that #includes would be fine too.
>>
>> > Would be better to just pull in lib/string.c
>>
>> Maybe. Size is fairly important
>
> Why is size important here?

For the same reason that we compress the kernel. Wink

This is the one chunk of code that we don't compress so every extra
byte makes our executable bigger. Now I think the code size is
actually in the 32k - 64k range so as long as it is a minor change
it doesn't really matter.

The big pain with using lib/string.c and
arch/x86_64/kernel/early_printk.c is that it is significant change
in how the code of misc.c is constructed. Which means some
serious reevaluation of all kinds of things need to be considered.
Making it a lot of work Smile

One of the practical dangers is that we make it more likely
we can kill the boot by messing up the shared code.

I'm not certain what to think when even including normal
kernel headers causes problems. It certainly makes me leery
of including normal kernel code. But it might simplify some
of the problems too.

Whichever way I go scrutinizing that possibility carefully is
a lot of work.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Andi Kleen · External Since: Jul 07, 2006 Posts: 1925

On Wednesday 02 August 2006 06:57, Eric W. Biederman wrote:
> Andi Kleen <ak.TakeThisOut@suse.de> writes:
>
> >> > Actually the best way to reuse would be to first do 64bit uncompressor
> >> > and linker directly, but short of that #includes would be fine too.
> >>
> >> > Would be better to just pull in lib/string.c
> >>
> >> Maybe. Size is fairly important
> >
> > Why is size important here?
>
> For the same reason that we compress the kernel. Wink

>
> This is the one chunk of code that we don't compress so every extra
> byte makes our executable bigger. Now I think the code size is
> actually in the 32k - 64k range so as long as it is a minor change
> it doesn't really matter.

text data bss dec hex filename
1909 352 12 2273 8e1 arch/x86_64/kernel/early_printk.o
2212 0 0 2212 8a4 lib/string.o

It's minor.

>
> The big pain with using lib/string.c and
> arch/x86_64/kernel/early_printk.c is that it is significant change
> in how the code of misc.c is constructed.

Not if you use #include

> Which means some
> serious reevaluation of all kinds of things need to be considered.
> Making it a lot of work Smile

>
> One of the practical dangers is that we make it more likely
> we can kill the boot by messing up the shared code.

If they're messed up the later boot will fail too. Doesn't make
too much difference.

>
> I'm not certain what to think when even including normal
> kernel headers causes problems. It certainly makes me leery
> of including normal kernel code. But it might simplify some
> of the problems too.

On x86-64 some trouble comes from it being 32bit code.
That is why I suggested making it 64bit first, which would
avoid many of the problems.

> Whichever way I go scrutinizing that possibility carefully is
> a lot of work.

64bit conversion would be some work, the rest isn't I think.

Alternatively if you don't like it we can just drop these compressor patches.
I don't think they were essential.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Eric W. Biederman · External Since: May 19, 2006 Posts: 1134

Andi Kleen <ak.DeleteThis@suse.de> writes:

>> > /* WARNING!!
>> > * This code is compiled with -fPIC and it is relocated dynamically
>> > * at run time, but no relocation processing is performed.
>> > * This means that it is not safe to place pointers in static structures.
>> > */
>
> iirc the only static relocation in early_printk is the one to initialize
> the console pointers - that could certainly be moved to be at run
> time.

The function pointers in the console structure are also a problem.
static struct console simnow_console = {
.name = "simnow",
.write = simnow_write,
.flags = CON_PRINTBUFFER,
.index = -1,
};

>> lib/string.c might be useful. The fact that the functions are not
>> static slightly concerns me. I have a vague memory of non-static
>> functions generating relocations for no good reason.
>
> Would surprise me.

The context where it bit me was memtest86, if I recall correctly.
The problem there was I did process relocations and I discovered simply
by making functions static or at least non-exported I had many fewer
relocations to process.

Since I am relying on a very clever trick to generate code that
doesn't have relocations at run time I have to be careful.

So if I want to continue not processing relocations.
I need to be careful not to use constructs that will generate
a procedure linkage table, which I think only kicks in with
external functions and multiple files.

I need to be careful not to put pointers in statically allocated
data structures.

Ideally the code would be setup so you can compile out consoles
the user finds uninteresting.

It is annoying to have to call strlen on all of the strings
you want to print..

So there are plenty of mismatches, there.
But we may be able to harmonized them, and reuse early_printk.

Eric

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Eric W. Biederman · External Since: May 19, 2006 Posts: 1134

Andi Kleen <ak DeleteThis @suse.de> writes:

>> It is probably patch 17:
>> "x86_64: Separate normal memory map initialization from the hotplug case"
>
> Ok that messes things up. Actually I think i prefered the previous
> code - it was not that bad as you make it. The two variants.
> are really doing mostly the same. So best you drop that.

All of my complaints are real. But yes I do think a reasonable
case can be made for merging them. In several of the worst cases
simply calling memset before initializing the page is probably
sufficient to remove a test later on.

As that code sits right now you need way too much global context
to understand what is going on. It is the kind of code that cause
obviously correct patches to fail, and I'm clever enough to know
clever code is very dangerous. Smile

So before I get back to that I will probably look and see if there
is some more heavy lifting I can do to make that code less of a land mine.

>> I don't see any other patches that touch arch/x86_64/mm/init.c
>> before that. At least not in 2.6.18-rc3, which is the base of
>> my patchset.
>
> I got three patches that touch mm/init.c in my patchkit
> (ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/)
>
> BTW I didn't merge any further patches currently, but might
> after the next round when the current comments are addressed.

Ok. I will take a look.

Having any patches merged on a simple request for comments was a bit of a surprise Smile

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Andi Kleen · External Since: Jul 07, 2006 Posts: 1925

> The function pointers in the console structure are also a problem.
> static struct console simnow_console = {
> .name = "simnow",
> .write = simnow_write,
> .flags = CON_PRINTBUFFER,
> .index = -1,
> };

Yes just patch them at runtime.

> Ideally the code would be setup so you can compile out consoles
> the user finds uninteresting.

Seems overkill for early_printk

> It is annoying to have to call strlen on all of the strings
> you want to print..

What strlen?

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Magnus Damm · External Since: May 25, 2006 Posts: 48

On 8/1/06, Eric W. Biederman <ebiederm.DeleteThis@xmission.com> wrote:
>
> The problem:
>
> We can't always run the kernel at 1MB or 2MB, and so people who need
> different addresses must build multiple kernels. The bzImage format
> can't even represent loading a kernel at other than it's default address.
> With kexec on panic now starting to be used by distros having a kernel
> not running at the default load address is starting to become common.
>
> The goal of this patch series is to build kernels that are relocatable
> at run time, and to extend the bzImage format to make it capable of
> expressing a relocatable kernel.

Nice work. I'd really like to see support for relocatable kernels in
mainline (and kexec-tools!).

Eric, could you please list the advantages of your run-time relocation
code over my incomplete relocate-in-userspace prototype posted to
fastboot a few weeks ago?

One thing I know for sure is that your implementation supports bzImage
while my only supports relocation of vmlinux files. Are there any
other uses for relocatable bzImage except kdump?

Thanks!

/ magnus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Eric W. Biederman · External Since: May 19, 2006 Posts: 1134

Andi Kleen <ak.DeleteThis@suse.de> writes:

> On Wednesday 02 August 2006 06:57, Eric W. Biederman wrote:
>
> On x86-64 some trouble comes from it being 32bit code.
> That is why I suggested making it 64bit first, which would
> avoid many of the problems.

Smile

>> Whichever way I go scrutinizing that possibility carefully is
>> a lot of work.
>
> 64bit conversion would be some work, the rest isn't I think.

Except for the head.S work the 64bit conversion was practically a noop.

> Alternatively if you don't like it we can just drop these compressor patches.
> I don't think they were essential.

Agreed. The printing portion wasn't essential.

At this point I think dropping the non-essential bits just to get the size
of the patchset down makes sense.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Eric W. Biederman · External Since: May 19, 2006 Posts: 1134

"Magnus Damm" <magnus.damm.TakeThisOut@gmail.com> writes:

> On 8/1/06, Eric W. Biederman <ebiederm.TakeThisOut@xmission.com> wrote:
>>
>> The problem:
>>
>> We can't always run the kernel at 1MB or 2MB, and so people who need
>> different addresses must build multiple kernels. The bzImage format
>> can't even represent loading a kernel at other than it's default address.
>> With kexec on panic now starting to be used by distros having a kernel
>> not running at the default load address is starting to become common.
>>
>> The goal of this patch series is to build kernels that are relocatable
>> at run time, and to extend the bzImage format to make it capable of
>> expressing a relocatable kernel.
>
> Nice work. I'd really like to see support for relocatable kernels in
> mainline (and kexec-tools!).

kexec-tools already have initial support for ELF ET_DYN executables.
Vivek posted a fix a day two ago so I expect that the support
should be working.

> Eric, could you please list the advantages of your run-time relocation
> code over my incomplete relocate-in-userspace prototype posted to
> fastboot a few weeks ago?

If you watch an architecture evolve one thing you will notice is that
the kinds of relocations keep growing. An ever growing list of things
to for the bootloader to do is a pain. Especially when bootloaders
generally need to be as simple and as fixed as possible because bootloaders
are not something you generally want to update.

Beyond that if you look at head.S the code to process the relocations
(after I have finished post processing them at build time) is 9 instructions.
Which is absolutely trivial, at least for now.

By keeping the bzImage processing the relocations we have kept the
bootloader/kernel interface simple.

> One thing I know for sure is that your implementation supports bzImage
> while my only supports relocation of vmlinux files. Are there any
> other uses for relocatable bzImage except kdump?

I can't think of any volume users. A hypervisor that would actually report
the real physical addresses would be a candidate. It's a general purpose
facility so if it is interesting users will show up. Static
relocation has already found another use on x86_64.

There are definitely users of an ELF bzImage beyond the kdump case.
Anything that doesn't have a traditional 16bit BIOS on it. LinuxBIOS,
and Xen, and some others.

Not having to keep track of anything but your bzImage to boot is also
a serious advantage. It's the one binary to rule them all. Smile

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Eric W. Biederman · External Since: May 19, 2006 Posts: 1134

Andi Kleen <ak.TakeThisOut@suse.de> writes:

>
>> The function pointers in the console structure are also a problem.
>> static struct console simnow_console = {
>> .name = "simnow",
>> .write = simnow_write,
>> .flags = CON_PRINTBUFFER,
>> .index = -1,
>> };
>
> Yes just patch them at runtime.

I guess that can work. It's a bit of a pain though.

>> Ideally the code would be setup so you can compile out consoles
>> the user finds uninteresting.
>
> Seems overkill for early_printk

At least compiling completely out probably isn't.
I have had too many times where the size of a bzImage was an important
factor on a project.

>> It is annoying to have to call strlen on all of the strings
>> you want to print..
>
> What strlen?

The strlen that is needed to convert putstr(char *s) into the
write method for the early_printk helpers.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Magnus Damm · External Since: May 25, 2006 Posts: 48

On 8/2/06, Eric W. Biederman <ebiederm DeleteThis @xmission.com> wrote:
> "Magnus Damm" <magnus.damm DeleteThis @gmail.com> writes:
> > Eric, could you please list the advantages of your run-time relocation
> > code over my incomplete relocate-in-userspace prototype posted to
> > fastboot a few weeks ago?
>
> If you watch an architecture evolve one thing you will notice is that
> the kinds of relocations keep growing. An ever growing list of things
> to for the bootloader to do is a pain. Especially when bootloaders
> generally need to be as simple and as fixed as possible because bootloaders
> are not something you generally want to update.

I agree that updating bootloaders is something you want to avoid. I'm
not however sure that I would call kexec-tools a bootloader...

> Beyond that if you look at head.S the code to process the relocations
> (after I have finished post processing them at build time) is 9 instructions.
> Which is absolutely trivial, at least for now.

Yeah, but the 33 patches are touching more than 9 instructions. =)

> By keeping the bzImage processing the relocations we have kept the
> bootloader/kernel interface simple.

Agreed. I think your patch makes sense.

> > One thing I know for sure is that your implementation supports bzImage
> > while my only supports relocation of vmlinux files. Are there any
> > other uses for relocatable bzImage except kdump?
>
> I can't think of any volume users. A hypervisor that would actually report
> the real physical addresses would be a candidate. It's a general purpose
> facility so if it is interesting users will show up. Static
> relocation has already found another use on x86_64.
>
> There are definitely users of an ELF bzImage beyond the kdump case.
> Anything that doesn't have a traditional 16bit BIOS on it. LinuxBIOS,
> and Xen, and some others.
>
> Not having to keep track of anything but your bzImage to boot is also
> a serious advantage. It's the one binary to rule them all. Smile

One binary to rule them all... If that is true, is there any simple
way then to extract vmlinux from the bzImage?

Thanks!

/ magnus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Eric W. Biederman · External Since: May 19, 2006 Posts: 1134

"Magnus Damm" <magnus.damm RemoveThis @gmail.com> writes:

> On 8/2/06, Eric W. Biederman <ebiederm RemoveThis @xmission.com> wrote:
>> "Magnus Damm" <magnus.damm RemoveThis @gmail.com> writes:
>> > Eric, could you please list the advantages of your run-time relocation
>> > code over my incomplete relocate-in-userspace prototype posted to
>> > fastboot a few weeks ago?
>>
>> If you watch an architecture evolve one thing you will notice is that
>> the kinds of relocations keep growing. An ever growing list of things
>> to for the bootloader to do is a pain. Especially when bootloaders
>> generally need to be as simple and as fixed as possible because bootloaders
>> are not something you generally want to update.
>
> I agree that updating bootloaders is something you want to avoid. I'm
> not however sure that I would call kexec-tools a bootloader...

On the truly insane possibilities. It is actually possible to load
a relocated bzImage. run setup16.S below 1M and have it jump
to the kernel at any address below 4G.

>> Beyond that if you look at head.S the code to process the relocations
>> (after I have finished post processing them at build time) is 9 instructions.
>> Which is absolutely trivial, at least for now.
>
> Yeah, but the 33 patches are touching more than 9 instructions. =)

True. I at of that is general clean ups to allow the kernel to be
relocated though. Not to actually perform the relocation.

> One binary to rule them all... If that is true, is there any simple
> way then to extract vmlinux from the bzImage?

Unfortunately the process is a little lossy Sad

So that still means you still need the vmlinux to get the debug
symbols.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Eric W. Biederman · External Since: May 19, 2006 Posts: 1134

Sam Ravnborg <sam.RemoveThis@ravnborg.org> writes:

> On Tue, Aug 01, 2006 at 05:03:19AM -0600, Eric W. Biederman wrote:
>> Defining __PHYSICAL_START and __KERNEL_START in asm-i386/page.h works but
>> it triggers a full kernel rebuild for the silliest of reasons. This
>> modifies the users to directly use CONFIG_PHYSICAL_START and linux/config.h
>> which prevents the full rebuild problem, which makes the code much
>> more maintainer and hopefully user friendly.
>>
>> Signed-off-by: Eric W. Biederman <ebiederm.RemoveThis@xmission.com>
>> ---
>> arch/i386/boot/compressed/head.S | 8 ++++----
>> arch/i386/boot/compressed/misc.c | 8 ++++----
>> arch/i386/kernel/vmlinux.lds.S | 3 ++-
>> include/asm-i386/page.h | 3 ---
>> 4 files changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/i386/boot/compressed/head.S
> b/arch/i386/boot/compressed/head.S
>> index b5893e4..8f28ecd 100644
>> --- a/arch/i386/boot/compressed/head.S
>> +++ b/arch/i386/boot/compressed/head.S
>> @@ -23,9 +23,9 @@
>> */
>> .text
>>
>> +#include <linux/config.h>
>
> You already have full access to all CONFIG_* symbols - kbuild includes
> it on the commandline. So please kill this include.

Stupid questions:
- Why do we still have a linux/config.h if it is totally redundant.
- Why don't we have at least a #warning in linux/config.h that would
tell us not to include it.
- Why do we still have about 200 includes of linux/config.h in the
kernel tree?

I would much rather have a compile error, or at least a compile
warning rather than needed a code review to notice this error.

We haven't needed this header since october of last year.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Don Zickus · External Since: Aug 02, 2006 Posts: 6

>
> There is one outstanding issue where I am probably requiring too much alignment
> on the arch/i386 kernel.

There was posts awhile ago about optimizing the kernel performance by
loading it at a 4MB offset.

http://www.lkml.org/lkml/2006/2/23/189

Your changes breaks that on i386 (not aligned on a 4MB boundary). But a
5MB offset works. Is that the correct update or does that break the
original idea?

Cheers,
Don
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Eric W. Biederman · External Since: May 19, 2006 Posts: 1134

Don Zickus <dzickus.TakeThisOut@redhat.com> writes:

>>
>> There is one outstanding issue where I am probably requiring too much
> alignment
>> on the arch/i386 kernel.
>
> There was posts awhile ago about optimizing the kernel performance by
> loading it at a 4MB offset.
>
> http://www.lkml.org/lkml/2006/2/23/189
>
> Your changes breaks that on i386 (not aligned on a 4MB boundary). But a
> 5MB offset works. Is that the correct update or does that break the
> original idea?

That patch should still apply and work as described.

Actually when this stuipd cold I have stops slowing me down,
and I fix the alignment to what it really needs to be ~= 8KB.

Then bootloaders should be able to make the decision.

HPA Does that sound at all interesting?

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

H. Peter Anvin · External Since: Feb 08, 2006 Posts: 1055

Eric W. Biederman wrote:
> Don Zickus <dzickus RemoveThis @redhat.com> writes:
>
>>> There is one outstanding issue where I am probably requiring too much
>> alignment
>>> on the arch/i386 kernel.
>> There was posts awhile ago about optimizing the kernel performance by
>> loading it at a 4MB offset.
>>
>> http://www.lkml.org/lkml/2006/2/23/189
>>
>> Your changes breaks that on i386 (not aligned on a 4MB boundary). But a
>> 5MB offset works. Is that the correct update or does that break the
>> original idea?
>
> That patch should still apply and work as described.
>
> Actually when this stuipd cold I have stops slowing me down,
> and I fix the alignment to what it really needs to be ~= 8KB.
>
> Then bootloaders should be able to make the decision.
>
> HPA Does that sound at all interesting?
>

I'm sorry, it's not clear to me what you're asking here.

The bootloaders will load bzImage at the 1 MB point, and it's up to the
decompressor to locate it appropriately. It has (correctly) been
pointed out that it would be faster if the decompressed kernel is
located to the 4 MB point -- large pages don't work below 2/4 MB due to
interference with the fixed MTRRs -- but that's doesn't affect the boot
protocol in any way.

I was under the impression that your relocatable patches allows the boot
loader to load the bzImage at a different address than the usual
0x100000; but again, that shouldn't affect the kernel's final resting place.

-hpa

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Sam Ravnborg · External Since: May 17, 2006 Posts: 644

>
> Stupid questions:
> - Why do we still have a linux/config.h if it is totally redundant.
> - Why don't we have at least a #warning in linux/config.h that would
> tell us not to include it.
> - Why do we still have about 200 includes of linux/config.h in the
> kernel tree?
>
> I would much rather have a compile error, or at least a compile
> warning rather than needed a code review to notice this error.
In progress. As part of the ongoing header cleanup all include
<config.h> are being removed and a warning is included in config.h.

When the change was done I did not want to spew out thousands of warning
for a simple thing like this.

Sam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Vivek Goyal · External Since: May 15, 2006 Posts: 286

On Tue, Aug 01, 2006 at 04:58:49AM -0600, Eric W. Biederman wrote:
>
> The problem:
>
> We can't always run the kernel at 1MB or 2MB, and so people who need
> different addresses must build multiple kernels. The bzImage format
> can't even represent loading a kernel at other than it's default address.
> With kexec on panic now starting to be used by distros having a kernel
> not running at the default load address is starting to become common.
>
Hi Eric,

There seems to be a small anomaly in the current set of patches for i386.

For example if one compiles the kernel with CONFIG_RELOCATABLE=y
and CONFIG_PHYSICAL_START=0x400000 (4MB) and he uses grub to load
the kernel then kernel would run from 1MB location. I think user would
expect it to run from 4MB location.

I think distro's might want to keep above config options enabled.
CONFIG_RELOCATABLE=y so that kexec can load kdump kernel at a
different address and CONFIG_PHYSICAL_START=non 1MB location, to
extract better performance. (As we had discussions on mailing list
some time back.)

In principle this is a limitation on boot-loaders part but as we can
not fix the boot-loaders out there, probably we can try fixing it
at kernel level.

What I have done here is that decompressor code will determine the
final resting place of the kernel based on boot loader type. So
if I have been loaded by kexec, I am supposed to run from loaded address
otherwise I am supposed to run from CONFIG_PHYSICAL_START as I have been
loaded at 1MB address due to boot loader limitation and that's not the
intention.

A prototype patch is attached with the mail. I have assumed that I can
assign a boot loader type id 9 to kexec (Documentation/i386/boot.txt).
Also assuming that all other boot loaders apart from kexec have got 1MB
limitation. If not, its trivial to include their boot loader ids also.

I have tested this patch and it works fine. What do you think about
this approach ?

Thanks
Vivek

Signed-off-by: Vivek Goyal <vgoyal DeleteThis @in.ibm.com>
---

arch/i386/boot/compressed/head.S | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)

diff -puN arch/i386/boot/compressed/head.S~debug1-patch arch/i386/boot/compressed/head.S
--- linux-2.6.18-rc3-1M/arch/i386/boot/compressed/head.S~debug1-patch 2006-08-04 18:03:02.000000000 -0400
+++ linux-2.6.18-rc3-1M-root/arch/i386/boot/compressed/head.S 2006-08-04 18:18:26.000000000 -0400
@@ -60,13 +60,32 @@ startup_32:
* a relocatable kernel this is the delta to our load address otherwise
* this is the delta to CONFIG_PHYSICAL start.
*/
+
#ifdef CONFIG_RELOCTABLE
+ /* If loaded by non kexec boot loader, then we will be loaded
+ * at 1MB fixed address. But probably the intention is to run
+ * from a address for which kernel has been compiled which can
+ * be non 1MB.
+ */
+ xorl %eax, %eax
+ movb 0x210(%esi), %al
+
+ / * check boot loader type. Kexec bootloader id 9, version any */
+ shrl $4, %eax
+ subl $0x9, %eax
+ jnz 1f
+
+ /* Run kernel from the location it has been loaded at. */
movl %ebp, %ebx
+ jmp 2f
+
+ /* Run the kernel from compiled destination location. */
+1: movl $(CONFIG_PHYSICAL_START - startup32), %ebx
#else
movl $(CONFIG_PHYSICAL_START - startup_32), %ebx
#endif

- /* Replace the compressed data size with the uncompressed size */
+2: /* Replace the compressed data size with the uncompressed size */
subl input_len(%ebp), %ebx
movl output_len(%ebp), %eax
addl %eax, %ebx
@@ -95,7 +114,16 @@ startup_32:
/* Compute the kernel start address.
*/
#ifdef CONFIG_RELOCATABLE
+ xorl %eax, %eax
+ movb 0x210(%esi), %al
+ / * check boot loader type. Kexec bootloader id 9, version any */
+ shrl $4, %eax
+ subl $0x9, %eax
+ jnz 3f
leal startup_32(%ebp), %ebp
+ jmp 4f
+3:
+ movl $CONFIG_PHYSICAL_START, %ebp
#else
movl $CONFIG_PHYSICAL_START, %ebp
#endif
@@ -103,7 +131,7 @@ startup_32:
/*
* Jump to the relocated address.
*/
- leal relocated(%ebx), %eax
+4: leal relocated(%ebx), %eax
jmp *%eax
.section ".text"
relocated:
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Eric W. Biederman · External Since: May 19, 2006 Posts: 1134

Vivek Goyal <vgoyal.TakeThisOut@in.ibm.com> writes:

> On Tue, Aug 01, 2006 at 04:58:49AM -0600, Eric W. Biederman wrote:
>>
>> The problem:
>>
>> We can't always run the kernel at 1MB or 2MB, and so people who need
>> different addresses must build multiple kernels. The bzImage format
>> can't even represent loading a kernel at other than it's default address.
>> With kexec on panic now starting to be used by distros having a kernel
>> not running at the default load address is starting to become common.
>>
> Hi Eric,
>
> There seems to be a small anomaly in the current set of patches for i386.
>
> For example if one compiles the kernel with CONFIG_RELOCATABLE=y
> and CONFIG_PHYSICAL_START=0x400000 (4MB) and he uses grub to load
> the kernel then kernel would run from 1MB location. I think user would
> expect it to run from 4MB location.

Agreed. That is a non-intuitive, and should probably be fixed.

> I think distro's might want to keep above config options enabled.
> CONFIG_RELOCATABLE=y so that kexec can load kdump kernel at a
> different address and CONFIG_PHYSICAL_START=non 1MB location, to
> extract better performance. (As we had discussions on mailing list
> some time back.)
>
> In principle this is a limitation on boot-loaders part but as we can
> not fix the boot-loaders out there, probably we can try fixing it
> at kernel level.
>
> What I have done here is that decompressor code will determine the
> final resting place of the kernel based on boot loader type. So
> if I have been loaded by kexec, I am supposed to run from loaded address
> otherwise I am supposed to run from CONFIG_PHYSICAL_START as I have been
> loaded at 1MB address due to boot loader limitation and that's not the
> intention.
>
> A prototype patch is attached with the mail. I have assumed that I can
> assign a boot loader type id 9 to kexec (Documentation/i386/boot.txt).
> Also assuming that all other boot loaders apart from kexec have got 1MB
> limitation. If not, its trivial to include their boot loader ids also.
>
> I have tested this patch and it works fine. What do you think about
> this approach ?

I think there is some value in it. But I need to digest it.

I have a cold right now and am running pretty weak, so it is going to take me
a little bit to look at this.

I don't like taking action based upon bootloader type. As that assumes
all kinds of things. But having better rules for when we perform relocation
makes sense. There might be a way to detect coming from setup.S

I gave it some care last time, I worked through this and it didn't quite work.

I guess the practical question is do people see a real performance benefit
when loading the kernel at 4MB?

Possibly the right solution is to do like I did on x86_64 and simply remove
CONFIG_PHYSICAL_START, and always place the kernel at 4MB, or something like
that.

The practical question is what to do to keep the complexity from spinning
out of control. Removing CONFIG_PHYSICAL_START would seriously help with
that.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/