Skip to content

Enforce clippy integration test on aarch64 #1961

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

Merged
merged 4 commits into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 21 additions & 22 deletions src/arch/src/aarch64/fdt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ pub enum Error {
type Result<T> = result::Result<T, Error>;

/// Creates the flattened device tree for this aarch64 microVM.
pub fn create_fdt<T: DeviceInfoForFDT + Clone + Debug>(
pub fn create_fdt<T: DeviceInfoForFDT + Clone + Debug, S: std::hash::BuildHasher>(
guest_mem: &GuestMemoryMmap,
vcpu_mpidr: Vec<u64>,
cmdline: &CStr,
device_info: &HashMap<(DeviceType, String), T>,
gic_device: &Box<dyn GICDevice>,
device_info: &HashMap<(DeviceType, String), T, S>,
gic_device: &dyn GICDevice,
initrd: &Option<InitrdConfig>,
) -> Result<Vec<u8>> {
// Alocate stuff necessary for the holding the blob.
Expand Down Expand Up @@ -116,7 +116,7 @@ pub fn create_fdt<T: DeviceInfoForFDT + Clone + Debug>(
create_timer_node(&mut fdt)?;
create_clock_node(&mut fdt)?;
create_psci_node(&mut fdt)?;
create_devices_node(&mut fdt, device_info)?;
create_devices_node(&mut fdt, &device_info)?;

// End Header node.
append_end_node(&mut fdt)?;
Expand Down Expand Up @@ -299,26 +299,28 @@ fn generate_prop64(cells: &[u64]) -> Vec<u8> {
}

// Following are the auxiliary function for creating the different nodes that we append to our FDT.
fn create_cpu_nodes(fdt: &mut Vec<u8>, vcpu_mpidr: &Vec<u64>) -> Result<()> {
fn create_cpu_nodes(fdt: &mut Vec<u8>, vcpu_mpidr: &[u64]) -> Result<()> {
// See https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/arm/cpus.yaml.
append_begin_node(fdt, "cpus")?;
// As per documentation, on ARM v8 64-bit systems value should be set to 2.
append_property_u32(fdt, "#address-cells", 0x02)?;
append_property_u32(fdt, "#size-cells", 0x0)?;
let num_cpus = vcpu_mpidr.len();

for cpu_index in 0..num_cpus {
let num_cpus = vcpu_mpidr.len();
for (cpu_index, mpidr) in vcpu_mpidr.iter().enumerate() {
let cpu_name = format!("cpu@{:x}", cpu_index);
append_begin_node(fdt, &cpu_name)?;
append_property_string(fdt, "device_type", "cpu")?;
append_property_string(fdt, "compatible", "arm,arm-v8")?;
if num_cpus > 1 {
// This is required on armv8 64-bit. See aforementioned documentation.
// If the microVM has more than 1 vcpu we need to enable the power
// state coordination interface (PSCI) which will decide for us which
// vcpu is running or halted.
append_property_string(fdt, "enable-method", "psci")?;
}
// Set the field to first 24 bits of the MPIDR - Multiprocessor Affinity Register.
// See http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0488c/BABHBJCI.html.
append_property_u64(fdt, "reg", vcpu_mpidr[cpu_index] & 0x7FFFFF)?;
append_property_u64(fdt, "reg", mpidr & 0x7F_FFFF)?;
append_end_node(fdt)?;
}
append_end_node(fdt)?;
Expand Down Expand Up @@ -364,7 +366,7 @@ fn create_chosen_node(
Ok(())
}

fn create_gic_node(fdt: &mut Vec<u8>, gic_device: &Box<dyn GICDevice>) -> Result<()> {
fn create_gic_node(fdt: &mut Vec<u8>, gic_device: &dyn GICDevice) -> Result<()> {
let gic_reg_prop = generate_prop64(gic_device.device_properties());

append_begin_node(fdt, "intc")?;
Expand Down Expand Up @@ -400,7 +402,7 @@ fn create_clock_node(fdt: &mut Vec<u8>) -> Result<()> {
append_begin_node(fdt, "apb-pclk")?;
append_property_string(fdt, "compatible", "fixed-clock")?;
append_property_u32(fdt, "#clock-cells", 0x0)?;
append_property_u32(fdt, "clock-frequency", 24000000)?;
append_property_u32(fdt, "clock-frequency", 24_000_000)?;
append_property_string(fdt, "clock-output-names", "clk24mhz")?;
append_property_u32(fdt, "phandle", CLOCK_PHANDLE)?;
append_end_node(fdt)?;
Expand Down Expand Up @@ -498,9 +500,9 @@ fn create_rtc_node<T: DeviceInfoForFDT + Clone + Debug>(
Ok(())
}

fn create_devices_node<T: DeviceInfoForFDT + Clone + Debug>(
fn create_devices_node<T: DeviceInfoForFDT + Clone + Debug, S: std::hash::BuildHasher>(
fdt: &mut Vec<u8>,
dev_info: &HashMap<(DeviceType, String), T>,
dev_info: &HashMap<(DeviceType, String), T, S>,
) -> Result<()> {
// Create one temp Vec to store all virtio devices
let mut ordered_virtio_device: Vec<&T> = Vec::new();
Expand Down Expand Up @@ -571,15 +573,12 @@ mod tests {
),
(
(DeviceType::Virtio(1), "virtio".to_string()),
MMIODeviceInfo {
addr: 0x00 + LEN,
irq: 2,
},
MMIODeviceInfo { addr: LEN, irq: 2 },
),
(
(DeviceType::RTC, "rtc".to_string()),
MMIODeviceInfo {
addr: 0x00 + 2 * LEN,
addr: 2 * LEN,
irq: 3,
},
),
Expand All @@ -595,7 +594,7 @@ mod tests {
vec![0],
&CString::new("console=tty0").unwrap(),
&dev_info,
&gic,
gic.as_ref(),
&None,
)
.is_ok())
Expand All @@ -613,7 +612,7 @@ mod tests {
vec![0],
&CString::new("console=tty0").unwrap(),
&HashMap::<(DeviceType, std::string::String), MMIODeviceInfo>::new(),
&gic,
gic.as_ref(),
&None,
)
.unwrap();
Expand Down Expand Up @@ -654,7 +653,7 @@ mod tests {
let vm = kvm.create_vm().unwrap();
let gic = create_gic(&vm, 1).unwrap();
let initrd = InitrdConfig {
address: GuestAddress(0x10000000),
address: GuestAddress(0x1000_0000),
size: 0x1000,
};

Expand All @@ -663,7 +662,7 @@ mod tests {
vec![0],
&CString::new("console=tty0").unwrap(),
&HashMap::<(DeviceType, std::string::String), MMIODeviceInfo>::new(),
&gic,
gic.as_ref(),
&Some(initrd),
)
.unwrap();
Expand Down
16 changes: 8 additions & 8 deletions src/arch/src/aarch64/gic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub trait GICDevice {
Self: Sized;

/// Setup the device-specific attributes
fn init_device_attributes(gic_device: &Box<dyn GICDevice>) -> Result<()>
fn init_device_attributes(gic_device: &dyn GICDevice) -> Result<()>
where
Self: Sized;

Expand Down Expand Up @@ -76,10 +76,10 @@ pub trait GICDevice {
Self: Sized,
{
let attr = kvm_bindings::kvm_device_attr {
group: group,
attr: attr,
addr: addr,
flags: flags,
group,
attr,
addr,
flags,
};
fd.set_device_attr(&attr)
.map_err(Error::SetDeviceAttribute)?;
Expand All @@ -88,7 +88,7 @@ pub trait GICDevice {
}

/// Finalize the setup of a GIC device
fn finalize_device(gic_device: &Box<dyn GICDevice>) -> Result<()>
fn finalize_device(gic_device: &dyn GICDevice) -> Result<()>
where
Self: Sized,
{
Expand Down Expand Up @@ -128,9 +128,9 @@ pub trait GICDevice {

let device = Self::create_device(vgic_fd, vcpu_count);

Self::init_device_attributes(&device)?;
Self::init_device_attributes(device.as_ref())?;

Self::finalize_device(&device)?;
Self::finalize_device(device.as_ref())?;

Ok(device)
}
Expand Down
6 changes: 3 additions & 3 deletions src/arch/src/aarch64/gicv2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,18 @@ impl GICDevice for GICv2 {

fn create_device(fd: DeviceFd, vcpu_count: u64) -> Box<dyn GICDevice> {
Box::new(GICv2 {
fd: fd,
fd,
properties: [
GICv2::get_dist_addr(),
GICv2::get_dist_size(),
GICv2::get_cpu_addr(),
GICv2::get_cpu_size(),
],
vcpu_count: vcpu_count,
vcpu_count,
})
}

fn init_device_attributes(gic_device: &Box<dyn GICDevice>) -> Result<()> {
fn init_device_attributes(gic_device: &dyn GICDevice) -> Result<()> {
/* Setting up the distributor attribute.
We are placing the GIC below 1GB so we need to substract the size of the distributor. */
Self::set_device_attribute(
Expand Down
8 changes: 4 additions & 4 deletions src/arch/src/aarch64/gicv3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,18 @@ impl GICDevice for GICv3 {

fn create_device(fd: DeviceFd, vcpu_count: u64) -> Box<dyn GICDevice> {
Box::new(GICv3 {
fd: fd,
fd,
properties: [
GICv3::get_dist_addr(),
GICv3::get_dist_size(),
GICv3::get_redists_addr(vcpu_count),
GICv3::get_redists_size(vcpu_count),
],
vcpu_count: vcpu_count,
vcpu_count,
})
}

fn init_device_attributes(gic_device: &Box<dyn GICDevice>) -> Result<()> {
fn init_device_attributes(gic_device: &dyn GICDevice) -> Result<()> {
/* Setting up the distributor attribute.
We are placing the GIC below 1GB so we need to substract the size of the distributor.
*/
Expand All @@ -108,7 +108,7 @@ impl GICDevice for GICv3 {
&gic_device.device_fd(),
kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ADDR,
u64::from(kvm_bindings::KVM_VGIC_V3_ADDR_TYPE_REDIST),
&GICv3::get_redists_addr(u64::from(gic_device.vcpu_count())) as *const u64 as u64,
&GICv3::get_redists_addr(gic_device.vcpu_count()) as *const u64 as u64,
0,
)?;

Expand Down
12 changes: 6 additions & 6 deletions src/arch/src/aarch64/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ pub fn arch_memory_regions(size: usize) -> Vec<(GuestAddress, usize)> {
/// * `device_info` - A hashmap containing the attached devices for building FDT device nodes.
/// * `gic_device` - The GIC device.
/// * `initrd` - Information about an optional initrd.
pub fn configure_system<T: DeviceInfoForFDT + Clone + Debug>(
pub fn configure_system<T: DeviceInfoForFDT + Clone + Debug, S: std::hash::BuildHasher>(
guest_mem: &GuestMemoryMmap,
cmdline_cstring: &CStr,
vcpu_mpidr: Vec<u64>,
device_info: &HashMap<(DeviceType, String), T>,
gic_device: &Box<dyn GICDevice>,
device_info: &HashMap<(DeviceType, String), T, S>,
gic_device: &dyn GICDevice,
initrd: &Option<super::InitrdConfig>,
) -> super::Result<()> {
fdt::create_fdt(
Expand All @@ -84,12 +84,12 @@ pub fn initrd_load_addr(guest_mem: &GuestMemoryMmap, initrd_size: usize) -> supe
{
Some(offset) => {
if guest_mem.address_in_range(offset) {
return Ok(offset.raw_value());
Ok(offset.raw_value())
} else {
return Err(Error::InitrdAddress);
Err(Error::InitrdAddress)
}
}
None => return Err(Error::InitrdAddress),
None => Err(Error::InitrdAddress),
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/arch/src/aarch64/regs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const PSTATE_FAULT_BITS_64: u64 = PSR_MODE_EL1h | PSR_A_BIT | PSR_F_BIT | PSR_I_
// we're just doing pointer math on it, so in theory, it should safe.
macro_rules! offset__of {
($str:ty, $field:ident) => {
unsafe { &(*(0 as *const $str)).$field as *const _ as usize }
unsafe { &(*(std::ptr::null::<$str>())).$field as *const _ as usize }
};
}

Expand Down Expand Up @@ -188,6 +188,6 @@ mod tests {
assert!(read_mpidr(&vcpu).is_err());

vcpu.vcpu_init(&kvi).unwrap();
assert_eq!(read_mpidr(&vcpu).unwrap(), 0x80000000);
assert_eq!(read_mpidr(&vcpu).unwrap(), 0x8000_0000);
}
}
14 changes: 7 additions & 7 deletions src/devices/src/legacy/rtc_pl031.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ mod tests {

// Read and write to the MR register.
byte_order::write_le_u32(&mut data, 123);
rtc.write(RTCMR, &mut data);
rtc.write(RTCMR, &data);
rtc.read(RTCMR, &mut data);
let v = byte_order::read_le_u32(&data[..]);
assert_eq!(v, 123);
Expand All @@ -210,7 +210,7 @@ mod tests {
let v = utils::time::get_time(utils::time::ClockType::Real);
byte_order::write_le_u32(&mut data, (v / utils::time::NANOS_PER_SECOND) as u32);
let previous_now_before = rtc.previous_now;
rtc.write(RTCLR, &mut data);
rtc.write(RTCLR, &data);

assert!(rtc.previous_now > previous_now_before);

Expand All @@ -222,7 +222,7 @@ mod tests {
// Test with non zero value.
let non_zero = 1;
byte_order::write_le_u32(&mut data, non_zero);
rtc.write(RTCIMSC, &mut data);
rtc.write(RTCIMSC, &data);
// The interrupt line should be on.
assert!(rtc.interrupt_evt.read().unwrap() == 1);
rtc.read(RTCIMSC, &mut data);
Expand All @@ -231,14 +231,14 @@ mod tests {

// Now test with 0.
byte_order::write_le_u32(&mut data, 0);
rtc.write(RTCIMSC, &mut data);
rtc.write(RTCIMSC, &data);
rtc.read(RTCIMSC, &mut data);
let v = byte_order::read_le_u32(&data[..]);
assert_eq!(0, v);

// Read and write to the ICR register.
byte_order::write_le_u32(&mut data, 1);
rtc.write(RTCICR, &mut data);
rtc.write(RTCICR, &data);
// The interrupt line should be on.
assert!(rtc.interrupt_evt.read().unwrap() > 1);
let v_before = byte_order::read_le_u32(&data[..]);
Expand All @@ -253,7 +253,7 @@ mod tests {

// Attempts to turn off the RTC should not go through.
byte_order::write_le_u32(&mut data, 0);
rtc.write(RTCCR, &mut data);
rtc.write(RTCCR, &data);
rtc.read(RTCCR, &mut data);
let v = byte_order::read_le_u32(&data[..]);
assert_eq!(v, 1);
Expand All @@ -262,7 +262,7 @@ mod tests {
// the CID and PID from.
byte_order::write_le_u32(&mut data, 0);
let no_errors_before = METRICS.rtc.error_count.count();
rtc.write(AMBA_ID_LOW, &mut data);
rtc.write(AMBA_ID_LOW, &data);
let no_errors_after = METRICS.rtc.error_count.count();
assert_eq!(no_errors_after - no_errors_before, 1);
// However, reading from the AMBA_ID_LOW should succeed upon read.
Expand Down
2 changes: 1 addition & 1 deletion src/kernel/src/loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ where
.seek(SeekFrom::Start(0))
.map_err(|_| Error::SeekKernelImage)?;

kernel_load_offset = kernel_load_offset + start_address;
kernel_load_offset += start_address;
guest_mem
.read_from(
GuestAddress(kernel_load_offset),
Expand Down
2 changes: 1 addition & 1 deletion src/vmm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ pub fn configure_system_for_boot(
.map_err(Internal)?;
}

let vcpu_mpidr = vcpus.into_iter().map(|cpu| cpu.get_mpidr()).collect();
let vcpu_mpidr = vcpus.iter_mut().map(|cpu| cpu.get_mpidr()).collect();
arch::aarch64::configure_system(
&vmm.guest_memory,
&boot_cmdline.as_cstring().map_err(LoadCommandline)?,
Expand Down
9 changes: 6 additions & 3 deletions src/vmm/src/vstate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use vmm_config::machine_config::CpuFeaturesTemplate;
#[cfg(target_arch = "x86_64")]
const MAGIC_IOPORT_SIGNAL_GUEST_BOOT_COMPLETE: u64 = 0x03f0;
#[cfg(target_arch = "aarch64")]
const MAGIC_IOPORT_SIGNAL_GUEST_BOOT_COMPLETE: u64 = 0x40000000;
const MAGIC_IOPORT_SIGNAL_GUEST_BOOT_COMPLETE: u64 = 0x4000_0000;
const MAGIC_VALUE_SIGNAL_GUEST_BOOT_COMPLETE: u8 = 123;

/// Signal number (SIGRTMIN) used to kick Vcpus.
Expand Down Expand Up @@ -480,8 +480,11 @@ impl Vm {

/// Gets a reference to the irqchip of the VM
#[cfg(target_arch = "aarch64")]
pub fn get_irqchip(&self) -> &Box<dyn GICDevice> {
&self.irqchip_handle.as_ref().expect("IRQ chip not set")
pub fn get_irqchip(&self) -> &dyn GICDevice {
self.irqchip_handle
.as_ref()
.expect("IRQ chip not set")
.as_ref()
}

/// Gets a reference to the kvm file descriptor owned by this VM.
Expand Down
Loading