Separate logical and hardware cpu ids

src/acpi/madt/arch/x86.rs

@@ -9,7 +9,7 @@ use crate::{
     device::local_apic::the_local_apic,
     memory::{allocate_p2frame, Frame, KernelMapper},
     paging::{Page, PageFlags, PhysicalAddress, RmmA, RmmArch, VirtualAddress, PAGE_SIZE},
-    start::{kstart_ap, AP_READY, CPU_COUNT},
+    start::{kstart_ap, AP_READY},
 };
 
 use super::{Madt, MadtEntry};
@@ -61,10 +61,7 @@ pub(super) fn init(madt: Madt) {
                         println!("        This is my local APIC");
                     } else {
                         if ap_local_apic.flags & 1 == 1 {
-                            // Increase CPU ID
-                            CPU_COUNT.fetch_add(1, Ordering::SeqCst);
-
-                            let cpu_id = LogicalCpuId::new(ap_local_apic.processor.into());
+                            let cpu_id = LogicalCpuId::next();
 
                             // Allocate a stack
                             let stack_start = allocate_p2frame(4)

src/arch/aarch64/start.rs

@@ -4,7 +4,7 @@
 //! defined in other files inside of the `arch` module
 use core::{
     slice,
-    sync::atomic::{AtomicBool, AtomicU32, Ordering},
+    sync::atomic::{AtomicBool, Ordering},
 };
 
 use fdt::Fdt;
@@ -25,7 +25,6 @@ static mut BSS_TEST_ZERO: usize = 0;
 /// Test of non-zero values in data.
 static mut DATA_TEST_NONZERO: usize = 0xFFFF_FFFF_FFFF_FFFF;
 
-pub static CPU_COUNT: AtomicU32 = AtomicU32::new(0);
 pub static AP_READY: AtomicBool = AtomicBool::new(false);
 static BSP_READY: AtomicBool = AtomicBool::new(false);
 
@@ -174,7 +173,6 @@ pub unsafe extern "C" fn kstart(args_ptr: *const KernelArgs) -> ! {
             crate::misc::init(crate::cpu_set::LogicalCpuId::new(0));
 
             // Reset AP variables
-            CPU_COUNT.store(1, Ordering::SeqCst);
             AP_READY.store(false, Ordering::SeqCst);
             BSP_READY.store(false, Ordering::SeqCst);
 
@@ -215,7 +213,7 @@ pub unsafe extern "C" fn kstart(args_ptr: *const KernelArgs) -> ! {
             }
         };
 
-        crate::kmain(CPU_COUNT.load(Ordering::SeqCst), bootstrap);
+        crate::kmain(bootstrap);
     }
 }
 

src/arch/riscv64/start.rs

@@ -1,7 +1,7 @@
 use core::{
     arch::asm,
     slice,
-    sync::atomic::{AtomicU32, AtomicUsize, Ordering},
+    sync::atomic::{AtomicUsize, Ordering},
 };
 use fdt::Fdt;
 
@@ -24,7 +24,6 @@ static mut BSS_TEST_ZERO: usize = 0;
 /// Test of non-zero values in data.
 static mut DATA_TEST_NONZERO: usize = 0xFFFF_FFFF_FFFF_FFFF;
 
-pub static CPU_COUNT: AtomicU32 = AtomicU32::new(0);
 pub static BOOT_HART_ID: AtomicUsize = AtomicUsize::new(0);
 
 #[repr(packed)]
@@ -186,8 +185,6 @@ pub unsafe extern "C" fn kstart(args_ptr: *const KernelArgs) -> ! {
 
             crate::misc::init(crate::cpu_set::LogicalCpuId::new(0));
 
-            CPU_COUNT.store(1, Ordering::SeqCst);
-
             // Setup kernel heap
             allocator::init();
 
@@ -207,6 +204,6 @@ pub unsafe extern "C" fn kstart(args_ptr: *const KernelArgs) -> ! {
             bootstrap
         };
 
-        crate::kmain(CPU_COUNT.load(Ordering::SeqCst), bootstrap);
+        crate::kmain(bootstrap);
     }
 }

src/arch/x86_shared/start.rs

@@ -5,7 +5,7 @@
 use core::{
     cell::SyncUnsafeCell,
     hint, slice,
-    sync::atomic::{AtomicBool, AtomicU32, Ordering},
+    sync::atomic::{AtomicBool, Ordering},
 };
 
 #[cfg(feature = "acpi")]
@@ -26,9 +26,6 @@ static BSS_TEST_ZERO: SyncUnsafeCell<usize> = SyncUnsafeCell::new(0);
 /// Test of non-zero values in data.
 static DATA_TEST_NONZERO: SyncUnsafeCell<usize> = SyncUnsafeCell::new(usize::max_value());
 
-// TODO: This probably shouldn't be an atomic. Only the BSP starts APs.
-pub static CPU_COUNT: AtomicU32 = AtomicU32::new(0);
-
 pub static AP_READY: AtomicBool = AtomicBool::new(false);
 static BSP_READY: AtomicBool = AtomicBool::new(false);
 
@@ -164,7 +161,6 @@ pub unsafe extern "C" fn kstart(args_ptr: *const KernelArgs) -> ! {
             interrupt::syscall::init();
 
             // Reset AP variables
-            CPU_COUNT.store(1, Ordering::SeqCst);
             AP_READY.store(false, Ordering::SeqCst);
             BSP_READY.store(false, Ordering::SeqCst);
 
@@ -212,7 +208,7 @@ pub unsafe extern "C" fn kstart(args_ptr: *const KernelArgs) -> ! {
             }
         };
 
-        crate::kmain(CPU_COUNT.load(Ordering::SeqCst), bootstrap);
+        crate::kmain(bootstrap);
     }
 }
 

src/cpu_set.rs

@@ -2,11 +2,11 @@ use core::sync::atomic::{AtomicUsize, Ordering};
 
 use alloc::string::{String, ToString};
 
+use crate::CPU_COUNT;
+
 /// A unique number used internally by the kernel to identify CPUs.
 ///
 /// This is usually but not necessarily the same as the APIC ID.
-
-// TODO: Differentiate between logical CPU IDs and hardware CPU IDs (e.g. APIC IDs)
 #[derive(Clone, Copy, Eq, PartialEq, Hash)]
 // TODO: NonMaxUsize?
 // TODO: Optimize away this type if not cfg!(feature = "multi_core")
@@ -15,6 +15,12 @@ pub struct LogicalCpuId(u32);
 impl LogicalCpuId {
     pub const BSP: Self = Self::new(0);
 
+    pub fn next() -> Self {
+        let id = CPU_COUNT.fetch_add(1, Ordering::Relaxed);
+        assert!(id < MAX_CPU_COUNT);
+        Self(id)
+    }
+
     pub const fn new(inner: u32) -> Self {
         Self(inner)
     }

src/main.rs

@@ -155,7 +155,7 @@ fn cpu_id() -> crate::cpu_set::LogicalCpuId {
 }
 
 /// The count of all CPUs that can have work scheduled
-static CPU_COUNT: AtomicU32 = AtomicU32::new(0);
+static CPU_COUNT: AtomicU32 = AtomicU32::new(1);
 
 /// Get the number of CPUs currently active
 #[inline(always)]
@@ -180,16 +180,14 @@ struct Bootstrap {
 static BOOTSTRAP: spin::Once<Bootstrap> = spin::Once::new();
 
 /// This is the kernel entry point for the primary CPU. The arch crate is responsible for calling this
-fn kmain(cpu_count: u32, bootstrap: Bootstrap) -> ! {
-    CPU_COUNT.store(cpu_count, Ordering::SeqCst);
-
+fn kmain(bootstrap: Bootstrap) -> ! {
     //Initialize the first context, stored in kernel/src/context/mod.rs
     context::init();
 
     //Initialize global schemes, such as `acpi:`.
     scheme::init_globals();
 
-    info!("BSP: {}", cpu_count);
+    info!("BSP: {}", cpu_count());
     info!("Env: {:?}", ::core::str::from_utf8(bootstrap.env));
 
     BOOTSTRAP.call_once(|| bootstrap);

src/memory/kernel_mapper.rs

@@ -1,12 +1,12 @@
 use crate::cpu_set::LogicalCpuId;
 use core::sync::{
     atomic,
-    atomic::{AtomicUsize, Ordering},
+    atomic::{AtomicU32, AtomicUsize, Ordering},
 };
 use rmm::{PageMapper, TableKind};
 
-const NO_PROCESSOR: usize = !0;
-static LOCK_OWNER: AtomicUsize = AtomicUsize::new(NO_PROCESSOR);
+const NO_PROCESSOR: u32 = !0;
+static LOCK_OWNER: AtomicU32 = AtomicU32::new(NO_PROCESSOR);
 static LOCK_COUNT: AtomicUsize = AtomicUsize::new(0);
 
 // TODO: Support, perhaps via const generics, embedding address checking in PageMapper, thereby
@@ -23,17 +23,17 @@ pub struct KernelMapper {
     ro: bool,
 }
 impl KernelMapper {
-    fn lock_inner(current_processor: usize) -> bool {
+    fn lock_inner(current_processor: LogicalCpuId) -> bool {
         loop {
             match LOCK_OWNER.compare_exchange_weak(
                 NO_PROCESSOR,
-                current_processor,
+                current_processor.get(),
                 Ordering::Acquire,
                 Ordering::Relaxed,
             ) {
                 Ok(_) => break,
                 // already owned by this hardware thread
-                Err(id) if id == current_processor => break,
+                Err(id) if id == current_processor.get() => break,
                 // either CAS failed, or some other hardware thread holds the lock
                 Err(_) => core::hint::spin_loop(),
             }
@@ -48,7 +48,7 @@ impl KernelMapper {
         current_processor: LogicalCpuId,
         mapper: crate::paging::PageMapper,
     ) -> Self {
-        let ro = Self::lock_inner(current_processor.get() as usize);
+        let ro = Self::lock_inner(current_processor);
         Self { mapper, ro }
     }
     pub fn lock_manually(current_processor: LogicalCpuId) -> Self {