diff mbox series

[4/8] x86: slimbootloader: Support 64-bit operation

Message ID 20200422004507.2025-5-aiden.park@intel.com
State New
Headers show
Series Support 64-bit U-Boot for Slim Bootloader | expand

Commit Message

Park, Aiden April 22, 2020, 12:45 a.m. UTC 
From: Aiden Park <aiden.park at intel.com>

This supports 64-bit U-Boot as a Slim Bootloader payload.

Signed-off-by: Aiden Park <aiden.park at intel.com>
---
 arch/x86/cpu/slimbootloader/Makefile         |  9 +++++++--
 arch/x86/cpu/slimbootloader/entry64.S        | 14 ++++++++++++++
 arch/x86/cpu/slimbootloader/slimbootloader.c | 17 +++++++++++++++--
 3 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 arch/x86/cpu/slimbootloader/entry64.S

Comments

Simon Glass April 26, 2020, 8:15 p.m. UTC | #1 
Hi Aiden,

On Tue, 21 Apr 2020 at 18:45, <aiden.park at intel.com> wrote:
>
> From: Aiden Park <aiden.park at intel.com>
>
> This supports 64-bit U-Boot as a Slim Bootloader payload.
>
> Signed-off-by: Aiden Park <aiden.park at intel.com>
> ---
>  arch/x86/cpu/slimbootloader/Makefile         |  9 +++++++--
>  arch/x86/cpu/slimbootloader/entry64.S        | 14 ++++++++++++++
>  arch/x86/cpu/slimbootloader/slimbootloader.c | 17 +++++++++++++++--
>  3 files changed, 36 insertions(+), 4 deletions(-)
>  create mode 100644 arch/x86/cpu/slimbootloader/entry64.S
>
> diff --git a/arch/x86/cpu/slimbootloader/Makefile b/arch/x86/cpu/slimbootloader/Makefile
> index aac9fa3db8..79fa699501 100644
> --- a/arch/x86/cpu/slimbootloader/Makefile
> +++ b/arch/x86/cpu/slimbootloader/Makefile
> @@ -1,5 +1,10 @@
>  # SPDX-License-Identifier: GPL-2.0+
>  #
> -# Copyright (C) 2019 Intel Corporation <www.intel.com>
> +# Copyright (C) 2019-2020 Intel Corporation <www.intel.com>
>
> -obj-y += car.o slimbootloader.o sdram.o serial.o
> +ifeq ($(CONFIG_X86_64),y)
> +obj-y += entry64.o
> +else
> +obj-y += car.o
> +endif
> +obj-y += slimbootloader.o sdram.o serial.o
> diff --git a/arch/x86/cpu/slimbootloader/entry64.S b/arch/x86/cpu/slimbootloader/entry64.S
> new file mode 100644
> index 0000000000..5e101e18a9
> --- /dev/null
> +++ b/arch/x86/cpu/slimbootloader/entry64.S
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2020 Intel Corporation <www.intel.com>
> + */
> +
> +#include <generated/asm-offsets.h>
> +
> +.section .text
> +
> +.globl init_64bit_entry
> +init_64bit_entry:
> +       /* Save hob pointer parameter */
> +       mov     %rcx, %r10
> +       jmp     init_64bit_entry_ret
> diff --git a/arch/x86/cpu/slimbootloader/slimbootloader.c b/arch/x86/cpu/slimbootloader/slimbootloader.c
> index 21dcfb2142..7857e4cd8b 100644
> --- a/arch/x86/cpu/slimbootloader/slimbootloader.c
> +++ b/arch/x86/cpu/slimbootloader/slimbootloader.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - * Copyright (C) 2019 Intel Corporation <www.intel.com>
> + * Copyright (C) 2019-2020 Intel Corporation <www.intel.com>
>   */
>
>  #include <common.h>
> @@ -43,11 +43,23 @@ static void tsc_init(void)
>
>  int arch_cpu_init(void)
>  {
> +       int ret = 0;
> +
>         tsc_init();
>
> -       return x86_cpu_init_f();
> +#if !CONFIG_IS_ENABLED(X86_64)

Can you use if() instead of #if ?

> +       ret = x86_cpu_init_f();
> +#endif
> +       return ret;
>  }
>
> +#if CONFIG_IS_ENABLED(X86_64)

