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.lan...@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@lepotato:~# > > root@lepotato:~# reboot > > ... > > => cli set 2021 > > => cli get > > 2021 > > => printenv fdtfile > > fdtfile=amlogic/meson-gxl-s905x-libretech-cc.dtb > > => boot > > ... > > root@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