[U-Boot,1/5] ARM: socfpga: Add boot trampoline for Arria10

Message ID 20180415133704.18950-1-marex@denx.de
State New
Delegated to: Marek Vasut
Headers show
Series
  • [U-Boot,1/5] ARM: socfpga: Add boot trampoline for Arria10
Related show

Commit Message

Marek Vasut April 15, 2018, 1:37 p.m.
The Arria10 uses slightly different boot image header than the Gen5 SoCs,
in particular the header itself contains an offset from the start of the
header to which the Arria10 jumps. This offset must not be negative, yet
the header is placed at offset 0x40 of the bootable binary. Therefore, to
jump into U-Boot, add a trampoline just past the Arria10 boot header and
point to this trampoline at fixed offset from the header generated using
the mkimage -T socfpgaimage_v1 . Note that it is not needed to jump back
to offset 0x0 of the image, it is possible to jump directly at the reset
label and save processing two instructions.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Chin Liang See <chin.liang.see@intel.com>
---
 arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

See, Chin Liang April 17, 2018, 8:40 a.m. | #1
Hi Marek,

On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> The Arria10 uses slightly different boot image header than the Gen5
> SoCs,
> in particular the header itself contains an offset from the start of
> the
> header to which the Arria10 jumps. This offset must not be negative,
> yet
> the header is placed at offset 0x40 of the bootable binary.
> Therefore, to
> jump into U-Boot, add a trampoline just past the Arria10 boot header
> and
> point to this trampoline at fixed offset from the header generated
> using
> the mkimage -T socfpgaimage_v1 . Note that it is not needed to jump
> back
> to offset 0x0 of the image, it is possible to jump directly at the
> reset
> label and save processing two instructions.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Chin Liang See <chin.liang.see@intel.com>
> ---
>  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
> b/arch/arm/mach-socfpga/include/mach/boot0.h
> index d6b9435d33..06bbe27d2c 100644
> --- a/arch/arm/mach-socfpga/include/mach/boot0.h
> +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
> @@ -18,10 +18,10 @@ _start:
>  	.word	0xcafec0d3;	/* Checksum, zero-pad */
>  	nop;
>  
> -	b reset;		/* SoCFPGA jumps here */
> -	nop;
> +	b reset;		/* SoCFPGA Gen5 jumps here */
>  	nop;
>  	nop;
> +	b reset;		/* SoCFPGA Gen10 trampoline */

Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder can we
standardize that by using 0x14 instead of proposed 0x18 in this patch?

Thanks
Chin Liang

>  #endif
>  
>  #endif /* __BOOT0_H */
Marek Vasut April 17, 2018, 8:46 a.m. | #2
On 04/17/2018 10:40 AM, See, Chin Liang wrote:
> Hi Marek,
> 
> On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
>> The Arria10 uses slightly different boot image header than the Gen5
>> SoCs,
>> in particular the header itself contains an offset from the start of
>> the
>> header to which the Arria10 jumps. This offset must not be negative,
>> yet
>> the header is placed at offset 0x40 of the bootable binary.
>> Therefore, to
>> jump into U-Boot, add a trampoline just past the Arria10 boot header
>> and
>> point to this trampoline at fixed offset from the header generated
>> using
>> the mkimage -T socfpgaimage_v1 . Note that it is not needed to jump
>> back
>> to offset 0x0 of the image, it is possible to jump directly at the
>> reset
>> label and save processing two instructions.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> Cc: Chin Liang See <chin.liang.see@intel.com>
>> ---
>>  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
>> b/arch/arm/mach-socfpga/include/mach/boot0.h
>> index d6b9435d33..06bbe27d2c 100644
>> --- a/arch/arm/mach-socfpga/include/mach/boot0.h
>> +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
>> @@ -18,10 +18,10 @@ _start:
>>  	.word	0xcafec0d3;	/* Checksum, zero-pad */
>>  	nop;
>>  
>> -	b reset;		/* SoCFPGA jumps here */
>> -	nop;
>> +	b reset;		/* SoCFPGA Gen5 jumps here */
>>  	nop;
>>  	nop;
>> +	b reset;		/* SoCFPGA Gen10 trampoline */
> 
> Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder can we
> standardize that by using 0x14 instead of proposed 0x18 in this patch?

