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 <[EMAIL PROTECTED]>
> 

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
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Reply via email to