[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