All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.