Re: [U-Boot] [PATCH 06/11] imx: imx-common: introduce boot auxiliary core

[Peng Fan]

Hi Stefan,

Sorry for this late reply.

On Wed, Jan 13, 2016 at 12:45:05PM -0800, Stefan Agner wrote:
>Hi,
>
>I would like to keep the discussion going and shed some light on the
>image format introduced here, see below...
>
>On 2016-01-07 00:38, Peng Fan wrote:
>> Hi Stefan,
>> On Wed, Jan 06, 2016 at 10:59:17PM -0800, Stefan Agner wrote:
>>>On 2016-01-04 21:56, Peng Fan wrote:
 From: Peng Fan 

 To boot a auxiliary core in asymmetric multicore system, introduce the
 new command "bootaux" to do it. Example of boot auxliary core from
 0x7000 where stores the boot head information that should be
 parsed by auxiliary core, "bootaux 0x7000".
>>>
>>>This reminds me of a question which was nagging me lately:
>>>Is the M4 core of SoloX/MX7 running a boot ROM? Or who/what is "parsing
>>>the boot head information"?
>> 
>> There is no bootrom for m4 core. The bootimage for M4 contains stack
>> info and pc info. bootaux command will use the info extracted from bootimage.
>
>
>So the image expected by bootaux is really a raw binary image, with the
>only notion that the first two words need to be the stack pointer and
>the reset handler (firmware entry point).

Yeah.

>
>U-Boot has other commands which work with "raw" images, such as the
>bootz command. The bootz command also expects a certain "raw" format,
>hence I guess it is ok to introduce something like that also for the
>auxiliary core.

Here bootaux will not let uboot lose control. bootz will let os get the control
to soc. But I agree we add a simliar command, I personally think boot[aux]
is good :).

