[U-Boot] [PATCH v2] sunxi: Increase SPL header size to 64 bytes to avoid code corruption

2016-05-13 Thread Siarhei Siamashka 
The current SPL header, created by the 'mksunxiboot' tool, has size
32 bytes. But the code in the boot ROM stores the information about
the boot media at the offset 0x28 before passing control to the SPL.
For example, when booting from the SD card, the magic number written
by the boot ROM is 0. And when booting from the SPI flash, the magic
number is 3. NAND and eMMC probably have their own special magic
numbers too.

Currently the corrupted byte is a part of one of the instructions in
the reset vectors table:

b reset
ldr   pc, _undefined_instruction
ldr   pc, _software_interrupt  <- Corruption happens here
ldr   pc, _prefetch_abort
ldr   pc, _data_abort
ldr   pc, _not_used
ldr   pc, _irq
ldr   pc, _fiq

In practice this does not cause any visible problems, but it's still
better to fix it. As a bonus, the reported boot media type can be
later used in the 'spl_boot_device' function, but this is out of
the scope of this patch.

Signed-off-by: Siarhei Siamashka 
---

Changes in v2:
 - Increase the header size to 64 bytes instead of 48 bytes in order
   to satisfy the VBAR alignment requirements.

 arch/arm/include/asm/arch-sunxi/spl.h |  8 +++-
 include/configs/sunxi-common.h| 12 ++--
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/spl.h 
b/arch/arm/include/asm/arch-sunxi/spl.h
index ca9a4f9..a0f33b0 100644
--- a/arch/arm/include/asm/arch-sunxi/spl.h
+++ b/arch/arm/include/asm/arch-sunxi/spl.h
@@ -18,6 +18,10 @@
 #define SPL_ADDR   0x0
 #endif
 
+/* The low 8-bits of the 'boot_media' field in the SPL header */
+#define SUNXI_BOOTED_FROM_MMC0 0
+#define SUNXI_BOOTED_FROM_SPI  3
+
 /* boot head definition from sun4i boot code */
 struct boot_file_head {
uint32_t b_instruction; /* one intruction jumping to real code */
@@ -45,7 +49,9 @@ struct boot_file_head {
uint8_t spl_signature[4];
};
uint32_t fel_script_address;
-   uint32_t reserved;  /* padding, align to 32 bytes */
+   uint32_t reserved1[3];
+   uint32_t boot_media;/* written here by the boot ROM */
+   uint32_t reserved2[5];  /* padding, align to 64 bytes */
 };
 
 #define is_boot0_magic(addr)   (memcmp((void *)addr, BOOT0_MAGIC, 8) == 0)
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 2406115..e2ee908 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -189,14 +189,14 @@
 #define CONFIG_SPL_BOARD_LOAD_IMAGE
 
 #if defined(CONFIG_MACH_SUN9I)
-#define CONFIG_SPL_TEXT_BASE   0x10020 /* sram start+header */
-#define CONFIG_SPL_MAX_SIZE0x5fe0  /* ? KiB on sun9i */
+#define CONFIG_SPL_TEXT_BASE   0x10040 /* sram start+header */
+#define CONFIG_SPL_MAX_SIZE0x5fc0  /* ? KiB on sun9i */
 #elif defined(CONFIG_MACH_SUN50I)
-#define CONFIG_SPL_TEXT_BASE   0x10020 /* sram start+header */
-#define CONFIG_SPL_MAX_SIZE0x7fe0  /* 32 KiB on sun50i */
+#define CONFIG_SPL_TEXT_BASE   0x10040 /* sram start+header */
+#define CONFIG_SPL_MAX_SIZE0x7fc0  /* 32 KiB on sun50i */
 #else
-#define CONFIG_SPL_TEXT_BASE   0x20/* sram start+header */
-#define CONFIG_SPL_MAX_SIZE0x5fe0  /* 24KB on sun4i/sun7i 
*/
+#define CONFIG_SPL_TEXT_BASE   0x40/* sram start+header */
+#define CONFIG_SPL_MAX_SIZE0x5fc0  /* 24KB on sun4i/sun7i 
*/
 #endif
 
 #define CONFIG_SPL_LIBDISK_SUPPORT
-- 
2.7.3

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

[U-Boot] [PATCH v2] sunxi: Increase SPL header size to 64 bytes to avoid code corruption

2016-05-16 Thread Bernhard Nortmann 
Given that there now are quite a few additional "reserved" entries, and 
while we're still at SPL_HEADER_VERSION 1, I'd like to renew my request 
of dedicating one of these fields to the script length - which would 
enable us to set the U-Boot ${filesize} accordingly.


i.e.
--- arch-arm-include-asm-arch-sunxi-spl.h
+++ arch-arm-include-asm-arch-sunxi-spl.new.h
@@ -49,7 +49,8 @@
uint8_t spl_signature[4];
};
uint32_t fel_script_address;
-   uint32_t reserved1[3];
+   uint32_t fel_script_length;
+   uint32_t reserved1[2];
uint32_t boot_media;/* written here by the boot ROM */
uint32_t reserved2[5];  /* padding, align to 64 bytes */
 };


I do not intend to further push my specific use cases, however I still 
consider the (then somewhat theoretical) ability to do "import -t 
${fel_script_addr} ${filesize}" useful. For reference, the previous 
discussion related to this was somewhere around 
http://lists.denx.de/pipermail/u-boot/2015-September/227454.html


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

Re: [U-Boot] [PATCH v2] sunxi: Increase SPL header size to 64 bytes to avoid code corruption

2016-06-02 Thread Siarhei Siamashka 
On Mon, 16 May 2016 19:52:33 +0200
Hans de Goede  wrote:

> Hi,
> 
> On 16-05-16 11:56, Bernhard Nortmann wrote:
> > Given that there now are quite a few additional "reserved" entries, and 
> > while we're still at SPL_HEADER_VERSION 1, I'd like to renew my request of 
> > dedicating one of these fields to the script length - which would enable us 
> > to set the U-Boot ${filesize} accordingly.
> >
> > i.e.
> > --- arch-arm-include-asm-arch-sunxi-spl.h
> > +++ arch-arm-include-asm-arch-sunxi-spl.new.h
> > @@ -49,7 +49,8 @@
> > uint8_t spl_signature[4];
> > };
> > uint32_t fel_script_address;
> > -   uint32_t reserved1[3];
> > +   uint32_t fel_script_length;
> > +   uint32_t reserved1[2];
> > uint32_t boot_media;/* written here by the boot ROM */
> > uint32_t reserved2[5];  /* padding, align to 64 bytes */
> >  };
> >
> >
> > I do not intend to further push my specific use cases, however I still 
> > consider the (then somewhat theoretical) ability to do "import -t 
> > ${fel_script_addr} ${filesize}" useful. For reference, the previous 
> > discussion related to this was somewhere around 
> > http://lists.denx.de/pipermail/u-boot/2015-September/227454.html  
> 
> Hmm, given that the boot-rom touches some of these, I wonder if
> we should be putting anything here at all.

Yes, this came as a bit of surprise because this was not clearly
documented anywhere. Still it looks like that's just a single
byte getting modified, albeit at a bit strange location.

BTW, do you remember what I said earlier about not always being in
perfect control?

http://lists.denx.de/pipermail/u-boot/2015-September/228727.html

This particular issue just serves as a very nice demonstration :-)

Anyway, I think that we are already reasonably well prepared to handle
it. The worst thing that can happen is that the boot ROM in the future
Allwinner SoCs starts patching even more bytes in the header or moves
this boot device id variable to some other address. If/when this
happens, we can always update the SPL header format (do the "major"
version change trick).

> Other then that worry, I see no problem with adding a
> fel_script_length, Siarhei what is your opinion on this ?

I personally have no objections.

-- 
Best regards,
Siarhei Siamashka
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2] sunxi: Increase SPL header size to 64 bytes to avoid code corruption

2016-06-03 Thread Bernhard Nortmann 

Am 02.06.2016 um 16:57 schrieb Siarhei Siamashka:

On Mon, 16 May 2016 19:52:33 +0200
Hans de Goede  wrote:


[...]
Other then that worry, I see no problem with adding a
fel_script_length, Siarhei what is your opinion on this ?

I personally have no objections.



Does it make sense if I submit a patch for this, or should we simply
squash that one-line change into your "Store the device tree name in
the SPL header" series?

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

Re: [U-Boot] [PATCH v2] sunxi: Increase SPL header size to 64 bytes to avoid code corruption

2016-06-03 Thread Hans de Goede 

Hi,

On 03-06-16 09:30, Bernhard Nortmann wrote:

Am 02.06.2016 um 16:57 schrieb Siarhei Siamashka:

On Mon, 16 May 2016 19:52:33 +0200
Hans de Goede  wrote:


[...]
Other then that worry, I see no problem with adding a
fel_script_length, Siarhei what is your opinion on this ?

I personally have no objections.



Does it make sense if I submit a patch for this, or should we simply
squash that one-line change into your "Store the device tree name in
the SPL header" series?


This seems like an unrelated change to me and as such should be
a separate patch.

Regards,

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

Re: [U-Boot] [PATCH v2] sunxi: Increase SPL header size to 64 bytes to avoid code corruption

2016-05-15 Thread Hans de Goede 

Hi,

On 14-05-16 03:13, Siarhei Siamashka wrote:

The current SPL header, created by the 'mksunxiboot' tool, has size
32 bytes. But the code in the boot ROM stores the information about
the boot media at the offset 0x28 before passing control to the SPL.
For example, when booting from the SD card, the magic number written
by the boot ROM is 0. And when booting from the SPI flash, the magic
number is 3. NAND and eMMC probably have their own special magic
numbers too.

Currently the corrupted byte is a part of one of the instructions in
the reset vectors table:

b reset
ldr   pc, _undefined_instruction
ldr   pc, _software_interrupt  <- Corruption happens here
ldr   pc, _prefetch_abort
ldr   pc, _data_abort
ldr   pc, _not_used
ldr   pc, _irq
ldr   pc, _fiq

