labath added a comment. I like this. Apart from the inline comments (which are mostly about removing excessive mentions of arm(64) -- I want to make that text feel generic, since the file is in a generic place), I have one high-level comment. Given the current layout of the repo, it seems like these scripts would be better placed under `lldb/scripts` than `lldb/tools`. E.g., this scripts are fairly similar to the `macos-setup-codesign.sh` script, which also lives in that folder.
================ Comment at: lldb/tools/lldb-test-qemu/README.txt:2 +---------------------------------------------------------------------- + LLDB testing on Arm/AArch64 Linux using QEMU system mode emulation +---------------------------------------------------------------------- ---------------- ================ Comment at: lldb/tools/lldb-test-qemu/README.txt:9-14 +Main motivation for writing this guide is to test AArch64 features like SVE, +MTE, Pointer Authentication etc. These scripts can be used as reference to set +up similar testing environment for other architectures supported by QEMU. + +We have written helper scripts under llvm-project/lldb/tools/lldb-test-qemu +which can help quickly setup a Arm or AArch64 testing environment using QEMU. ---------------- ================ Comment at: lldb/tools/lldb-test-qemu/README.txt:16 + +* setup.sh is used to build AArch64 and Arm Linux kernel image and QEMU + system emulation executable(s) from source. ---------------- ================ Comment at: lldb/tools/lldb-test-qemu/README.txt:20 +* rootfs.sh is used to generate Ubuntu root file system images to be used for + QEMU Arm and AArch64 system emulation. + ---------------- ================ Comment at: lldb/tools/lldb-test-qemu/README.txt:22 + +* run-qemu.sh utilizes QEMU to boot an Arm or AArch64 Linux kernel image with + a given root file system image. ---------------- ================ Comment at: lldb/tools/lldb-test-qemu/rootfs.sh:57-65 +if [[ "$rfs_arch" != "arm64" && "$rfs_arch" != "armhf" ]]; then + echo "Invalid architecture: $rfs_arch" + print_usage 1 +fi + +if [[ "$rfs_distro" != "focal" && "$rfs_distro" != "bionic" ]]; then + echo "Invalid distribution: $rfs_distro" ---------------- You're only passing these to `qemu-debootstrap`. It sounds like things could "just work" for a bunch of other distro/arch combinations if we just removed these checks. ================ Comment at: lldb/tools/lldb-test-qemu/run-qemu.sh:83-87 + if [[ "$ARCH" == "arm" ]]; then + QEMU_BIN=$(pwd)/qemu.git/arm-softmmu/qemu-system-arm + elif [[ "$ARCH" == "arm64" ]]; then + QEMU_BIN=$(pwd)/qemu.git/aarch64-softmmu/qemu-system-aarch64 + fi ---------------- It would be nice if this searched for the qemu binary on the PATH if it was not explicitly provided. Something like: ``` if [ -d qemu.git ]; then QEMU_BIN=$(pwd)/qemu.git/arm-softmmu/qemu-system-arm else QEMU_BIN=qemu-system-arm fi ``` ================ Comment at: lldb/tools/lldb-test-qemu/run-qemu.sh:99-101 + if [[ $SVE ]]; then + invalid_arg "--sve" + fi ---------------- How about we check for this in the parsing while loop? That would mean the --arch argument has to come before --sve, but that does not seem like a bad thing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82064/new/ https://reviews.llvm.org/D82064 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
