Ps2 incremental ms #50

andreea-35 · 2025-09-26T13:19:42Z

Pull Request Overview

This PR wires the QEMU i486 “Q35” board to the PS/2 mouse:

Testing Strategy

This pull request was tested by running on QEMU.

TODO or Help Wanted

Mouse packets aren't getting displayed as they should. Scan codes don't get printed upon key press either.

Documentation Updated

Updated the relevant files in /docs, or no updates are required.

Formatting

Ran make prepush.

step1: PS/2 controller self-test (clock & IRQ still disabled) step2: enable keyboard clock (IRQ 1 still masked) step3: full PS/2 driver – self-test, clock on, IRQ1 unmasked, single-byte buffering step4: drain-loop IRQ handler; full ring-buffer active step5: add send_with_ack() with RESEND retry; enable keyboard scanning (0xF4) step7: implemented some unit tests to debug buffers fix: readded comments from previous controller, moved ps2.init() to enable debug macro feature: added IRQ1 self-test when init.

workflow: rebased with upstream

…e PIC access

…for init, reworked ISR to handle drop parity/timeout => make/break logs + prefixes

…e ring

…lf to keep ISR tny, minor polish

…dIoT/tock into ps2-incremental

old --> new process

added content for dv_ms.rs must be revised in the future

also minor changes to mouse

initialisation of ms_buffer used the keyboard buffer's constant instead of the mouse's

domnudragota · 2025-09-26T14:00:51Z

If I recall correctly, I disabled the output of keycodes/ASCII a while ago on the controller. They still are sent/received, but not shown on VGA. You can enable them for testing in the controller plus add your debug packets messages for your driver. Let me check.

domnudragota · 2025-09-26T14:06:53Z

chips/x86_q35/src/chip.rs

+    // pub fn with_mouse(mut self, mouse: &'a crate::dv_ms::Mouse<'a>) -> Self {
+    //     // ← new
+    //     self.mouse = Some(mouse);
+    //     self
+    // }


Why are these lines commented ? Are they temporarily ?

domnudragota · 2025-09-26T14:07:02Z

chips/x86_q35/src/chip.rs

+    /// PS/2 Mouse
+    // pub ms: &'a crate::dv_ms::Mouse<'a>,
+


Why are these lines commented ? Are they temporarily ?

domnudragota · 2025-09-26T14:07:07Z

chips/x86_q35/src/chip.rs

+        // PS/2 instance supplied via .with_ps2(...)
+        let ps2 = self.ps2.expect("PcComponent::with_ps2 was not called");
+        //let mouse = self.mouse.expect("PcComponent::with_mouse was not called"); // added new 4 the mouse


This is from an older version of the PS/2 controller.

domnudragota · 2025-09-26T14:09:47Z

boards/qemu_i486_q35/src/main.rs

+    // Build the PC chip and attach our
+    // PS/2 controller so that `service_pending_interrupts` will dispatch IRQ1.
    let chip = PcComponent::new(
        &mut *ptr::addr_of_mut!(PAGE_DIR),
        &mut *ptr::addr_of_mut!(PAGE_TABLE),
    )
+    .with_ps2(ps2)


This was changed removed in the newest version of the controller.

domnudragota · 2025-09-26T14:22:27Z

chips/x86_q35/src/ps2.rs

