tomaka/redshirt

Use MONITOR/MWAIT when available rather than HLT

Opened this issue · 2 comments

At the moment, the x86 executor puts the CPU to a halt when they have nothing to do, and waking them up requires an interrupt, which unfortunately comes at an expensive cost.

See chapter 8.10.4 of volume 3.

Here is a very basic patch that doesn't take into account processor support nor the size of the watched memory lines:

diff --git a/kernel/standalone/src/arch/x86_64/executor.rs b/kernel/standalone/src/arch/x86_64/executor.rs
index f9c0b10..d58bd6a 100644
--- a/kernel/standalone/src/arch/x86_64/executor.rs
+++ b/kernel/standalone/src/arch/x86_64/executor.rs
@@ -73,13 +73,6 @@ impl Executor {
                 // `Future`s that were waiting for an interrupt to happen.
                 interrupts::process_wakers();
 
-                debug_assert!(x86_64::instructions::interrupts::are_enabled());
-                x86_64::instructions::interrupts::disable();
-
-                // We store `true` in `need_ipi` before checking `woken_up`, otherwise there could
-                // be a state where `need_ipi` is `false` but we've already checked `woken_up`.
-                local_wake.need_ipi.store(true, atomic::Ordering::SeqCst);
-
                 if local_wake
                     .woken_up
                     .compare_and_swap(true, false, atomic::Ordering::SeqCst)
@@ -90,10 +83,21 @@ impl Executor {
                     break;
                 }
 
-                // An `sti` opcode only takes effect after the *next* opcode, which is `hlt` here.
-                // It is not possible for an interrupt to happen between `sti` and `hlt`.
-                x86_64::instructions::interrupts::enable();
-                x86_64::instructions::hlt();
+                // TODO: must be write-back cache
+                unsafe {
+                    asm!(
+                        "monitor",
+                        in("eax") local_wake.woken_up.as_mut_ptr(),
+                        in("ecx") 0, in("edx") 0,
+                        options(readonly, nostack, preserves_flags)
+                    );
+                }
+
+                if !local_wake.woken_up.load(atomic::Ordering::SeqCst) {
+                    unsafe {
+                        asm!("mwait", in("eax") 0, in("ecx") 0, options(readonly, nostack, preserves_flags));
+                    }
+                }
             }
         }
     }
@@ -123,17 +127,5 @@ struct Waker {
 impl ArcWake for Waker {
     fn wake_by_ref(arc_self: &Arc<Self>) {
         arc_self.woken_up.store(true, atomic::Ordering::SeqCst);
-
-        if arc_self
-            .need_ipi
-            .compare_and_swap(true, false, atomic::Ordering::SeqCst)
-        {
-            if arc_self.processor_to_wake != arc_self.apic.current_apic_id() {
-                arc_self.apic.send_interprocessor_interrupt(
-                    arc_self.processor_to_wake,
-                    arc_self.interrupt_vector,
-                );
-            }
-        }
     }
 }
diff --git a/kernel/standalone/src/main.rs b/kernel/standalone/src/main.rs
index 34c3cb1..cb9efbf 100644
--- a/kernel/standalone/src/main.rs
+++ b/kernel/standalone/src/main.rs
@@ -19,6 +19,7 @@
 #![no_main]
 #![feature(allocator_api)] // TODO: https://github.com/rust-lang/rust/issues/32838
 #![feature(alloc_error_handler)] // TODO: https://github.com/rust-lang/rust/issues/66741
+#![feature(atomic_mut_ptr)] // TODO: https://github.com/rust-lang/rust/issues/66893
 #![feature(asm)] // TODO: https://github.com/rust-lang/rust/issues/72016
 #![feature(const_if_match)] // TODO: https://github.com/rust-lang/rust/issues/49146
 #![feature(llvm_asm)] // TODO: replace all occurrences of `llvm_asm!` with `asm!`

The most difficult requirement is:

The address range provided to the MONITOR instruction must be of write-back caching type. Only write-back
memory type stores to the monitored address range will trigger the monitor hardware. If the address range is not
in memory of write-back type, the address monitor hardware may not be set up properly or the monitor hardware
may not be armed.

At the moment, this isn't possible.
Depends on #400