Skip to content
This repository was archived by the owner on Dec 7, 2023. It is now read-only.

Conversation

luxas
Copy link
Contributor

@luxas luxas commented Jan 28, 2020

This PR makes sure keep_bootcon is given to the kernel args on ARM64.
It also updates the default kernel params to the new recommendations.

cc @chanwit @stealthybox

@luxas luxas added this to the v0.7.0 milestone Jan 28, 2020
@luxas luxas requested a review from twelho as a code owner January 28, 2020 20:35
// Refer to https://github.com/firecracker-microvm/firecracker/blob/master/src/vmm/src/vmm_config/boot_source.rs
// TODO: The Firecracker team don't use console=ttyS0 anymore, but 8250.nr_uarts=0 instead
// See https://github.com/firecracker-microvm/firecracker/pull/313 for more info
VM_DEFAULT_KERNEL_ARGS = "console=ttyS0 reboot=k panic=1 pci=off ip=dhcp i8042.noaux i8042.nomux i8042.nopnp i8042.dumbkbd"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know how involved this console related TODO is?
Looks like this patch is mostly enabling the i8042 keyboard.

// On ARM64, keep_bootcon needs to be applied to the kernel arguments
if runtime.GOARCH == "arm64" && !strings.Contains(cmdLine, "keep_bootcon") {
// Prepend keep_bootcon as per https://github.com/firecracker-microvm/firecracker/blob/master/docs/getting-started.md
cmdLine = fmt.Sprintf("keep_bootcon %s", cmdLine)
Copy link
Contributor

@stealthybox stealthybox Jan 29, 2020

Choose a reason for hiding this comment

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

Why would we only enable this arg for ARM VM's ?
It looks to be mainly related to debugging boot:
https://lore.kernel.org/patchwork/patch/232096/

Did you find your aarch64 kernels did not boot without this option?

This seems like a strange thing to enable just because it's mentioned once in getting-started conditional with no comment.

@luxas
Copy link
Contributor Author

luxas commented Mar 16, 2020

Closing this as the Firecracker team said keep_bootcon probably is not necessary

@luxas luxas closed this Mar 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants