| Message ID | 20180808095433.230882-1-sjg@chromium.org |
|---|---|
| Headers | show |
| Series | efi: Enable sandbox support for EFI loader | expand |
On 08.08.18 11:54, Simon Glass wrote: > A limitation of the EFI loader at present is that it does not build with > sandbox. This makes it hard to write tests, since sandbox is used for most > testing in U-Boot. > > This series enables the EFI loader feature. It allows sandbox to build and > run a trivial function which calls the EFI API to output a message. > > Also included in v8 is support for running the full EFI self tests. These > run OK with some tweaks to a few parts of the code. > > With v9, various EFI patches have been applied which change things. This > series includes a partial review of one, which makes 'bootefi test' work. > But there are still problems with 'bootefi selftest': > > $ sandbox/u-boot -D -c "bootefi selftest" > ... > Executing 'block device' > /home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest_block_device.c(385): > TODO: Wrong volume label 'xxa1', expected 'U-BOOT TEST' > map_to_sysmem: Added map from 00007ffd0782d2a0 to 8000000 > phys_to_virt: Used map from 8000000 to 00007ffd0782d2a0 > writing /u-boot.txt > find_tag: Used map from 00007ffd0782d2a0 to 8000000 > phys_to_virt: Used map from 8000000 to 00007ffd0782d2a0 > /home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest_block_device.c(458): > ERROR: Unexpected file content > /home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest.c(109): > ERROR: Executing 'block device' failed > > ... > > Executing 'simple network protocol' > DHCP Discover > DHCP Discover > DHCP Discover > DHCP Discover > DHCP Discover > DHCP Discover > DHCP Discover > DHCP Discover > DHCP Discover > DHCP Discover > /home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest_snp.c(311): > ERROR: Timeout occurred > /home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest.c(109): > ERROR: Executing 'simple network protocol' failed > > This series is at u-boot-dm/efi-working Thanks a lot again for pushing this forward. My ultimate goal is that sandbox is not a special case, but just "yet another" target we support. That means things like a special "bootefi test" command really don't make any sense and should just go. The other thing we need to make sure is that every divergence we find between sandbox and non-sandbox behavior potentially hints at a real problem: - If exit doesn't work, don't just disable it, instead please debug why and resolve it for real. - If tests are failing that really shouldn't fail, please figure out why. Maybe they used the stack in some cases which breaks your mapping logic? At the end of the day, I want to have as little divergence between the sandbox target and a real x86_64 QEMU target for example. As long as we don't exit boot services, both should really behave the same. Alex
Hi Alex, On 26 August 2018 at 19:28, Alexander Graf <agraf@suse.de> wrote: > > > On 08.08.18 11:54, Simon Glass wrote: >> A limitation of the EFI loader at present is that it does not build with >> sandbox. This makes it hard to write tests, since sandbox is used for most >> testing in U-Boot. >> >> This series enables the EFI loader feature. It allows sandbox to build and >> run a trivial function which calls the EFI API to output a message. >> >> Also included in v8 is support for running the full EFI self tests. These >> run OK with some tweaks to a few parts of the code. >> >> With v9, various EFI patches have been applied which change things. This >> series includes a partial review of one, which makes 'bootefi test' work. >> But there are still problems with 'bootefi selftest': >> >> $ sandbox/u-boot -D -c "bootefi selftest" >> ... >> Executing 'block device' >> /home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest_block_device.c(385): >> TODO: Wrong volume label 'xxa1', expected 'U-BOOT TEST' >> map_to_sysmem: Added map from 00007ffd0782d2a0 to 8000000 >> phys_to_virt: Used map from 8000000 to 00007ffd0782d2a0 >> writing /u-boot.txt >> find_tag: Used map from 00007ffd0782d2a0 to 8000000 >> phys_to_virt: Used map from 8000000 to 00007ffd0782d2a0 >> /home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest_block_device.c(458): >> ERROR: Unexpected file content >> /home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest.c(109): >> ERROR: Executing 'block device' failed I was able to fix this one. >> >> ... >> >> Executing 'simple network protocol' >> DHCP Discover >> DHCP Discover >> DHCP Discover >> DHCP Discover >> DHCP Discover >> DHCP Discover >> DHCP Discover >> DHCP Discover >> DHCP Discover >> DHCP Discover >> /home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest_snp.c(311): >> ERROR: Timeout occurred >> /home/sjg/c/src/third_party/u-boot/files/lib/efi_selftest/efi_selftest.c(109): >> ERROR: Executing 'simple network protocol' failed I am not sure what is going on here. I think it is best that Heinrich takes a look once this stuff lands and before we enable the tests on travis-ci. >> >> This series is at u-boot-dm/efi-working > > Thanks a lot again for pushing this forward. > > My ultimate goal is that sandbox is not a special case, but just "yet > another" target we support. > > That means things like a special "bootefi test" command really don't > make any sense and should just go. I've renamed this to 'bootefi ping'. While I understand your POV, I feel that a simple sanity check is useful. At present 'bootefi selftest' quits sandbox when it finishes, so itis not really working the way it should anyway. > > The other thing we need to make sure is that every divergence we find > between sandbox and non-sandbox behavior potentially hints at a real > problem: > > - If exit doesn't work, don't just disable it, instead please debug > why and resolve it for real. > > - If tests are failing that really shouldn't fail, please figure out > why. Maybe they used the stack in some cases which breaks your mapping > logic? Please see above. The tests all used to pass 6 months ago when I revised this work. I think we should get something in and then do some more work. > > At the end of the day, I want to have as little divergence between the > sandbox target and a real x86_64 QEMU target for example. As long as we > don't exit boot services, both should really behave the same. Fair enough, but Rome wasn't built in a day. I think that's my main problem with this painful year-long process, that you want everything to be done in one series, instead of collaboratively moving towards the ultimate end goal. We would have had this running a long time ago if we have just taken an incremental approach. Regards, Simon