Patchwork [U-Boot,3/9] x86: Allow excluding reset handling code from u-boot.

login
register
mail settings
Submitter Simon Glass
Date Oct. 4, 2012, 12:39 a.m.
Message ID <1349311168-3524-4-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/188976/
State Superseded, archived
Delegated to: Graeme Russ
Headers show

Comments

Simon Glass - Oct. 4, 2012, 12:39 a.m.
From: Gabe Black <gabeblack@chromium.org>

When running from coreboot we don't want this code.

This version works by ifdef-ing out all of the code that would go
into those sections and all the code that refers to it. The sections are
then empty, and the linker will either leave them empty for the loader
to ignore or remove them entirely.

Signed-off-by: Gabe Black <gabeblack@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/x86/cpu/resetvec.S                          |    6 ++++++
 arch/x86/cpu/start16.S                           |    4 ++++
 arch/x86/cpu/u-boot.lds                          |    3 +++
 board/chromebook-x86/coreboot/coreboot_start16.S |    6 ++++++
 board/eNET/eNET_start16.S                        |    4 ++++
 5 files changed, 23 insertions(+), 0 deletions(-)
Graeme Russ - Oct. 4, 2012, 1:01 a.m.
Hi Simon,

At first I thought this patch dealt with the 'board reset' code but
then realised it deals with the 'reset vector' - Can you fix the patch
subject please

On Thu, Oct 4, 2012 at 10:39 AM, Simon Glass <sjg@chromium.org> wrote:
> From: Gabe Black <gabeblack@chromium.org>
>
> When running from coreboot we don't want this code.
>
> This version works by ifdef-ing out all of the code that would go
> into those sections and all the code that refers to it. The sections are
> then empty, and the linker will either leave them empty for the loader
> to ignore or remove them entirely.

Could this be done by #ifdef'ing the section in the linker script?

> Signed-off-by: Gabe Black <gabeblack@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/x86/cpu/resetvec.S                          |    6 ++++++
>  arch/x86/cpu/start16.S                           |    4 ++++
>  arch/x86/cpu/u-boot.lds                          |    3 +++
>  board/chromebook-x86/coreboot/coreboot_start16.S |    6 ++++++
>  board/eNET/eNET_start16.S                        |    4 ++++
>  5 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/cpu/resetvec.S b/arch/x86/cpu/resetvec.S
> index 44aee5f..5b359ff 100644
> --- a/arch/x86/cpu/resetvec.S
> +++ b/arch/x86/cpu/resetvec.S
> @@ -25,6 +25,10 @@
>
>  /* Reset vector, jumps to start16.S */
>
> +#include <config.h>
> +
> +#ifndef CONFIG_NO_RESET_CODE
> +
>  .extern start16
>
>  .section .resetvec, "ax"
> @@ -36,3 +40,5 @@ reset_vector:
>
>         .org 0xf
>         nop
> +
> +#endif

Condition it out in the Makefile instead

> diff --git a/arch/x86/cpu/start16.S b/arch/x86/cpu/start16.S
> index cc393ff..d944840 100644
> --- a/arch/x86/cpu/start16.S
> +++ b/arch/x86/cpu/start16.S
> @@ -28,11 +28,14 @@
>
>  #include <asm/global_data.h>
>  #include <asm/processor-flags.h>
> +#include <config.h>
>
>  #define BOOT_SEG       0xffff0000      /* linear segment of boot code */
>  #define a32            .byte 0x67;
>  #define o32            .byte 0x66;
>
> +#ifndef CONFIG_NO_RESET_CODE
> +
>  .section .start16, "ax"
>  .code16
>  .globl start16
> @@ -141,3 +144,4 @@ gdt:
>         .byte   0x93            /* access */
>         .byte   0xcf            /* flags + limit_high */
>         .byte   0x00            /* base_high */
> +#endif

Ditto