What difference does it make, the entire image is generated during the
build anyway ? This patch uses offset 0x1c, but what is the reason for
address 0x14 in your proprietary tool, is there one ?
See, Chin Liang April 17, 2018, 8:52 a.m. | #3
On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
> On 04/17/2018 10:40 AM, See, Chin Liang wrote:
> > 
> > Hi Marek,
> > 
> > On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> > > 
> > > The Arria10 uses slightly different boot image header than the
> > > Gen5
> > > SoCs,
> > > in particular the header itself contains an offset from the start
> > > of
> > > the
> > > header to which the Arria10 jumps. This offset must not be
> > > negative,
> > > yet
> > > the header is placed at offset 0x40 of the bootable binary.
> > > Therefore, to
> > > jump into U-Boot, add a trampoline just past the Arria10 boot
> > > header
> > > and
> > > point to this trampoline at fixed offset from the header
> > > generated
> > > using
> > > the mkimage -T socfpgaimage_v1 . Note that it is not needed to
> > > jump
> > > back
> > > to offset 0x0 of the image, it is possible to jump directly at
> > > the
> > > reset
> > > label and save processing two instructions.
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > > Cc: Chin Liang See <chin.liang.see@intel.com>
> > > ---
> > >  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > index d6b9435d33..06bbe27d2c 100644
> > > --- a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > @@ -18,10 +18,10 @@ _start:
> > >  	.word	0xcafec0d3;	/* Checksum, zero-pad */
> > >  	nop;
> > >  
> > > -	b reset;		/* SoCFPGA jumps here */
> > > -	nop;
> > > +	b reset;		/* SoCFPGA Gen5 jumps here */
> > >  	nop;
> > >  	nop;
> > > +	b reset;		/* SoCFPGA Gen10 trampoline */
> > Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder can
> > we
> > standardize that by using 0x14 instead of proposed 0x18 in this
> > patch?
> What difference does it make, the entire image is generated during
> the
> build anyway ? This patch uses offset 0x1c, but what is the reason
> for
> address 0x14 in your proprietary tool, is there one ?

Our A10 header ended at 0x13 today. Hence we are continuing the code at
0x14 without any spacing.

While for 0x1c, should it be 3 nop?

Thanks
Chin Liang

>
Marek Vasut April 17, 2018, 9:01 a.m. | #4
On 04/17/2018 10:52 AM, See, Chin Liang wrote:
> On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
>> On 04/17/2018 10:40 AM, See, Chin Liang wrote:
>>>
>>> Hi Marek,
>>>
>>> On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
>>>>
>>>> The Arria10 uses slightly different boot image header than the
>>>> Gen5
>>>> SoCs,
>>>> in particular the header itself contains an offset from the start
>>>> of
>>>> the
>>>> header to which the Arria10 jumps. This offset must not be
>>>> negative,
>>>> yet
>>>> the header is placed at offset 0x40 of the bootable binary.
>>>> Therefore, to
>>>> jump into U-Boot, add a trampoline just past the Arria10 boot
>>>> header
>>>> and
>>>> point to this trampoline at fixed offset from the header
>>>> generated
>>>> using
>>>> the mkimage -T socfpgaimage_v1 . Note that it is not needed to
>>>> jump
>>>> back
>>>> to offset 0x0 of the image, it is possible to jump directly at
>>>> the
>>>> reset
>>>> label and save processing two instructions.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>> ---
>>>>  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>> b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>> index d6b9435d33..06bbe27d2c 100644
>>>> --- a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>> +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>> @@ -18,10 +18,10 @@ _start:
>>>>  	.word	0xcafec0d3;	/* Checksum, zero-pad */
>>>>  	nop;
>>>>  
>>>> -	b reset;		/* SoCFPGA jumps here */
>>>> -	nop;
>>>> +	b reset;		/* SoCFPGA Gen5 jumps here */
>>>>  	nop;
>>>>  	nop;
>>>> +	b reset;		/* SoCFPGA Gen10 trampoline */
>>> Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder can
>>> we
>>> standardize that by using 0x14 instead of proposed 0x18 in this
>>> patch?
>> What difference does it make, the entire image is generated during
>> the
>> build anyway ? This patch uses offset 0x1c, but what is the reason
>> for
>> address 0x14 in your proprietary tool, is there one ?
> 
> Our A10 header ended at 0x13 today. Hence we are continuing the code at
> 0x14 without any spacing.
> 
> While for 0x1c, should it be 3 nop?

Yes, gives enough space were the header grow for whatever reason. Mind
you, the NOPs are not executed, the socfpga jumps to 0x1c directly via
0x0c -- Image entry offset
See, Chin Liang April 17, 2018, 9:11 a.m. | #5
On Tue, 2018-04-17 at 11:01 +0200, Marek Vasut wrote:
> On 04/17/2018 10:52 AM, See, Chin Liang wrote:
> > 
> > On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
> > > 
> > > On 04/17/2018 10:40 AM, See, Chin Liang wrote:
> > > > 
> > > > 
> > > > Hi Marek,
> > > > 
> > > > On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > The Arria10 uses slightly different boot image header than
> > > > > the
> > > > > Gen5
> > > > > SoCs,
> > > > > in particular the header itself contains an offset from the
> > > > > start
> > > > > of
> > > > > the
> > > > > header to which the Arria10 jumps. This offset must not be
> > > > > negative,
> > > > > yet
> > > > > the header is placed at offset 0x40 of the bootable binary.
> > > > > Therefore, to
> > > > > jump into U-Boot, add a trampoline just past the Arria10 boot
> > > > > header
> > > > > and
> > > > > point to this trampoline at fixed offset from the header
> > > > > generated
> > > > > using
> > > > > the mkimage -T socfpgaimage_v1 . Note that it is not needed
> > > > > to
> > > > > jump
> > > > > back
> > > > > to offset 0x0 of the image, it is possible to jump directly
> > > > > at
> > > > > the
> > > > > reset
> > > > > label and save processing two instructions.
> > > > > 
> > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > > > > Cc: Chin Liang See <chin.liang.see@intel.com>
> > > > > ---
> > > > >  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > index d6b9435d33..06bbe27d2c 100644
> > > > > --- a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > @@ -18,10 +18,10 @@ _start:
> > > > >  	.word	0xcafec0d3;	/* Checksum, zero-
> > > > > pad */
> > > > >  	nop;
> > > > >  
> > > > > -	b reset;		/* SoCFPGA jumps here */
> > > > > -	nop;
> > > > > +	b reset;		/* SoCFPGA Gen5 jumps here
> > > > > */
> > > > >  	nop;
> > > > >  	nop;
> > > > > +	b reset;		/* SoCFPGA Gen10 trampoline
> > > > > */
> > > > Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder
> > > > can
> > > > we
> > > > standardize that by using 0x14 instead of proposed 0x18 in this
> > > > patch?
> > > What difference does it make, the entire image is generated
> > > during
> > > the
> > > build anyway ? This patch uses offset 0x1c, but what is the
> > > reason
> > > for
> > > address 0x14 in your proprietary tool, is there one ?
> > Our A10 header ended at 0x13 today. Hence we are continuing the
> > code at
> > 0x14 without any spacing.
> > 
> > While for 0x1c, should it be 3 nop?
> Yes, gives enough space were the header grow for whatever reason.
> Mind
> you, the NOPs are not executed, the socfpga jumps to 0x1c directly
> via
> 0x0c -- Image entry offset

Ok, I don't have strong objection on this. We can claim that we don't
support use case where we use mkpimage tools from SCOEDS to sign SPL
binary from mainstream.

Acked-By: Chin Liang See <chin.liang.see@intel.com>


Thanks
Chin Liang
Marek Vasut April 17, 2018, 9:28 a.m. | #6
On 04/17/2018 11:11 AM, See, Chin Liang wrote:
> On Tue, 2018-04-17 at 11:01 +0200, Marek Vasut wrote:
>> On 04/17/2018 10:52 AM, See, Chin Liang wrote:
>>>
>>> On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
>>>>
>>>> On 04/17/2018 10:40 AM, See, Chin Liang wrote:
>>>>>
>>>>>
>>>>> Hi Marek,
>>>>>
>>>>> On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> The Arria10 uses slightly different boot image header than
>>>>>> the
>>>>>> Gen5
>>>>>> SoCs,
>>>>>> in particular the header itself contains an offset from the
>>>>>> start
>>>>>> of
>>>>>> the
>>>>>> header to which the Arria10 jumps. This offset must not be
>>>>>> negative,
>>>>>> yet
>>>>>> the header is placed at offset 0x40 of the bootable binary.
>>>>>> Therefore, to
>>>>>> jump into U-Boot, add a trampoline just past the Arria10 boot
>>>>>> header
>>>>>> and
>>>>>> point to this trampoline at fixed offset from the header
>>>>>> generated
>>>>>> using
>>>>>> the mkimage -T socfpgaimage_v1 . Note that it is not needed
>>>>>> to
>>>>>> jump
>>>>>> back
>>>>>> to offset 0x0 of the image, it is possible to jump directly
>>>>>> at
>>>>>> the
>>>>>> reset
>>>>>> label and save processing two instructions.
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>>>> ---
>>>>>>  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>> b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>> index d6b9435d33..06bbe27d2c 100644
>>>>>> --- a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>> +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>> @@ -18,10 +18,10 @@ _start:
>>>>>>  	.word	0xcafec0d3;	/* Checksum, zero-
>>>>>> pad */
>>>>>>  	nop;
>>>>>>  
>>>>>> -	b reset;		/* SoCFPGA jumps here */
>>>>>> -	nop;
>>>>>> +	b reset;		/* SoCFPGA Gen5 jumps here
>>>>>> */
>>>>>>  	nop;
>>>>>>  	nop;
>>>>>> +	b reset;		/* SoCFPGA Gen10 trampoline
>>>>>> */
>>>>> Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder
>>>>> can
>>>>> we
>>>>> standardize that by using 0x14 instead of proposed 0x18 in this
>>>>> patch?
>>>> What difference does it make, the entire image is generated
>>>> during
>>>> the
>>>> build anyway ? This patch uses offset 0x1c, but what is the
>>>> reason
>>>> for
>>>> address 0x14 in your proprietary tool, is there one ?
>>> Our A10 header ended at 0x13 today. Hence we are continuing the
>>> code at
>>> 0x14 without any spacing.
>>>
>>> While for 0x1c, should it be 3 nop?
>> Yes, gives enough space were the header grow for whatever reason.
>> Mind
>> you, the NOPs are not executed, the socfpga jumps to 0x1c directly
>> via
>> 0x0c -- Image entry offset
> 
> Ok, I don't have strong objection on this. We can claim that we don't
> support use case where we use mkpimage tools from SCOEDS to sign SPL
> binary from mainstream.

Which you can, why wouldn't it work ?

What is the benefit of using mkpimage if mkimage does the same thing
though ?

And what do you mean by "signing" ?
See, Chin Liang April 19, 2018, 5:51 a.m. | #7
On Tue, 2018-04-17 at 11:28 +0200, Marek Vasut wrote:
> On 04/17/2018 11:11 AM, See, Chin Liang wrote:
> > 
> > On Tue, 2018-04-17 at 11:01 +0200, Marek Vasut wrote:
> > > 
> > > On 04/17/2018 10:52 AM, See, Chin Liang wrote:
> > > > 
> > > > 
> > > > On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 04/17/2018 10:40 AM, See, Chin Liang wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > Hi Marek,
> > > > > > 
> > > > > > On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > The Arria10 uses slightly different boot image header
> > > > > > > than
> > > > > > > the
> > > > > > > Gen5
> > > > > > > SoCs,
> > > > > > > in particular the header itself contains an offset from
> > > > > > > the
> > > > > > > start
> > > > > > > of
> > > > > > > the
> > > > > > > header to which the Arria10 jumps. This offset must not
> > > > > > > be
> > > > > > > negative,
> > > > > > > yet
> > > > > > > the header is placed at offset 0x40 of the bootable
> > > > > > > binary.
> > > > > > > Therefore, to
> > > > > > > jump into U-Boot, add a trampoline just past the Arria10
> > > > > > > boot
> > > > > > > header
> > > > > > > and
> > > > > > > point to this trampoline at fixed offset from the header
> > > > > > > generated
> > > > > > > using
> > > > > > > the mkimage -T socfpgaimage_v1 . Note that it is not
> > > > > > > needed
> > > > > > > to
> > > > > > > jump
> > > > > > > back
> > > > > > > to offset 0x0 of the image, it is possible to jump
> > > > > > > directly
> > > > > > > at
> > > > > > > the
> > > > > > > reset
> > > > > > > label and save processing two instructions.
> > > > > > > 
> > > > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > > > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > > > > > > Cc: Chin Liang See <chin.liang.see@intel.com>
> > > > > > > ---
> > > > > > >  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
> > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > > > b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > > > index d6b9435d33..06bbe27d2c 100644
> > > > > > > --- a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > > > +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > > > @@ -18,10 +18,10 @@ _start:
> > > > > > >  	.word	0xcafec0d3;	/* Checksum,
> > > > > > > zero-
> > > > > > > pad */
> > > > > > >  	nop;
> > > > > > >  
> > > > > > > -	b reset;		/* SoCFPGA jumps here */
> > > > > > > -	nop;
> > > > > > > +	b reset;		/* SoCFPGA Gen5 jumps
> > > > > > > here
> > > > > > > */
> > > > > > >  	nop;
> > > > > > >  	nop;
> > > > > > > +	b reset;		/* SoCFPGA Gen10
> > > > > > > trampoline
> > > > > > > */
> > > > > > Our mkpimage tools from SOCEDS is using 0x14 as offset.
> > > > > > Wonder
> > > > > > can
> > > > > > we
> > > > > > standardize that by using 0x14 instead of proposed 0x18 in
> > > > > > this
> > > > > > patch?
> > > > > What difference does it make, the entire image is generated
> > > > > during
> > > > > the
> > > > > build anyway ? This patch uses offset 0x1c, but what is the
> > > > > reason
> > > > > for
> > > > > address 0x14 in your proprietary tool, is there one ?
> > > > Our A10 header ended at 0x13 today. Hence we are continuing the
> > > > code at
> > > > 0x14 without any spacing.
> > > > 
> > > > While for 0x1c, should it be 3 nop?
> > > Yes, gives enough space were the header grow for whatever reason.
> > > Mind
> > > you, the NOPs are not executed, the socfpga jumps to 0x1c
> > > directly
> > > via
> > > 0x0c -- Image entry offset
> > Ok, I don't have strong objection on this. We can claim that we
> > don't
> > support use case where we use mkpimage tools from SCOEDS to sign
> > SPL
> > binary from mainstream.
> Which you can, why wouldn't it work ?
> 

Rethinking on this, yes as the nop shall able to handle this. Just the
vice versa won't work since downstream U-Boot already have the entry
fixed at 0x14.


> What is the benefit of using mkpimage if mkimage does the same thing
> though ?

Initial thought is for having both tools compatible. But rethinking the
makefile will handle this, we just advise user directly use sfp file
directly.

Thanks
Chin Liang
Marek Vasut April 19, 2018, 9:06 a.m. | #8
On 04/19/2018 07:51 AM, See, Chin Liang wrote:
> On Tue, 2018-04-17 at 11:28 +0200, Marek Vasut wrote:
>> On 04/17/2018 11:11 AM, See, Chin Liang wrote:
>>>
>>> On Tue, 2018-04-17 at 11:01 +0200, Marek Vasut wrote:
>>>>
>>>> On 04/17/2018 10:52 AM, See, Chin Liang wrote:
>>>>>
>>>>>
>>>>> On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 04/17/2018 10:40 AM, See, Chin Liang wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hi Marek,
>>>>>>>
>>>>>>> On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The Arria10 uses slightly different boot image header
>>>>>>>> than
>>>>>>>> the
>>>>>>>> Gen5
>>>>>>>> SoCs,
>>>>>>>> in particular the header itself contains an offset from
>>>>>>>> the
>>>>>>>> start
>>>>>>>> of
>>>>>>>> the
>>>>>>>> header to which the Arria10 jumps. This offset must not
>>>>>>>> be
>>>>>>>> negative,
>>>>>>>> yet
>>>>>>>> the header is placed at offset 0x40 of the bootable
>>>>>>>> binary.
>>>>>>>> Therefore, to
>>>>>>>> jump into U-Boot, add a trampoline just past the Arria10
>>>>>>>> boot
>>>>>>>> header
>>>>>>>> and
>>>>>>>> point to this trampoline at fixed offset from the header
>>>>>>>> generated
>>>>>>>> using
>>>>>>>> the mkimage -T socfpgaimage_v1 . Note that it is not
>>>>>>>> needed
>>>>>>>> to
>>>>>>>> jump
>>>>>>>> back
>>>>>>>> to offset 0x0 of the image, it is possible to jump
>>>>>>>> directly
>>>>>>>> at
>>>>>>>> the
>>>>>>>> reset
>>>>>>>> label and save processing two instructions.
>>>>>>>>
>>>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>>>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>>>>>> ---
>>>>>>>>  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
>>>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>>>> b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>>>> index d6b9435d33..06bbe27d2c 100644
>>>>>>>> --- a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>>>> +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>>>> @@ -18,10 +18,10 @@ _start:
>>>>>>>>  	.word	0xcafec0d3;	/* Checksum,
>>>>>>>> zero-
>>>>>>>> pad */
>>>>>>>>  	nop;
>>>>>>>>  
>>>>>>>> -	b reset;		/* SoCFPGA jumps here */
>>>>>>>> -	nop;
>>>>>>>> +	b reset;		/* SoCFPGA Gen5 jumps
>>>>>>>> here
>>>>>>>> */
>>>>>>>>  	nop;
>>>>>>>>  	nop;
>>>>>>>> +	b reset;		/* SoCFPGA Gen10
>>>>>>>> trampoline
>>>>>>>> */
>>>>>>> Our mkpimage tools from SOCEDS is using 0x14 as offset.
>>>>>>> Wonder
>>>>>>> can
>>>>>>> we
>>>>>>> standardize that by using 0x14 instead of proposed 0x18 in
>>>>>>> this
>>>>>>> patch?
>>>>>> What difference does it make, the entire image is generated
>>>>>> during
>>>>>> the
>>>>>> build anyway ? This patch uses offset 0x1c, but what is the
>>>>>> reason
>>>>>> for
>>>>>> address 0x14 in your proprietary tool, is there one ?
>>>>> Our A10 header ended at 0x13 today. Hence we are continuing the
>>>>> code at
>>>>> 0x14 without any spacing.
>>>>>
>>>>> While for 0x1c, should it be 3 nop?
>>>> Yes, gives enough space were the header grow for whatever reason.
>>>> Mind
>>>> you, the NOPs are not executed, the socfpga jumps to 0x1c
>>>> directly
>>>> via
>>>> 0x0c -- Image entry offset
>>> Ok, I don't have strong objection on this. We can claim that we
>>> don't
>>> support use case where we use mkpimage tools from SCOEDS to sign
>>> SPL
>>> binary from mainstream.
>> Which you can, why wouldn't it work ?
>>
> 
> Rethinking on this, yes as the nop shall able to handle this. Just the
> vice versa won't work since downstream U-Boot already have the entry
> fixed at 0x14.
> 
> 
>> What is the benefit of using mkpimage if mkimage does the same thing
>> though ?
> 
> Initial thought is for having both tools compatible. But rethinking the
> makefile will handle this, we just advise user directly use sfp file
> directly.

I mean, I can place the trampoline to 0x14 , it doesn't really matter.
But is there some hidden gem in the SoCFPGA bootrom which might bite us
later ?

Patch

diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h b/arch/arm/mach-socfpga/include/mach/boot0.h
index d6b9435d33..06bbe27d2c 100644
--- a/arch/arm/mach-socfpga/include/mach/boot0.h
+++ b/arch/arm/mach-socfpga/include/mach/boot0.h
@@ -18,10 +18,10 @@  _start:
 	.word	0xcafec0d3;	/* Checksum, zero-pad */
 	nop;
 
-	b reset;		/* SoCFPGA jumps here */
-	nop;
+	b reset;		/* SoCFPGA Gen5 jumps here */
 	nop;
 	nop;
+	b reset;		/* SoCFPGA Gen10 trampoline */
 #endif
 
 #endif /* __BOOT0_H */