PS/2 Mouse Clarification

[rkennedy9064]

Currently every time an interrupt is fired, I read a byte from port 0x60, but now I'm wondering if I need to read 3 bytes at a time?

No, you should read only one byte at a time. The PS/2 controller is dumb and doesn't know how many bytes are in one packet.

The PS/2 controller is shared by the keyboard as well, so a mistake there could interfere with your mouse driver.

My reads wait for the data by polling port 0x64 before reading from port 0x60.

You don't need to do this. The IRQ already indicates that you may read one byte from port 0x60.

I can also see multiple implementations issuing command 0xAD before those three reads and 0xAE afterwards (some kind of synchronization/acknowledgement?; the writes also poll port 0x64). I can't find where these two commands are coming from. You probably shouldn't send them until you know they're needed.

This sounds like a mistake due to poor understanding of the PS/2 controller. Those commands disable and enable the keyboard port, presumably to ensure that all bytes are being received from the mouse and not the keyboard, but you can receive from both devices at the same time. You can tell which device sent the byte by which IRQ was raised.

So, yeah, read all 3 bytes every time you enter the ISR and use(poll) port 0x64 to know when you can read/write a byte of data.

No, you should read one byte each time without polling port 0x64.

So now I'm more confused then before. Does that mean the way I originally handled the interrupt is the correct way to do it? For every interrupt, read one and only one value from port 0x60? I also have an interrupt setup for the keyboard, which is working and continues to work even after the mouse packets get screwed up.

If that is the correct way to do it, do you have any suggestions as to why the packets start to get screwed up? Here are both my interrupts for reference.

Code: Select all

extern "x86-interrupt" fn mouse_interrupt_handler(_stack_frame: &mut InterruptStackFrame) {
    let mut port = PortReadOnly::new(0x60);
    let packet: u8 = unsafe { port.read() };
    println!("{:08b}", packet);

    unsafe {
        PICS.lock()
            .notify_end_of_interrupt(InterruptIndex::Mouse.into());
    }
}

extern "x86-interrupt" fn keyboard_interrupt_handler(_stack_frame: &mut InterruptStackFrame) {
    use pc_keyboard::{layouts, DecodedKey, HandleControl, Keyboard, ScancodeSet1};
    use x86_64::instructions::port::PortReadOnly;

    static KEYBOARD: Lazy<Spinlock<Keyboard<layouts::Us104Key, ScancodeSet1>>> = Lazy::new(|| {
        Spinlock::new(Keyboard::new(
            layouts::Us104Key,
            ScancodeSet1,
            HandleControl::Ignore,
        ))
    });

    let mut keyboard = KEYBOARD.lock();
    let mut port = PortReadOnly::new(0x60);

    let scancode: u8 = unsafe { port.read() };
    if let Ok(Some(key_event)) = keyboard.add_byte(scancode) {
        if let Some(key) = keyboard.process_keyevent(key_event) {
            match key {
                DecodedKey::Unicode(character) => print!("{}", character),
                DecodedKey::RawKey(key) => print!("{:?}", key),
            }
        }
    }

    unsafe {
        PICS.lock()
            .notify_end_of_interrupt(InterruptIndex::Keyboard.into());
    }
}

[alexfru]

So now I'm more confused then before. Does that mean the way I originally handled the interrupt is the correct way to do it? For every interrupt, read one and only one value from port 0x60?

I have several implementations that read 3 bytes from the mouse in the ISR like I described earlier. One of them worked for me some 10+ years ago. It is possible that reading 3 bytes in a row, if not all of them are immediately available, wastes CPU time in the ISR. Anyway, it should be easy to try both ways and see if there's any noticeable difference in how things work functionally and performance-wise.

As for the keyboard, perhaps there may be some interplay between the keyboard and the mouse, but I don't remember having any problem with the two (which, of course, doesn't mean there can't be).

[rkennedy9064]

I just tried the read 3 bytes per interrupt approach and it seems to have fixed the issue? All my values are generally between -8 -> 8 for both axis and the flag packet doesn't swap positions anymore. I'm not sure if it will work on real hardware, but for bochs it looks like it works.

[BenLunt]

I thought I would put my two cents in too. Alex and Octocontrabass have explained most of the things you need.

Here is what I think you should do:

1) At initialize time, figure out how many bytes there will be per packet.
2) Keep track of which byte of the packet you are currently reading. For example, start with 0, then increment at the end of each IRQ.
3) At initialize time: cur_index = -1; // initialize to unknown index
4) IRQ fires
5) if (cur_index < 0) && (bit 3 is clear) ignore the byte and EOI
6) if (cur_index < 0) && (bit 3 is set) set cur_index to zero and start your handler process:

Code: Select all

  switch (cur_index) {
    case 0:
      if (byte & 0x08) {
        save_byte = byte;
        // extract buttons and save them
      } else
         cur_index = -1;  // we think we should be at index 0, but bit 3 isn't set, 
                          // so start the process over, ignoring all bytes until bit 3 is set.
      break;
    case 1:
      if ((save_byte & 0x40) == 0) {  //  overflow ?
        //extract the x coord
        // here is an easy way to do this using the 'sign bit'
        if (save_byte & 0x10) // is signed
           x = (int) (((unsigned int) -1 & ~0xFF) | byte);  // the (unsigned int) cast is probably redundant
        else  // is unsigned
           x = (int) (unsigned int) byte;
      }
      break;
    case 2:
      // do the same thing as above, except with 'y'
      //  and y's overflow and sign bit
      break;
    case 3:
      // even though a standard PS2 mouse may not have a forth byte, 
      //  go ahead and have code for it here because the code below
      //  will make sure to never get here if we don't have four bytes
      // 
      //  However, if we do have four bytes, the code is already here:
      //   extract z coord here
      //   if id byte == 3 use whole byte as signed integer
      //   if id byte == 4 use only the lower 4 bits
      break;
   }
   cur_index++;
   if (cur_index >= packet_size) {
     // send the current packet to our main mouse handler
     // reset the packet for the next time
     cur_index = 0;
   }

   // eoi
}

Quite simple really.

Ben
- http://www.fysnet.net/input_and_output_devices.htm

[rkennedy9064]

Okay, with the help of everyone in this thread I believe I now have a working version of a basic mouse interrupt. I believe the issue with my single read per interrupt was the fact that I wasn't checking the always set bit properly, so eventually the packets got out of order. In case anyone is interested, or this helps someone in the future, this was the working solution. Thanks again for all the help everyone.

Code: Select all

extern "x86-interrupt" fn mouse_interrupt_handler(_stack_frame: &mut InterruptStackFrame) {
    let mut port = PortReadOnly::new(0x60);
    let packet = unsafe { port.read() };
    MOUSE.lock().process_packets(packet);

    unsafe {
        PICS.lock()
            .notify_end_of_interrupt(InterruptIndex::Mouse.into());
    }
}

pub fn process_packets(&mut self, packet: u8) {
    match self.current_packet {
        0 => {
            let flags = MouseFlags::from_bits_truncate(packet);
            if !flags.contains(MouseFlags::ALWAYS_ONE) {
                return;
            }
            self.data_packet = flags;
        }
        1 => self.process_x_movement(packet),
        2 => self.process_y_movement(packet),
        _ => unreachable!(),
    }

    self.current_packet = (self.current_packet + 1) % 3;
}

fn process_x_movement(&mut self, packet: u8) {
    if !self.data_packet.contains(MouseFlags::X_OVERFLOW) {
        self.x = if self.data_packet.contains(MouseFlags::X_SIGN) {
            self.sign_extend(packet)
        } else {
            packet as i16
        };
    }
}

fn process_y_movement(&mut self, packet: u8) {
    if !self.data_packet.contains(MouseFlags::Y_OVERFLOW) {
        self.y = if self.data_packet.contains(MouseFlags::Y_SIGN) {
            self.sign_extend(packet)
        } else {
            packet as i16
        };
    }
}

fn sign_extend(&self, packet: u8) -> i16 {
    ((packet as u16) | 0xFF00) as i16
}

[BenLunt]

May I ask what language that is? It looks bizarre to me.

For example, what on earth does the following do:

Code: Select all

        1 => self.process_x_movement(packet),
        2 => self.process_y_movement(packet),
        _ => unreachable!(),

To me it looks like a switch() statement in C. If this is the case, then you already had part of my suggestion with the saving of the current part of the packet.

Anyway, I am just curious to see what language that is.

Ben
- http://www.fysnet.net/osdesign_book_series.htm

[alexfru]

May I ask what language that is?

Rust.

[rkennedy9064]

May I ask what language that is?

Rust.

Like Alexfru said, it's rust. Its pretty similar to a switch statement in C, but you can do a lot of extra things like match on patterns and values and certain types. There's a good explanation of it in the rust book here https://doc.rust-lang.org/stable/book/c ... match.html