[PATCH 0/3] x86_64 EFI runtime service support

[PATCH 0/3] x86_64 EFI runtime service support

[Huang, Ying]

Following sets of patches add EFI/UEFI (Unified Extensible Firmware
Interface) runtime services support to x86_64 architecture. The
patches have been tested against 2.6.23-rc2 kernel on Intel platforms
with EFI1.10 and UEFI2.0 firmware. This patch set is based on previous
x86_64 EFI boot support patch set.

Known issues:

- Virtual mode support is still retained in this patch. The fixmap is
  used to map IO region used by EFI runtime service. This makes kexec
  workable under EFI.

- The variable efi_enabled is used throughout across architectures 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. But, the EFI time runtime service is changed to
 use function pointers, and the EFI reset_system runtime service is
 changed to use reboot_type variable.

Looking forward to your comments,

Best Regards,
Huang Ying

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Andrew Morton]

On Mon, 13 Aug 2007 15:30:19 +0800
"Huang, Ying" <ying.huang@intel.com> wrote:

> Following sets of patches add EFI/UEFI (Unified Extensible Firmware
> Interface) runtime services support to x86_64 architecture.

I had to rework these a bit due to clashes with
x86_64-add-acpi-reboot-option.patch.  Please check carefully that
everything landed OK.

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Huang, Ying]

On Thu, 2007-08-16 at 06:42 +0800, Andrew Morton wrote:
> On Mon, 13 Aug 2007 15:30:19 +0800
> "Huang, Ying" <ying.huang@intel.com> wrote:
> 
> > Following sets of patches add EFI/UEFI (Unified Extensible Firmware
> > Interface) runtime services support to x86_64 architecture.
> 
> I had to rework these a bit due to clashes with
> x86_64-add-acpi-reboot-option.patch.  Please check carefully that
> everything landed OK.
> 
OK, I will be more careful next time.

Best Regards,
Huang Ying

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Andi Kleen]

Huang, Ying wrote:
> On Thu, 2007-08-16 at 06:42 +0800, Andrew Morton wrote:
>> On Mon, 13 Aug 2007 15:30:19 +0800
>> "Huang, Ying" <ying.huang@intel.com> wrote:
>>
>>> Following sets of patches add EFI/UEFI (Unified Extensible Firmware
>>> Interface) runtime services support to x86_64 architecture.
>> I had to rework these a bit due to clashes with
>> x86_64-add-acpi-reboot-option.patch.  Please check carefully that
>> everything landed OK.
>>
> OK, I will be more careful next time.


Has the zero page documentation and version numbering project
made any progress? I think we cannot merge this without at least
the version number

-Andi

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Huang, Ying]

On Thu, 2007-08-16 at 16:11 +0200, Andi Kleen wrote:
> Huang, Ying wrote:
> > On Thu, 2007-08-16 at 06:42 +0800, Andrew Morton wrote:
> >> On Mon, 13 Aug 2007 15:30:19 +0800
> >> "Huang, Ying" <ying.huang@intel.com> wrote:
> >>
> >>> Following sets of patches add EFI/UEFI (Unified Extensible Firmware
> >>> Interface) runtime services support to x86_64 architecture.
> >> I had to rework these a bit due to clashes with
> >> x86_64-add-acpi-reboot-option.patch.  Please check carefully that
> >> everything landed OK.
> >>
> > OK, I will be more careful next time.
> 
> 
> Has the zero page documentation and version numbering project
> made any progress? I think we cannot merge this without at least
> the version number

OK, I will work on the zero page documentation and version numbering
project.

Best Regards,
Huang Ying

Re: [PATCH 0/3] x86_64 EFI runtime service support

[H. Peter Anvin]

Huang, Ying wrote:
>>
>> Has the zero page documentation and version numbering project
>> made any progress? I think we cannot merge this without at least
>> the version number
> 

More than that.  You need to be able to boot a 32-bit kernel on a 64-bit
system, so anything that breaks that is a nonstarter.  Of course, if EFI
itself inherently breaks that, then, well, that's just another reason to
avoid EFI like the plague, but I can't think that even EFI is that broken.

> OK, I will work on the zero page documentation and version numbering
> project.

There already is documentation (Documentation/i386/zero-page.txt); as
far as version numbering, that means sticking in a field with a number,
and adding a magic number (since there isn't anything that guarantees
that fields are otherwise zero.)

Anything that conforms to the updated standard should guarantee
undefined fields are zero.

However, we also have an immediate need to define how to grow past 4K,
and if we're going to have a major revision in mechanism I would like to
see that happen now.

I propose that, in addition to the aforementioned version number and
magic fields, we add a pointer, which should be the last pointer added
that doesn't point into I/O space or reserved memory (i.e. memory that
is off limit anyway for the operating system.)

This pointer should point to a linked list of suggested form:

struct setup_data {
	u64 next;
	u32 type;
	u32 len;
	u8 data[];
};

This can thus encapsulate large objects as necessary, and the early
kernel entry can linearize them if it needs to move them out of the way.
 Better yet, this information can be made available to sysfs for
debuggability, and/or use by kexec.

I haven't heard anything from the kexec people on this, and they are the
main users of the PM entrypoint as far as I can tell.

Also, is there a maintainer for 32-bit EFI?

	-hpa


	-hpa

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Huang, Ying]

On Fri, 2007-08-17 at 09:11 -0700, H. Peter Anvin wrote:
> Huang, Ying wrote:
> >>
> >> Has the zero page documentation and version numbering project
> >> made any progress? I think we cannot merge this without at least
> >> the version number
> > 
> 
> More than that.  You need to be able to boot a 32-bit kernel on a 64-bit
> system, so anything that breaks that is a nonstarter.  Of course, if EFI
> itself inherently breaks that, then, well, that's just another reason to
> avoid EFI like the plague, but I can't think that even EFI is that broken.

The 32-bit EFI runtime service and 64-bit EFI runtime service is not
binary compatible. So, it is difficult for 32-bit Linux kernel to
support 64-bit EFI runtime service on x86_64 machine with 64-bit EFI
firmware. The 32-bit Linux kernel must switch between 32-bit mode and
64-bit mode before and after calling EFI runtime service. It is a little
complex too for 64-bit Linux kernel to support 32-bit EFI runtime
service on x86_64 machine with 32-bit EFI firmware.

But EFI runtime service is not essential for Linux booting, the Linux
kernel can boot (even operate) normally without EFI runtime service
support.

Summary, on x86_64 machine:

firmware	Linux kernel		boot		runtime service
------------------------------------------------------------------------
EFI-64		32-bit			Yes		No
EFI-64		64-bit			Yes		Yes
EFI-32		32-bit			Yes		Yes
EFI-32		64-bit			Yes		No

So, Why we add support for EFI runtime service? I think there are at
least following reasons:

1. Some legacy hardware (such as CMOS clock) may be replaced silently in
the future when most legacy BIOS is replaced by EFI firmware. With EFI
runtime service, the new hardware can be supported immediately.

2. EFI variable service can be used to store some information cross
rebooting, such as the OOPS information can be saved into flash through
EFI variable service as proposed by Andi Kleen.

3. EFI reboot service can be used to support warm reboot.

The problem remains is that should we support 32-bit EFI runtime service
in 64-bit Linux kernel, or should we support 64-bit EFI runtime service
in 32-bit Linux kernel? I think we can add these supports later if
needed.

> > OK, I will work on the zero page documentation and version numbering
> > project.
> 
> There already is documentation (Documentation/i386/zero-page.txt); as
> far as version numbering, that means sticking in a field with a number,
> and adding a magic number (since there isn't anything that guarantees
> that fields are otherwise zero.)
> 
> Anything that conforms to the updated standard should guarantee
> undefined fields are zero.

Yes. This is necessary for extensibility.

> However, we also have an immediate need to define how to grow past 4K,
> and if we're going to have a major revision in mechanism I would like to
> see that happen now.
> 
> I propose that, in addition to the aforementioned version number and
> magic fields, we add a pointer, which should be the last pointer added
> that doesn't point into I/O space or reserved memory (i.e. memory that
> is off limit anyway for the operating system.)
> 
> This pointer should point to a linked list of suggested form:
> 
> struct setup_data {
> 	u64 next;
> 	u32 type;
> 	u32 len;
> 	u8 data[];
> };
> 
> This can thus encapsulate large objects as necessary, and the early
> kernel entry can linearize them if it needs to move them out of the way.
>  Better yet, this information can be made available to sysfs for
> debuggability, and/or use by kexec.

This is a good interface. It is extensible. In the future, when we need
more boot information, we can just define a new type.

And with this interface, it is not necessary to export the "zero page
protocol" as a external boot protocol (ABI) of the kernel. So, I
proposed that:

1. Keep "zero page" as a internal interface, even make that explicitly
in the document.
2. Increase the version number of standard boot protocol.
3. Append this pointer (pointed to linked list of struct setup_data) to
standard boot protocol.
4. Define a set of types of struct setup_data, each for one zero_page
field.
5. The kernel 16-bit setup code generates "linked list of struct
setup_data" instead of generating "zero page". On machine with BIOS
other than legacy BIOS (such as EFI, LinuxBIOS, etc), the bootloader
(incuding kexec) generates the "linked list of struct setup_data"
instead of generating "zero page" too.
6. In kernel early booting code, the "zero page" is generated from the
"linked list of struct setup_data".
7. In the future, most booting code uses "linked list of struct
setup_data" directly instead of "zero page".

So, we need not define a new boot protocol, just extend the standard
boot protocol. The version number and magic need not to be added to
"zero page". But, at the same time, the bootloader on EFI, LinuxBIOS and
kexec must be changed accordingly.

> I haven't heard anything from the kexec people on this, and they are the
> main users of the PM entrypoint as far as I can tell.
> 
> Also, is there a maintainer for 32-bit EFI?

As far as I know, There is no maintainer for 32-bit EFI.

Best Regards,
Huang Ying

Re: [PATCH 0/3] x86_64 EFI runtime service support

[H. Peter Anvin]

Huang, Ying wrote:
>>
>> I propose that, in addition to the aforementioned version number and
>> magic fields, we add a pointer, which should be the last pointer added
>> that doesn't point into I/O space or reserved memory (i.e. memory that
>> is off limit anyway for the operating system.)
>>
>> This pointer should point to a linked list of suggested form:
>>
>> struct setup_data {
>> 	u64 next;
>> 	u32 type;
>> 	u32 len;
>> 	u8 data[];
>> };
>>
>> This can thus encapsulate large objects as necessary, and the early
>> kernel entry can linearize them if it needs to move them out of the way.
>>  Better yet, this information can be made available to sysfs for
>> debuggability, and/or use by kexec.
> 
> This is a good interface. It is extensible. In the future, when we need
> more boot information, we can just define a new type.
> 
> And with this interface, it is not necessary to export the "zero page
> protocol" as a external boot protocol (ABI) of the kernel. So, I
> proposed that:
> 
> 1. Keep "zero page" as a internal interface, even make that explicitly
> in the document.
> 2. Increase the version number of standard boot protocol.
> 3. Append this pointer (pointed to linked list of struct setup_data) to
> standard boot protocol.
> 4. Define a set of types of struct setup_data, each for one zero_page
> field.
> 5. The kernel 16-bit setup code generates "linked list of struct
> setup_data" instead of generating "zero page". On machine with BIOS
> other than legacy BIOS (such as EFI, LinuxBIOS, etc), the bootloader
> (incuding kexec) generates the "linked list of struct setup_data"
> instead of generating "zero page" too.
> 6. In kernel early booting code, the "zero page" is generated from the
> "linked list of struct setup_data".
> 7. In the future, most booting code uses "linked list of struct
> setup_data" directly instead of "zero page".
> 
> So, we need not define a new boot protocol, just extend the standard
> boot protocol. The version number and magic need not to be added to
> "zero page". But, at the same time, the bootloader on EFI, LinuxBIOS and
> kexec must be changed accordingly.
> 

I think this is too radical of a change to be practical.  Instead, I
propose the following:

- "struct boot_params" (the zeropage) is kept as a legacy interface.
%esi will continue to point to it on entry to the 32-bit entrypoint
(presumably that is %rsi on entry to a 64-bit entrypoint?)

- We add a magic number and a pointer chain to the zeropage.  The
presence of the magic number indicates that:

	- Any unused fields in the zeropage is zero;
	- The pointer chain is valid (unless zero);
	- The old "HD info" fields (and possibly the EDD fields)
	  can be recycled.

What a post-upgrade kernel should do upon encountering an old structure
(sans magic) is to zero out any fields that wasn't present in the legacy
structure definition, including the pointer chain, then it can use it as-is.

We should avoid adding fields to the zero_page after that, unless
necessary for backwards compatibility reasons.  In general, new data
items should be added as pointer capabilities.

Compare this to the legacy PCI header vs. PCI capabilities.

	-hpa

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Huang, Ying]

