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?
>
> > +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
