Read one mouse packet byte per interrupt

kernel/mouse/src/lib.rs

@@ -4,7 +4,7 @@
 #![feature(abi_x86_interrupt)]
 
 use log::{error, warn};
-use spin::Once;
+use spin::{Mutex, Once};
 use mpmc::Queue;
 use event_types::Event;
 use x86_64::structures::idt::InterruptStackFrame;
@@ -15,11 +15,51 @@ use ps2::{PS2Mouse, MousePacket};
 /// Because we perform the typical PIC remapping, the remapped IRQ vector number is 0x2C.
 const PS2_MOUSE_IRQ: u8 = interrupts::IRQ_BASE_OFFSET + 0xC;
 
+const PS2_MAX_MOUSE_BYTES: usize = 4;
+
 static MOUSE: Once<MouseInterruptParams> = Once::new();
 
+/// Everything we need in [`ps2_mouse_handler`].
 struct MouseInterruptParams {
     mouse: PS2Mouse<'static>,
     queue: Queue<Event>,
+    packet_bytes: PacketBytes,
+}
+
+/// Somewhat like an array/vec mixture to allow pushing single bytes
+/// of a mouse packet per interrupt into an array.
+/// This can handle MouseId 0 (3 bytes) and 3, 4 (4 bytes).
+struct PacketBytes {
+    len: usize,
+    inner: Mutex<[u8; PS2_MAX_MOUSE_BYTES]>,
+    // where to push the next element
+    cursor: Mutex<usize>,
+}
+
+impl PacketBytes {
+    const fn new(len: usize) -> Self {
+        Self { len, inner: Mutex::new([0; PS2_MAX_MOUSE_BYTES]), cursor: Mutex::new(0) }
+    }
+    // TODO: we can use a u32 once we switch to the bilge crate
+    fn push(&self, value: u8) {
+        let mut cursor = self.cursor.lock();
+        if *cursor < PS2_MAX_MOUSE_BYTES {
+            self.inner.lock()[*cursor] = value;
+            *cursor += 1;
+        }
+    }
+
+    /// Return the packet bytes if they're filled
+    fn filled_bytes(&self) -> Option<[u8; 4]> {
+        let mut cursor = self.cursor.lock();
+
+        if *cursor == self.len {
+            *cursor = 0;
+            Some(*self.inner.lock())
+        } else {
+            None
+        }
+    }
 }
 
 /// Initialize the PS/2 mouse driver and register its interrupt handler.
@@ -41,9 +81,13 @@ pub fn init(mut mouse: PS2Mouse<'static>, mouse_queue_producer: Queue<Event>) ->
         "PS/2 mouse IRQ was already in use! Sharing IRQs is currently unsupported."
     })?;
 
+    // Initialize the mouse packet bytes, which will be filled by 3-4 interrupts,
+    // depending on the MouseId
+    let packet_bytes = PacketBytes::new(mouse.packet_size());
+
     // Final step: set the producer end of the mouse event queue.
     // Also add the mouse struct for access during interrupts.
-    MOUSE.call_once(|| MouseInterruptParams { mouse, queue: mouse_queue_producer });
+    MOUSE.call_once(|| MouseInterruptParams { mouse, queue: mouse_queue_producer, packet_bytes });
     Ok(())
 }
 
@@ -56,18 +100,20 @@ pub fn init(mut mouse: PS2Mouse<'static>, mouse_queue_producer: Queue<Event>) ->
 /// 
 /// In some cases (e.g. on device init), [the PS/2 controller can also send an interrupt](https://wiki.osdev.org/%228042%22_PS/2_Controller#Interrupts).
 extern "x86-interrupt" fn ps2_mouse_handler(_stack_frame: InterruptStackFrame) {
-    if let Some(MouseInterruptParams { mouse, queue }) = MOUSE.get() {
+    if let Some(MouseInterruptParams { mouse, queue, packet_bytes }) = MOUSE.get() {
+        // using `while` here didn't interact well with the window manager and increases handler runtime
         if mouse.is_output_buffer_full() {
-            // NOTE: having read some more forum comments now, if this ever breaks on real hardware,
-            // try to redesign this to only get one byte per interrupt instead of the 3-4 bytes we
-            // currently get in read_mouse_packet and merge them afterwards
-            let mouse_packet = mouse.read_mouse_packet();
-
-            if mouse_packet.always_one() != 1 {
-                // this could signal a hardware error or a mouse which doesn't conform to the rule
-                warn!("ps2_mouse_handler(): Discarding mouse data packet since its third bit should always be 1.");
-            } else if let Err(e) = handle_mouse_input(mouse_packet, queue) {
-                error!("ps2_mouse_handler(): {e:?}");
+            packet_bytes.push(mouse.read_packet_byte());
+
+            if let Some(bytes) = packet_bytes.filled_bytes() {
+                let mouse_packet = mouse.packet_from_bytes(bytes);
+
+                if mouse_packet.always_one() != 1 {
+                    // this could signal a hardware error or a mouse which doesn't conform to the rule
+                    warn!("ps2_mouse_handler(): Discarding mouse data packet since its third bit should always be 1.");
+                } else if let Err(e) = handle_mouse_input(mouse_packet, queue) {
+                    error!("ps2_mouse_handler(): {e:?}");
+                }
             }
         }
     } else {

kernel/ps2/src/lib.rs

@@ -427,30 +427,37 @@ impl<'c> PS2Mouse<'c> {
             .map_err(|_| "failed to set the mouse resolution")
     }
 
-    /// read the correct [MousePacket] according to [MouseId]
-    pub fn read_mouse_packet(&self) -> MousePacket {
-        let read_data = || self.controller.read_data();
+    /// construct the correct [MousePacket] according to [MouseId]
+    pub fn packet_from_bytes(&self, bytes: [u8; 4]) -> MousePacket {
         match self.id {
             MouseId::Zero => MousePacket::Zero(
-                MousePacketGeneric::from_bytes([
-                    read_data(), read_data(), read_data()
-                ])
+                MousePacketGeneric::from_bytes([bytes[0], bytes[1], bytes[2]])
             ),
             MouseId::Three => MousePacket::Three(
-                MousePacket3::from_bytes([
-                    read_data(), read_data(), read_data(), read_data()
-                ])
+                MousePacket3::from_bytes(bytes)
             ),
             MouseId::Four => MousePacket::Four(
-                MousePacket4::from_bytes([
-                    read_data(), read_data(), read_data(), read_data()
-                ])
+                MousePacket4::from_bytes(bytes)
             ),
         }
     }
 
-    /// Returns `true` if there is content in the PS/2 Mouse's output buffer
-    /// that can be read from.
+    /// read one byte of mouse packet from the PS/2 data port
+    pub fn read_packet_byte(&self) -> u8 {
+        self.controller.read_data()
+    }
+
+    /// get the mouse packet size corresponding to the mouse id
+    pub fn packet_size(&self) -> usize {
+        match self.id {
+            MouseId::Zero => 3,
+            _ => 4,
+        }
+    }
+
+    /// Convenience method to see if the mouse can be polled
+    /// 
+    /// Returns `true` if there is content in the PS/2 Mouse's output buffer.
     /// 
     /// This also checks the status register's `output_buffer_full` bit.
     /// Otherwise `mouse_id` would read ACK (0xFA) instead of mouse id.
@@ -596,6 +603,8 @@ impl<'c> PS2Keyboard<'c> {
     }
 
     /// Convenience method to see if the keyboard can be polled
+    /// 
+    /// Note: we don't need to check !mouse_output_buffer_full().
     fn is_output_buffer_full(&self) -> bool {
         self.controller.status_register().output_buffer_full()
     }