On Mon, 2007-08-20 at 10:12 -0700, H. Peter Anvin wrote:
> Huang, Ying wrote:
> >>
> >> I propose that, in addition to the aforementioned version number and
> >> magic fields, we add a pointer, which should be the last pointer added
> >> that doesn't point into I/O space or reserved memory (i.e. memory that
> >> is off limit anyway for the operating system.)
> >>
> >> This pointer should point to a linked list of suggested form:
> >>
> >> struct setup_data {
> >> 	u64 next;
> >> 	u32 type;
> >> 	u32 len;
> >> 	u8 data[];
> >> };
> >>

I think the "next" field can be u32 instead of u64. Because the linked
list of struct setup_data is prepared by bootloader, which can control
the memory location.

To facilitate the kernel early mapping, I think the memory location of
"linked list" should be kept to a predefined range (in the real mode
memory area?).

> > So, we need not define a new boot protocol, just extend the standard
> > boot protocol. The version number and magic need not to be added to
> > "zero page". But, at the same time, the bootloader on EFI, LinuxBIOS and
> > kexec must be changed accordingly.
> > 
> 
> I think this is too radical of a change to be practical.  Instead, I
> propose the following:
> 
> - "struct boot_params" (the zeropage) is kept as a legacy interface.
> %esi will continue to point to it on entry to the 32-bit entrypoint
> (presumably that is %rsi on entry to a 64-bit entrypoint?)
> 
> - We add a magic number and a pointer chain to the zeropage.  The
> presence of the magic number indicates that:
> 
> 	- Any unused fields in the zeropage is zero;
> 	- The pointer chain is valid (unless zero);
> 	- The old "HD info" fields (and possibly the EDD fields)
> 	  can be recycled.

Previously, I think the "zero page" is not external formally, so we can
ignore the user. But it is used by some bootloaders. So your proposal
may be better, especially for these bootloaders.

I think something others need to be done:

- Increase the version number of standard boot protocol.
- Add the contents of zero page into standard boot protocol document as
a optional part for 32-bit entry (and 64-bit entry?).
- Make 32-bit (64-bit?) boot protocol formal in standard boot protocol
document.

> What a post-upgrade kernel should do upon encountering an old structure
> (sans magic) is to zero out any fields that wasn't present in the legacy
> structure definition, including the pointer chain, then it can use it as-is.

As for the magic number in zero page, do you think it should be used
only by 16-bit kernel setup code?

> We should avoid adding fields to the zero_page after that, unless
> necessary for backwards compatibility reasons.  In general, new data
> items should be added as pointer capabilities.

Yes.

> Compare this to the legacy PCI header vs. PCI capabilities.

Best Regards,
Huang Ying

Re: [PATCH 0/3] x86_64 EFI runtime service support

[H. Peter Anvin]

Huang, Ying wrote:
> 
> I think the "next" field can be u32 instead of u64. Because the linked
> list of struct setup_data is prepared by bootloader, which can control
> the memory location.
> 

That's making some pretty serious assumptions on future boot loaders and
environments.

> Previously, I think the "zero page" is not external formally, so we can
> ignore the user. But it is used by some bootloaders. So your proposal
> may be better, especially for these bootloaders.
> 
> I think something others need to be done:
> 
> - Increase the version number of standard boot protocol.
> - Add the contents of zero page into standard boot protocol document as
> a optional part for 32-bit entry (and 64-bit entry?).

Probably, yes.

> As for the magic number in zero page, do you think it should be used
> only by 16-bit kernel setup code?

Absolutely not.

	-hpa

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Huang, Ying]

On Mon, 2007-08-20 at 20:54 -0700, H. Peter Anvin wrote:
> Huang, Ying wrote:
> > 
> > I think the "next" field can be u32 instead of u64. Because the linked
> > list of struct setup_data is prepared by bootloader, which can control
> > the memory location.
> > 
> 
> That's making some pretty serious assumptions on future boot loaders and
> environments.

I think this assumption is reasonable. The range of u32 is big enough
for a boot protocol. Do we need to convert that to u128, when 128-bit
machine emerges?

I think there is another possible scheme. An array with variable length
element instead of a linked list can be used too. The type of element is
as follow:

struct setup_data {
	u32 type;
	u32 len;
	u8 data[0];
};

The next element is at (char *)&setup_data + len. The boot information
will be held in a block of memory. This makes initial memory mapping of
kernel more easily. And the memory location of boot information may need
to be kept in a limited range too.

> > As for the magic number in zero page, do you think it should be used
> > only by 16-bit kernel setup code?
> 
> Absolutely not.

If a old boot loader (not aware of magic number) is used to load the new
kernel image. The old boot loader will set the magic number to zero.
Does this mean that the new kernel image can not be booted by the old
boot loader.

Best Regards,
Huang Ying

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Andi Kleen]

> - "struct boot_params" (the zeropage) is kept as a legacy interface.

Legacy interface for what?  Just for kexec utils which never should
have been using it anyways keeping backwards cruft around seems
misplac.ed

-Andi

Re: [PATCH 0/3] x86_64 EFI runtime service support

[H. Peter Anvin]

Andi Kleen wrote:
>> - "struct boot_params" (the zeropage) is kept as a legacy interface.
> 
> Legacy interface for what?  Just for kexec utils which never should
> have been using it anyways keeping backwards cruft around seems
> misplac.ed

Worse.  LinuxBIOS.  :(

	-hpa

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Andi Kleen]

On Tue, Aug 21, 2007 at 03:41:44AM -0700, H. Peter Anvin wrote:
> Andi Kleen wrote:
> >> - "struct boot_params" (the zeropage) is kept as a legacy interface.
> > 
> > Legacy interface for what?  Just for kexec utils which never should
> > have been using it anyways keeping backwards cruft around seems
> > misplac.ed
> 
> Worse.  LinuxBIOS.  :(

Sigh. Perhaps it should be renamed AntiLinuxBios: it seems to be actively adverse.

-Andi

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Yinghai Lu]

On 8/21/07, Andi Kleen <ak@suse.de> wrote:
> On Tue, Aug 21, 2007 at 03:41:44AM -0700, H. Peter Anvin wrote:
> > Andi Kleen wrote:
> > >> - "struct boot_params" (the zeropage) is kept as a legacy interface.
> > >
> > > Legacy interface for what?  Just for kexec utils which never should
> > > have been using it anyways keeping backwards cruft around seems
> > > misplac.ed
> >
> > Worse.  LinuxBIOS.  :(
>
> Sigh. Perhaps it should be renamed AntiLinuxBios: it seems to be actively adverse.

current LinuxBIOS's path: the elfboot in LinuxBIOS will prepare the
e820 table, and jump to startup_32 in kernel. is that not good and
simple? kernel is not supposed to switch back and forth to get such
memmap...

Why not using ACPI mean AntiLinux?

YH

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Andi Kleen]