+impl DeferredCallClient for Ps2Controller {
+    fn handle_deferred_call(&self) {
+        // drain and print MAKE/BREAKs
+        self.decode_and_log_stream();


I think you're draining the BH twice by adding the keyboard and mouse bytes. You can write smth small that sends, receives and logs at the same time the mouse, in order to separate from keyboard.

domnudragota · 2025-09-26T14:47:42Z

chips/x86_q35/src/ps2.rs

+pub trait Ps2Client {
+    fn receive_scancode(&self, code: u8);
+}
+


Define a trait here for Ps2MouseClient, you're setting it below but not declaring it.

domnudragota · 2025-09-26T14:49:03Z

chips/x86_q35/src/ps2.rs

+            ms_buffer: RefCell::new([[0; MOUSE_PACKET_SIZE]; MOUSE_BUFFER_SIZE]),
+            ms_head: Cell::new(0),
+            ms_tail: Cell::new(0),
+            ms_count: Cell::new(0),
+            ms_packet: Cell::new([0; MOUSE_PACKET_SIZE]),
+            ms_index: Cell::new(0),
+            ms_overruns: Cell::new(0),
+            mouse_client: OptionalCell::empty(), //added mouse client 4 cursor delete if needed, just for testing


Is it possible that the controller could only send bytes and leave the packaging job to the mouse driver ?

domnudragota · 2025-09-26T14:53:20Z

chips/x86_q35/src/ps2.rs

+            self.deferred_call.set();
+        }
+    }
+    pub fn handle_mouse_interrupt(&self) {


I think you can combine these 2 handle_interrupt functions into 1

domnudragota · 2025-09-26T14:54:21Z

chips/x86_q35/src/ps2.rs

+/// PS/2 controller driver (the “8042” peripheral)
+pub struct Ps2Controller {


Just before here, you can write a small helper function to init. in chip.rs

domnudragota · 2025-09-26T14:54:47Z

chips/x86_q35/src/ps2.rs

+    ms_buffer: RefCell<[[u8; MOUSE_PACKET_SIZE]; MOUSE_BUFFER_SIZE]>,
+    ms_head: Cell<usize>,
+    ms_tail: Cell<usize>,
+    ms_count: Cell<usize>, // new field to track number of valid entries
+    ms_packet: Cell<[u8; MOUSE_PACKET_SIZE]>,
+    ms_index: Cell<usize>,
+    ms_overruns: Cell<usize>,
+
+    mouse_client: OptionalCell<&'static dyn Ps2MouseClient>, //added mouse client 4 cursor delete if needed, just for testing
+}


Is it possible that the controller could only send bytes and leave the packaging job to the mouse driver ?

domnudragota · 2025-09-26T15:03:08Z

chips/x86_q35/src/chip.rs

+
+    ps2: Option<&'a crate::ps2::Ps2Controller>,
+    // mouse: Option<&'a crate::dv_ms::Mouse<'a>>, // added 4 mouse!!!!!!


Why is this needed ?

domnudragota · 2025-09-26T15:03:51Z

chips/x86_q35/src/chip.rs

+            pt,
+            ps2: None,
+            // mouse: None,


Why are these 2 needed here ?

domnudragota · 2025-09-26T15:05:37Z

chips/x86_q35/src/chip.rs

+            ps2,
+            // ms: mouse, // ← pass the mouse


You don't need to add the mouse here, it's better to add it below under macro_rules

domnudragota · 2025-09-26T15:06:40Z

Rebase this with the latest ps2-incremental

domnudragota · 2025-09-26T15:11:43Z

chips/x86_q35/src/chip.rs

+
+        // controller bring-up owned by the chip (no logging here)
+        let _ = ps2.init_early();


Here you can init. the mouse, after the controller.

domnudragota and others added 26 commits August 1, 2025 15:17

feature: added comments for doc

367b6b0

fix: changed doc + added keyboard port test

a9dc3f3

feature: better explaind wait for input/output functions

35ff84d

feature: added comments for doc

911ec12

workflow: rebased with upstream

fix: changed doc + added keyboard port test

27cf8e0

feature: better explaind wait for input/output functions

831d3b5

feature: changed ps/2 bring-up in chip, controller init_early, no mor…

a16a303

…e PIC access

feauture: add ps2error/ps2result bounded waits, added config helpers …

365178b

…for init, reworked ISR to handle drop parity/timeout => make/break logs + prefixes

feature: implemented deffcallclient + register, minimal ISR + IRQ saf…

c8774c7

…e ring

feature: add ps2health + health_snapshots,implement defcall bottom ha…

ae0670b

…lf to keep ISR tny, minor polish

Merge branch 'ps2-incremental' of https://github.com/WyliodrinEmbedde…

77b4139

…dIoT/tock into ps2-incremental

mouse driver groundwork

aa05d4d

feature temp: Prep dv_ms by using old version

42891cc

old --> new process

feature: tweaks for chip.rs

73223d3

added content for dv_ms.rs must be revised in the future

changed 2024 -> 2025 ;3

9bb39f5

changed 2024 -> 2025 ;3

8125792

Issue: Must figure why the kernel freezes

3697f16

try_read_data_bounded + flush buffer functions

91173be

also minor changes to mouse

new controller, adapted to new functions

40af3e7

mouse send with ack + init functions

15343fe

mouse init succesful?

433ec06

testing

f346b9f

added tests

c33a5d9

mouse decode function + small correction in mouse buffer fields

6f452eb

initialisation of ms_buffer used the keyboard buffer's constant instead of the mouse's

domnudragota reviewed Sep 26, 2025

View reviewed changes

andreea-35 self-assigned this Sep 28, 2025

		/// PS/2 controller driver (the “8042” peripheral)
		pub struct Ps2Controller {


		ps2: Option<&'a crate::ps2::Ps2Controller>,
		// mouse: Option<&'a crate::dv_ms::Mouse<'a>>, // added 4 mouse!!!!!!


		// controller bring-up owned by the chip (no logging here)
		let _ = ps2.init_early();

Ps2 incremental ms #50

Are you sure you want to change the base?

Ps2 incremental ms #50

Uh oh!

Conversation

andreea-35 commented Sep 26, 2025

Pull Request Overview

Testing Strategy

TODO or Help Wanted

Documentation Updated

Formatting

Uh oh!

domnudragota commented Sep 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

domnudragota commented Sep 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants