Kexec command line length

Kexec command line length

[Neil Horman]

Hey all-
	Regarding this bug:
	http://bugzilla.kernel.org/show_bug.cgi?id=9641
	I'd like to look into putting together a patch for it, and wanted to
solicit comments for the best way to go about doing it.  Currently I've got it
fixed up in the Red Hat tree by bumping COMMAND_LINE_SIZE to 2048 and
eliminating the reserved buffer of the x86_linux_faked_param_header, which works
well, but isn't backwards compatible as Bernhard pointed out.  Given that extra
constraint, I thought it woudl e best to unify the command line and reserved
buffers in x86_linux_faked_param_header to one contiguour 2048 byte block and
maintain a separate variable that defines the command line length based on a
parsing of the UTS_VERSION.  Does that sound reasonable to everyone, or is there
a better way that someone has in mind?

Regards
Neil


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Bernhard Walle]

* Neil Horman <nhorman@tuxdriver.com> [2008-01-14 14:43]:
> Hey all-
> 	Regarding this bug:
> 	http://bugzilla.kernel.org/show_bug.cgi?id=9641
> 	I'd like to look into putting together a patch for it, and wanted to
> solicit comments for the best way to go about doing it.  Currently I've got it
> fixed up in the Red Hat tree by bumping COMMAND_LINE_SIZE to 2048 and
> eliminating the reserved buffer of the x86_linux_faked_param_header, which works
> well, but isn't backwards compatible as Bernhard pointed out.  Given that extra
> constraint, I thought it woudl e best to unify the command line and reserved
> buffers in x86_linux_faked_param_header to one contiguour 2048 byte block and
> maintain a separate variable that defines the command line length based on a
> parsing of the UTS_VERSION.  Does that sound reasonable to everyone, or is there
> a better way that someone has in mind?

For bzImage we can take the size in the header (that's also better for
the future).


        Bernhard

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Mon, Jan 14, 2008 at 03:50:15PM +0100, Bernhard Walle wrote:
> * Neil Horman <nhorman@tuxdriver.com> [2008-01-14 14:43]:
> > Hey all-
> > 	Regarding this bug:
> > 	http://bugzilla.kernel.org/show_bug.cgi?id=9641
> > 	I'd like to look into putting together a patch for it, and wanted to
> > solicit comments for the best way to go about doing it.  Currently I've got it
> > fixed up in the Red Hat tree by bumping COMMAND_LINE_SIZE to 2048 and
> > eliminating the reserved buffer of the x86_linux_faked_param_header, which works
> > well, but isn't backwards compatible as Bernhard pointed out.  Given that extra
> > constraint, I thought it woudl e best to unify the command line and reserved
> > buffers in x86_linux_faked_param_header to one contiguour 2048 byte block and
> > maintain a separate variable that defines the command line length based on a
> > parsing of the UTS_VERSION.  Does that sound reasonable to everyone, or is there
> > a better way that someone has in mind?
> 
> For bzImage we can take the size in the header (that's also better for
> the future).
> 

Ok, but thats just one case, what about the other 4 image types that we support?
Regards
Neil

> 
>         Bernhard
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@redhat.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

On Mon, Jan 14, 2008 at 08:43:03AM -0500, Neil Horman wrote:
> Hey all-
> 	Regarding this bug:
> 	http://bugzilla.kernel.org/show_bug.cgi?id=9641
> 	I'd like to look into putting together a patch for it, and wanted to
> solicit comments for the best way to go about doing it.  Currently I've got it
> fixed up in the Red Hat tree by bumping COMMAND_LINE_SIZE to 2048 and
> eliminating the reserved buffer of the x86_linux_faked_param_header, which works
> well, but isn't backwards compatible as Bernhard pointed out.  Given that extra
> constraint, I thought it woudl e best to unify the command line and reserved
> buffers in x86_linux_faked_param_header to one contiguour 2048 byte block and
> maintain a separate variable that defines the command line length based on a
> parsing of the UTS_VERSION.  Does that sound reasonable to everyone, or is there
> a better way that someone has in mind?
> 

Hi Neil,

Looking at UTS_VERSION and then deciding the command line length seems
ok. 

When I look at inclue/asm-x86/bootparam.h in kernel, area starting from
0x2d0 to all the way up to 4K has been reserved for e820 maps and EDD buf.

Does that mean newer boot loaders are putting command line outside of 
4K page and only putting the pointer to cmdline in 4K page. If that's the
case then we might have to do the same for kexec.

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Tue, Jan 15, 2008 at 10:27:10AM -0500, Vivek Goyal wrote:
> On Mon, Jan 14, 2008 at 08:43:03AM -0500, Neil Horman wrote:
> > Hey all-
> > 	Regarding this bug:
> > 	http://bugzilla.kernel.org/show_bug.cgi?id=9641
> > 	I'd like to look into putting together a patch for it, and wanted to
> > solicit comments for the best way to go about doing it.  Currently I've got it
> > fixed up in the Red Hat tree by bumping COMMAND_LINE_SIZE to 2048 and
> > eliminating the reserved buffer of the x86_linux_faked_param_header, which works
> > well, but isn't backwards compatible as Bernhard pointed out.  Given that extra
> > constraint, I thought it woudl e best to unify the command line and reserved
> > buffers in x86_linux_faked_param_header to one contiguour 2048 byte block and
> > maintain a separate variable that defines the command line length based on a
> > parsing of the UTS_VERSION.  Does that sound reasonable to everyone, or is there
> > a better way that someone has in mind?
> > 
> 
> Hi Neil,
> 
> Looking at UTS_VERSION and then deciding the command line length seems
> ok. 
> 
> When I look at inclue/asm-x86/bootparam.h in kernel, area starting from
> 0x2d0 to all the way up to 4K has been reserved for e820 maps and EDD buf.
> 
> Does that mean newer boot loaders are putting command line outside of 
> 4K page and only putting the pointer to cmdline in 4K page. If that's the
> case then we might have to do the same for kexec.
> 

That would seem to be the case yes, although it appears the kernel still
supports the old boot protocol if you want to restrict the command line length
to the old 256 chars (see copy_boot_data)

Regards
Neil

> Thanks
> Vivek
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@redhat.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

On Tue, Jan 15, 2008 at 12:09:50PM -0500, Neil Horman wrote:
> On Tue, Jan 15, 2008 at 10:27:10AM -0500, Vivek Goyal wrote:
> > On Mon, Jan 14, 2008 at 08:43:03AM -0500, Neil Horman wrote:
> > > Hey all-
> > > 	Regarding this bug:
> > > 	http://bugzilla.kernel.org/show_bug.cgi?id=9641
> > > 	I'd like to look into putting together a patch for it, and wanted to
> > > solicit comments for the best way to go about doing it.  Currently I've got it
> > > fixed up in the Red Hat tree by bumping COMMAND_LINE_SIZE to 2048 and
> > > eliminating the reserved buffer of the x86_linux_faked_param_header, which works
> > > well, but isn't backwards compatible as Bernhard pointed out.  Given that extra
> > > constraint, I thought it woudl e best to unify the command line and reserved
> > > buffers in x86_linux_faked_param_header to one contiguour 2048 byte block and
> > > maintain a separate variable that defines the command line length based on a
> > > parsing of the UTS_VERSION.  Does that sound reasonable to everyone, or is there
> > > a better way that someone has in mind?
> > > 
> > 
> > Hi Neil,
> > 
> > Looking at UTS_VERSION and then deciding the command line length seems
> > ok. 
> > 
> > When I look at inclue/asm-x86/bootparam.h in kernel, area starting from
> > 0x2d0 to all the way up to 4K has been reserved for e820 maps and EDD buf.
> > 
> > Does that mean newer boot loaders are putting command line outside of 
> > 4K page and only putting the pointer to cmdline in 4K page. If that's the
> > case then we might have to do the same for kexec.
> > 
> 
> That would seem to be the case yes, although it appears the kernel still
> supports the old boot protocol if you want to restrict the command line length
> to the old 256 chars (see copy_boot_data)
> 

If that's the case then we probably should not be merging command_line
and reserved area. Instead we might have to put command line somewhere
else and pass the pointer to it in param page. If command line is with-in
256, then we can continue to pass it in param page. 

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Tue, Jan 15, 2008 at 12:37:53PM -0500, Vivek Goyal wrote:
> On Tue, Jan 15, 2008 at 12:09:50PM -0500, Neil Horman wrote:
> > On Tue, Jan 15, 2008 at 10:27:10AM -0500, Vivek Goyal wrote:
> > > On Mon, Jan 14, 2008 at 08:43:03AM -0500, Neil Horman wrote:
> > > > Hey all-
> > > > 	Regarding this bug:
> > > > 	http://bugzilla.kernel.org/show_bug.cgi?id=9641
> > > > 	I'd like to look into putting together a patch for it, and wanted to
> > > > solicit comments for the best way to go about doing it.  Currently I've got it
> > > > fixed up in the Red Hat tree by bumping COMMAND_LINE_SIZE to 2048 and
> > > > eliminating the reserved buffer of the x86_linux_faked_param_header, which works
> > > > well, but isn't backwards compatible as Bernhard pointed out.  Given that extra
> > > > constraint, I thought it woudl e best to unify the command line and reserved
> > > > buffers in x86_linux_faked_param_header to one contiguour 2048 byte block and
> > > > maintain a separate variable that defines the command line length based on a
> > > > parsing of the UTS_VERSION.  Does that sound reasonable to everyone, or is there
> > > > a better way that someone has in mind?
> > > > 
> > > 
> > > Hi Neil,
> > > 
> > > Looking at UTS_VERSION and then deciding the command line length seems
> > > ok. 
> > > 
> > > When I look at inclue/asm-x86/bootparam.h in kernel, area starting from
> > > 0x2d0 to all the way up to 4K has been reserved for e820 maps and EDD buf.
> > > 
> > > Does that mean newer boot loaders are putting command line outside of 
> > > 4K page and only putting the pointer to cmdline in 4K page. If that's the
> > > case then we might have to do the same for kexec.
> > > 
> > 
> > That would seem to be the case yes, although it appears the kernel still
> > supports the old boot protocol if you want to restrict the command line length
> > to the old 256 chars (see copy_boot_data)
> > 
> 
> If that's the case then we probably should not be merging command_line
> and reserved area. Instead we might have to put command line somewhere
> else and pass the pointer to it in param page. If command line is with-in
> 256, then we can continue to pass it in param page. 
> 
Actually, Vivek, I take back what I said (at least in part).  Looking at the
kernel side of this, it seems that the traditional COMMAND_LINE_SIZE has been
extended as well to 2048 bytes for i386/x86_64.  So unless I'm mistaken (and I
may be), it looks like we can either:

a) use the new protocol
OR
b) just gobble up the no-longer-existing reserved area which is now used fully
for command line 

Clearly the ability to do both would be good, but it would seem my earlier,
simple approach is workable.  Thoughts?

Regards
Neil

> Thanks
> Vivek
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

On Fri, Jan 25, 2008 at 07:35:58AM -0500, Neil Horman wrote:
> On Tue, Jan 15, 2008 at 12:37:53PM -0500, Vivek Goyal wrote:
> > On Tue, Jan 15, 2008 at 12:09:50PM -0500, Neil Horman wrote:
> > > On Tue, Jan 15, 2008 at 10:27:10AM -0500, Vivek Goyal wrote:
> > > > On Mon, Jan 14, 2008 at 08:43:03AM -0500, Neil Horman wrote:
> > > > > Hey all-
> > > > > 	Regarding this bug:
> > > > > 	http://bugzilla.kernel.org/show_bug.cgi?id=9641
> > > > > 	I'd like to look into putting together a patch for it, and wanted to
> > > > > solicit comments for the best way to go about doing it.  Currently I've got it
> > > > > fixed up in the Red Hat tree by bumping COMMAND_LINE_SIZE to 2048 and
> > > > > eliminating the reserved buffer of the x86_linux_faked_param_header, which works
> > > > > well, but isn't backwards compatible as Bernhard pointed out.  Given that extra
> > > > > constraint, I thought it woudl e best to unify the command line and reserved
> > > > > buffers in x86_linux_faked_param_header to one contiguour 2048 byte block and
> > > > > maintain a separate variable that defines the command line length based on a
> > > > > parsing of the UTS_VERSION.  Does that sound reasonable to everyone, or is there
> > > > > a better way that someone has in mind?
> > > > > 
> > > > 
> > > > Hi Neil,
> > > > 
> > > > Looking at UTS_VERSION and then deciding the command line length seems
> > > > ok. 
> > > > 
> > > > When I look at inclue/asm-x86/bootparam.h in kernel, area starting from
> > > > 0x2d0 to all the way up to 4K has been reserved for e820 maps and EDD buf.
> > > > 
> > > > Does that mean newer boot loaders are putting command line outside of 
> > > > 4K page and only putting the pointer to cmdline in 4K page. If that's the
> > > > case then we might have to do the same for kexec.
> > > > 
> > > 
> > > That would seem to be the case yes, although it appears the kernel still
> > > supports the old boot protocol if you want to restrict the command line length
> > > to the old 256 chars (see copy_boot_data)
> > > 
> > 
> > If that's the case then we probably should not be merging command_line
> > and reserved area. Instead we might have to put command line somewhere
> > else and pass the pointer to it in param page. If command line is with-in
> > 256, then we can continue to pass it in param page. 
> > 
> Actually, Vivek, I take back what I said (at least in part).  Looking at the
> kernel side of this, it seems that the traditional COMMAND_LINE_SIZE has been
> extended as well to 2048 bytes for i386/x86_64.  So unless I'm mistaken (and I
> may be), it looks like we can either:
> 
> a) use the new protocol
> OR
> b) just gobble up the no-longer-existing reserved area which is now used fully
> for command line 
> 
> Clearly the ability to do both would be good, but it would seem my earlier,
> simple approach is workable.  Thoughts?
> 
Hi Neil,

I had a closer look at the code and following are my thoughts.

If I look at the 2.6.24 x86 boot code, I think this code does not expect a
boot loader to put 2048 size command line in 4K page (so called zero page).
I think what it expects is that a boot loader puts command line somewhere
outside the zero page and just pass the pointer to that command line in zero
page.

So far kexec has been putting command line in zero page. I think it worked
because command line was small (256) and zero page had lots of free
reserved area. (E820MAX was 32). 

Now in latest kernel code E820MAX has been increased to 128 and I don't see
lot of free space in bootparam where we can put the 2048 size command line.

If we just continue to do what we are doing and just extend the command
line size to 2048 in kexec-tools, i think this will overlap with some other
area, either with EDD or E820 map etc and real mode code will overwrite part
of command line as passed by kexec, on some systems.

So I think we should modify kexec-tools and start putting the 2048
size command line outside the setup/zero page.

CCing HPA and Eric. They should be able to guide us better.

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[H. Peter Anvin]

Vivek Goyal wrote:
> Hi Neil,
> 
> I had a closer look at the code and following are my thoughts.
> 
> If I look at the 2.6.24 x86 boot code, I think this code does not expect a
> boot loader to put 2048 size command line in 4K page (so called zero page).
> I think what it expects is that a boot loader puts command line somewhere
> outside the zero page and just pass the pointer to that command line in zero
> page.

No, and it never has to the best of my knowledge.

> So far kexec has been putting command line in zero page. I think it worked
> because command line was small (256) and zero page had lots of free
> reserved area. (E820MAX was 32). 
> 
> Now in latest kernel code E820MAX has been increased to 128 and I don't see
> lot of free space in bootparam where we can put the 2048 size command line.
> 
> If we just continue to do what we are doing and just extend the command
> line size to 2048 in kexec-tools, i think this will overlap with some other
> area, either with EDD or E820 map etc and real mode code will overwrite part
> of command line as passed by kexec, on some systems.
> 
> So I think we should modify kexec-tools and start putting the 2048
> size command line outside the setup/zero page.
> 
> CCing HPA and Eric. They should be able to guide us better.

You should put it outside struct bootparams.  Furthermore, you should 
expect an increasing amount of information to be needed beyond struct 
bootparams in the future; the intent is to have a tagged linked list of 
expansion information in the future.

We are already out of e820 *and* EDD information space...

	-hpa

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Fri, Jan 25, 2008 at 07:44:29AM -0800, H. Peter Anvin wrote:
> Vivek Goyal wrote:
> >Hi Neil,
> >
> >I had a closer look at the code and following are my thoughts.
> >
> >If I look at the 2.6.24 x86 boot code, I think this code does not expect a
> >boot loader to put 2048 size command line in 4K page (so called zero page).
> >I think what it expects is that a boot loader puts command line somewhere
> >outside the zero page and just pass the pointer to that command line in 
> >zero
> >page.
> 
> No, and it never has to the best of my knowledge.
> 
> >So far kexec has been putting command line in zero page. I think it worked
> >because command line was small (256) and zero page had lots of free
> >reserved area. (E820MAX was 32). 
> >
> >Now in latest kernel code E820MAX has been increased to 128 and I don't see
> >lot of free space in bootparam where we can put the 2048 size command line.
> >
> >If we just continue to do what we are doing and just extend the command
> >line size to 2048 in kexec-tools, i think this will overlap with some other
> >area, either with EDD or E820 map etc and real mode code will overwrite 
> >part
> >of command line as passed by kexec, on some systems.
> >
> >So I think we should modify kexec-tools and start putting the 2048
> >size command line outside the setup/zero page.
> >
> >CCing HPA and Eric. They should be able to guide us better.
> 
> You should put it outside struct bootparams.  Furthermore, you should 
> expect an increasing amount of information to be needed beyond struct 
> bootparams in the future; the intent is to have a tagged linked list of 
> expansion information in the future.
> 
> We are already out of e820 *and* EDD information space...
> 

Actually, unless I'm mistaken, thats what horms tree does already.  It was
different in Erics tree IIRC, but in horms tree we just find an open space in
the reserved area to store our parameter header adn point to it, from
elf_x86_64_load:
....
struct x86_linux_faked_param_header *hdr;
                unsigned long param_base;
                const unsigned char *ramdisk_buf;
                off_t ramdisk_length;
                struct entry64_regs regs;
                int rc=0;

                /* Get the linux parameter header */
                hdr = xmalloc(sizeof(*hdr));
                param_base = add_buffer(info, hdr, sizeof(*hdr), sizeof(*hdr),
                        16, 0, max_addr, 1);

....

And then we just point to it inside setup_linux_bootloader_parameters:
...
if (real_mode->protocol_version >= 0x0202) {
                real_mode->cmd_line_ptr = real_mode_base + cmdline_offset;
        }
....

So it seems, given that we already don't put it inside struct bootparam, we
should be able to just expand it, yes?


Regards
Neil


-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@redhat.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[H. Peter Anvin]

Neil Horman wrote:
> 
> And then we just point to it inside setup_linux_bootloader_parameters:
> ...
> if (real_mode->protocol_version >= 0x0202) {
>                 real_mode->cmd_line_ptr = real_mode_base + cmdline_offset;
>         }
> ....
> 
> So it seems, given that we already don't put it inside struct bootparam, we
> should be able to just expand it, yes?

Yup, you should.

	-hpa


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Fri, Jan 25, 2008 at 11:54:03AM -0800, H. Peter Anvin wrote:
> Neil Horman wrote:
> >
> >And then we just point to it inside setup_linux_bootloader_parameters:
> >...
> >if (real_mode->protocol_version >= 0x0202) {
> >                real_mode->cmd_line_ptr = real_mode_base + cmdline_offset;
> >        }
> >....
> >
> >So it seems, given that we already don't put it inside struct bootparam, we
> >should be able to just expand it, yes?
> 
> Yup, you should.
> 
Woo hoo! I understood how this crap works!  Yea!  Thanks Peter :)
Neil

> 	-hpa
> 

-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@redhat.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

On Fri, Jan 25, 2008 at 02:50:58PM -0500, Neil Horman wrote:
> On Fri, Jan 25, 2008 at 07:44:29AM -0800, H. Peter Anvin wrote:
> > Vivek Goyal wrote:
> > >Hi Neil,
> > >
> > >I had a closer look at the code and following are my thoughts.
> > >
> > >If I look at the 2.6.24 x86 boot code, I think this code does not expect a
> > >boot loader to put 2048 size command line in 4K page (so called zero page).
> > >I think what it expects is that a boot loader puts command line somewhere
> > >outside the zero page and just pass the pointer to that command line in 
> > >zero
> > >page.
> > 
> > No, and it never has to the best of my knowledge.
> > 
> > >So far kexec has been putting command line in zero page. I think it worked
> > >because command line was small (256) and zero page had lots of free
> > >reserved area. (E820MAX was 32). 
> > >
> > >Now in latest kernel code E820MAX has been increased to 128 and I don't see
> > >lot of free space in bootparam where we can put the 2048 size command line.
> > >
> > >If we just continue to do what we are doing and just extend the command
> > >line size to 2048 in kexec-tools, i think this will overlap with some other
> > >area, either with EDD or E820 map etc and real mode code will overwrite 
> > >part
> > >of command line as passed by kexec, on some systems.
> > >
> > >So I think we should modify kexec-tools and start putting the 2048
> > >size command line outside the setup/zero page.
> > >
> > >CCing HPA and Eric. They should be able to guide us better.
> > 
> > You should put it outside struct bootparams.  Furthermore, you should 
> > expect an increasing amount of information to be needed beyond struct 
> > bootparams in the future; the intent is to have a tagged linked list of 
> > expansion information in the future.
> > 
> > We are already out of e820 *and* EDD information space...
> > 
> 
> Actually, unless I'm mistaken, thats what horms tree does already.  It was
> different in Erics tree IIRC, but in horms tree we just find an open space in
> the reserved area to store our parameter header adn point to it, from
> elf_x86_64_load:
> ....
> struct x86_linux_faked_param_header *hdr;
>                 unsigned long param_base;
>                 const unsigned char *ramdisk_buf;
>                 off_t ramdisk_length;
>                 struct entry64_regs regs;
>                 int rc=0;
> 
>                 /* Get the linux parameter header */
>                 hdr = xmalloc(sizeof(*hdr));
>                 param_base = add_buffer(info, hdr, sizeof(*hdr), sizeof(*hdr),
>                         16, 0, max_addr, 1);
> 
> ....
> 
> And then we just point to it inside setup_linux_bootloader_parameters:
> ...
> if (real_mode->protocol_version >= 0x0202) {
>                 real_mode->cmd_line_ptr = real_mode_base + cmdline_offset;
>         }
> ....
> 
> So it seems, given that we already don't put it inside struct bootparam, we
> should be able to just expand it, yes?
> 
> 

Hi Neil,

IIUC, cmdline_offset in this case is nothing but pointer to
command_line[COMMAND_LINE_SIZE] array inside x86_linux_faked_param_header.
To me, x86_linux_faked_param_header is just a 4K page which is essentially
the zero page or struct bootparam.  That means even in Horms's tree, we
are putting command line inside struct bootparam and we should not be
doing that. 

I guess we might be able to extend the size of
x86_linux_faked_param_header to say 8K and then we can put command line
in the higher 4K. First 4K will be the zero page/struct bootparam.

May be we can import the definition of bootparam from latest kernel and
make new fake param header definition like this.

struct x86_linux_faked_param_header {
        struct boot_param boot_param;      /* 0x00 - 0x1000*/
        uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x1000 - 0x1800 */
        uint8_t reserved18[2048];               /* 0x1800 - 0x2000 */
};

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Fri, Jan 25, 2008 at 03:13:05PM -0500, Vivek Goyal wrote:
> On Fri, Jan 25, 2008 at 02:50:58PM -0500, Neil Horman wrote:
> > On Fri, Jan 25, 2008 at 07:44:29AM -0800, H. Peter Anvin wrote:
> > > Vivek Goyal wrote:
> > > >Hi Neil,
> > > >
> > > >I had a closer look at the code and following are my thoughts.
> > > >
> > > >If I look at the 2.6.24 x86 boot code, I think this code does not expect a
> > > >boot loader to put 2048 size command line in 4K page (so called zero page).
> > > >I think what it expects is that a boot loader puts command line somewhere
> > > >outside the zero page and just pass the pointer to that command line in 
> > > >zero
> > > >page.
> > > 
> > > No, and it never has to the best of my knowledge.
> > > 
> > > >So far kexec has been putting command line in zero page. I think it worked
> > > >because command line was small (256) and zero page had lots of free
> > > >reserved area. (E820MAX was 32). 
> > > >
> > > >Now in latest kernel code E820MAX has been increased to 128 and I don't see
> > > >lot of free space in bootparam where we can put the 2048 size command line.
> > > >
> > > >If we just continue to do what we are doing and just extend the command
> > > >line size to 2048 in kexec-tools, i think this will overlap with some other
> > > >area, either with EDD or E820 map etc and real mode code will overwrite 
> > > >part
> > > >of command line as passed by kexec, on some systems.
> > > >
> > > >So I think we should modify kexec-tools and start putting the 2048
> > > >size command line outside the setup/zero page.
> > > >
> > > >CCing HPA and Eric. They should be able to guide us better.
> > > 
> > > You should put it outside struct bootparams.  Furthermore, you should 
> > > expect an increasing amount of information to be needed beyond struct 
> > > bootparams in the future; the intent is to have a tagged linked list of 
> > > expansion information in the future.
> > > 
> > > We are already out of e820 *and* EDD information space...
> > > 
> > 
> > Actually, unless I'm mistaken, thats what horms tree does already.  It was
> > different in Erics tree IIRC, but in horms tree we just find an open space in
> > the reserved area to store our parameter header adn point to it, from
> > elf_x86_64_load:
> > ....
> > struct x86_linux_faked_param_header *hdr;
> >                 unsigned long param_base;
> >                 const unsigned char *ramdisk_buf;
> >                 off_t ramdisk_length;
> >                 struct entry64_regs regs;
> >                 int rc=0;
> > 
> >                 /* Get the linux parameter header */
> >                 hdr = xmalloc(sizeof(*hdr));
> >                 param_base = add_buffer(info, hdr, sizeof(*hdr), sizeof(*hdr),
> >                         16, 0, max_addr, 1);
> > 
> > ....
> > 
> > And then we just point to it inside setup_linux_bootloader_parameters:
> > ...
> > if (real_mode->protocol_version >= 0x0202) {
> >                 real_mode->cmd_line_ptr = real_mode_base + cmdline_offset;
> >         }
> > ....
> > 
> > So it seems, given that we already don't put it inside struct bootparam, we
> > should be able to just expand it, yes?
> > 
> > 
> 
> Hi Neil,
> 
> IIUC, cmdline_offset in this case is nothing but pointer to
> command_line[COMMAND_LINE_SIZE] array inside x86_linux_faked_param_header.
> To me, x86_linux_faked_param_header is just a 4K page which is essentially
> the zero page or struct bootparam.  That means even in Horms's tree, we
> are putting command line inside struct bootparam and we should not be
> doing that. 
> 

Ok, I see what your saying, since We supercede the x86_linux_param_header struct
to append the command line contents at the end, we're still living down in the
zero page.

Looking at bootparam.h for x86 in the latest linus kernel though, we still seem
to be reasonably in line, struct setup_header still matches
x86_linux_param_header almost exactly.  We're just missing a few parameters at
the end.

> I guess we might be able to extend the size of
> x86_linux_faked_param_header to say 8K and then we can put command line
> in the higher 4K. First 4K will be the zero page/struct bootparam.
> 
That makes good sense to me, at least for now.  Eventually I guess we should do
a separate malloc & add_buffer for the command line size, but just adding an
extra reserved section to bump the faked_param_header up to the next page would
be a good interim solution, plus it lets us add in the cmdline_length and other
missing parameters from the setup header.  I'll get on writing this on monday.

> May be we can import the definition of bootparam from latest kernel and
> make new fake param header definition like this.
> 
I think youre referring to setup_header (from above).  We're very close to that
already, but I think you're right, its a good thing to do.  I'll incorporate
this into my patch.

Regards
Neil

> struct x86_linux_faked_param_header {
>         struct boot_param boot_param;      /* 0x00 - 0x1000*/
>         uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x1000 - 0x1800 */
>         uint8_t reserved18[2048];               /* 0x1800 - 0x2000 */
> };
> 
T
> Thanks
> Vivek

-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@redhat.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

Patch to clean up kexec-tools command line encoding.  It does four things:

1) Move the command line out of the zero page, as per Viveks suggestion.  New
padding scheme places the command line starting at 4096 bytes

2) Increase command line length to support maximum size of 2048 bytes

3) Pull in new variables from the latest kernels struct setup_header

4) Where appropriate (currently only in bzImage_load) check the cmdline_size in
setup header to ensure that cmdline_size isn't being violated

Tested by me, with successful results.

Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 include/x86/x86-linux.h         |   19 +++++++++++++------
 kexec/arch/i386/kexec-bzImage.c |    7 +++++++
 2 files changed, 20 insertions(+), 6 deletions(-)


diff --git a/include/x86/x86-linux.h b/include/x86/x86-linux.h
index afe66bd..0794fe0 100644
--- a/include/x86/x86-linux.h
+++ b/include/x86/x86-linux.h
@@ -144,18 +144,22 @@ struct x86_linux_param_header {
 	/* 2.04+ */
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  reserved15[0x2d0 - 0x235];	/* 0x230 */
+	uint8_t  reserved15[3];			/* 0x237 */
+	uint32_t cmdline_size;			/* 0x23B */
+	uint32_t hardware_subarch;		/* 0x23F */
+	uint64_t hardware_subarch_data;		/* 0x247 */
+	uint8_t  reserved16[0x2d0 - 0x248];	/* 0x248 */
 #endif
 	struct e820entry e820_map[E820MAX];	/* 0x2d0 */
 						/* 0x550 */
-#define COMMAND_LINE_SIZE 256
+#define COMMAND_LINE_SIZE 2048 
 };
 
 struct x86_linux_faked_param_header {
 	struct x86_linux_param_header hdr;	/* 0x00 */
-	uint8_t reserved16[688];		/* 0x550 */
-	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x800 */
-	uint8_t reserved17[1792];		/* 0x900 - 0x1000 */
+	uint8_t reserved16[0xab0];		/* 0x550 */
+	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x1000 */
+	uint8_t reserved17[0x200];		/* 0x1800 - 0x2000 */
 };
 
 struct x86_linux_header {
@@ -206,7 +210,10 @@ struct x86_linux_header {
 #else
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  tail[32*1024 - 0x235];		/* 0x230 */
+	uint32_t cmdline_size;                  /* 0x23B */
+	uint32_t hardware_subarch;              /* 0x23F */
+	uint64_t hardware_subarch_data;         /* 0x247 */
+	uint8_t  tail[32*1024 - 0x248];		/* 0x248 */
 #endif
 } PACKED;
 
diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
index 8fde799..4f2a294 100644
--- a/kexec/arch/i386/kexec-bzImage.c
+++ b/kexec/arch/i386/kexec-bzImage.c
@@ -134,6 +134,13 @@ int do_bzImage_load(struct kexec_info *info,
 		return -1;
 	}
 
+	if (setup_header.protocol_version >= 0x0206) {
+		if (command_line_len > setup_header.cmdline_size) {
+			dbgprintf("Kernel command line too long for kernel!\n");
+			return -1;
+		}
+	}
+
 	if (setup_header.protocol_version >= 0x0205) {
 		relocatable_kernel = setup_header.relocatable_kernel;
 		dbgprintf("bzImage is relocatable\n");
-- 
/****************************************************
 * Neil Horman <nhorman@tuxdriver.com>
 * Software Engineer, Red Hat
 ****************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

On Mon, Jan 28, 2008 at 12:08:11PM -0500, Neil Horman wrote:
> Patch to clean up kexec-tools command line encoding.  It does four things:
> 
> 1) Move the command line out of the zero page, as per Viveks suggestion.  New
> padding scheme places the command line starting at 4096 bytes
> 
> 2) Increase command line length to support maximum size of 2048 bytes
> 
> 3) Pull in new variables from the latest kernels struct setup_header
> 
> 4) Where appropriate (currently only in bzImage_load) check the cmdline_size in
> setup header to ensure that cmdline_size isn't being violated
> 
> Tested by me, with successful results.
> 
> Regards
> Neil
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 

Hi Neil,

Patch looks good. Some minor nits follow.

> 
>  include/x86/x86-linux.h         |   19 +++++++++++++------
>  kexec/arch/i386/kexec-bzImage.c |    7 +++++++
>  2 files changed, 20 insertions(+), 6 deletions(-)
> 
> 
> diff --git a/include/x86/x86-linux.h b/include/x86/x86-linux.h
> index afe66bd..0794fe0 100644
> --- a/include/x86/x86-linux.h
> +++ b/include/x86/x86-linux.h
> @@ -144,18 +144,22 @@ struct x86_linux_param_header {
>  	/* 2.04+ */
>  	uint32_t kernel_alignment;		/* 0x230 */
>  	uint8_t  relocatable_kernel;		/* 0x234 */
> -	uint8_t  reserved15[0x2d0 - 0x235];	/* 0x230 */
> +	uint8_t  reserved15[3];			/* 0x237 */
> +	uint32_t cmdline_size;			/* 0x23B */
> +	uint32_t hardware_subarch;		/* 0x23F */
> +	uint64_t hardware_subarch_data;		/* 0x247 */

In general convetion for /* xxx */ seems to be that xxx represents the
offset where that data structure starts. We might want to maintain that.

> +	uint8_t  reserved16[0x2d0 - 0x248];	/* 0x248 */
>  #endif
>  	struct e820entry e820_map[E820MAX];	/* 0x2d0 */
>  						/* 0x550 */
> -#define COMMAND_LINE_SIZE 256
> +#define COMMAND_LINE_SIZE 2048 
>  };
>  
>  struct x86_linux_faked_param_header {
>  	struct x86_linux_param_header hdr;	/* 0x00 */
> -	uint8_t reserved16[688];		/* 0x550 */
> -	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x800 */
> -	uint8_t reserved17[1792];		/* 0x900 - 0x1000 */
> +	uint8_t reserved16[0xab0];		/* 0x550 */

reserved16 is now already used up in x86_linux_param_header. We might
want to bump up the reservation number here.

Ideally I would have liked to put all the 4K page in
x86_linux_param_header and replace that definition with struct bootparam.
But I think thats' fine for the time being. We might want to do that in 
future as reading and mapping the code to kernel boot protocol becomes
easy.

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Mon, Jan 28, 2008 at 02:37:41PM -0500, Vivek Goyal wrote:
> On Mon, Jan 28, 2008 at 12:08:11PM -0500, Neil Horman wrote:
> > Patch to clean up kexec-tools command line encoding.  It does four things:
> > 
> > 1) Move the command line out of the zero page, as per Viveks suggestion.  New
> > padding scheme places the command line starting at 4096 bytes
> > 
> > 2) Increase command line length to support maximum size of 2048 bytes
> > 
> > 3) Pull in new variables from the latest kernels struct setup_header
> > 
> > 4) Where appropriate (currently only in bzImage_load) check the cmdline_size in
> > setup header to ensure that cmdline_size isn't being violated
> > 
> > Tested by me, with successful results.
> > 
> > Regards
> > Neil
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > 
> 
> Hi Neil,
> 
> Patch looks good. Some minor nits follow.
> 
<snip>

> In general convetion for /* xxx */ seems to be that xxx represents the
> offset where that data structure starts. We might want to maintain that.
> 
Copy that.  Corrected.

<snip>
> reserved16 is now already used up in x86_linux_param_header. We might
> want to bump up the reservation number here.
> 
Yeah, good point.  Corrected. Thanks

> Ideally I would have liked to put all the 4K page in
> x86_linux_param_header and replace that definition with struct bootparam.
> But I think thats' fine for the time being. We might want to do that in 
> future as reading and mapping the code to kernel boot protocol becomes
> easy.
> 
Yeah, I had considered simply removing the x86_linux_param_header and just
inluding bootparam.h, but that becomes tricky if you're compiling for alternate
kernels, so I just left well enough alone.

New Patch attached, with youre notes incorporated.  Patch does 4 things:

1) Move the command line out of the zero page, as per Viveks suggestion.  New
   padding scheme places the command line starting at 4096 bytes

2) Increase command line length to support maximum size of 2048 bytes

3) Pull in new variables from the latest kernels struct setup_header

4) Where appropriate (currently only in bzImage_load) check the cmdline_size in
   setup header to ensure that cmdline_size isn't being violated


Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 include/x86/x86-linux.h         |   19 +++++++++++++------
 kexec/arch/i386/kexec-bzImage.c |    7 +++++++
 2 files changed, 20 insertions(+), 6 deletions(-)


diff --git a/include/x86/x86-linux.h b/include/x86/x86-linux.h
index afe66bd..e75689c 100644
--- a/include/x86/x86-linux.h
+++ b/include/x86/x86-linux.h
@@ -144,18 +144,22 @@ struct x86_linux_param_header {
 	/* 2.04+ */
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  reserved15[0x2d0 - 0x235];	/* 0x230 */
+	uint8_t  reserved15[3];			/* 0x235 */
+	uint32_t cmdline_size;			/* 0x238 */
+	uint32_t hardware_subarch;		/* 0x23C */
+	uint64_t hardware_subarch_data;		/* 0x240 */
+	uint8_t  reserved16[0x2d0 - 0x248];	/* 0x248 */
 #endif
 	struct e820entry e820_map[E820MAX];	/* 0x2d0 */
 						/* 0x550 */
-#define COMMAND_LINE_SIZE 256
+#define COMMAND_LINE_SIZE 2048 
 };
 
 struct x86_linux_faked_param_header {
 	struct x86_linux_param_header hdr;	/* 0x00 */
-	uint8_t reserved16[688];		/* 0x550 */
-	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x800 */
-	uint8_t reserved17[1792];		/* 0x900 - 0x1000 */
+	uint8_t reserved17[0xab0];		/* 0x550 */
+	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x1000 */
+	uint8_t reserved18[0x200];		/* 0x1800 - 0x2000 */
 };
 
 struct x86_linux_header {
@@ -206,7 +210,10 @@ struct x86_linux_header {
 #else
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  tail[32*1024 - 0x235];		/* 0x230 */
+	uint32_t cmdline_size;                  /* 0x235 */
+	uint32_t hardware_subarch;              /* 0x239 */
+	uint64_t hardware_subarch_data;         /* 0x23D */
+	uint8_t  tail[32*1024 - 0x245];		/* 0x245 */
 #endif
 } PACKED;
 
diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
index 8fde799..4f2a294 100644
--- a/kexec/arch/i386/kexec-bzImage.c
+++ b/kexec/arch/i386/kexec-bzImage.c
@@ -134,6 +134,13 @@ int do_bzImage_load(struct kexec_info *info,
 		return -1;
 	}
 
+	if (setup_header.protocol_version >= 0x0206) {
+		if (command_line_len > setup_header.cmdline_size) {
+			dbgprintf("Kernel command line too long for kernel!\n");
+			return -1;
+		}
+	}
+
 	if (setup_header.protocol_version >= 0x0205) {
 		relocatable_kernel = setup_header.relocatable_kernel;
 		dbgprintf("bzImage is relocatable\n");
/****************************************************
 * Neil Horman <nhorman@tuxdriver.com>
 * Software Engineer, Red Hat
 ****************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

[..]
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> 
>  include/x86/x86-linux.h         |   19 +++++++++++++------
>  kexec/arch/i386/kexec-bzImage.c |    7 +++++++
>  2 files changed, 20 insertions(+), 6 deletions(-)
> 
> 
> diff --git a/include/x86/x86-linux.h b/include/x86/x86-linux.h
> index afe66bd..e75689c 100644
> --- a/include/x86/x86-linux.h
> +++ b/include/x86/x86-linux.h
> @@ -144,18 +144,22 @@ struct x86_linux_param_header {
>  	/* 2.04+ */
>  	uint32_t kernel_alignment;		/* 0x230 */
>  	uint8_t  relocatable_kernel;		/* 0x234 */
> -	uint8_t  reserved15[0x2d0 - 0x235];	/* 0x230 */
> +	uint8_t  reserved15[3];			/* 0x235 */
> +	uint32_t cmdline_size;			/* 0x238 */
> +	uint32_t hardware_subarch;		/* 0x23C */
> +	uint64_t hardware_subarch_data;		/* 0x240 */
> +	uint8_t  reserved16[0x2d0 - 0x248];	/* 0x248 */
>  #endif
>  	struct e820entry e820_map[E820MAX];	/* 0x2d0 */
>  						/* 0x550 */
> -#define COMMAND_LINE_SIZE 256
> +#define COMMAND_LINE_SIZE 2048 
>  };
>  
>  struct x86_linux_faked_param_header {
>  	struct x86_linux_param_header hdr;	/* 0x00 */
> -	uint8_t reserved16[688];		/* 0x550 */
> -	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x800 */
> -	uint8_t reserved17[1792];		/* 0x900 - 0x1000 */
> +	uint8_t reserved17[0xab0];		/* 0x550 */
> +	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x1000 */
> +	uint8_t reserved18[0x200];		/* 0x1800 - 0x2000 */
>  };
>  
>  struct x86_linux_header {
> @@ -206,7 +210,10 @@ struct x86_linux_header {
>  #else
>  	uint32_t kernel_alignment;		/* 0x230 */
>  	uint8_t  relocatable_kernel;		/* 0x234 */
> -	uint8_t  tail[32*1024 - 0x235];		/* 0x230 */
> +	uint32_t cmdline_size;                  /* 0x235 */
> +	uint32_t hardware_subarch;              /* 0x239 */
> +	uint64_t hardware_subarch_data;         /* 0x23D */

We need a padding of 3 bytes here too between relocatable kernel and
cmdline_size, in the same way as x86_linux_param_header?

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Mon, Jan 28, 2008 at 03:20:41PM -0500, Vivek Goyal wrote:
> >  
> >  struct x86_linux_header {
> > @@ -206,7 +210,10 @@ struct x86_linux_header {
> >  #else
> >  	uint32_t kernel_alignment;		/* 0x230 */
> >  	uint8_t  relocatable_kernel;		/* 0x234 */
> > -	uint8_t  tail[32*1024 - 0x235];		/* 0x230 */
> > +	uint32_t cmdline_size;                  /* 0x235 */
> > +	uint32_t hardware_subarch;              /* 0x239 */
> > +	uint64_t hardware_subarch_data;         /* 0x23D */
> 
> We need a padding of 3 bytes here too between relocatable kernel and
> cmdline_size, in the same way as x86_linux_param_header?
> 

Oh, good catch, yes we do need that.  I expect it passed my testing because the
garbage that wound up in cmdline_size was big enough to pass the check in
do_bzImage_load.

New patch, same summary as before, with the above correction

Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 include/x86/x86-linux.h         |   20 ++++++++++++++------
 kexec/arch/i386/kexec-bzImage.c |    7 +++++++
 2 files changed, 21 insertions(+), 6 deletions(-)


diff --git a/include/x86/x86-linux.h b/include/x86/x86-linux.h
index afe66bd..6e4f984 100644
--- a/include/x86/x86-linux.h
+++ b/include/x86/x86-linux.h
@@ -144,18 +144,22 @@ struct x86_linux_param_header {
 	/* 2.04+ */
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  reserved15[0x2d0 - 0x235];	/* 0x230 */
+	uint8_t  reserved15[3];			/* 0x235 */
+	uint32_t cmdline_size;			/* 0x238 */
+	uint32_t hardware_subarch;		/* 0x23C */
+	uint64_t hardware_subarch_data;		/* 0x240 */
+	uint8_t  reserved16[0x2d0 - 0x248];	/* 0x248 */
 #endif
 	struct e820entry e820_map[E820MAX];	/* 0x2d0 */
 						/* 0x550 */
-#define COMMAND_LINE_SIZE 256
+#define COMMAND_LINE_SIZE 2048 
 };
 
 struct x86_linux_faked_param_header {
 	struct x86_linux_param_header hdr;	/* 0x00 */
-	uint8_t reserved16[688];		/* 0x550 */
-	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x800 */
-	uint8_t reserved17[1792];		/* 0x900 - 0x1000 */
+	uint8_t reserved17[0xab0];		/* 0x550 */
+	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x1000 */
+	uint8_t reserved18[0x200];		/* 0x1800 - 0x2000 */
 };
 
 struct x86_linux_header {
@@ -206,7 +210,11 @@ struct x86_linux_header {
 #else
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  tail[32*1024 - 0x235];		/* 0x230 */
+	uint8_t  reserved6[3];			/* 0x235 */
+	uint32_t cmdline_size;                  /* 0x238 */
+	uint32_t hardware_subarch;              /* 0x23C */
+	uint64_t hardware_subarch_data;         /* 0x240 */
+	uint8_t  tail[32*1024 - 0x248];		/* 0x248 */
 #endif
 } PACKED;
 
diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
index 8fde799..4f2a294 100644
--- a/kexec/arch/i386/kexec-bzImage.c
+++ b/kexec/arch/i386/kexec-bzImage.c
@@ -134,6 +134,13 @@ int do_bzImage_load(struct kexec_info *info,
 		return -1;
 	}
 
+	if (setup_header.protocol_version >= 0x0206) {
+		if (command_line_len > setup_header.cmdline_size) {
+			dbgprintf("Kernel command line too long for kernel!\n");
+			return -1;
+		}
+	}
+
 	if (setup_header.protocol_version >= 0x0205) {
 		relocatable_kernel = setup_header.relocatable_kernel;
 		dbgprintf("bzImage is relocatable\n");
-- 
/****************************************************
 * Neil Horman <nhorman@tuxdriver.com>
 * Software Engineer, Red Hat
 ****************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

On Mon, Jan 28, 2008 at 03:53:24PM -0500, Neil Horman wrote:
> On Mon, Jan 28, 2008 at 03:20:41PM -0500, Vivek Goyal wrote:
> > >  
> > >  struct x86_linux_header {
> > > @@ -206,7 +210,10 @@ struct x86_linux_header {
> > >  #else
> > >  	uint32_t kernel_alignment;		/* 0x230 */
> > >  	uint8_t  relocatable_kernel;		/* 0x234 */
> > > -	uint8_t  tail[32*1024 - 0x235];		/* 0x230 */
> > > +	uint32_t cmdline_size;                  /* 0x235 */
> > > +	uint32_t hardware_subarch;              /* 0x239 */
> > > +	uint64_t hardware_subarch_data;         /* 0x23D */
> > 
> > We need a padding of 3 bytes here too between relocatable kernel and
> > cmdline_size, in the same way as x86_linux_param_header?
> > 
> 
> Oh, good catch, yes we do need that.  I expect it passed my testing because the
> garbage that wound up in cmdline_size was big enough to pass the check in
> do_bzImage_load.
> 
> New patch, same summary as before, with the above correction
> 
> Regards
> Neil
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> 

Thanks. Looks good to me.

Regards
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Bernhard Walle]

* Neil Horman <nhorman@tuxdriver.com> [2008-01-28 21:53]:
>  		return -1;
>  	}
>  
> +	if (setup_header.protocol_version >= 0x0206) {
> +		if (command_line_len > setup_header.cmdline_size) {
> +			dbgprintf("Kernel command line too long for kernel!\n");
> +			return -1;
> +		}
> +	}
> +
>  	if (setup_header.protocol_version >= 0x0205) {
>  		relocatable_kernel = setup_header.relocatable_kernel;
>  		dbgprintf("bzImage is relocatable\n");

I know that there was a kernel release with 2048 _and_ still the old
boot protocol, but wouldn't it be better to warn the user if the size
is beyond 256 and the old kernel is used? I think new kexec-tools
should still support old kernels without problems ...



        Bernhard

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Mon, Jan 28, 2008 at 10:29:10PM +0100, Bernhard Walle wrote:
> * Neil Horman <nhorman@tuxdriver.com> [2008-01-28 21:53]:
> >  		return -1;
> >  	}
> >  
> > +	if (setup_header.protocol_version >= 0x0206) {
> > +		if (command_line_len > setup_header.cmdline_size) {
> > +			dbgprintf("Kernel command line too long for kernel!\n");
> > +			return -1;
> > +		}
> > +	}
> > +
> >  	if (setup_header.protocol_version >= 0x0205) {
> >  		relocatable_kernel = setup_header.relocatable_kernel;
> >  		dbgprintf("bzImage is relocatable\n");
> 
> I know that there was a kernel release with 2048 _and_ still the old
> boot protocol, but wouldn't it be better to warn the user if the size
> is beyond 256 and the old kernel is used? I think new kexec-tools
> should still support old kernels without problems ...
> 

I don't know how important that really is, but I don't see a particular problem 
with it either.  From my reading of i386/boot.txt, versions prior to boot 
protocol 2.02 only supported a 256 bytes command line patch, so what if we just 
add an extra check in do_bzImage_load.  If the protocol version of the boot 
header is lexx than 0x0202, then we fail if the command line length is more than 
256 bytes.  Note there are two other locations where we use a linux boot 
protocol header, but they are both constructed heders, not read headers, and use 
protocol version 2.03, which support 2048 byte command lines.

So, patch to clean up command line header construction in kexec.  Does 5 
things:

1) moves command line out of the zero page (struct bootparam)

2) extends command line length to support 2K command lines

3) adds a check to ensure that command line length is reasonably sized for new 
boot protocols

4) adds a check to ensure that command line length is reasonably sized for old 
boot protocols

5) imports variables from latest struct setup_header in kernel bootparams.h

Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 include/x86/x86-linux.h         |   20 ++++++++++++++------
 kexec/arch/i386/kexec-bzImage.c |   14 ++++++++++++++
 2 files changed, 28 insertions(+), 6 deletions(-)


diff --git a/include/x86/x86-linux.h b/include/x86/x86-linux.h
index afe66bd..4f3507e 100644
--- a/include/x86/x86-linux.h
+++ b/include/x86/x86-linux.h
@@ -144,18 +144,22 @@ struct x86_linux_param_header {
 	/* 2.04+ */
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  reserved15[0x2d0 - 0x235];	/* 0x230 */
+	uint8_t  reserved15[3];			/* 0x235 */
+	uint32_t cmdline_size;			/* 0x238 */
+	uint32_t hardware_subarch;		/* 0x23C */
+	uint64_t hardware_subarch_data;		/* 0x240 */
+	uint8_t  reserved16[0x2d0 - 0x248];	/* 0x248 */
 #endif
 	struct e820entry e820_map[E820MAX];	/* 0x2d0 */
 						/* 0x550 */
-#define COMMAND_LINE_SIZE 256
+#define COMMAND_LINE_SIZE 2048 
 };
 
 struct x86_linux_faked_param_header {
 	struct x86_linux_param_header hdr;	/* 0x00 */
-	uint8_t reserved16[688];		/* 0x550 */
-	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x800 */
-	uint8_t reserved17[1792];		/* 0x900 - 0x1000 */
+	uint8_t reserved17[0xab0];		/* 0x550 */
+	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x1000 */
+	uint8_t reserved18[0x200];		/* 0x1800 - 0x2000 */
 };
 
 struct x86_linux_header {
@@ -206,7 +210,11 @@ struct x86_linux_header {
 #else
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  tail[32*1024 - 0x235];		/* 0x230 */
+	uint8_t  reserved6[3];			/* 0x235 */
+	uint32_t cmdline_size;                  /* 0x238 */
+	uint32_t hardware_subarch;              /* 0x23C */
+	uint64_t hardware_subarch_data;         /* 0x240 */
+	uint8_t  tail[32*1024 - 0x248];		/* 0x248 */
 #endif
 } PACKED;
 
diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
index 8fde799..6b1d818 100644
--- a/kexec/arch/i386/kexec-bzImage.c
+++ b/kexec/arch/i386/kexec-bzImage.c
@@ -134,6 +134,20 @@ int do_bzImage_load(struct kexec_info *info,
 		return -1;
 	}
 
+	if (setup_header.protocol_version >= 0x0206) {
+		if (command_line_len > setup_header.cmdline_size) {
+			dbgprintf("Kernel command line too long for kernel!\n");
+			return -1;
+		}
+	}
+
+	if (setup_header.protocol_version < 0x0202) {
+		if (command_line_len > 256) {
+			dbgprintf("Kernel only supports 256 byte command line!\n");
+			return -1;
+		}
+	}
+
 	if (setup_header.protocol_version >= 0x0205) {
 		relocatable_kernel = setup_header.relocatable_kernel;
 		dbgprintf("bzImage is relocatable\n");

-- 
/****************************************************
 * Neil Horman <nhorman@tuxdriver.com>
 * Software Engineer, Red Hat
 ****************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

On Mon, Jan 28, 2008 at 08:01:22PM -0500, Neil Horman wrote:
> On Mon, Jan 28, 2008 at 10:29:10PM +0100, Bernhard Walle wrote:
> > * Neil Horman <nhorman@tuxdriver.com> [2008-01-28 21:53]:
> > >  		return -1;
> > >  	}
> > >  
> > > +	if (setup_header.protocol_version >= 0x0206) {
> > > +		if (command_line_len > setup_header.cmdline_size) {
> > > +			dbgprintf("Kernel command line too long for kernel!\n");
> > > +			return -1;
> > > +		}
> > > +	}
> > > +
> > >  	if (setup_header.protocol_version >= 0x0205) {
> > >  		relocatable_kernel = setup_header.relocatable_kernel;
> > >  		dbgprintf("bzImage is relocatable\n");
> > 
> > I know that there was a kernel release with 2048 _and_ still the old
> > boot protocol, but wouldn't it be better to warn the user if the size
> > is beyond 256 and the old kernel is used? I think new kexec-tools
> > should still support old kernels without problems ...
> > 
> 
> I don't know how important that really is, but I don't see a particular problem 
> with it either.  From my reading of i386/boot.txt, versions prior to boot 
> protocol 2.02 only supported a 256 bytes command line patch, so what if we just 
> add an extra check in do_bzImage_load.  If the protocol version of the boot 
> header is lexx than 0x0202, then we fail if the command line length is more than 
> 256 bytes.  Note there are two other locations where we use a linux boot 
> protocol header, but they are both constructed heders, not read headers, and use 
> protocol version 2.03, which support 2048 byte command lines.
> 

I think 2048 command line support came much later. I think it came between
version 2.05 and 2.06 (But somebody needs to dive into archive to verify).
Because command line size could go beyond 256, we introduced cmdline_size
parameter in version 2.06 to let a boot loader know.

What Bernanrd seems to be talking about a small window where boot protocol
was 2.05 but supported command line size was still 2048.

I think if we are loading any kernel older than 2.06 (and newer than
2.05?), and if command line size is greater than 256, we just might want to
give a debug warning to user, saying command line is great than 256 and kernel
does not tell me what's the supported command line size. (Somthing like that).

So it might look something like this.

if (protocol version > 2.06)
	error user depending on cmdline_size;
else if (protocol version > 2.05 && protocol version < 2.06)
	warn on cmdline being more than 256. We don't know for sure.
else
	error out if cmdline is greater than 256

I am not very sure about the boundary version 2.05, somebody needs to
verify.

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Bernhard Walle]

* Vivek Goyal <vgoyal@redhat.com> [2008-01-29 16:41]:
> 
> I think 2048 command line support came much later. I think it came between
> version 2.05 and 2.06 (But somebody needs to dive into archive to verify).
> Because command line size could go beyond 256, we introduced cmdline_size
> parameter in version 2.06 to let a boot loader know.

Large command line support was added with

    cca97de1184f6000d22b4106d47687b31cca1fa3
    v2.6.20-1929-gcca97de

which means that it was merged 2.6.21.

> What Bernanrd seems to be talking about a small window where boot protocol
> was 2.05 but supported command line size was still 2048.

2.05 was added with

    be274eeaf20b4c7155242645d5e2c48b023e609b
    v2.6.19-1394-gbe274ee

which means that it was merged 2.6.20. But it got implemented for i386
only that time, so x86_64 still had the 2.04 version.

The command line length in the boot protocol was added with

    8f9aeca7a081d81c4c9862be1e04f15b5ab5461f
    v2.6.21-1864-g8f9aeca

which menas that it was merged 2.6.22.

> if (protocol version > 2.06)
> 	error user depending on cmdline_size;
> else if (protocol version > 2.05 && protocol version < 2.06)
> 	warn on cmdline being more than 256. We don't know for sure.
> else
> 	error out if cmdline is greater than 256

The problem is 2.6.21: It had on x86_64 2.04 boot protocol but
2048 command line size. I just verified this via the tarball. :(

Maybe it could be implemented with versions ...


        Bernhard

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Tue, Jan 29, 2008 at 07:17:01PM +0100, Bernhard Walle wrote:
> * Vivek Goyal <vgoyal@redhat.com> [2008-01-29 16:41]:
> > 
> > I think 2048 command line support came much later. I think it came between
> > version 2.05 and 2.06 (But somebody needs to dive into archive to verify).
> > Because command line size could go beyond 256, we introduced cmdline_size
> > parameter in version 2.06 to let a boot loader know.
> 
> Large command line support was added with
> 
>     cca97de1184f6000d22b4106d47687b31cca1fa3
>     v2.6.20-1929-gcca97de
> 
> which means that it was merged 2.6.21.
> 
> > What Bernanrd seems to be talking about a small window where boot protocol
> > was 2.05 but supported command line size was still 2048.
> 
> 2.05 was added with
> 
>     be274eeaf20b4c7155242645d5e2c48b023e609b
>     v2.6.19-1394-gbe274ee
> 
> which means that it was merged 2.6.20. But it got implemented for i386
> only that time, so x86_64 still had the 2.04 version.
> 
> The command line length in the boot protocol was added with
> 
>     8f9aeca7a081d81c4c9862be1e04f15b5ab5461f
>     v2.6.21-1864-g8f9aeca
> 
> which menas that it was merged 2.6.22.
> 
> > if (protocol version > 2.06)
> > 	error user depending on cmdline_size;
> > else if (protocol version > 2.05 && protocol version < 2.06)
> > 	warn on cmdline being more than 256. We don't know for sure.
> > else
> > 	error out if cmdline is greater than 256
> 
> The problem is 2.6.21: It had on x86_64 2.04 boot protocol but
> 2048 command line size. I just verified this via the tarball. :(
> 
> Maybe it could be implemented with versions ...
> 

Ugh.  Ok, on the bright side we only need to worry about this for bzImage files,
since we generate the header for elf files, and we can just force the header we
generate to be version 2.06 and fill it out appropriately.  For bzImage files, I
think in do_bzImage_load we can just check both our arch and the boot protocol
version for the appropriate value.  I'm testing a patch currenly and will post
shortly.

Neil

> 
>         Bernhard

-- 
/****************************************************
 * Neil Horman <nhorman@tuxdriver.com>
 * Software Engineer, Red Hat
 ****************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

Ok, I've got a new patch here.  In response to Bernhard and Viveks concerns,
I've added a few checks.  Bernhard, thank you for digging into the history of
the addition of the 2048 byte command line.  As I understand what you've posted
heres what we need to capture:

On X86:
	Boot loader version >= 2.06
		error if cmdline_len > setup_header.cmdline_size
	2.05 > Boot loader version > 2.06
		warn if cmdline_len > 255
	Boot loader version < 2.05
		error if cmdline_len > 255

on X86_64:
	Boot loader version >= 2.04
		error if cmdline_len > setup_header.cmdline_size
	Boot loader version < 2.04
		error if cmdline_len > 255

There are three place in kexec-tools that we need to worry about this:
1) do_bzImage_load
2) elf_x86_load
3) elf_x86_64_load

Items 2 & 3 are non-issues, as they actually construct their own setup_header
for the kdump kernel.  I've modified the construction routine to specify version
2.06 of the bootloader and fill it out appropriately, and testing on my x86 and
x86_64 box shows that it works well.

The only remaining case is (1), do_bzImage_load.  To solve that I took the
bulldozer approach.  Since x86_64 borrows the i386 code for this, I simply added
a -D$(ARCH) in the CPPFLAGS for the Makefile, and preformed the above checks
based on weather we were building for 32 or 64 bit systems.

everything else is the same, and holds to the same summary as before:


This patch does 5 things:
1) moves command line out of the zero page (struct bootparam)

2) extends command line length to support 2K command lines

3) adds a check to ensure that command line length is reasonably sized for new
boot protocols

4) adds a check to ensure that command line length is reasonably sized for old
boot protocols

5) imports variables from latest struct setup_header in kernel bootparams.h


Testing works well for me here


Regards
Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

 include/x86/x86-linux.h           |   20 ++++++++++++++------
 kexec/Makefile                    |    2 +-
 kexec/arch/i386/kexec-bzImage.c   |   38 +++++++++++++++++++++++++++++++++++++-
 kexec/arch/i386/x86-linux-setup.c |    3 ++-
 4 files changed, 54 insertions(+), 9 deletions(-)



diff --git a/include/x86/x86-linux.h b/include/x86/x86-linux.h
index afe66bd..4f3507e 100644
--- a/include/x86/x86-linux.h
+++ b/include/x86/x86-linux.h
@@ -144,18 +144,22 @@ struct x86_linux_param_header {
 	/* 2.04+ */
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  reserved15[0x2d0 - 0x235];	/* 0x230 */
+	uint8_t  reserved15[3];			/* 0x235 */
+	uint32_t cmdline_size;			/* 0x238 */
+	uint32_t hardware_subarch;		/* 0x23C */
+	uint64_t hardware_subarch_data;		/* 0x240 */
+	uint8_t  reserved16[0x2d0 - 0x248];	/* 0x248 */
 #endif
 	struct e820entry e820_map[E820MAX];	/* 0x2d0 */
 						/* 0x550 */
-#define COMMAND_LINE_SIZE 256
+#define COMMAND_LINE_SIZE 2048 
 };
 
 struct x86_linux_faked_param_header {
 	struct x86_linux_param_header hdr;	/* 0x00 */
-	uint8_t reserved16[688];		/* 0x550 */
-	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x800 */
-	uint8_t reserved17[1792];		/* 0x900 - 0x1000 */
+	uint8_t reserved17[0xab0];		/* 0x550 */
+	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x1000 */
+	uint8_t reserved18[0x200];		/* 0x1800 - 0x2000 */
 };
 
 struct x86_linux_header {
@@ -206,7 +210,11 @@ struct x86_linux_header {
 #else
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  tail[32*1024 - 0x235];		/* 0x230 */
+	uint8_t  reserved6[3];			/* 0x235 */
+	uint32_t cmdline_size;                  /* 0x238 */
+	uint32_t hardware_subarch;              /* 0x23C */
+	uint64_t hardware_subarch_data;         /* 0x240 */
+	uint8_t  tail[32*1024 - 0x248];		/* 0x248 */
 #endif
 } PACKED;
 
diff --git a/kexec/Makefile b/kexec/Makefile
index 29534d0..1b41013 100644
--- a/kexec/Makefile
+++ b/kexec/Makefile
@@ -40,7 +40,7 @@ $(KEXEC): $(KEXEC_OBJS) $(UTIL_LIB)
 	@$(MKDIR) -p $(@D)
 	$(LINK.o) -o $@ $^
 
-$(KEXEC): CPPFLAGS+=-I$(srcdir)/kexec/arch/$(ARCH)/include
+$(KEXEC): CPPFLAGS+=-I$(srcdir)/kexec/arch/$(ARCH)/include -D$(ARCH)
 
 $(KEXEC_MANPAGE): kexec/kexec.8
 	@$(MKDIR) -p     $(MANDIR)/man8
diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
index 8fde799..535d9ff 100644
--- a/kexec/arch/i386/kexec-bzImage.c
+++ b/kexec/arch/i386/kexec-bzImage.c
@@ -134,11 +134,47 @@ int do_bzImage_load(struct kexec_info *info,
 		return -1;
 	}
 
+	if (setup_header.protocol_version >= 0x0206) {
+		if (command_line_len > setup_header.cmdline_size) {
+			dbgprintf("Kernel command line too long for kernel!\n");
+			return -1;
+		}
+	}
+
+#ifdef i386
+	if ((setup_header.protocol_version < 0x0206)  &&
+	   (setup_header.protocol_version >= 0x0205)) {
+		if (command_line_len >= 256) {
+			dbgprintf("Kernel may only support 256 byte command line!\n");
+		}
+	}
+
+	if (setup_header.protocol_version < 0x0205) {
+		if (command_line_len >= 256) {
+			dbgprintf("Kernel only supports 256 byte command line!\n");
+			return -1;
+		}
+	}
+
 	if (setup_header.protocol_version >= 0x0205) {
 		relocatable_kernel = setup_header.relocatable_kernel;
 		dbgprintf("bzImage is relocatable\n");
 	}
-
+#else
+	/* Special check for 2.04 boot protocol in x86_64 */
+	if (setup_header.protocol_version >= 0x0204) {
+		if (command_line_len > setup_header.cmdline_size) {
+			dbgprintf("Kernel command line too long for kernel!\n");
+			return -1;
+		}
+	} else {
+		if (command_line_len >= 256) {
+			dbgprintf("Kernel only supports 256 byte command lines!\n");
+			return -1;
+		}
+	}
+		
+#endif
 	/* Can't use bzImage for crash dump purposes with real mode entry */
 	if((info->kexec_flags & KEXEC_ON_CRASH) && real_mode_entry) {
 		fprintf(stderr, "Can't use bzImage for crash dump purposes"
diff --git a/kexec/arch/i386/x86-linux-setup.c b/kexec/arch/i386/x86-linux-setup.c
index df2f5c0..68234fa 100644
--- a/kexec/arch/i386/x86-linux-setup.c
+++ b/kexec/arch/i386/x86-linux-setup.c
@@ -38,8 +38,9 @@ void init_linux_parameters(struct x86_linux_param_header *real_mode)
 
 	/* Boot block magic */
 	memcpy(real_mode->header_magic, "HdrS", 4);
-	real_mode->protocol_version = 0x0203;
+	real_mode->protocol_version = 0x0206;
 	real_mode->initrd_addr_max = DEFAULT_INITRD_ADDR_MAX;
+	real_mode->cmdline_size = COMMAND_LINE_SIZE;
 }
 
 void setup_linux_bootloader_parameters(
-- 
/****************************************************
 * Neil Horman <nhorman@tuxdriver.com>
 * Software Engineer, Red Hat
 ****************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

On Tue, Jan 29, 2008 at 02:57:53PM -0500, Neil Horman wrote:
> 
> Ok, I've got a new patch here.  In response to Bernhard and Viveks concerns,
> I've added a few checks.  Bernhard, thank you for digging into the history of
> the addition of the 2048 byte command line.  As I understand what you've posted
> heres what we need to capture:
> 
> On X86:
> 	Boot loader version >= 2.06
> 		error if cmdline_len > setup_header.cmdline_size
> 	2.05 > Boot loader version > 2.06
> 		warn if cmdline_len > 255
> 	Boot loader version < 2.05
> 		error if cmdline_len > 255
> 
> on X86_64:
> 	Boot loader version >= 2.04
> 		error if cmdline_len > setup_header.cmdline_size

cmdline_size was introduced only in version 2.06. So this will run
into trouble on x86_64 version 2.04 and 2.05, isn't it?

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Wed, Jan 30, 2008 at 03:53:23PM -0500, Vivek Goyal wrote:
> On Tue, Jan 29, 2008 at 02:57:53PM -0500, Neil Horman wrote:
> > 
> > Ok, I've got a new patch here.  In response to Bernhard and Viveks concerns,
> > I've added a few checks.  Bernhard, thank you for digging into the history of
> > the addition of the 2048 byte command line.  As I understand what you've posted
> > heres what we need to capture:
> > 
> > On X86:
> > 	Boot loader version >= 2.06
> > 		error if cmdline_len > setup_header.cmdline_size
> > 	2.05 > Boot loader version > 2.06
> > 		warn if cmdline_len > 255
> > 	Boot loader version < 2.05
> > 		error if cmdline_len > 255
> > 
> > on X86_64:
> > 	Boot loader version >= 2.04
> > 		error if cmdline_len > setup_header.cmdline_size
> 
> cmdline_size was introduced only in version 2.06. So this will run
> into trouble on x86_64 version 2.04 and 2.05, isn't it?
> 
Then how did version 2.04 on x86_64 determine its maximum command line length?
Or did it just assume a max length of 2048 bytes?
Neil

> Thanks
> Vivek
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@redhat.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

On Wed, Jan 30, 2008 at 03:59:47PM -0500, Neil Horman wrote:
> On Wed, Jan 30, 2008 at 03:53:23PM -0500, Vivek Goyal wrote:
> > On Tue, Jan 29, 2008 at 02:57:53PM -0500, Neil Horman wrote:
> > > 
> > > Ok, I've got a new patch here.  In response to Bernhard and Viveks concerns,
> > > I've added a few checks.  Bernhard, thank you for digging into the history of
> > > the addition of the 2048 byte command line.  As I understand what you've posted
> > > heres what we need to capture:
> > > 
> > > On X86:
> > > 	Boot loader version >= 2.06
> > > 		error if cmdline_len > setup_header.cmdline_size
> > > 	2.05 > Boot loader version > 2.06
> > > 		warn if cmdline_len > 255
> > > 	Boot loader version < 2.05
> > > 		error if cmdline_len > 255
> > > 
> > > on X86_64:
> > > 	Boot loader version >= 2.04
> > > 		error if cmdline_len > setup_header.cmdline_size
> > 
> > cmdline_size was introduced only in version 2.06. So this will run
> > into trouble on x86_64 version 2.04 and 2.05, isn't it?
> > 
> Then how did version 2.04 on x86_64 determine its maximum command line length?
> Or did it just assume a max length of 2048 bytes?

I think it just did not tell. There was no way for a kernel bzImage to
tell boot-loader what's the supported command line size is (Pre 2.06).

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Wed, Jan 30, 2008 at 04:08:15PM -0500, Vivek Goyal wrote:
> On Wed, Jan 30, 2008 at 03:59:47PM -0500, Neil Horman wrote:
> > On Wed, Jan 30, 2008 at 03:53:23PM -0500, Vivek Goyal wrote:
> > > On Tue, Jan 29, 2008 at 02:57:53PM -0500, Neil Horman wrote:
> > > > 
> > > > Ok, I've got a new patch here.  In response to Bernhard and Viveks concerns,
> > > > I've added a few checks.  Bernhard, thank you for digging into the history of
> > > > the addition of the 2048 byte command line.  As I understand what you've posted
> > > > heres what we need to capture:
> > > > 
> > > > On X86:
> > > > 	Boot loader version >= 2.06
> > > > 		error if cmdline_len > setup_header.cmdline_size
> > > > 	2.05 > Boot loader version > 2.06
> > > > 		warn if cmdline_len > 255
> > > > 	Boot loader version < 2.05
> > > > 		error if cmdline_len > 255
> > > > 
> > > > on X86_64:
> > > > 	Boot loader version >= 2.04
> > > > 		error if cmdline_len > setup_header.cmdline_size
> > > 
> > > cmdline_size was introduced only in version 2.06. So this will run
> > > into trouble on x86_64 version 2.04 and 2.05, isn't it?
> > > 
> > Then how did version 2.04 on x86_64 determine its maximum command line length?
> > Or did it just assume a max length of 2048 bytes?
> 
> I think it just did not tell. There was no way for a kernel bzImage to
> tell boot-loader what's the supported command line size is (Pre 2.06).
> 
So, for 2.04 on x86_64, we should just assume a 2048 byte command line length
then?

Neil

> Thanks
> Vivek

-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@redhat.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Vivek Goyal]

On Wed, Jan 30, 2008 at 04:18:34PM -0500, Neil Horman wrote:
> On Wed, Jan 30, 2008 at 04:08:15PM -0500, Vivek Goyal wrote:
> > On Wed, Jan 30, 2008 at 03:59:47PM -0500, Neil Horman wrote:
> > > On Wed, Jan 30, 2008 at 03:53:23PM -0500, Vivek Goyal wrote:
> > > > On Tue, Jan 29, 2008 at 02:57:53PM -0500, Neil Horman wrote:
> > > > > 
> > > > > Ok, I've got a new patch here.  In response to Bernhard and Viveks concerns,
> > > > > I've added a few checks.  Bernhard, thank you for digging into the history of
> > > > > the addition of the 2048 byte command line.  As I understand what you've posted
> > > > > heres what we need to capture:
> > > > > 
> > > > > On X86:
> > > > > 	Boot loader version >= 2.06
> > > > > 		error if cmdline_len > setup_header.cmdline_size
> > > > > 	2.05 > Boot loader version > 2.06
> > > > > 		warn if cmdline_len > 255
> > > > > 	Boot loader version < 2.05
> > > > > 		error if cmdline_len > 255
> > > > > 
> > > > > on X86_64:
> > > > > 	Boot loader version >= 2.04
> > > > > 		error if cmdline_len > setup_header.cmdline_size
> > > > 
> > > > cmdline_size was introduced only in version 2.06. So this will run
> > > > into trouble on x86_64 version 2.04 and 2.05, isn't it?
> > > > 
> > > Then how did version 2.04 on x86_64 determine its maximum command line length?
> > > Or did it just assume a max length of 2048 bytes?
> > 
> > I think it just did not tell. There was no way for a kernel bzImage to
> > tell boot-loader what's the supported command line size is (Pre 2.06).
> > 
> So, for 2.04 on x86_64, we should just assume a 2048 byte command line length
> then?
> 

No. 2.04 is really old. As per boot.txt, it was introduced in 2.6.14.

Anyway, thinking about it more, Large command lines were introduced in
2.6.21 kernel (for both i386 and x86_64). cmdline_size was introduced
in 2.6.22 (for both i386 and x86_64). So how about following test
condition for both the arch.

if (boot_protocol >= 2.06)
	error message based on cmdline_size field
else
	Warn user if command line is more than 256 but continue to work.
  	Let the user know that it is not known if kernel being loaded
	supports command line size greater than 256. Command line will
	be truncated by destination kernel if it does not support large
	command lines. User is taking a chance

I think above will work both for i386 and x86_64. This is not precise
message. If we want to do better, then we shall have to rely on linux
version string. But If somebody is building custom kernel with their
own versioning, then we will run into issues. So I think, for the time
being we are good with above simple condition.

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

Fine, New patch attached.


This patch does 5 things:
1) moves command line out of the zero page (struct bootparam)

2) extends command line length to support 2K command lines

3) adds a check to ensure that command line length is reasonably sized for new
boot protocols

4) adds a check to ensure that command line length is reasonably sized for old
boot protocols

5) imports variables from latest struct setup_header in kernel bootparams.h

Incorporates simplified version checking for boot protocol and conservatively
warns if the kernels boot protocol is below version 2.06 which is guaranteed to
have 2K commandlines (even though some arches  may have that support in
older boot protocols)


Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 include/x86/x86-linux.h           |   20 ++++++++++++++------
 kexec/arch/i386/kexec-bzImage.c   |   11 +++++++++++
 kexec/arch/i386/x86-linux-setup.c |    3 ++-
 3 files changed, 27 insertions(+), 7 deletions(-)



diff --git a/include/x86/x86-linux.h b/include/x86/x86-linux.h
index afe66bd..4f3507e 100644
--- a/include/x86/x86-linux.h
+++ b/include/x86/x86-linux.h
@@ -144,18 +144,22 @@ struct x86_linux_param_header {
 	/* 2.04+ */
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  reserved15[0x2d0 - 0x235];	/* 0x230 */
+	uint8_t  reserved15[3];			/* 0x235 */
+	uint32_t cmdline_size;			/* 0x238 */
+	uint32_t hardware_subarch;		/* 0x23C */
+	uint64_t hardware_subarch_data;		/* 0x240 */
+	uint8_t  reserved16[0x2d0 - 0x248];	/* 0x248 */
 #endif
 	struct e820entry e820_map[E820MAX];	/* 0x2d0 */
 						/* 0x550 */
-#define COMMAND_LINE_SIZE 256
+#define COMMAND_LINE_SIZE 2048 
 };
 
 struct x86_linux_faked_param_header {
 	struct x86_linux_param_header hdr;	/* 0x00 */
-	uint8_t reserved16[688];		/* 0x550 */
-	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x800 */
-	uint8_t reserved17[1792];		/* 0x900 - 0x1000 */
+	uint8_t reserved17[0xab0];		/* 0x550 */
+	uint8_t command_line[COMMAND_LINE_SIZE]; /* 0x1000 */
+	uint8_t reserved18[0x200];		/* 0x1800 - 0x2000 */
 };
 
 struct x86_linux_header {
@@ -206,7 +210,11 @@ struct x86_linux_header {
 #else
 	uint32_t kernel_alignment;		/* 0x230 */
 	uint8_t  relocatable_kernel;		/* 0x234 */
-	uint8_t  tail[32*1024 - 0x235];		/* 0x230 */
+	uint8_t  reserved6[3];			/* 0x235 */
+	uint32_t cmdline_size;                  /* 0x238 */
+	uint32_t hardware_subarch;              /* 0x23C */
+	uint64_t hardware_subarch_data;         /* 0x240 */
+	uint8_t  tail[32*1024 - 0x248];		/* 0x248 */
 #endif
 } PACKED;
 
diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
index 8fde799..93e37a4 100644
--- a/kexec/arch/i386/kexec-bzImage.c
+++ b/kexec/arch/i386/kexec-bzImage.c
@@ -134,6 +134,17 @@ int do_bzImage_load(struct kexec_info *info,
 		return -1;
 	}
 
+	if (setup_header.protocol_version >= 0x0206) {
+		if (command_line_len > setup_header.cmdline_size) {
+			dbgprintf("Kernel command line too long for kernel!\n");
+			return -1;
+		}
+	} else {
+		if (command_line_len > 255) {
+			dbgprintf("WARNING: This kernel may only support 255 byte command lines\n");
+		}
+	}
+
 	if (setup_header.protocol_version >= 0x0205) {
 		relocatable_kernel = setup_header.relocatable_kernel;
 		dbgprintf("bzImage is relocatable\n");
diff --git a/kexec/arch/i386/x86-linux-setup.c b/kexec/arch/i386/x86-linux-setup.c
index df2f5c0..68234fa 100644
--- a/kexec/arch/i386/x86-linux-setup.c
+++ b/kexec/arch/i386/x86-linux-setup.c
@@ -38,8 +38,9 @@ void init_linux_parameters(struct x86_linux_param_header *real_mode)
 
 	/* Boot block magic */
 	memcpy(real_mode->header_magic, "HdrS", 4);
-	real_mode->protocol_version = 0x0203;
+	real_mode->protocol_version = 0x0206;
 	real_mode->initrd_addr_max = DEFAULT_INITRD_ADDR_MAX;
+	real_mode->cmdline_size = COMMAND_LINE_SIZE;
 }
 
 void setup_linux_bootloader_parameters(

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Bernhard Walle]

* Vivek Goyal <vgoyal@redhat.com> [2008-01-30 22:45]:
> 
> No. 2.04 is really old. As per boot.txt, it was introduced in 2.6.14.
> 
> Anyway, thinking about it more, Large command lines were introduced in
> 2.6.21 kernel (for both i386 and x86_64). cmdline_size was introduced
> in 2.6.22 (for both i386 and x86_64). So how about following test
> condition for both the arch.
> 
> if (boot_protocol >= 2.06)
> 	error message based on cmdline_size field
> else
> 	Warn user if command line is more than 256 but continue to work.
>   	Let the user know that it is not known if kernel being loaded
> 	supports command line size greater than 256. Command line will
> 	be truncated by destination kernel if it does not support large
> 	command lines. User is taking a chance

Yes, I also think that's sufficient.


        Bernhard

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Neil Horman]

On Thu, Jan 31, 2008 at 08:16:25AM +0100, Bernhard Walle wrote:
> * Vivek Goyal <vgoyal@redhat.com> [2008-01-30 22:45]:
> > 
> > No. 2.04 is really old. As per boot.txt, it was introduced in 2.6.14.
> > 
> > Anyway, thinking about it more, Large command lines were introduced in
> > 2.6.21 kernel (for both i386 and x86_64). cmdline_size was introduced
> > in 2.6.22 (for both i386 and x86_64). So how about following test
> > condition for both the arch.
> > 
> > if (boot_protocol >= 2.06)
> > 	error message based on cmdline_size field
> > else
> > 	Warn user if command line is more than 256 but continue to work.
> >   	Let the user know that it is not known if kernel being loaded
> > 	supports command line size greater than 256. Command line will
> > 	be truncated by destination kernel if it does not support large
> > 	command lines. User is taking a chance
> 
> Yes, I also think that's sufficient.
> 
> 
>         Bernhard
> 
So, has anyone heard out of Simon?  I've noted that he hasn't updated the git
tree for kexec-tools nor has he posted to the list is almost 3 weeks.  Anyone
know if he's ok?  Simon, if you're out there, give us a shout! :)

Thanks & Regards
Neil

> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

-- 
/****************************************************
 * Neil Horman <nhorman@tuxdriver.com>
 * Software Engineer, Red Hat
 ****************************************************/

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: Kexec command line length

[Simon Horman]

On Tue, Feb 12, 2008 at 04:11:57PM -0500, Neil Horman wrote:
> On Thu, Jan 31, 2008 at 08:16:25AM +0100, Bernhard Walle wrote:
> > * Vivek Goyal <vgoyal@redhat.com> [2008-01-30 22:45]:
> > > 
> > > No. 2.04 is really old. As per boot.txt, it was introduced in 2.6.14.
> > > 
> > > Anyway, thinking about it more, Large command lines were introduced in
> > > 2.6.21 kernel (for both i386 and x86_64). cmdline_size was introduced
> > > in 2.6.22 (for both i386 and x86_64). So how about following test
> > > condition for both the arch.
> > > 
> > > if (boot_protocol >= 2.06)
> > > 	error message based on cmdline_size field
> > > else
> > > 	Warn user if command line is more than 256 but continue to work.
> > >   	Let the user know that it is not known if kernel being loaded
> > > 	supports command line size greater than 256. Command line will
> > > 	be truncated by destination kernel if it does not support large
> > > 	command lines. User is taking a chance
> > 
> > Yes, I also think that's sufficient.
> > 
> > 
> >         Bernhard
> > 
> So, has anyone heard out of Simon?  I've noted that he hasn't updated the git
> tree for kexec-tools nor has he posted to the list is almost 3 weeks.  Anyone
> know if he's ok?  Simon, if you're out there, give us a shout! :)

Hi All,

sorry to be quiet. I've been travling + other modes of busy.

I won't pretend to be an expert on the x86 boot protocol,
but this patch does look clean to me.

-- 
Horms


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec