[PATCH 0/5] x86_64 EFI support -v3

[PATCH 0/5] x86_64 EFI support -v3

[Huang, Ying]

Following sets of patches add EFI/UEFI (Unified Extensible Firmware
Interface) support to x86_64 architecture. The patches have been
tested against 2.6.23-rc1 kernel on Intel platforms with EFI1.10 and
UEFI2.0 firmware.

UEFI specification can be found here: http://www.uefi.org

For booting the UEFI x86_64 enabled kernel, the machine with EFI/UEFI
firmware and the support of bootloader is required. Detailed usage
guide can be found in Documentation/x86_64/uefi.txt, which is added in
the patch: efi-doc.patch.

Issues _not_ addressed (per feedback from Eric Biederman)

- Virtual mode support is still retained in this patch. There is at
  least one EFI call is fast path: efi_set_rtc_mmss, which must
  complete as soon as possible.

- The variable efi_enabled is used throughout across architecutres if
 CONFIG_EFI option is enabled. The i386 code also uses this variable.
 This is something that can be revisited with code consolidation
 across architectures.

Looking forward to your comments,

Best Regards,
Huang Ying

Re: [PATCH 0/5] x86_64 EFI support -v3

[Eric W. Biederman]

"Huang, Ying" <ying.huang@intel.com> writes:

> Following sets of patches add EFI/UEFI (Unified Extensible Firmware
> Interface) support to x86_64 architecture. The patches have been
> tested against 2.6.23-rc1 kernel on Intel platforms with EFI1.10 and
> UEFI2.0 firmware.
>
> UEFI specification can be found here: http://www.uefi.org

Is it still the case you must sign a license you won't implement
it if you read the specification?

> For booting the UEFI x86_64 enabled kernel, the machine with EFI/UEFI
> firmware and the support of bootloader is required. Detailed usage
> guide can be found in Documentation/x86_64/uefi.txt, which is added in
> the patch: efi-doc.patch.
>
> Issues _not_ addressed (per feedback from Eric Biederman)

Thank you for acknowledging them.

I would really prefer to see something start simple and obviously
correct and grow (typical unix/linux development) rather then
attempt to use all of the cool efi features at once.

> - Virtual mode support is still retained in this patch. There is at
>   least one EFI call is fast path: efi_set_rtc_mmss, which must
>   complete as soon as possible.

Bogus.  You are setting the wall clock time in the granularity
of a second.  Yes we can achieve high accuracy by setting things
as soon after the change as we can.  I don't see a couple of
extra micro second being a bit deal here.  If a couple of
extra micro seconds are a big deal we shouldn't be going through
efi to perform this logic in the first place.  This is x86 
and we know the hardware programming interface.

Why in the world are we going through efi for real time clock
operations anyway.  That seems completely silly.

> - The variable efi_enabled is used throughout across architecutres if
>  CONFIG_EFI option is enabled. The i386 code also uses this variable.
>  This is something that can be revisited with code consolidation
>  across architectures.

Fix it first. arch/i386/ efi support is horrible, and show what happens
when things are not done properly the first time.  Later doesn't happen.
With the partvirt logic we have a lot of operations properly split out
already.  Figure out how to use them. 

Ok. Looking what more I can tear into.

Eric

Re: [PATCH 0/5] x86_64 EFI support -v3

[Huang, Ying]

On Mon, 2007-07-30 at 22:16 -0600, Eric W. Biederman wrote:
> "Huang, Ying" <ying.huang@intel.com> writes:
> > - The variable efi_enabled is used throughout across architecutres if
> >  CONFIG_EFI option is enabled. The i386 code also uses this variable.
> >  This is something that can be revisited with code consolidation
> >  across architectures.
> 
> Fix it first. arch/i386/ efi support is horrible, and show what happens
> when things are not done properly the first time.  Later doesn't happen.
> With the partvirt logic we have a lot of operations properly split out
> already.  Figure out how to use them.

What do you suggest to use instead of efi_enabled?

Current method is (efi_enabled based):

(1) Encapsulate EFI based implementation and legacy BIOS based
implementation into separate functions.
(2) Define a wrapper function for each interface in (1), efi_enabled is
used to choose implementation between EFI and legacy BIOS.

Another possible method is (function pointer based):

1. Encapsulate EFI based implementation and legacy BIOS based
implementation into separate functions.
2. Define a function pointer for each interface in (1), the function
pointer is set to legacy BIOS based implementation by default and
changed to EFI based implementation if appropriate.

Because there are only two possible choice, I think the function pointer
based method has no big advantages over the efi_enabled based method.

Best Regards,
Huang Ying

Re: [PATCH 0/5] x86_64 EFI support -v3

[Eric W. Biederman]

"Huang, Ying" <ying.huang@intel.com> writes:

> On Mon, 2007-07-30 at 22:16 -0600, Eric W. Biederman wrote:
>> "Huang, Ying" <ying.huang@intel.com> writes:
>> > - The variable efi_enabled is used throughout across architecutres if
>> >  CONFIG_EFI option is enabled. The i386 code also uses this variable.
>> >  This is something that can be revisited with code consolidation
>> >  across architectures.
>> 
>> Fix it first. arch/i386/ efi support is horrible, and show what happens
>> when things are not done properly the first time.  Later doesn't happen.
>> With the partvirt logic we have a lot of operations properly split out
>> already.  Figure out how to use them.
>
> What do you suggest to use instead of efi_enabled?
>
> Current method is (efi_enabled based):
>
> (1) Encapsulate EFI based implementation and legacy BIOS based
> implementation into separate functions.
> (2) Define a wrapper function for each interface in (1), efi_enabled is
> used to choose implementation between EFI and legacy BIOS.
>
> Another possible method is (function pointer based):

Exactly.  Which is what everything else in the kernel does and is
extensible.

> 1. Encapsulate EFI based implementation and legacy BIOS based
> implementation into separate functions.

> 2. Define a function pointer for each interface in (1), the function
> pointer is set to legacy BIOS based implementation by default and
> changed to EFI based implementation if appropriate.
>
> Because there are only two possible choice, I think the function pointer
> based method has no big advantages over the efi_enabled based method.

Not at all every hypervisor does these things differently as well,
so in the real world there are a lot of choices. 

Eric

Re: [PATCH 0/5] x86_64 EFI support -v3

[Eric W. Biederman]

"Huang, Ying" <ying.huang@intel.com> writes:

> Looking forward to your comments,

After reading through the patches.  I don't see any compelling
reason to use the efi runtime support.  It looks like we get
the interesting support for efi without out.  The graphics
and the memory map.  We have ACPI which already can do handle
this case.

Further the code has dead code sitting in the patches that you never
intend to use. EFI_MEMMAP.

The boot protocols are now out of sync for arch/i386 and arch/x86_64
for no obvious reason.

Using efi_set_virtual means kdump doesn't work which means that no
one is going to use this in a prebuilt kernel.

So mostly this patchset looks like a bad idea.

Eric

Re: [PATCH 0/5] x86_64 EFI support -v3

[Huang, Ying]

On Tue, 2007-07-31 at 12:47 +0800, Eric W. Biederman wrote:
> Using efi_set_virtual means kdump doesn't work which means that no
> one is going to use this in a prebuilt kernel.

It is possible to make kexec/kdump work with EFI virtual mode, in
following ways:

1. Do not turn on EFI in kexeced kernel. That is, when kexec prepares
the boot parameters for kexeced kernel, do not set boot parameter
"EFI_LOADER_SIG" to be "EFIL". And, if the boot parameter
"screen_info.orig_video_isVGA" is set to VIDEO_TYPE_EFI and other
members of "screen_info" are set properly, the EFI framebuffer can work
properly too. With this method, a EFI disabled kernel can be kexeced
from a EFI enabled kernel. This is OK for kdump to work.

2. If it is intended to kexec a EFI enabled kernel from a EFI enabled
kernel, the same method as IA64 EFI virtual mode support can be used.
That is, the memory area used by EFI runtime service is mapped to exact
same address in both kernels, and the "efi_set_virtual" is not called in
kexeced kernel. A fixmap area can be used to map memory mapped IO area
of EFI runtime service, the code and data area of EFI runtime service
are always mapped to same address in direct map area.

Best Regards,
Huang Ying

Re: [PATCH 0/5] x86_64 EFI support -v3

[Eric W. Biederman]

"Huang, Ying" <ying.huang@intel.com> writes:

> On Tue, 2007-07-31 at 12:47 +0800, Eric W. Biederman wrote:
>> Using efi_set_virtual means kdump doesn't work which means that no
>> one is going to use this in a prebuilt kernel.
>
> It is possible to make kexec/kdump work with EFI virtual mode, in
> following ways:

Yes we can make it work if we absolutely have to.  There doesn't
seem to be much point to jump through hoops for a feature
we don't need.  A fixed virtual address and a fixed physical
address have essentially the same implications for the kernel.
A fixed address we are stuck with and if the kernel diverge
by very much we have problems.

Since there are people actively investigating things like booting
OpenBSD via kexec things get even worse. Nothing hardly runs
on ia64 so that issue doesn't come up.

As for not using EFI at all.  If we can avoid it/not use it in the
dump kernel there is very little point in having it in the primary
kernel.

Especially since it appears EFI is only 64 bit or only 32 bit I
think we need to forget about efi_set_virtual_address and only
call into EFI via a trampoline that set things up properly
whatever mode EFI is running in on that particular box.

So far there don't seem to be any compelling advantages to running
EFI in virtual address mode and several compelling disadvantages
included having to change the permissions on the kernels memory
map to running EFI in virtual mode.

Please let's stick to a physical mode trampoline and only revisit
the topic when users start having problems because of the performance
hit of going through our trampoline to the EFI runtime services.

Eric

Re: [PATCH 0/5] x86_64 EFI support -v3

[Andi Kleen]

ebiederm@xmission.com (Eric W. Biederman) writes:
> 
> Since there are people actively investigating things like booting
> OpenBSD via kexec things get even worse. Nothing hardly runs
> on ia64 so that issue doesn't come up.

If you want to do a popularity contest I expect there are far more ia64 
linux users than kexec-of-openbsd users.

> As for not using EFI at all.  If we can avoid it/not use it in the
> dump kernel there is very little point in having it in the primary
> kernel.

One interesting area is to use it for saving oops data. But
that has to be simple. I'm not sure complicated context switches
are a good idea here. 

However I agree it probably doesn't make sense to do virtual
mode just for the clock services -- so far we seem to be fine
just talking to the hardware directly.

> So far there don't seem to be any compelling advantages to running
> EFI in virtual address mode and several compelling disadvantages
> included having to change the permissions on the kernels memory
> map to running EFI in virtual mode.

I don't think it's a big issue to have a few less NX bits. Just
the original patch for it was ugly.
 
> Please let's stick to a physical mode trampoline and only revisit
> the topic when users start having problems because of the performance
> hit of going through our trampoline to the EFI runtime services.

So you want to switch to new page tables when calling EFI services
after boot? 

Potential problems:
- Interrupts have to be disabled. Is that ok? 
- When EFI BIOS start crashing how do we set up exception handlers
for this? 

I guess it would get complex long term. Also doesn't really sound 
attractive.

-Andi