From 56b16ce8978a4a75a7c0b143dfa16747b985393b Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 4 Jul 2023 11:27:34 +0200 Subject: [PATCH] Avoid static mut thread locals Instead use UnsafeCell inside the thread locals. --- src/arch/x86_64/gdt.rs | 35 ++++++++++++++++++++--------------- src/arch/x86_64/idt.rs | 2 +- src/arch/x86_64/start.rs | 29 +++++++++++++++-------------- 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/arch/x86_64/gdt.rs b/src/arch/x86_64/gdt.rs index 057f09574d..520c8cdc9d 100644 --- a/src/arch/x86_64/gdt.rs +++ b/src/arch/x86_64/gdt.rs @@ -1,5 +1,6 @@ //! Global descriptor table +use core::cell::UnsafeCell; use core::convert::TryInto; use core::mem; @@ -53,7 +54,7 @@ static mut INIT_GDT: [GdtEntry; 4] = [ ]; #[thread_local] -pub static mut GDT: [GdtEntry; 10] = [ +pub static GDT: UnsafeCell<[GdtEntry; 10]> = UnsafeCell::new([ // Null GdtEntry::new(0, 0, 0, 0), // Kernel code @@ -75,7 +76,7 @@ pub static mut GDT: [GdtEntry; 10] = [ // Unused entry which stores the CPU ID. This is necessary for paranoid interrupts as they have // no other way of determining it. GdtEntry::new(0, 0, 0, 0), -]; +]); #[repr(C, align(16))] pub struct ProcessorControlRegion { @@ -90,7 +91,7 @@ pub struct ProcessorControlRegion { pub struct TssWrapper(pub TaskStateSegment); #[thread_local] -pub static mut KPCR: ProcessorControlRegion = ProcessorControlRegion { +pub static KPCR: UnsafeCell = UnsafeCell::new(ProcessorControlRegion { tcb_end: 0, user_rsp_tmp: 0, tss: TssWrapper(TaskStateSegment { @@ -102,18 +103,18 @@ pub static mut KPCR: ProcessorControlRegion = ProcessorControlRegion { reserved4: 0, iomap_base: 0xFFFF }), -}; +}); #[cfg(feature = "pti")] pub unsafe fn set_tss_stack(stack: usize) { use super::pti::{PTI_CPU_STACK, PTI_CONTEXT_STACK}; - KPCR.tss.0.rsp[0] = (PTI_CPU_STACK.as_ptr() as usize + PTI_CPU_STACK.len()) as u64; + (*KPCR.get()).tss.0.rsp[0] = (PTI_CPU_STACK.as_ptr() as usize + PTI_CPU_STACK.len()) as u64; PTI_CONTEXT_STACK = stack; } #[cfg(not(feature = "pti"))] pub unsafe fn set_tss_stack(stack: usize) { - KPCR.tss.0.rsp[0] = stack as u64; + (*KPCR.get()).tss.0.rsp[0] = stack as u64; } // Initialize GDT @@ -152,13 +153,13 @@ pub unsafe fn init_paging(cpu_id: u32, tcb_offset: usize, stack_offset: usize) { // Now that we have access to thread locals, begin by getting a pointer to the Processor // Control Region. - let kpcr = &mut KPCR; + let kpcr = KPCR.get(); // Then, setup the AP's individual GDT - let limit = (GDT.len() * mem::size_of::() - 1) + let limit = ((*GDT.get()).len() * mem::size_of::() - 1) .try_into() .expect("main GDT way too large"); - let base = GDT.as_ptr() as *const SegmentDescriptor; + let base = GDT.get() as *const SegmentDescriptor; let gdtr: DescriptorTablePointer = DescriptorTablePointer { limit, @@ -166,23 +167,27 @@ pub unsafe fn init_paging(cpu_id: u32, tcb_offset: usize, stack_offset: usize) { }; // Once we have fetched the real KPCR address, set the TLS segment to the TCB pointer there. - kpcr.tcb_end = (tcb_offset as *const usize).read(); + (*kpcr).tcb_end = (tcb_offset as *const usize).read(); { // We can now access our TSS, via the KPCR, which is a thread local - let tss = &kpcr.tss.0 as *const _ as usize as u64; + let tss = &(*kpcr).tss.0 as *const _ as usize as u64; let tss_lo = (tss & 0xFFFF_FFFF) as u32; let tss_hi = (tss >> 32) as u32; - GDT[GDT_TSS].set_offset(tss_lo); - GDT[GDT_TSS].set_limit(mem::size_of::() as u32); + (*GDT.get())[GDT_TSS].set_offset(tss_lo); + (*GDT.get())[GDT_TSS].set_limit(mem::size_of::() as u32); - (&mut GDT[GDT_TSS_HIGH] as *mut GdtEntry).cast::().write(tss_hi); + (&mut (*GDT.get())[GDT_TSS_HIGH] as *mut GdtEntry) + .cast::() + .write(tss_hi); } // And finally, populate the last GDT entry with the current CPU ID, to allow paranoid // interrupt handlers to safely use TLS. - (&mut GDT[GDT_CPU_ID_CONTAINER] as *mut GdtEntry).cast::().write(cpu_id); + (&mut (*GDT.get())[GDT_CPU_ID_CONTAINER] as *mut GdtEntry) + .cast::() + .write(cpu_id); // Set the stack pointer to use when coming back from userspace. set_tss_stack(stack_offset); diff --git a/src/arch/x86_64/idt.rs b/src/arch/x86_64/idt.rs index f9d3ddef78..5107ab9206 100644 --- a/src/arch/x86_64/idt.rs +++ b/src/arch/x86_64/idt.rs @@ -176,7 +176,7 @@ pub unsafe fn init_generic(is_bsp: bool, idt: &mut Idt) { let address = base_address.data() + BACKUP_STACK_SIZE; // Put them in the 1st entry of the IST. - crate::gdt::KPCR.tss.0.ist[usize::from(index - 1)] = address as u64; + (*crate::gdt::KPCR.get()).tss.0.ist[usize::from(index - 1)] = address as u64; index }; diff --git a/src/arch/x86_64/start.rs b/src/arch/x86_64/start.rs index 92e3bbcac0..5bcc17f507 100644 --- a/src/arch/x86_64/start.rs +++ b/src/arch/x86_64/start.rs @@ -3,6 +3,7 @@ /// It must create the IDT with the correct entries, those entries are /// defined in other files inside of the `arch` module +use core::cell::Cell; use core::slice; use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -26,10 +27,10 @@ static BSS_TEST_ZERO: usize = 0; static DATA_TEST_NONZERO: usize = usize::max_value(); /// Test of zero values in thread BSS #[thread_local] -static mut TBSS_TEST_ZERO: usize = 0; +static TBSS_TEST_ZERO: Cell = Cell::new(0); /// Test of non-zero values in thread data. #[thread_local] -static mut TDATA_TEST_NONZERO: usize = usize::max_value(); +static TDATA_TEST_NONZERO: Cell = Cell::new(usize::max_value()); pub static KERNEL_BASE: AtomicUsize = AtomicUsize::new(0); pub static KERNEL_SIZE: AtomicUsize = AtomicUsize::new(0); @@ -144,12 +145,12 @@ pub unsafe extern fn kstart(args_ptr: *const KernelArgs) -> ! { // Test tdata and tbss { - assert_eq!(TBSS_TEST_ZERO, 0); - TBSS_TEST_ZERO += 1; - assert_eq!(TBSS_TEST_ZERO, 1); - assert_eq!(TDATA_TEST_NONZERO, usize::max_value()); - TDATA_TEST_NONZERO -= 1; - assert_eq!(TDATA_TEST_NONZERO, usize::max_value() - 1); + assert_eq!(TBSS_TEST_ZERO.get(), 0); + TBSS_TEST_ZERO.set(TBSS_TEST_ZERO.get() + 1); + assert_eq!(TBSS_TEST_ZERO.get(), 1); + assert_eq!(TDATA_TEST_NONZERO.get(), usize::max_value()); + TDATA_TEST_NONZERO.set(TDATA_TEST_NONZERO.get() - 1); + assert_eq!(TDATA_TEST_NONZERO.get(), usize::max_value() - 1); } // Reset AP variables @@ -249,12 +250,12 @@ pub unsafe extern fn kstart_ap(args_ptr: *const KernelArgsAp) -> ! { // Test tdata and tbss { - assert_eq!(TBSS_TEST_ZERO, 0); - TBSS_TEST_ZERO += 1; - assert_eq!(TBSS_TEST_ZERO, 1); - assert_eq!(TDATA_TEST_NONZERO, usize::max_value()); - TDATA_TEST_NONZERO -= 1; - assert_eq!(TDATA_TEST_NONZERO, usize::max_value() - 1); + assert_eq!(TBSS_TEST_ZERO.get(), 0); + TBSS_TEST_ZERO.set(TBSS_TEST_ZERO.get() + 1); + assert_eq!(TBSS_TEST_ZERO.get(), 1); + assert_eq!(TDATA_TEST_NONZERO.get(), usize::max_value()); + TDATA_TEST_NONZERO.set(TDATA_TEST_NONZERO.get() - 1); + assert_eq!(TDATA_TEST_NONZERO.get(), usize::max_value() - 1); } // Initialize devices (for AP)