From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Wed, 29 Apr 2020 12:03:47 -0600
Subject: [PATCH 4/8] x86: slimbootloader: Support 64-bit operation
In-Reply-To: <DM5PR11MB2043357B2B534F90022FFB428EAD0@DM5PR11MB2043.namprd11.prod.outlook.com>
References: <20200422004507.2025-1-aiden.park@intel.com>
 <20200422004507.2025-5-aiden.park@intel.com>
 <CAPnjgZ0yCBsbi4cCsJsd1_cxe3e7RKAUaEdtXa-D0FGFQ+KTyg@mail.gmail.com>
 <DM5PR11MB2043357B2B534F90022FFB428EAD0@DM5PR11MB2043.namprd11.prod.outlook.com>
Message-ID: <CAPnjgZ3Vj9-QDpvp2LWXOEv2JzKLaqSxkSM94j4NsrEQynXtvQ@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Aiden,

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