-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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
[runtimes] Add a qemu-system executor script #68643
Conversation
@llvm/pr-subscribers-libcxx Author: Alexander Richardson (arichardson) ChangesThis is useful when trying to test libc++/libc++abi/libunwind against a baremetal enviroment (e.g. picolibc). See also https://reviews.llvm.org/D155521 Full diff: https://github.com/llvm/llvm-project/pull/68643.diff 1 Files Affected:
diff --git a/libcxx/utils/qemu_baremetal.py b/libcxx/utils/qemu_baremetal.py
new file mode 100755
index 000000000000000..63139cb3638e2d3
--- /dev/null
+++ b/libcxx/utils/qemu_baremetal.py
@@ -0,0 +1,73 @@
+#!/usr/bin/env python
+# ===----------------------------------------------------------------------===##
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+# ===----------------------------------------------------------------------===##
+
+"""qemu_baremetal.py is a utility for running a program with QEMU's system mode.
+
+It is able to pass command line arguments to the program and forward input and
+output (if the underlying baremetal enviroment supports QEMU semihosting).
+"""
+
+import argparse
+import os
+import platform
+import subprocess
+import sys
+
+
+def main():
+ parser = argparse.ArgumentParser()
+ parser.add_argument("--qemu", type=str, required=True)
+ parser.add_argument("--cpu", type=str, required=False)
+ parser.add_argument("--machine", type=str, default="virt")
+ parser.add_argument(
+ "--qemu-arg", dest="qemu_args", type=str, action="append", default=[]
+ )
+ parser.add_argument(
+ "--semihosting", type=argparse.BooleanOptionalAction, default=True
+ )
+ parser.add_argument("--execdir", type=str, required=True)
+ parser.add_argument("test_binary")
+ parser.add_argument("test_args", nargs=argparse.ZERO_OR_MORE, default=[])
+ args = parser.parse_args()
+ if not os.path.exists(args.test_binary):
+ sys.exit(f"Expected argument to be a test executable: '{args.test_binary}'")
+ qemu_commandline = [
+ args.qemu,
+ "-chardev",
+ "stdio,mux=on,id=stdio0",
+ "-monitor",
+ "none",
+ "-serial",
+ "none",
+ "-machine",
+ f"{args.machine},accel=tcg",
+ "-device",
+ f"loader,file={args.test_binary},cpu-num=0",
+ "-nographic",
+ *args.qemu_args,
+ ]
+ if args.cpu:
+ qemu_commandline += ["-cpu", args.cpu]
+
+ if args.semihosting:
+ # Use QEMU's semihosting support to pass argv (supported by picolibc)
+ semihosting_config = f"enable=on,chardev=stdio0,arg={args.test_binary}"
+ for arg in args.test_args:
+ semihosting_config += f",arg={arg}"
+ qemu_commandline += ["-semihosting-config", semihosting_config]
+ elif args.test_args:
+ sys.exit(
+ "Got non-empty test arguments but do no know how to pass them to "
+ "QEMU without semihosting support"
+ )
+ os.execvp(qemu_commandline[0], qemu_commandline)
+
+
+if __name__ == "__main__":
+ exit(main())
|
I think @mplatings is probably the right person to review this since I have no familiarity with QEMU. However I have no concerns with the patch itself, feel free to merge once @mplatings is happy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to accept as libc++ just for the sake of the process.
ping @mplatings |
I'd prefer to land https://reviews.llvm.org/D154246 and https://reviews.llvm.org/D155521 as-is since they're well tested. I'm not sure whether this will work in all circumstances since it doesn't support the full interface of |
I've run it locally with picolibc so it definitely works, the codesigner interface of run.py is only needed if you use the script for Apple platforms. Would you be okay with landing this change, then you could rebase the phabricator reviews on top of this and use the python script instead of qemu.sh in those reviews? They need to be rebased anyway to get a clean ci run so hopefully this doesn't add too much work? |
@domin144 is taking over from me so I hand your question to him... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @arichardson,
I run the tests from https://reviews.llvm.org/D155521, with this script and it works. I had to do two workarounds though.
- I had to avoid feature from python above 3.8 (see comment inline)
- I had to update picolibc - older one used different numeration of arguments in
argv
I am happy to rebase and use the new script in the picolibc tests review, when this script is merged.
libcxx/utils/qemu_baremetal.py
Outdated
) | ||
parser.add_argument( | ||
"--semihosting", type=argparse.BooleanOptionalAction, default=True | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BooleanOptionalAction
is a python 3.9 feature. The system image used to run the picolibc is Ubuntu focal, which has python 3.8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll update to remove that too keep older python compat.
This is useful when trying to test libc++/libc++abi/libunwind against a baremetal enviroment (e.g. picolibc). See also https://reviews.llvm.org/D155521
625d331
to
395798f
Compare
395798f
to
9a0a440
Compare
This is useful when trying to test libc++/libc++abi/libunwind against a baremetal enviroment (e.g. picolibc). See also https://reviews.llvm.org/D155521
This is useful when trying to test libc++/libc++abi/libunwind against a baremetal enviroment (e.g. picolibc).
See also https://reviews.llvm.org/D155521