Skip to contentSkip to conversation

efi patch queue 2018-07-25

GIT PULL NAK Alexander Graf 4 participants 20 messages Jul 25, 2018 – Aug 7, 2018

lore
Share & tools
lore
#0 lore raw

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit 323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23 14:33:21 -0400)

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

----------------------------------------------------------------
Patch queue for efi - 2018-07-25

Highlights this time:

- Many small fixes to improve spec compatibility (found by SCT)
- Almost enough to run with sandbox target
- GetTime() improvements
- Enable EFI_LOADER and HYP entry on ARMv7 with NONSEC=y

----------------------------------------------------------------
Alexander Graf (11):
efi_loader: Use compiler constants for image loader
efi_loader: Use map_sysmem() in bootefi command
efi_loader: Allow SMBIOS tables in highmem
efi_loader: Disable miniapps on sandbox
efi_loader: Introduce ms abi vararg helpers
efi_loader: Move to compiler based target architecture determination
elf: Move x86 reloc defines to common elf.h
efi_loader: Use common elf.h reloc defines
efi_loader: Expose U-Boot addresses in memory map for sandbox
efi_loader: Rename sections to allow for implicit data
x86: Add efi_loader bits to x86_64 linker script

Heinrich Schuchardt (29):
efi_selftest: update .gitignore
efi_loader: efi_allocate_pages is too restrictive
efi_loader: check parameters of CreateEvent
efi_loader: check parameters in memory allocation
efi_loader: check parameters of GetMemoryMap
efi_loader: check map_key in ExitBootServices
fs: fat: cannot write to subdirectories
efi_selftest: test writing to file
efi_driver: set DM_FLAG_NAME_ALLOCED flag
efi_loader: set revision in loaded image protocol
efi_loader: EFI_SIMPLE_TEXT_INPUT_PROTOCOL.Reset()
efi_loader: clear screen has to reset cursor position
efi_loader: specify UEFI spec revision
efi_loader: correct EFI_RUNTIME_SERVICES_SIGNATURE
efi_loader: correct headersize EFI tables
efi_loader: provide firmware revision
efi_loader: calculate crc32 for EFI tables
efi_loader: allocate configuration table array
efi_selftest: test InstallConfigurationTable()
efi_loader: correct signature of CalculateCrc32()
efi_loader: update crc32 in InstallConfigurationTable
efi_selftest: check crc32 for InstallConfigurationTable
efi_selftest: unit test for CalculateCrc32()
rtc: remove CONFIG_CMD_DATE dependency
efi_loader: remove unused efi_get_time_init()
efi_loader: complete implementation of GetTime()
efi_selftest: support printing leading zeroes
efi_selftest: unit test for GetTime()
MAINTAINERS: assign lib/charset.c

Mark Kettenis (5):
ARM: HYP/non-sec: migrate stack
efi_loader: ARM: run EFI payloads non-secure
efi_loader: ARM: don't attempt to enter non-secure mode twice
ARM: HYP/non-sec: enable ARMV7_LPAE if HYP mode is supported Revert "efi_loader: no support for ARMV7_NONSEC=y"

Simon Glass (5):
efi: sandbox: Adjust memory usage for sandbox
vsprintf: Handle NULL with %pU
efi_selftest: Clean up a few comments and messages
efi: Tidy up device-tree-size calculation in copy_fdt()
efi: Drop error return in efi_carve_out_dt_rsv()

 MAINTAINERS                                  |   1 +
 arch/arm/config.mk                           |   4 +-
 arch/arm/cpu/armv7/Kconfig                   |   2 +-
 arch/arm/cpu/armv7/nonsec_virt.S             |   2 +
 arch/arm/cpu/armv8/u-boot.lds                |  24 ++-
 arch/arm/cpu/u-boot.lds                      |  36 ++--
 arch/arm/mach-zynq/u-boot.lds                |  36 ++--
 arch/riscv/cpu/ax25/u-boot.lds               |  26 ++-
 arch/sandbox/config.mk                       |   3 +
 arch/sandbox/cpu/u-boot.lds                  |   9 +-
 arch/x86/config.mk                           |   2 +
 arch/x86/cpu/start.S                         |   2 +-
 arch/x86/cpu/start64.S                       |   2 +-
 arch/x86/cpu/u-boot-64.lds                   |  37 +++-
 arch/x86/cpu/u-boot.lds                      |  34 ++--
 arch/x86/include/asm/elf.h                   |  45 -----
 arch/x86/lib/reloc_ia32_efi.c                |   1 -
 arch/x86/lib/reloc_x86_64_efi.c              |   1 -
 board/qualcomm/dragonboard410c/u-boot.lds    |  17 +-
 board/qualcomm/dragonboard820c/u-boot.lds    |  24 ++-
 board/ti/am335x/u-boot.lds                   |  36 ++--
 cmd/bootefi.c                                |  90 +++++++--
 doc/README.uefi                              |   2 -
 drivers/rtc/at91sam9_rtt.c                   |   4 -
 drivers/rtc/davinci.c                        |   2 -
 drivers/rtc/ds1302.c                         |   4 -
 drivers/rtc/ds1306.c                         |   4 -
 drivers/rtc/ds1307.c                         |   4 -
 drivers/rtc/ds1337.c                         |   4 -
 drivers/rtc/ds1374.c                         |   3 -
 drivers/rtc/ds164x.c                         |   4 -
 drivers/rtc/ds174x.c                         |   4 -
 drivers/rtc/ds3231.c                         |   4 -
 drivers/rtc/imxdi.c                          |   4 -
 drivers/rtc/m41t11.c                         |   3 -
 drivers/rtc/m41t60.c                         |   3 -
 drivers/rtc/m41t62.c                         |   4 -
 drivers/rtc/m48t35ax.c                       |   4 -
 drivers/rtc/max6900.c                        |   4 -
 drivers/rtc/mc146818.c                       |   3 -
 drivers/rtc/mcfrtc.c                         |   4 -
 drivers/rtc/mk48t59.c                        |   4 -
 drivers/rtc/pcf8563.c                        |   4 -
 drivers/rtc/rs5c372.c                        |   3 -
 drivers/rtc/rx8025.c                         |   4 -
 drivers/rtc/s3c24x0_rtc.c                    |   4 -
 drivers/rtc/x1205.c                          |   4 -
 fs/fat/fat_write.c                           |  16 +-
 include/efi.h                                |   8 +
 include/efi_api.h                            |  16 +-
 include/efi_loader.h                         |  13 +-
 include/elf.h                                |  35 ++++
 lib/efi_driver/efi_block_device.c            |   2 +
 lib/efi_loader/Kconfig                       |   2 -
 lib/efi_loader/Makefile                      |   3 +
 lib/efi_loader/efi_boottime.c                | 169 +++++++++++------
 lib/efi_loader/efi_console.c                 |   9 +-
 lib/efi_loader/efi_image_loader.c            |  12 +-
 lib/efi_loader/efi_memory.c                  |  65 +++++--
 lib/efi_loader/efi_runtime.c                 |  83 ++++++---
 lib/efi_loader/efi_smbios.c                  |  11 +-
 lib/efi_selftest/.gitignore                  |   4 +-
 lib/efi_selftest/Makefile                    |   5 +-
 lib/efi_selftest/efi_selftest.c              |  14 +-
 lib/efi_selftest/efi_selftest_block_device.c |  70 +++++++
 lib/efi_selftest/efi_selftest_config_table.c | 266 +++++++++++++++++++++++++++
 lib/efi_selftest/efi_selftest_console.c      |  33 ++--
 lib/efi_selftest/efi_selftest_crc32.c        | 141 ++++++++++++++
 lib/efi_selftest/efi_selftest_rtc.c          |  67 +++++++
 lib/vsprintf.c                               |   5 +-
 70 files changed, 1172 insertions(+), 402 deletions(-)
 delete mode 100644 arch/x86/include/asm/elf.h
 create mode 100644 lib/efi_selftest/efi_selftest_config_table.c
 create mode 100644 lib/efi_selftest/efi_selftest_crc32.c
 create mode 100644 lib/efi_selftest/efi_selftest_rtc.c
#1 lore raw
Quoted 11 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit 323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23 14:33:21 -0400)

Quoted 8 lines

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

NAK, this breaks one of the filesystem tests. Specifically:
commit 0dc1bfb7302d220a48364263d5632d6d572b069b

Author: Heinrich Schuchardt <[email protected]>
Date:   Mon Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180728/07114250/attachment.sig>
#2 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 25 lines · Tom Rini
Quoted 13 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23 14:33:21 -0400)

Quoted 9 lines

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

Quoted 9 lines

NAK, this breaks one of the filesystem tests. Specifically: commit 0dc1bfb7302d220a48364263d5632d6d572b069b Author: Heinrich
Schuchardt <[email protected]> Date: Mon Jul 2 02:41:23 2018
+0200

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

please, provide the link to the Travis log with the failure.

Best regards

Heinrich
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAltclK0ACgkQxIHbvCwF GsTc9w/8Cqd6WakCF+v2WI95qSG2EF9wl0ruM900n/IeOsh22vCkzBd7L0L3Zqbj hIiYUVmEfw7Q5DHWfo+nXXqEmjfsmewI9vcMQNYJtUbCeBq+uv0GPDjJupQ8wJyT B8nnFTWt5jKSFmF+zOdcVa8JPesSD6kmQs0SysVIRb5532rpO0LKjRwCR1uJ4f12 7PRZ1d0Mf4JcY/3RlaaVS480zqo/GRlk7ViwFaq82DDg5MWzjql9AEbYZPb1c+r5 NAyFYEk2X3OPwkRxMvPpmlV5M8xiV58Ht9KnXeqa5aivKsUFBsBoy5Yb4eLQkPAe H385U0yueZUsKil1cFYvOhkm/Nnj3JMa7eT4LovnvnGF23hu+hp2jSCamskyW06a IPHdaUDU8Wnog9loHt4yko0nNHLJ4x9Ryi9WvsFE6P6rS/58t3UcNEhLOkR6oCiJ 44hpE1MxvOzVFkIWrw1ze/86ZzMz2Cj5I65bseBMHcJIOnsZhjTpU2VFYax6gWno mNtSj8OZcwT1dd0XUB3FQofXYpIcgOlc1/cn4fhwQiH8/ZoCRlTUCseSOoa0EOCr W1QgbbR9GTpYKYV6yEy2RmC4qteWB7PvUyfddiOvaTtHeqkp8R8dWeyAmevQxIh6 RFzsXW7EZA0qa95P/5MtrkMzDKzMoD8rbjv0/9Y08TQ2vsGF1vg= =iTo4
-----END PGP SIGNATURE-----

#3 lore raw
Quoted 34 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 30 lines · Tom Rini
Quoted 13 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23 14:33:21 -0400)

Quoted 9 lines

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

NAK, this breaks one of the filesystem tests. Specifically: commit 0dc1bfb7302d220a48364263d5632d6d572b069b Author: Heinrich
Schuchardt <[email protected]> Date: Mon Jul 2 02:41:23 2018
+0200

Quoted 8 lines
Quoted 4 lines

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

please, provide the link to the Travis log with the failure.

It's actually not in travis. Running test/fs/fs-test.sh is annoying to automate:
FSTST=./test/fs/fs-test.sh 2>&1 | tail -n 3 | head -n 1
echo $FSTST | grep -q "TOTAL PASS: 204 TOTAL FAIL: 12" && exit 0 || exit 1

but I should see if I can get that into .travis.yml.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180728/bf071c85/attachment.sig>
#4 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 45 lines · Tom Rini
Quoted 42 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 27 lines · Tom Rini
Quoted 15 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm
(2018-07-23 14:33:21 -0400)

are available in the git repository at:

Quoted 8 lines

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

Quoted 9 lines

NAK, this breaks one of the filesystem tests. Specifically:
commit 0dc1bfb7302d220a48364263d5632d6d572b069b Author:
Heinrich Schuchardt <[email protected]> Date: Mon Jul 2
02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

Quoted 9 lines
Quoted 1 line

please, provide the link to the Travis log with the failure.

It's actually not in travis. Running test/fs/fs-test.sh is annoying to automate: FSTST=`./test/fs/fs-test.sh 2>&1 | tail -n 3

| head -n 1` echo $FSTST | grep -q "TOTAL PASS: 204 TOTAL FAIL: 12"
&& exit 0 || exit 1

but I should see if I can get that into .travis.yml.

./test/fs/fs-test.sh
Missing mkfs binary. Exiting!

You wouldn't run tests as root? Is this test meant to be run with fakeroot?

Best regards

Heinrich
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAltcmB8ACgkQxIHbvCwF GsRfyg/7ByTTAsNNZOa1jXxqY4Swp+F495ybdjRlUMkiIF+zxgQJLCcLpkVO1HLR 4TP62LTq+gGdnHvEOjQmxVGCGXNjlFCyBDbGL7rB8eLZFIBgJ1343Q1V4V0FURir HdCpIVFRHUNwMV3DU6iK04QwjldC7MurJG/yJaE6RySrRxdGSNxSP9kYDkDrolU+ BOibDleXhxaGT65Vb7V77dAmkGwGWYhvCD/3AgDxlztGKOCSIW1XotUyWrORQvhf dUL3VddKaVqQoUc3MZJllc5II/qCdFO8e4jXlPMXvm6iHeqGBdt7oHssylbAZkuS V+9Ocfi1/qz7c4ZR7MhC7Lqs8zmVBO8gnseLnt4nI3HbrSa4etR1Aqr+YY57id24 stCe7miK71DK7FiBVdgeJqRLiT82pcMZHSRCCW0DlJM70v4SPRZ/rNOyGnQnxSvo /2SJQz/jVOMhk2SqjT9mjtEfHPjyt1EUllqjQUjEYtMvqCgx2b5sWKQUEkiGiJTB Ydj+LqnRCBzKUVbS1JVU56jhuHA8zqM67QRaTRgj1MQqkrNEDqj4CHA7a3tol159 OWY2Sv7BcxXozHTVF5l0yImh2w1Bx3ot0XYY5u2L35eEQl60Xyi9TmSrC1l1qv29 PHVYLwynvecQP56RbqHpAws1TuiIPWpnW5Rc66/Y1j8hplD2F6w= =g8NU
-----END PGP SIGNATURE-----

#5 lore raw
Quoted 56 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 52 lines · Tom Rini
Quoted 35 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 32 lines · Tom Rini
Quoted 15 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm
(2018-07-23 14:33:21 -0400)

are available in the git repository at:

Quoted 8 lines

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

NAK, this breaks one of the filesystem tests. Specifically:
commit 0dc1bfb7302d220a48364263d5632d6d572b069b Author:
Heinrich Schuchardt <[email protected]> Date: Mon Jul 2
02:41:23 2018 +0200

Quoted 8 lines
Quoted 4 lines

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

please, provide the link to the Travis log with the failure.

It's actually not in travis. Running test/fs/fs-test.sh is annoying to automate: FSTST=`./test/fs/fs-test.sh 2>&1 | tail -n 3