> diff --git a/arch/x86/cpu/u-boot.lds b/arch/x86/cpu/u-boot.lds
> index fe28030..2a90a01 100644
> --- a/arch/x86/cpu/u-boot.lds
> +++ b/arch/x86/cpu/u-boot.lds
> @@ -85,6 +85,8 @@ SECTIONS
>         __bios_start = LOADADDR(.bios);
>         __bios_size = SIZEOF(.bios);
>
> +#ifndef CONFIG_NO_RESET_CODE
> +
>         /*
>          * The following expressions place the 16-bit Real-Mode code and
>          * Reset Vector at the end of the Flash ROM
> @@ -94,4 +96,5 @@ SECTIONS
>
>         . = RESET_VEC_LOC;
>         .resetvec : AT (CONFIG_SYS_TEXT_BASE + (CONFIG_SYS_MONITOR_LEN - RESET_SEG_SIZE + RESET_VEC_LOC)) { KEEP(*(.resetvec)); }
> +#endif
>  }

The commit comment states "and the linker will either leave them empty
for the loader to ignore or remove them entirely" but you are actually
explicitly remove them anyway - If they are not compiled, is this
necessary?

> diff --git a/board/chromebook-x86/coreboot/coreboot_start16.S b/board/chromebook-x86/coreboot/coreboot_start16.S
> index 9ad06df..6fac3d6 100644
> --- a/board/chromebook-x86/coreboot/coreboot_start16.S
> +++ b/board/chromebook-x86/coreboot/coreboot_start16.S
> @@ -28,6 +28,10 @@
>   * that is used by U-boot to its final destination.
>   */
>
> +#include <config.h>
> +
> +#ifndef CONFIG_NO_RESET_CODE
> +
>  .text
>  .section .start16, "ax"
>  .code16
> @@ -35,6 +39,8 @@
>  board_init16:
>         jmp     board_init16_ret
>
> +#endif
> +
>  .section .bios, "ax"
>  .code16
>  .globl realmode_reset

Hmm, I doubt coreboot really need a board level start16.S and (quite
frankly) the whole 'realmode reset' code (i.e. BIOS reset) is crap and
should be globally tossed (no one will ever call it)

> diff --git a/board/eNET/eNET_start16.S b/board/eNET/eNET_start16.S
> index 5e3f44c..43dda2f 100644
> --- a/board/eNET/eNET_start16.S
> +++ b/board/eNET/eNET_start16.S
> @@ -32,6 +32,8 @@
>  #include <asm/arch/sc520.h>
>  #include <generated/asm-offsets.h>
>
> +#ifndef CONFIG_NO_RESET_CODE
> +
>  .text
>  .section .start16, "ax"
>  .code16
> @@ -63,6 +65,8 @@ board_init16:
>
>         jmp     board_init16_ret
>
> +#endif
> +
>  .section .bios, "ax"
>  .code16
>  .globl realmode_reset

All the above should mean there is no reason to touch the eNET code

Regards,

Graeme
Simon Glass - Oct. 9, 2012, 9:15 p.m.
Hi Graeme,

On Wed, Oct 3, 2012 at 6:01 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Hi Simon,
>
> At first I thought this patch dealt with the 'board reset' code but
> then realised it deals with the 'reset vector' - Can you fix the patch
> subject please

Will do.

>
> On Thu, Oct 4, 2012 at 10:39 AM, Simon Glass <sjg@chromium.org> wrote:
>> From: Gabe Black <gabeblack@chromium.org>
>>
>> When running from coreboot we don't want this code.
>>
>> This version works by ifdef-ing out all of the code that would go
>> into those sections and all the code that refers to it. The sections are
>> then empty, and the linker will either leave them empty for the loader
>> to ignore or remove them entirely.
>
> Could this be done by #ifdef'ing the section in the linker script?

Yes, in fact that is already in this patch.

>
>> Signed-off-by: Gabe Black <gabeblack@chromium.org>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>  arch/x86/cpu/resetvec.S                          |    6 ++++++
>>  arch/x86/cpu/start16.S                           |    4 ++++
>>  arch/x86/cpu/u-boot.lds                          |    3 +++
>>  board/chromebook-x86/coreboot/coreboot_start16.S |    6 ++++++
>>  board/eNET/eNET_start16.S                        |    4 ++++
>>  5 files changed, 23 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/cpu/resetvec.S b/arch/x86/cpu/resetvec.S
>> index 44aee5f..5b359ff 100644
>> --- a/arch/x86/cpu/resetvec.S
>> +++ b/arch/x86/cpu/resetvec.S
>> @@ -25,6 +25,10 @@
>>
>>  /* Reset vector, jumps to start16.S */
>>
>> +#include <config.h>
>> +
>> +#ifndef CONFIG_NO_RESET_CODE
>> +
>>  .extern start16
>>
>>  .section .resetvec, "ax"
>> @@ -36,3 +40,5 @@ reset_vector:
>>
>>         .org 0xf
>>         nop
>> +
>> +#endif
>
> Condition it out in the Makefile instead

