Skip to content

Commit

Permalink
cleanup: Remove now unneeded wrapper types from mptable.rs
Browse files Browse the repository at this point in the history 
Before e0c9ac9 we needed a workaround
to be able to implement the ByteValued trait for the various mpspec
structures, as they were defined in a different crate (and that crate
contained autogenerated bindings). Thus we introduced wrapper types to
work around the orphan rules.

Now however all code lives in the vmm crate, meaning the wrapper types
are no longer neccessary.

Furthermore, we do not need to use `unsafe` in compute_checksum, as it
is only called for types that implement `ByteValued`.

Signed-off-by: Patrick Roy <[email protected]>
  • Loading branch information
@roypat
roypat committed Sep 19, 2023
1 parent ec738c8 commit 48e1d3f
Showing 1 changed file with 121 additions and 140 deletions.
261 changes: 121 additions & 140 deletions src/vmm/src/arch/x86_64/mptable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,49 +7,29 @@

use std::convert::TryFrom;
use std::fmt::Debug;
use std::{io, mem, slice};
use std::{io, mem};

use libc::c_char;
use utils::vm_memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemory, GuestMemoryMmap};

use crate::arch::IRQ_MAX;
use crate::arch_gen::x86::mpspec;

// This is a workaround to the Rust enforcement specifying that any implementation of a foreign
// trait (in this case `ByteValued`) where:
// * the type that is implementing the trait is foreign or
// * all of the parameters being passed to the trait (if there are any) are also foreign
// is prohibited.
#[derive(Copy, Clone, Default)]
struct MpcBusWrapper(mpspec::mpc_bus);
#[derive(Copy, Clone, Default)]
struct MpcCpuWrapper(mpspec::mpc_cpu);
#[derive(Copy, Clone, Default)]
struct MpcIntsrcWrapper(mpspec::mpc_intsrc);
#[derive(Copy, Clone, Default)]
struct MpcIoapicWrapper(mpspec::mpc_ioapic);
#[derive(Copy, Clone, Default)]
struct MpcTableWrapper(mpspec::mpc_table);
#[derive(Copy, Clone, Default)]
struct MpcLintsrcWrapper(mpspec::mpc_lintsrc);
#[derive(Copy, Clone, Default)]
struct MpfIntelWrapper(mpspec::mpf_intel);

// These `mpspec` wrapper types are only data, reading them from data is a safe initialization.
// SAFETY: POD
unsafe impl ByteValued for MpcBusWrapper {}
unsafe impl ByteValued for mpspec::mpc_bus {}
// SAFETY: POD
unsafe impl ByteValued for MpcCpuWrapper {}
unsafe impl ByteValued for mpspec::mpc_cpu {}
// SAFETY: POD
unsafe impl ByteValued for MpcIntsrcWrapper {}
unsafe impl ByteValued for mpspec::mpc_intsrc {}
// SAFETY: POD
unsafe impl ByteValued for MpcIoapicWrapper {}
unsafe impl ByteValued for mpspec::mpc_ioapic {}
// SAFETY: POD
unsafe impl ByteValued for MpcTableWrapper {}
unsafe impl ByteValued for mpspec::mpc_table {}
// SAFETY: POD
unsafe impl ByteValued for MpcLintsrcWrapper {}
unsafe impl ByteValued for mpspec::mpc_lintsrc {}
// SAFETY: POD
unsafe impl ByteValued for MpfIntelWrapper {}
unsafe impl ByteValued for mpspec::mpf_intel {}

// MPTABLE, describing VCPUS.
const MPTABLE_START: u64 = 0x9fc00;
Expand Down Expand Up @@ -106,14 +86,9 @@ const CPU_STEPPING: u32 = 0x600;
const CPU_FEATURE_APIC: u32 = 0x200;
const CPU_FEATURE_FPU: u32 = 0x001;