> current LinuxBIOS's path: the elfboot in LinuxBIOS will prepare the
> e820 table, and jump to startup_32 in kernel. is that not good and
> simple? 

The problem is that the zero page cannot be changed at all in this
setup. Or rather it can be only changed by breaking LinuxBios.

-Andi

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Yinghai Lu]

On 8/21/07, Andi Kleen <ak@suse.de> wrote:
> > current LinuxBIOS's path: the elfboot in LinuxBIOS will prepare the
> > e820 table, and jump to startup_32 in kernel. is that not good and
> > simple?
>
> The problem is that the zero page cannot be changed at all in this
> setup. Or rather it can be only changed by breaking LinuxBios.

So you want to construct e820 table in x86_boot_params from LinuxBIOS
memmap in head64.c?

YH

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Andi Kleen]

On Tue, Aug 21, 2007 at 11:43:38PM -0700, Yinghai Lu wrote:
> On 8/21/07, Andi Kleen <ak@suse.de> wrote:
> > > current LinuxBIOS's path: the elfboot in LinuxBIOS will prepare the
> > > e820 table, and jump to startup_32 in kernel. is that not good and
> > > simple?
> >
> > The problem is that the zero page cannot be changed at all in this
> > setup. Or rather it can be only changed by breaking LinuxBios.
> 
> So you want to construct e820 table in x86_boot_params from LinuxBIOS
> memmap in head64.c?

The short term fix is probably to just add a version number to
the zero page and make sure new changes only add stuff to the
end and the kernel has reasonable compat code for old versions.

Then LinuxBIOS would need to be changed to supply that version number.

Perhaps it could also include a checksum just to detect old BIOS
that don't supply a version number.

Long term I'm not sure. Adding LinuxBIOS specific tables also doesn't
sound very attractive.

-Andi

Re: [PATCH 0/3] x86_64 EFI runtime service support

[huang ying]

On 8/22/07, Andi Kleen <ak@suse.de> wrote:
> The short term fix is probably to just add a version number to
> the zero page and make sure new changes only add stuff to the
> end and the kernel has reasonable compat code for old versions.
>
> Then LinuxBIOS would need to be changed to supply that version number.
>
> Perhaps it could also include a checksum just to detect old BIOS
> that don't supply a version number.
>
> Long term I'm not sure. Adding LinuxBIOS specific tables also doesn't
> sound very attractive.

My proposal: Use Peter proposed "linked list of struct setup_data"
style boot protocol as long term goal.

To smooth the transforming process, the following back compatible
scheme can be taken:

1. Keep zero page as an informal external boot protocol, and marked it
as deprecated for external usage.
2. Add a magic number to standard boot protocol, which is set by
bootloader to indicate the new style or old style boot protocol is
used.
3. Add the pointer to "linked list of struct setup_data" to standard
boot protocol.
4. If kernel is booted with correct magic number, the kernel will
convert "linked list" to zero page, or use "linked list" directly. If
kernel is booted with incorrect magic number, the kernel will use the
"zero page" from bootloader or convert "zero page" to "linked list".

The current kexec/LinuxBIOS using "informal" zero page protocol can
boot the new kernel, because the "zero page" protocol is kept for
short term. New version of bootloader should use "linked list"
protocol instead of "zero page" protocol.

In the future, when all bootloaders use new protocol, the "zero page"
is made internal formally.

Any comment is welcome.

Best Regards,
Huang Ying

Re: [PATCH 0/3] x86_64 EFI runtime service support

[H. Peter Anvin]

huang ying wrote:
> 
> My proposal: Use Peter proposed "linked list of struct setup_data"
> style boot protocol as long term goal.
> 
> To smooth the transforming process, the following back compatible
> scheme can be taken:
> 
> 1. Keep zero page as an informal external boot protocol, and marked it
> as deprecated for external usage.
> 2. Add a magic number to standard boot protocol, which is set by
> bootloader to indicate the new style or old style boot protocol is
> used.
> 3. Add the pointer to "linked list of struct setup_data" to standard
> boot protocol.
> 4. If kernel is booted with correct magic number, the kernel will
> convert "linked list" to zero page, or use "linked list" directly. If
> kernel is booted with incorrect magic number, the kernel will use the
> "zero page" from bootloader or convert "zero page" to "linked list".
> 

You're making it needlessly complicated.

	-hpa

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Huang, Ying]

On Thu, 2007-08-23 at 00:28 +0800, H. Peter Anvin wrote:
> huang ying wrote:
> >
> > My proposal: Use Peter proposed "linked list of struct setup_data"
> > style boot protocol as long term goal.
> >
> > To smooth the transforming process, the following back compatible
> > scheme can be taken:
> >
> > 1. Keep zero page as an informal external boot protocol, and marked
> it
> > as deprecated for external usage.
> > 2. Add a magic number to standard boot protocol, which is set by
> > bootloader to indicate the new style or old style boot protocol is
> > used.
> > 3. Add the pointer to "linked list of struct setup_data" to standard
> > boot protocol.
> > 4. If kernel is booted with correct magic number, the kernel will
> > convert "linked list" to zero page, or use "linked list" directly.
> If
> > kernel is booted with incorrect magic number, the kernel will use
> the
> > "zero page" from bootloader or convert "zero page" to "linked list".
> >
> You're making it needlessly complicated.

My intention is that we have 3 possible schemes for kernel to use boot
information.

1. Use "linked list" only. Then if booted with old bootloader which uses
"zero page" protocol, the "zero page" information provided by bootloader
should be converted to "linked list" for other part of kernel to use.

2. Use "zero page" only. Then if booted with new bootloader which
provides "linked list" but not "zero page", the "linked list"
information provided by bootloader should be converted to "zero page"
for other part of kernel to use.

3. Use "zero page" + "linked list". Then if booted with old bootloader,
the "linked list" is empty. If booted with new bootloader, both the
"zero page" and "linked list" are used.

We need to choose one from schemes above.

- The scheme 1 appears the most clean one.
- The scheme 2 has 4k "zero page" constraint, so it is not good.
- The scheme 3 is easiest to be implemented.

Personally, I prefer the scheme 1. But the scheme 3 is OK too.

Best Regards,
Huang Ying

Re: [PATCH 0/3] x86_64 EFI runtime service support

[H. Peter Anvin]

Huang, Ying wrote:
> 
> My intention is that we have 3 possible schemes for kernel to use boot
> information.
> 

That's not an intention, it's an observation.

> 1. Use "linked list" only. Then if booted with old bootloader which uses
> "zero page" protocol, the "zero page" information provided by bootloader
> should be converted to "linked list" for other part of kernel to use.
> 
> 2. Use "zero page" only. Then if booted with new bootloader which
> provides "linked list" but not "zero page", the "linked list"
> information provided by bootloader should be converted to "zero page"
> for other part of kernel to use.
> 
> 3. Use "zero page" + "linked list". Then if booted with old bootloader,
> the "linked list" is empty. If booted with new bootloader, both the
> "zero page" and "linked list" are used.
> 
> We need to choose one from schemes above.
> 
> - The scheme 1 appears the most clean one.
> - The scheme 2 has 4k "zero page" constraint, so it is not good.
> - The scheme 3 is easiest to be implemented.
> 
> Personally, I prefer the scheme 1. But the scheme 3 is OK too.

Scheme 3 is the only realistic way to move away from the current
situation (scheme 2).  Scheme 1 just means unnecessary divergences
between codepaths.  If it wasn't for LunacyBIOS and (to a smaller
extent) kexec, we would probably be OK, but that's not the real world.

	-hpa

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Eric W. Biederman]

Andi Kleen <ak@suse.de> writes:

> On Tue, Aug 21, 2007 at 11:43:38PM -0700, Yinghai Lu wrote:
>> On 8/21/07, Andi Kleen <ak@suse.de> wrote:
>> > > current LinuxBIOS's path: the elfboot in LinuxBIOS will prepare the
>> > > e820 table, and jump to startup_32 in kernel. is that not good and
>> > > simple?
>> >
>> > The problem is that the zero page cannot be changed at all in this
>> > setup. Or rather it can be only changed by breaking LinuxBios.
>> 
>> So you want to construct e820 table in x86_boot_params from LinuxBIOS
>> memmap in head64.c?
>
> The short term fix is probably to just add a version number to
> the zero page and make sure new changes only add stuff to the
> end and the kernel has reasonable compat code for old versions.
>
> Then LinuxBIOS would need to be changed to supply that version number.

To clear up a small misconception.  It isn't LinuxBIOS but the LinuxBIOS
bootloaders that need to be changed.  A subtle distinction. 

> Perhaps it could also include a checksum just to detect old BIOS
> that don't supply a version number.
>
> Long term I'm not sure. Adding LinuxBIOS specific tables also doesn't
> sound very attractive.

Second thinking of this as the empty_zero_page is probably the wrong
mental model of how things work.

At least until HPA's recent changes bzImage is organized as follows:

bootsector (512 bytes)
Various boot protocol variables.
  Some filled in or read by bootloaders 
  some filled in by the real mode code.
real mode code.
32bit Data+Code to load at 1MB.

The empty zero page really is the first 4K or whatever of
the current setup.S.  Any sane bootloader loading the bzImage
will just load the 16bit portion and overwrite the variables
it knows how to fill in.

To ensure there was enough space the code did:

> trampoline:    call    start_of_setup
>                .align 16
>                                        # The offset at this point is 0x240
>                .space  (0xeff-0x240+1) # E820 & EDD space (ending at 0xeff)

We already have the boot protocol revision and if this is something we
are going to support that should be enough of a revision field.  If
the bootloader can't support what the kernel is doing the bootloader
should just give up.  We have major and minor fields there so we can
distinguish between compatible and incompatible changes as well.

Further that arrangement ensures all of the data fields are initialized.

Further there is no 4K limit when used that way except as a convention
and as an implementation detail.  The bootloaders don't care.  The
have to load all of the 16bit real mode section.

I need to review HPA changes a little more from his rewrite in C.
I think he messed up with the guarantee of initialization in that
rewrite, but I'm not quite certain.

Eric

Re: [PATCH 0/3] x86_64 EFI runtime service support

[H. Peter Anvin]

Eric W. Biederman wrote:
> I need to review HPA changes a little more from his rewrite in C.
> I think he messed up with the guarantee of initialization in that
> rewrite, but I'm not quite certain.

I beg to differ, sir!

In my rewritten code, the "zeropage" is 4K of zero-initialized .bss,
into which the setup header is copied.

So any undefined field will be initialized to zero, not to whatever
assembly code happened to be there.

You know that with the old code, we got boot failures after changing the
error message in the dummy boot sector?

	-hpa

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Yinghai Lu]

On 8/21/07, Andi Kleen <ak@suse.de> wrote:
> > current LinuxBIOS's path: the elfboot in LinuxBIOS will prepare the
> > e820 table, and jump to startup_32 in kernel. is that not good and
> > simple?
>
> The problem is that the zero page cannot be changed at all in this
> setup. Or rather it can be only changed by breaking LinuxBios.

FYI.

current mkelfImage is using
#define REAL_MODE_DATA_LOC      0x20000
the %esi will be 0x20000

YH

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Andrew Morton]

On Mon, 13 Aug 2007 15:30:19 +0800
"Huang, Ying" <ying.huang@intel.com> wrote:

> Following sets of patches add EFI/UEFI (Unified Extensible Firmware
> Interface) runtime services support to x86_64 architecture.

OK, we have a major trainwreck when these patches meet Peter's
get-newsetup.patch.

I'm halfway into fixing it when I see this.  You have:

 #define SYS_DESC_TABLE (*(struct sys_desc_table_struct*)(PARAM+0xa0))
+#define EFI_LOADER_SIG ((unsigned char *)(PARAM+0x1c0))
+#define EFI_MEMDESC_SIZE (*((unsigned int *) (PARAM+0x1c4)))
+#define EFI_MEMDESC_VERSION (*((unsigned int *) (PARAM+0x1c8)))
+#define EFI_MEMMAP_SIZE (*((unsigned int *) (PARAM+0x1cc)))
+#define EFI_MEMMAP (*((unsigned long *)(PARAM+0x1d0)))
+#define EFI_SYSTAB (*((unsigned long *)(PARAM+0x1d8)))
 #define MOUNT_ROOT_RDONLY (*(unsigned short *) (PARAM+0x1F2))

But Peter's include/asm-i386/bootparam.h has:

struct efi_info {
	u32 _pad1;
	u32 efi_systab;
	u32 efi_memdesc_size;
	u32 efi_memdesc_version;
	u32 efi_memmap;
	u32 efi_memmap_size;
	u32 _pad2[2];
};