>
>Also, since every standard Cortex-M4 vector table begins with SP and
>reset handler, it is very likely that a firmwares linker file put the
>vector table also at the beginning of the image. Otherwise, one would
>need to adopt the linker file to create a "compatible" image. Btw, as Ye
>Li pointed out, the CPU needs those two words located at 0x00,
>this is what bootaux is actually doing, coping the information to the
>right location for the M4 CPU. As an alternative, SCB_VTOR could be used
>to point to the vector table. However that can only be controlled from
>the M4 itself, hence is not an option for the bootaux command (as I did
>it for the libopencm3 Cortex-M4 support for Vybrid, see
>https://github.com/libopencm3/libopencm3/blob/master/lib/vf6xx/vector_chipset.c).
>
>However, IMHO, this patch should at least add this
>information/restriction to a README and maybe even give a hint in the
>help text what is expected here

Add a README for bootaux command? or you mean the image format?

>
>The other restriction is that the binary needs to be loaded to the
>location where the firmware has been linked to. Since the firmware is a
>"raw" image, this information need to be carried along the binary (I
>need to know that binary xyz.bin is linked to work at memory location
>0x7F8000...). This is different to the bootz command: A raw Linux kernel
>zImage has a position independent loader at the very beginning and hence
>can be placed anywhere.

Good point. How about let bootaux to handle libz compressed M4 image?
>
>I don't really like that this information need to be carried along
>separately.
>
>In our downstream U-Boot for Vybrid, which has a M4 core too, I
>introduced a m4boot command which makes use of the FIT image format to
>carry this information, see:
>http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex-next=d1d416f272e840e8139aec911f89a70fe5523eb2
>
>It comes with the cost that each firmware need to be packed into a FIT
>image using mkimage, but then one can just call m4boot image> and the command will make sure that the image is placed at the
>right location in memory.
>
>Another possible option would be to introduce elf support. I like this
>approach since the Linux framework remoteproc also supports loading elf
>firmwares, hence this would align nicely with what has been done in the
>Linux kernel.
>
>The elf format has all information required to load the firmware:
>Sections and their location (in M4 terms) as well as the start address.
>The platform data would only need to have a translation table for the
>section addresses (0x1fff8000 <=> 0x7F8000) to be able to load the
>firmware to the correct location. It comes with the cost of coping the
>firmware to the right location, but it would be much easier to handle
>elf firmware files.

If we need to let uboot support more functions such as interact with M4 cores,
remoteproc maybe a better choice. For simple usage only needs to boot M4,
bootaux is enough, I think.


Regards,
Peng.

>
>Other opinions?
>
>One more thing below:
>
>> 
>> See the following code:
>> "
>> int arch_auxiliary_core_up(u32 core_id, u32 boot_private_data)
>> {
>>  struct src *src_reg;
>>  u32 stack, pc;
>> 
>>  if (!boot_private_data)
>>  return 1;
>> 
>>  stack = *(u32 *)boot_private_data;
>>  pc = *(u32 

Re: [U-Boot] [PATCH 06/11] imx: imx-common: introduce boot auxiliary core

[Peng Fan]

Hi Simon,

Sorry for this late reply.
On Thu, Jan 14, 2016 at 10:17:20AM -0700, Simon Glass wrote:
>+Bin
>
>Hi Peng,
>
>On 4 January 2016 at 22:56, Peng Fan  wrote:
>> From: Peng Fan 
>>
>> To boot a auxiliary core in asymmetric multicore system, introduce the
>> new command "bootaux" to do it. Example of boot auxliary core from
>> 0x7000 where stores the boot head information that should be
>> parsed by auxiliary core, "bootaux 0x7000".
>> Introduce Kconfig option IMX_BOOTAUX.
>>
>> Signed-off-by: Ye.Li 
>> Signed-off-by: Peng Fan 
>> ---
>>  arch/arm/imx-common/Kconfig   |  6 
>>  arch/arm/imx-common/Makefile  |  1 +
>>  arch/arm/imx-common/imx_bootaux.c | 59 
>> +++
>>  3 files changed, 66 insertions(+)
>>  create mode 100644 arch/arm/imx-common/imx_bootaux.c
>>
>
>Could this use the CPU uclass?

Here we are just talking what kind of image and what command to boot a M4
core.
I do not have plan to use CPU uclass for this, since we have not
covert to support dtb in uboot for i.MX.

Thanks,
Peng.

>
>[snip]
>
>Regards,
>Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 06/11] imx: imx-common: introduce boot auxiliary core

[Stefan Agner]

On 2016-01-14 09:15, Tom Rini wrote:
> On Wed, Jan 13, 2016 at 12:45:05PM -0800, Stefan Agner wrote:
>> Hi,
>>
>> I would like to keep the discussion going and shed some light on the
>> image format introduced here, see below...
>>
>> On 2016-01-07 00:38, Peng Fan wrote:
>> > Hi Stefan,
>> > On Wed, Jan 06, 2016 at 10:59:17PM -0800, Stefan Agner wrote:
>> >>On 2016-01-04 21:56, Peng Fan wrote:
>> >>> From: Peng Fan 
>> >>>
>> >>> To boot a auxiliary core in asymmetric multicore system, introduce the
>> >>> new command "bootaux" to do it. Example of boot auxliary core from
>> >>> 0x7000 where stores the boot head information that should be
>> >>> parsed by auxiliary core, "bootaux 0x7000".
>> >>
>> >>This reminds me of a question which was nagging me lately:
>> >>Is the M4 core of SoloX/MX7 running a boot ROM? Or who/what is "parsing
>> >>the boot head information"?
>> >
>> > There is no bootrom for m4 core. The bootimage for M4 contains stack
>> > info and pc info. bootaux command will use the info extracted from 
>> > bootimage.
>>
>>
>> So the image expected by bootaux is really a raw binary image, with the
>> only notion that the first two words need to be the stack pointer and
>> the reset handler (firmware entry point).
>>
>> U-Boot has other commands which work with "raw" images, such as the
>> bootz command. The bootz command also expects a certain "raw" format,
>> hence I guess it is ok to introduce something like that also for the
>> auxiliary core.
> 
> I suppose my only contribution right now is that for another project
> where we have U-Boot (on the M4) kicking off NuttX we've had to do a
> custom command to stack pointer thing, I agree with the need to add some
> way to easily kick these cases off.

So the NuttX image was also kind of a raw image? Is that command
upstream?

--
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 06/11] imx: imx-common: introduce boot auxiliary core

[Tom Rini]

On Thu, Jan 14, 2016 at 09:54:24AM -0800, Stefan Agner wrote:
> On 2016-01-14 09:15, Tom Rini wrote:
> > On Wed, Jan 13, 2016 at 12:45:05PM -0800, Stefan Agner wrote:
> >> Hi,
> >>
> >> I would like to keep the discussion going and shed some light on the
> >> image format introduced here, see below...
> >>
> >> On 2016-01-07 00:38, Peng Fan wrote:
> >> > Hi Stefan,
> >> > On Wed, Jan 06, 2016 at 10:59:17PM -0800, Stefan Agner wrote:
> >> >>On 2016-01-04 21:56, Peng Fan wrote:
> >> >>> From: Peng Fan 
> >> >>>
> >> >>> To boot a auxiliary core in asymmetric multicore system, introduce the
> >> >>> new command "bootaux" to do it. Example of boot auxliary core from
> >> >>> 0x7000 where stores the boot head information that should be
> >> >>> parsed by auxiliary core, "bootaux 0x7000".
> >> >>
> >> >>This reminds me of a question which was nagging me lately:
> >> >>Is the M4 core of SoloX/MX7 running a boot ROM? Or who/what is "parsing
> >> >>the boot head information"?
> >> >
> >> > There is no bootrom for m4 core. The bootimage for M4 contains stack
> >> > info and pc info. bootaux command will use the info extracted from 
> >> > bootimage.
> >>
> >>
> >> So the image expected by bootaux is really a raw binary image, with the
> >> only notion that the first two words need to be the stack pointer and
> >> the reset handler (firmware entry point).
> >>
> >> U-Boot has other commands which work with "raw" images, such as the
> >> bootz command. The bootz command also expects a certain "raw" format,
> >> hence I guess it is ok to introduce something like that also for the
> >> auxiliary core.
> > 
> > I suppose my only contribution right now is that for another project
> > where we have U-Boot (on the M4) kicking off NuttX we've had to do a
> > custom command to stack pointer thing, I agree with the need to add some
> > way to easily kick these cases off.
> 
> So the NuttX image was also kind of a raw image? Is that command
> upstream?

The NuttX image is the "normal" raw cortex-m image format (everyone else
just writes it to flash and boots directly).  I haven't pushed the
patches upstream as the rest of the platform also isn't ready to go
upstream.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 06/11] imx: imx-common: introduce boot auxiliary core

[Tom Rini]

On Wed, Jan 13, 2016 at 12:45:05PM -0800, Stefan Agner wrote:
> Hi,
> 
> I would like to keep the discussion going and shed some light on the
> image format introduced here, see below...
> 
> On 2016-01-07 00:38, Peng Fan wrote:
> > Hi Stefan,
> > On Wed, Jan 06, 2016 at 10:59:17PM -0800, Stefan Agner wrote:
> >>On 2016-01-04 21:56, Peng Fan wrote:
> >>> From: Peng Fan 
> >>>
> >>> To boot a auxiliary core in asymmetric multicore system, introduce the
> >>> new command "bootaux" to do it. Example of boot auxliary core from
> >>> 0x7000 where stores the boot head information that should be
> >>> parsed by auxiliary core, "bootaux 0x7000".
> >>
> >>This reminds me of a question which was nagging me lately:
> >>Is the M4 core of SoloX/MX7 running a boot ROM? Or who/what is "parsing
> >>the boot head information"?
> > 
> > There is no bootrom for m4 core. The bootimage for M4 contains stack
> > info and pc info. bootaux command will use the info extracted from 
> > bootimage.
> 
> 
> So the image expected by bootaux is really a raw binary image, with the
> only notion that the first two words need to be the stack pointer and
> the reset handler (firmware entry point).
> 
> U-Boot has other commands which work with "raw" images, such as the
> bootz command. The bootz command also expects a certain "raw" format,
> hence I guess it is ok to introduce something like that also for the
> auxiliary core.