fn compute_checksum<T: Copy + Debug>(v: &T) -> u8 {
// SAFETY: Safe because we are only reading the bytes within the size of the `T` reference `v`.
let v_slice = unsafe {
let ptr = (v as *const T).cast::<u8>();
slice::from_raw_parts(ptr, mem::size_of::<T>())
};
fn compute_checksum<T: ByteValued>(v: &T) -> u8 {
let mut checksum: u8 = 0;
for i in v_slice.iter() {
for i in v.as_slice() {
checksum = checksum.wrapping_add(*i);
}
checksum
Expand All @@ -125,13 +100,13 @@ fn mpf_intel_compute_checksum(v: &mpspec::mpf_intel) -> u8 {
}

fn compute_mp_size(num_cpus: u8) -> usize {
mem::size_of::<MpfIntelWrapper>()
+ mem::size_of::<MpcTableWrapper>()
+ mem::size_of::<MpcCpuWrapper>() * (num_cpus as usize)
+ mem::size_of::<MpcIoapicWrapper>()
+ mem::size_of::<MpcBusWrapper>()
+ mem::size_of::<MpcIntsrcWrapper>() * (IRQ_MAX as usize + 1)
+ mem::size_of::<MpcLintsrcWrapper>() * 2
mem::size_of::<mpspec::mpf_intel>()
+ mem::size_of::<mpspec::mpc_table>()
+ mem::size_of::<mpspec::mpc_cpu>() * (num_cpus as usize)
+ mem::size_of::<mpspec::mpc_ioapic>()
+ mem::size_of::<mpspec::mpc_bus>()
+ mem::size_of::<mpspec::mpc_intsrc>() * (IRQ_MAX as usize + 1)
+ mem::size_of::<mpspec::mpc_lintsrc>() * 2
}

/// Performs setup of the MP table for the given `num_cpus`.
Expand Down Expand Up @@ -162,13 +137,15 @@ pub fn setup_mptable(mem: &GuestMemoryMmap, num_cpus: u8) -> Result<(), MptableE
.map_err(|_| MptableError::Clear)?;

{
let mut mpf_intel = MpfIntelWrapper(mpspec::mpf_intel::default());
let size = mem::size_of::<MpfIntelWrapper>() as u64;
mpf_intel.0.signature = SMP_MAGIC_IDENT;
mpf_intel.0.length = 1;
mpf_intel.0.specification = 4;
mpf_intel.0.physptr = (base_mp.raw_value() + size) as u32;
mpf_intel.0.checksum = mpf_intel_compute_checksum(&mpf_intel.0);
let size = mem::size_of::<mpspec::mpf_intel>() as u64;
let mut mpf_intel = mpspec::mpf_intel {
signature: SMP_MAGIC_IDENT,
physptr: (base_mp.raw_value() + size) as u32,
length: 1,
specification: 4,
..mpspec::mpf_intel::default()
};
mpf_intel.checksum = mpf_intel_compute_checksum(&mpf_intel);
mem.write_obj(mpf_intel, base_mp)
.map_err(|_| MptableError::WriteMpfIntel)?;
base_mp = base_mp.unchecked_add(size);
Expand All @@ -177,117 +154,126 @@ pub fn setup_mptable(mem: &GuestMemoryMmap, num_cpus: u8) -> Result<(), MptableE
// We set the location of the mpc_table here but we can't fill it out until we have the length
// of the entire table later.
let table_base = base_mp;
base_mp = base_mp.unchecked_add(mem::size_of::<MpcTableWrapper>() as u64);
base_mp = base_mp.unchecked_add(mem::size_of::<mpspec::mpc_table>() as u64);

{
let size = mem::size_of::<MpcCpuWrapper>() as u64;
let size = mem::size_of::<mpspec::mpc_cpu>() as u64;
for cpu_id in 0..num_cpus {
let mut mpc_cpu = MpcCpuWrapper(mpspec::mpc_cpu::default());
mpc_cpu.0.type_ = mpspec::MP_PROCESSOR as u8;
mpc_cpu.0.apicid = cpu_id;
mpc_cpu.0.apicver = APIC_VERSION;
mpc_cpu.0.cpuflag = mpspec::CPU_ENABLED as u8
| if cpu_id == 0 {
mpspec::CPU_BOOTPROCESSOR as u8
} else {
0
};
mpc_cpu.0.cpufeature = CPU_STEPPING;
mpc_cpu.0.featureflag = CPU_FEATURE_APIC | CPU_FEATURE_FPU;
let mpc_cpu = mpspec::mpc_cpu {
type_: mpspec::MP_PROCESSOR as u8,
apicid: cpu_id,
apicver: APIC_VERSION,
cpuflag: mpspec::CPU_ENABLED as u8
| if cpu_id == 0 {
mpspec::CPU_BOOTPROCESSOR as u8
} else {
0
},
cpufeature: CPU_STEPPING,
featureflag: CPU_FEATURE_APIC | CPU_FEATURE_FPU,
..Default::default()
};
mem.write_obj(mpc_cpu, base_mp)
.map_err(|_| MptableError::WriteMpcCpu)?;
base_mp = base_mp.unchecked_add(size);
checksum = checksum.wrapping_add(compute_checksum(&mpc_cpu.0));
checksum = checksum.wrapping_add(compute_checksum(&mpc_cpu));
}
}
{
let size = mem::size_of::<MpcBusWrapper>() as u64;
let mut mpc_bus = MpcBusWrapper(mpspec::mpc_bus::default());
mpc_bus.0.type_ = mpspec::MP_BUS as u8;
mpc_bus.0.busid = 0;
mpc_bus.0.bustype = BUS_TYPE_ISA;
let size = mem::size_of::<mpspec::mpc_bus>() as u64;
let mpc_bus = mpspec::mpc_bus {
type_: mpspec::MP_BUS as u8,
busid: 0,
bustype: BUS_TYPE_ISA,
};
mem.write_obj(mpc_bus, base_mp)
.map_err(|_| MptableError::WriteMpcBus)?;
base_mp = base_mp.unchecked_add(size);
checksum = checksum.wrapping_add(compute_checksum(&mpc_bus.0));
checksum = checksum.wrapping_add(compute_checksum(&mpc_bus));
}
{
let size = mem::size_of::<MpcIoapicWrapper>() as u64;
let mut mpc_ioapic = MpcIoapicWrapper(mpspec::mpc_ioapic::default());
mpc_ioapic.0.type_ = mpspec::MP_IOAPIC as u8;
mpc_ioapic.0.apicid = ioapicid;
mpc_ioapic.0.apicver = APIC_VERSION;
mpc_ioapic.0.flags = mpspec::MPC_APIC_USABLE as u8;
mpc_ioapic.0.apicaddr = IO_APIC_DEFAULT_PHYS_BASE;
let size = mem::size_of::<mpspec::mpc_ioapic>() as u64;
let mpc_ioapic = mpspec::mpc_ioapic {
type_: mpspec::MP_IOAPIC as u8,
apicid: ioapicid,
apicver: APIC_VERSION,
flags: mpspec::MPC_APIC_USABLE as u8,
apicaddr: IO_APIC_DEFAULT_PHYS_BASE,
};
mem.write_obj(mpc_ioapic, base_mp)
.map_err(|_| MptableError::WriteMpcIoapic)?;
base_mp = base_mp.unchecked_add(size);
checksum = checksum.wrapping_add(compute_checksum(&mpc_ioapic.0));
checksum = checksum.wrapping_add(compute_checksum(&mpc_ioapic));
}
// Per kvm_setup_default_irq_routing() in kernel
for i in 0..=u8::try_from(IRQ_MAX).map_err(|_| MptableError::TooManyIrqs)? {
let size = mem::size_of::<MpcIntsrcWrapper>() as u64;
let mut mpc_intsrc = MpcIntsrcWrapper(mpspec::mpc_intsrc::default());
mpc_intsrc.0.type_ = mpspec::MP_INTSRC as u8;
mpc_intsrc.0.irqtype = mpspec::mp_irq_source_types_mp_INT as u8;
mpc_intsrc.0.irqflag = mpspec::MP_IRQPOL_DEFAULT as u16;
mpc_intsrc.0.srcbus = 0;
mpc_intsrc.0.srcbusirq = i;
mpc_intsrc.0.dstapic = ioapicid;
mpc_intsrc.0.dstirq = i;
let size = mem::size_of::<mpspec::mpc_intsrc>() as u64;
let mpc_intsrc = mpspec::mpc_intsrc {
type_: mpspec::MP_INTSRC as u8,
irqtype: mpspec::mp_irq_source_types_mp_INT as u8,
irqflag: mpspec::MP_IRQPOL_DEFAULT as u16,
srcbus: 0,
srcbusirq: i,
dstapic: ioapicid,
dstirq: i,
};
mem.write_obj(mpc_intsrc, base_mp)
.map_err(|_| MptableError::WriteMpcIntsrc)?;
base_mp = base_mp.unchecked_add(size);
checksum = checksum.wrapping_add(compute_checksum(&mpc_intsrc.0));
checksum = checksum.wrapping_add(compute_checksum(&mpc_intsrc));
}
{
let size = mem::size_of::<MpcLintsrcWrapper>() as u64;
let mut mpc_lintsrc = MpcLintsrcWrapper(mpspec::mpc_lintsrc::default());
mpc_lintsrc.0.type_ = mpspec::MP_LINTSRC as u8;
mpc_lintsrc.0.irqtype = mpspec::mp_irq_source_types_mp_ExtINT as u8;
mpc_lintsrc.0.irqflag = mpspec::MP_IRQPOL_DEFAULT as u16;
mpc_lintsrc.0.srcbusid = 0;
mpc_lintsrc.0.srcbusirq = 0;
mpc_lintsrc.0.destapic = 0;
mpc_lintsrc.0.destapiclint = 0;
let size = mem::size_of::<mpspec::mpc_lintsrc>() as u64;
let mpc_lintsrc = mpspec::mpc_lintsrc {
type_: mpspec::MP_LINTSRC as u8,
irqtype: mpspec::mp_irq_source_types_mp_ExtINT as u8,
irqflag: mpspec::MP_IRQPOL_DEFAULT as u16,
srcbusid: 0,
srcbusirq: 0,
destapic: 0,
destapiclint: 0,
};
mem.write_obj(mpc_lintsrc, base_mp)
.map_err(|_| MptableError::WriteMpcLintsrc)?;
base_mp = base_mp.unchecked_add(size);
checksum = checksum.wrapping_add(compute_checksum(&mpc_lintsrc.0));
checksum = checksum.wrapping_add(compute_checksum(&mpc_lintsrc));
}
{
let size = mem::size_of::<MpcLintsrcWrapper>() as u64;
let mut mpc_lintsrc = MpcLintsrcWrapper(mpspec::mpc_lintsrc::default());
mpc_lintsrc.0.type_ = mpspec::MP_LINTSRC as u8;
mpc_lintsrc.0.irqtype = mpspec::mp_irq_source_types_mp_NMI as u8;
mpc_lintsrc.0.irqflag = mpspec::MP_IRQPOL_DEFAULT as u16;
mpc_lintsrc.0.srcbusid = 0;
mpc_lintsrc.0.srcbusirq = 0;
mpc_lintsrc.0.destapic = 0xFF; // to all local APICs
mpc_lintsrc.0.destapiclint = 1;
let size = mem::size_of::<mpspec::mpc_lintsrc>() as u64;
let mpc_lintsrc = mpspec::mpc_lintsrc {
type_: mpspec::MP_LINTSRC as u8,
irqtype: mpspec::mp_irq_source_types_mp_NMI as u8,
irqflag: mpspec::MP_IRQPOL_DEFAULT as u16,
srcbusid: 0,
srcbusirq: 0,
destapic: 0xFF,
destapiclint: 1,
};
mem.write_obj(mpc_lintsrc, base_mp)
.map_err(|_| MptableError::WriteMpcLintsrc)?;
base_mp = base_mp.unchecked_add(size);
checksum = checksum.wrapping_add(compute_checksum(&mpc_lintsrc.0));
checksum = checksum.wrapping_add(compute_checksum(&mpc_lintsrc));
}

// At this point we know the size of the mp_table.
let table_end = base_mp;

{
let mut mpc_table = MpcTableWrapper(mpspec::mpc_table::default());
mpc_table.0.signature = MPC_SIGNATURE;
// it's safe to use unchecked_offset_from because
// table_end > table_base
mpc_table.0.length = table_end.unchecked_offset_from(table_base) as u16;
mpc_table.0.spec = MPC_SPEC;
mpc_table.0.oem = MPC_OEM;
mpc_table.0.productid = MPC_PRODUCT_ID;
mpc_table.0.lapic = APIC_DEFAULT_PHYS_BASE;
checksum = checksum.wrapping_add(compute_checksum(&mpc_table.0));
let mut mpc_table = mpspec::mpc_table {
signature: MPC_SIGNATURE,
// it's safe to use unchecked_offset_from because
// table_end > table_base
length: table_end.unchecked_offset_from(table_base) as u16,
spec: MPC_SPEC,
oem: MPC_OEM,
productid: MPC_PRODUCT_ID,
lapic: APIC_DEFAULT_PHYS_BASE,
..Default::default()
};
checksum = checksum.wrapping_add(compute_checksum(&mpc_table));
#[allow(clippy::cast_possible_wrap)]
let checksum_final = (!checksum).wrapping_add(1) as i8;
mpc_table.0.checksum = checksum_final;
mpc_table.checksum = checksum_final;
mem.write_obj(mpc_table, table_base)
.map_err(|_| MptableError::WriteMpcTable)?;
}
Expand All @@ -303,11 +289,11 @@ mod tests {

fn table_entry_size(type_: u8) -> usize {
match u32::from(type_) {
mpspec::MP_PROCESSOR => mem::size_of::<MpcCpuWrapper>(),
mpspec::MP_BUS => mem::size_of::<MpcBusWrapper>(),
mpspec::MP_IOAPIC => mem::size_of::<MpcIoapicWrapper>(),
mpspec::MP_INTSRC => mem::size_of::<MpcIntsrcWrapper>(),
mpspec::MP_LINTSRC => mem::size_of::<MpcLintsrcWrapper>(),
mpspec::MP_PROCESSOR => mem::size_of::<mpspec::mpc_cpu>(),
mpspec::MP_BUS => mem::size_of::<mpspec::mpc_bus>(),
mpspec::MP_IOAPIC => mem::size_of::<mpspec::mpc_ioapic>(),
mpspec::MP_INTSRC => mem::size_of::<mpspec::mpc_intsrc>(),
mpspec::MP_LINTSRC => mem::size_of::<mpspec::mpc_lintsrc>(),
_ => panic!("unrecognized mpc table entry type: {}", type_),
}
}
Expand Down Expand Up @@ -347,12 +333,9 @@ mod tests {

setup_mptable(&mem, num_cpus).unwrap();

let mpf_intel: MpfIntelWrapper = mem.read_obj(GuestAddress(MPTABLE_START)).unwrap();
let mpf_intel: mpspec::mpf_intel = mem.read_obj(GuestAddress(MPTABLE_START)).unwrap();

assert_eq!(
mpf_intel_compute_checksum(&mpf_intel.0),
mpf_intel.0.checksum
);
assert_eq!(mpf_intel_compute_checksum(&mpf_intel), mpf_intel.checksum);
}

#[test]
Expand All @@ -366,9 +349,9 @@ mod tests {

setup_mptable(&mem, num_cpus).unwrap();

let mpf_intel: MpfIntelWrapper = mem.read_obj(GuestAddress(MPTABLE_START)).unwrap();
let mpc_offset = GuestAddress(u64::from(mpf_intel.0.physptr));
let mpc_table: MpcTableWrapper = mem.read_obj(mpc_offset).unwrap();
let mpf_intel: mpspec::mpf_intel = mem.read_obj(GuestAddress(MPTABLE_START)).unwrap();
let mpc_offset = GuestAddress(u64::from(mpf_intel.physptr));
let mpc_table: mpspec::mpc_table = mem.read_obj(mpc_offset).unwrap();

#[derive(Debug)]
struct Sum(u8);
Expand All @@ -385,7 +368,7 @@ mod tests {
}

let mut sum = Sum(0);
mem.write_to(mpc_offset, &mut sum, mpc_table.0.length as usize)
mem.write_to(mpc_offset, &mut sum, mpc_table.length as usize)
.unwrap();
assert_eq!(sum.0, 0);
}
Expand All @@ -404,15 +387,13 @@ mod tests {
for i in 0..MAX_SUPPORTED_CPUS as u8 {
setup_mptable(&mem, i).unwrap();

let mpf_intel: MpfIntelWrapper = mem.read_obj(GuestAddress(MPTABLE_START)).unwrap();
let mpc_offset = GuestAddress(u64::from(mpf_intel.0.physptr));
let mpc_table: MpcTableWrapper = mem.read_obj(mpc_offset).unwrap();
let mpc_end = mpc_offset
.checked_add(u64::from(mpc_table.0.length))
.unwrap();
let mpf_intel: mpspec::mpf_intel = mem.read_obj(GuestAddress(MPTABLE_START)).unwrap();
let mpc_offset = GuestAddress(u64::from(mpf_intel.physptr));
let mpc_table: mpspec::mpc_table = mem.read_obj(mpc_offset).unwrap();
let mpc_end = mpc_offset.checked_add(u64::from(mpc_table.length)).unwrap();

let mut entry_offset = mpc_offset
.checked_add(mem::size_of::<MpcTableWrapper>() as u64)
.checked_add(mem::size_of::<mpspec::mpc_table>() as u64)
.unwrap();
let mut cpu_count = 0;
while entry_offset < mpc_end {
Expand Down

0 comments on commit 48e1d3f

Please sign in to comment.