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

[Eric W. Biederman]

"Huang, Ying" <ying.huang@xxxxxxxxx> 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
-
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/