Patchwork [U-Boot,4/4] x86: Remove 16-bit reset code

login
register
mail settings
Submitter Simon Glass
Date Feb. 8, 2013, 4:42 p.m.
Message ID <1360341763-19831-5-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/219216/
State Superseded, archived
Headers show

Comments

Simon Glass - Feb. 8, 2013, 4:42 p.m.
This code is not needed now, since we boot U-Boot from Coreboot on x86.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 Makefile                   |   4 --
 README                     |   4 --
 arch/x86/cpu/Makefile      |   1 -
 arch/x86/cpu/resetvec.S    |  38 ------------
 arch/x86/cpu/start16.S     | 146 ---------------------------------------------
 include/configs/coreboot.h |   1 -
 6 files changed, 194 deletions(-)
 delete mode 100644 arch/x86/cpu/resetvec.S
 delete mode 100644 arch/x86/cpu/start16.S
Graeme Russ - Feb. 10, 2013, 10:20 p.m.
Hi Simon,

On Sat, Feb 9, 2013 at 3:42 AM, Simon Glass <sjg@chromium.org> wrote:
> This code is not needed now, since we boot U-Boot from Coreboot on x86.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  Makefile                   |   4 --
>  README                     |   4 --
>  arch/x86/cpu/Makefile      |   1 -
>  arch/x86/cpu/resetvec.S    |  38 ------------
>  arch/x86/cpu/start16.S     | 146 ---------------------------------------------
>  include/configs/coreboot.h |   1 -
>  6 files changed, 194 deletions(-)
>  delete mode 100644 arch/x86/cpu/resetvec.S
>  delete mode 100644 arch/x86/cpu/start16.S

As mentioned in reply to the previous patch, I would like to see this code stay

> diff --git a/Makefile b/Makefile
> index 51bd918..1924d4b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -230,10 +230,6 @@ endif
>  # U-Boot objects....order is important (i.e. start must be first)
>
>  OBJS  = $(CPUDIR)/start.o
> -ifeq ($(CPU),x86)
> -RESET_OBJS-$(CONFIG_X86_NO_RESET_VECTOR) += $(CPUDIR)/start16.o
> -RESET_OBJS-$(CONFIG_X86_NO_RESET_VECTOR) += $(CPUDIR)/resetvec.o
> -endif

Hmm, odd - the logic here seems to be inverted - the reset vector code
appears to be included if CONFIG_X86_NO_RESET_VECTOR is defined...

Oh, and as discussed before, this can actually be moved out of the
main Makefile and into arch/x86/cpu/Makefile. Hmm, that's odd, it
looks like it already is:

START-y = start.o
RESET_OBJS-$(CONFIG_X86_NO_RESET_VECTOR) += resetvec.o start16.o
COBJS   = interrupts.o cpu.o timer.o

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

But again, the logic seems to be inverted...

> diff --git a/README b/README
> index 103036f..061898c 100644
> --- a/README
> +++ b/README
> @@ -3816,10 +3816,6 @@ 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_X86_NO_RESET_VECTOR
> -               If defined, the x86 reset vector code is excluded. You will need
> -               to do this when U-Boot is running from Coreboot.
> -

Maybe we could just change this to CONFIG_X86_RESET_VECTOR and make
exclusion of the 16-bit reset vector and protected mode switch the
default case

Regards,

Graeme
Simon Glass - Feb. 14, 2013, 1:42 p.m.
Hi Graeme,

On Sun, Feb 10, 2013 at 2:20 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Hi Simon,
>
> On Sat, Feb 9, 2013 at 3:42 AM, Simon Glass <sjg@chromium.org> wrote:
>> This code is not needed now, since we boot U-Boot from Coreboot on x86.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>  Makefile                   |   4 --
>>  README                     |   4 --
>>  arch/x86/cpu/Makefile      |   1 -
>>  arch/x86/cpu/resetvec.S    |  38 ------------
>>  arch/x86/cpu/start16.S     | 146 ---------------------------------------------
>>  include/configs/coreboot.h |   1 -
>>  6 files changed, 194 deletions(-)
>>  delete mode 100644 arch/x86/cpu/resetvec.S
>>  delete mode 100644 arch/x86/cpu/start16.S
>
> As mentioned in reply to the previous patch, I would like to see this code stay

Do you mean I should just drop these last two patches? Sorry, I'm not
quite sure what to do here.

