Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QEMU: ensure output folder exists #1235

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

axel-h
Copy link
Member

@axel-h axel-h commented Mar 31, 2024

When starting on a fresh workspace with a custom seL4 plain CMake build flow, it turns out that CMake has not created CMAKE_BINARY_DIR yet when the QEMU DTB dumping command runs. QEMU will not create the DTB file parent folder(s) either, but fails with an error. Then CMake prints the error and finishes gracefully, creating CMAKE_BINARY_DIR with some configuration cache.

$ cmake -G Ninja -S /mnt/repos/sel4/sel4test -B build-sel4test-qemu-riscv-virt -DCMAKE_MODULE_PATH=/mnt/repos/sel4/seL4;/mnt/repos/sel4/seL4_tools;/mnt/repos/sel4/seL4_tools/cmake-tool/helpers;/mnt/repos/sel4/seL4_tools/elfloader-tool;/mnt/repos/sel4/musllibc;/mnt/repos/sel4/seL4_libs;/mnt/repos/sel4/seL4_projects_libs;/mnt/repos/sel4/sel4runtime;/mnt/repos/sel4/util_libs;/mnt/repos/sel4/projects_libs -DNANOPB_SRC_ROOT_FOLDER=/mnt/repos/nanopb/nanopb -DOPENSBI_PATH=/mnt/repos/riscv-software-src/opensbi -DPLATFORM=qemu-riscv-virt -DRELEASE=FALSE -DSIMULATION=TRUE -DElfloaderImage=binary -C /mnt/repos/sel4/sel4test/settings.cmake
loading initial cache file /mnt/repos/sel4/sel4test/settings.cmake
-- Set platform details from PLATFORM=qemu-riscv-virt
--   KernelPlatform: qemu-riscv-virt
-- Found seL4: /mnt/repos/sel4/seL4  
-- platform qemu-riscv-virt supports multiple architectures, none was given
--   defaulting to: riscv64
-- Extracting device tree from QEMU
-- qemu-system-riscv64: qemu_fdt_dumpdtb: Failed dumping dtb to /workspace/build-sel4test-qemu-riscv-virt/qemu-riscv-virt.dtb
CMake Error at /mnt/repos/sel4/seL4/src/plat/qemu-riscv-virt/config.cmake:141 (message):
  Failed to dump DTB using qemu-system-riscv64: 1
Call Stack (most recent call first):
  /mnt/repos/sel4/seL4/configs/seL4Config.cmake:170 (include)
  /mnt/repos/sel4/seL4/FindseL4.cmake:21 (include)
  settings.cmake:32 (sel4_configure_platform_settings)

Now, when running CMake a second time CMAKE_BINARY_DIR exists and QEMU can dump the DTB so the build will pass "magically".
Seem I've found a corner case here, as this issues only happens when invoking CMake from a bare custom build process, which does not use all the seL4 build system scripts. I have not yet traced things down where CMAKE_BINARY_DIR is created when e.g. CI runs sel4test. Creating CMAKE_BINARY_DIR manually fixes the issue for me, but it seems more a hack. A cleaner solution would be moving all the DTB/DTS creation out of the CMake configuration phase. But that requires much more changes in the build system.

@axel-h axel-h added the build-system related to the build system label Mar 31, 2024
@axel-h axel-h force-pushed the patch-axel-100 branch 2 times, most recently from 3d4580f to 913af72 Compare March 31, 2024 01:16
@axel-h
Copy link
Member Author

axel-h commented Mar 31, 2024

Note that running the CMake command line needs seL4/sel4test#117 also.

@Indanz
Copy link
Contributor

Indanz commented Mar 31, 2024

There is something buggy going on with the DTB creation, if you change the configuration, the DTB isn't recreated properly either I think (at least the resulting image crashes, hinting at a mismatch between what the kernel does and the DTB memory regions). It seems proper dependency is missing for the DTB creation part. Once that added, creating the output directory manually may not be needed anymore.

@axel-h
Copy link
Member Author

axel-h commented Apr 1, 2024

I agree the QEMU DTB extraction is a bit messy still, but that is the best we have at the moment. I can improve the comment in the CMake file to state that there is an open follow up action needed here. However, I don't know when I find time to look into this and the changes are likely not trivial.
Getting this change merged seems the best option to avoid this nasty issue, and it addresses the problem where it arises. I'd really prefer to not solve this in a (external) build wrapper, as that looses some transparency about the issue. Note that I have also filed a Cmake bug at https://gitlab.kitware.com/cmake/cmake/-/issues/25848 to get some clarification how CMake is supposed to behave here.

Axel Heider added 2 commits May 23, 2024 11:40
Signed-off-by: Axel Heider <axel.heider@codasip.com>
Signed-off-by: Axel Heider <axel.heider@codasip.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-system related to the build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants