[RFC PATCH v8 00/23] Modernize U-Boot shell

Francis Laniel francis.laniel at amarulasolutions.com
Sat May 13 23:42:20 CEST 2023


Le samedi 13 mai 2023, 10:54:10 WEST Peter Robinson a écrit :
> On Fri, May 12, 2023 at 9:04 PM Francis Laniel
> 
> <francis.laniel at amarulasolutions.com> wrote:
> > Hi.
> > 
> > 
> > During 2021 summer, Sean Anderson wrote a contribution to add a new shell,
> > based on LIL, to U-Boot [1, 2].
> > While one of the goals of this contribution was to address the fact actual
> > U-Boot shell, which is based on Busybox hush, is old there was a
> > discussion
> > about adding a new shell versus updating the actual one [3, 4].
> > 
> > So, in this series, with Harald Seiler, we updated the actual U-Boot shell
> > to reflect what is currently in Busybox source code.
> > Basically, this contribution is about taking a snapshot of Busybox
> > shell/hush.c file (as it exists in commit 37460f5da) and adapt it to suit
> > U-Boot needs.
> > 
> > This contribution was written to be as backward-compatible as possible to
> > avoid breaking the existing.
> > So, the 2021 hush flavor offers the same as the actual, that is to say:
> > 1. Variable expansion.
> > 2. Instruction lists (;, && and ||).
> > 3. If, then and else.
> > 4. Loops (for, while and until).
> > No new features offered by Busybox hush were implemented (e.g. functions).
> 
> What sort of impact does this have on firmware sizes?

Without any numbers, I would have said this increase it as I had to use 
CONFIG_LTO for some boards to be inside the board limit.

So, I compiled all amlogic boards with and without the new parser:
odroid-c4;796;800;4;0.503
libretech-s905d-pc;764;768;4;0.524
beelink-gsking-x;796;796;0;0
bananapi-m2s;816;816;0;0
beelink-gtking;796;800;4;0.503
odroid-n2l;784;788;4;0.51
libretech-cc_v2;740;744;4;0.541
beelink-gt1-ultimate;736;740;4;0.543
odroid-hc4;852;856;4;0.469
p200;592;596;4;0.676
sei510;880;884;4;0.455
khadas-vim3l_android_ab;972;976;4;0.412
khadas-vim3;876;880;4;0.457
libretech-s912-pc;764;768;4;0.524
s400;692;696;4;0.578
libretech-cc;728;732;4;0.549
jethub_j80;712;716;4;0.562
wetek-play2;696;700;4;0.575
sei610;884;884;0;0
wetek-core2;748;752;4;0.535
khadas-vim3l;872;876;4;0.459
khadas-vim3l_android;968;972;4;0.413
khadas-vim3_android;968;972;4;0.413
jethub_j100;728;732;4;0.549
khadas-vim2;712;716;4;0.562
p201;592;592;0;0
radxa-zero2;768;772;4;0.521
bananapi-m5;796;800;4;0.503
p212;644;648;4;0.621
khadas-vim3_android_ab;972;976;4;0.412
u200;732;736;4;0.546
nanopi-k2;596;600;4;0.671
khadas-vim;684;688;4;0.585
libretech-ac;760;764;4;0.526
wetek-hub;696;700;4;0.575
odroid-go-ultra;752;756;4;0.532
odroid-c2;696;700;4;0.575
bananapi-m2-pro;788;792;4;0.508
bananapi-cm4-cm4io;820;824;4;0.488
odroid-n2;808;812;4;0.495
radxa-zero;772;772;0;0
beelink-gtkingpro;796;800;4;0.503

I tried to pretty everything with "column" but when I paste the output in my 
mailer this is not pretty, so I prefer to paste the CSV.
Here are the explanations for the columns:
1. The first indicates the board name.
2. The second indicates u-boot.bin size in K (given by "du -sh") compiled for 
old parser.
3. The third indicates u-boot.bin size in K (given by "du -sh") compiled for 
new parser.
4. The fourth indicates the absolute difference, so third column minus second 
one.
5. The fifth indicates the relative difference in percent, rounded with 10^-3 
precision.

The sample is indeed not representative of all u-boot boards as I compiled 
only one family.
But we can see that the increase is in the worst case of 4K, which seems 
acceptable with regard to the original size.