>
>> diff --git a/Makefile b/Makefile
>> index 51bd918..1924d4b 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -230,10 +230,6 @@ endif
>>  # U-Boot objects....order is important (i.e. start must be first)
>>
>>  OBJS  = $(CPUDIR)/start.o
>> -ifeq ($(CPU),x86)
>> -RESET_OBJS-$(CONFIG_X86_NO_RESET_VECTOR) += $(CPUDIR)/start16.o
>> -RESET_OBJS-$(CONFIG_X86_NO_RESET_VECTOR) += $(CPUDIR)/resetvec.o
>> -endif
>
> Hmm, odd - the logic here seems to be inverted - the reset vector code
> appears to be included if CONFIG_X86_NO_RESET_VECTOR is defined...

Actually see where RESET_OBJS is used below - it is inverted there.

>
> Oh, and as discussed before, this can actually be moved out of the
> main Makefile and into arch/x86/cpu/Makefile. Hmm, that's odd, it
> looks like it already is:

Yes it is in the library, and it seems like the link script should
take care of putting things in the right place. However I'm not able
to test a change like this. Still, perhaps this problem doesn't apply
with the other boards removed.

>
> START-y = start.o
> RESET_OBJS-$(CONFIG_X86_NO_RESET_VECTOR) += resetvec.o start16.o
> COBJS   = interrupts.o cpu.o timer.o
>
> SRCS    := $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c)
> OBJS    := $(addprefix $(obj),$(SOBJS) $(COBJS))
> START   := $(addprefix $(obj),$(START-y) $(RESET_OBJS-))
>
> But again, the logic seems to be inverted...

See above.

>
>> diff --git a/README b/README
>> index 103036f..061898c 100644
>> --- a/README
>> +++ b/README
>> @@ -3816,10 +3816,6 @@ 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_X86_NO_RESET_VECTOR
>> -               If defined, the x86 reset vector code is excluded. You will need
>> -               to do this when U-Boot is running from Coreboot.
>> -
>
> Maybe we could just change this to CONFIG_X86_RESET_VECTOR and make
> exclusion of the 16-bit reset vector and protected mode switch the
> default case

OK I can create a patch for that.

Regards,
Simon

>
> Regards,
>
> Graeme

Patch

diff --git a/Makefile b/Makefile
index 51bd918..1924d4b 100644
--- a/Makefile
+++ b/Makefile
@@ -230,10 +230,6 @@  endif
 # U-Boot objects....order is important (i.e. start must be first)
 
 OBJS  = $(CPUDIR)/start.o
-ifeq ($(CPU),x86)
-RESET_OBJS-$(CONFIG_X86_NO_RESET_VECTOR) += $(CPUDIR)/start16.o
-RESET_OBJS-$(CONFIG_X86_NO_RESET_VECTOR) += $(CPUDIR)/resetvec.o
-endif
 ifeq ($(CPU),ppc4xx)
 OBJS += $(CPUDIR)/resetvec.o
 endif
diff --git a/README b/README
index 103036f..061898c 100644
--- a/README
+++ b/README
@@ -3816,10 +3816,6 @@  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_X86_NO_RESET_VECTOR
-		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 57324b6..f27064f 100644
--- a/arch/x86/cpu/Makefile
+++ b/arch/x86/cpu/Makefile
@@ -29,7 +29,6 @@  include $(TOPDIR)/config.mk
 LIB	= $(obj)lib$(CPU).o
 
 START-y	= start.o
-RESET_OBJS-$(CONFIG_X86_NO_RESET_VECTOR) += resetvec.o start16.o
 COBJS	= interrupts.o cpu.o timer.o
 
 SRCS	:= $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c)
diff --git a/arch/x86/cpu/resetvec.S b/arch/x86/cpu/resetvec.S
deleted file mode 100644
index 44aee5f..0000000
--- a/arch/x86/cpu/resetvec.S
+++ /dev/null
@@ -1,38 +0,0 @@ 
-/*
- *  U-boot - x86 Startup Code
- *
- * (C) Copyright 2002
- * Daniel Engström, Omicron Ceti AB, <daniel@omicron.se>
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-/* Reset vector, jumps to start16.S */
-
-.extern start16
-
-.section .resetvec, "ax"
-.code16
-reset_vector:
-	cli
-	cld
-	jmp start16
-
-	.org 0xf
-	nop
diff --git a/arch/x86/cpu/start16.S b/arch/x86/cpu/start16.S
deleted file mode 100644
index 603bf1d..0000000
--- a/arch/x86/cpu/start16.S
+++ /dev/null
@@ -1,146 +0,0 @@ 
-/*
- *  U-boot - x86 Startup Code
- *
- * (C) Copyright 2008-2011
- * Graeme Russ, <graeme.russ@gmail.com>
- *
- * (C) Copyright 2002,2003
- * Daniel Engström, Omicron Ceti AB, <daniel@omicron.se>
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#include <asm/global_data.h>
-#include <asm/processor-flags.h>
-
-#define BOOT_SEG	0xffff0000	/* linear segment of boot code */
-#define a32		.byte 0x67;
-#define o32		.byte 0x66;
-
-.section .start16, "ax"
-.code16
-.globl start16
-start16:
-	/* Set the Cold Boot / Hard Reset flag */
-	movl	$GD_FLG_COLD_BOOT, %ebx
-
-	/*
-	 * First we let the BSP do some early initialization
-	 * this code have to map the flash to its final position
-	 */
-	jmp	board_init16
-.globl board_init16_ret
-board_init16_ret:
-
-	/* Turn of cache (this might require a 486-class CPU) */
-	movl	%cr0, %eax
-	orl	$(X86_CR0_NW | X86_CR0_CD), %eax
-	movl	%eax, %cr0
-	wbinvd
-
-	/* load the temporary Global Descriptor Table */
-o32 cs	lidt	idt_ptr
-o32 cs	lgdt	gdt_ptr
-
-	/* Now, we enter protected mode */
-	movl	%cr0, %eax
-	orl	$X86_CR0_PE, %eax
-	movl	%eax, %cr0
-
-	/* Flush the prefetch queue */
-	jmp	ff
-ff:
-	/* Finally jump to the 32bit initialization code */
-	movw	$code32start, %ax
-	movw	%ax, %bp
-o32 cs	ljmp	*(%bp)
-
-	/* 48-bit far pointer */
-code32start:
-	.long	_start		/* offset */
-	.word	0x10		/* segment */
-
-idt_ptr:
-	.word	0		/* limit */
-	.long	0		/* base */
-
-/*
- * The following Global Descriptor Table is just enough to get us into
- * 'Flat Protected Mode' - It will be discarded as soon as the final
- * GDT is setup in a safe location in RAM
- */
-gdt_ptr:
-	.word	0x20		/* limit (32 bytes = 4 GDT entries) */
-	.long	BOOT_SEG + gdt	/* base */
-
-/* Some CPUs are picky about GDT alignment... */
-.align 16
-gdt:
-	/*
-	 * The GDT table ...
-	 *
-	 *	 Selector	Type
-	 *	 0x00		NULL
-	 *	 0x08		Unused
-	 *	 0x10		32bit code
-	 *	 0x18		32bit data/stack
-	 */
-	/* The NULL Desciptor - Mandatory */
-	.word	0x0000		/* limit_low */
-	.word	0x0000		/* base_low */
-	.byte	0x00		/* base_middle */
-	.byte	0x00		/* access */
-	.byte	0x00		/* flags + limit_high */
-	.byte	0x00		/* base_high */
-
-	/* Unused Desciptor - (matches Linux) */
-	.word	0x0000		/* limit_low */
-	.word	0x0000		/* base_low */
-	.byte	0x00		/* base_middle */
-	.byte	0x00		/* access */
-	.byte	0x00		/* flags + limit_high */
-	.byte	0x00		/* base_high */
-
-	/*
-	 * The Code Segment Descriptor:
-	 * - Base   = 0x00000000
-	 * - Size   = 4GB
-	 * - Access = Present, Ring 0, Exec (Code), Readable
-	 * - Flags  = 4kB Granularity, 32-bit
-	 */
-	.word	0xffff		/* limit_low */
-	.word	0x0000		/* base_low */
-	.byte	0x00		/* base_middle */
-	.byte	0x9b		/* access */
-	.byte	0xcf		/* flags + limit_high */
-	.byte	0x00		/* base_high */
-
-	/*
-	 * The Data Segment Descriptor:
-	 * - Base   = 0x00000000
-	 * - Size   = 4GB
-	 * - Access = Present, Ring 0, Non-Exec (Data), Writable
-	 * - Flags  = 4kB Granularity, 32-bit
-	 */
-	.word	0xffff		/* limit_low */
-	.word	0x0000		/* base_low */
-	.byte	0x00		/* base_middle */
-	.byte	0x93		/* access */
-	.byte	0xcf		/* flags + limit_high */
-	.byte	0x00		/* base_high */
diff --git a/include/configs/coreboot.h b/include/configs/coreboot.h
index d8aabd4..c7f36ff 100644
--- a/include/configs/coreboot.h
+++ b/include/configs/coreboot.h
@@ -37,7 +37,6 @@ 
 #define CONFIG_SYS_COREBOOT
 #define CONFIG_SHOW_BOOT_PROGRESS
 #define CONFIG_LAST_STAGE_INIT
-#define CONFIG_X86_NO_RESET_VECTOR
 #define CONFIG_SYS_VSNPRINTF
 #define CONFIG_INTEL_CORE_ARCH	/* Sandy bridge and ivy bridge chipsets. */
 #define CONFIG_ZBOOT_32