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

Create kernel_qemu_aarch64_run rule #1060

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

Conversation

sudhakar-mamillapalli
Copy link

Create kernel_qemu_aarch64_run rule.

  • We want to be able to run a aarch64 qemu machine emulating E1 system complex which connect to the SFA FE model via netpci for dev and testing.

  • So based on kernel_qemu_run which works for x86 arch, kernel_qemu_aarch64_run target is introduced which pretty much follows the same flow as kernel_qemu_run with changes needed for aarch64.

bazel/linux/providers.bzl Outdated Show resolved Hide resolved
bazel/linux/defs.bzl Outdated Show resolved Hide resolved
bazel/linux/runner.bzl Outdated Show resolved Hide resolved
bazel/linux/qemu.bzl Outdated Show resolved Hide resolved
@@ -195,6 +201,13 @@ def create_runner(ctx, archs, code, runfiles = None, extra = {}):
),
)

if ki.arch == "aarch64":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a dumb note, nothing to do --- the compiler and bazel both use aarch64, but the Linux kernel uses arm64. it so dumb these are different in different places... i think what you have here is correct. when writing anything bazel use aarch64.

@sudhakar-mamillapalli
Copy link
Author

Rebased.

bazel/linux/qemu.bzl Outdated Show resolved Hide resolved
bazel/linux/qemu.bzl Outdated Show resolved Hide resolved
bazel/linux/runner.bzl Outdated Show resolved Hide resolved
Copy link
Contributor

@cbrune cbrune left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good to me. Just a few minor nits that you could ignore.

echo 1>&2 '$' "$QEMU_BINARY" "${{QEMU_FLAGS[@]}}" "${{WRAPPER_OPTS[@]}}"
if [ -z "$INTERACTIVE" -a -z "$SINGLE" ]; then
"$QEMU_BINARY" "${{QEMU_FLAGS[@]}}" "${{WRAPPER_OPTS[@]}}" </dev/null | tee "$OUTPUT_FILE"
"$QEMU_BINARY" "${{QEMU_FLAGS[*]}}" "${{KERNEL_FLAGS[*]}}" "${{WRAPPER_OPTS[*]}}" </dev/null | tee "$OUTPUT_FILE"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code had:

QEMU_FLAGS+=("-append" "${{KERNEL_FLAGS[*]}} ${{KERNEL_OPTS[*]}}")

... what happened to KERNEL_OPTS in the new code? Can't find anywhere the -append, and handling of KERNEL_OPTS?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccontavalli Very sorry. Seems like I dropped KERNEL_OPTS when I mucked with args. As for -append I pass all the args (kernel/qemu and wrapper args) separately to the model-wrapper-socket script in internal repo and do the append there (a different PR https://github.com/enfabrica/internal/pull/40251).

The change is at
https://github.com/enfabrica/internal/pull/40251/commits/3e8fa4754bb040fae9bb968e9e04371ef6eb6151#diff-2d20aee07523ca136cf9db6baeb3534f46d4c33e6bb9add34e88a9f63821f0eaR87

Any way thinking again, I went back the old way of doing things since if I do the above I realized there are other model-wrapper scripts which need to be changed too. So I just pass the arguments similar to how we use to do so before.

Also I just use the machine name now to know if the qemu to use is aarch64 or x86 one, instead of passing it as a wrapper option. That way do not have to muck with wrapper opts. If you rather have the arch passed as a wapper opts, I can change that. Please let me know.

Also to make it easy to read I am force pushing the diff. Please take a look again. Sorry again for the mess-up and thanks for finding the issue.

    * For launching aarch64 qemu acf-machine a bootram image is needed.
      This image is loaded into SPS bootram and on qemu launch execution
      starts there. The actual image (bootloader + kernel + initramfs + devicetree) is loaded
      into DRAM.  The bootram image does mimimal stuff and jumps to bootloader
      which continues rest of the boot.

    * This commit just creates a provider for the BootRam image.
      * We want to be able to run a qemu machine emulating
        E1 system complex which connect to the SFA FE model
        via netpci for dev and testing.

      * So based on kernel_qemu_run which works for x86 arch,
        kernel_qemu_aarch64_run target is introduced which
        pretty much follows the same flow as kernel_qemu_run
        with changes needed for aarch64.
@sudhakar-mamillapalli
Copy link
Author

rebase

),
**{
"archs": attr.string_list(
default = ["aarch64"],
Copy link
Contributor

@skarat-enf skarat-enf Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are passing the archs, do we need a separate kernel_qemu_run rule ? Could we not just use the existing one ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants