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

boot-qemu.sh: Support using KVM on arm64 hardware #23

Merged
merged 2 commits into from
Aug 13, 2020

Conversation

nathanchance
Copy link
Member

No description provided.

Tested on a Raspberry Pi 4.

Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
@@ -139,8 +139,13 @@ function setup_qemu_args() {
arm64)
KIMAGE=Image.gz
APPEND_STRING+="console=ttyAMA0 "
if [[ "$(uname -m)" = "aarch64" && -e /dev/kvm ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Is there anything we can do to make the host check more consistent with the x86 check?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there anything we can do to make the host check more consistent with the x86 check?

What kind of consistency were you looking for? The logic for this check was copied from the cpu-checker package on Debian and Ubuntu, which contains kvm-ok. Apparently all AArch64 hardware is KVM compatible, all the script does is check if the KVM module is loaded (i.e. /dev/kvm exists).

I can probably refine the second half of the x86 check to just be "does /dev/kvm exist" but I would need to test it later.

Copy link
Member

Choose a reason for hiding this comment

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

why do we have uname guards on one and not both? Is there something in cpuinfo that we should check on aarch64?

I would expect both or neither to have cpuinfo checks.

I would expect both or neither to have uname checks.

Not different approaches per target.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not all x86 CPUs have hardware virtualization; we have to check /proc/cpuinfo for the feature flags (vmx for Intel, svm for AMD). We do not need a uname check for that as non-x86 processors will not have those flags. All AArch64 CPUs have hardware virtualization so we just need to check if we are running on an AArch64 processor to determine if we have hardware virtualization; I do not see any flags in /proc/cpuinfo that we can use to check.

Copy link
Member

Choose a reason for hiding this comment

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

Will /dev/kvm exist on x86 cpus that don't support vmx|svm? If not, then it seems better to do a uname -m check for x86 hosts, rather than check cpuinfo. What if svm appears in an aarch64 host's /dev/cpuinfo trying to boot x86?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will /dev/kvm exist on x86 cpus that don't support vmx|svm? If not, then it seems better to do a uname -m check for x86 hosts, rather than check cpuinfo. What if svm appears in an aarch64 host's /dev/cpuinfo trying to boot x86?

Ah, I see what you are saying. I do not think that /dev/kvm can exist if the CPU does not have hardware virtualization. I highly doubt that svm would appear on an AArch64 device but who knows? I still think the check is consistent with how it is in kvm-ok but I am fine with changing it (although I guess I will need to check for either x86_64 or i386 through i686 right?)

This brings us more in line with the AArch64 one that was just added.

Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
@nickdesaulniers nickdesaulniers merged commit 3b21a5b into ClangBuiltLinux:master Aug 13, 2020
@nathanchance nathanchance deleted the arm64-kvm branch August 14, 2020 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants