fix load_tss and GlobalDescriptorTable

[Freax13]

ltr atomically writes to memory when it changes the referenced tss entry's type to Busy 64-bit TSS. This means that the inline assembly in load_tss should not have the nomem option and that the table field in GlobalDescriptorTable must use a type that is interiorly mutable.

Unfortunately this is a breaking change because I changed the type of GlobalDescriptorTable::table to [AtomicU64; 8] and GlobalDescriptorTable::as_raw_slice exposes this field to the public api. Returning &[u64] wouldn't be safe anyways because [u64] isn't interioly mutable.

Technically this means that the old code was probably UB, but my guess is it's unlikely that anyone was actually affected by this.

#322 initially brought up that load_tss changes the GDT. This pr does not fix #322, but #322 should be easy to fix in a follow up pr once this pr is merged.

[Freax13]

I also think that we should reevaluate some of the safety comments on GlobalDescriptorTable::load_unsafe. I don't see anything in Intel's or AMD's manuals that says that a GDT mustn't be modified after it's loaded and IIRC it should even be perfectly fine to have GDTR pointing to garbage memory as long as no new selectors are loaded.

[phil-opp]

ltr atomically writes to memory when it changes the referenced tss entry's type to Busy 64-bit TSS. This means that the inline assembly in load_tss should not have the nomem option and that the table field in GlobalDescriptorTable must use a type that is interiorly mutable.

Good catch!

I don't see anything in Intel's or AMD's manuals that says that a GDT mustn't be modified after it's loaded and IIRC it should even be perfectly fine to have GDTR pointing to garbage memory as long as no new selectors are loaded.

AFAIK, the segment registers are reloaded on every iret or user/kernel mode switch, and probably in more cases. So the GDT should stay valid as long as it's loaded.

I don't think that it's a good idea to share the same TSS across cores (even in 64-bit mode) because it contains tables of stack pointers, to which the CPU automatically switches (e.g. on certain interrupts or on privilege switches). By sharing the same TSS across cores, we risk that they share the same stack pointer in some situations, which is a very bad idea.

[Freax13]

I don't see anything in Intel's or AMD's manuals that says that a GDT mustn't be modified after it's loaded and IIRC it should even be perfectly fine to have GDTR pointing to garbage memory as long as no new selectors are loaded.

AFAIK, the segment registers are reloaded on every iret or user/kernel mode switch, and probably in more cases. So the GDT should stay valid as long as it's loaded.

Oops, fair enough, iret, far jumps, calls and returns, etc load from the GDT. Interestingly enough syscall and sysret load fixed values instead.

I don't think that it's a good idea to share the same TSS across cores (even in 64-bit mode) because it contains tables of stack pointers, to which the CPU automatically switches (e.g. on certain interrupts or on privilege switches). By sharing the same TSS across cores, we risk that they share the same stack pointer in some situations, which is a very bad idea.

I agree that it's probably not the best idea, but if a kernel was mapping the same pages for the stack with different backing physical frames, it should be possible to share a TSS safely.

[Freax13]

Perhaps I've been approaching this the wrong way then: If we consider the GDT a per core structure, we can make GlobalDescriptorTable::load take a mutable reference and don't have to deal with interior mutability anymore. This would also have the added advantage of not requiring AtomicU64 which isn't supported on all platforms.

[phil-opp]

I agree that it's probably not the best idea, but if a kernel was mapping the same pages for the stack with different backing physical frames, it should be possible to share a TSS safely.

Fair, but I don't think that this is a common approach. And the GDT isn't that large, so the overhead of duplicating should be negligible.

Perhaps I've been approaching this the wrong way then: If we consider the GDT a per core structure, we can make GlobalDescriptorTable::load take a mutable reference and don't have to deal with interior mutability anymore. This would also have the added advantage of not requiring AtomicU64 which isn't supported on all platforms.

I like the approach, but we then need an easy and safe way to create a &'static mut. Using static mut is discouraged and UnsafeCell is unsafe. Maybe there is some crate that wraps an UnsafeCell and an AtomicBool that provides a safe try_get_mut(&self) -> Option<&mut T> method that only returns Some once? If not, we could write one ourselves...

[Freax13]

I like the approach, but we then need an easy and safe way to create a &'static mut. Using static mut is discouraged and UnsafeCell is unsafe. Maybe there is some crate that wraps an UnsafeCell and an AtomicBool that provides a safe try_get_mut(&self) -> Option<&mut T> method that only returns Some once? If not, we could write one ourselves...

Personally I just use Box::leak however that requires setting up a heap before setting up the GDT. The cortex-m crate has a singleton! macro that almost does what you described. Their macro internally uses a static mut without any synchronization and initializes the value at runtime. I'd suggest that we'd store a UnsafeCell<T> and an AtomicBool in static global variables and do the checks atomically instead. It feels a bit weird to initialize a global variable at runtime, but I also can't think of any downsides too this, so I'm fine with keeping it this way.

[phil-opp]

Personally I just use Box::leak however that requires setting up a heap before setting up the GDT.

Yeah, Box::leak works well for this, but we should still support setting up the GDT without alloc.

The cortex-m crate has a singleton! macro that almost does what you described. Their macro internally uses a static mut without any synchronization and initializes the value at runtime.

I think they rely on the fact that most (all?) cortex m CPUs only have a single core. So we shouldn't do it this way.

I'd suggest that we'd store a UnsafeCell<T> and an AtomicBool in static global variables and do the checks atomically instead. It feels a bit weird to initialize a global variable at runtime, but I also can't think of any downsides too this, so I'm fine with keeping it this way.

The constructor functions of UnsafeCell and AtomicBool are both const, so we should still be able to initialize everything at compile time, e.g. using a Singleton::new(...) function. The Singleton type would then provide a try_get_mut method (just an example name) to return an Option<&'static mut T> and update the flag.

[phil-opp]

Looks good now!