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

Changes for successful build on aarch64 #759

Merged
merged 10 commits into from
Jan 18, 2019
6 changes: 6 additions & 0 deletions .cargo/config
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
[build]
target = "x86_64-unknown-linux-musl"

[target.aarch64-unknown-linux-musl]
Copy link
Contributor

Choose a reason for hiding this comment

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

If the config file supports comments, it would be nice to add a short explanation/link regarding these particular configuration options and why the workaround is needed. We could even write it as an issue of our own, because we'll probably forget the workaround is in place here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am already referencing the original issue in the conversation of this PR and in a separate issue of our own: #756.
I also added a comment where I explain why the workaround is needed. let me know if you think something else is needed.

# On aarch64 musl depends on some libgcc functions (i.e `__addtf3` and other `*tf3` functions) for logic that uses
# long double. Such functions are not builtin in the rust compiler, so we need to get them from libgcc.
# No need for the `crt_static` flag as rustc appends it by default.
rustflags = [ "-C", "link-arg=-lgcc" ]
38 changes: 23 additions & 15 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api_server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ sys_util = { path = "../sys_util" }
vmm = { path = "../vmm" }

[dev-dependencies]
arch = { path = "../arch" }
devices = { path = "../devices" }
kernel = { path = "../kernel" }
memory_model = { path = "../memory_model" }
net_util = { path = "../net_util" }
rate_limiter = { path = "../rate_limiter" }
x86_64 = { path = "../x86_64" }

[features]
vsock = ["vmm/vsock"]
4 changes: 2 additions & 2 deletions api_server/src/request/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ impl PartialEq for ParsedRequest {

#[cfg(test)]
mod tests {
extern crate arch;
extern crate devices;
extern crate kernel;
extern crate memory_model;
extern crate net_util;
extern crate x86_64;

use self::devices::virtio::net::Error as VirtioNetError;
use self::memory_model::GuestMemoryError;
Expand Down Expand Up @@ -300,7 +300,7 @@ mod tests {
check_error_response(vmm_resp, StatusCode::InternalServerError);
let vmm_resp = VmmActionError::StartMicrovm(
ErrorKind::Internal,
StartMicrovmError::ConfigureSystem(x86_64::Error::E820Configuration),
StartMicrovmError::ConfigureSystem(arch::Error::ZeroPagePastRamEnd),
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why did the error type change here?

Copy link
Contributor Author

@dianpopa dianpopa Jan 8, 2019

Choose a reason for hiding this comment

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

The arch::Error enum from arch/src/lib.rs is intended to contain following types of errors:
Errors common to multiple platforms (like ZeroPagePastRamEnd..)
Errors specific to x86_64 (X86_64Setup(x86_64::Error))
Errors specific to other arch (none yet)
I changed the type of the error (this is a test only for a function that could return multiple types of errors) so that it returns an error which is not x86 specific.

);
check_error_response(vmm_resp, StatusCode::InternalServerError);
let vmm_resp = VmmActionError::StartMicrovm(
Expand Down
3 changes: 2 additions & 1 deletion x86_64/Cargo.toml → arch/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[package]
name = "x86_64"
name = "arch"
version = "0.1.0"
authors = ["The Chromium OS Authors"]

Expand All @@ -8,6 +8,7 @@ byteorder = "=1.2.1"
kvm_wrapper = ">=0.1.0"
libc = ">=0.2.39"

arch_gen = { path = "../arch_gen" }
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: In the commit message you still refer to this crate (I think) as arch_sys.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. Adjusted commit message.

kvm = { path = "../kvm" }
memory_model = { path = "../memory_model" }
sys_util = { path = "../sys_util" }
Expand Down
7 changes: 7 additions & 0 deletions arch/src/aarch64/layout.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

/// Kernel command line start address.
pub const CMDLINE_START: usize = 0x0;
/// Kernel command line start address maximum size.
pub const CMDLINE_MAX_SIZE: usize = 0x0;
26 changes: 26 additions & 0 deletions arch/src/aarch64/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

pub mod layout;

use memory_model::{GuestAddress, GuestMemory};

/// Stub function that needs to be implemented when aarch64 functionality is added.
pub fn arch_memory_regions(size: usize) -> Vec<(GuestAddress, usize)> {
vec![(GuestAddress(0), size)]
}

/// Stub function that needs to be implemented when aarch64 functionality is added.
pub fn configure_system(
_guest_mem: &GuestMemory,
_cmdline_addr: GuestAddress,
_cmdline_size: usize,
_num_cpus: u8,
) -> super::Result<()> {
Ok(())
}

/// Stub function that needs to be implemented when aarch64 functionality is added.
pub fn get_reserved_mem_addr() -> usize {
0
}
46 changes: 46 additions & 0 deletions arch/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

extern crate byteorder;
extern crate kvm_wrapper;
extern crate libc;

extern crate arch_gen;
extern crate kvm;
extern crate memory_model;
extern crate sys_util;

use std::result;

#[derive(Debug, PartialEq)]
pub enum Error {
#[cfg(target_arch = "x86_64")]
Copy link
Member

Choose a reason for hiding this comment

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

Is this code only x86_64 specific? I am expecting the setup to be sort of the same for both x86 and x86_64 so I don't think we should be so restrictive with errors. I suspect that changes would be minor between the 2 which means that the 2 architecture (x86 & x86_64) can share one module and have maybe just few lines specific to each architecture.

/// X86_64 specific error triggered during system configuration.
X86_64Setup(x86_64::Error),
/// The zero page extends past the end of guest_mem.
ZeroPagePastRamEnd,
/// Error writing the zero page of guest memory.
ZeroPageSetup,
}
pub type Result<T> = result::Result<T, Error>;

// 1MB. We don't put anything above here except the kernel itself.
pub const HIMEM_START: usize = 0x100000;

#[cfg(target_arch = "aarch64")]
pub mod aarch64;

#[cfg(target_arch = "aarch64")]
pub use aarch64::{
arch_memory_regions, configure_system, get_reserved_mem_addr, layout::CMDLINE_MAX_SIZE,
layout::CMDLINE_START,
};

#[cfg(target_arch = "x86_64")]
pub mod x86_64;

#[cfg(target_arch = "x86_64")]
pub use x86_64::{
arch_memory_regions, configure_system, get_32bit_gap_start as get_reserved_mem_addr,
layout::CMDLINE_MAX_SIZE, layout::CMDLINE_START,
};
File renamed without changes.
File renamed without changes.
23 changes: 23 additions & 0 deletions arch/src/x86_64/layout.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
//
// Portions Copyright 2017 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the THIRD-PARTY file.

/// Magic addresses externally used to lay out x86_64 VMs.

/// Initial stack for the boot CPU.
pub const BOOT_STACK_START: usize = 0x8000;
pub const BOOT_STACK_POINTER: usize = 0x8ff0;

/// Kernel command line start address.
pub const CMDLINE_START: usize = 0x20000;
/// Kernel command line start address maximum size.
pub const CMDLINE_MAX_SIZE: usize = 0x10000;

/// Address for the TSS setup.
pub const KVM_TSS_ADDRESS: usize = 0xfffbd000;

/// The 'zero page', a.k.a linux kernel bootparams.
pub const ZERO_PAGE_START: usize = 0x7000;
Loading