public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Bernhard Nortmann <bernhard.nortmann@web.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] sunxi: Add the ability to pass (script) filesize in the SPL header
Date: Tue, 7 Jun 2016 16:09:58 +0200	[thread overview]
Message-ID: <5756D5B6.8050201@web.de> (raw)
In-Reply-To: <20160606122026.2cb26778@i7>

Hello Siarhei!

Am 06.06.2016 um 11:20 schrieb Siarhei Siamashka:
> On Sun, 5 Jun 2016 15:01:30 +0200
> Bernhard Nortmann <bernhard.nortmann@web.de> wrote:
>
>> Hi Siarhei!
>>
>> [...]
>>
>> No, you're right and not missing anything. Setting ${filesize} alone
>> doesn't achieve much, and would require further customization to do the
>> actual import. Since this whole idea of having the script length didn't
>> go down too well when I initially proposed it (back in September 2015),
>> I stayed away from adding additional U-Boot modifications this time.
> Maybe you can add the necessary changes to the U-Boot default
> environment in the v2 patch? Either way, we are not going to
> make any progress until it is feature complete and usable.

Back in 2015 Hans expressed concerns about overcomplicating what is already
an exotic use-case. That is why I wanted to keep v1 as simple and 'generic'
as possible - working from the assumption that if a user is sufficiently
advanced to get fel_script_length set, (s)he'd also be proficient enough to
tailor U-Boot to make actual use of ${filesize} according to his/her needs.

Changing the default environment to use the existing "import -t" command
was just one example of what might be done - maybe there could be other
future uses, which I currently haven't thought of. My basic goal was and is
a way to pass a payload from sunxi-fel to U-Boot in a format-agnostic way.
This requires a "length" / file size in addition to the memory address.

The discussion here seems to narrow down on "uEnv.txt style" usage now.
That's okay for me - I can create a v2 which focuses on that, and fleshes
out the needed details.

>
>> One approach would be to have a modified "bootcmd_fel" that either tests
>> for a non-empty ${filesize}, or tries to import the script data as a
>> fallback ("source ${fel_scriptaddr} || import -t <...>;"). Another way is
>> to always assume "uEnv.txt style" whenever fel_script_length is set, and
>> do a himport_r() of the script data right away (the programmatic equivalent
>> of "import -t"). I'm doing this in an experimental branch of mine, as it
>> allows overriding everything from the default environment - including the
>> "bootcmd" itself.
>>
>> Of course, all of this also requires support from the sunxi-fel side
>> of things (i.e. fel_script_length getting set in the first place). A
>> quick-and-dirty solution I'm currently using is to assume a uEnv.txt
>> script (and set fel_script_length) whenever sunxi-fel detects uploads of
>> pure ASCII data.
> The boot.scr file is nice because it has its own format and a magic
> signature. The uEnv.txt is difficult to recognize automatically, but
> maybe we can borrow the https://en.wikipedia.org/wiki/Shebang_%28Unix%29
> approach used by scripting languages?
>
> I mean, we can require that the first line of uEnv.txt is a comment in a
> special format. This can be described in the sunxi-tools documentation.
> And also the sunxi-fel tool could print a warning if it detects upload
> of pure ASCII data from a file with "uEnv" part in the name and ".txt"
> as the extension.

I dislike involving filename conventions here, and I'd also prefer not
having to "tag" the uEnv-style files (which are basically <key>=<value>
pairs) with a special marker. If sunxi-fel requires 'extra work' to
recognise these files, we might just as well use a dedicated command
for uploading them - e.g. "env" instead of "write". This command could
also do sanity checks, like issue a warning if the file contains non-ASCII
data or fails to meet the expected syntax.

>
>> This could also be done easily with a dedicated command,
>> and I can even image sunxi-fel building the uEnv.txt string itself from
>> given ("env") key/value pairs.
>>
>>> I have no real opinion about this, but "filesize" looks like a
>>> rather generic name for this environment variable. Are there some
>>> advantages/disadvantages picking this particular name instead
>>> of something like "fel_scriptsize"?
>> Well... this _is_ the generic name that U-Boot itself tends to use for
>> data transfers. E.g. "load" or "tftp" commands set the ${filesize} too.
> So you are suggesting to pretend that there was a "load" command
> executed for "uEnv.txt" right before running the code from the default
> environment? This seems to be rather fragile and does not look like
> it offers any real advantages over "fel_scriptsize".