| head -n 1` echo $FSTST | grep -q "TOTAL PASS: 204 TOTAL FAIL: 12"
&& exit 0 || exit 1
Quoted 8 lines
Quoted 2 lines

but I should see if I can get that into .travis.yml.

./test/fs/fs-test.sh
Missing mkfs binary. Exiting!

You wouldn't run tests as root? Is this test meant to be run with fakeroot?

It requires sudo to work along with various utilities to make the various filesystems.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180728/dcfee3fd/attachment.sig>
#6 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 61 lines · Tom Rini
Quoted 59 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 55 lines · Tom Rini
Quoted 36 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 33 lines · Tom Rini
Quoted 15 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm
(2018-07-23 14:33:21 -0400)

are available in the git repository at:

Quoted 8 lines

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

NAK, this breaks one of the filesystem tests.
Specifically: commit 0dc1bfb7302d220a48364263d5632d6d572b069b Author: Heinrich
Schuchardt <[email protected]> Date: Mon Jul 2 02:41:23
2018 +0200

Quoted 8 lines
Quoted 4 lines

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

please, provide the link to the Travis log with the failure.

It's actually not in travis. Running test/fs/fs-test.sh is annoying to automate: FSTST=./test/fs/fs-test.sh 2>&1 | tail -n 3 | head -n 1 echo $FSTST | grep -q "TOTAL PASS: 204 TOTAL
FAIL: 12" && exit 0 || exit 1

but I should see if I can get that into .travis.yml.

Quoted 8 lines
Quoted 4 lines

./test/fs/fs-test.sh Missing mkfs binary. Exiting!

You wouldn't run tests as root? Is this test meant to be run with fakeroot?

It requires sudo to work along with various utilities to make the various filesystems.

Tom please, have a look at the files created by the tests w/o my patch.

This is what the find command returns:

sandbox/test/fs/mnt
sandbox/test/fs/mnt/SUBDIR
sandbox/test/fs/mnt/2.5GB.file
sandbox/test/fs/mnt/1MB.file
sandbox/test/fs/mnt/1MB.file.w
sandbox/test/fs/mnt/1MB.file.w2
sandbox/test/fs/mnt/./1MB.file.w2

You observe that the last file has an illegal file name (yes, the filename itself is "./1MB.file.w2". It should never have been created.

Without my patch this illegal file is not created.

Why should this be a reason to dismiss my patch?

Best regards

Heinrich
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAltco4YACgkQxIHbvCwF GsSTxhAAhPbNHg16YZg7DZE6pMPZ5v64eVrAPPiM6LoIb8rXyLtKP9lDh/V1tk33 t7nHRfPfS5Ow3KYjneBxoxnw3A4uxU4Rf16xb5KZX9UvLPUusumwjaRUW9/FytiQ MkZeZQGdtAHLy9cFL3sRkIM4Vz3kWo5XBNgYjJaYQAN0/BlWt6oknTUKAYrciGCz O+B7i6Vf7hXj1d1RdFjLfS4sinSibjUMd1Bn5HoAZvjLo3Gfmi+e/ZIPXKb75QfP fQwH6oGuEOL6WqL+OyS5CVXiBeGbOtrJoH7o5C8TEixcPwp2cKGP7p/L8UBIOBbM a/zXIgumFZhAyDhRdLcvevvUxfBRX4opCgGYk8aG8/QlpnoZO7eeQJxKU7+t/JxY UCRQNZXzh6WbTDLkRkP5u3vIA0qGqpGNkH9D7lGU8W9Iq9awkr3gtLls1HGuyGI+ zdYpLeC9JYYRPPZSG/Guy58NphuYRG3vCj6D8tvLqMzE3dYCIBwxobWESV26s40h do3oqPTpmFYWJNJG4C1x0H3JfMW32X5ibrk8Hn6AFvHn4DYsdD4c24Ofol9IcIqf nbgsYwrf5rdyiqgK5MUVXoFztNkPot6BSI+sU/ft3CcxX+0/cpSYiZ3M3QOk8BGt J5Q7/57oBxcLrZrlEmE9PE+rmGDpw6F60DqZpGo5EwyzZmb4nKM= =V/cr
-----END PGP SIGNATURE-----

#7 lore raw
Quoted 86 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 61 lines · Tom Rini
Quoted 59 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 55 lines · Tom Rini
Quoted 36 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 33 lines · Tom Rini
Quoted 15 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm
(2018-07-23 14:33:21 -0400)

are available in the git repository at:

Quoted 8 lines

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

NAK, this breaks one of the filesystem tests.
Specifically: commit 0dc1bfb7302d220a48364263d5632d6d572b069b Author: Heinrich
Schuchardt <[email protected]> Date: Mon Jul 2 02:41:23
2018 +0200

Quoted 8 lines
Quoted 4 lines

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

please, provide the link to the Travis log with the failure.

It's actually not in travis. Running test/fs/fs-test.sh is annoying to automate: FSTST=./test/fs/fs-test.sh 2>&1 | tail -n 3 | head -n 1 echo $FSTST | grep -q "TOTAL PASS: 204 TOTAL
FAIL: 12" && exit 0 || exit 1

but I should see if I can get that into .travis.yml.

Quoted 8 lines
Quoted 4 lines

./test/fs/fs-test.sh Missing mkfs binary. Exiting!

You wouldn't run tests as root? Is this test meant to be run with fakeroot?

It requires sudo to work along with various utilities to make the various filesystems.

Tom please, have a look at the files created by the tests w/o my patch.

This is what the find command returns:

sandbox/test/fs/mnt
sandbox/test/fs/mnt/SUBDIR
sandbox/test/fs/mnt/2.5GB.file
sandbox/test/fs/mnt/1MB.file
sandbox/test/fs/mnt/1MB.file.w
sandbox/test/fs/mnt/1MB.file.w2
sandbox/test/fs/mnt/./1MB.file.w2

Quoted 6 lines

You observe that the last file has an illegal file name (yes, the filename itself is "./1MB.file.w2". It should never have been created.

Without my patch this illegal file is not created.

Why should this be a reason to dismiss my patch?

Ah, OK, thanks for looking. Please submit a patch that updates the tests.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180728/cb4f63bf/attachment.sig>
#8 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 92 lines · Tom Rini
Quoted 90 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 64 lines · Tom Rini
Quoted 62 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 58 lines · Tom Rini
Quoted 40 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 37 lines · Tom Rini
Quoted 16 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please
pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm
(2018-07-23 14:33:21 -0400)

are available in the git repository at:

Quoted 9 lines

git://github.com/agraf/u-boot.git
tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25
15:00:24 +0200)

NAK, this breaks one of the filesystem tests.
Specifically: commit
0dc1bfb7302d220a48364263d5632d6d572b069b Author:
Heinrich Schuchardt <[email protected]> Date: Mon
Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Quoted 7 lines
Quoted 2 lines

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

please, provide the link to the Travis log with the failure.

It's actually not in travis. Running test/fs/fs-test.sh is
annoying to automate: FSTST=`./test/fs/fs-test.sh 2>&1 |
tail -n 3 | head -n 1` echo $FSTST | grep -q "TOTAL PASS:
204 TOTAL FAIL: 12" && exit 0 || exit 1

but I should see if I can get that into .travis.yml.

Quoted 8 lines
Quoted 4 lines

./test/fs/fs-test.sh Missing mkfs binary. Exiting!

You wouldn't run tests as root? Is this test meant to be run with fakeroot?

It requires sudo to work along with various utilities to make the various filesystems.

Tom please, have a look at the files created by the tests w/o my patch.

This is what the find command returns:

sandbox/test/fs/mnt sandbox/test/fs/mnt/SUBDIR sandbox/test/fs/mnt/2.5GB.file sandbox/test/fs/mnt/1MB.file sandbox/test/fs/mnt/1MB.file.w sandbox/test/fs/mnt/1MB.file.w2 sandbox/test/fs/mnt/./1MB.file.w2

You observe that the last file has an illegal file name (yes, the filename itself is "./1MB.file.w2". It should never have been created.

Quoted 7 lines
Quoted 3 lines

Without my patch this illegal file is not created.

Why should this be a reason to dismiss my patch?

Ah, OK, thanks for looking. Please submit a patch that updates the tests.

With Takahiro's patch series

fs: fat: extend FAT write operations
https://patchwork.ozlabs.org/project/uboot/list/?series=56580
https://lists.denx.de/pipermail/u-boot/2018-July/335683.html

the FAT driver will finally correctly support paths with subdirectories.

With that patch series the created files are:

sandbox/test/fs/mnt
sandbox/test/fs/mnt/SUBDIR
sandbox/test/fs/mnt/2.5GB.file
sandbox/test/fs/mnt/1MB.file
sandbox/test/fs/mnt/1MB.file.w
sandbox/test/fs/mnt/1MB.file.w2

There is nothing wrong with the TC13 test. After writing it tries to do the verification with (b) and without (c) a relative path. If both subtests are passed the file system is working as expected. And as you already will have observed TC13b and TC13c are not passed without Takahiro's patch series.

Best regards

Heinrich
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAltc4P8ACgkQxIHbvCwF GsSfXg/9G6RqPbMGb3wl3SOZSnPM2VzTv9tvnnpqZq8Bmaao4I7sgnI3w/Ts6r/k iBU0c4iOrcytOiVWiQU9FPJ3L8zKOMqfK7vE9186NUZFh1u10sJrmLfESV6pbKF8 +pxKy6xM2y8F7AWE0VhgAf7lxFbA2tnhFAQPbP687mJaFKYKG48CQ0r7GwXO/iEl YvE5KflSOEIuTJ6VD7sVuOdEWw8v9yXZTevxcjYSMQppvGg2JIpILtUc89aMlNNC rFTesESdkqcm5V8NUaaMLv404QiW1z93ya+za7IwoUiXAlxYFuXCWJtOqWXxz4Mu EbGOXTYXUq0xXqNJDZDjtwzOQilUANvFGAwXUJc1F6q3F0XJNLOStC7Vr6SrBjij ibiJAj1XOgEbNip6m1yDB0ycyCjKjASr+l6RrfWzFli1AiQon4Fk6P38Rb27RAiU DGTMKp6gqhj1tTLaNQTEzwdgcr19GvRfJQpfDYkZX0ujESETwMCyO+1i4VuM7bJo v2h7C+1m7uk7qVDbwJAtTS0YiL7k7ZqyFv55AmGtRH0ZNVNIgIFByfXYchukrHqo q89l1mYtMba05ZZ4JVp/21AcEFH94WbtlPvkizXhpcxeH2ZpGR1GhmZEXNJ/a9uP d9eDngVapglWzJCya+ojtMh+RZi4Rn01Y3Qv45PfRptmzxJn2CI= =aYo7
-----END PGP SIGNATURE-----

#9 lore raw
Quoted 121 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 92 lines · Tom Rini
Quoted 90 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 64 lines · Tom Rini
Quoted 62 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 58 lines · Tom Rini
Quoted 40 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 37 lines · Tom Rini
Quoted 16 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please
pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm
(2018-07-23 14:33:21 -0400)

are available in the git repository at:

Quoted 9 lines

git://github.com/agraf/u-boot.git
tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25
15:00:24 +0200)

NAK, this breaks one of the filesystem tests.
Specifically: commit
0dc1bfb7302d220a48364263d5632d6d572b069b Author:
Heinrich Schuchardt <[email protected]> Date: Mon
Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Quoted 7 lines
Quoted 2 lines

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

please, provide the link to the Travis log with the failure.

It's actually not in travis. Running test/fs/fs-test.sh is
annoying to automate: FSTST=`./test/fs/fs-test.sh 2>&1 |
tail -n 3 | head -n 1` echo $FSTST | grep -q "TOTAL PASS:
204 TOTAL FAIL: 12" && exit 0 || exit 1

but I should see if I can get that into .travis.yml.

Quoted 8 lines
Quoted 4 lines

./test/fs/fs-test.sh Missing mkfs binary. Exiting!

You wouldn't run tests as root? Is this test meant to be run with fakeroot?

It requires sudo to work along with various utilities to make the various filesystems.

Tom please, have a look at the files created by the tests w/o my patch.

This is what the find command returns:

sandbox/test/fs/mnt sandbox/test/fs/mnt/SUBDIR sandbox/test/fs/mnt/2.5GB.file sandbox/test/fs/mnt/1MB.file sandbox/test/fs/mnt/1MB.file.w sandbox/test/fs/mnt/1MB.file.w2 sandbox/test/fs/mnt/./1MB.file.w2

You observe that the last file has an illegal file name (yes, the filename itself is "./1MB.file.w2". It should never have been created.

Quoted 7 lines
Quoted 3 lines

Without my patch this illegal file is not created.

Why should this be a reason to dismiss my patch?

Ah, OK, thanks for looking. Please submit a patch that updates the tests.

With Takahiro's patch series

fs: fat: extend FAT write operations
https://patchwork.ozlabs.org/project/uboot/list/?series=56580
https://lists.denx.de/pipermail/u-boot/2018-July/335683.html

the FAT driver will finally correctly support paths with subdirectories.

With that patch series the created files are:

sandbox/test/fs/mnt
sandbox/test/fs/mnt/SUBDIR
sandbox/test/fs/mnt/2.5GB.file
sandbox/test/fs/mnt/1MB.file
sandbox/test/fs/mnt/1MB.file.w
sandbox/test/fs/mnt/1MB.file.w2

Quoted 5 lines

There is nothing wrong with the TC13 test. After writing it tries to do the verification with (b) and without (c) a relative path. If both subtests are passed the file system is working as expected. And as you already will have observed TC13b and TC13c are not passed without Takahiro's patch series.

Then I guess the answer is an update to fs-test.sh to note that the expected, for now, results should be 200/16 and to make sure that Takahiro's series also updates fs-test.sh results. Thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180728/653b7407/attachment.sig>
#10 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 128 lines · Tom Rini
Quoted 120 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 94 lines · Tom Rini
Quoted 92 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 65 lines · Tom Rini
Quoted 63 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 59 lines · Tom Rini
Quoted 40 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 37 lines · Tom Rini
Quoted 16 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please
pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm
(2018-07-23 14:33:21 -0400)

are available in the git repository at:

Quoted 9 lines

git://github.com/agraf/u-boot.git
tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25
15:00:24 +0200)

NAK, this breaks one of the filesystem tests.
Specifically: commit
0dc1bfb7302d220a48364263d5632d6d572b069b Author:
Heinrich Schuchardt <[email protected]> Date:
Mon Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Quoted 7 lines
Quoted 2 lines

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

please, provide the link to the Travis log with the failure.

It's actually not in travis. Running test/fs/fs-test.sh is annoying to automate:
FSTST=./test/fs/fs-test.sh 2>&1 | tail -n 3 | head -n 1 echo $FSTST | grep -q "TOTAL PASS: 204 TOTAL FAIL:
12" && exit 0 || exit 1

but I should see if I can get that into .travis.yml.

Quoted 8 lines
Quoted 4 lines

./test/fs/fs-test.sh Missing mkfs binary. Exiting!

You wouldn't run tests as root? Is this test meant to be run with fakeroot?

It requires sudo to work along with various utilities to make the various filesystems.

Tom please, have a look at the files created by the tests w/o my patch.

This is what the find command returns:

sandbox/test/fs/mnt sandbox/test/fs/mnt/SUBDIR sandbox/test/fs/mnt/2.5GB.file sandbox/test/fs/mnt/1MB.file sandbox/test/fs/mnt/1MB.file.w sandbox/test/fs/mnt/1MB.file.w2 sandbox/test/fs/mnt/./1MB.file.w2

You observe that the last file has an illegal file name (yes, the filename itself is "./1MB.file.w2". It should never have been created.

Quoted 7 lines
Quoted 3 lines

Without my patch this illegal file is not created.

Why should this be a reason to dismiss my patch?

Ah, OK, thanks for looking. Please submit a patch that updates the tests.

With Takahiro's patch series

fs: fat: extend FAT write operations
https://patchwork.ozlabs.org/project/uboot/list/?series=56580
https://lists.denx.de/pipermail/u-boot/2018-July/335683.html

the FAT driver will finally correctly support paths with subdirectories.

With that patch series the created files are:

sandbox/test/fs/mnt sandbox/test/fs/mnt/SUBDIR sandbox/test/fs/mnt/2.5GB.file sandbox/test/fs/mnt/1MB.file sandbox/test/fs/mnt/1MB.file.w sandbox/test/fs/mnt/1MB.file.w2

Quoted 5 lines

There is nothing wrong with the TC13 test. After writing it tries to do the verification with (b) and without (c) a relative path. If both subtests are passed the file system is working as expected. And as you already will have observed TC13b and TC13c are not passed without Takahiro's patch series.

Quoted 4 lines

Then I guess the answer is an update to fs-test.sh to note that the expected, for now, results should be 200/16 and to make sure that Takahiro's series also updates fs-test.sh results. Thanks!

@Tom:
I am not able to reproduce that 200/16 result, I get 189/27. The difference are probably the 11 fails on ext4.

Creating files in ext4 image if not already present.
mount: /home/user/u-boot/sandbox/test/fs/mnt: cannot mount; probably corrupted filesystem on /dev/loop0.
umount: sandbox/test/fs/mnt: not mounted.
rmdir: failed to remove 'sandbox/test/fs/mnt': Directory not empty ** Start sandbox/test/fs/fs-test.fs.ext4.out_clean
pass - TC1: ls of 2.5GB.file
pass - TC1: ls of 1MB.file
pass - TC2: size of 1MB.file
pass - TC2: size of 1MB.file via a path using '..'
pass - TC3: size of 2.5GB.file
pass - TC4: load of 1MB.file size
FAIL - TC4: load from 1MB.file
pass - TC5: load of 1st MB from 2.5GB.file size
FAIL - TC5: load of 1st MB from 2.5GB.file
pass - TC6: load of last MB from 2.5GB.file size
FAIL - TC6: load of last MB from 2.5GB.file
pass - TC7: load of last 1mb chunk of 2GB from 2.5GB.file size FAIL - TC7: load of last 1mb chunk of 2GB from 2.5GB.file pass - TC8: load 1st MB chunk after 2GB from 2.5GB.file size FAIL - TC8: load 1st MB chunk after 2GB from 2.5GB.file pass - TC9: load 1MB chunk crossing 2GB boundary from 2.5GB.file size FAIL - TC9: load 1MB chunk crossing 2GB boundary from 2.5GB.file pass - TC10: load 2MB from the last 1MB of 2.5GB.file loads 1MB FAIL - TC11: 1MB write to 1MB.file.w - write succeeded FAIL - TC11: 1MB write to 1MB.file.w - content verified pass - TC12: 1MB write to . - write denied
FAIL - TC13: 1MB write to ./1MB.file.w2 - write succeeded FAIL - TC13: 1MB read from ./1MB.file.w2 - content verified FAIL - TC13: 1MB read from 1MB.file.w2 - content verified ** End sandbox/test/fs/fs-test.fs.ext4.out_clean
Summary: PASS: 13 FAIL: 11

When rerunning the test scripts I get "Total Summary: TOTAL PASS: 178 TOTAL FAIL: 38".

I think this test script is a mess:

- - The file systems used for testing are not delivered with U-Boot. So they might differ between host systems. I would prefer gzipped file systems to be delivered with the source. Then on each test run unzip the original file system.
- - The test requires running as root. Running software as root is a big security issue. And it is totally unnecessary. File systems can be mounted in user space using fuse.
- - The test misses to clean up what it has changed. It cannot be rerun without manually deleting the /sandbox directory or you get different results on the 2nd run.
- - Each individual test that is expected to fail should not write FAIL but TODO, so that we know what is an unexpected failure.
- - The test should be integrated in Travis.

If this is the only patch you are worried about, couldn't you, please, merge the rest of the EFI queue.

Best regards

Heinrich
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEbcT5xx8ppvoGt20zxIHbvCwFGsQFAltdb84ACgkQxIHbvCwF GsQLdA/8Cxi0wuX6/CK/JiulQyBBPWhLYmQxHxJQs6es6JSm7mbq9RDddc4apsJG XKembfUCrRJ6hyd653mWCaSknJewEp0z8EqEh0XinwCjL7nY0ZDbJdJmZu5nGxSc 3OIivCd3H7PQRDFDqn1zVm7dw+HMWEzLsXu3jsuyAClgOsnwfdzsQm0Bv1sfLoaD TY/AFNJSe3XKFoMf2TOq4ROwEUSgbFJC+PIgTho++wqwtgHwgNv4yuaJo3Ces7p4 1hfaYmQRdWAhG25CrjiHmSHW7vypqYfuaCKryPgqBFY8qQqpYJHRiYHyZySYpJam +PW1+qTrj8iZwRBx5mZyErzufrT50b2LOGsuVN9yfzVCht3jtHLYdF/UNlzyFv/v tlZ+HWL+/Skn3lQ0Syhb2HyJMnFD9DyDU05rE/kyzmr1rVY8mCKmKQ39P9pn1/j3 mjex5cgVwQhpXsKv1hO7mZF55ZZxaQjp3OzrQq/84cjeCIAhILG+3lz7IgGcuOzk AcZ01OndTYuNubaSJ/CwY+ipOTndiC8SDUhA4aUxIdsdmOKHpyGRFr38dzppcz/i wxXDvBdSlQsWuqU0mFJIRsuiWftJwbV4KOB18v2OqvegQ7naj9Pl8DqX7yA1nSvz xGxQrlNo/7Jmp5T6IdOGF12ioKbglok8h7XwRRYE1pupfk/XHkE= =lS4a
-----END PGP SIGNATURE-----

#11 lore raw
Quoted 179 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 128 lines · Tom Rini
Quoted 120 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 94 lines · Tom Rini
Quoted 92 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 65 lines · Tom Rini
Quoted 63 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 59 lines · Tom Rini
Quoted 40 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 37 lines · Tom Rini
Quoted 16 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please
pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm
(2018-07-23 14:33:21 -0400)

are available in the git repository at:

Quoted 9 lines

git://github.com/agraf/u-boot.git
tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25
15:00:24 +0200)

NAK, this breaks one of the filesystem tests.
Specifically: commit
0dc1bfb7302d220a48364263d5632d6d572b069b Author:
Heinrich Schuchardt <[email protected]> Date:
Mon Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Quoted 7 lines
Quoted 2 lines

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

please, provide the link to the Travis log with the failure.

It's actually not in travis. Running test/fs/fs-test.sh is annoying to automate:
FSTST=./test/fs/fs-test.sh 2>&1 | tail -n 3 | head -n 1 echo $FSTST | grep -q "TOTAL PASS: 204 TOTAL FAIL:
12" && exit 0 || exit 1

but I should see if I can get that into .travis.yml.

Quoted 8 lines
Quoted 4 lines

./test/fs/fs-test.sh Missing mkfs binary. Exiting!

You wouldn't run tests as root? Is this test meant to be run with fakeroot?

It requires sudo to work along with various utilities to make the various filesystems.

Tom please, have a look at the files created by the tests w/o my patch.

This is what the find command returns:

sandbox/test/fs/mnt sandbox/test/fs/mnt/SUBDIR sandbox/test/fs/mnt/2.5GB.file sandbox/test/fs/mnt/1MB.file sandbox/test/fs/mnt/1MB.file.w sandbox/test/fs/mnt/1MB.file.w2 sandbox/test/fs/mnt/./1MB.file.w2

You observe that the last file has an illegal file name (yes, the filename itself is "./1MB.file.w2". It should never have been created.

Quoted 7 lines
Quoted 3 lines

Without my patch this illegal file is not created.

Why should this be a reason to dismiss my patch?

Ah, OK, thanks for looking. Please submit a patch that updates the tests.

With Takahiro's patch series

fs: fat: extend FAT write operations
https://patchwork.ozlabs.org/project/uboot/list/?series=56580
https://lists.denx.de/pipermail/u-boot/2018-July/335683.html

the FAT driver will finally correctly support paths with subdirectories.

With that patch series the created files are:

sandbox/test/fs/mnt sandbox/test/fs/mnt/SUBDIR sandbox/test/fs/mnt/2.5GB.file sandbox/test/fs/mnt/1MB.file sandbox/test/fs/mnt/1MB.file.w sandbox/test/fs/mnt/1MB.file.w2

Quoted 5 lines

There is nothing wrong with the TC13 test. After writing it tries to do the verification with (b) and without (c) a relative path. If both subtests are passed the file system is working as expected. And as you already will have observed TC13b and TC13c are not passed without Takahiro's patch series.

Quoted 4 lines

Then I guess the answer is an update to fs-test.sh to note that the expected, for now, results should be 200/16 and to make sure that Takahiro's series also updates fs-test.sh results. Thanks!

@Tom:
I am not able to reproduce that 200/16 result, I get 189/27. The difference are probably the 11 fails on ext4.

Creating files in ext4 image if not already present.
mount: /home/user/u-boot/sandbox/test/fs/mnt: cannot mount; probably corrupted filesystem on /dev/loop0.
umount: sandbox/test/fs/mnt: not mounted.
rmdir: failed to remove 'sandbox/test/fs/mnt': Directory not empty ** Start sandbox/test/fs/fs-test.fs.ext4.out_clean
pass - TC1: ls of 2.5GB.file
pass - TC1: ls of 1MB.file
pass - TC2: size of 1MB.file
pass - TC2: size of 1MB.file via a path using '..'
pass - TC3: size of 2.5GB.file
pass - TC4: load of 1MB.file size
FAIL - TC4: load from 1MB.file
pass - TC5: load of 1st MB from 2.5GB.file size
FAIL - TC5: load of 1st MB from 2.5GB.file
pass - TC6: load of last MB from 2.5GB.file size
FAIL - TC6: load of last MB from 2.5GB.file
pass - TC7: load of last 1mb chunk of 2GB from 2.5GB.file size FAIL - TC7: load of last 1mb chunk of 2GB from 2.5GB.file pass - TC8: load 1st MB chunk after 2GB from 2.5GB.file size FAIL - TC8: load 1st MB chunk after 2GB from 2.5GB.file pass - TC9: load 1MB chunk crossing 2GB boundary from 2.5GB.file size FAIL - TC9: load 1MB chunk crossing 2GB boundary from 2.5GB.file pass - TC10: load 2MB from the last 1MB of 2.5GB.file loads 1MB FAIL - TC11: 1MB write to 1MB.file.w - write succeeded FAIL - TC11: 1MB write to 1MB.file.w - content verified pass - TC12: 1MB write to . - write denied
FAIL - TC13: 1MB write to ./1MB.file.w2 - write succeeded FAIL - TC13: 1MB read from ./1MB.file.w2 - content verified FAIL - TC13: 1MB read from 1MB.file.w2 - content verified ** End sandbox/test/fs/fs-test.fs.ext4.out_clean
Summary: PASS: 13 FAIL: 11

When rerunning the test scripts I get "Total Summary: TOTAL PASS: 178 TOTAL FAIL: 38".

I think this test script is a mess:

Quoted 13 lines

- - The file systems used for testing are not delivered with U-Boot. So they might differ between host systems. I would prefer gzipped file systems to be delivered with the source. Then on each test run unzip the original file system.
- - The test requires running as root. Running software as root is a big security issue. And it is totally unnecessary. File systems can be mounted in user space using fuse.
- - The test misses to clean up what it has changed. It cannot be rerun without manually deleting the /sandbox directory or you get different results on the 2nd run.
- - Each individual test that is expected to fail should not write FAIL but TODO, so that we know what is an unexpected failure.
- - The test should be integrated in Travis.

Yes. It's been a long standing ask if anyone can improve the fs tests as there are various headaches in running them. It is also however what we have today and with a little prep work the expected results can be replicated.

Quoted 2 lines

If this is the only patch you are worried about, couldn't you, please, merge the rest of the EFI queue.

I'll send the patch then to update the expected outputs/results for now then.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180730/fbec45c9/attachment.sig>
#12 lore raw
In reply to Heinrich Schuchardt
Quoted 121 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Quoted 92 lines · Tom Rini
Quoted 90 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 64 lines · Tom Rini
Quoted 62 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 58 lines · Tom Rini
Quoted 40 lines · Heinrich Schuchardt

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Quoted 37 lines · Tom Rini
Quoted 16 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please
pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm
(2018-07-23 14:33:21 -0400)

are available in the git repository at:

Quoted 9 lines

git://github.com/agraf/u-boot.git
tags/signed-efi-next

for you to fetch changes up to
0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25
15:00:24 +0200)

NAK, this breaks one of the filesystem tests.
Specifically: commit
0dc1bfb7302d220a48364263d5632d6d572b069b Author:
Heinrich Schuchardt <[email protected]> Date: Mon
Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Quoted 7 lines
Quoted 2 lines

Breaks TC13: 1MB write to ./1MB.file.w2

Hello Tom,

please, provide the link to the Travis log with the failure.

It's actually not in travis. Running test/fs/fs-test.sh is
annoying to automate: FSTST=`./test/fs/fs-test.sh 2>&1 |
tail -n 3 | head -n 1` echo $FSTST | grep -q "TOTAL PASS:
204 TOTAL FAIL: 12" && exit 0 || exit 1

but I should see if I can get that into .travis.yml.

Quoted 8 lines
Quoted 4 lines

./test/fs/fs-test.sh Missing mkfs binary. Exiting!

You wouldn't run tests as root? Is this test meant to be run with fakeroot?

It requires sudo to work along with various utilities to make the various filesystems.

Tom please, have a look at the files created by the tests w/o my patch.

This is what the find command returns:

sandbox/test/fs/mnt sandbox/test/fs/mnt/SUBDIR sandbox/test/fs/mnt/2.5GB.file sandbox/test/fs/mnt/1MB.file sandbox/test/fs/mnt/1MB.file.w sandbox/test/fs/mnt/1MB.file.w2 sandbox/test/fs/mnt/./1MB.file.w2

You observe that the last file has an illegal file name (yes, the filename itself is "./1MB.file.w2". It should never have been created.

Quoted 7 lines
Quoted 3 lines

Without my patch this illegal file is not created.

Why should this be a reason to dismiss my patch?

Ah, OK, thanks for looking. Please submit a patch that updates the tests.

With Takahiro's patch series

fs: fat: extend FAT write operations
https://patchwork.ozlabs.org/project/uboot/list/?series=56580
https://lists.denx.de/pipermail/u-boot/2018-July/335683.html

the FAT driver will finally correctly support paths with subdirectories.

With that patch series the created files are:

sandbox/test/fs/mnt
sandbox/test/fs/mnt/SUBDIR
sandbox/test/fs/mnt/2.5GB.file
sandbox/test/fs/mnt/1MB.file
sandbox/test/fs/mnt/1MB.file.w
sandbox/test/fs/mnt/1MB.file.w2

Quoted 5 lines

There is nothing wrong with the TC13 test. After writing it tries to do the verification with (b) and without (c) a relative path. If both subtests are passed the file system is working as expected. And as you already will have observed TC13b and TC13c are not passed without Takahiro's patch series.

OK, I'm coming back in here. The log file from running the test is sandbox/test/fs/fs-test.fs.fat32.out_clean and if we look for TC13, this is what we see:
=> # Write it via "same directory", i.e. "." dirent
=> # Test Case 13a - Check directory traversal
=> save host 0:0 0x01000008 ./1MB.file.w2 $filesize
FAT: illegal filename (./1MB.file.w2)

So the change is leading to us not being able to write "./1MB.file.w2" at all, rather than (as the test was intended to catch!) writing "1MB.file.w2" to ".". Yes, the current code is wrong as it writes the illegal file "./1MB.file.w2" but the new behavior is wrong too. I am going to take the EFI PR as-is and I'm going to push a commit that updates the expected results and explains why. I'm going to leave it to you and Takahiro-san's series to address the problem and allow for "./file" to write "file" to "." as intended.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180730/baba4341/attachment.sig>
#13 lore raw
In reply to Tom Rini

Hi,

Quoted 22 lines · Tom Rini
Quoted 11 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit 323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23 14:33:21 -0400)

Quoted 8 lines

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

Quoted 8 lines

NAK, this breaks one of the filesystem tests. Specifically:
commit 0dc1bfb7302d220a48364263d5632d6d572b069b

Author: Heinrich Schuchardt <[email protected]>
Date:   Mon Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

This is getting a little painful for me :-)

The EFI tree contains a few of the patches needed to make sandbox support EFI, but not all. A rebase on this tree provides me with a segfault, so it needs more work. But we are right at RC1.

Looking at the state of things it is clear that I cannot just pull in my previously applied patches since there are various changes. I'm going to have to send a new set of patches.

Perhaps as a way forward we could drop this one patch and apply to master? Then I can do my side of things while the patch is worked on?

Regards,
Simon

#14 lore raw
Quoted 38 lines · Simon Glass

Hi,

Quoted 21 lines · Tom Rini
Quoted 11 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit 323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23 14:33:21 -0400)

Quoted 8 lines

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

   MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)
Quoted 8 lines

NAK, this breaks one of the filesystem tests. Specifically:
commit 0dc1bfb7302d220a48364263d5632d6d572b069b

Author: Heinrich Schuchardt <[email protected]>
Date:   Mon Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

This is getting a little painful for me :-)

The EFI tree contains a few of the patches needed to make sandbox support EFI, but not all. A rebase on this tree provides me with a segfault, so it needs more work. But we are right at RC1.

Quoted 6 lines

Looking at the state of things it is clear that I cannot just pull in my previously applied patches since there are various changes. I'm going to have to send a new set of patches.

Perhaps as a way forward we could drop this one patch and apply to master? Then I can do my side of things while the patch is worked on?

What is really missing to just make the test pass? All we need to do is adapt the fail/success rate of the fat test, right?

How do I quickly run the test to cook up a patch?

Alex

#15 lore raw
Quoted 43 lines · Alexander Graf
Quoted 42 lines · Simon Glass

Hi,

Quoted 21 lines · Tom Rini
Quoted 11 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit 323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23 14:33:21 -0400)

Quoted 8 lines

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

Quoted 8 lines

NAK, this breaks one of the filesystem tests. Specifically:
commit 0dc1bfb7302d220a48364263d5632d6d572b069b

Author: Heinrich Schuchardt <[email protected]>
Date:   Mon Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

This is getting a little painful for me :-)

The EFI tree contains a few of the patches needed to make sandbox support EFI, but not all. A rebase on this tree provides me with a segfault, so it needs more work. But we are right at RC1.

Looking at the state of things it is clear that I cannot just pull in my previously applied patches since there are various changes. I'm going to have to send a new set of patches.

Quoted 7 lines
Quoted 2 lines

Perhaps as a way forward we could drop this one patch and apply to master? Then I can do my side of things while the patch is worked on?

What is really missing to just make the test pass? All we need to do is adapt the fail/success rate of the fat test, right?

How do I quickly run the test to cook up a patch?

Run ./test/fs/fs-test.sh and see what you get for totals, as the starting point. And then you can also join in on cringing at how bad our current filesystem related tests are :)

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180730/2b961a6c/attachment.sig>
#16 lore raw
In reply to Tom Rini
Quoted 22 lines · Tom Rini
Quoted 11 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit 323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23 14:33:21 -0400)

Quoted 8 lines

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

Quoted 8 lines

NAK, this breaks one of the filesystem tests. Specifically:
commit 0dc1bfb7302d220a48364263d5632d6d572b069b

Author: Heinrich Schuchardt <[email protected]>
Date:   Mon Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

I've applied this PR along with a patch that changes the expected results of fs-test.sh as we've changed from writing an illegal file to not allowing a valid path + file to be written, which is a step in the right general direction at least.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180730/89d5a9ee/attachment.sig>
#17 lore raw
Quoted 28 lines · Tom Rini
Quoted 26 lines · Tom Rini
Quoted 11 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit 323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23 14:33:21 -0400)

Quoted 8 lines

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

   MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

NAK, this breaks one of the filesystem tests. Specifically:
commit 0dc1bfb7302d220a48364263d5632d6d572b069b

Author: Heinrich Schuchardt <[email protected]>
Date:   Mon Jul 2 02:41:23 2018 +0200
Quoted 7 lines
Quoted 3 lines

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

I've applied this PR along with a patch that changes the expected results of fs-test.sh as we've changed from writing an illegal file to not allowing a valid path + file to be written, which is a step in the right general direction at least.

Thanks :)

Alex

#18 lore raw

Hi Tom,

Quoted 36 lines · Alexander Graf
Quoted 35 lines · Tom Rini
Quoted 31 lines · Tom Rini
Quoted 13 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23
14:33:21 -0400)

Quoted 8 lines

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

   MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

NAK, this breaks one of the filesystem tests. Specifically:
commit 0dc1bfb7302d220a48364263d5632d6d572b069b

Author: Heinrich Schuchardt <[email protected]>
Date:   Mon Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

Quoted 7 lines
Quoted 4 lines

I've applied this PR along with a patch that changes the expected results of fs-test.sh as we've changed from writing an illegal file to not allowing a valid path + file to be written, which is a step in the right general direction at least.

Thanks :)

Thanks Tom.

I'm going to have to send a v9 series for sandbox on EFI since there are various changes. I'll include a partial revert as well since there is a sandbox change that I don't think we want.

Once people have had a change to review it, we can see whether we can get sandbox EFI support into this release. But it might be too late.

Regards,
Simon

#19 lore raw
Quoted 47 lines · Simon Glass

Hi Tom,

Quoted 36 lines · Alexander Graf
Quoted 35 lines · Tom Rini
Quoted 31 lines · Tom Rini
Quoted 13 lines · Alexander Graf

Hi Tom,

This is my current patch queue for efi. Please pull.

Alex

The following changes since commit
323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:

mtd: nand: add new enum for storing ECC algorithm (2018-07-23
14:33:21 -0400)

Quoted 8 lines

are available in the git repository at:

git://github.com/agraf/u-boot.git tags/signed-efi-next

for you to fetch changes up to 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:

   MAINTAINERS: assign lib/charset.c (2018-07-25 15:00:24 +0200)

NAK, this breaks one of the filesystem tests. Specifically:
commit 0dc1bfb7302d220a48364263d5632d6d572b069b

Author: Heinrich Schuchardt <[email protected]>
Date:   Mon Jul 2 02:41:23 2018 +0200

fs: fat: cannot write to subdirectories

Breaks TC13: 1MB write to ./1MB.file.w2

Quoted 7 lines
Quoted 4 lines

I've applied this PR along with a patch that changes the expected results of fs-test.sh as we've changed from writing an illegal file to not allowing a valid path + file to be written, which is a step in the right general direction at least.

Thanks :)

Quoted 8 lines

Thanks Tom.

I'm going to have to send a v9 series for sandbox on EFI since there are various changes. I'll include a partial revert as well since there is a sandbox change that I don't think we want.

Once people have had a change to review it, we can see whether we can get sandbox EFI support into this release. But it might be too late.

OK, thanks for the update!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180807/0bf46a6c/attachment.sig>