/* The so-called "zeropage" */
struct boot_params {
	struct screen_info screen_info;			/* 0x000 */
	struct apm_bios_info apm_bios_info;		/* 0x040 */
	u8  _pad2[12];					/* 0x054 */
	struct ist_info ist_info;			/* 0x060 */
	u8  _pad3[16];					/* 0x070 */
	u8  hd0_info[16];	/* obsolete! */		/* 0x080 */
	u8  hd1_info[16];	/* obsolete! */		/* 0x090 */
	struct sys_desc_table sys_desc_table;		/* 0x0a0 */
	u8  _pad4[144];					/* 0x0b0 */
	struct edid_info edid_info;			/* 0x140 */
	struct efi_info efi_info;			/* 0x1c0 */
	u32 alt_mem_k;					/* 0x1e0 */

So for example, Peter has memdesc_size at 0x1c8 and you have it at 0x1c4.

I'll give up and will drop the EFI patches.  I'd suggest that you work with
Peter on getting these patches integrated.

Re: [PATCH 0/3] x86_64 EFI runtime service support

[H. Peter Anvin]

Andrew Morton wrote:
> On Mon, 13 Aug 2007 15:30:19 +0800
> "Huang, Ying" <ying.huang@intel.com> wrote:
> 
>> Following sets of patches add EFI/UEFI (Unified Extensible Firmware
>> Interface) runtime services support to x86_64 architecture.
> 
> OK, we have a major trainwreck when these patches meet Peter's
> get-newsetup.patch.
> 
> I'm halfway into fixing it when I see this.  You have:
> 
>  #define SYS_DESC_TABLE (*(struct sys_desc_table_struct*)(PARAM+0xa0))
> +#define EFI_LOADER_SIG ((unsigned char *)(PARAM+0x1c0))
> +#define EFI_MEMDESC_SIZE (*((unsigned int *) (PARAM+0x1c4)))
> +#define EFI_MEMDESC_VERSION (*((unsigned int *) (PARAM+0x1c8)))
> +#define EFI_MEMMAP_SIZE (*((unsigned int *) (PARAM+0x1cc)))
> +#define EFI_MEMMAP (*((unsigned long *)(PARAM+0x1d0)))
> +#define EFI_SYSTAB (*((unsigned long *)(PARAM+0x1d8)))
>  #define MOUNT_ROOT_RDONLY (*(unsigned short *) (PARAM+0x1F2))
> 

Please, no more of these kinds of macros.  We have already had
collisions.  Go ahead and redefine the efi_info structure if necessary,
but use fixed types (u8, u16, u32, u64), *NOT* unsigned long which is
different between i386 and x86-64.  Also keep in mind the boot code
might in the future be compiled with a 16-bit compiler, so assuming
"unsigned int" == 32 bits is also a Bad Thing.

> 
> I'll give up and will drop the EFI patches.  I'd suggest that you work with
> Peter on getting these patches integrated.
> 

Thanks.

	-hpa

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Huang, Ying]

On Thu, 2007-08-16 at 07:22 +0800, H. Peter Anvin wrote:
> Andrew Morton wrote:
> > On Mon, 13 Aug 2007 15:30:19 +0800
> > "Huang, Ying" <ying.huang@intel.com> wrote:
> >
> >> Following sets of patches add EFI/UEFI (Unified Extensible Firmware
> >> Interface) runtime services support to x86_64 architecture.
> >
> > OK, we have a major trainwreck when these patches meet Peter's
> > get-newsetup.patch.
> >
> > I'm halfway into fixing it when I see this.  You have:
> >
> >  #define SYS_DESC_TABLE (*(struct sys_desc_table_struct*)(PARAM
> +0xa0))
> > +#define EFI_LOADER_SIG ((unsigned char *)(PARAM+0x1c0))
> > +#define EFI_MEMDESC_SIZE (*((unsigned int *) (PARAM+0x1c4)))
> > +#define EFI_MEMDESC_VERSION (*((unsigned int *) (PARAM+0x1c8)))
> > +#define EFI_MEMMAP_SIZE (*((unsigned int *) (PARAM+0x1cc)))
> > +#define EFI_MEMMAP (*((unsigned long *)(PARAM+0x1d0)))
> > +#define EFI_SYSTAB (*((unsigned long *)(PARAM+0x1d8)))
> >  #define MOUNT_ROOT_RDONLY (*(unsigned short *) (PARAM+0x1F2))
> >
> 
> Please, no more of these kinds of macros.  We have already had
> collisions.  Go ahead and redefine the efi_info structure if
> necessary,

I missed the git-newsetup.patch. I will change EFI runtime service patch
according to the git-newsetup.patch.

One question:

The boot_params.efi_info.efi_systab is defined as u32. But it should be
u64 on x86_64, because it comes from firmware and is not controlled by
bootloader. But, changing it from u32 to u64 will break current i386 EFI
support, should we change it and fix the i386 EFI bootloader?

Best Regards,
Huang Ying

Re: [PATCH 0/3] x86_64 EFI runtime service support

[H. Peter Anvin]

Huang, Ying wrote:
> 
> One question:
> 
> The boot_params.efi_info.efi_systab is defined as u32. But it should be
> u64 on x86_64, because it comes from firmware and is not controlled by
> bootloader. But, changing it from u32 to u64 will break current i386 EFI
> support, should we change it and fix the i386 EFI bootloader?
> 

The other option is to have a union of a 32-bit and a 64-bit structure.
 I personally don't care, as long as it's consistent, but I think you
need to deal with the people working on EFI currently about that...

	-hpa

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Eric W. Biederman]

"H. Peter Anvin" <hpa@zytor.com> writes:

> Huang, Ying wrote:
>> 
>> One question:
>> 
>> The boot_params.efi_info.efi_systab is defined as u32. But it should be
>> u64 on x86_64, because it comes from firmware and is not controlled by
>> bootloader. But, changing it from u32 to u64 will break current i386 EFI
>> support, should we change it and fix the i386 EFI bootloader?
>> 
>
> The other option is to have a union of a 32-bit and a 64-bit structure.
>  I personally don't care, as long as it's consistent, but I think you
> need to deal with the people working on EFI currently about that...

It sounds like the 64bit EFI is currently binary incompatible with
the 32bit EFI.

Eric

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Eric W. Biederman]

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