In practice this does not cause any visible problems, but it's still
better to fix it. As a bonus, the reported boot media type can be
later used in the 'spl_boot_device' function, but this is out of
the scope of this patch.

Signed-off-by: Siarhei Siamashka 


Thanks, looks good to me applied to:

http://git.denx.de/?p=u-boot/u-boot-sunxi.git;a=shortlog;h=refs/heads/next

This will be part of my first pull-req for u-boot v2016.07.

Regards,

Hans




---

Changes in v2:
 - Increase the header size to 64 bytes instead of 48 bytes in order
   to satisfy the VBAR alignment requirements.

 arch/arm/include/asm/arch-sunxi/spl.h |  8 +++-
 include/configs/sunxi-common.h| 12 ++--
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/spl.h 
b/arch/arm/include/asm/arch-sunxi/spl.h
index ca9a4f9..a0f33b0 100644
--- a/arch/arm/include/asm/arch-sunxi/spl.h
+++ b/arch/arm/include/asm/arch-sunxi/spl.h
@@ -18,6 +18,10 @@
 #define SPL_ADDR   0x0
 #endif

+/* The low 8-bits of the 'boot_media' field in the SPL header */
+#define SUNXI_BOOTED_FROM_MMC0 0
+#define SUNXI_BOOTED_FROM_SPI  3
+
 /* boot head definition from sun4i boot code */
 struct boot_file_head {
uint32_t b_instruction; /* one intruction jumping to real code */
@@ -45,7 +49,9 @@ struct boot_file_head {
uint8_t spl_signature[4];
};
uint32_t fel_script_address;
-   uint32_t reserved;  /* padding, align to 32 bytes */
+   uint32_t reserved1[3];
+   uint32_t boot_media;/* written here by the boot ROM */
+   uint32_t reserved2[5];  /* padding, align to 64 bytes */
 };

 #define is_boot0_magic(addr)   (memcmp((void *)addr, BOOT0_MAGIC, 8) == 0)
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 2406115..e2ee908 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -189,14 +189,14 @@
 #define CONFIG_SPL_BOARD_LOAD_IMAGE

 #if defined(CONFIG_MACH_SUN9I)
-#define CONFIG_SPL_TEXT_BASE   0x10020 /* sram start+header */
-#define CONFIG_SPL_MAX_SIZE0x5fe0  /* ? KiB on sun9i */
+#define CONFIG_SPL_TEXT_BASE   0x10040 /* sram start+header */
+#define CONFIG_SPL_MAX_SIZE0x5fc0  /* ? KiB on sun9i */
 #elif defined(CONFIG_MACH_SUN50I)
-#define CONFIG_SPL_TEXT_BASE   0x10020 /* sram start+header */
-#define CONFIG_SPL_MAX_SIZE0x7fe0  /* 32 KiB on sun50i */
+#define CONFIG_SPL_TEXT_BASE   0x10040 /* sram start+header */
+#define CONFIG_SPL_MAX_SIZE0x7fc0  /* 32 KiB on sun50i */
 #else
-#define CONFIG_SPL_TEXT_BASE   0x20/* sram start+header */
-#define CONFIG_SPL_MAX_SIZE0x5fe0  /* 24KB on sun4i/sun7i 
*/
+#define CONFIG_SPL_TEXT_BASE   0x40/* sram start+header */
+#define CONFIG_SPL_MAX_SIZE0x5fc0  /* 24KB on sun4i/sun7i 
*/
 #endif

 #define CONFIG_SPL_LIBDISK_SUPPORT


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

Re: [U-Boot] [PATCH v2] sunxi: Increase SPL header size to 64 bytes to avoid code corruption

2016-05-16 Thread Hans de Goede 

Hi,

On 16-05-16 11:56, Bernhard Nortmann wrote:

Given that there now are quite a few additional "reserved" entries, and while 
we're still at SPL_HEADER_VERSION 1, I'd like to renew my request of dedicating one of 
these fields to the script length - which would enable us to set the U-Boot ${filesize} 
accordingly.

i.e.
--- arch-arm-include-asm-arch-sunxi-spl.h
+++ arch-arm-include-asm-arch-sunxi-spl.new.h
@@ -49,7 +49,8 @@
uint8_t spl_signature[4];
};
uint32_t fel_script_address;
-   uint32_t reserved1[3];
+   uint32_t fel_script_length;
+   uint32_t reserved1[2];
uint32_t boot_media;/* written here by the boot ROM */
uint32_t reserved2[5];  /* padding, align to 64 bytes */
 };


I do not intend to further push my specific use cases, however I still consider the (then 
somewhat theoretical) ability to do "import -t ${fel_script_addr} ${filesize}" 
useful. For reference, the previous discussion related to this was somewhere around 
http://lists.denx.de/pipermail/u-boot/2015-September/227454.html


Hmm, given that the boot-rom touches some of these, I wonder if
we should be putting anything here at all.

Other then that worry, I see no problem with adding a
fel_script_length, Siarhei what is your opinion on this ?

Regards,

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