| Message ID | 1441199831-25541-1-git-send-email-bernhard.nortmann@web.de |
|---|---|
| State | Accepted |
| Delegated to: | Hans de Goede |
| Headers | show |
On Wed, 2 Sep 2015 15:17:11 +0200 Bernhard Nortmann <bernhard.nortmann@web.de> wrote: > This patch follows up on a discussion of ways to improve support > for the sunxi FEL ("USB boot") mechanism, especially with regard > to boot scripts, see: > https://groups.google.com/d/msg/linux-sunxi/wBEGUoLNRro/rHGq6nSYCQAJ > > The idea is to convert the (currently unused) "pad" bytes in the > SPL header into an area where data can be passed to U-Boot. To > do this safely, we have to make sure that we're actually using > our "sunxi" flavor of the SPL, and not the Allwinner boot0. > > The modified mksunxiboot introduces a special signature to the > SPL header in place of the "pub_head_size" field. This can be > used to reliably distinguish between compatible versions of sunxi > SPL and anything else (older variants or Allwinner's boot0). > > Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de> Thanks for starting the work on improving this stuff. Do I understand it right that this patch is supposed to be a part of a bigger set, implementing some useful functionality in U-Boot? Probably it would be best if these patches are submitted, reviewed and applied together. > --- > > tools/mksunxiboot.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/tools/mksunxiboot.c b/tools/mksunxiboot.c > index 3361251..58bc5c7 100644 > --- a/tools/mksunxiboot.c > +++ b/tools/mksunxiboot.c > @@ -27,12 +27,31 @@ struct boot_file_head { > * by the boot ROM. To be compatible with Allwinner tools we > * would need to implement the proper fields here instead of > * padding. > + * > + * Actually we want the ability to recognize our "sunxi" variant > + * of the SPL. To do so, let's place a special signature into the > + * "pub_head_size" field. We can reasonably expect Allwinner's > + * boot0 to always have the upper 16 bits of this set to 0 (after > + * all the value shouldn't be larger than the limit imposed by > + * SRAM size). > + * If the signature is present (at 0x14), then we know it's safe > + * to use the remaining 8 bytes (at 0x18) for our own purposes. > + * (E.g. sunxi-tools "fel" utility can pass information there.) > + * > + * The overall header size still sums up to 32 bytes. > */ > - uint8_t pad[12]; /* align to 32 bytes */ > + union { > + uint32_t pub_head_size; > + uint8_t spl_signature[4]; > + }; > + uint32_t fel_data_address; > + uint32_t fel_data_size; Do these new fields imply that the actual fel data is supposed to be stored in some other place? We may have troubles finding/allocating this other place in such a way that it does not clash with anything else. If you need more space in the SPL header for the fel data, then it can be easily increased from 32 bytes to something bigger. In order to do this, we can change CONFIG_SPL_TEXT_BASE in http://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/sunxi-common.h;h=1abf73c311792b21f2fa40857b5f11e43fbd76cc#l203 and update the first branch instruction in the SPL header. But I thought that we initially only wanted to have something like this at the end of the SPL header: uint32_t bootscript_address; uint8_t pad[4]; Where the 'bootscript_address' would be a pointer to the 'boot.scr' data, uploaded by the 'fel' tool to DRAM after executing the SPL and before executing the main U-Boot part. > }; > > #define BOOT0_MAGIC "eGON.BT0" > #define STAMP_VALUE 0x5F0A6C39 > +#define SPL_SIGNATURE "SPL" /* marks "sunxi" header */ > +#define SPL_HEADER_VERSION 1 > > /* check sum functon from sun4i boot code */ > int gen_check_sum(struct boot_file_head *head_p) > @@ -133,6 +152,12 @@ int main(int argc, char *argv[]) > ALIGN(file_size + sizeof(struct boot_file_head), BLOCK_SIZE); > img.header.b_instruction = cpu_to_le32(img.header.b_instruction); > img.header.length = cpu_to_le32(img.header.length); > + > + memcpy(img.header.spl_signature, SPL_SIGNATURE, 3); /* "sunxi" marker */ > + img.header.spl_signature[3] = SPL_HEADER_VERSION; > + img.header.fel_data_address = 0; /* ensure fields are zeroed */ > + img.header.fel_data_size = 0; > + > gen_check_sum(&img.header); > > count = write(fd_out, &img, le32_to_cpu(img.header.length));
Am 02.09.2015 um 16:51 schrieb Siarhei Siamashka: > On Wed, 2 Sep 2015 15:17:11 +0200 > Bernhard Nortmann <bernhard.nortmann@web.de> wrote: > >> This patch follows up on a discussion of ways to improve support >> for the sunxi FEL ("USB boot") mechanism, especially with regard >> to boot scripts, see: >> https://groups.google.com/d/msg/linux-sunxi/wBEGUoLNRro/rHGq6nSYCQAJ >> >> The idea is to convert the (currently unused) "pad" bytes in the >> SPL header into an area where data can be passed to U-Boot. To >> do this safely, we have to make sure that we're actually using >> our "sunxi" flavor of the SPL, and not the Allwinner boot0. >> >> The modified mksunxiboot introduces a special signature to the >> SPL header in place of the "pub_head_size" field. This can be >> used to reliably distinguish between compatible versions of sunxi >> SPL and anything else (older variants or Allwinner's boot0). >> >> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de> > Thanks for starting the work on improving this stuff. > > Do I understand it right that this patch is supposed to be a part of > a bigger set, implementing some useful functionality in U-Boot? > Probably it would be best if these patches are submitted, reviewed > and applied together. Yes, this patch alone would only be a step in preparation. We'd need at least one more change within U-Boot itself (and another one to the "fel" utility, but that's sunxi-tools) to make actual use of any information provided by the FEL boot process. As you suggested this will probably involve setting specific environment variables in misc_init_r(). I agree that this should probably make a series of patches, but still appreciate early feedback on it. Perhaps it would have been better if I had made this "[RFC]" first... > >> --- >> >> tools/mksunxiboot.c | 27 ++++++++++++++++++++++++++- >> 1 file changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/tools/mksunxiboot.c b/tools/mksunxiboot.c >> index 3361251..58bc5c7 100644 >> --- a/tools/mksunxiboot.c >> +++ b/tools/mksunxiboot.c >> @@ -27,12 +27,31 @@ struct boot_file_head { >> * by the boot ROM. To be compatible with Allwinner tools we >> * would need to implement the proper fields here instead of >> * padding. >> + * >> + * Actually we want the ability to recognize our "sunxi" variant >> + * of the SPL. To do so, let's place a special signature into the >> + * "pub_head_size" field. We can reasonably expect Allwinner's >> + * boot0 to always have the upper 16 bits of this set to 0 (after >> + * all the value shouldn't be larger than the limit imposed by >> + * SRAM size). >> + * If the signature is present (at 0x14), then we know it's safe >> + * to use the remaining 8 bytes (at 0x18) for our own purposes. >> + * (E.g. sunxi-tools "fel" utility can pass information there.) >> + * >> + * The overall header size still sums up to 32 bytes. >> */ >> - uint8_t pad[12]; /* align to 32 bytes */ >> + union { >> + uint32_t pub_head_size; >> + uint8_t spl_signature[4]; >> + }; >> + uint32_t fel_data_address; >> + uint32_t fel_data_size; > Do these new fields imply that the actual fel data is supposed to > be stored in some other place? We may have troubles finding/allocating > this other place in such a way that it does not clash with anything > else. > > If you need more space in the SPL header for the fel data, then it can > be easily increased from 32 bytes to something bigger. In order to do > this, we can change CONFIG_SPL_TEXT_BASE in > http://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/sunxi-common.h;h=1abf73c311792b21f2fa40857b5f11e43fbd76cc#l203 > and update the first branch instruction in the SPL header. > > But I thought that we initially only wanted to have something like this > at the end of the SPL header: > > uint32_t bootscript_address; > uint8_t pad[4]; > > Where the 'bootscript_address' would be a pointer to the 'boot.scr' > data, uploaded by the 'fel' tool to DRAM after executing the SPL > and before executing the main U-Boot part. Right. I just wanted something more "generic" and currently didn't see a reason to limit the information to the address only. It's not supposed to introduce another location (or level of 'indirection'), i.e. in the use case we have in mind for FEL, the fel_data_address would "be" (= point to) the boot script. I'm simply striving for a little bit of extra flexibility here. So far, we've mainly been considering boot.scr (with its dedicated header), but I could image this "FEL transport" mechanism also to be useful for passing other information (of possibly different nature) from the "fel" utility. fel_data_size would likely be optional (and default to 0). For example a user might wish to upload uEnv.txt-style data. The "fel" could be modified to support an "env" command that transfers this data, and then sets fel_data_address and fel_data_size accordingly - allowing U-Boot to simply retrieve modifications to the default environment by doing something like "import -t ${fel_data_addr} ${fel_data_size}". Regards, B. Nortmann
Hi, On 02-09-15 15:17, Bernhard Nortmann wrote: > This patch follows up on a discussion of ways to improve support > for the sunxi FEL ("USB boot") mechanism, especially with regard > to boot scripts, see: > https://groups.google.com/d/msg/linux-sunxi/wBEGUoLNRro/rHGq6nSYCQAJ > > The idea is to convert the (currently unused) "pad" bytes in the > SPL header into an area where data can be passed to U-Boot. To > do this safely, we have to make sure that we're actually using > our "sunxi" flavor of the SPL, and not the Allwinner boot0. > > The modified mksunxiboot introduces a special signature to the > SPL header in place of the "pub_head_size" field. This can be > used to reliably distinguish between compatible versions of sunxi > SPL and anything else (older variants or Allwinner's boot0). > > Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de> > --- > > tools/mksunxiboot.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/tools/mksunxiboot.c b/tools/mksunxiboot.c > index 3361251..58bc5c7 100644 > --- a/tools/mksunxiboot.c > +++ b/tools/mksunxiboot.c > @@ -27,12 +27,31 @@ struct boot_file_head { > * by the boot ROM. To be compatible with Allwinner tools we > * would need to implement the proper fields here instead of > * padding. > + * > + * Actually we want the ability to recognize our "sunxi" variant > + * of the SPL. To do so, let's place a special signature into the > + * "pub_head_size" field. We can reasonably expect Allwinner's > + * boot0 to always have the upper 16 bits of this set to 0 (after > + * all the value shouldn't be larger than the limit imposed by > + * SRAM size). > + * If the signature is present (at 0x14), then we know it's safe > + * to use the remaining 8 bytes (at 0x18) for our own purposes. > + * (E.g. sunxi-tools "fel" utility can pass information there.) > + * > + * The overall header size still sums up to 32 bytes. > */ > - uint8_t pad[12]; /* align to 32 bytes */ > + union { > + uint32_t pub_head_size; > + uint8_t spl_signature[4]; > + }; > + uint32_t fel_data_address; > + uint32_t fel_data_size; I believe these 2 should be renamed to: uint32_t fel_boot_script_address; uint32_t fel_boot_script_size; To properly reflect what they are (they are not some abstract data, they are specifically a boot.scr image) With that changed and Siarhei's ack for the series (I'm assuming Sairhei will take care of the sunxi-tools side of things), I'm ok with merging this. Regards, Hans I > }; > > #define BOOT0_MAGIC "eGON.BT0" > #define STAMP_VALUE 0x5F0A6C39 > +#define SPL_SIGNATURE "SPL" /* marks "sunxi" header */ > +#define SPL_HEADER_VERSION 1 > > /* check sum functon from sun4i boot code */ > int gen_check_sum(struct boot_file_head *head_p) > @@ -133,6 +152,12 @@ int main(int argc, char *argv[]) > ALIGN(file_size + sizeof(struct boot_file_head), BLOCK_SIZE); > img.header.b_instruction = cpu_to_le32(img.header.b_instruction); > img.header.length = cpu_to_le32(img.header.length); > + > + memcpy(img.header.spl_signature, SPL_SIGNATURE, 3); /* "sunxi" marker */ > + img.header.spl_signature[3] = SPL_HEADER_VERSION; > + img.header.fel_data_address = 0; /* ensure fields are zeroed */ > + img.header.fel_data_size = 0; > + > gen_check_sum(&img.header); > > count = write(fd_out, &img, le32_to_cpu(img.header.length)); >
Hello Hans! Thanks for looking into this. Am 10.09.2015 um 20:29 schrieb Hans de Goede: > > I believe these 2 should be renamed to: > > uint32_t fel_boot_script_address; > uint32_t fel_boot_script_size; > > To properly reflect what they are (they are not some abstract > data, they are specifically a boot.scr image) > > With that changed and Siarhei's ack for the series (I'm assuming > Sairhei will take care of the sunxi-tools side of things), > I'm ok with merging this. > > Regards, > > Hans Sure, I can rename them accordingly. With Siarhei's change to clear the entire image area upfront, setting individual header fields to zero will also be no longer required. Regards, B. Nortmann
diff --git a/tools/mksunxiboot.c b/tools/mksunxiboot.c index 3361251..58bc5c7 100644 --- a/tools/mksunxiboot.c +++ b/tools/mksunxiboot.c @@ -27,12 +27,31 @@ struct boot_file_head { * by the boot ROM. To be compatible with Allwinner tools we * would need to implement the proper fields here instead of * padding. + * + * Actually we want the ability to recognize our "sunxi" variant + * of the SPL. To do so, let's place a special signature into the + * "pub_head_size" field. We can reasonably expect Allwinner's + * boot0 to always have the upper 16 bits of this set to 0 (after + * all the value shouldn't be larger than the limit imposed by + * SRAM size). + * If the signature is present (at 0x14), then we know it's safe + * to use the remaining 8 bytes (at 0x18) for our own purposes. + * (E.g. sunxi-tools "fel" utility can pass information there.) + * + * The overall header size still sums up to 32 bytes. */ - uint8_t pad[12]; /* align to 32 bytes */ + union { + uint32_t pub_head_size; + uint8_t spl_signature[4]; + }; + uint32_t fel_data_address; + uint32_t fel_data_size; }; #define BOOT0_MAGIC "eGON.BT0" #define STAMP_VALUE 0x5F0A6C39 +#define SPL_SIGNATURE "SPL" /* marks "sunxi" header */ +#define SPL_HEADER_VERSION 1 /* check sum functon from sun4i boot code */ int gen_check_sum(struct boot_file_head *head_p) @@ -133,6 +152,12 @@ int main(int argc, char *argv[]) ALIGN(file_size + sizeof(struct boot_file_head), BLOCK_SIZE); img.header.b_instruction = cpu_to_le32(img.header.b_instruction); img.header.length = cpu_to_le32(img.header.length); + + memcpy(img.header.spl_signature, SPL_SIGNATURE, 3); /* "sunxi" marker */ + img.header.spl_signature[3] = SPL_HEADER_VERSION; + img.header.fel_data_address = 0; /* ensure fields are zeroed */ + img.header.fel_data_size = 0; + gen_check_sum(&img.header); count = write(fd_out, &img, le32_to_cpu(img.header.length));
This patch follows up on a discussion of ways to improve support for the sunxi FEL ("USB boot") mechanism, especially with regard to boot scripts, see: https://groups.google.com/d/msg/linux-sunxi/wBEGUoLNRro/rHGq6nSYCQAJ The idea is to convert the (currently unused) "pad" bytes in the SPL header into an area where data can be passed to U-Boot. To do this safely, we have to make sure that we're actually using our "sunxi" flavor of the SPL, and not the Allwinner boot0. The modified mksunxiboot introduces a special signature to the SPL header in place of the "pub_head_size" field. This can be used to reliably distinguish between compatible versions of sunxi SPL and anything else (older variants or Allwinner's boot0). Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de> --- tools/mksunxiboot.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)