I suspect the reason it was done here is these lines in the top-level Makefile.

OBJS  = $(CPUDIR)/start.o
ifeq ($(CPU),x86)
OBJS += $(CPUDIR)/start16.o
OBJS += $(CPUDIR)/resetvec.o
endif


If we just take it out of the .lds file then start16.o and resetvec.o
are not included in the image. But they will still be built. We could
add an additional condition here perhaps, like:

OBJS  = $(CPUDIR)/start.o
ifeq ($(CPU),x86)
	ifneq ($(CONFIG_NO_RESET_CODE),y)
		OBJS += $(CPUDIR)/start16.o
		OBJS += $(CPUDIR)/resetvec.o
	endif
endif


Here is the menu as I see it - what would you prefer?
- top level Makefile change
- arch/arm/cpu/Makefile change (pointless if top level Makefile
includes these files anyway)
- building everything but removing unneeded object files in the link script

>
>> diff --git a/arch/x86/cpu/start16.S b/arch/x86/cpu/start16.S
>> index cc393ff..d944840 100644
>> --- a/arch/x86/cpu/start16.S
>> +++ b/arch/x86/cpu/start16.S
>> @@ -28,11 +28,14 @@
>>
>>  #include <asm/global_data.h>
>>  #include <asm/processor-flags.h>
>> +#include <config.h>
>>
>>  #define BOOT_SEG       0xffff0000      /* linear segment of boot code */
>>  #define a32            .byte 0x67;
>>  #define o32            .byte 0x66;
>>
>> +#ifndef CONFIG_NO_RESET_CODE
>> +
>>  .section .start16, "ax"
>>  .code16
>>  .globl start16
>> @@ -141,3 +144,4 @@ gdt:
>>         .byte   0x93            /* access */
>>         .byte   0xcf            /* flags + limit_high */
>>         .byte   0x00            /* base_high */
>> +#endif
>
> Ditto
>
>> diff --git a/arch/x86/cpu/u-boot.lds b/arch/x86/cpu/u-boot.lds
>> index fe28030..2a90a01 100644
>> --- a/arch/x86/cpu/u-boot.lds
>> +++ b/arch/x86/cpu/u-boot.lds
>> @@ -85,6 +85,8 @@ SECTIONS
>>         __bios_start = LOADADDR(.bios);
>>         __bios_size = SIZEOF(.bios);
>>
>> +#ifndef CONFIG_NO_RESET_CODE
>> +
>>         /*
>>          * The following expressions place the 16-bit Real-Mode code and
>>          * Reset Vector at the end of the Flash ROM
>> @@ -94,4 +96,5 @@ SECTIONS
>>
>>         . = RESET_VEC_LOC;
>>         .resetvec : AT (CONFIG_SYS_TEXT_BASE + (CONFIG_SYS_MONITOR_LEN - RESET_SEG_SIZE + RESET_VEC_LOC)) { KEEP(*(.resetvec)); }
>> +#endif
>>  }
>
> The commit comment states "and the linker will either leave them empty
> for the loader to ignore or remove them entirely" but you are actually
> explicitly remove them anyway - If they are not compiled, is this
> necessary?

We need either or.

>
>> diff --git a/board/chromebook-x86/coreboot/coreboot_start16.S b/board/chromebook-x86/coreboot/coreboot_start16.S
>> index 9ad06df..6fac3d6 100644
>> --- a/board/chromebook-x86/coreboot/coreboot_start16.S
>> +++ b/board/chromebook-x86/coreboot/coreboot_start16.S
>> @@ -28,6 +28,10 @@
>>   * that is used by U-boot to its final destination.
>>   */
>>
>> +#include <config.h>
>> +
>> +#ifndef CONFIG_NO_RESET_CODE
>> +
>>  .text
>>  .section .start16, "ax"
>>  .code16
>> @@ -35,6 +39,8 @@
>>  board_init16:
>>         jmp     board_init16_ret
>>
>> +#endif
>> +
>>  .section .bios, "ax"
>>  .code16
>>  .globl realmode_reset
>
> Hmm, I doubt coreboot really need a board level start16.S and (quite
> frankly) the whole 'realmode reset' code (i.e. BIOS reset) is crap and
> should be globally tossed (no one will ever call it)

Will drop this.

>
>> diff --git a/board/eNET/eNET_start16.S b/board/eNET/eNET_start16.S
>> index 5e3f44c..43dda2f 100644
>> --- a/board/eNET/eNET_start16.S
>> +++ b/board/eNET/eNET_start16.S
>> @@ -32,6 +32,8 @@
>>  #include <asm/arch/sc520.h>
>>  #include <generated/asm-offsets.h>
>>
>> +#ifndef CONFIG_NO_RESET_CODE
>> +
>>  .text
>>  .section .start16, "ax"
>>  .code16
>> @@ -63,6 +65,8 @@ board_init16:
>>
>>         jmp     board_init16_ret
>>
>> +#endif
>> +
>>  .section .bios, "ax"
>>  .code16
>>  .globl realmode_reset
>
> All the above should mean there is no reason to touch the eNET code

Will remove this change.

>
> Regards,
>
> Graeme

Regards,
Simon
Graeme Russ - Oct. 9, 2012, 10:58 p.m.
Hi Simon,

On Wed, Oct 10, 2012 at 8:15 AM, Simon Glass <sjg@chromium.org> wrote:

>>> diff --git a/arch/x86/cpu/resetvec.S b/arch/x86/cpu/resetvec.S
>>> index 44aee5f..5b359ff 100644
>>> --- a/arch/x86/cpu/resetvec.S
>>> +++ b/arch/x86/cpu/resetvec.S
>>> @@ -25,6 +25,10 @@
>>>
>>>  /* Reset vector, jumps to start16.S */
>>>
>>> +#include <config.h>
>>> +
>>> +#ifndef CONFIG_NO_RESET_CODE
>>> +
>>>  .extern start16
>>>
>>>  .section .resetvec, "ax"
>>> @@ -36,3 +40,5 @@ reset_vector:
>>>
>>>         .org 0xf
>>>         nop
>>> +
>>> +#endif
>>
>> Condition it out in the Makefile instead
>
> I suspect the reason it was done here is these lines in the top-level Makefile.
>
> OBJS  = $(CPUDIR)/start.o
> ifeq ($(CPU),x86)
>         OBJS += $(CPUDIR)/start16.o
>         OBJS += $(CPUDIR)/resetvec.o
> endif

I have often wondered about these lines in the top-level Makefile
considering they are also in arch/x86/cpu/Makefile. I keep meaning to test
if they are actually needed in the top-level Makefile but keep forgetting

(I see why now - see below)

> If we just take it out of the .lds file then start16.o and resetvec.o
> are not included in the image. But they will still be built. We could
> add an additional condition here perhaps, like:

I don't see a huge problem with that. Yes, it's a waste of CPU cycles
during the build but really, who cares.

> OBJS  = $(CPUDIR)/start.o
> ifeq ($(CPU),x86)
>         ifneq ($(CONFIG_NO_RESET_CODE),y)
>                 OBJS += $(CPUDIR)/start16.o
>                 OBJS += $(CPUDIR)/resetvec.o
>         endif
> endif

Looks good for the time being (again, see beloW).

> Here is the menu as I see it - what would you prefer?
> - top level Makefile change
> - arch/arm/cpu/Makefile change (pointless if top level Makefile
> includes these files anyway)
> - building everything but removing unneeded object files in the link script

Can we not invert the logic of CONFIG_X86_NO_RESET_VECTOR using some
Makefile magic and then do this in arch/x86/cpu/Makefile:

START-$(INCLUDE_X86_RESET_VECTOR) += resetvec.o
START-y = start.o
START-$(INCLUDE_X86_RESET_VECTOR) += start16.o

Actuall, to be honest, it should be:

SOBJS-y += start.o

SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += resetvec.o
SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += start16.o

SRCS    := $(SOBJS16-y:.o=.S) $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
OBJS    := $(addprefix $(obj),$(SOBJS) $(COBJS))
OBJS16  := $(addprefix $(obj),$(SOBJS16))

all:    $(obj).depend $(OBJS16) $(LIB)

start.S is not at all related to the reset vector / protected mode switch,
and so can safely be moved into the main (32-bit) lib. ENTRY(_start) in
the linker script and:

.section .text
.code32
.globl _start
.type _start, @function  .globl _start
.type _start, @function

in start.S will always guarantee that the code in start.S appears first
in u-boot.bin.

Ah Ha! now I get it - Now I see why the top-level Makefile includes:

OBJS  = $(CPUDIR)/start.o
ifeq ($(CPU),x86)
        OBJS += $(CPUDIR)/start16.o
        OBJS += $(CPUDIR)/resetvec.o
endif

These files are not in $(CPUDIR)/lib$(CPU).o so they must be pulled in
individually!

OK, by moving start.o into the lib we can drop the first line...

Now, if we create a 16-bit lib in arch/x86/cpu/Makefile:

LIB     = $(obj)lib$(CPU).o
LIB16   = $(obj)lib16$(CPU).o

SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += resetvec.o
SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += start16.o

SOBJS-y += start.o
COBJS-y += cpu.o
COBJS-y += interrupts.o

SRCS    := $(SOBJS16-y:.o=.S) $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
OBJS16  := $(addprefix $(obj),$(SOBJS16))
OBJS    := $(addprefix $(obj),$(SOBJS) $(COBJS))

all:    $(obj).depend $(LIB) $(LIB16)

$(LIB): $(OBJS)
        $(call cmd_link_o_target, $(OBJS))

$(LIB16): $(OBJS16)
        $(call cmd_link_o_target, $(OBJS16))

And then in the top-level Makefile:

LIBS-$(INCLUDE_X86_RESET_VECTOR) += $(CPUDIR)/lib16$(CPU).o

Much cleaner :)

(Hmmm, looking at arch/x86/lib/Makefile is appears that it is safe to mix
16- and 32-bit code in the same lib - maybe that is a better solution...)

But don't worry too much about all that in these patches - Make the changes
as you have already suggested and I will tweak the rest later

Regards,

Graeme
Simon Glass - Oct. 9, 2012, 11:11 p.m.
Hi Graeme,

On Tue, Oct 9, 2012 at 3:58 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Hi Simon,
>
> On Wed, Oct 10, 2012 at 8:15 AM, Simon Glass <sjg@chromium.org> wrote:
>
>>>> diff --git a/arch/x86/cpu/resetvec.S b/arch/x86/cpu/resetvec.S
>>>> index 44aee5f..5b359ff 100644
>>>> --- a/arch/x86/cpu/resetvec.S
>>>> +++ b/arch/x86/cpu/resetvec.S
>>>> @@ -25,6 +25,10 @@
>>>>
>>>>  /* Reset vector, jumps to start16.S */
>>>>
>>>> +#include <config.h>
>>>> +
>>>> +#ifndef CONFIG_NO_RESET_CODE
>>>> +
>>>>  .extern start16
>>>>
>>>>  .section .resetvec, "ax"
>>>> @@ -36,3 +40,5 @@ reset_vector:
>>>>
>>>>         .org 0xf
>>>>         nop
>>>> +
>>>> +#endif
>>>
>>> Condition it out in the Makefile instead
>>
>> I suspect the reason it was done here is these lines in the top-level Makefile.
>>
>> OBJS  = $(CPUDIR)/start.o
>> ifeq ($(CPU),x86)
>>         OBJS += $(CPUDIR)/start16.o
>>         OBJS += $(CPUDIR)/resetvec.o
>> endif
>
> I have often wondered about these lines in the top-level Makefile
> considering they are also in arch/x86/cpu/Makefile. I keep meaning to test
> if they are actually needed in the top-level Makefile but keep forgetting
>
> (I see why now - see below)
>
>> If we just take it out of the .lds file then start16.o and resetvec.o
>> are not included in the image. But they will still be built. We could
>> add an additional condition here perhaps, like:
>
> I don't see a huge problem with that. Yes, it's a waste of CPU cycles
> during the build but really, who cares.
>
>> OBJS  = $(CPUDIR)/start.o
>> ifeq ($(CPU),x86)
>>         ifneq ($(CONFIG_NO_RESET_CODE),y)
>>                 OBJS += $(CPUDIR)/start16.o
>>                 OBJS += $(CPUDIR)/resetvec.o
>>         endif
>> endif
>
> Looks good for the time being (again, see beloW).
>
>> Here is the menu as I see it - what would you prefer?
>> - top level Makefile change
>> - arch/arm/cpu/Makefile change (pointless if top level Makefile
>> includes these files anyway)
>> - building everything but removing unneeded object files in the link script
>
> Can we not invert the logic of CONFIG_X86_NO_RESET_VECTOR using some
> Makefile magic and then do this in arch/x86/cpu/Makefile:
>
> START-$(INCLUDE_X86_RESET_VECTOR) += resetvec.o
> START-y = start.o
> START-$(INCLUDE_X86_RESET_VECTOR) += start16.o
>
> Actuall, to be honest, it should be:
>
> SOBJS-y += start.o
>
> SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += resetvec.o
> SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += start16.o
>
> SRCS    := $(SOBJS16-y:.o=.S) $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
> OBJS    := $(addprefix $(obj),$(SOBJS) $(COBJS))
> OBJS16  := $(addprefix $(obj),$(SOBJS16))
>
> all:    $(obj).depend $(OBJS16) $(LIB)
>
> start.S is not at all related to the reset vector / protected mode switch,
> and so can safely be moved into the main (32-bit) lib. ENTRY(_start) in
> the linker script and:
>
> .section .text
> .code32
> .globl _start
> .type _start, @function  .globl _start
> .type _start, @function
>
> in start.S will always guarantee that the code in start.S appears first
> in u-boot.bin.
>
> Ah Ha! now I get it - Now I see why the top-level Makefile includes:
>
> OBJS  = $(CPUDIR)/start.o
> ifeq ($(CPU),x86)
>         OBJS += $(CPUDIR)/start16.o
>         OBJS += $(CPUDIR)/resetvec.o
> endif
>
> These files are not in $(CPUDIR)/lib$(CPU).o so they must be pulled in
> individually!
>
> OK, by moving start.o into the lib we can drop the first line...
>
> Now, if we create a 16-bit lib in arch/x86/cpu/Makefile:
>
> LIB     = $(obj)lib$(CPU).o
> LIB16   = $(obj)lib16$(CPU).o
>
> SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += resetvec.o
> SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += start16.o
>
> SOBJS-y += start.o
> COBJS-y += cpu.o
> COBJS-y += interrupts.o
>
> SRCS    := $(SOBJS16-y:.o=.S) $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
> OBJS16  := $(addprefix $(obj),$(SOBJS16))
> OBJS    := $(addprefix $(obj),$(SOBJS) $(COBJS))
>
> all:    $(obj).depend $(LIB) $(LIB16)
>
> $(LIB): $(OBJS)
>         $(call cmd_link_o_target, $(OBJS))
>
> $(LIB16): $(OBJS16)
>         $(call cmd_link_o_target, $(OBJS16))
>
> And then in the top-level Makefile:
>
> LIBS-$(INCLUDE_X86_RESET_VECTOR) += $(CPUDIR)/lib16$(CPU).o
>
> Much cleaner :)
>
> (Hmmm, looking at arch/x86/lib/Makefile is appears that it is safe to mix
> 16- and 32-bit code in the same lib - maybe that is a better solution...)
>
> But don't worry too much about all that in these patches - Make the changes
> as you have already suggested and I will tweak the rest later

Looks good - yes I will prepare a new series to send likely on Thursday.

Not that it matters for now, but who will define INCLUDE_X86_RESET_VECTOR?

Regards,
Simon

>
> Regards,
>
> Graeme
Graeme Russ - Oct. 9, 2012, 11:18 p.m.
Hi Simon,

On Wed, Oct 10, 2012 at 10:11 AM, Simon Glass <sjg@chromium.org> wrote:

>
> Looks good - yes I will prepare a new series to send likely on Thursday.
>
> Not that it matters for now, but who will define INCLUDE_X86_RESET_VECTOR?

It's calculated in the Makefile as an inversion of  CONFIG_X86_NO_RESET_VECTOR

Regards,

Graeme

Patch

diff --git a/arch/x86/cpu/resetvec.S b/arch/x86/cpu/resetvec.S
index 44aee5f..5b359ff 100644
--- a/arch/x86/cpu/resetvec.S
+++ b/arch/x86/cpu/resetvec.S
@@ -25,6 +25,10 @@ 
 
 /* Reset vector, jumps to start16.S */
 
+#include <config.h>
+
+#ifndef CONFIG_NO_RESET_CODE
+
 .extern start16
 
 .section .resetvec, "ax"
@@ -36,3 +40,5 @@  reset_vector:
 
 	.org 0xf
 	nop
+
+#endif
diff --git a/arch/x86/cpu/start16.S b/arch/x86/cpu/start16.S
index cc393ff..d944840 100644
--- a/arch/x86/cpu/start16.S
+++ b/arch/x86/cpu/start16.S
@@ -28,11 +28,14 @@ 
 
 #include <asm/global_data.h>
 #include <asm/processor-flags.h>
+#include <config.h>
 
 #define BOOT_SEG	0xffff0000	/* linear segment of boot code */
 #define a32		.byte 0x67;
 #define o32		.byte 0x66;
 
+#ifndef CONFIG_NO_RESET_CODE
+
 .section .start16, "ax"
 .code16
 .globl start16
@@ -141,3 +144,4 @@  gdt:
 	.byte	0x93		/* access */
 	.byte	0xcf		/* flags + limit_high */
 	.byte	0x00		/* base_high */
+#endif
diff --git a/arch/x86/cpu/u-boot.lds b/arch/x86/cpu/u-boot.lds
index fe28030..2a90a01 100644
--- a/arch/x86/cpu/u-boot.lds
+++ b/arch/x86/cpu/u-boot.lds
@@ -85,6 +85,8 @@  SECTIONS
 	__bios_start = LOADADDR(.bios);
 	__bios_size = SIZEOF(.bios);
 
+#ifndef CONFIG_NO_RESET_CODE
+
 	/*
 	 * The following expressions place the 16-bit Real-Mode code and
 	 * Reset Vector at the end of the Flash ROM
@@ -94,4 +96,5 @@  SECTIONS
 
 	. = RESET_VEC_LOC;
 	.resetvec : AT (CONFIG_SYS_TEXT_BASE + (CONFIG_SYS_MONITOR_LEN - RESET_SEG_SIZE + RESET_VEC_LOC)) { KEEP(*(.resetvec)); }
+#endif
 }
diff --git a/board/chromebook-x86/coreboot/coreboot_start16.S b/board/chromebook-x86/coreboot/coreboot_start16.S
index 9ad06df..6fac3d6 100644
--- a/board/chromebook-x86/coreboot/coreboot_start16.S
+++ b/board/chromebook-x86/coreboot/coreboot_start16.S
@@ -28,6 +28,10 @@ 
  * that is used by U-boot to its final destination.
  */
 
+#include <config.h>
+
+#ifndef CONFIG_NO_RESET_CODE
+
 .text
 .section .start16, "ax"
 .code16
@@ -35,6 +39,8 @@ 
 board_init16:
 	jmp	board_init16_ret
 
+#endif
+
 .section .bios, "ax"
 .code16
 .globl realmode_reset
diff --git a/board/eNET/eNET_start16.S b/board/eNET/eNET_start16.S
index 5e3f44c..43dda2f 100644
--- a/board/eNET/eNET_start16.S
+++ b/board/eNET/eNET_start16.S
@@ -32,6 +32,8 @@ 
 #include <asm/arch/sc520.h>
 #include <generated/asm-offsets.h>
 
+#ifndef CONFIG_NO_RESET_CODE
+
 .text
 .section .start16, "ax"
 .code16
@@ -63,6 +65,8 @@  board_init16:
 
 	jmp	board_init16_ret
 
+#endif
+
 .section .bios, "ax"
 .code16
 .globl realmode_reset