It's based on the paradigm that any kind of data transfer (might) provide
exactly this kind of information in ${filesize}. U-Boot users know this
from a variety of "load" commands (Ymodem anyone?) - so if you like: yes,
I'm pretending that something similar happened. Of course I can rename it
to anything that you prefer. But if we're taking the uEnv route, we might
easily do away with setting any dedicated environment variable at all
(see below).

>
>>> That said, I have no objections to supporting "uEnv.txt" for FEL boot,
>>> as long as it works correctly and does not regress the existing
>>> "boot.scr" support.
>>>   
>> Our sunxi-fel utility is already testing for the script.bin format
>> and can preserve the existing functionality by simply forcing
>> fel_script_length to 0 in that case. And additional safeguards might
>> be placed before "actively" setting that value to anything non-zero.
> So it would serve both as the uEnv.txt length and also as the format
> type indicator (boot.scr or uEnv.txt)? This is probably okay if it is
> documented properly.
>

It would be trival for sunxi-fel to pass the size of .scr files, too -
there's just no point in doing so, since this information is already
redundant (available from the header anyway). Setting the field to 0 seemed
natural to me, and it also reflects what existing versions of the sunxi-fel
utility would use (as they only know about setting fel_script_addr).

Actually that might be the right idea to take the whole thing forward, if
I modify some of my working assumptions accordingly:

* Redefine "fel_script_length" as "fel_env_size", and associate a 'format
   type indicator' meaning with it. Any non-zero value will also require the
   fel_script_addr to be specified, and signals uEnv-style data suitable for
   "import -t" (i.e. ASCII text with <key>=<value> lines).

* (fel_env_size == 0 && fel_script_addr != 0) means that U-Boot is safe to
   assume a .scr format was passed. This reflects and preserves the previous
   use case and existing sunxi-fel implementations. New sunxi-fel versions
   will make sure to pass (fel_env_size == 0) whenever they detect the
   transfers of a mkimage-type script.

* sunxi-fel will pass a non-zero fel_env_size if (and only if) requirements
   and/or safety checks are met, in a way that makes it safe for U-Boot to
   rely on the assumption of uEnv-style format. This may range from simple
   user request ("env" command) to actual syntax validation.

* If both fel_script_addr and fel_env_size are non-zero, U-Boot will
   auto-import the data right away upon start, and afterwards present a
   modified environment (merge of the default environment with anything the
   'script' sets/overrides). This eliminates the need to further modify
   default env settings (e.g. sneak in "import -t" to some bootcmd) and also
   avoids setting dedicated environment variables just for this purpose.

Regards, B. Nortmann

  reply	other threads:[~2016-06-07 14:09 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-14  1:13 [U-Boot] [PATCH v2] sunxi: Increase SPL header size to 64 bytes to avoid code corruption Siarhei Siamashka
2016-05-15 10:04 ` Hans de Goede
2016-05-16  9:56 ` Bernhard Nortmann
2016-05-16 17:52   ` Hans de Goede
2016-06-02 14:57     ` Siarhei Siamashka
2016-06-03  7:30       ` Bernhard Nortmann
2016-06-03 10:53         ` Hans de Goede
2016-06-04 17:12           ` [U-Boot] [PATCH] sunxi: Add the ability to pass (script) filesize in the SPL header Bernhard Nortmann
2016-06-05 11:44             ` Siarhei Siamashka
2016-06-05 13:01               ` Bernhard Nortmann
2016-06-06  9:20                 ` Siarhei Siamashka
2016-06-07 14:09                   ` Bernhard Nortmann [this message]
2016-06-07 17:14                     ` Siarhei Siamashka
2016-06-08 18:23             ` [U-Boot] [PATCH v2] sunxi: Add the ability to recognize and auto-import uEnv-style data Bernhard Nortmann
2016-06-08 20:13               ` Hans de Goede
2016-06-08 21:29                 ` Bernhard Nortmann
2016-06-09  0:14                 ` Siarhei Siamashka
2016-06-09  5:37                   ` [U-Boot] [PATCH v3] sunxi: FEL - " Bernhard Nortmann
2016-06-10 19:31                     ` [U-Boot] [U-Boot, " Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5756D5B6.8050201@web.de \
    --to=bernhard.nortmann@web.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox