Hi Aiden, On Wed, 29 Apr 2020 at 00:01, Park, Aiden <aiden.p...@intel.com> wrote: > > Hi Simon, > > > -----Original Message----- > > From: Simon Glass <s...@chromium.org> > > Sent: Sunday, April 26, 2020 1:16 PM > > To: Park, Aiden <aiden.p...@intel.com> > > Cc: Bin Meng <bmeng...@gmail.com>; U-Boot Mailing List <u- > > b...@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.p...@intel.com> wrote: > > > > > > From: Aiden Park <aiden.p...@intel.com> > > > > > > This supports 64-bit U-Boot as a Slim Bootloader payload. > > > > > > Signed-off-by: Aiden Park <aiden.p...@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