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

From: Robin Holt
Date: Tue Mar 05 2013 - 14:52:34 EST


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@xxxxxxxxx> 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


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