It should be safe to define both of these functions so I don't think
you need the #ifdef

> +int set_hob_list(void *hob_list)
> +{
> +       gd->arch.hob_list = hob_list;
> +       return 0;
> +}
> +#else
>  int checkcpu(void)
>  {
>         return 0;
> @@ -57,3 +69,4 @@ int print_cpuinfo(void)
>  {
>         return default_print_cpuinfo();
>  }
> +#endif
> --
> 2.20.1
>

Regards,
Simon
Park, Aiden April 29, 2020, 6:01 a.m. UTC | #2 
Hi Simon,

> -----Original Message-----
> From: Simon Glass <sjg at chromium.org>
> Sent: Sunday, April 26, 2020 1:16 PM
> To: Park, Aiden <aiden.park at intel.com>
> Cc: Bin Meng <bmeng.cn at gmail.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>
> Subject: Re: [PATCH 4/8] x86: slimbootloader: Support 64-bit operation
> 
> Hi Aiden,
> 
> On Tue, 21 Apr 2020 at 18:45, <aiden.park at intel.com> wrote:
> >
> > From: Aiden Park <aiden.park at intel.com>
> >
> > This supports 64-bit U-Boot as a Slim Bootloader payload.
> >
> > Signed-off-by: Aiden Park <aiden.park at intel.com>
> > ---
> >  arch/x86/cpu/slimbootloader/Makefile         |  9 +++++++--
> >  arch/x86/cpu/slimbootloader/entry64.S        | 14 ++++++++++++++
> >  arch/x86/cpu/slimbootloader/slimbootloader.c | 17 +++++++++++++++--
> >  3 files changed, 36 insertions(+), 4 deletions(-)  create mode 100644
> > arch/x86/cpu/slimbootloader/entry64.S
> >
> > diff --git a/arch/x86/cpu/slimbootloader/Makefile
> > b/arch/x86/cpu/slimbootloader/Makefile
> > index aac9fa3db8..79fa699501 100644
> > --- a/arch/x86/cpu/slimbootloader/Makefile
> > +++ b/arch/x86/cpu/slimbootloader/Makefile
> > @@ -1,5 +1,10 @@
> >  # SPDX-License-Identifier: GPL-2.0+
> >  #
> > -# Copyright (C) 2019 Intel Corporation <www.intel.com>
> > +# Copyright (C) 2019-2020 Intel Corporation <www.intel.com>
> >
> > -obj-y += car.o slimbootloader.o sdram.o serial.o
> > +ifeq ($(CONFIG_X86_64),y)
> > +obj-y += entry64.o
> > +else
> > +obj-y += car.o
> > +endif
> > +obj-y += slimbootloader.o sdram.o serial.o
> > diff --git a/arch/x86/cpu/slimbootloader/entry64.S
> > b/arch/x86/cpu/slimbootloader/entry64.S
> > new file mode 100644
> > index 0000000000..5e101e18a9
> > --- /dev/null
> > +++ b/arch/x86/cpu/slimbootloader/entry64.S
> > @@ -0,0 +1,14 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (C) 2020 Intel Corporation <www.intel.com>  */
> > +
> > +#include <generated/asm-offsets.h>
> > +
> > +.section .text
> > +
> > +.globl init_64bit_entry
> > +init_64bit_entry:
> > +       /* Save hob pointer parameter */
> > +       mov     %rcx, %r10
> > +       jmp     init_64bit_entry_ret
> > diff --git a/arch/x86/cpu/slimbootloader/slimbootloader.c
> > b/arch/x86/cpu/slimbootloader/slimbootloader.c
> > index 21dcfb2142..7857e4cd8b 100644
> > --- a/arch/x86/cpu/slimbootloader/slimbootloader.c
> > +++ b/arch/x86/cpu/slimbootloader/slimbootloader.c
> > @@ -1,6 +1,6 @@
> >  // SPDX-License-Identifier: GPL-2.0+
> >  /*
> > - * Copyright (C) 2019 Intel Corporation <www.intel.com>
> > + * Copyright (C) 2019-2020 Intel Corporation <www.intel.com>
> >   */
> >
> >  #include <common.h>
> > @@ -43,11 +43,23 @@ static void tsc_init(void)
> >
> >  int arch_cpu_init(void)
> >  {
> > +       int ret = 0;
> > +
> >         tsc_init();
> >
> > -       return x86_cpu_init_f();
> > +#if !CONFIG_IS_ENABLED(X86_64)
> 
> Can you use if() instead of #if ?
I will do it.

> 
> > +       ret = x86_cpu_init_f();
> > +#endif
> > +       return ret;
> >  }
> >
> > +#if CONFIG_IS_ENABLED(X86_64)
> 
> It should be safe to define both of these functions so I don't think you need the
> #ifdef
These are defined in arch/x86/cpu/x86_64/cpu.c already.
Is it okay to make them weak reference or can I keep this as it is?

> 
> > +int set_hob_list(void *hob_list)
> > +{
> > +       gd->arch.hob_list = hob_list;
> > +       return 0;
> > +}
> > +#else
> >  int checkcpu(void)
> >  {
> >         return 0;
> > @@ -57,3 +69,4 @@ int print_cpuinfo(void)  {
> >         return default_print_cpuinfo();  }
> > +#endif
> > --
> > 2.20.1
> >
> 
> Regards,
> Simon

Best Regards,
Aiden
Simon Glass April 29, 2020, 6:03 p.m. UTC | #3 
Hi Aiden,

On Wed, 29 Apr 2020 at 00:01, Park, Aiden <aiden.park at intel.com> wrote:
>
> Hi Simon,
>
> > -----Original Message-----
> > From: Simon Glass <sjg at chromium.org>
> > Sent: Sunday, April 26, 2020 1:16 PM
> > To: Park, Aiden <aiden.park at intel.com>
> > Cc: Bin Meng <bmeng.cn at gmail.com>; U-Boot Mailing List <u-
> > boot at lists.denx.de>
> > Subject: Re: [PATCH 4/8] x86: slimbootloader: Support 64-bit operation
> >
> > Hi Aiden,
> >
> > On Tue, 21 Apr 2020 at 18:45, <aiden.park at intel.com> wrote:
> > >
> > > From: Aiden Park <aiden.park at intel.com>
> > >
> > > This supports 64-bit U-Boot as a Slim Bootloader payload.
> > >
> > > Signed-off-by: Aiden Park <aiden.park at intel.com>
> > > ---
> > >  arch/x86/cpu/slimbootloader/Makefile         |  9 +++++++--
> > >  arch/x86/cpu/slimbootloader/entry64.S        | 14 ++++++++++++++
> > >  arch/x86/cpu/slimbootloader/slimbootloader.c | 17 +++++++++++++++--
> > >  3 files changed, 36 insertions(+), 4 deletions(-)  create mode 100644
> > > arch/x86/cpu/slimbootloader/entry64.S
> > >
> > > diff --git a/arch/x86/cpu/slimbootloader/Makefile
> > > b/arch/x86/cpu/slimbootloader/Makefile
> > > index aac9fa3db8..79fa699501 100644
> > > --- a/arch/x86/cpu/slimbootloader/Makefile
> > > +++ b/arch/x86/cpu/slimbootloader/Makefile
> > > @@ -1,5 +1,10 @@
> > >  # SPDX-License-Identifier: GPL-2.0+
> > >  #
> > > -# Copyright (C) 2019 Intel Corporation <www.intel.com>
> > > +# Copyright (C) 2019-2020 Intel Corporation <www.intel.com>
> > >
> > > -obj-y += car.o slimbootloader.o sdram.o serial.o
> > > +ifeq ($(CONFIG_X86_64),y)
> > > +obj-y += entry64.o
> > > +else
> > > +obj-y += car.o
> > > +endif
> > > +obj-y += slimbootloader.o sdram.o serial.o
> > > diff --git a/arch/x86/cpu/slimbootloader/entry64.S
> > > b/arch/x86/cpu/slimbootloader/entry64.S
> > > new file mode 100644
> > > index 0000000000..5e101e18a9
> > > --- /dev/null
> > > +++ b/arch/x86/cpu/slimbootloader/entry64.S
> > > @@ -0,0 +1,14 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * Copyright (C) 2020 Intel Corporation <www.intel.com>  */
> > > +
> > > +#include <generated/asm-offsets.h>
> > > +
> > > +.section .text
> > > +
> > > +.globl init_64bit_entry
> > > +init_64bit_entry:
> > > +       /* Save hob pointer parameter */
> > > +       mov     %rcx, %r10
> > > +       jmp     init_64bit_entry_ret
> > > diff --git a/arch/x86/cpu/slimbootloader/slimbootloader.c
> > > b/arch/x86/cpu/slimbootloader/slimbootloader.c
> > > index 21dcfb2142..7857e4cd8b 100644
> > > --- a/arch/x86/cpu/slimbootloader/slimbootloader.c
> > > +++ b/arch/x86/cpu/slimbootloader/slimbootloader.c
> > > @@ -1,6 +1,6 @@
> > >  // SPDX-License-Identifier: GPL-2.0+
> > >  /*
> > > - * Copyright (C) 2019 Intel Corporation <www.intel.com>
> > > + * Copyright (C) 2019-2020 Intel Corporation <www.intel.com>
> > >   */
> > >
> > >  #include <common.h>
> > > @@ -43,11 +43,23 @@ static void tsc_init(void)
> > >
> > >  int arch_cpu_init(void)
> > >  {
> > > +       int ret = 0;
> > > +
> > >         tsc_init();
> > >
> > > -       return x86_cpu_init_f();
> > > +#if !CONFIG_IS_ENABLED(X86_64)
> >
> > Can you use if() instead of #if ?
> I will do it.
>
> >
> > > +       ret = x86_cpu_init_f();
> > > +#endif
> > > +       return ret;
> > >  }
> > >
> > > +#if CONFIG_IS_ENABLED(X86_64)
> >
> > It should be safe to define both of these functions so I don't think you need the
> > #ifdef
> These are defined in arch/x86/cpu/x86_64/cpu.c already.
> Is it okay to make them weak reference or can I keep this as it is?

Oh I see. I am not a fan of weak functions so perhaps we should keep them as is.


>
> >
> > > +int set_hob_list(void *hob_list)
> > > +{
> > > +       gd->arch.hob_list = hob_list;
> > > +       return 0;
> > > +}
> > > +#else
> > >  int checkcpu(void)
> > >  {
> > >         return 0;
> > > @@ -57,3 +69,4 @@ int print_cpuinfo(void)  {
> > >         return default_print_cpuinfo();  }
> > > +#endif
> > > --
> > > 2.20.1
> > >
Regards,
Simon
Park, Aiden May 1, 2020, 6:34 p.m. UTC | #4 
Hi Simon,

> -----Original Message-----
> From: Simon Glass <sjg at chromium.org>
> Sent: Wednesday, April 29, 2020 11:04 AM
> To: Park, Aiden <aiden.park at intel.com>
> Cc: Bin Meng <bmeng.cn at gmail.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>
> Subject: Re: [PATCH 4/8] x86: slimbootloader: Support 64-bit operation
> 
> Hi Aiden,
> 
> On Wed, 29 Apr 2020 at 00:01, Park, Aiden <aiden.park at intel.com> wrote:
> >
> > Hi Simon,
> >
> > > -----Original Message-----
> > > From: Simon Glass <sjg at chromium.org>
> > > Sent: Sunday, April 26, 2020 1:16 PM
> > > To: Park, Aiden <aiden.park at intel.com>
> > > Cc: Bin Meng <bmeng.cn at gmail.com>; U-Boot Mailing List <u-
> > > boot at lists.denx.de>
> > > Subject: Re: [PATCH 4/8] x86: slimbootloader: Support 64-bit
> > > operation
> > >
> > > Hi Aiden,
> > >
> > > On Tue, 21 Apr 2020 at 18:45, <aiden.park at intel.com> wrote:
> > > >
> > > > From: Aiden Park <aiden.park at intel.com>
> > > >
> > > > This supports 64-bit U-Boot as a Slim Bootloader payload.
> > > >
> > > > Signed-off-by: Aiden Park <aiden.park at intel.com>
> > > > ---
> > > >  arch/x86/cpu/slimbootloader/Makefile         |  9 +++++++--
> > > >  arch/x86/cpu/slimbootloader/entry64.S        | 14 ++++++++++++++
> > > >  arch/x86/cpu/slimbootloader/slimbootloader.c | 17
> > > > +++++++++++++++--
> > > >  3 files changed, 36 insertions(+), 4 deletions(-)  create mode
> > > > 100644 arch/x86/cpu/slimbootloader/entry64.S
> > > >
> > > > diff --git a/arch/x86/cpu/slimbootloader/Makefile
> > > > b/arch/x86/cpu/slimbootloader/Makefile
> > > > index aac9fa3db8..79fa699501 100644
> > > > --- a/arch/x86/cpu/slimbootloader/Makefile
> > > > +++ b/arch/x86/cpu/slimbootloader/Makefile
> > > > @@ -1,5 +1,10 @@
> > > >  # SPDX-License-Identifier: GPL-2.0+  # -# Copyright (C) 2019
> > > > Intel Corporation <www.intel.com>
> > > > +# Copyright (C) 2019-2020 Intel Corporation <www.intel.com>
> > > >
> > > > -obj-y += car.o slimbootloader.o sdram.o serial.o
> > > > +ifeq ($(CONFIG_X86_64),y)
> > > > +obj-y += entry64.o
> > > > +else
> > > > +obj-y += car.o
> > > > +endif
> > > > +obj-y += slimbootloader.o sdram.o serial.o
> > > > diff --git a/arch/x86/cpu/slimbootloader/entry64.S
> > > > b/arch/x86/cpu/slimbootloader/entry64.S
> > > > new file mode 100644
> > > > index 0000000000..5e101e18a9
> > > > --- /dev/null
> > > > +++ b/arch/x86/cpu/slimbootloader/entry64.S
> > > > @@ -0,0 +1,14 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > +/*
> > > > + * Copyright (C) 2020 Intel Corporation <www.intel.com>  */
> > > > +
> > > > +#include <generated/asm-offsets.h>
> > > > +
> > > > +.section .text
> > > > +
> > > > +.globl init_64bit_entry
> > > > +init_64bit_entry:
> > > > +       /* Save hob pointer parameter */
> > > > +       mov     %rcx, %r10
> > > > +       jmp     init_64bit_entry_ret
> > > > diff --git a/arch/x86/cpu/slimbootloader/slimbootloader.c
> > > > b/arch/x86/cpu/slimbootloader/slimbootloader.c
> > > > index 21dcfb2142..7857e4cd8b 100644
> > > > --- a/arch/x86/cpu/slimbootloader/slimbootloader.c
> > > > +++ b/arch/x86/cpu/slimbootloader/slimbootloader.c
> > > > @@ -1,6 +1,6 @@
> > > >  // SPDX-License-Identifier: GPL-2.0+
> > > >  /*
> > > > - * Copyright (C) 2019 Intel Corporation <www.intel.com>
> > > > + * Copyright (C) 2019-2020 Intel Corporation <www.intel.com>
> > > >   */
> > > >
> > > >  #include <common.h>
> > > > @@ -43,11 +43,23 @@ static void tsc_init(void)
> > > >
> > > >  int arch_cpu_init(void)
> > > >  {
> > > > +       int ret = 0;
> > > > +
> > > >         tsc_init();
> > > >
> > > > -       return x86_cpu_init_f();
> > > > +#if !CONFIG_IS_ENABLED(X86_64)
> > >
> > > Can you use if() instead of #if ?
> > I will do it.
> >
> > >
> > > > +       ret = x86_cpu_init_f();
> > > > +#endif
> > > > +       return ret;
> > > >  }
> > > >
> > > > +#if CONFIG_IS_ENABLED(X86_64)
> > >
> > > It should be safe to define both of these functions so I don't think
> > > you need the #ifdef
> > These are defined in arch/x86/cpu/x86_64/cpu.c already.
> > Is it okay to make them weak reference or can I keep this as it is?
> 
> Oh I see. I am not a fan of weak functions so perhaps we should keep them as is.
I agree with you about the weak functions. Let me keep this as is. Thanks.

> 
> 
> >
> > >
> > > > +int set_hob_list(void *hob_list)
> > > > +{
> > > > +       gd->arch.hob_list = hob_list;
> > > > +       return 0;
> > > > +}
> > > > +#else
> > > >  int checkcpu(void)
> > > >  {
> > > >         return 0;
> > > > @@ -57,3 +69,4 @@ int print_cpuinfo(void)  {
> > > >         return default_print_cpuinfo();  }
> > > > +#endif
> > > > --
> > > > 2.20.1
> > > >
> Regards,
> Simon

Best Regards,
Aiden
diff mbox series

Patch

diff --git a/arch/x86/cpu/slimbootloader/Makefile b/arch/x86/cpu/slimbootloader/Makefile
index aac9fa3db8..79fa699501 100644
--- a/arch/x86/cpu/slimbootloader/Makefile
+++ b/arch/x86/cpu/slimbootloader/Makefile
@@ -1,5 +1,10 @@ 
 # SPDX-License-Identifier: GPL-2.0+
 #
-# Copyright (C) 2019 Intel Corporation <www.intel.com>
+# Copyright (C) 2019-2020 Intel Corporation <www.intel.com>
 
-obj-y += car.o slimbootloader.o sdram.o serial.o
+ifeq ($(CONFIG_X86_64),y)
+obj-y += entry64.o
+else
+obj-y += car.o
+endif
+obj-y += slimbootloader.o sdram.o serial.o
diff --git a/arch/x86/cpu/slimbootloader/entry64.S b/arch/x86/cpu/slimbootloader/entry64.S
new file mode 100644
index 0000000000..5e101e18a9
--- /dev/null
+++ b/arch/x86/cpu/slimbootloader/entry64.S
@@ -0,0 +1,14 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2020 Intel Corporation <www.intel.com>
+ */
+
+#include <generated/asm-offsets.h>
+
+.section .text
+
+.globl init_64bit_entry
+init_64bit_entry:
+	/* Save hob pointer parameter */
+	mov	%rcx, %r10
+	jmp	init_64bit_entry_ret
diff --git a/arch/x86/cpu/slimbootloader/slimbootloader.c b/arch/x86/cpu/slimbootloader/slimbootloader.c
index 21dcfb2142..7857e4cd8b 100644
--- a/arch/x86/cpu/slimbootloader/slimbootloader.c
+++ b/arch/x86/cpu/slimbootloader/slimbootloader.c
@@ -1,6 +1,6 @@ 
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Copyright (C) 2019 Intel Corporation <www.intel.com>
+ * Copyright (C) 2019-2020 Intel Corporation <www.intel.com>
  */
 
 #include <common.h>
@@ -43,11 +43,23 @@  static void tsc_init(void)
 
 int arch_cpu_init(void)
 {
+	int ret = 0;
+
 	tsc_init();
 
-	return x86_cpu_init_f();
+#if !CONFIG_IS_ENABLED(X86_64)
+	ret = x86_cpu_init_f();
+#endif
+	return ret;
 }
 
+#if CONFIG_IS_ENABLED(X86_64)
+int set_hob_list(void *hob_list)
+{
+	gd->arch.hob_list = hob_list;
+	return 0;
+}
+#else
 int checkcpu(void)
 {
 	return 0;
@@ -57,3 +69,4 @@  int print_cpuinfo(void)
 {
 	return default_print_cpuinfo();
 }
+#endif