Re: Revert commit 5dcd14ecd4 - breaks EFI boot with SLES11 elilo.efi

[Robin Holt <holt@sgi.com>]

That fixed it for me.

Can you help me understand why sentinel is non-zero?  It looks to me
like 3.14 allocates 16kB plus strlen of the command line, zeros it,
and then proceeds to fill in fields, some differing from what is in the
boot_params structure.  That said, it looks like the sentinel field
should remain 0.  I am still trying to understand, but if this patch
makes it in, I am happy.

Thanks,
Robin

On Tue, Mar 05, 2013 at 11:12:08AM -0800, Yinghai Lu wrote:
> On Tue, Mar 5, 2013 at 7:22 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> > Yes, please do the analysis I asked for.
> 
> it uses first 2 pages in bzImage to bootparams.
> 
> then update the fields with ===> X
> 
> struct boot_params {
>         struct screen_info screen_info;                 /* 0x000 */   ===> X
>         struct apm_bios_info apm_bios_info;             /* 0x040 */   ===> X
>         __u8  _pad2[4];                                 /* 0x054 */
>         __u64  tboot_addr;                              /* 0x058 */
>         struct ist_info ist_info;                       /* 0x060 */
>         __u8  _pad3[16];                                /* 0x070 */
>         __u8  hd0_info[16];     /* obsolete! */         /* 0x080 */   ===> X
>         __u8  hd1_info[16];     /* obsolete! */         /* 0x090 */   ===> X
>         struct sys_desc_table sys_desc_table;           /* 0x0a0 */   ===> X
>         struct olpc_ofw_header olpc_ofw_header;         /* 0x0b0 */
>         __u32 ext_ramdisk_image;                        /* 0x0c0 */
>         __u32 ext_ramdisk_size;                         /* 0x0c4 */
>         __u32 ext_cmd_line_ptr;                         /* 0x0c8 */
>         __u8  _pad4[116];                               /* 0x0cc */
>         struct edid_info edid_info;                     /* 0x140 */
>         struct efi_info efi_info;                       /* 0x1c0 */   ===> X
>         __u32 alt_mem_k;                                /* 0x1e0 */   ===> X
>         __u32 scratch;          /* Scratch field! */    /* 0x1e4 */
>         __u8  e820_entries;                             /* 0x1e8 */  ===> X
>         __u8  eddbuf_entries;                           /* 0x1e9 */
>         __u8  edd_mbr_sig_buf_entries;                  /* 0x1ea */
>         __u8  kbd_status;                               /* 0x1eb */
>         __u8  _pad5[3];                                 /* 0x1ec */
>         /*
>          * The sentinel is set to a nonzero value (0xff) in header.S.
>          *
>          * A bootloader is supposed to only take setup_header and put
>          * it into a clean boot_params buffer. If it turns out that
>          * it is clumsy or too generous with the buffer, it most
>          * probably will pick up the sentinel variable too. The fact
>          * that this variable then is still 0xff will let kernel
>          * know that some variables in boot_params are invalid and
>          * kernel should zero out certain portions of boot_params.
>          */
>         __u8  sentinel;                                 /* 0x1ef */
>         __u8  _pad6[1];                                 /* 0x1f0 */
>         struct setup_header hdr;    /* setup header */  /* 0x1f1 */  ===> X
>         __u8  _pad7[0x290-0x1f1-sizeof(struct setup_header)];
>         __u32 edd_mbr_sig_buffer[EDD_MBR_SIG_MAX];      /* 0x290 */
>         struct e820entry e820_map[E820MAX];             /* 0x2d0 */  ===> X
>         __u8  _pad8[48];                                /* 0xcd0 */
>         struct edd_info eddbuf[EDDMAXNR];               /* 0xd00 */
>         __u8  _pad9[276];                               /* 0xeec */
> 
> so sentinel will be kept as 0xff, so efi_info get cleared by kernel...
> 
> Attached patches should avoid the clearing of efi_info when elilo is used.
> 
> Do we need to clear edd and pad2 and pad3 for elilo?
> 
> Thanks
> 
> Yinghai