Patchwork [U-Boot,v2,03/17] x86: Remove GDR related magic numbers

login
register
mail settings
Submitter Graeme Russ
Date Jan. 4, 2012, 7:59 p.m.
Message ID <1325707195-3218-3-git-send-email-graeme.russ@gmail.com>
Download mbox | patch
Permalink /patch/134339/
State Awaiting Upstream
Delegated to: Graeme Russ
Headers show

Comments

Graeme Russ - Jan. 4, 2012, 7:59 p.m.
Signed-off-by: Graeme Russ <graeme.russ@gmail.com>
---
Changes for v2:
 - Use an enum
 - Add defined for GDT size (previously added in patch 7)
 - Use X86_ namespace (as per Linux headers)

 arch/x86/cpu/cpu.c               |    8 ++++----
 arch/x86/cpu/start.S             |    3 ++-
 arch/x86/include/asm/processor.h |   23 +++++++++++++++++++----
 3 files changed, 25 insertions(+), 9 deletions(-)

--
1.7.5.2.317.g391b14
Simon Glass - Jan. 7, 2012, 10:05 p.m.
HI Graeme,

On Wed, Jan 4, 2012 at 11:59 AM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Signed-off-by: Graeme Russ <graeme.russ@gmail.com>
> ---
> Changes for v2:
>  - Use an enum
>  - Add defined for GDT size (previously added in patch 7)
>  - Use X86_ namespace (as per Linux headers)
>
>  arch/x86/cpu/cpu.c               |    8 ++++----
>  arch/x86/cpu/start.S             |    3 ++-
>  arch/x86/include/asm/processor.h |   23 +++++++++++++++++++----
>  3 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index 61d0b69..209ff29 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -63,13 +63,13 @@ static void reload_gdt(void)
>         */
>        static const u64 boot_gdt[] __attribute__((aligned(16))) = {
>                /* CS: code, read/execute, 4 GB, base 0 */
> -               [GDT_ENTRY_32BIT_CS] = GDT_ENTRY(0xc09b, 0, 0xfffff),
> +               [X86_GDT_ENTRY_32BIT_CS] = GDT_ENTRY(0xc09b, 0, 0xfffff),
>                /* DS: data, read/write, 4 GB, base 0 */
> -               [GDT_ENTRY_32BIT_DS] = GDT_ENTRY(0xc093, 0, 0xfffff),
> +               [X86_GDT_ENTRY_32BIT_DS] = GDT_ENTRY(0xc093, 0, 0xfffff),
>                /* 16-bit CS: code, read/execute, 64 kB, base 0 */
> -               [GDT_ENTRY_16BIT_CS] = GDT_ENTRY(0x109b, 0, 0x0ffff),
> +               [X86_GDT_ENTRY_16BIT_CS] = GDT_ENTRY(0x109b, 0, 0x0ffff),
>                /* 16-bit DS: data, read/write, 64 kB, base 0 */
> -               [GDT_ENTRY_16BIT_DS] = GDT_ENTRY(0x1093, 0, 0x0ffff),
> +               [X86_GDT_ENTRY_16BIT_DS] = GDT_ENTRY(0x1093, 0, 0x0ffff),
>        };
>        static struct gdt_ptr gdt;
>
> diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
> index f87633b..6027f54 100644
> --- a/arch/x86/cpu/start.S
> +++ b/arch/x86/cpu/start.S
> @@ -29,6 +29,7 @@
>  #include <config.h>
>  #include <version.h>
>  #include <asm/global_data.h>
> +#include <asm/processor.h>
>  #include <asm/processor-flags.h>
>  #include <generated/asm-offsets.h>
>
> @@ -58,7 +59,7 @@ _start:
>        /* This is the 32-bit cold-reset entry point */
>
>        /* Load the segement registes to match the gdt loaded in start16.S */
> -       movl    $0x18, %eax
> +       movl    $(X86_GDT_ENTRY_32BIT_DS * X86_GDT_ENTRY_SIZE), %eax
>        movw    %ax, %fs
>        movw    %ax, %ds
>        movw    %ax, %gs
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 1e5dccd..aa8188e 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -24,9 +24,24 @@
>  #ifndef __ASM_PROCESSOR_H_
>  #define __ASM_PROCESSOR_H_ 1
>
> -#define GDT_ENTRY_32BIT_CS     2
> -#define GDT_ENTRY_32BIT_DS     (GDT_ENTRY_32BIT_CS + 1)
> -#define GDT_ENTRY_16BIT_CS     (GDT_ENTRY_32BIT_DS + 1)
> -#define GDT_ENTRY_16BIT_DS     (GDT_ENTRY_16BIT_CS + 1)
> +#define X86_GDT_ENTRY_SIZE     8
> +
> +#ifndef __ASSEMBLY__
> +
> +enum {
> +       X86_GDT_ENTRY_NULL = 0,
> +       X86_GDT_ENTRY_UNUSED,
> +       X86_GDT_ENTRY_32BIT_CS,
> +       X86_GDT_ENTRY_32BIT_DS,
> +       X86_GDT_ENTRY_16BIT_CS,
> +       X86_GDT_ENTRY_16BIT_DS,
> +       X86_GDT_NUM_ENTRIES
> +};
> +#else
> +/* NOTE: If the above enum is modified, this define must be checked */
> +#define X86_GDT_ENTRY_32BIT_DS 3

Well then I suppose you could put an _ASM prefix on the #define
version, put it above the #ifdef and then add something like

#if X86_GDT_ENTRY_32BIT_DS != X86_GDT_ENTRY_32BIT_DS_ASM
#error "..."
#endif

but that it probably overkill since I suspect you are unlikely to change it.

Regards,
Simon

> +#endif
> +
> +#define X86_GDT_SIZE           (X86_GDT_NUM_ENTRIES * X86_GDT_ENTRY_SIZE)
>
>  #endif
> --
> 1.7.5.2.317.g391b14
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Simon Glass - Jan. 12, 2012, 4:46 a.m.
On Wed, Jan 4, 2012 at 11:59 AM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Signed-off-by: Graeme Russ <graeme.russ@gmail.com>

Acked-by: Simon Glass <sjg@chromium.org>

> ---
> Changes for v2:
>  - Use an enum
>  - Add defined for GDT size (previously added in patch 7)
>  - Use X86_ namespace (as per Linux headers)
>
>  arch/x86/cpu/cpu.c               |    8 ++++----
>  arch/x86/cpu/start.S             |    3 ++-
>  arch/x86/include/asm/processor.h |   23 +++++++++++++++++++----
>  3 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index 61d0b69..209ff29 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -63,13 +63,13 @@ static void reload_gdt(void)
>         */
>        static const u64 boot_gdt[] __attribute__((aligned(16))) = {
>                /* CS: code, read/execute, 4 GB, base 0 */
> -               [GDT_ENTRY_32BIT_CS] = GDT_ENTRY(0xc09b, 0, 0xfffff),
> +               [X86_GDT_ENTRY_32BIT_CS] = GDT_ENTRY(0xc09b, 0, 0xfffff),
>                /* DS: data, read/write, 4 GB, base 0 */
> -               [GDT_ENTRY_32BIT_DS] = GDT_ENTRY(0xc093, 0, 0xfffff),
> +               [X86_GDT_ENTRY_32BIT_DS] = GDT_ENTRY(0xc093, 0, 0xfffff),
>                /* 16-bit CS: code, read/execute, 64 kB, base 0 */
> -               [GDT_ENTRY_16BIT_CS] = GDT_ENTRY(0x109b, 0, 0x0ffff),
> +               [X86_GDT_ENTRY_16BIT_CS] = GDT_ENTRY(0x109b, 0, 0x0ffff),
>                /* 16-bit DS: data, read/write, 64 kB, base 0 */
> -               [GDT_ENTRY_16BIT_DS] = GDT_ENTRY(0x1093, 0, 0x0ffff),
> +               [X86_GDT_ENTRY_16BIT_DS] = GDT_ENTRY(0x1093, 0, 0x0ffff),
>        };
>        static struct gdt_ptr gdt;
>
> diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
> index f87633b..6027f54 100644
> --- a/arch/x86/cpu/start.S
> +++ b/arch/x86/cpu/start.S
> @@ -29,6 +29,7 @@
>  #include <config.h>
>  #include <version.h>
>  #include <asm/global_data.h>
> +#include <asm/processor.h>
>  #include <asm/processor-flags.h>
>  #include <generated/asm-offsets.h>
>
> @@ -58,7 +59,7 @@ _start:
>        /* This is the 32-bit cold-reset entry point */
>
>        /* Load the segement registes to match the gdt loaded in start16.S */
> -       movl    $0x18, %eax
> +       movl    $(X86_GDT_ENTRY_32BIT_DS * X86_GDT_ENTRY_SIZE), %eax
>        movw    %ax, %fs
>        movw    %ax, %ds
>        movw    %ax, %gs
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 1e5dccd..aa8188e 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -24,9 +24,24 @@
>  #ifndef __ASM_PROCESSOR_H_
>  #define __ASM_PROCESSOR_H_ 1
>
> -#define GDT_ENTRY_32BIT_CS     2
> -#define GDT_ENTRY_32BIT_DS     (GDT_ENTRY_32BIT_CS + 1)
> -#define GDT_ENTRY_16BIT_CS     (GDT_ENTRY_32BIT_DS + 1)
> -#define GDT_ENTRY_16BIT_DS     (GDT_ENTRY_16BIT_CS + 1)
> +#define X86_GDT_ENTRY_SIZE     8
> +
> +#ifndef __ASSEMBLY__
> +
> +enum {
> +       X86_GDT_ENTRY_NULL = 0,
> +       X86_GDT_ENTRY_UNUSED,
> +       X86_GDT_ENTRY_32BIT_CS,
> +       X86_GDT_ENTRY_32BIT_DS,
> +       X86_GDT_ENTRY_16BIT_CS,
> +       X86_GDT_ENTRY_16BIT_DS,
> +       X86_GDT_NUM_ENTRIES
> +};
> +#else
> +/* NOTE: If the above enum is modified, this define must be checked */
> +#define X86_GDT_ENTRY_32BIT_DS 3
> +#endif
> +
> +#define X86_GDT_SIZE           (X86_GDT_NUM_ENTRIES * X86_GDT_ENTRY_SIZE)
>
>  #endif
> --
> 1.7.5.2.317.g391b14
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Patch

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index 61d0b69..209ff29 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -63,13 +63,13 @@  static void reload_gdt(void)
 	 */
 	static const u64 boot_gdt[] __attribute__((aligned(16))) = {
 		/* CS: code, read/execute, 4 GB, base 0 */
-		[GDT_ENTRY_32BIT_CS] = GDT_ENTRY(0xc09b, 0, 0xfffff),
+		[X86_GDT_ENTRY_32BIT_CS] = GDT_ENTRY(0xc09b, 0, 0xfffff),
 		/* DS: data, read/write, 4 GB, base 0 */
-		[GDT_ENTRY_32BIT_DS] = GDT_ENTRY(0xc093, 0, 0xfffff),
+		[X86_GDT_ENTRY_32BIT_DS] = GDT_ENTRY(0xc093, 0, 0xfffff),
 		/* 16-bit CS: code, read/execute, 64 kB, base 0 */
-		[GDT_ENTRY_16BIT_CS] = GDT_ENTRY(0x109b, 0, 0x0ffff),
+		[X86_GDT_ENTRY_16BIT_CS] = GDT_ENTRY(0x109b, 0, 0x0ffff),
 		/* 16-bit DS: data, read/write, 64 kB, base 0 */
-		[GDT_ENTRY_16BIT_DS] = GDT_ENTRY(0x1093, 0, 0x0ffff),
+		[X86_GDT_ENTRY_16BIT_DS] = GDT_ENTRY(0x1093, 0, 0x0ffff),
 	};
 	static struct gdt_ptr gdt;

diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
index f87633b..6027f54 100644
--- a/arch/x86/cpu/start.S
+++ b/arch/x86/cpu/start.S
@@ -29,6 +29,7 @@ 
 #include <config.h>
 #include <version.h>
 #include <asm/global_data.h>
+#include <asm/processor.h>
 #include <asm/processor-flags.h>
 #include <generated/asm-offsets.h>

@@ -58,7 +59,7 @@  _start:
 	/* This is the 32-bit cold-reset entry point */

 	/* Load the segement registes to match the gdt loaded in start16.S */
-	movl	$0x18, %eax
+	movl	$(X86_GDT_ENTRY_32BIT_DS * X86_GDT_ENTRY_SIZE), %eax
 	movw	%ax, %fs
 	movw	%ax, %ds
 	movw	%ax, %gs
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 1e5dccd..aa8188e 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -24,9 +24,24 @@ 
 #ifndef __ASM_PROCESSOR_H_
 #define __ASM_PROCESSOR_H_ 1

-#define GDT_ENTRY_32BIT_CS	2
-#define GDT_ENTRY_32BIT_DS	(GDT_ENTRY_32BIT_CS + 1)
-#define GDT_ENTRY_16BIT_CS	(GDT_ENTRY_32BIT_DS + 1)
-#define GDT_ENTRY_16BIT_DS	(GDT_ENTRY_16BIT_CS + 1)
+#define X86_GDT_ENTRY_SIZE	8
+
+#ifndef __ASSEMBLY__
+
+enum {
+	X86_GDT_ENTRY_NULL = 0,
+	X86_GDT_ENTRY_UNUSED,
+	X86_GDT_ENTRY_32BIT_CS,
+	X86_GDT_ENTRY_32BIT_DS,
+	X86_GDT_ENTRY_16BIT_CS,
+	X86_GDT_ENTRY_16BIT_DS,
+	X86_GDT_NUM_ENTRIES
+};
+#else
+/* NOTE: If the above enum is modified, this define must be checked */
+#define X86_GDT_ENTRY_32BIT_DS	3
+#endif
+
+#define X86_GDT_SIZE		(X86_GDT_NUM_ENTRIES * X86_GDT_ENTRY_SIZE)

 #endif