sunxi: extend SPL header to propagate DRAM size
Subsystems (2)
cover sunxi: extend SPL header to propagate DRAM size
This series tries to solve three issues we currently have on Allwinner boards:
- The DRAM sizing routine can only cope with power-of-two sized DRAM.
- The DRAM sizing routine steps through all DRAM, possibly hitting secure memory.
- The SPL header versioning is quite strict and tends to break every time we need to update it.
So I thought about introducing something along the lines of semantic versioning[1], where we can add backwards-compatible changes to the SPL header without breaking every tool. This is introduced in the first patch. The second patch does some refactoring, so that the third patch can use the newly gained freedom to store the DRAM size. The SPL knows the DRAM size very well, so we store this in the SPL header, so that U-Boot proper can pick it up from there. This saves the call to get_ram_size() with its deficiencies.
More information in the respective commit messages.
I understand that this versioning solution is not fully future-proof, but we have only one byte for the version, and I just wanted to start discussion on this.
There is a corresponding patch for sunxi-tools as well I am posting shortly.
Cheers,
Andre.
Andre Przywara (3):
sunxi: Extend SPL header versioning
sunxi: board.c: refactor SPL header checks
sunxi: store DRAM size in SPL header
arch/arm/include/asm/arch-sunxi/spl.h | 22 ++++++++---- board/sunxi/board.c | 66 ++++++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 18 deletions(-)
Signature · 1 line
2.14.1
RFC PATCH 1/3 sunxi: Extend SPL header versioning
On Allwinner SoCs we use some free bytes at the beginning of the SPL image to store various information. We have a version byte to allow updates, but changing this always requires all tools to be updated as well.
Introduce the concept of semantic versioning [1] to the SPL header:
The major part of the version number only changes on incompatible updates, a minor number bump indicates backward compatibility. This patch just documents the major/minor split, adds some comments to the header file and uses the versioning information for the existing users.
--- arch/arm/include/asm/arch-sunxi/spl.h | 19 ++++++++++++++----- board/sunxi/board.c | 4 ++-- 2 files changed, 16 insertions(+), 7 deletions(-)
arch/arm/include/asm/arch-sunxi/spl.h +14 -5
@@ -9,7 +9,16 @@#define BOOT0_MAGIC "eGON.BT0"#define SPL_SIGNATURE "SPL"/* marks "sunxi" SPL header */ -#define SPL_HEADER_VERSION 2 +#define SPL_MAJOR_BITS 3 +#define SPL_MINOR_BITS 5 +#define SPL_VERSION (maj, min) \ + ((((maj) & ((1U << SPL_MAJOR_BITS) - 1)) << SPL_MINOR_BITS) | \ + ((min) & ((1U << SPL_MINOR_BITS) - 1))) + +#define SPL_HEADER_VERSION SPL_VERSION(0, 2) + +#define SPL_ENV_HEADER_VERSION SPL_VERSION(0, 1) +#define SPL_DT_HEADER_VERSION SPL_VERSION(0, 2)#ifdef CONFIG_SUNXI_HIGH_SRAM #define SPL_ADDR 0x10000
@@ -49,14 +58,14 @@ struct boot_file_head {uint32_t pub_head_size;uint8_t spl_signature[4 ]; }; -uint32_t fel_script_address; +uint32_t fel_script_address;/* since v0.1, set by sunxi-fel */ /* * If the fel_uEnv_length member below is set to a non-zero value, * it specifies the size (byte count) of data@fel_script_address. * At the same time this indicates that the data is in uEnv.txt * compatible format, ready to be imported via "env import -t". */ -uint32_t fel_uEnv_length; +uint32_t fel_uEnv_length;/* since v0.1, set by sunxi-fel */ /* * Offset of an ASCIIZ string (relative to the SPL header), which * contains the default device tree name (CONFIG_DEFAULT_DEVICE_TREE).
@@ -64,11 +73,11 @@ struct boot_file_head {* by flash programming tools for providing nice informative messages * to the users. */ -uint32_t dt_name_offset; +uint32_t dt_name_offset;/* since v0.2, set by mksunxiboot */ uint32_t reserved1;uint32_t boot_media;/* written here by the boot ROM */ /* A padding area (may be used for storing text strings) */ -uint32_t string_pool[13 ]; +uint32_t string_pool[13 ];/* since v0.2, filled by mksunxiboot */ /* The header must be a multiple of 32 bytes (for VBAR alignment) */ };
board/sunxi/board.c +2 -2
@@ -631,9 +631,9 @@ static void parse_spl_header(const uint32_t spl_addr)return ;/* signature mismatch, no usable header */ uint8_t spl_header_version = spl->spl_signature[3 ]; -if (spl_header_version !=SPL_HEADER_VERSION ) { +if (spl_header_version <SPL_ENV_HEADER_VERSION ) {printf ("sunxi SPL version mismatch: expected %u, got %u\n" , -SPL_HEADER_VERSION , spl_header_version); +SPL_ENV_HEADER_VERSION , spl_header_version);return ; }if (!spl->fel_script_address)
Signature · 1 line
2.14.1
RFC PATCH 1/3 sunxi: Extend SPL header versioning
Quoted 3 lines · On Thu, 17 May 2018 09:16:59 +0100 Andre Przywara <[email protected]>…
On Allwinner SoCs we use some free bytes at the beginning of the SPL image to store various information. We have a version byte to allow updates, but changing this always requires all tools to be updated as well.
The tools do not need to be updated together with U-Boot even now.
U-Boot may freely increment the SPL version number as long as the new header is a superset of the old one.
Quoted 8 lines
Introduce the concept of semantic versioning [1] to the SPL header:
The major part of the version number only changes on incompatible updates, a minor number bump indicates backward compatibility. This patch just documents the major/minor split, adds some comments to the header file and uses the versioning information for the existing users.
So basically you are implementing the versioning scheme that I proposed back in 2015:
https://lists.denx.de/pipermail/u-boot/2015-September/228727.html
Hans de Goede thought that the major/minor versioning was too complex and unnecessary (if I remember correctly, we had several discussion threads which concluded in the same way), so we did not implement it explicitly back then. But a potential non-compatible SPL header upgrade still could be handled, albeit less gracefully:
Yes, we can also always change the SPL header signature in the case of introducing incompatible changes. But the down side is that the "fel" tool would treat this situation as "unknown bootloader" instead of "incompatible U-Boot".
Quoted 1 line
Signed-off-by Andre Przywara <[email protected]>
In general, improvements in this area are welcome. Just some comments below.
Quoted 28 lines
--- arch/arm/include/asm/arch-sunxi/spl.h | 19 ++++++++++++++----- board/sunxi/board.c | 4 ++-- 2 files changed, 16 insertions(+), 7 deletions(-)arch/arm/include/asm/arch-sunxi/spl.h +10
-1@@ -9,7 +9,16 @@#define BOOT0_MAGIC "eGON.BT0"#define SPL_SIGNATURE "SPL"/* marks "sunxi" SPL header */ -#define SPL_HEADER_VERSION 2 +#define SPL_MAJOR_BITS 3 +#define SPL_MINOR_BITS 5 +#define SPL_VERSION (maj, min) \ + ((((maj) & ((1U << SPL_MAJOR_BITS) - 1)) << SPL_MINOR_BITS) | \ + ((min) & ((1U << SPL_MINOR_BITS) - 1))) + +#define SPL_HEADER_VERSION SPL_VERSION(0, 2) + +#define SPL_ENV_HEADER_VERSION SPL_VERSION(0, 1) +#define SPL_DT_HEADER_VERSION SPL_VERSION(0, 2)#ifdef CONFIG_SUNXI_HIGH_SRAM #define SPL_ADDR 0x10000
arch/arm/include/asm/arch-sunxi/spl.h struct boot_file_head { · line 61
Quoted 5 lines
arch/arm/include/asm/arch-sunxi/spl.h +1
-1@@ -49,14 +58,14 @@ struct boot_file_head {uint32_t pub_head_size;uint8_t spl_signature[4 ]; }; -uint32_t fel_script_address; +uint32_t fel_script_address;/* since v0.1, set by sunxi-fel */
Thanks, it's nice to have these comments about the versions where these features were introduced.
Quoted 12 lines
arch/arm/include/asm/arch-sunxi/spl.h +1
-1/* * If the fel_uEnv_length member below is set to a non-zero value, * it specifies the size (byte count) of data at fel_script_address. * At the same time this indicates that the data is in uEnv.txt * compatible format, ready to be imported via "env import -t". */ -uint32_t fel_uEnv_length; +uint32_t fel_uEnv_length;/* since v0.1, set by sunxi-fel */ /* * Offset of an ASCIIZ string (relative to the SPL header), which * contains the default device tree name (CONFIG_DEFAULT_DEVICE_TREE).
arch/arm/include/asm/arch-sunxi/spl.h struct boot_file_head { · line 80
Quoted 10 lines
arch/arm/include/asm/arch-sunxi/spl.h +2
-2@@ -64,11 +73,11 @@ struct boot_file_head {* by flash programming tools for providing nice informative messages * to the users. */ -uint32_t dt_name_offset; +uint32_t dt_name_offset;/* since v0.2, set by mksunxiboot */ uint32_t reserved1;uint32_t boot_media;/* written here by the boot ROM */ /* A padding area (may be used for storing text strings) */ -uint32_t string_pool[13 ]; +uint32_t string_pool[13 ];/* since v0.2, filled by mksunxiboot */
The 0.2 version of the SPL header does not specify the exact format of this 'string_pool' padding area. So I think that this comment is unnecessary.
In principle, the device tree name string can be stored even somewhere in the const data section of the SPL binary and referenced from the SPL header. Not that it makes any sense to do this, but it is technically possible.
board/sunxi/board.c static void parse_spl_header(const uint32_t spl_addr) · line 634
Quoted 13 lines
arch/arm/include/asm/arch-sunxi/spl.h +0
-0/* The header must be a multiple of 32 bytes (for VBAR alignment) */ };board/sunxi/board.c +1
-1@@ -631,9 +631,9 @@ static void parse_spl_header(const uint32_t spl_addr)return ;/* signature mismatch, no usable header */ uint8_t spl_header_version = spl->spl_signature[3 ]; -if (spl_header_version !=SPL_HEADER_VERSION ) { +if (spl_header_version <SPL_ENV_HEADER_VERSION ) {
We have already discussed this particular part of code earlier:
https://lists.denx.de/pipermail/u-boot/2015-September/227999.html
Essentially, unless we have a real use case for mixing the SPL and the main U-Boot parts built from different U-Boot versions, I don't see any purpose for doing anything other than just exact version check here.
If this particular check fails (the SPL part does not match the main U-Boot part), then something is already very wrong.
Quoted 6 lines
board/sunxi/board.c +1
-1printf ("sunxi SPL version mismatch: expected %u, got %u\n" , -SPL_HEADER_VERSION , spl_header_version); +SPL_ENV_HEADER_VERSION , spl_header_version);return ; }if (!spl->fel_script_address)
Signature · 2 lines
Best regards, Siarhei Siamashka
RFC PATCH 1/3 sunxi: Extend SPL header versioning
于 2018年5月17日 GMT+08:00 下午7:05:15, Siarhei Siamashka <[email protected]> 写到:
Quoted 5 lines
Quoted 1 line · On Thu, 17 May 2018 09:16:59 +0100 Andre Przywara <[email protected]>…
On Allwinner SoCs we use some free bytes at the beginning of the SPL
image
Quoted 8 lines
Quoted 1 line
to store various information. We have a version byte to allow
updates,
Quoted 1 line
but changing this always requires all tools to be updated as well.
The tools do not need to be updated together with U-Boot even now.
U-Boot may freely increment the SPL version number as long as the new header is a superset of the old one.
But now sunxi-fel will work when SPL ver not recognized.
Quoted 136 lines
Quoted 6 lines
Introduce the concept of semantic versioning [1] to the SPL header:
The major part of the version number only changes on incompatible updates, a minor number bump indicates backward compatibility. This patch just documents the major/minor split, adds some comments to the header file and uses the versioning information for theexisting
Quoted 3 lines
users.
So basically you are implementing the versioning scheme that I proposed back in 2015:
https://lists.denx.de/pipermail/u-boot/2015-September/228727.htmlHans de Goede thought that the major/minor versioning was too complex and unnecessary (if I remember correctly, we had several discussion threads which concluded in the same way), so we did not implement it explicitly back then. But a potential non-compatible SPL header upgrade still could be handled, albeit less gracefully:
Yes, we can also always change the SPL header signature in the case of introducing incompatible changes. But the down side is that the "fel" tool would treat this situation as "unknown bootloader" instead of "incompatible U-Boot".
Quoted 1 line
Signed-off-by Andre Przywara <[email protected]>In general, improvements in this area are welcome. Just some comments below.
Review
Quoted 6 lines
--- arch/arm/include/asm/arch-sunxi/spl.h | 19 ++++++++++++++----- board/sunxi/board.c | 4 ++-- 2 files changed, 16 insertions(+), 7 deletions(-)(quoted diff) +0
-0
b/arch/arm/include/asm/arch-sunxi/spl.hQuoted 22 lines
index
4277d836e5..7cf89c8db2100644arch/arm/include/asm/arch-sunxi/spl.h +10
-1@@ -9,7 +9,16 @@#define BOOT0_MAGIC "eGON.BT0"#define SPL_SIGNATURE "SPL"/* marks "sunxi" SPL header */ -#define SPL_HEADER_VERSION 2 +#define SPL_MAJOR_BITS 3 +#define SPL_MINOR_BITS 5 +#define SPL_VERSION (maj, min) \ + ((((maj) & ((1U << SPL_MAJOR_BITS) - 1)) << SPL_MINOR_BITS) | \ + ((min) & ((1U << SPL_MINOR_BITS) - 1))) + +#define SPL_HEADER_VERSION SPL_VERSION(0, 2) + +#define SPL_ENV_HEADER_VERSION SPL_VERSION(0, 1) +#define SPL_DT_HEADER_VERSION SPL_VERSION(0, 2)#ifdef CONFIG_SUNXI_HIGH_SRAM #define SPL_ADDR 0x10000arch/arm/include/asm/arch-sunxi/spl.h struct boot_file_head { · line 61
Quoted 5 lines
arch/arm/include/asm/arch-sunxi/spl.h +1
-1@@ -49,14 +58,14 @@ struct boot_file_head {uint32_t pub_head_size;uint8_t spl_signature[4 ]; }; -uint32_t fel_script_address; +uint32_t fel_script_address;/* since v0.1, set by sunxi-fel */ Thanks, it's nice to have these comments about the versions where these features were introduced.
arch/arm/include/asm/arch-sunxi/spl.h struct boot_file_head {
Quoted 3 lines
arch/arm/include/asm/arch-sunxi/spl.h +0
-0/* * If the fel_uEnv_length member below is set to a non-zero value, * it specifies the size (byte count) of data at
fel_script_address.arch/arm/include/asm/arch-sunxi/spl.h struct boot_file_head {
Quoted 8 lines
arch/arm/include/asm/arch-sunxi/spl.h +1
-1* At the same time this indicates that the data is in uEnv.txt * compatible format, ready to be imported via "env import -t". */ -uint32_t fel_uEnv_length; +uint32_t fel_uEnv_length;/* since v0.1, set by sunxi-fel */ /* * Offset of an ASCIIZ string (relative to the SPL header), which * contains the default device tree name (
CONFIG_DEFAULT_DEVICE_TREE).arch/arm/include/asm/arch-sunxi/spl.h struct boot_file_head { · line 73
Quoted 2 lines
arch/arm/include/asm/arch-sunxi/spl.h +0
-0@@ -64,11 +73,11 @@ struct boot_file_head {* by flash programming tools for providing nice informative messages
arch/arm/include/asm/arch-sunxi/spl.h struct boot_file_head { · line 80
Quoted 9 lines
arch/arm/include/asm/arch-sunxi/spl.h +2
-2* to the users. */ -uint32_t dt_name_offset; +uint32_t dt_name_offset;/* since v0.2, set by mksunxiboot */ uint32_t reserved1;uint32_t boot_media;/* written here by the boot ROM */ /* A padding area (may be used for storing text strings) */ -uint32_t string_pool[13 ]; +uint32_t string_pool[13 ];/* since v0.2, filled by mksunxiboot */ The 0.2 version of the SPL header does not specify the exact format of this '
string_pool' padding area. So I think that this comment is unnecessary.In principle, the device tree name string can be stored even somewhere in the const data section of the SPL binary and referenced from the SPL header. Not that it makes any sense to do this, but it is technically possible.
arch/arm/include/asm/arch-sunxi/spl.h struct boot_file_head { · line 81
Quoted 1 line
arch/arm/include/asm/arch-sunxi/spl.h +0
-0/* The header must be a multiple of 32 bytes (for VBAR alignment) */
board/sunxi/board.c static void parse_spl_header(const uint32_t
Quoted 7 lines
arch/arm/include/asm/arch-sunxi/spl.h +0
-0};board/sunxi/board.c +0
-0@@ -631,9 +631,9 @@ static void parse_spl_header(const uint32_t
spl_addr)board/sunxi/board.c static void parse_spl_header(const uint32_t spl_addr) · line 634
Quoted 5 lines
board/sunxi/board.c +1
-1return ;/* signature mismatch, no usable header */ uint8_t spl_header_version = spl->spl_signature[3 ]; -if (spl_header_version !=SPL_HEADER_VERSION ) { +if (spl_header_version <SPL_ENV_HEADER_VERSION ) {We have already discussed this particular part of code earlier:
https://lists.denx.de/pipermail/u-boot/2015-September/227999.htmlEssentially, unless we have a real use case for mixing the SPL and the main U-Boot parts built from different U-Boot versions, I don't see any purpose for doing anything other than just exact version check here.
If this particular check fails (the SPL part does not match the main U-Boot part), then something is already very wrong.
Quoted 6 lines
board/sunxi/board.c +1
-1printf ("sunxi SPL version mismatch: expected %u, got %u\n" , -SPL_HEADER_VERSION , spl_header_version); +SPL_ENV_HEADER_VERSION , spl_header_version);return ; }if (!spl->fel_script_address)
RFC PATCH 1/3 sunxi: Extend SPL header versioning
In reply to Siarhei SiamashkaHi,
Quoted 19 lines · Siarhei Siamashka
Quoted 3 lines · On Thu, 17 May 2018 09:16:59 +0100 Andre Przywara <[email protected]>…
On Allwinner SoCs we use some free bytes at the beginning of the SPL image to store various information. We have a version byte to allow updates, but changing this always requires all tools to be updated as well.
The tools do not need to be updated together with U-Boot even now.
U-Boot may freely increment the SPL version number as long as the new header is a superset of the old one.
Quoted 7 lines
Introduce the concept of semantic versioning [1] to the SPL header:
The major part of the version number only changes on incompatible updates, a minor number bump indicates backward compatibility. This patch just documents the major/minor split, adds some comments to the header file and uses the versioning information for the existing users.
Quoted 5 lines
Quoted 1 line
So basically you are implementing the versioning scheme that I proposed back in 2015:
https://lists.denx.de/pipermail/u-boot/2015-September/228727.html
Ah, sorry, that predates my sunxi involvement ;-)
Quoted 71 lines
Hans de Goede thought that the major/minor versioning was too complex and unnecessary (if I remember correctly, we had several discussion threads which concluded in the same way), so we did not implement it explicitly back then. But a potential non-compatible SPL header upgrade still could be handled, albeit less gracefully:
Yes, we can also always change the SPL header signature in the case of introducing incompatible changes. But the down side is that the "fel" tool would treat this situation as "unknown bootloader" instead of "incompatible U-Boot".
Quoted 1 line
Signed-off-by Andre Przywara <[email protected]>In general, improvements in this area are welcome. Just some comments below.
Quoted 28 lines
--- arch/arm/include/asm/arch-sunxi/spl.h | 19 ++++++++++++++----- board/sunxi/board.c | 4 ++-- 2 files changed, 16 insertions(+), 7 deletions(-)arch/arm/include/asm/arch-sunxi/spl.h +10
-1@@ -9,7 +9,16 @@#define BOOT0_MAGIC "eGON.BT0"#define SPL_SIGNATURE "SPL"/* marks "sunxi" SPL header */ -#define SPL_HEADER_VERSION 2 +#define SPL_MAJOR_BITS 3 +#define SPL_MINOR_BITS 5 +#define SPL_VERSION (maj, min) \ + ((((maj) & ((1U << SPL_MAJOR_BITS) - 1)) << SPL_MINOR_BITS) | \ + ((min) & ((1U << SPL_MINOR_BITS) - 1))) + +#define SPL_HEADER_VERSION SPL_VERSION(0, 2) + +#define SPL_ENV_HEADER_VERSION SPL_VERSION(0, 1) +#define SPL_DT_HEADER_VERSION SPL_VERSION(0, 2)#ifdef CONFIG_SUNXI_HIGH_SRAM #define SPL_ADDR 0x10000arch/arm/include/asm/arch-sunxi/spl.h struct boot_file_head { · line 61
Quoted 5 lines
arch/arm/include/asm/arch-sunxi/spl.h +1
-1@@ -49,14 +58,14 @@ struct boot_file_head {uint32_t pub_head_size;uint8_t spl_signature[4 ]; }; -uint32_t fel_script_address; +uint32_t fel_script_address;/* since v0.1, set by sunxi-fel */ Thanks, it's nice to have these comments about the versions where these features were introduced.
Quoted 18 lines
arch/arm/include/asm/arch-sunxi/spl.h +2
-2/* * If the fel_uEnv_length member below is set to a non-zero value, * it specifies the size (byte count) of data at fel_script_address. * At the same time this indicates that the data is in uEnv.txt * compatible format, ready to be imported via "env import -t". */ -uint32_t fel_uEnv_length; +uint32_t fel_uEnv_length;/* since v0.1, set by sunxi-fel */ /* * Offset of an ASCIIZ string (relative to the SPL header), which * contains the default device tree name (CONFIG_DEFAULT_DEVICE_TREE). @@ -64,11 +73,11 @@ struct boot_file_head {* by flash programming tools for providing nice informative messages * to the users. */ -uint32_t dt_name_offset; +uint32_t dt_name_offset;/* since v0.2, set by mksunxiboot */ uint32_t reserved1;
Quoted 8 lines
arch/arm/include/asm/arch-sunxi/spl.h struct boot_file_head { · line 80
Quoted 4 lines
arch/arm/include/asm/arch-sunxi/spl.h +1
-1uint32_t boot_media;/* written here by the boot ROM */ /* A padding area (may be used for storing text strings) */ -uint32_t string_pool[13 ]; +uint32_t string_pool[13 ];/* since v0.2, filled by mksunxiboot */ The 0.2 version of the SPL header does not specify the exact format of this '
string_pool' padding area. So I think that this comment is unnecessary.
Well, I understand that it *could* be everywhere, but I wanted to give a heads up to the reader that the actual mksunxiboot implementation fills that area, so it's not really vacant anymore. That may become of importance if we need more fields.
Quoted 20 lines
In principle, the device tree name string can be stored even somewhere in the const data section of the SPL binary and referenced from the SPL header. Not that it makes any sense to do this, but it is technically possible.
board/sunxi/board.c static void parse_spl_header(const uint32_t spl_addr) · line 634
Quoted 13 lines
arch/arm/include/asm/arch-sunxi/spl.h +0
-0/* The header must be a multiple of 32 bytes (for VBAR alignment) */ };board/sunxi/board.c +1
-1@@ -631,9 +631,9 @@ static void parse_spl_header(const uint32_t spl_addr)return ;/* signature mismatch, no usable header */ uint8_t spl_header_version = spl->spl_signature[3 ]; -if (spl_header_version !=SPL_HEADER_VERSION ) { +if (spl_header_version <SPL_ENV_HEADER_VERSION ) {We have already discussed this particular part of code earlier:
Quoted 5 lines
https://lists.denx.de/pipermail/u-boot/2015-September/227999.htmlEssentially, unless we have a real use case for mixing the SPL and the main U-Boot parts built from different U-Boot versions, I don't see any purpose for doing anything other than just exact version check here.
For FEL booting we might trigger this. My personal use case is to have FEL capable 32-bit SPL binaries for the ARMv8 SoCs lying around[2], and using them with any version of U-Boot proper I want to test/debug. So I hit this there. I see that this is quite a stretch, but this "<" relation is what the code actually requires, especially now with the semantic versioning, so I changed that.
Cheers,
Andre.
[2] https://github.com/apritzel/pine64/tree/master/binaries
Quoted 10 lines
If this particular check fails (the SPL part does not match the main U-Boot part), then something is already very wrong.
Quoted 6 lines
board/sunxi/board.c +1
-1printf ("sunxi SPL version mismatch: expected %u, got %u\n" , -SPL_HEADER_VERSION , spl_header_version); +SPL_ENV_HEADER_VERSION , spl_header_version);return ; }if (!spl->fel_script_address)
RFC PATCH 2/3 sunxi: board.c: refactor SPL header checks
In reply to Andre PrzywaraSo far we have two users which want to look at the SPL header. We will get more in the future.
Refactor the existing SPL header checks into a common function, to simplify reusing the code.
Now that this is easy, add proper version checks to the DT name parsing.
--- board/sunxi/board.c | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-)
board/sunxi/board.c +30 -11
@@ -253,6 +253,30 @@ int board_init(void)return soft_i2c_board_init (); } +/* +* On older SoCs the SPL is actually at address zero, so using NULL as +* an error value does not work. +*/ +#define INVALID_SPL_HEADER ((void *)~0UL) + +static struct boot_file_head *get_spl_header (uint8_t req_version) +{ +struct boot_file_head *spl = (void *)(ulong )SPL_ADDR ; +uint8_t spl_header_version = spl->spl_signature[3 ]; + +/* Is there really the SPL header (still) there? */ +if (memcmp (spl->spl_signature,SPL_SIGNATURE ,3 ) !=0 ) +return INVALID_SPL_HEADER ; + +if (spl_header_version < req_version) { +printf ("sunxi SPL version mismatch: expected %u, got %u\n" , + req_version, spl_header_version); +return INVALID_SPL_HEADER ; + } + +return spl; +} +int dram_init (void ) { gd->ram_size =get_ram_size ((long *)PHYS_SDRAM_0 ,PHYS_SDRAM_0_SIZE );
@@ -626,16 +650,11 @@ void get_board_serial(struct tag_serialnr *serialnr)*/ static void parse_spl_header (const uint32_t spl_addr) { -struct boot_file_head *spl = (void *)(ulong )spl_addr; -if (memcmp (spl->spl_signature,SPL_SIGNATURE ,3 ) !=0 ) -return ;/* signature mismatch, no usable header */ +struct boot_file_head *spl =get_spl_header (SPL_ENV_HEADER_VERSION ); -uint8_t spl_header_version = spl->spl_signature[3 ]; -if (spl_header_version <SPL_ENV_HEADER_VERSION ) { -printf ("sunxi SPL version mismatch: expected %u, got %u\n" , -SPL_ENV_HEADER_VERSION , spl_header_version); +if (spl ==INVALID_SPL_HEADER )return ; - } +if (!spl->fel_script_address)return ;
@@ -777,11 +796,11 @@ int ft_board_setup(void *blob, bd_t *bd)#ifdef CONFIG_SPL_LOAD_FIT int board_fit_config_name_match (const char *name) { -struct boot_file_head *spl = (void *)(ulong )SPL_ADDR ; -const char *cmp_str = (void *)(ulong )SPL_ADDR ; +struct boot_file_head *spl =get_spl_header (SPL_DT_HEADER_VERSION ); +const char *cmp_str = (const char *)spl;/* Check if there is a DT name stored in the SPL header and use that. */ -if (spl->dt_name_offset) { +if (spl !=INVALID_SPL_HEADER && spl->dt_name_offset) { cmp_str += spl->dt_name_offset; }else {#ifdef CONFIG_DEFAULT_DEVICE_TREE
Signature · 1 line
2.14.1
RFC PATCH 3/3 sunxi: store DRAM size in SPL header
In reply to Andre PrzywaraAt the moment we rely on the infamous get_ram_size() function to learn the actual DRAM size in U-Boot proper. This function has two issues:
1) It only works if the DRAM size is a power of two. We start to see boards which have 3GB of (usable) DRAM, so this does not fit anymore.
2) As U-Boot has no notion of reserved memory so far, it will happily ride through the DRAM, possibly stepping on secure-only memory. This could be a region of DRAM reserved for OP-TEE or some other secure payload, for instance. It will most likely crash in that case.
As the SPL DRAM init routine has very accurate knowledge of the actual DRAM size, lets propagate this wisdom to U-Boot proper.
We re-purpose a currently reserved word in our SPL header for that. The SPL itself stores the detected DRAM size there, and bumps the SPL header version number in that case. U-Boot proper checks for a valid SPL header and a high enough version number, then uses the DRAM size from there. If the SPL header field is not sufficient, we fall back to the old DRAM scanning routine.
--- arch/arm/include/asm/arch-sunxi/spl.h | 3 ++- board/sunxi/board.c | 25 ++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-)
arch/arm/include/asm/arch-sunxi/spl.h +2 -1
@@ -19,6 +19,7 @@#define SPL_ENV_HEADER_VERSION SPL_VERSION(0, 1)#define SPL_DT_HEADER_VERSION SPL_VERSION(0, 2) +#define SPL_DRAM_HEADER_VERSION SPL_VERSION(0, 3)#ifdef CONFIG_SUNXI_HIGH_SRAM #define SPL_ADDR 0x10000
@@ -74,7 +75,7 @@ struct boot_file_head {* to the users. */ uint32_t dt_name_offset;/* since v0.2, set by mksunxiboot */ -uint32_t reserved1; +uint32_t dram_size;/* in MiB, since v0.3, set by SPL */ uint32_t boot_media;/* written here by the boot ROM */ /* A padding area (may be used for storing text strings) */ uint32_t string_pool[13 ];/* since v0.2, filled by mksunxiboot */
board/sunxi/board.c +24 -1
@@ -279,7 +279,13 @@ static struct boot_file_head * get_spl_header(uint8_t req_version)int dram_init (void ) { - gd->ram_size =get_ram_size ((long *)PHYS_SDRAM_0 ,PHYS_SDRAM_0_SIZE ); +struct boot_file_head *spl =get_spl_header (SPL_DRAM_HEADER_VERSION ); + +if (spl ==INVALID_SPL_HEADER ) + gd->ram_size =get_ram_size ((long *)PHYS_SDRAM_0 , +PHYS_SDRAM_0_SIZE ); +else + gd->ram_size = (phys_addr_t )spl->dram_size <<20 ;return 0 ; }
@@ -537,6 +543,21 @@ int board_mmc_init(bd_t *bis)#endif #ifdef CONFIG_SPL_BUILD + +static void sunxi_spl_store_dram_size (phys_addr_t dram_size) +{ +struct boot_file_head *spl =get_spl_header (SPL_DT_HEADER_VERSION ); + +if (spl ==INVALID_SPL_HEADER ) +return ; + +/* Promote the header version for U-Boot proper, if needed. */ +if (spl->spl_signature[3 ] <SPL_DRAM_HEADER_VERSION ) + spl->spl_signature[3 ] =SPL_DRAM_HEADER_VERSION ; + + spl->dram_size = dram_size >>20 ; +} +void sunxi_board_init (void ) {int power_failed =0 ;
@@ -605,6 +626,8 @@ void sunxi_board_init(void)if (!gd->ram_size)hang (); +sunxi_spl_store_dram_size (gd->ram_size); +/* * Only clock up the CPU to full speed if we are reasonably * assured it's being powered with suitable core voltage
Signature · 1 line
2.14.1
cover sunxi: extend SPL header to propagate DRAM size
In reply to Andre PrzywaraQuoted 18 lines · Andre Przywara
This series tries to solve three issues we currently have on Allwinner boards:
- The DRAM sizing routine can only cope with power-of-two sized DRAM.
- The DRAM sizing routine steps through all DRAM, possibly hitting secure memory.
- The SPL header versioning is quite strict and tends to break every time we need to update it.So I thought about introducing something along the lines of semantic versioning[1], where we can add backwards-compatible changes to the SPL header without breaking every tool. This is introduced in the first patch. The second patch does some refactoring, so that the third patch can use the newly gained freedom to store the DRAM size. The SPL knows the DRAM size very well, so we store this in the SPL header, so that U-Boot proper can pick it up from there. This saves the call to
get_ram_size()with its deficiencies.
More information in the respective commit messages.
Quoted 6 lines
I understand that this versioning solution is not fully future-proof, but we have only one byte for the version, and I just wanted to start discussion on this.
There is a corresponding patch for sunxi-tools as well I am posting shortly.
I'm not sure I have a lot of comments to make, this looks sane to me :)
Maxime
Signature · 10 lines
Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180517/5ce8e1f8/attachment.sig>
cover sunxi: extend SPL header to propagate DRAM size
In reply to Andre Przywara于 2018年5月17日 GMT+08:00 下午4:16:58, Andre Przywara <[email protected]> 写到:
Quoted 10 lines
This series tries to solve three issues we currently have on Allwinner boards:
- The DRAM sizing routine can only cope with power-of-two sized DRAM.
- The DRAM sizing routine steps through all DRAM, possibly hitting secure
memory.
- The SPL header versioning is quite strict and tends to break every time
we need to update it.
Quoted 3 lines
So I thought about introducing something along the lines of semantic versioning[1], where we can add backwards-compatible changes to the SPL header without breaking every tool. This is introduced in the first
How to define the "backwards-compatible"?
Should it have a standard? (e.g. tools have no change needed and U-Boot will have fallback code)
Quoted 30 lines
patch.
The second patch does some refactoring, so that the third patch can use the newly gained freedom to store the DRAM size. The SPL knows the DRAM size very well, so we store this in the SPL header, so that U-Boot proper
can pick it up from there. This saves the call toget_ram_size()with its deficiencies.
More information in the respective commit messages.I understand that this versioning solution is not fully future-proof, but
we have only one byte for the version, and I just wanted to start discussion on this.
There is a corresponding patch for sunxi-tools as well I am posting shortly.Cheers,
Andre.Andre Przywara (3):
sunxi: Extend SPL header versioning
sunxi:board.c: refactor SPL header checks
sunxi: store DRAM size in SPL header
arch/arm/include/asm/arch-sunxi/spl.h| 22 ++++++++----board/sunxi/board.c| 66
++++++++++++++++++++++++++++-------2 files changed, 70 insertions(+), 18 deletions(-)
cover sunxi: extend SPL header to propagate DRAM size
Hi,
Quoted 13 lines · Icenowy Zheng
于 2018年5月17日 GMT+08:00 下午4:16:58, Andre Przywara <[email protected]> 写到:
Quoted 10 lines
This series tries to solve three issues we currently have on Allwinner boards:
- The DRAM sizing routine can only cope with power-of-two sized DRAM.
- The DRAM sizing routine steps through all DRAM, possibly hitting secure
memory.
- The SPL header versioning is quite strict and tends to break every time
we need to update it.
Quoted 5 lines
Quoted 3 lines
So I thought about introducing something along the lines of semantic versioning[1], where we can add backwards-compatible changes to the SPL header without breaking every tool. This is introduced in the first
How to define the "backwards-compatible"?
Yeah, that's a good question I was hoping to get some input on. Ideally backwards-compatible means that older tools can cope with the new feature, so for instance anyone not knowing about the DRAM size introduced in v3 would still be happy (either ignoring it or not using it).
Now what's special about our situation is that we have several agents dealing with the SPL header:
- The BootROM puts the boot source in there. That's probably a no-brainer for now, but later BootROMs might behave differently. We might need to update the major version when they do so without changing the magic number.
- mksunxiboot creates the header and puts the version number in there. It populates the DT name field since v2. I kept it at v2 for now, since it's only the SPL populating the v3 DRAM size field.
- sunxi-fel reads the header and adds the FEL script address in there, requiring at least v1. In the moment it insists on the version number being at most 2, which my sunxi-tools patch tries to fix.
- Now the SPL and U-Boot use a field themselves (DRAM size), and I bumped the version if needed.
Quoted 2 lines
Should it have a standard? (e.g. tools have no change needed and U-Boot will have fallback code)
Yeah, that sounds like some sensible definition.
The only problem I see is what happens when we need a v4 feature for mksunxiboot. This would lead U-Boot proper to believe the DRAM size field is valid, even though the SPL might not have populated it. Not sure we care about it, or if there is a better solution (feature bits?).
Cheers,
Andre.
Quoted 30 lines
Quoted 30 lines
patch.
The second patch does some refactoring, so that the third patch can use the newly gained freedom to store the DRAM size. The SPL knows the DRAM size very well, so we store this in the SPL header, so that U-Boot proper
can pick it up from there. This saves the call toget_ram_size()with its deficiencies.
More information in the respective commit messages.I understand that this versioning solution is not fully future-proof, but
we have only one byte for the version, and I just wanted to start discussion on this.
There is a corresponding patch for sunxi-tools as well I am posting shortly.Cheers,
Andre.Andre Przywara (3):
sunxi: Extend SPL header versioning
sunxi:board.c: refactor SPL header checks
sunxi: store DRAM size in SPL header
arch/arm/include/asm/arch-sunxi/spl.h| 22 ++++++++----board/sunxi/board.c| 66
++++++++++++++++++++++++++++-------2 files changed, 70 insertions(+), 18 deletions(-)
cover sunxi: extend SPL header to propagate DRAM size
In reply to Andre PrzywaraQuoted 25 lines · 在 2018-05-17四的 09:16 +0100，Andre Przywara写道：
This series tries to solve three issues we currently have on Allwinner boards:
- The DRAM sizing routine can only cope with power-of-two sized DRAM.
- The DRAM sizing routine steps through all DRAM, possibly hitting secure
memory.
- The SPL header versioning is quite strict and tends to break every time
we need to update it.So I thought about introducing something along the lines of semantic versioning[1], where we can add backwards-compatible changes to the SPL
header without breaking every tool. This is introduced in the first patch.
The second patch does some refactoring, so that the third patch can use
the newly gained freedom to store the DRAM size. The SPL knows the DRAM
size very well, so we store this in the SPL header, so that U-Boot proper
can pick it up from there. This saves the call toget_ram_size()with its deficiencies.
More information in the respective commit messages.
Quoted 8 lines
I understand that this versioning solution is not fully future-proof, but
we have only one byte for the version, and I just wanted to start discussion on this.
There is a corresponding patch for sunxi-tools as well I am posting shortly.
Could I do some small reworks on this patchset and resend it?
We're now facing 3GiB Pine H64 releasing very soon.
Quoted 17 lines
Cheers,
Andre.Andre Przywara (3):
sunxi: Extend SPL header versioning
sunxi:board.c: refactor SPL header checks
sunxi: store DRAM size in SPL headerarch/arm/include/asm/arch-sunxi/spl.h | 22 ++++++++---- board/sunxi/board.c | 66 ++++++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 18 deletions(-)Signature · 1 line
2.14.1
cover sunxi: extend SPL header to propagate DRAM size
Quoted 33 lines · Icenowy Zheng
Quoted 32 lines · 在 2018-05-17四的 09:16 +0100，Andre Przywara写道：
This series tries to solve three issues we currently have on Allwinner boards:
- The DRAM sizing routine can only cope with power-of-two sized DRAM.
- The DRAM sizing routine steps through all DRAM, possibly hitting secure
memory.
- The SPL header versioning is quite strict and tends to break every time
we need to update it.So I thought about introducing something along the lines of semantic versioning[1], where we can add backwards-compatible changes to the SPL
header without breaking every tool. This is introduced in the first patch.
The second patch does some refactoring, so that the third patch can use
the newly gained freedom to store the DRAM size. The SPL knows the DRAM
size very well, so we store this in the SPL header, so that U-Boot proper
can pick it up from there. This saves the call toget_ram_size()with its deficiencies.
More information in the respective commit messages.I understand that this versioning solution is not fully future-proof, but
we have only one byte for the version, and I just wanted to start discussion on this.
There is a corresponding patch for sunxi-tools as well I am posting shortly.
Quoted 3 lines
Quoted 1 line
Could I do some small reworks on this patchset and resend it?
Yes, please. I was thinking about resending it as well.
The only issue is that we need the corresponding patch in sunxi-fel as well, ideally before this one goes in.
But activity was somewhat low on sunxi-tools last time I mentioned this...
Cheers,
Andre.
Quoted 21 lines
We're now facing 3GiB Pine H64 releasing very soon.
Quoted 17 lines
Cheers,
Andre.Andre Przywara (3):
sunxi: Extend SPL header versioning
sunxi:board.c: refactor SPL header checks
sunxi: store DRAM size in SPL headerarch/arm/include/asm/arch-sunxi/spl.h | 22 ++++++++---- board/sunxi/board.c | 66 ++++++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 18 deletions(-)Signature · 1 line
2.14.1
cover sunxi: extend SPL header to propagate DRAM size
Quoted 46 lines · 在 2018-10-22一的 09:17 +0100，André Przywara写道：
Quoted 45 lines · Icenowy Zheng
Quoted 44 lines · 在 2018-05-17四的 09:16 +0100，Andre Przywara写道：
This series tries to solve three issues we currently have on Allwinner boards:
- The DRAM sizing routine can only cope with power-of-two sized DRAM.
- The DRAM sizing routine steps through all DRAM, possibly hitting
secure
memory.
- The SPL header versioning is quite strict and tends to break every
time
we need to update it.So I thought about introducing something along the lines of semantic
versioning[1], where we can add backwards-compatible changes to the
SPL
header without breaking every tool. This is introduced in the first
patch.
The second patch does some refactoring, so that the third patch can
use
the newly gained freedom to store the DRAM size. The SPL knows the
DRAM
size very well, so we store this in the SPL header, so that U- Boot
proper
can pick it up from there. This saves the call toget_ram_size()with
its deficiencies.
More information in the respective commit messages.I understand that this versioning solution is not fully future- proof,
but
we have only one byte for the version, and I just wanted to start discussion on this.
There is a corresponding patch for sunxi-tools as well I am posting
shortly.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Could I do some small reworks on this patchset and resend it?
Yes, please. I was thinking about resending it as well. The only issue is that we need the corresponding patch in sunxi-fel as
well, ideally before this one goes in.
I think technically this patchset should be merged BEFORE the sunxi-fel patch, because there's no document, and U-Boot works as the document.
Quoted 28 lines
But activity was somewhat low on sunxi-tools last time I mentioned this...
Cheers,
Andre.Quoted 19 lines
We're now facing 3GiB Pine H64 releasing very soon.
Quoted 17 lines
Cheers,
Andre.Andre Przywara (3):
sunxi: Extend SPL header versioning
sunxi:board.c: refactor SPL header checks
sunxi: store DRAM size in SPL headerarch/arm/include/asm/arch-sunxi/spl.h | 22 ++++++++---- board/sunxi/board.c | 66 ++++++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 18 deletions(-)Signature · 1 line
2.14.1