> > It is possible to change the parser at runtime using the "cli" command:
> > => cli print
> > old
> > => cli set 2021
> > => cli print
> > 2021
> > => cli set old
> > The default parser is the old one.
> > Note that to use both parser, you would need to set both
> > CONFIG_HUSH_2021_PARSER and CONFIG_HUSH_OLD_PARSER.
> > 
> > In terms of testing, new unit tests were added to ut to ensure the new
> > behavior is the same as the old one and it does not add regression.
> > Nonetheless, if old behavior was buggy and fixed upstream, the fix is then
> > added to U-Boot [5].
> > In sandbox, all of these tests pass smoothly:
> > => printenv board
> > board=sandbox
> > => ut hush
> > Running 20 hush tests
> > ...
> > Failures: 0
> > => parser set 2021
> > => ut hush
> > Running 20 hush tests
> > ...
> > Failures: 0
> > 
> > Thanks to the effort of Harald Seiler, I was successful booting a board:
> > => printenv fdtfile
> > fdtfile=amlogic/meson-gxl-s905x-libretech-cc.dtb
> > => cli get
> > old
> > => boot
> > ...
> > root at lepotato:~#
> > root at lepotato:~# reboot
> > ...
> > => cli set 2021
> > => cli get
> > 2021
> > => printenv fdtfile
> > fdtfile=amlogic/meson-gxl-s905x-libretech-cc.dtb
> > => boot
> > ...
> > root at lepotato:~#
> > 
> > I was able to have the CI passes but I had to not use
> > CONFIG_HUSH_2021_PARSER for the keymile board.
> > Indeed, the keymile board family is the only set of boards to call
> > get_local_var(), set_local_var() and unset_local_var().
> > Sadly, these functions are static in this contribution.
> > I could have change all of them to introduce code like this:
> > *_local_var(/*...*/)
> > {
> > 
> >         if (gd->flags & GD_FLG_HUSH_OLD_PARSER)
> >         
> >                 return *_local_var_old(/*...*/);
> >         
> >         if (gd->flags & GD_FLG_HUSH_2021_PARSER)
> >         
> >                 return *_local_var_2021(/*...*/);
> > 
> > }
> > But this would have mean renaming all old hush functions calls and I did
> > not want to change the old hush particularly to avoid breaking things.
> > Instead, I change the keymile board to use environment variable instead
> > of local ones.
> > I think this particularities can be addressed in future works.
> > 
> > I also had to enable CONFIG_LTO for kirkwoord sheevaplug and phytec bk4r1,
> > so they do not hit their limits.
> > 
> > For all these reasons, I marked this contribution as RFC to indeed collect
> > your opinions.
> > My goal is not to change suddenly actual shell to this one, we clearly
> > need a transition period to think about it.
> > I think it is better to see this contribution as a proof of concept which
> > shows it is possible to update the actual shell.
> > 
> > If you want to review it - your review will really be appreciated - here
> > are some information regarding the commits:
> > * commits marked as "test:" deal with unit tests.
> > * commit "cli: Add Busybox upstream hush.c file." copies Busybox
> > shell/hush.c into U-Boot tree, this explain why this commit contains
> > around 12000 additions. * commit "cli: Port Busybox 2021 hush to U-Boot."
> > modifies previously added file to permit us to use this as new shell.
> > The really good idea of #include'ing Busybox code into a wrapper file to
> > define some particular functions while minimizing modifications to
> > upstream code comes from Harald Seiler.
> > * commit "cmd: Add new parser command" adds a new command which permits
> > selecting parser at runtime.
> > I am not really satisfied with the fact it calls cli_init() and cli_loop()
> > each time the parser is set, so your reviews would be welcomed.
> > * Other commits focus on enabling features we need (e.g. if).
> > 
> > Changes since:
> >  v2:
> >   * Added a small fix to compile sandbox with NO_SDL=1.
> >   * Added a command to change parser at runtime.
> >   * Added 2021 parser function to all run_command*().
> >  
> >  v3:
> >   * Various bug fixes pointed by the CI.
> >   * Added upstream busybox hush commits until 6th February 2022.
> >  
> >  v4:
> >   * Various cleaning.
> >   * Modified python test to accept failure output when the test are
> >   designed to fail.
> >   * Bumped upstream busybox hush commits until 24h March 2022.
> >  
> >  v5:
> >   * Bumped upstream busybox hush commits until 30th January 2023.
> >   * Fix how hush interprets '<' and '>', indeed we needed to escape them
> >   but I removed this behavior as tests are handled by test command and
> >   not hush itself. This permitted to have the ut fdt to pass.
> >   * Fix a problem with how exit was handled. This was reported by the ut
> >   exit
> >   test.
> >  
> >  v6:
> >   * There was no v6 and I got mixed up with version.
> >  
> >  v7:
> >   * Bumped upstream busybox hush commits until 9th May 2023.
> >   * Renamed parser command to change parser at runtime to cli and added
> >   documentation.
> >   * Added better separation of patches.
> >   * Removed code about __gnu_thumb1_case_si as it was merged in another
> >   series. * Various cleaning.
> > 
> > Francis Laniel (23):
> >   test: Add framework to test hush behavior
> >   test: hush: Test hush if/else
> >   test/py: hush_if_test: Remove the test file
> >   test: hush: Test hush variable expansion
> >   test: hush: Test hush commands list
> >   test: hush: Test hush loops
> >   cli: Add Busybox upstream hush.c file
> >   cli: Port Busybox 2021 hush to U-Boot
> >   cli: Add menu for hush parser
> >   global_data.h: add GD_FLG_HUSH_OLD_PARSER flag
> >   cmd: Add new cli command
> >   cli: Enables using hush 2021 parser as command line parser
> >   cli: hush_2021: Enable variables expansion for hush 2021
> >   cli: hush_2021: Add functions to be called from run_command()
> >   cli: add hush 2021 as parser for run_command*()
> >   test: hush: Fix instructions list tests for hush 2021
> >   test: hush: Fix variable expansion tests for hush 2021
> >   cli: hush_2021: Enable using < and > as string compare operators
> >   cli: hush_2021: Enable if keyword
> >   cli: hush_2021: Enable loops
> >   test: hush: Fix loop tests for hush 2021
> >   cli: hush_2021: Add upstream commits up to 9th May 2023.
> >   DO NOT MERGE: only to make CI happy
> >  
> >  cmd/Kconfig                        |    22 +
> >  cmd/Makefile                       |     2 +
> >  cmd/cli.c                          |   125 +
> >  common/Makefile                    |     3 +-
> >  common/cli.c                       |    82 +-
> >  common/cli_hush_2021.c             |   306 +
> >  common/cli_hush_upstream.c         | 13021 +++++++++++++++++++++++++++
> >  configs/sheevaplug_defconfig       |     1 +
> >  doc/usage/cmd/cli.rst              |    74 +
> >  doc/usage/index.rst                |     1 +
> >  include/asm-generic/global_data.h  |     8 +
> >  include/cli_hush.h                 |    51 +-
> >  include/test/hush.h                |    15 +
> >  include/test/suites.h              |     1 +
> >  test/Makefile                      |     3 +
> >  test/cmd_ut.c                      |     6 +
> >  test/hush/Makefile                 |    10 +
> >  test/hush/cmd_ut_hush.c            |    20 +
> >  test/hush/dollar.c                 |   226 +
> >  test/hush/if.c                     |   316 +
> >  test/hush/list.c                   |   140 +
> >  test/hush/loop.c                   |    91 +
> >  test/py/tests/test_hush_if_test.py |   197 -
> >  test/py/tests/test_ut.py           |     8 +-
> >  tools/patman/series.py             |     4 +
> >  25 files changed, 14521 insertions(+), 212 deletions(-)
> >  create mode 100644 cmd/cli.c
> >  create mode 100644 common/cli_hush_2021.c
> >  create mode 100644 common/cli_hush_upstream.c
> >  create mode 100644 doc/usage/cmd/cli.rst
> >  create mode 100644 include/test/hush.h
> >  create mode 100644 test/hush/Makefile
> >  create mode 100644 test/hush/cmd_ut_hush.c
> >  create mode 100644 test/hush/dollar.c
> >  create mode 100644 test/hush/if.c
> >  create mode 100644 test/hush/list.c
> >  create mode 100644 test/hush/loop.c
> >  delete mode 100644 test/py/tests/test_hush_if_test.py
> > 
> > Best regards and thank you in advance.
> > ---
> > [1] https://lists.denx.de/pipermail/u-boot/2021-July/453347.html
> > [2] https://runtimeterror.com/tech/lil/
> > [3] https://lists.denx.de/pipermail/u-boot/2021-July/453790.html
> > [4] https://lists.denx.de/pipermail/u-boot/2021-July/453848.html
> > [5] https://lists.denx.de/pipermail/u-boot/2021-August/458569.html
> > --
> > 2.34.1






More information about the U-Boot mailing list