[U-Boot] [PATCH v5 2/9] x86: Allow excluding reset vector code from u-boot

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 --- Changes in v5: - Use CONFIG_NO_RESET_CODE again (as v3), check it in arch config.h
Changes in v4: - Use CONFIG_SYS_X86_RESET_VECTOR instead CONFIG_NO_RESET_CODE - Add note about CONFIG_SYS_X86_RESET_VECTOR to README
Changes in v3: - Fix incorrect repeated line in Makefile
Changes in v2: - Put CONFIG_NO_RESET_CODE into Makefile instead of source files
Makefile | 8 +++----- README | 4 ++++ arch/x86/cpu/Makefile | 5 +++-- arch/x86/cpu/u-boot.lds | 3 +++ arch/x86/include/asm/config.h | 5 +++++ 5 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile index 1a17be9..dcd3a0c 100644 --- a/Makefile +++ b/Makefile @@ -230,10 +230,8 @@ endif # U-Boot objects....order is important (i.e. start must be first)
OBJS = $(CPUDIR)/start.o -ifeq ($(CPU),x86) -OBJS += $(CPUDIR)/start16.o -OBJS += $(CPUDIR)/resetvec.o -endif +OBJS-$(CONFIG_SYS_X86_RESET_VECTOR) += $(CPUDIR)/start16.o +OBJS-$(CONFIG_SYS_X86_RESET_VECTOR) += $(CPUDIR)/resetvec.o ifeq ($(CPU),ppc4xx) OBJS += $(CPUDIR)/resetvec.o endif @@ -241,7 +239,7 @@ ifeq ($(CPU),mpc85xx) OBJS += $(CPUDIR)/resetvec.o endif
-OBJS := $(addprefix $(obj),$(OBJS)) +OBJS := $(addprefix $(obj),$(OBJS) $(OBJS-y))
HAVE_VENDOR_COMMON_LIB = $(if $(wildcard board/$(VENDOR)/common/Makefile),y,n)
diff --git a/README b/README index 2dc0984..dd7fb9d 100644 --- a/README +++ b/README @@ -3621,6 +3621,10 @@ Low Level (hardware related) configuration options: be used if available. These functions may be faster under some conditions but may increase the binary size.
+- CONFIG_NO_RESET_CODE + If defined, the x86 reset vector code is excluded. You will need + to do this when U-Boot is running from Coreboot. + Freescale QE/FMAN Firmware Support: -----------------------------------
diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile index 7f1fc18..b0c350c 100644 --- a/arch/x86/cpu/Makefile +++ b/arch/x86/cpu/Makefile @@ -28,12 +28,13 @@ include $(TOPDIR)/config.mk
LIB = $(obj)lib$(CPU).o
-START = start.o start16.o resetvec.o +START-y = start.o +START-$(CONFIG_SYS_X86_RESET_VECTOR) += resetvec.o start16.o COBJS = interrupts.o cpu.o
SRCS := $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS)) -START := $(addprefix $(obj),$(START)) +START := $(addprefix $(obj),$(START-y))
all: $(obj).depend $(START) $(LIB)
diff --git a/arch/x86/cpu/u-boot.lds b/arch/x86/cpu/u-boot.lds index a1ecefa..de6dfdc 100644 --- a/arch/x86/cpu/u-boot.lds +++ b/arch/x86/cpu/u-boot.lds @@ -86,6 +86,8 @@ SECTIONS __bios_start = LOADADDR(.bios); __bios_size = SIZEOF(.bios);
+#ifdef CONFIG_SYS_X86_RESET_VECTOR + /* * The following expressions place the 16-bit Real-Mode code and * Reset Vector at the end of the Flash ROM @@ -95,4 +97,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/arch/x86/include/asm/config.h b/arch/x86/include/asm/config.h index 049c44e..e3cae0b 100644 --- a/arch/x86/include/asm/config.h +++ b/arch/x86/include/asm/config.h @@ -21,4 +21,9 @@ #ifndef _ASM_CONFIG_H_ #define _ASM_CONFIG_H_
+/* Include the reset fector code unless the board configure doesn't want it */ +#ifndef CONFIG_NO_RESET_CODE +#define CONFIG_SYS_X86_RESET_VECTOR +#endif + #endif

Now that coreboot doesn't need the start16 code, remove it. We need to remove the CONFIG_SYS_X86_RESET_VECTOR option from coreboot.h also.
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v5: - Revert back to v2, using CONFIG_NO_RESET_CODE
Changes in v4: - Remove CONFIG_SYS_X86_RESET_VECTOR from coreboot.h
Changes in v3: None Changes in v2: - Add new patch to remove coreboot start16 code.
board/chromebook-x86/coreboot/coreboot_start16.S | 13 ------------- include/configs/coreboot.h | 2 +- 2 files changed, 1 insertions(+), 14 deletions(-)
diff --git a/board/chromebook-x86/coreboot/coreboot_start16.S b/board/chromebook-x86/coreboot/coreboot_start16.S index 9ad06df..6b3d92d 100644 --- a/board/chromebook-x86/coreboot/coreboot_start16.S +++ b/board/chromebook-x86/coreboot/coreboot_start16.S @@ -22,19 +22,6 @@ * MA 02111-1307 USA */
-/* - * 16bit initialization code. - * This code have to map the area of the boot flash - * that is used by U-boot to its final destination. - */ - -.text -.section .start16, "ax" -.code16 -.globl board_init16 -board_init16: - jmp board_init16_ret - .section .bios, "ax" .code16 .globl realmode_reset diff --git a/include/configs/coreboot.h b/include/configs/coreboot.h index cc95e2b..c1a6832 100644 --- a/include/configs/coreboot.h +++ b/include/configs/coreboot.h @@ -37,7 +37,7 @@ #define CONFIG_SYS_COREBOOT #undef CONFIG_SHOW_BOOT_PROGRESS #define CONFIG_LAST_STAGE_INIT - +#define CONFIG_NO_RESET_CODE
/*----------------------------------------------------------------------- * Watchdog Configuration

Dear Simon Glass,
In message 1353961997-32762-3-git-send-email-sjg@chromium.org you wrote:
+- CONFIG_NO_RESET_CODE
If defined, the x86 reset vector code is excluded. You will need
to do this when U-Boot is running from Coreboot.
Sorry fr never ending nitpicking - but either this is some x86 specific stuff, then it would probably make sense to have such indication in the name of the define, or it is not, then the comment should be fixed.
+START-$(CONFIG_SYS_X86_RESET_VECTOR) += resetvec.o start16.o
CONFIG_SYS_X86_RESET_VECTOR - is this just another new, undocumented variable?
Or a mismatch between documentation and code?
Best regards,
Wolfgang Denk

Hi Wolfgang,
On Mon, Nov 26, 2012 at 3:03 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message 1353961997-32762-3-git-send-email-sjg@chromium.org you wrote:
+- CONFIG_NO_RESET_CODE
If defined, the x86 reset vector code is excluded. You will need
to do this when U-Boot is running from Coreboot.
Sorry fr never ending nitpicking - but either this is some x86 specific stuff, then it would probably make sense to have such indication in the name of the define, or it is not, then the comment should be fixed.
+START-$(CONFIG_SYS_X86_RESET_VECTOR) += resetvec.o start16.o
CONFIG_SYS_X86_RESET_VECTOR - is this just another new, undocumented variable?
Yes it is new and undocumented, because it is internal to the x86 implementation and I don't want people to set it.
Or a mismatch between documentation and code?
Well there are now two options:
1. The user-facing CONFIG_NO_RESET_CODE option which, if not defined, asserts CONFIG_SYS_X86_RESET_VECTOR 2. CONFIG_SYS_X86_RESET_VECTOR which is used in the Makefiles
I would rather have a single positive option (CONFIG_SYS_X86_RESET_VECTOR, as series v4). Failing that I would rather have ifneq in the Makefile (and just use CONFIG_NO_RESET_CODE). I am not sure how to have a negative option without an ifneq in the Makefile. I looked pretty hard but could not find an example in U-Boot.
Re CONFIG_NO_RESET_CODE, it probably should be CONFIG_SYS_X86_NO_RESET_CODE, but it would be even better if we could just have one option. I worry that people will be confused by one that they set, which affects another that is set if they don't see the first...In a month I will probably find it confusing.
This is possible a minor point since I think Graeme said that the only other x86 board will move to Coreboot soon.
Regards, Simon
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de "What if" is a trademark of Hewlett Packard, so stop using it in your sentences without permission, or risk being sued.

Hi Simon,
On Tue, Nov 27, 2012 at 10:14 AM, Simon Glass sjg@chromium.org wrote:
Hi Wolfgang,
On Mon, Nov 26, 2012 at 3:03 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message 1353961997-32762-3-git-send-email-sjg@chromium.org you
wrote:
+- CONFIG_NO_RESET_CODE
If defined, the x86 reset vector code is excluded. You
will need
to do this when U-Boot is running from Coreboot.
Sorry fr never ending nitpicking - but either this is some x86 specific stuff, then it would probably make sense to have such indication in the name of the define, or it is not, then the comment should be fixed.
+START-$(CONFIG_SYS_X86_RESET_VECTOR) += resetvec.o start16.o
CONFIG_SYS_X86_RESET_VECTOR - is this just another new, undocumented variable?
Yes it is new and undocumented, because it is internal to the x86 implementation and I don't want people to set it.
Or a mismatch between documentation and code?
Well there are now two options:
- The user-facing CONFIG_NO_RESET_CODE option which, if not defined,
asserts CONFIG_SYS_X86_RESET_VECTOR 2. CONFIG_SYS_X86_RESET_VECTOR which is used in the Makefiles
I would rather have a single positive option (CONFIG_SYS_X86_RESET_VECTOR, as series v4). Failing that I would rather have ifneq in the Makefile (and just use CONFIG_NO_RESET_CODE). I am not sure how to have a negative option without an ifneq in the Makefile. I looked pretty hard but could not find an example in U-Boot.
That is the crux of the problem - we want to have a config define which _excludes_ the compilation of some code (in this case the reset vector). This should be expanded to optionally exclude the 16-bit 'Real Mode' code (with the ultimate goal of dropping that code altogether).
My vote would be for a CONFIG_X86_NO_RESET_VECTOR (and CONFIG_X86_NO_REAL_MODE) and just use ifneq in the Makefile
Re CONFIG_NO_RESET_CODE, it probably should be CONFIG_SYS_X86_NO_RESET_CODE, but it would be even better if we could just have one option. I worry that people will be confused by one that they set, which affects another that is set if they don't see the first...In a month I will probably find it confusing.
This is possible a minor point since I think Graeme said that the only other x86 board will move to Coreboot soon.
Actually, I said that the only other x86 board will be dropped from U-Boot as the processor is no longer available.
Regards,
Graeme

Dear Graeme Russ,
In message CALButCJ43_3d_PgNwnC3w9q4Fxy0rgZAXd1KTzb+SDytqE3Y1Q@mail.gmail.com you wrote:
My vote would be for a CONFIG_X86_NO_RESET_VECTOR (and CONFIG_X86_NO_REAL_MODE) and just use ifneq in the Makefile
ACK for the names, but ther eis no need for ifneq or such.
Best regards,
Wolfgang Denk

Hi,
On Tue, Nov 27, 2012 at 5:42 AM, Wolfgang Denk wd@denx.de wrote:
Dear Graeme Russ,
In message CALButCJ43_3d_PgNwnC3w9q4Fxy0rgZAXd1KTzb+SDytqE3Y1Q@mail.gmail.com you wrote:
My vote would be for a CONFIG_X86_NO_RESET_VECTOR (and CONFIG_X86_NO_REAL_MODE) and just use ifneq in the Makefile
ACK for the names, but ther eis no need for ifneq or such.
Thank you both, I will make it so.
Regards, Simon
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de A good marriage would be between a blind wife and deaf husband. -- Michel de Montaigne

Dear Simon Glass,
In message CAPnjgZ2EjgHHnNj-0dyHgMMHhomYLuVJE=KF7pFCpwRSbDgY8g@mail.gmail.com you wrote:
CONFIG_SYS_X86_RESET_VECTOR - is this just another new, undocumented variable?
Yes it is new and undocumented, because it is internal to the x86 implementation and I don't want people to set it.
Then document it as internal and not to be touched, but documented it must be.
Well there are now two options:
- The user-facing CONFIG_NO_RESET_CODE option which, if not defined,
asserts CONFIG_SYS_X86_RESET_VECTOR 2. CONFIG_SYS_X86_RESET_VECTOR which is used in the Makefiles
I would rather have a single positive option (CONFIG_SYS_X86_RESET_VECTOR, as series v4). Failing that I would rather have ifneq in the Makefile (and just use CONFIG_NO_RESET_CODE). I am not sure how to have a negative option without an ifneq in the Makefile. I looked pretty hard but could not find an example in U-Boot.
First, you can #define CONFIG_SYS_X86_RESET_VECTOR in some global header file, and in the boards that don't want it add an #undef in the board config file.
Second, you can have some
RESET_OBJS-$(CONFIG_SYS_X86_RESET_VECTOR) = <your_list>
in your Makefile, and then use COBJS := $(sort $(COBJS-y) $(RESET_OBJS-))
which will include <your_list> only if CONFIG_SYS_X86_RESET_VECTOR is _not_ set or empty.
etc.
This is possible a minor point since I think Graeme said that the only other x86 board will move to Coreboot soon.
...for the time being, maybe. But we should not bar other options without serious need.
Best regards,
Wolfgang Denk
participants (3)
-
Graeme Russ
-
Simon Glass
-
Wolfgang Denk