tools: zynqmpbif: Add support for load=after

Message ID 20180426113739.45157-1-agraf@suse.de
State New
Headers show
Series
  • tools: zynqmpbif: Add support for load=after
Related show

Commit Message

Alexander Graf April 26, 2018, 11:37 a.m.
Some times it's handy to have a partition loaded immediately after
the end of the previous blob. The most obvious example for this is
a U-Boot binary (coming from .elf) and a device tree file.

This patch adds that logic. With this, the following bif snippet
does what you would expect:

  [destination_cpu=a5x-0, exception_level=el-2] u-boot.elf
  [load=after] u-boot.dtb

converts to

  FSBL payload on CPU a5x-0 (PS):
    Offset     : 0x00590500
    Size       : 577768 (0x8d0e8) bytes
    Load       : 0x08000000
    Attributes : EL2
    Checksum   : 0xefca2cad
  FSBL payload on CPU none (PS):
    Offset     : 0x0061d640
    Size       : 129760 (0x1fae0) bytes
    Load       : 0x0808d0e8 (entry=0x00000000)
    Attributes : EL3
    Checksum   : 0xf7dd3d49

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 tools/zynqmpbif.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Michal Simek April 26, 2018, 4:33 p.m. | #1
On 26.4.2018 13:37, Alexander Graf wrote:
> Some times it's handy to have a partition loaded immediately after
> the end of the previous blob. The most obvious example for this is
> a U-Boot binary (coming from .elf) and a device tree file.
> 
> This patch adds that logic. With this, the following bif snippet
> does what you would expect:
> 
>   [destination_cpu=a5x-0, exception_level=el-2] u-boot.elf
>   [load=after] u-boot.dtb
> 
> converts to
> 
>   FSBL payload on CPU a5x-0 (PS):
>     Offset     : 0x00590500
>     Size       : 577768 (0x8d0e8) bytes
>     Load       : 0x08000000
>     Attributes : EL2
>     Checksum   : 0xefca2cad
>   FSBL payload on CPU none (PS):
>     Offset     : 0x0061d640
>     Size       : 129760 (0x1fae0) bytes
>     Load       : 0x0808d0e8 (entry=0x00000000)
>     Attributes : EL3
>     Checksum   : 0xf7dd3d49
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  tools/zynqmpbif.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/tools/zynqmpbif.c b/tools/zynqmpbif.c
> index 6c8f66055d..47c233c15f 100644
> --- a/tools/zynqmpbif.c
> +++ b/tools/zynqmpbif.c
> @@ -42,6 +42,7 @@ enum bif_flag {
>  	BIF_FLAG_PUF_FILE,
>  	BIF_FLAG_AARCH32,
>  	BIF_FLAG_PART_OWNER_UBOOT,
> +	BIF_FLAG_LOAD_AFTER,
>  
>  	/* Internal flags */
>  	BIF_FLAG_BIT_FILE,
> @@ -151,6 +152,11 @@ static char *parse_load(char *line, struct bif_entry *bf)
>  {
>  	char *endptr;
>  
> +	if (!strncmp(line, "after", strlen("after"))) {
> +		bf->flags |= (1ULL << BIF_FLAG_LOAD_AFTER);
> +		return line + strlen("after");
> +	}
> +
>  	bf->load = strtoll(line, &endptr, 0);
>  
>  	return endptr;
> @@ -336,6 +342,15 @@ static int bif_add_part(struct bif_entry *bf, const char *data, size_t len)
>  	if (r)
>  		return r;
>  
> +	if (bf->flags & (1ULL << BIF_FLAG_LOAD_AFTER) &&
> +	    bif_output.last_part) {
> +		struct partition_header *p = bif_output.last_part;
> +		uint64_t load = le64_to_cpu(p->load_address);
> +
> +		load += le32_to_cpu(p->len) * 4;
> +		parthdr.load_address = cpu_to_le64(load);
> +	}
> +
>  	parthdr.offset = cpu_to_le32(bf->offset / 4);
>  
>  	if (bf->flags & (1ULL << BIF_FLAG_BOOTLOADER)) {
> 

As we have discussed over IRC this is out of current bif supported
options and load option with OF_SEPARATE/OF_BOARD should be used instead.
Some platforms are using CONFIG_SYS_FDT_BASE for that.
Generic solution for that would be good because right now is fdt_blob is
at &_end.

At least like this.

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 320ee1dc56b3..4262ac0677d3 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1292,8 +1292,12 @@ __weak void *board_fdt_blob_setup(void)
        else
                fdt_blob = (ulong *)&__bss_end;
 #else
+#ifndef CONFIG_SYS_FDT_BASE
        /* FDT is at end of image */
        fdt_blob = (ulong *)&_end;
+#else
+       fdt_blob = CONFIG_SYS_FDT_BASE;
+#endif
 #endif
        return fdt_blob;
 }

Thanks,
Michal
Alexander Graf April 26, 2018, 4:46 p.m. | #2
On 04/26/2018 06:33 PM, Michal Simek wrote:
> On 26.4.2018 13:37, Alexander Graf wrote:
>> Some times it's handy to have a partition loaded immediately after
>> the end of the previous blob. The most obvious example for this is
>> a U-Boot binary (coming from .elf) and a device tree file.
>>
>> This patch adds that logic. With this, the following bif snippet
>> does what you would expect:
>>
>>    [destination_cpu=a5x-0, exception_level=el-2] u-boot.elf
>>    [load=after] u-boot.dtb
>>
>> converts to
>>
>>    FSBL payload on CPU a5x-0 (PS):
>>      Offset     : 0x00590500
>>      Size       : 577768 (0x8d0e8) bytes
>>      Load       : 0x08000000
>>      Attributes : EL2
>>      Checksum   : 0xefca2cad
>>    FSBL payload on CPU none (PS):
>>      Offset     : 0x0061d640
>>      Size       : 129760 (0x1fae0) bytes
>>      Load       : 0x0808d0e8 (entry=0x00000000)
>>      Attributes : EL3
>>      Checksum   : 0xf7dd3d49
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>   tools/zynqmpbif.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/tools/zynqmpbif.c b/tools/zynqmpbif.c
>> index 6c8f66055d..47c233c15f 100644
>> --- a/tools/zynqmpbif.c
>> +++ b/tools/zynqmpbif.c
>> @@ -42,6 +42,7 @@ enum bif_flag {
>>   	BIF_FLAG_PUF_FILE,
>>   	BIF_FLAG_AARCH32,
>>   	BIF_FLAG_PART_OWNER_UBOOT,
>> +	BIF_FLAG_LOAD_AFTER,
>>   
>>   	/* Internal flags */
>>   	BIF_FLAG_BIT_FILE,
>> @@ -151,6 +152,11 @@ static char *parse_load(char *line, struct bif_entry *bf)
>>   {
>>   	char *endptr;
>>   
>> +	if (!strncmp(line, "after", strlen("after"))) {
>> +		bf->flags |= (1ULL << BIF_FLAG_LOAD_AFTER);
>> +		return line + strlen("after");
>> +	}
>> +
>>   	bf->load = strtoll(line, &endptr, 0);
>>   
>>   	return endptr;
>> @@ -336,6 +342,15 @@ static int bif_add_part(struct bif_entry *bf, const char *data, size_t len)
>>   	if (r)
>>   		return r;
>>   
>> +	if (bf->flags & (1ULL << BIF_FLAG_LOAD_AFTER) &&
>> +	    bif_output.last_part) {
>> +		struct partition_header *p = bif_output.last_part;
>> +		uint64_t load = le64_to_cpu(p->load_address);
>> +
>> +		load += le32_to_cpu(p->len) * 4;
>> +		parthdr.load_address = cpu_to_le64(load);
>> +	}
>> +
>>   	parthdr.offset = cpu_to_le32(bf->offset / 4);
>>   
>>   	if (bf->flags & (1ULL << BIF_FLAG_BOOTLOADER)) {
>>
> As we have discussed over IRC this is out of current bif supported
> options and load option with OF_SEPARATE/OF_BOARD should be used instead.
> Some platforms are using CONFIG_SYS_FDT_BASE for that.
> Generic solution for that would be good because right now is fdt_blob is
> at &_end.

Yes, the &_end is what this patch enables to map in bif.

> At least like this.
>
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 320ee1dc56b3..4262ac0677d3 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -1292,8 +1292,12 @@ __weak void *board_fdt_blob_setup(void)
>          else
>                  fdt_blob = (ulong *)&__bss_end;
>   #else
> +#ifndef CONFIG_SYS_FDT_BASE
>          /* FDT is at end of image */
>          fdt_blob = (ulong *)&_end;
> +#else
> +       fdt_blob = CONFIG_SYS_FDT_BASE;
> +#endif

I agree that this is an alternative approach to this, but I like it much 
less :). It just feels wrong to hard code random addresses to load 
things from when everything is easy enough to pull out dynamically.

I'd much rather prefer to extend the official bif syntax with this. If 
we can't have that, I guess the next "best" option is to just create a 
combined .bin file and manually set the load/entry address from the elf 
header in my boot.bin generation script.


Alex
Michal Simek April 27, 2018, 5:53 a.m. | #3
On 26.4.2018 18:46, Alexander Graf wrote:
> On 04/26/2018 06:33 PM, Michal Simek wrote:
>> On 26.4.2018 13:37, Alexander Graf wrote:
>>> Some times it's handy to have a partition loaded immediately after
>>> the end of the previous blob. The most obvious example for this is
>>> a U-Boot binary (coming from .elf) and a device tree file.
>>>
>>> This patch adds that logic. With this, the following bif snippet
>>> does what you would expect:
>>>
>>>    [destination_cpu=a5x-0, exception_level=el-2] u-boot.elf
>>>    [load=after] u-boot.dtb
>>>
>>> converts to
>>>
>>>    FSBL payload on CPU a5x-0 (PS):
>>>      Offset     : 0x00590500
>>>      Size       : 577768 (0x8d0e8) bytes
>>>      Load       : 0x08000000
>>>      Attributes : EL2
>>>      Checksum   : 0xefca2cad
>>>    FSBL payload on CPU none (PS):
>>>      Offset     : 0x0061d640
>>>      Size       : 129760 (0x1fae0) bytes
>>>      Load       : 0x0808d0e8 (entry=0x00000000)
>>>      Attributes : EL3
>>>      Checksum   : 0xf7dd3d49
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>   tools/zynqmpbif.c | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/tools/zynqmpbif.c b/tools/zynqmpbif.c
>>> index 6c8f66055d..47c233c15f 100644
>>> --- a/tools/zynqmpbif.c
>>> +++ b/tools/zynqmpbif.c
>>> @@ -42,6 +42,7 @@ enum bif_flag {
>>>       BIF_FLAG_PUF_FILE,
>>>       BIF_FLAG_AARCH32,
>>>       BIF_FLAG_PART_OWNER_UBOOT,
>>> +    BIF_FLAG_LOAD_AFTER,
>>>         /* Internal flags */
>>>       BIF_FLAG_BIT_FILE,
>>> @@ -151,6 +152,11 @@ static char *parse_load(char *line, struct
>>> bif_entry *bf)
>>>   {
>>>       char *endptr;
>>>   +    if (!strncmp(line, "after", strlen("after"))) {
>>> +        bf->flags |= (1ULL << BIF_FLAG_LOAD_AFTER);
>>> +        return line + strlen("after");
>>> +    }
>>> +
>>>       bf->load = strtoll(line, &endptr, 0);
>>>         return endptr;
>>> @@ -336,6 +342,15 @@ static int bif_add_part(struct bif_entry *bf,
>>> const char *data, size_t len)
>>>       if (r)
>>>           return r;
>>>   +    if (bf->flags & (1ULL << BIF_FLAG_LOAD_AFTER) &&
>>> +        bif_output.last_part) {
>>> +        struct partition_header *p = bif_output.last_part;
>>> +        uint64_t load = le64_to_cpu(p->load_address);
>>> +
>>> +        load += le32_to_cpu(p->len) * 4;
>>> +        parthdr.load_address = cpu_to_le64(load);
>>> +    }
>>> +
>>>       parthdr.offset = cpu_to_le32(bf->offset / 4);
>>>         if (bf->flags & (1ULL << BIF_FLAG_BOOTLOADER)) {
>>>
>> As we have discussed over IRC this is out of current bif supported
>> options and load option with OF_SEPARATE/OF_BOARD should be used instead.
>> Some platforms are using CONFIG_SYS_FDT_BASE for that.
>> Generic solution for that would be good because right now is fdt_blob is
>> at &_end.
> 
> Yes, the &_end is what this patch enables to map in bif.
> 
>> At least like this.
>>
>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> index 320ee1dc56b3..4262ac0677d3 100644
>> --- a/lib/fdtdec.c
>> +++ b/lib/fdtdec.c
>> @@ -1292,8 +1292,12 @@ __weak void *board_fdt_blob_setup(void)
>>          else
>>                  fdt_blob = (ulong *)&__bss_end;
>>   #else
>> +#ifndef CONFIG_SYS_FDT_BASE
>>          /* FDT is at end of image */
>>          fdt_blob = (ulong *)&_end;
>> +#else
>> +       fdt_blob = CONFIG_SYS_FDT_BASE;
>> +#endif
> 
> I agree that this is an alternative approach to this, but I like it much
> less :). It just feels wrong to hard code random addresses to load
> things from when everything is easy enough to pull out dynamically.
> 
> I'd much rather prefer to extend the official bif syntax with this. If
> we can't have that, I guess the next "best" option is to just create a
> combined .bin file and manually set the load/entry address from the elf
> header in my boot.bin generation script.

I don't think this is going to happen for zynqmp and will see what can
be done for next gen. But my guess is that chance is quite low.

That's how it is done for OF_SEPARATE today
  cat u-boot-nodtb.bin dts/dt.dtb > u-boot-dtb.bin
  cp u-boot-dtb.bin u-boot.bin

It means the same will be in your script.

Thanks,
Michal



> 
> 
> Alex
>

Patch

diff --git a/tools/zynqmpbif.c b/tools/zynqmpbif.c
index 6c8f66055d..47c233c15f 100644
--- a/tools/zynqmpbif.c
+++ b/tools/zynqmpbif.c
@@ -42,6 +42,7 @@  enum bif_flag {
 	BIF_FLAG_PUF_FILE,
 	BIF_FLAG_AARCH32,
 	BIF_FLAG_PART_OWNER_UBOOT,
+	BIF_FLAG_LOAD_AFTER,
 
 	/* Internal flags */
 	BIF_FLAG_BIT_FILE,
@@ -151,6 +152,11 @@  static char *parse_load(char *line, struct bif_entry *bf)
 {
 	char *endptr;
 
+	if (!strncmp(line, "after", strlen("after"))) {
+		bf->flags |= (1ULL << BIF_FLAG_LOAD_AFTER);
+		return line + strlen("after");
+	}
+
 	bf->load = strtoll(line, &endptr, 0);
 
 	return endptr;
@@ -336,6 +342,15 @@  static int bif_add_part(struct bif_entry *bf, const char *data, size_t len)
 	if (r)
 		return r;
 
+	if (bf->flags & (1ULL << BIF_FLAG_LOAD_AFTER) &&
+	    bif_output.last_part) {
+		struct partition_header *p = bif_output.last_part;
+		uint64_t load = le64_to_cpu(p->load_address);
+
+		load += le32_to_cpu(p->len) * 4;
+		parthdr.load_address = cpu_to_le64(load);
+	}
+
 	parthdr.offset = cpu_to_le32(bf->offset / 4);
 
 	if (bf->flags & (1ULL << BIF_FLAG_BOOTLOADER)) {