impl cpuid_model_family_stepping

Author: francis-starlab

.mythril_githooks/pre-commit

@@ -1,5 +1,6 @@
 use crate::error::Result;
 use crate::vcpu::VCpu;
+use crate::virtdev::lapic::VirtualAPICID;
 use crate::{vcpu, vmexit};
 use arrayvec::ArrayVec;
 use bitfield::bitfield;
@@ -142,15 +143,22 @@ bitfield! {
     mtrr, set_mtrr: 12;
 }
 
-fn cpuid_model_family_stepping(actual: CpuIdResult) -> CpuIdResult {
+fn cpuid_model_family_stepping(cpu: &VCpu, actual: CpuIdResult) -> CpuIdResult {
     let family_model_stepping =
         IntelTypeFamilyModelSteppingIDEaxRes(actual.eax);
     //we can change family_model_stepping, but for now just use actual.
     let eax = family_model_stepping.0;
     let mut brand_cflush_max_initial = BrandCFlushMaxIDsInitialAPIC(actual.ebx);
-    brand_cflush_max_initial.set_apic_id(todo!("Waiting on virtual APICs"));
     brand_cflush_max_initial
-        .set_max_processor_ids(todo!("Waiting on virtual APICs"));
+        .set_apic_id(cpu.local_apic.virtual_apic_id.0 as u32);
+    let max =
+        cpu.vm
+            .virtual_ids
+            .values()
+            .fold(VirtualAPICID(0), |accum, this_id| {
+                *if this_id.0 > accum.0 { this_id } else { accum }
+            });
+    brand_cflush_max_initial.set_max_processor_ids(max.0 as u32);
     let ebx = brand_cflush_max_initial.0;
     let mut features_ecx = FeatureInformationECX(actual.ecx);
     let mut features_edx = FeatureInformationEDX(actual.edx);

mythril/src/emulate/cpuid.rs

@@ -41,7 +41,8 @@ pub fn mp_entry_point() -> ! {
         vm
     };
 
-    let mut vcpu = VCpu::new(vm.clone()).expect("Failed to create vcpu");
+    let mut vcpu =
+        VCpu::new(vm.clone(), core_id).expect("Failed to create vcpu");
 
     let vm_id = vm.id;
     let is_vm_bsp = vm.config.bsp_id() == core_id;
@@ -102,17 +103,24 @@ impl VCpu {
     /// Note that the result must be `Pin`, as the `VCpu` pushes its own
     /// address on to the per-core host stack so it can be retrieved on
     /// VMEXIT.
-    pub fn new(vm: Arc<VirtualMachine>) -> Result<Pin<Box<Self>>> {
+    pub fn new(
+        vm: Arc<VirtualMachine>,
+        core_id: percore::CoreId,
+    ) -> Result<Pin<Box<Self>>> {
         let vmx = vmx::Vmx::enable()?;
         let vmcs = vmcs::Vmcs::new()?.activate(vmx)?;
 
         // Allocate 1MB for host stack space
         let stack = vec![0u8; 1024 * 1024];
 
+        let virtual_core_id = vm.virtual_ids
+            .get(&core_id)
+            .expect("Couldn't find virtual core id. Something is broken when initing virtual_ids");
+
         let mut vcpu = Box::pin(Self {
             vm,
             vmcs,
-            local_apic: virtdev::lapic::LocalApic::new(),
+            local_apic: virtdev::lapic::LocalApic::new(*virtual_core_id),
             stack,
             pending_interrupts: BTreeMap::new(),
         });

mythril/src/vcpu.rs

@@ -103,15 +103,30 @@ impl TryFrom<u16> for ApicRegisterOffset {
     }
 }
 
-#[derive(Default)]
+/// CPUID only returns APIC ID values up to 8 bits.
+/// This does raise the question of what AMD are
+/// going to do when they add more cores to threadripper.
+/// And also what Xeon Phi does.
+/// Apparently it does this:
+/// "The local APIC registers have expanded fields for
+/// the APIC ID, Logical APIC ID, and APIC Destination ID. "
+/// https://www.intel.com/content/dam/www/public/us/en/
+/// documents/product-briefs/xeon-phi
+/// -coprocessor-system-software-developers-guide.pdf
+///
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+pub struct VirtualAPICID(pub u8);
+
 pub struct LocalApic {
     icr_destination: Option<u32>,
+    pub virtual_apic_id: VirtualAPICID,
 }
 
 impl LocalApic {
-    pub fn new() -> Self {
+    pub fn new(virtual_apic_id: VirtualAPICID) -> Self {
         LocalApic {
             icr_destination: None,
+            virtual_apic_id,
         }
     }
 
@@ -213,8 +228,7 @@ impl LocalApic {
         let offset = ApicRegisterOffset::try_from(offset)?;
         match offset {
             ApicRegisterOffset::Simple(ApicRegisterSimpleOffset::ApicId) => {
-                // FIXME(alschwalm): we shouldn't really use the core id for this
-                Ok(percore::read_core_id().raw)
+                Ok(self.virtual_apic_id.0 as u32)
             }
             _ => Ok(0),
         }
@@ -248,6 +262,7 @@ impl LocalApic {
                     }
                     1 => {
                         self.icr_destination = Some(value);
+                        todo!("make sure it is known that acr is value")
                     }
                     _ => unreachable!(),
                 }

mythril/src/virtdev/lapic.rs

@@ -13,6 +13,7 @@ use crate::percore;
 use crate::physdev;
 use crate::time;
 use crate::vcpu;
+use crate::virtdev::lapic::VirtualAPICID;
 use crate::virtdev::{
     self, DeviceEvent, DeviceInteraction, DeviceMap, Event, ResponseEventArray,
 };
@@ -324,7 +325,7 @@ impl VirtualMachineConfig {
             virtual_devices: DeviceMap::default(),
             physical_devices,
             memory,
-            override_cpu_name: todo!(),
+            override_cpu_name: true,
         })
     }
 
@@ -396,6 +397,8 @@ pub struct VirtualMachine {
 
     /// The number of vcpus that are up and waiting to start
     cpus_ready: AtomicU32,
+
+    pub virtual_ids: BTreeMap<percore::CoreId, VirtualAPICID>,
 }
 
 impl VirtualMachine {
@@ -420,13 +423,22 @@ impl VirtualMachine {
             );
         }
 
+        let mut virutal_ids = BTreeMap::new();
+
+        let mut virtual_core = 0;
+        for core in config.cpus.as_slice() {
+            virutal_ids.insert(*core, VirtualAPICID(virtual_core));
+            virtual_core += 1;
+        }
+
         let vm = Arc::new(Self {
-            id: id,
-            config: config,
-            guest_space: guest_space,
+            id,
+            config,
+            guest_space,
             apic_access_page: Raw4kPage([0u8; 4096]),
             logical_apic_state: logical_apic_states,
             cpus_ready: AtomicU32::new(0),
+            virtual_ids: virutal_ids,
         });
 
         // Map the guest local apic addr to the access page. This will be set in each
@@ -527,6 +539,7 @@ impl VirtualMachine {
         apic_state
             .logical_destination
             .store(dest, core::sync::atomic::Ordering::SeqCst);
+        todo!("need to update to handle new apic ids")
     }
 
     /// Resolve a guest GSI to a specific CoreId, vector and interrupt type