Re: [U-Boot] [PATCH v5 1/8] x86: Add new slimbootloader CPU type

2019-07-23 Thread Bin Meng
Hi Aiden,

On Wed, Jul 24, 2019 at 10:37 AM Park, Aiden  wrote:
>
> Hi Andy,
>
> > -Original Message-
> > From: Andy Shevchenko [mailto:andy.shevche...@gmail.com]
> > Sent: Monday, July 22, 2019 8:14 AM
> > To: Park, Aiden 
> > Cc: U-Boot Mailing List ; Simon Glass
> > ; Bin Meng 
> > Subject: Re: [PATCH v5 1/8] x86: Add new slimbootloader CPU type
> >
> > On Wed, Jul 17, 2019 at 7:41 AM Park, Aiden  wrote:
> > >
> > > This slimbootloader cpu type is to enable U-Boot as a payload which
> >
> > cpu -> CPU
> >
> Let me change this.
>
> > > runs on top of Slim Bootloader(https://github.com/slimbootloader).
> > > The Slim Bootloader is designed with multi-stage architecture for the
> > > execution from reset vector to OS booting, and supports qemu,
> >
> > qemu -> QEMU
> >
> Let me change this.
>
> > > Apollolake, Whiskeylake and Coffeelake platforms consuming Intel FSP
> > > (https://github.com/IntelFsp) for silicon initialization including CAR
> > > and memory initialization.
> > > The Slim Bootloader generates new HOB(Hand Off Block) which are serial
> > > port info, memory map info, performance data info and so on, and
> > > passes it to a Payload. U-Boot as a payload will use these HOB
> > > information for basic initialization such as serial console.
> >
> > > +config SYS_SLIMBOOTLOADER
> >
> > > +   bool
> > > +   default y
> >
> > def_bool y ?
> >
> Thanks. Let me fix this and select SYS_SLIMBOOTLOADER in board Kconfig.
>
> > > +   imply SYS_NS16550
> > > +   imply AHCI_PCI
> > > +   imply SCSI
> > > +   imply SCSI_AHCI
> > > +   imply MMC
> > > +   imply MMC_PCI
> > > +   imply MMC_SDHCI
> > > +   imply MMC_SDHCI_SDMA
> > > +   imply USB
> > > +   imply USB_EHCI_HCD
> > > +   imply USB_XHCI_HCD
> > > +   imply USB_STORAGE
> > > +   imply USB_KEYBOARD
> > > +   imply E1000
> >
> > > +   imply RTL8169
> >
> > Is it part of SoC? I dunno we have Realtek inside, usually either Intel or 
> > Synopsys.
> >
> Yes on silicon Slim Bootloader supports, but not verified with this series.
> I think removing this one would be better until this is really required.
>
> Hi Bin, RTL8169 has been added as your recommended, but it hasn't been 
> verified.
> Is it okay to skip adding RTL8169 in this series? Let me add this later if it 
> is really required.
>

RTL8169 is a common PCIe NIC so it's possible to have that wired on
board. As I mentioned, for any board drivers, let's move that to board
Kconfig file.

[snip]

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v5 1/8] x86: Add new slimbootloader CPU type

2019-07-23 Thread Park, Aiden
Hi Andy,

> -Original Message-
> From: Andy Shevchenko [mailto:andy.shevche...@gmail.com]
> Sent: Monday, July 22, 2019 8:14 AM
> To: Park, Aiden 
> Cc: U-Boot Mailing List ; Simon Glass
> ; Bin Meng 
> Subject: Re: [PATCH v5 1/8] x86: Add new slimbootloader CPU type
> 
> On Wed, Jul 17, 2019 at 7:41 AM Park, Aiden  wrote:
> >
> > This slimbootloader cpu type is to enable U-Boot as a payload which
> 
> cpu -> CPU
> 
Let me change this.

> > runs on top of Slim Bootloader(https://github.com/slimbootloader).
> > The Slim Bootloader is designed with multi-stage architecture for the
> > execution from reset vector to OS booting, and supports qemu,
> 
> qemu -> QEMU
> 
Let me change this.

> > Apollolake, Whiskeylake and Coffeelake platforms consuming Intel FSP
> > (https://github.com/IntelFsp) for silicon initialization including CAR
> > and memory initialization.
> > The Slim Bootloader generates new HOB(Hand Off Block) which are serial
> > port info, memory map info, performance data info and so on, and
> > passes it to a Payload. U-Boot as a payload will use these HOB
> > information for basic initialization such as serial console.
> 
> > +config SYS_SLIMBOOTLOADER
> 
> > +   bool
> > +   default y
> 
> def_bool y ?
>
Thanks. Let me fix this and select SYS_SLIMBOOTLOADER in board Kconfig.
 
> > +   imply SYS_NS16550
> > +   imply AHCI_PCI
> > +   imply SCSI
> > +   imply SCSI_AHCI
> > +   imply MMC
> > +   imply MMC_PCI
> > +   imply MMC_SDHCI
> > +   imply MMC_SDHCI_SDMA
> > +   imply USB
> > +   imply USB_EHCI_HCD
> > +   imply USB_XHCI_HCD
> > +   imply USB_STORAGE
> > +   imply USB_KEYBOARD
> > +   imply E1000
> 
> > +   imply RTL8169
> 
> Is it part of SoC? I dunno we have Realtek inside, usually either Intel or 
> Synopsys.
>
Yes on silicon Slim Bootloader supports, but not verified with this series.
I think removing this one would be better until this is really required.

Hi Bin, RTL8169 has been added as your recommended, but it hasn't been verified.
Is it okay to skip adding RTL8169 in this series? Let me add this later if it 
is really required.   

> > -#ifndef CONFIG_HAVE_FSP
> > +#if !defined(CONFIG_HAVE_FSP)
> && !defined(CONFIG_SYS_SLIMBOOTLOADER)
> 
> > -#ifdef CONFIG_HAVE_FSP
> > +#if defined(CONFIG_HAVE_FSP) || defined(CONFIG_SYS_SLIMBOOTLOADER)
> 
> Hmm... Maybe reasonable to have an additional option to tell something
> CONFIG_WE_HAVE_HOB_BUT_FSP.
>
Okay, let me add a new option - CONFIG_USE_HOB.
This will be selected by CONFIG_HAVE_FSP or CONFIG_SYS_SLIMBOOTLOADER.
 
> > /* Store the HOB list if we have one */
> > test%esi, %esi
> > jz  skip_hob
> > movl%esi, GD_HOB_LIST(%edx)
> >
> > +#ifdef CONFIG_HAVE_FSP
> 
> > +#endif
> 
> > +#ifndef __SLIMBOOTLOADER_ARCH_H__
> > +#define __SLIMBOOTLOADER_ARCH_H__
> > +
> > +#include 
> 
> Is it going to be expanded later?
> Otherwise I do not really see a point.
>
Yes, this is expanded in next patches. To avoid this confusion, let me remove 
this in 1st patch.

> > +#endif
> 
> > -#ifdef CONFIG_HAVE_FSP
> > +#if defined(CONFIG_HAVE_FSP) || defined(CONFIG_SYS_SLIMBOOTLOADER)
> 
> > -#ifdef CONFIG_HAVE_FSP
> > +#if defined(CONFIG_HAVE_FSP) || defined(CONFIG_SYS_SLIMBOOTLOADER)
> 
> Same as above.
> 
> --
> With Best Regards,
> Andy Shevchenko
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v5 1/8] x86: Add new slimbootloader CPU type

2019-07-22 Thread Bin Meng
On Mon, Jul 22, 2019 at 11:14 PM Andy Shevchenko
 wrote:
>
> On Wed, Jul 17, 2019 at 7:41 AM Park, Aiden  wrote:
> >
> > This slimbootloader cpu type is to enable U-Boot as a payload which
>
> cpu -> CPU
>
> > runs on top of Slim Bootloader(https://github.com/slimbootloader).
> > The Slim Bootloader is designed with multi-stage architecture for
> > the execution from reset vector to OS booting, and supports qemu,
>
> qemu -> QEMU
>
> > Apollolake, Whiskeylake and Coffeelake platforms consuming Intel FSP
> > (https://github.com/IntelFsp) for silicon initialization including
> > CAR and memory initialization.
> > The Slim Bootloader generates new HOB(Hand Off Block) which are
> > serial port info, memory map info, performance data info and so on,
> > and passes it to a Payload. U-Boot as a payload will use these HOB
> > information for basic initialization such as serial console.
>
> > +config SYS_SLIMBOOTLOADER
>
> > +   bool
> > +   default y
>
> def_bool y ?

Good catch. I checked other x86 Kconfig files, and seems we should
just remove this line, and specify SYS_SLIMBOOTLOADER in the board
defconfig file.

>
> > +   imply SYS_NS16550
> > +   imply AHCI_PCI
> > +   imply SCSI
> > +   imply SCSI_AHCI
> > +   imply MMC
> > +   imply MMC_PCI
> > +   imply MMC_SDHCI
> > +   imply MMC_SDHCI_SDMA
> > +   imply USB
> > +   imply USB_EHCI_HCD
> > +   imply USB_XHCI_HCD
> > +   imply USB_STORAGE
> > +   imply USB_KEYBOARD
> > +   imply E1000
>
> > +   imply RTL8169
>
> Is it part of SoC? I dunno we have Realtek inside, usually either
> Intel or Synopsys.
>

I think the following should be moved to
board/intel/slimbootloader/Kconfig::BOARD_SPECIFIC_OPTIONS:

imply USB_STORAGE
imply USB_KEYBOARD
imply RTL8169

> > -#ifndef CONFIG_HAVE_FSP
> > +#if !defined(CONFIG_HAVE_FSP) && !defined(CONFIG_SYS_SLIMBOOTLOADER)
>
> > -#ifdef CONFIG_HAVE_FSP
> > +#if defined(CONFIG_HAVE_FSP) || defined(CONFIG_SYS_SLIMBOOTLOADER)
>

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v5 1/8] x86: Add new slimbootloader CPU type

2019-07-22 Thread Andy Shevchenko
On Wed, Jul 17, 2019 at 7:41 AM Park, Aiden  wrote:
>
> This slimbootloader cpu type is to enable U-Boot as a payload which

cpu -> CPU

> runs on top of Slim Bootloader(https://github.com/slimbootloader).
> The Slim Bootloader is designed with multi-stage architecture for
> the execution from reset vector to OS booting, and supports qemu,

qemu -> QEMU

> Apollolake, Whiskeylake and Coffeelake platforms consuming Intel FSP
> (https://github.com/IntelFsp) for silicon initialization including
> CAR and memory initialization.
> The Slim Bootloader generates new HOB(Hand Off Block) which are
> serial port info, memory map info, performance data info and so on,
> and passes it to a Payload. U-Boot as a payload will use these HOB
> information for basic initialization such as serial console.

> +config SYS_SLIMBOOTLOADER

> +   bool
> +   default y

def_bool y ?

> +   imply SYS_NS16550
> +   imply AHCI_PCI
> +   imply SCSI
> +   imply SCSI_AHCI
> +   imply MMC
> +   imply MMC_PCI
> +   imply MMC_SDHCI
> +   imply MMC_SDHCI_SDMA
> +   imply USB
> +   imply USB_EHCI_HCD
> +   imply USB_XHCI_HCD
> +   imply USB_STORAGE
> +   imply USB_KEYBOARD
> +   imply E1000

> +   imply RTL8169

Is it part of SoC? I dunno we have Realtek inside, usually either
Intel or Synopsys.

> -#ifndef CONFIG_HAVE_FSP
> +#if !defined(CONFIG_HAVE_FSP) && !defined(CONFIG_SYS_SLIMBOOTLOADER)

> -#ifdef CONFIG_HAVE_FSP
> +#if defined(CONFIG_HAVE_FSP) || defined(CONFIG_SYS_SLIMBOOTLOADER)

Hmm... Maybe reasonable to have an additional option to tell something
CONFIG_WE_HAVE_HOB_BUT_FSP.

> /* Store the HOB list if we have one */
> test%esi, %esi
> jz  skip_hob
> movl%esi, GD_HOB_LIST(%edx)
>
> +#ifdef CONFIG_HAVE_FSP

> +#endif

> +#ifndef __SLIMBOOTLOADER_ARCH_H__
> +#define __SLIMBOOTLOADER_ARCH_H__
> +
> +#include 

Is it going to be expanded later?
Otherwise I do not really see a point.

> +#endif

> -#ifdef CONFIG_HAVE_FSP
> +#if defined(CONFIG_HAVE_FSP) || defined(CONFIG_SYS_SLIMBOOTLOADER)

> -#ifdef CONFIG_HAVE_FSP
> +#if defined(CONFIG_HAVE_FSP) || defined(CONFIG_SYS_SLIMBOOTLOADER)

Same as above.

-- 
With Best Regards,
Andy Shevchenko
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v5 1/8] x86: Add new slimbootloader CPU type

2019-07-16 Thread Park, Aiden
This slimbootloader cpu type is to enable U-Boot as a payload which
runs on top of Slim Bootloader(https://github.com/slimbootloader).
The Slim Bootloader is designed with multi-stage architecture for
the execution from reset vector to OS booting, and supports qemu,
Apollolake, Whiskeylake and Coffeelake platforms consuming Intel FSP
(https://github.com/IntelFsp) for silicon initialization including
CAR and memory initialization.
The Slim Bootloader generates new HOB(Hand Off Block) which are
serial port info, memory map info, performance data info and so on,
and passes it to a Payload. U-Boot as a payload will use these HOB
information for basic initialization such as serial console.

As an initial commit,
- Add CONFIG_SYS_SLIMBOOTLOADER to enable slimbootloader CPU type
- Add new arch/x86/cpu/slimbootloader directory with minimum codes
- Get hob_list pointer from Slim Bootloader

Signed-off-by: Aiden Park 
Reviewed-by: Bin Meng 
---

Changes in v4:
  * Use common arch/x86/cpu/start.S
  * Update U-boot or u-boot to U-Boot in commit message

Changes in v3:
  * Add a brief description about Slim Bootloader
  * Enable USB_KEYBOARD, E1000 and RTL8169 by default
  * Fix comment from code review

 arch/x86/Kconfig  |  1 +
 arch/x86/cpu/Makefile |  1 +
 arch/x86/cpu/slimbootloader/Kconfig   | 26 +++
 arch/x86/cpu/slimbootloader/Makefile  |  5 
 arch/x86/cpu/slimbootloader/car.S | 14 ++
 arch/x86/cpu/slimbootloader/slimbootloader.c  | 21 +++
 arch/x86/cpu/start.S  |  6 +++--
 .../asm/arch-slimbootloader/slimbootloader.h  | 11 
 arch/x86/include/asm/global_data.h|  2 +-
 arch/x86/lib/asm-offsets.c|  2 +-
 10 files changed, 85 insertions(+), 4 deletions(-)
 create mode 100644 arch/x86/cpu/slimbootloader/Kconfig
 create mode 100644 arch/x86/cpu/slimbootloader/Makefile
 create mode 100644 arch/x86/cpu/slimbootloader/car.S
 create mode 100644 arch/x86/cpu/slimbootloader/slimbootloader.c
 create mode 100644 arch/x86/include/asm/arch-slimbootloader/slimbootloader.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 70f939869a..27b7b767b9 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -115,6 +115,7 @@ source "arch/x86/cpu/efi/Kconfig"
 source "arch/x86/cpu/qemu/Kconfig"
 source "arch/x86/cpu/quark/Kconfig"
 source "arch/x86/cpu/queensbay/Kconfig"
+source "arch/x86/cpu/slimbootloader/Kconfig"
 source "arch/x86/cpu/tangier/Kconfig"

 # architecture-specific options below
diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile
index 85fd5e616e..3f1f62da2b 100644
--- a/arch/x86/cpu/Makefile
+++ b/arch/x86/cpu/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_INTEL_BAYTRAIL) += baytrail/
 obj-$(CONFIG_INTEL_BRASWELL) += braswell/
 obj-$(CONFIG_INTEL_BROADWELL) += broadwell/
 obj-$(CONFIG_SYS_COREBOOT) += coreboot/
+obj-$(CONFIG_SYS_SLIMBOOTLOADER) += slimbootloader/
 obj-$(CONFIG_EFI) += efi/
 obj-$(CONFIG_QEMU) += qemu/
 obj-$(CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE) += ivybridge/
diff --git a/arch/x86/cpu/slimbootloader/Kconfig 
b/arch/x86/cpu/slimbootloader/Kconfig
new file mode 100644
index 00..439e4b1e2c
--- /dev/null
+++ b/arch/x86/cpu/slimbootloader/Kconfig
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (C) 2019 Intel Corporation 
+
+if TARGET_SLIMBOOTLOADER
+
+config SYS_SLIMBOOTLOADER
+   bool
+   default y
+   imply SYS_NS16550
+   imply AHCI_PCI
+   imply SCSI
+   imply SCSI_AHCI
+   imply MMC
+   imply MMC_PCI
+   imply MMC_SDHCI
+   imply MMC_SDHCI_SDMA
+   imply USB
+   imply USB_EHCI_HCD
+   imply USB_XHCI_HCD
+   imply USB_STORAGE
+   imply USB_KEYBOARD
+   imply E1000
+   imply RTL8169
+
+endif
diff --git a/arch/x86/cpu/slimbootloader/Makefile 
b/arch/x86/cpu/slimbootloader/Makefile
new file mode 100644
index 00..627a721e8c
--- /dev/null
+++ b/arch/x86/cpu/slimbootloader/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (C) 2019 Intel Corporation 
+
+obj-y += car.o slimbootloader.o
diff --git a/arch/x86/cpu/slimbootloader/car.S 
b/arch/x86/cpu/slimbootloader/car.S
new file mode 100644
index 00..6e0304333c
--- /dev/null
+++ b/arch/x86/cpu/slimbootloader/car.S
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2019 Intel Corporation 
+ */
+
+#include 
+
+.section .text
+
+.globl car_init
+car_init:
+   /* Get hob pointer parameter from previous stage's stack */
+   mov 0x4(%esp), %esi
+   jmp car_init_ret
diff --git a/arch/x86/cpu/slimbootloader/slimbootloader.c 
b/arch/x86/cpu/slimbootloader/slimbootloader.c
new file mode 100644
index 00..9f3a61ec61
--- /dev/null
+++ b/arch/x86/cpu/slimbootloader/slimbootloader.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Intel Corporation 
+ */
+
+#include