Skip to contentSkip to conversation

SPL: FIT: Align loading address for header

RFC PATCH In review Michal Simek 4 participants 9 messages Apr 28, 2016 – May 31, 2016

lore
Share & tools
lore

Michal Simek

author
#0 lore raw

RFC PATCH 1/1 SPL: FIT: Align loading address for header

If bl_len is not aligned it can caused a problem because another code expects that start is aligned.

Signed-off-by Michal Simek <[email protected]>

---

Not sure if this is the right way how to ensure it.
But patch is pointing to the problem. For example if bl_len is 1.

---
 common/spl/spl_fit.c | 1 +
 1 file changed, 1 insertion(+)
common/spl/spl_fit.c +1 -0
index e301927c87ac..08e432a52dbb 100644
@@ -119,6 +119,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 	 * be before CONFIG_SYS_TEXT_BASE.
 	 */
 	fit = (void *)(CONFIG_SYS_TEXT_BASE - size - info->bl_len);
+	fit = (void *)ALIGN((ulong)fit, 8);
 	sectors = (size + info->bl_len - 1) / info->bl_len;
 	count = info->read(info, sector, sectors, fit);
 	debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu\n",
Signature · 1 line
1.9.1
#1 lore raw

RFC PATCH 1/1 SPL: FIT: Align loading address for header

Quoted 8 lines · Michal Simek

If bl_len is not aligned it can caused a problem because another code expects that start is aligned.

Signed-off-by Michal Simek <[email protected]>

---

Not sure if this is the right way how to ensure it.
But patch is pointing to the problem. For example if bl_len is 1.

I suppose this is a non-block device. OK.

Quoted 17 lines
---
 common/spl/spl_fit.c | 1 +
 1 file changed, 1 insertion(+)
common/spl/spl_fit.c +1 -1
index e301927c87ac..08e432a52dbb 100644
@@ -119,6 +119,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
         * be before CONFIG_SYS_TEXT_BASE.
         */
        fit = (void *)(CONFIG_SYS_TEXT_BASE - size - info->bl_len);
+       fit = (void *)ALIGN((ulong)fit, 8);
        sectors = (size + info->bl_len - 1) / info->bl_len;
        count = info->read(info, sector, sectors, fit);
        debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu\n",
--
Quoted 2 lines

1.9.1

Reviewed-by Simon Glass <[email protected]>
#2 lore raw

RFC PATCH 1/1 SPL: FIT: Align loading address for header

In reply to Michal Simek
Quoted 2 lines · Michal Simek

If bl_len is not aligned it can caused a problem because another code expects that start is aligned.

What about loading of U-boot image and dtb image? I have posted a similar patch for solving this[1]. Can you see if it helps?

[1] http://patchwork.ozlabs.org/patch/606958/

Thanks and regards,
Lokesh

Quoted 24 lines
Signed-off-by Michal Simek <[email protected]>

---

Not sure if this is the right way how to ensure it.
But patch is pointing to the problem. For example if bl_len is 1.

---
 common/spl/spl_fit.c | 1 +
 1 file changed, 1 insertion(+)
common/spl/spl_fit.c +1 -0
index e301927c87ac..08e432a52dbb 100644
@@ -119,6 +119,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 	 * be before CONFIG_SYS_TEXT_BASE.
 	 */
 	fit = (void *)(CONFIG_SYS_TEXT_BASE - size - info->bl_len);
+	fit = (void *)ALIGN((ulong)fit, 8);
 	sectors = (size + info->bl_len - 1) / info->bl_len;
 	count = info->read(info, sector, sectors, fit);
 	debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu\n",
author
#3 lore raw

RFC PATCH 1/1 SPL: FIT: Align loading address for header

Hi,

Quoted 10 lines · Lokesh Vutla
Quoted 2 lines · Michal Simek

If bl_len is not aligned it can caused a problem because another code expects that start is aligned.

What about loading of U-boot image and dtb image? I have posted a similar patch for solving this[1]. Can you see if it helps?

[1] http://patchwork.ozlabs.org/patch/606958/

I have created this before I saw your patch. I am happy with your patch. Please ignore this one.

Thanks,
Michal

#4 lore raw
In reply to Michal Simek
Quoted 5 lines · Michal Simek

If bl_len is not aligned it can caused a problem because another code expects that start is aligned.

Signed-off-by Michal Simek <[email protected]>
Reviewed-by Simon Glass <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160530/fb1beb65/attachment.sig>
#5 lore raw
Quoted 9 lines · Tom Rini
Quoted 5 lines · Michal Simek

If bl_len is not aligned it can caused a problem because another code expects that start is aligned.

Signed-off-by Michal Simek <[email protected]>
Reviewed-by Simon Glass <[email protected]>

Applied to u-boot/master, thanks!

This patch is not required and should not be applied.

Thanks and regards,
Lokesh

Quoted 8 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#6 lore raw
Quoted 8 lines · Lokesh Vutla
Quoted 5 lines · Tom Rini
Quoted 3 lines · Michal Simek

If bl_len is not aligned it can caused a problem because another code expects that start is aligned.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Signed-off-by Michal Simek <[email protected]>
Reviewed-by Simon Glass <[email protected]>

Applied to u-boot/master, thanks!

This patch is not required and should not be applied.

Did you both agree on "spl: Allow to load a FIT containing U-Boot from FS" instead then? And thus what's in the tree needs a little re-work since I made the above apply on top of this one instead? Sorry/thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160531/c35a252c/attachment.sig>
#7 lore raw
Quoted 12 lines · Tom Rini
Quoted 11 lines · Lokesh Vutla
Quoted 8 lines · Tom Rini
Quoted 5 lines · Michal Simek

If bl_len is not aligned it can caused a problem because another code expects that start is aligned.

Signed-off-by Michal Simek <[email protected]>
Reviewed-by Simon Glass <[email protected]>
Quoted 7 lines
Quoted 3 lines
Quoted 1 line

Applied to u-boot/master, thanks!

This patch is not required and should not be applied.

Did you both agree on "spl: Allow to load a FIT containing U-Boot from FS" instead then? And thus what's in the tree needs a little re-work since I made the above apply on top of this one instead? Sorry/thanks!

Yes, Michal as well agreed that this is not required[1]. This patch might work but it can fail in corner cases.

If you want I can send a fix patch on top of latest.

[1] http://patchwork.ozlabs.org/patch/616065/

Thanks and regards,
Lokesh

Quoted 1 line
#8 lore raw
Quoted 19 lines · Lokesh Vutla
Quoted 16 lines · Tom Rini
Quoted 14 lines · Lokesh Vutla
Quoted 9 lines · Tom Rini
Quoted 5 lines · Michal Simek

If bl_len is not aligned it can caused a problem because another code expects that start is aligned.

Signed-off-by Michal Simek <[email protected]>
Reviewed-by Simon Glass <[email protected]>

Applied to u-boot/master, thanks!

This patch is not required and should not be applied.

Quoted 8 lines
Quoted 3 lines

Did you both agree on "spl: Allow to load a FIT containing U-Boot from FS" instead then? And thus what's in the tree needs a little re-work since I made the above apply on top of this one instead? Sorry/thanks!

Yes, Michal as well agreed that this is not required[1]. This patch might work but it can fail in corner cases.

If you want I can send a fix patch on top of latest.

Yes please, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160531/969eb16b/attachment.sig>