I suppose my only contribution right now is that for another project
where we have U-Boot (on the M4) kicking off NuttX we've had to do a
custom command to stack pointer thing, I agree with the need to add some
way to easily kick these cases off.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 06/11] imx: imx-common: introduce boot auxiliary core

[Simon Glass]

+Bin

Hi Peng,

On 4 January 2016 at 22:56, Peng Fan  wrote:
> From: Peng Fan 
>
> To boot a auxiliary core in asymmetric multicore system, introduce the
> new command "bootaux" to do it. Example of boot auxliary core from
> 0x7000 where stores the boot head information that should be
> parsed by auxiliary core, "bootaux 0x7000".
> Introduce Kconfig option IMX_BOOTAUX.
>
> Signed-off-by: Ye.Li 
> Signed-off-by: Peng Fan 
> ---
>  arch/arm/imx-common/Kconfig   |  6 
>  arch/arm/imx-common/Makefile  |  1 +
>  arch/arm/imx-common/imx_bootaux.c | 59 
> +++
>  3 files changed, 66 insertions(+)
>  create mode 100644 arch/arm/imx-common/imx_bootaux.c
>

Could this use the CPU uclass?

[snip]

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 06/11] imx: imx-common: introduce boot auxiliary core

[Stefan Agner]

Hi,

I would like to keep the discussion going and shed some light on the
image format introduced here, see below...

On 2016-01-07 00:38, Peng Fan wrote:
> Hi Stefan,
> On Wed, Jan 06, 2016 at 10:59:17PM -0800, Stefan Agner wrote:
>>On 2016-01-04 21:56, Peng Fan wrote:
>>> From: Peng Fan 
>>>
>>> To boot a auxiliary core in asymmetric multicore system, introduce the
>>> new command "bootaux" to do it. Example of boot auxliary core from
>>> 0x7000 where stores the boot head information that should be
>>> parsed by auxiliary core, "bootaux 0x7000".
>>
>>This reminds me of a question which was nagging me lately:
>>Is the M4 core of SoloX/MX7 running a boot ROM? Or who/what is "parsing
>>the boot head information"?
> 
> There is no bootrom for m4 core. The bootimage for M4 contains stack
> info and pc info. bootaux command will use the info extracted from bootimage.


So the image expected by bootaux is really a raw binary image, with the
only notion that the first two words need to be the stack pointer and
the reset handler (firmware entry point).

U-Boot has other commands which work with "raw" images, such as the
bootz command. The bootz command also expects a certain "raw" format,
hence I guess it is ok to introduce something like that also for the
auxiliary core.

Also, since every standard Cortex-M4 vector table begins with SP and
reset handler, it is very likely that a firmwares linker file put the
vector table also at the beginning of the image. Otherwise, one would
need to adopt the linker file to create a "compatible" image. Btw, as Ye
Li pointed out, the CPU needs those two words located at 0x00,
this is what bootaux is actually doing, coping the information to the
right location for the M4 CPU. As an alternative, SCB_VTOR could be used
to point to the vector table. However that can only be controlled from
the M4 itself, hence is not an option for the bootaux command (as I did
it for the libopencm3 Cortex-M4 support for Vybrid, see
https://github.com/libopencm3/libopencm3/blob/master/lib/vf6xx/vector_chipset.c).

However, IMHO, this patch should at least add this
information/restriction to a README and maybe even give a hint in the
help text what is expected here

The other restriction is that the binary needs to be loaded to the
location where the firmware has been linked to. Since the firmware is a
"raw" image, this information need to be carried along the binary (I
need to know that binary xyz.bin is linked to work at memory location
0x7F8000...). This is different to the bootz command: A raw Linux kernel
zImage has a position independent loader at the very beginning and hence
can be placed anywhere.

I don't really like that this information need to be carried along
separately.

In our downstream U-Boot for Vybrid, which has a M4 core too, I
introduced a m4boot command which makes use of the FIT image format to
carry this information, see:
http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex-next=d1d416f272e840e8139aec911f89a70fe5523eb2

It comes with the cost that each firmware need to be packed into a FIT
image using mkimage, but then one can just call m4boot  and the command will make sure that the image is placed at the
right location in memory.

Another possible option would be to introduce elf support. I like this
approach since the Linux framework remoteproc also supports loading elf
firmwares, hence this would align nicely with what has been done in the
Linux kernel.

The elf format has all information required to load the firmware:
Sections and their location (in M4 terms) as well as the start address.
The platform data would only need to have a translation table for the
section addresses (0x1fff8000 <=> 0x7F8000) to be able to load the
firmware to the correct location. It comes with the cost of coping the
firmware to the right location, but it would be much easier to handle
elf firmware files.

Other opinions?

One more thing below:

> 
> See the following code:
> "
> int arch_auxiliary_core_up(u32 core_id, u32 boot_private_data)
> {
>   struct src *src_reg;
>   u32 stack, pc;
> 
>   if (!boot_private_data)
>   return 1;
> 
>   stack = *(u32 *)boot_private_data;
>   pc = *(u32 *)(boot_private_data + 4);
> 
>   /* Set the stack and pc to M4 bootROM */
>   writel(stack, M4_BOOTROM_BASE_ADDR);
>   writel(pc, M4_BOOTROM_BASE_ADDR + 4);
> 
>   /* Enable M4 */
>   src_reg = (struct src *)SRC_BASE_ADDR;
>   setbits_le32(_reg->scr, 0x0040);
>   clrbits_le32(_reg->scr, 0x0010);
> 
>   return 0;
> }
> 
> "
> 
>>
>>
>>> Introduce Kconfig option IMX_BOOTAUX.
>>>
>>> Signed-off-by: Ye.Li 
>>> Signed-off-by: Peng Fan 
>>> ---
>>>  arch/arm/imx-common/Kconfig   |  6 
>>>  arch/arm/imx-common/Makefile  |  1 +
>>>  arch/arm/imx-common/imx_bootaux.c | 59 
>>> +++
>>>  3 files changed, 66 

Re: [U-Boot] [PATCH 06/11] imx: imx-common: introduce boot auxiliary core

[Ye Li]

Hi Stefan,

On 1/7/2016 4:52 PM, Peng Fan wrote:

Hi Stefan,
On Wed, Jan 06, 2016 at 10:59:17PM -0800, Stefan Agner wrote:


On 2016-01-04 21:56, Peng Fan wrote:


From: Peng Fan 

To boot a auxiliary core in asymmetric multicore system, introduce the
new command "bootaux" to do it. Example of boot auxliary core from
0x7000 where stores the boot head information that should be
parsed by auxiliary core, "bootaux 0x7000".



This reminds me of a question which was nagging me lately:
Is the M4 core of SoloX/MX7 running a boot ROM? Or who/what is "parsing
the boot head information"?



There is no bootrom for m4 core. The bootimage for M4 contains stack
info and pc info. bootaux command will use the info extracted from bootimage.

See the following code:
"
int arch_auxiliary_core_up(u32 core_id, u32 boot_private_data)
{
struct src *src_reg;
u32 stack, pc;

if (!boot_private_data)
return 1;

stack = *(u32 *)boot_private_data;
pc = *(u32 *)(boot_private_data + 4);

/* Set the stack and pc to M4 bootROM */
writel(stack, M4_BOOTROM_BASE_ADDR);
writel(pc, M4_BOOTROM_BASE_ADDR + 4);

/* Enable M4 */
src_reg = (struct src *)SRC_BASE_ADDR;
setbits_le32(_reg->scr, 0x0040);
clrbits_le32(_reg->scr, 0x0010);

return 0;
}

"


Per the cortex-M reference manual, the reset vector of M4 needs to exist at 0x0 
(TCMUL). The PC and SP are the first two addresses of that vector.  So to boot 
M4, the A core must build the M4's reset vector with getting the PC and SP from 
image and filling them to TCMUL. When M4 is kicked, it will load the PC and SP 
by itself. Freescale uses a back door (M4_BOOTROM_BASE_ADDR) at A core side for 
accessing the M4 TCMUL.









Introduce Kconfig option IMX_BOOTAUX.

Signed-off-by: Ye.Li 
Signed-off-by: Peng Fan 
---
 arch/arm/imx-common/Kconfig   |  6 
 arch/arm/imx-common/Makefile  |  1 +
 arch/arm/imx-common/imx_bootaux.c | 59 +++
 3 files changed, 66 insertions(+)
 create mode 100644 arch/arm/imx-common/imx_bootaux.c

diff --git a/arch/arm/imx-common/Kconfig b/arch/arm/imx-common/Kconfig
index c4f48bb..1b7da5a 100644
--- a/arch/arm/imx-common/Kconfig
+++ b/arch/arm/imx-common/Kconfig
@@ -11,3 +11,9 @@ config IMX_RDC
  i.MX Resource domain controller is used to assign masters
  and peripherals to differet domains. This can be used to
  isolate resources.
+
+config IMX_BOOTAUX
+   bool "Support boot auxiliary core"
+   depends on ARCH_MX7 || ARCH_MX6
+   help
+ bootaux [addr] to boot auxiliary core.
diff --git a/arch/arm/imx-common/Makefile b/arch/arm/imx-common/Makefile
index 568f41c..30e66ba 100644
--- a/arch/arm/imx-common/Makefile
+++ b/arch/arm/imx-common/Makefile
@@ -28,6 +28,7 @@ obj-y += cache.o init.o
 obj-$(CONFIG_CMD_SATA) += sata.o
 obj-$(CONFIG_IMX_VIDEO_SKIP) += video.o
 obj-$(CONFIG_IMX_RDC) += rdc-sema.o
+obj-$(CONFIG_IMX_BOOTAUX) += imx_bootaux.o
 obj-$(CONFIG_SECURE_BOOT)+= hab.o
 endif
 ifeq ($(SOC),$(filter $(SOC),vf610))
diff --git a/arch/arm/imx-common/imx_bootaux.c
b/arch/arm/imx-common/imx_bootaux.c
new file mode 100644
index 000..da424a7
--- /dev/null
+++ b/arch/arm/imx-common/imx_bootaux.c
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include 
+#include 
+
+/* Allow for arch specific config before we boot */
+static int __arch_auxiliary_core_up(u32 core_id, u32 boot_private_data)
+{
+   /* please define platform specific arch_auxiliary_core_up() */
+   return CMD_RET_FAILURE;
+}
+
+int arch_auxiliary_core_up(u32 core_id, u32 boot_private_data)
+   __attribute__((weak, alias("__arch_auxiliary_core_up")));
+
+/* Allow for arch specific config before we boot */
+static int __arch_auxiliary_core_check_up(u32 core_id)
+{
+   /* please define platform specific arch_auxiliary_core_check_up() */
+   return 0;
+}
+
+int arch_auxiliary_core_check_up(u32 core_id)
+   __attribute__((weak, alias("__arch_auxiliary_core_check_up")));
+
+int do_bootaux(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+   ulong addr;
+   int ret, up;
+
+   if (argc < 2)
+   return CMD_RET_USAGE;
+
+   up = arch_auxiliary_core_check_up(0);
+   if (up) {
+   printf("## Auxiliary core is already up\n");
+   return CMD_RET_SUCCESS;
+   }
+
+   addr = simple_strtoul(argv[1], NULL, 16);
+
+   printf("## Starting auxiliary core at 0x%08lX ...\n", addr);
+
+   ret = arch_auxiliary_core_up(0, addr);
+   if (ret)
+   return CMD_RET_FAILURE;
+
+   return CMD_RET_SUCCESS;
+}
+
+U_BOOT_CMD(
+   bootaux, CONFIG_SYS_MAXARGS, 1, do_bootaux,
+