[U-Boot] [PATCH] tools: xilinx: Fix zynq/zynqmp image recognition

[U-Boot] [PATCH] tools: xilinx: Fix zynq/zynqmp image recognition

[Michal Simek]

There is an issue to recognize zynq or zynqmp image because header
checking is just the same. That's why zynqmp images are recognized as
zynq one.
Check unused fields which are initialized to zero in zynq format
(__reserved1 0x38 and __reserved2 0x44) which are initialized for
zynqmp. This should ensure that images are properly recognized by:
./tools/mkimage -l spl/boot.bin

Also show image type as ZynqMP instead of Zynq which is confusing
and parse a53 64bit cpu select option in image_attributes field to make
sure that this is ZynqMP image.

Reported-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 tools/zynqimage.c   | 6 ++++++
 tools/zynqmpimage.c | 8 ++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/zynqimage.c b/tools/zynqimage.c
index 021d2d3fc91f..aa003a7543b6 100644
--- a/tools/zynqimage.c
+++ b/tools/zynqimage.c
@@ -147,6 +147,12 @@ static int zynqimage_verify_header(unsigned char *ptr, int image_size,
 	if (image_size < sizeof(struct zynq_header))
 		return -1;
 
+	if (zynqhdr->__reserved1 != 0)
+		return -1;
+
+	if (zynqhdr->__reserved2 != 0)
+		return -1;
+
 	if (zynqhdr->width_detection != HEADER_WIDTHDETECTION)
 		return -1;
 	if (zynqhdr->image_identifier != HEADER_IMAGEIDENTIFIER)
diff --git a/tools/zynqmpimage.c b/tools/zynqmpimage.c
index f48ac6dbe505..9cef78dc7cf6 100644
--- a/tools/zynqmpimage.c
+++ b/tools/zynqmpimage.c
@@ -63,6 +63,7 @@
 #define HEADER_REGINIT_NULL (cpu_to_le32(0xffffffff))
 #define HEADER_WIDTHDETECTION (cpu_to_le32(0xaa995566))
 #define HEADER_IMAGEIDENTIFIER (cpu_to_le32(0x584c4e58))
+#define HEADER_CPU_SELECT_A53_64BIT	(0x2 << 10)
 
 enum {
 	ENCRYPTION_EFUSE = 0xa5c3c5a3,
@@ -132,7 +133,7 @@ static void zynqmpimage_default_header(struct zynqmp_header *ptr)
 		return;
 
 	ptr->width_detection = HEADER_WIDTHDETECTION;
-	ptr->image_attributes = 0x800;
+	ptr->image_attributes = HEADER_CPU_SELECT_A53_64BIT;
 	ptr->image_identifier = HEADER_IMAGEIDENTIFIER;
 	ptr->encryption = cpu_to_le32(ENCRYPTION_NONE);
 
@@ -162,6 +163,9 @@ static int zynqmpimage_verify_header(unsigned char *ptr, int image_size,
 	if (image_size < sizeof(struct zynqmp_header))
 		return -1;
 
+	if (zynqhdr->image_attributes != HEADER_CPU_SELECT_A53_64BIT)
+		return -1;
+
 	if (zynqhdr->width_detection != HEADER_WIDTHDETECTION)
 		return -1;
 	if (zynqhdr->image_identifier != HEADER_IMAGEIDENTIFIER)
@@ -178,7 +182,7 @@ static void zynqmpimage_print_header(const void *ptr)
 	struct zynqmp_header *zynqhdr = (struct zynqmp_header *)ptr;
 	int i;
 
-	printf("Image Type   : Xilinx Zynq Boot Image support\n");
+	printf("Image Type   : Xilinx ZynqMP Boot Image support\n");
 	printf("Image Offset : 0x%08x\n", le32_to_cpu(zynqhdr->image_offset));
 	printf("Image Size   : %lu bytes (%lu bytes packed)\n",
 	       (unsigned long)le32_to_cpu(zynqhdr->image_size),
-- 
1.9.1

[U-Boot] [PATCH] tools: xilinx: Fix zynq/zynqmp image recognition

[Alexander Graf]

On 14.03.18 11:14, Michal Simek wrote:
> There is an issue to recognize zynq or zynqmp image because header
> checking is just the same. That's why zynqmp images are recognized as
> zynq one.
> Check unused fields which are initialized to zero in zynq format
> (__reserved1 0x38 and __reserved2 0x44) which are initialized for
> zynqmp. This should ensure that images are properly recognized by:
> ./tools/mkimage -l spl/boot.bin
> 
> Also show image type as ZynqMP instead of Zynq which is confusing
> and parse a53 64bit cpu select option in image_attributes field to make
> sure that this is ZynqMP image.

What if the target is on R5?

> Reported-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Tested-by: Alexander Graf <agraf@suse.de>


Alex

> ---
> 
>  tools/zynqimage.c   | 6 ++++++
>  tools/zynqmpimage.c | 8 ++++++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/zynqimage.c b/tools/zynqimage.c
> index 021d2d3fc91f..aa003a7543b6 100644
> --- a/tools/zynqimage.c
> +++ b/tools/zynqimage.c
> @@ -147,6 +147,12 @@ static int zynqimage_verify_header(unsigned char *ptr, int image_size,
>  	if (image_size < sizeof(struct zynq_header))
>  		return -1;
>  
> +	if (zynqhdr->__reserved1 != 0)
> +		return -1;
> +
> +	if (zynqhdr->__reserved2 != 0)
> +		return -1;
> +
>  	if (zynqhdr->width_detection != HEADER_WIDTHDETECTION)
>  		return -1;
>  	if (zynqhdr->image_identifier != HEADER_IMAGEIDENTIFIER)
> diff --git a/tools/zynqmpimage.c b/tools/zynqmpimage.c
> index f48ac6dbe505..9cef78dc7cf6 100644
> --- a/tools/zynqmpimage.c
> +++ b/tools/zynqmpimage.c
> @@ -63,6 +63,7 @@
>  #define HEADER_REGINIT_NULL (cpu_to_le32(0xffffffff))
>  #define HEADER_WIDTHDETECTION (cpu_to_le32(0xaa995566))
>  #define HEADER_IMAGEIDENTIFIER (cpu_to_le32(0x584c4e58))
> +#define HEADER_CPU_SELECT_A53_64BIT	(0x2 << 10)
>  
>  enum {
>  	ENCRYPTION_EFUSE = 0xa5c3c5a3,
> @@ -132,7 +133,7 @@ static void zynqmpimage_default_header(struct zynqmp_header *ptr)
>  		return;
>  
>  	ptr->width_detection = HEADER_WIDTHDETECTION;
> -	ptr->image_attributes = 0x800;
> +	ptr->image_attributes = HEADER_CPU_SELECT_A53_64BIT;
>  	ptr->image_identifier = HEADER_IMAGEIDENTIFIER;
>  	ptr->encryption = cpu_to_le32(ENCRYPTION_NONE);
>  
> @@ -162,6 +163,9 @@ static int zynqmpimage_verify_header(unsigned char *ptr, int image_size,
>  	if (image_size < sizeof(struct zynqmp_header))
>  		return -1;
>  
> +	if (zynqhdr->image_attributes != HEADER_CPU_SELECT_A53_64BIT)
> +		return -1;
> +
>  	if (zynqhdr->width_detection != HEADER_WIDTHDETECTION)
>  		return -1;
>  	if (zynqhdr->image_identifier != HEADER_IMAGEIDENTIFIER)
> @@ -178,7 +182,7 @@ static void zynqmpimage_print_header(const void *ptr)
>  	struct zynqmp_header *zynqhdr = (struct zynqmp_header *)ptr;
>  	int i;
>  
> -	printf("Image Type   : Xilinx Zynq Boot Image support\n");
> +	printf("Image Type   : Xilinx ZynqMP Boot Image support\n");
>  	printf("Image Offset : 0x%08x\n", le32_to_cpu(zynqhdr->image_offset));
>  	printf("Image Size   : %lu bytes (%lu bytes packed)\n",
>  	       (unsigned long)le32_to_cpu(zynqhdr->image_size),
> 

[U-Boot] [PATCH] tools: xilinx: Fix zynq/zynqmp image recognition

[Michal Simek]

On 15.3.2018 22:54, Alexander Graf wrote:
> 
> 
> On 14.03.18 11:14, Michal Simek wrote:
>> There is an issue to recognize zynq or zynqmp image because header
>> checking is just the same. That's why zynqmp images are recognized as
>> zynq one.
>> Check unused fields which are initialized to zero in zynq format
>> (__reserved1 0x38 and __reserved2 0x44) which are initialized for
>> zynqmp. This should ensure that images are properly recognized by:
>> ./tools/mkimage -l spl/boot.bin
>>
>> Also show image type as ZynqMP instead of Zynq which is confusing
>> and parse a53 64bit cpu select option in image_attributes field to make
>> sure that this is ZynqMP image.
> 
> What if the target is on R5?

I expect you know the answer. I have ported u-boot on R5 which could use
this feature but mkimage is not prepared for that.

I am happy to remove this zynqmp detection code because it should be
enough what it is done for zynq. Just let me know if you want to remove it.

> 
>> Reported-by: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> 
> Tested-by: Alexander Graf <agraf@suse.de>

It is question how exactly you want to use mkimage tools. If for u-boot
generated images or for generic boot.bin generated by bootgen.

Thanks,
Michal

[U-Boot] [PATCH] tools: xilinx: Fix zynq/zynqmp image recognition

[Alexander Graf]

> Am 16.03.2018 um 11:22 schrieb Michal Simek <michal.simek@xilinx.com>:
> 
>> On 15.3.2018 22:54, Alexander Graf wrote:
>> 
>> 
>>> On 14.03.18 11:14, Michal Simek wrote:
>>> There is an issue to recognize zynq or zynqmp image because header
>>> checking is just the same. That's why zynqmp images are recognized as
>>> zynq one.
>>> Check unused fields which are initialized to zero in zynq format
>>> (__reserved1 0x38 and __reserved2 0x44) which are initialized for
>>> zynqmp. This should ensure that images are properly recognized by:
>>> ./tools/mkimage -l spl/boot.bin
>>> 
>>> Also show image type as ZynqMP instead of Zynq which is confusing
>>> and parse a53 64bit cpu select option in image_attributes field to make
>>> sure that this is ZynqMP image.
>> 
>> What if the target is on R5?
> 
> I expect you know the answer. I have ported u-boot on R5 which could use
> this feature but mkimage is not prepared for that.
> 
> I am happy to remove this zynqmp detection code because it should be
> enough what it is done for zynq. Just let me know if you want to remove it.

Yes, please :).

> 
>> 
>>> Reported-by: Alexander Graf <agraf@suse.de>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> 
>> Tested-by: Alexander Graf <agraf@suse.de>
> 
> It is question how exactly you want to use mkimage tools. If for u-boot
> generated images or for generic boot.bin generated by bootgen.

I think it makes sense to converge the two mechanisms. I want to be able to swap individual components between the two; use fsbl with mkimage and spl with bootgen. I don‘t think we‘re very far from that.

Alternatively Xilinx could open source bootgen and we could just use that always ;).


Alex

[U-Boot] [PATCH] tools: xilinx: Fix zynq/zynqmp image recognition

[Michal Simek]

On 16.3.2018 11:33, Alexander Graf wrote:
> 
> 
>> Am 16.03.2018 um 11:22 schrieb Michal Simek <michal.simek@xilinx.com>:
>>
>>> On 15.3.2018 22:54, Alexander Graf wrote:
>>>
>>>
>>>> On 14.03.18 11:14, Michal Simek wrote:
>>>> There is an issue to recognize zynq or zynqmp image because header
>>>> checking is just the same. That's why zynqmp images are recognized as
>>>> zynq one.
>>>> Check unused fields which are initialized to zero in zynq format
>>>> (__reserved1 0x38 and __reserved2 0x44) which are initialized for
>>>> zynqmp. This should ensure that images are properly recognized by:
>>>> ./tools/mkimage -l spl/boot.bin
>>>>
>>>> Also show image type as ZynqMP instead of Zynq which is confusing
>>>> and parse a53 64bit cpu select option in image_attributes field to make
>>>> sure that this is ZynqMP image.
>>>
>>> What if the target is on R5?
>>
>> I expect you know the answer. I have ported u-boot on R5 which could use
>> this feature but mkimage is not prepared for that.
>>
>> I am happy to remove this zynqmp detection code because it should be
>> enough what it is done for zynq. Just let me know if you want to remove it.
> 
> Yes, please :).

v2 sent with this.

> 
>>
>>>
>>>> Reported-by: Alexander Graf <agraf@suse.de>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>
>>> Tested-by: Alexander Graf <agraf@suse.de>
>>
>> It is question how exactly you want to use mkimage tools. If for u-boot
>> generated images or for generic boot.bin generated by bootgen.
> 
> I think it makes sense to converge the two mechanisms. I want to be able to swap individual components between the two; use fsbl with mkimage and spl with bootgen. I don‘t think we‘re very far from that.

You can do it now without any issue. Bootgen expects files with elf
suffix which is not needed for mkimage.

> 
> Alternatively Xilinx could open source bootgen and we could just use that always ;).

Not my decision.

Thanks,
Michal