Re: [UPDATED PATCH] EFI support for ia32 kernels

From: Eric W. Biederman
Date: Fri Sep 05 2003 - 23:36:46 EST


Mark Gross <mgross@xxxxxxxxxxxxxxxxxx> writes:

> On Thu, 2003-09-04 at 08:35, Eric W. Biederman wrote:
> > "Tolentino, Matthew E" <matthew.e.tolentino@xxxxxxxxx> writes:
> > A problem is that set_virtual_address_space cannot be called multiple times,
> > and so it interacts badly with kexec. I was just about to disable it
> > on the ia64 tree, so I could use kexec. Besides that BIOS calls
> > should be quite infrequent so flipping to physical mode this should
> > not matter. That plus not being in physical mode looks like a great
> > way to trip up various implementation bugs when there are multiple
> > implementations.
>
> Hmmm. I bet there is some other way to get around this.

Why enable a slow path case that will make the system less reliable?

> Perhaps use
> efi_set_varriable to implement a counter in NV memory or a page reserved
> by the boot loader that gets initialized by elilo on the first boot up
> and then have the kernel test / write to it in the startup before
> calling set_vertual_address_space. They would requier a tweak to the
> boot loader to make work.

Sure you can do things like that but then you can't call EFI in physical
address mode.

set_virtual_address_space is unnecessary, on a slow path, tricky to
test, and potentially bug prone.

Please just deprecate set_virtual_address please.

> > > I'm not sure what you mean here. Nothing really, except that the loader
> passes
>
> > > the location of the initrd to the kernel, even though the loader is
> currently
>
> > > putting where the kernel expects it. However, in the future this may allow
> the
>
> > > initrd to be placed somewhere else.
> >
> > >
> > > > > +struct ia32_boot_params {
> > > > > + unsigned long size;
> > > > > + unsigned long command_line;
> > > > > + efi_system_table_t *efi_sys_tbl;
> > > > > + efi_memory_desc_t *efi_mem_map;
> > > > > + unsigned long efi_mem_map_size;
> > > > > + unsigned long efi_mem_desc_size;
> > > > > + unsigned long efi_mem_desc_version;
> > > > > + unsigned long initrd_start;
> > > > > + unsigned long initrd_size;
> > > > > + unsigned long loader_start;
> > > > > + unsigned long loader_size;
> > > > > + unsigned long kernel_start;
> > > > > + unsigned long kenrel_size;
> > > > > + unsigned long num_cols;
> > > > > + unsigned long num_rows;
> > > > > + unsigned long orig_x;
> > > > > + unsigned long orig_y;
> > > > > +};
> > > >
> > > > Interesting. What's all this, and how does the user interact with it?
> > >
> > > It's the boot parameters that the EFI linux boot loader (ELILO) passes to
> the
>
> > > kernel. It's only used in the early boot process.
> >
> > Hmm. You have added additional parameters passed to the kernel, but
> > have not updated the documentation. Nor have you bumped the protocol
> > number in setup.S.
>
> Bumping the boot protocal in setup.S doesn't make sence as this new boot
> protocal is only possible under EFI platforms. Legacy BIOS platform
> boot up processing shouldn't know anything about it. Its ment to be
> orthoganal to the older boot protocal.

Bumping the minor revision indicates new features are present.
You added new features therefore the minor rev needs to be bumped.

> This being said, some EFI boot protocall documentation could be cut and
> pasted out of the OLS talk into a new file the Documentation directory.
>
> >
> > Beyond that you have duplicated a bunch of variables that already have
> > perfectly valid ways of being passed to the kernel.
>
> Actualy this is a step in getting away from those legacy boot
> parrameters scattered about the boot parrameter block for the EFI boot
> up processing.
>
> The worst thing about continuing to use the legacy boot parrameters is
> that we then need to go hunting for holes in the existing structure
> where we can put the new EFI specific values as well as ending up
> carrying along baggage that dates way WAY back that doesn't get used or
> make sence any more.

The joy of x86. And no you don't need to look for holes all you need to
do is to append to the end.

> > initrd_start, initrd_size, num_cols, num_rows, orig_x, orig_y and the
> > command line should be passed in their original locations. At least
> > baring the creation of a subarch and starting from scratch.
> >
>
> We are hoping to avoid doing a subarch with this boot parrameter design
> and went for a coexistance approach that has zero impact to the current
> booting up on legacy platforms.

Which is a reasonable way to go.

> I think this is THE key issue to get to the bottom of. EFI enabled
> kernels need not be a new sub-architecture, as we can see that the EFI
> start up design supports booting on legacy firmware/bios, and has zero
> impact on execution flow for the legacy case. Do you think that doing a
> sub architecture is really needed for this?

If you want a clean slate a sub arch looks necessary. If you want to
coexist with the legacy you need to put of with the issues of being
compatible.

But realize you will also want to know you started from efi even if
you were loaded in pcbios compatibility mode with lilo or grub. So
this is not a boolean kind of thing EFI or no EFI. It is does my BIOS
have the EFI features. At least on x86. And at that point you
probably want EFI detection in Setup.S.

> > kernel_start, and kernel_size are not used.
> > loader_start, and loader_size are not used.
> >
>
> They are anticipated to be useful for embedded designs booting linux on
> EFI firmwware. They could be removed but I'd rather see them stay.

Except when you are directly loading vmlinux you don't have the information
to populate kernel_start and kernel_size properly. And at that point
you are missing other interesting parts of the kernel. Like which
boot protocol it supports.

The x86 boot protocol is crusty and has plenty of warts but it works.
Just having a length and no version number or any other way to detect
features is worse, and that is what you are proposing in the EFI case.

Things change and evolve. So far I know of two distinct versions of
EFI. The EFI that has been so nicely described by Mark Doran. And
the version I have actually used with is quite a different animal.


Eric

-
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/