> On Thu, 2007-08-16 at 07:22 +0800, H. Peter Anvin wrote:
>> Andrew Morton wrote:
>> > On Mon, 13 Aug 2007 15:30:19 +0800
>> > "Huang, Ying" <ying.huang@intel.com> wrote:
>> >
>> >> Following sets of patches add EFI/UEFI (Unified Extensible Firmware
>> >> Interface) runtime services support to x86_64 architecture.
>> >
>> > OK, we have a major trainwreck when these patches meet Peter's
>> > get-newsetup.patch.
>> >
>> > I'm halfway into fixing it when I see this.  You have:
>> >
>> >  #define SYS_DESC_TABLE (*(struct sys_desc_table_struct*)(PARAM
>> +0xa0))
>> > +#define EFI_LOADER_SIG ((unsigned char *)(PARAM+0x1c0))
>> > +#define EFI_MEMDESC_SIZE (*((unsigned int *) (PARAM+0x1c4)))
>> > +#define EFI_MEMDESC_VERSION (*((unsigned int *) (PARAM+0x1c8)))
>> > +#define EFI_MEMMAP_SIZE (*((unsigned int *) (PARAM+0x1cc)))
>> > +#define EFI_MEMMAP (*((unsigned long *)(PARAM+0x1d0)))
>> > +#define EFI_SYSTAB (*((unsigned long *)(PARAM+0x1d8)))
>> >  #define MOUNT_ROOT_RDONLY (*(unsigned short *) (PARAM+0x1F2))
>> >
>> 
>> Please, no more of these kinds of macros.  We have already had
>> collisions.  Go ahead and redefine the efi_info structure if
>> necessary,
>
> according to the git-newsetup.patch.
>
> One question:
>
> The boot_params.efi_info.efi_systab is defined as u32. But it should be
> u64 on x86_64, because it comes from firmware and is not controlled by
> bootloader. But, changing it from u32 to u64 will break current i386 EFI
> support, should we change it and fix the i386 EFI bootloader?

Be very very very  careful how you talk about this.

I have seen machines in the wild a 64bit processor and a 32bit EFI.
So this is not a linux architecture issue, or a cpu architecture
issue.  This is an EFI architecture issue.

This is an issue of do you have a 32bit or a 64bit EFI implementation
on your machine.  Which is very different.

We should be able to boot a 32bit kernel with a 64bit EFI.
We should be able to boot a 64bit kernel with a 32bit EFI.

Maybe our response is to ignore the information from elilo so
we don't attempt EFI runtime calls but the boot information should
be unambiguous.

So we need to be able to look at the data and answer these questions.
- Is EFI present?
- Is EFI 32bit?
- Is EFI 64bit?

Eric

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Yinghai Lu]

On 8/19/07, Eric W. Biederman <ebiederm@xmission.com> wrote:
> "Huang, Ying" <ying.huang@intel.com> writes:
>
> > On Thu, 2007-08-16 at 07:22 +0800, H. Peter Anvin wrote:
> >> Andrew Morton wrote:
> >> > On Mon, 13 Aug 2007 15:30:19 +0800
> >> > "Huang, Ying" <ying.huang@intel.com> wrote:
> >> >
> >> >> Following sets of patches add EFI/UEFI (Unified Extensible Firmware
> >> >> Interface) runtime services support to x86_64 architecture.
> >> >
> >> > OK, we have a major trainwreck when these patches meet Peter's
> >> > get-newsetup.patch.
> >> >
> >> > I'm halfway into fixing it when I see this.  You have:
> >> >
> >> >  #define SYS_DESC_TABLE (*(struct sys_desc_table_struct*)(PARAM
> >> +0xa0))
> >> > +#define EFI_LOADER_SIG ((unsigned char *)(PARAM+0x1c0))
> >> > +#define EFI_MEMDESC_SIZE (*((unsigned int *) (PARAM+0x1c4)))
> >> > +#define EFI_MEMDESC_VERSION (*((unsigned int *) (PARAM+0x1c8)))
> >> > +#define EFI_MEMMAP_SIZE (*((unsigned int *) (PARAM+0x1cc)))
> >> > +#define EFI_MEMMAP (*((unsigned long *)(PARAM+0x1d0)))
> >> > +#define EFI_SYSTAB (*((unsigned long *)(PARAM+0x1d8)))
> >> >  #define MOUNT_ROOT_RDONLY (*(unsigned short *) (PARAM+0x1F2))
> >> >
> >>
> >> Please, no more of these kinds of macros.  We have already had
> >> collisions.  Go ahead and redefine the efi_info structure if
> >> necessary,
> >
> > according to the git-newsetup.patch.
> >
> > One question:
> >
> > The boot_params.efi_info.efi_systab is defined as u32. But it should be
> > u64 on x86_64, because it comes from firmware and is not controlled by
> > bootloader. But, changing it from u32 to u64 will break current i386 EFI
> > support, should we change it and fix the i386 EFI bootloader?
>
> Be very very very  careful how you talk about this.
>
> I have seen machines in the wild a 64bit processor and a 32bit EFI.
> So this is not a linux architecture issue, or a cpu architecture
> issue.  This is an EFI architecture issue.
>
> This is an issue of do you have a 32bit or a 64bit EFI implementation
> on your machine.  Which is very different.
>
> We should be able to boot a 32bit kernel with a 64bit EFI.
> We should be able to boot a 64bit kernel with a 32bit EFI.
>
> Maybe our response is to ignore the information from elilo so
> we don't attempt EFI runtime calls but the boot information should
> be unambiguous.
>
> So we need to be able to look at the data and answer these questions.
> - Is EFI present?
> - Is EFI 32bit?
> - Is EFI 64bit?
>

someone told me that EFI PEI will be 32 bit ( for mem etc
initialization), and after that will be 64 bit, so the Run time
service will be 64 bit only, and it will only support 64 bit OS with
EFI. and they have another mode to emulate the legacy BIOS to boot
32bit OS.

YH

Re: [PATCH 0/3] x86_64 EFI runtime service support

[H. Peter Anvin]

Yinghai Lu wrote:
> 
> someone told me that EFI PEI will be 32 bit ( for mem etc
> initialization), and after that will be 64 bit, so the Run time
> service will be 64 bit only, and it will only support 64 bit OS with
> EFI. and they have another mode to emulate the legacy BIOS to boot
> 32bit OS.
> 

That seems unlikely for e.g. the existing Macs already in the field.

*Sigh.*

	-hpa

Re: [PATCH 0/3] x86_64 EFI runtime service support

[San Mehat]

On 8/20/07, H. Peter Anvin <hpa@zytor.com> wrote:
> Yinghai Lu wrote:
> >
> > someone told me that EFI PEI will be 32 bit ( for mem etc
> > initialization), and after that will be 64 bit, so the Run time
> > service will be 64 bit only, and it will only support 64 bit OS with
> > EFI. and they have another mode to emulate the legacy BIOS to boot
> > 32bit OS.
> >
>
> That seems unlikely for e.g. the existing Macs already in the field.
>
> *Sigh.*
>

Actually some implementations do run PEI in 32 bit and DXE/BDS in 64
bit; I'm not sure what Macintosh does though..

-san

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Matthew Garrett]

On Mon, Aug 20, 2007 at 10:05:11AM -0700, H. Peter Anvin wrote:
> Yinghai Lu wrote:
> > 
> > someone told me that EFI PEI will be 32 bit ( for mem etc
> > initialization), and after that will be 64 bit, so the Run time
> > service will be 64 bit only, and it will only support 64 bit OS with
> > EFI. and they have another mode to emulate the legacy BIOS to boot
> > 32bit OS.
> > 
> 
> That seems unlikely for e.g. the existing Macs already in the field.

In which direction? The existing Macs will boot a 32 (or 16) bit OS 
using legacy BIOS emulation. I've no idea if their EFI is 32 or 64 bit, 
though.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Huang, Ying]

On Sun, 2007-08-19 at 16:25 -0600, Eric W. Biederman wrote:
> "Huang, Ying" <ying.huang@intel.com> writes:
> >> > +#define EFI_LOADER_SIG ((unsigned char *)(PARAM+0x1c0))
> >> > +#define EFI_MEMDESC_SIZE (*((unsigned int *) (PARAM+0x1c4)))
> >> > +#define EFI_MEMDESC_VERSION (*((unsigned int *) (PARAM+0x1c8)))
> >> > +#define EFI_MEMMAP_SIZE (*((unsigned int *) (PARAM+0x1cc)))
> >> > +#define EFI_MEMMAP (*((unsigned long *)(PARAM+0x1d0)))
> >> > +#define EFI_SYSTAB (*((unsigned long *)(PARAM+0x1d8)))
> >> >  #define MOUNT_ROOT_RDONLY (*(unsigned short *) (PARAM+0x1F2))
> Be very very very  careful how you talk about this.
> 
> I have seen machines in the wild a 64bit processor and a 32bit EFI.
> So this is not a linux architecture issue, or a cpu architecture
> issue.  This is an EFI architecture issue.
> 
> This is an issue of do you have a 32bit or a 64bit EFI implementation
> on your machine.  Which is very different.
> 
> We should be able to boot a 32bit kernel with a 64bit EFI.
> We should be able to boot a 64bit kernel with a 32bit EFI.
> 
> Maybe our response is to ignore the information from elilo so
> we don't attempt EFI runtime calls but the boot information should
> be unambiguous.
> 
> So we need to be able to look at the data and answer these questions.
> - Is EFI present?
> - Is EFI 32bit?
> - Is EFI 64bit?

Yes, it is necessary to distinguish these situations. I think the
EFI_LOADER_SIG defined above can be used for that. For example the
following signature can be defined:

32-bit EFI	EL32
64-bit EFI	EL64

All other values will be treated as no EFI present.

If struct setup_data proposed by H. Peter Anvin is used for EFI
information passed from bootloader, two "type" can be defined for 32-bit
EFI and 64-bit EFI, such as SETUP_DATA_TYPE_EFI_32 and
SETUP_DATA_TYPE_EFI64. If either type is not presented in "linked list
of struct setup_data", it is considered that EFI is not present.

Best Regards,
Huang Ying

Re: [PATCH 0/3] x86_64 EFI runtime service support

[Huang, Ying]

On Thu, 2007-08-16 at 07:16 +0800, Andrew Morton wrote:
> On Mon, 13 Aug 2007 15:30:19 +0800
> "Huang, Ying" <ying.huang@intel.com> wrote:
> 
> > Following sets of patches add EFI/UEFI (Unified Extensible Firmware
> > Interface) runtime services support to x86_64 architecture.
> 
> OK, we have a major trainwreck when these patches meet Peter's
> get-newsetup.patch.
> 
> I'm halfway into fixing it when I see this.  You have:
> 
>  #define SYS_DESC_TABLE (*(struct sys_desc_table_struct*)(PARAM+0xa0))
> +#define EFI_LOADER_SIG ((unsigned char *)(PARAM+0x1c0))
> +#define EFI_MEMDESC_SIZE (*((unsigned int *) (PARAM+0x1c4)))
> +#define EFI_MEMDESC_VERSION (*((unsigned int *) (PARAM+0x1c8)))
> +#define EFI_MEMMAP_SIZE (*((unsigned int *) (PARAM+0x1cc)))
> +#define EFI_MEMMAP (*((unsigned long *)(PARAM+0x1d0)))
> +#define EFI_SYSTAB (*((unsigned long *)(PARAM+0x1d8)))
>  #define MOUNT_ROOT_RDONLY (*(unsigned short *) (PARAM+0x1F2))
> 
> But Peter's include/asm-i386/bootparam.h has:
> 
> struct efi_info {
>         u32 _pad1;
>         u32 efi_systab;
>         u32 efi_memdesc_size;
>         u32 efi_memdesc_version;
>         u32 efi_memmap;
>         u32 efi_memmap_size;
>         u32 _pad2[2];
> };
> 
> /* The so-called "zeropage" */
> struct boot_params {
>         struct screen_info screen_info;                 /* 0x000 */
>         struct apm_bios_info apm_bios_info;             /* 0x040 */
>         u8  _pad2[12];                                  /* 0x054 */
>         struct ist_info ist_info;                       /* 0x060 */
>         u8  _pad3[16];                                  /* 0x070 */
>         u8  hd0_info[16];       /* obsolete! */         /* 0x080 */
>         u8  hd1_info[16];       /* obsolete! */         /* 0x090 */
>         struct sys_desc_table sys_desc_table;           /* 0x0a0 */
>         u8  _pad4[144];                                 /* 0x0b0 */
>         struct edid_info edid_info;                     /* 0x140 */
>         struct efi_info efi_info;                       /* 0x1c0 */
>         u32 alt_mem_k;                                  /* 0x1e0 */
> 
> So for example, Peter has memdesc_size at 0x1c8 and you have it at
> 0x1c4.
> 
> I'll give up and will drop the EFI patches.  I'd suggest that you work
> with
> Peter on getting these patches integrated.
> 

OK, I will work with Peter to solve the problem.

Best Regards,
Huang Ying