Garbage input when trying to use PS/2 mouse

[cycl0ne]

Hi,

i have a strange phenomen. I wanted to reactivate my old code which worked 2 Years ago, but now, it still works, but Qemu gives me on the IRQ12 strange values for x/y.
Here my Initcode:

Code: Select all

  //Enable the auxiliary mouse device
  mouse_wait(1);
  outb(0x64, 0xA8);
 
  //Enable the interrupts
  mouse_wait(1);
  outb(0x64, 0x20);
  mouse_wait(0);
  _status=(inb(0x60) | 2);
  mouse_wait(1);
  outb(0x64, 0x60);
  mouse_wait(1);
  outb(0x60, _status);
 
  //Tell the mouse to use default settings
  mouse_write(0xF6);
  mouse_read();  //Acknowledge
 
  //Enable the mouse
  mouse_write(0xF4);
  mouse_read();  //Acknowledge

and here the output from IRQ12 Handler:
|0:0||0:0||0:0||0:0||0:0||0:0||45:0||45:-73||127:127|

Here the output code:

Code: Select all

			KPrintF("|%d:%d|", (int8_t)mouse_byte[1], (int8_t)mouse_byte[2]);

am i missing something? I can remember it worked fine..

Thanks for help and merry X-Mas!

[cycl0ne]

Ok i copied from another kernel the code (couldnt test his kernel, there are some compile errors), but still error. Upgraded QEMU to 2.1


Here my source. Im going nuts..

Code: Select all

#define MOUSE_IRQ 12

#define MOUSE_PORT   0x60
#define MOUSE_STATUS 0x64
#define MOUSE_ABIT   0x02
#define MOUSE_BBIT   0x01
#define MOUSE_WRITE  0xD4
#define MOUSE_F_BIT  0x20
#define MOUSE_V_BIT  0x08

#define IRQ_OFF { asm volatile ("cli"); }
#define IRQ_RES { asm volatile ("sti"); }

/*
 * inportb
 * Read from an I/O port.
 */
static unsigned char inportb(unsigned short _port) 
{
	unsigned char rv;
	asm volatile ("inb %1, %0" : "=a" (rv) : "dN" (_port));
	return rv;
}

/*
 * outportb
 * Write to an I/O port.
 */
static void outportb(unsigned short _port, unsigned char _data) 
{
	asm volatile ("outb %1, %0" : : "dN" (_port), "a" (_data));
}


static void mouse_wait(uint8_t a_type) {
	uint32_t timeout = 100000;
	if (!a_type) {
		while (--timeout) {
			if ((inportb(MOUSE_STATUS) & MOUSE_BBIT) == 1) {
				return;
			}
		}
//		debug_print(INFO, "mouse timeout");
		return;
	} else {
		while (--timeout) {
			if (!((inportb(MOUSE_STATUS) & MOUSE_ABIT))) {
				return;
			}
		}
//		debug_print(INFO, "mouse timeout");
		return;
	}
}

static void mouse_write(uint8_t write) {
	mouse_wait(1);
	outportb(MOUSE_STATUS, MOUSE_WRITE);
	mouse_wait(1);
	outportb(MOUSE_PORT, write);
}

static uint8_t mouse_read(void) {
	mouse_wait(0);
	char t = inportb(MOUSE_PORT);
	return t;
}

static void arch_ps2m_init(void)
{
	uint8_t status;
	IRQ_OFF;
	mouse_wait(1);
	outportb(MOUSE_STATUS, 0xA8);
	mouse_wait(1);
	outportb(MOUSE_STATUS, 0x20);
	mouse_wait(0);
	status = inportb(0x60) | 2;
	mouse_wait(1);
	outportb(MOUSE_STATUS, 0x60);
	mouse_wait(1);
	outportb(MOUSE_PORT, status);
	mouse_write(0xF6);
	mouse_read();
	mouse_write(0xF4);
	mouse_read();
	IRQ_RES;
}


static uint8_t mouse_cycle = 0;
static int8_t  mouse_byte[3];

static __attribute__((no_instrument_function)) BOOL mouse_handler(UINT32 number, APTR data)
{
	uint8_t status = inportb(MOUSE_STATUS);
	while (status & MOUSE_BBIT) 
	{
		int8_t mouse_in = inportb(MOUSE_PORT);
		if (status & MOUSE_F_BIT) 
		{
			switch (mouse_cycle) 
			{
				case 0:
					mouse_byte[0] = mouse_in;
					KPrintF("/%d:",mouse_in);
					if (!(mouse_in & MOUSE_V_BIT)) return 1;
					++mouse_cycle;
					break;
				case 1:
					mouse_byte[1] = mouse_in;
					KPrintF("/%d:",mouse_in);
					++mouse_cycle;
					break;
				case 2:
					mouse_byte[2] = mouse_in;
					KPrintF("/%d:",mouse_in);
					/* We now have a full mouse packet ready to use */
					if (mouse_byte[0] & 0x80 || mouse_byte[0] & 0x40) 
					{
						/* x/y overflow? bad packet! */
						break;
					}
					mouse_cycle = 0;
					break;
			}
		}
		status = inportb(MOUSE_STATUS);
	}

Im going craaaaazzzzzzyyy ..... thinking of missing something..

[cycl0ne]

Hmm, maybe its QEMU V2.x.. Anyone has a small sample OS with PS2 support (Link to it) to test it?

[KemyLand]

Hi!

Please elaborate a little bit more. We need your IRQ12 ISR's code. Please give as much information and details as possible. I'm pretty sure we can help you, if you help us help you .

[Brendan]

Hi,

and here the output from IRQ12 Handler:
|0:0||0:0||0:0||0:0||0:0||0:0||45:0||45:-73||127:127|

That looks potentially correct to me; except that you're displaying unsigned data as signed ("45:-73" should be "45:183" or even better "0x2D:0xB7") and you forgot to show the first byte (which contains the X and Y sign bits and the X and Y overflow bits) which is necessary to figure out how the lowest 8 bits of both X and Y should be interpreted.

Also note that (in general, in my opinion) it's a mistake for the mouse driver to touch any IO ports. A mouse driver should only ever send and receive bytes via. something else (e.g. by communicating with the PS/2 controller device driver).

There's also a whole load of stuff you've skipped (some that belongs in the PS/2 controller driver, and some that belongs in the PS/2 mouse driver); including PS/2 controller initialisation, all self tests (for all 3 devices - controller, keyboard and mouse), device detection, using the "resend" command when you get invalid data, handling "unexpected BAT", etc.


Cheers,

Brendan

[cycl0ne]

Hi!

Please elaborate a little bit more. We need your IRQ12 ISR's code. Please give as much information and details as possible. I'm pretty sure we can help you, if you help us help you .

this is my isr code in the 2nd code output:
static __attribute__((no_instrument_function)) BOOL mouse_handler(uint32_t number, APTR data);

the isr default handler is just like this (which calls the handler code):

Code: Select all

__attribute__((no_instrument_function))  void arch_irq_server(unsigned int exc_no, registers_t regs, APTR Data)
{
	struct Interrupt *irq;
	uint8_t irqNum = exc_no - 32; // in x86 all Ints are mapped to 32 - 48
	if (irqNum > 15) return; // We only have 15 IRQs
	
	irq = SystemBase->IntVectorList[irqNum];
        if (irq)
       {
	if (irq->is_Code(irqNum, irq->is_Data)) 
	{
			irq->is_Count++;
			break;
	}
	arch_irq_eoi(1<<irqNum);
}

[cycl0ne]

Hi,

and here the output from IRQ12 Handler:
|0:0||0:0||0:0||0:0||0:0||0:0||45:0||45:-73||127:127|

That looks potentially correct to me; except that you're displaying unsigned data as signed ("45:-73" should be "45:183" or even better "0x2D:0xB7") and you forgot to show the first byte (which contains the X and Y sign bits and the X and Y overflow bits) which is necessary to figure out how the lowest 8 bits of both X and Y should be interpreted.

that output was from my first code in first thread and signed is ok, its x and y and they are signed.
the first byte (mouse[0] shows 0x40, 0x0,...)
will dump a second example for the second code.

Also note that (in general, in my opinion) it's a mistake for the mouse driver to touch any IO ports. A mouse driver should only ever send and receive bytes via. something else (e.g. by communicating with the PS/2 controller device driver).

There's also a whole load of stuff you've skipped (some that belongs in the PS/2 controller driver, and some that belongs in the PS/2 mouse driver); including PS/2 controller initialisation, all self tests (for all 3 devices - controller, keyboard and mouse), device detection, using the "resend" command when you get invalid data, handling "unexpected BAT", etc.
Cheers,
Brendan

this is the ps2 mouse controller driver. and where did i skip something? look at the full code in thread2 there under ps2_init i init all (this is called seperately and not in irq of course).

this code does the io and puts the data into a queue, which are fetched later from a mouse process not in irq.

[cycl0ne]

here the output of the second code:

Code: Select all

mi:0x28 -/s40:
mi:0x7f -/x127:
mi:0xffffff81 -/y-127:
mi:0x28 -/s40:
mi:0x7f -/x127:
mi:0xffffff81 -/y-127:
mi:0x28 -/s40:
mi:0x7f -/x127:
mi:0xffffff81 -/y-127:
mi:0x28 -/s40:
mi:0x7f -/x127:
mi:0xffffff81 -/y-127:
mi:0x28 -/s40:
mi:0x7f -/x127:
mi:0xffffff81 -/y-127:

for me it seems to be a bug in qemu 2.x. could anyone check with his os and qemu 2.x? they changed something in the ps2 code i could see in the github mirror.

tried now with 2 other OS: tuouros and visopsys. both mouse implementation seem broken too.

[Nable]

Hm, it looks like you did the common mistake of using "registers_t" instead of "registers_t *".

Code: Select all

__attribute__((no_instrument_function))  void arch_irq_server(unsigned int exc_no, registers_t regs, APTR Data)

I'm not sure if it's the only mistake but just this thing can screw everything in a bad way (because compiler is free to trash contents of function arguments)

[Brendan]

Hi,

That looks potentially correct to me; except that you're displaying unsigned data as signed ("45:-73" should be "45:183" or even better "0x2D:0xB7") and you forgot to show the first byte (which contains the X and Y sign bits and the X and Y overflow bits) which is necessary to figure out how the lowest 8 bits of both X and Y should be interpreted.

that output was from my first code in first thread and signed is ok, its x and y and they are signed.
the first byte (mouse[0] shows 0x40, 0x0,...)
will dump a second example for the second code.

Um? They're from the first piece of code; where X and Y are 9-bit, and where the values in "mouse_byte[1]" and "mouse_byte[2]" only contain the lowest 8 bits, which does not include the sign bit (and without that 9th/sign bit, the lowest 8 bits are not signed).

Also note that (in general, in my opinion) it's a mistake for the mouse driver to touch any IO ports. A mouse driver should only ever send and receive bytes via. something else (e.g. by communicating with the PS/2 controller device driver).

this is the ps2 mouse controller driver.

It's a "hybrid PS/2 controller driver + PS/2 mouse driver" which completely fails to separate the 2 completely difference concerns. If the user has 2 USB keyboards and decides to plug in two PS/2 mouses, how does your OS handle that? What if it's the opposite - a pair of PS/2 keyboards? For the first case the OS probably fails to check if the first PS/2 device is a keyboard or not so moving the mouse causes "random keypresses"; and for the second case the OS fails to check if the second PS/2 device is a mouse so pressing a key causes "random mouse packets".

There's also a whole load of stuff you've skipped (some that belongs in the PS/2 controller driver, and some that belongs in the PS/2 mouse driver); including PS/2 controller initialisation, all self tests (for all 3 devices - controller, keyboard and mouse), device detection, using the "resend" command when you get invalid data, handling "unexpected BAT", etc.

and where did i skip something? look at the full code in thread2 there under ps2_init i init all (this is called seperately and not in irq of course).

If you mean "arch_ps2m_init(void)" in your second post; then:

• It doesn't check if the PS/2 controller exists at all
• It doesn't do the PS/2 controller's self test (command 0xAA)
• It doesn't disable the silly "scancode set 2 to scancode set 1" translation for the first PS/2 port
• It doesn't check if the PS/2 controller supports 2 PS/2 ports or if it's an older controller that only supports one PS/2 port
• It doesn't do the "interface tests" for either PS/2 ports
• It does send the "reset/self test" command to the device on either PS/2 port (device command 0xFF)
• It doesn't check if the device reported "OK" or "faulty" after the "reset/self test" command
• It doesn't check the device ID if the device reported "OK" after the "reset/self test" command; which is needed to determine if the device is a mouse or something else (keyboard, bar-code scanner, touch screen, whatever)
• When sending commands to the mouse; you don't check if it replies with 0xFA (OK), 0xFC (error) or 0xFE (resend)
• You don't poll periodically to determine if the device has been removed
• If the user unplugs the device and plugs in a difference device (e.g. keyboard) then the new device sends a "BAT completion code" and you ignore that and continue pretending a mouse is connected
• If the user unplugs the device and plugs the exact same device back in; then the device sends a "BAT completion code" and you ignore that and fail to reset/reinitialise the device
• If you get invalid data from the mouse for any reason, you don't ask the mouse to resend the packet and just silently discard information until you see a byte that may or may not be the first byte of a packet

Cheers,

Brendan

[cycl0ne]

Hm, it looks like you did the common mistake of using "registers_t" instead of "registers_t *".

Code: Select all

__attribute__((no_instrument_function))  void arch_irq_server(unsigned int exc_no, registers_t regs, APTR Data)

I'm not sure if it's the only mistake but just this thing can screw everything in a bad way (because compiler is free to trash contents of function arguments)

I will try, but yes could be a bug, but all other things work (network, sound, sata, keyboard,...) all going to the same handler.

[KemyLand]

Hm, it looks like you did the common mistake of using "registers_t" instead of "registers_t *".

Code: Select all

__attribute__((no_instrument_function))  void arch_irq_server(unsigned int exc_no, registers_t regs, APTR Data)

I'm not sure if it's the only mistake but just this thing can screw everything in a bad way (because compiler is free to trash contents of function arguments)

I will try, but yes could be a bug, but all other things work (network, sound, sata, keyboard,...) all going to the same handler.

I don't think the compiler can sanely do this (maybe with -Ofast ). Anyway, if he says everything works, except the mouse, the problem is pretty obvious. Brendan (again) recolected a lot of details he's mising (thanks to the archaic developers from IBM that seem to like a little asynchronous client/server + IRQs fashion!)

[cycl0ne]

Hi,
If you mean "arch_ps2m_init(void)" in your second post; then:

• It doesn't check if the PS/2 controller exists at all
• It doesn't do the PS/2 controller's self test (command 0xAA)
• It doesn't disable the silly "scancode set 2 to scancode set 1" translation for the first PS/2 port
• It doesn't check if the PS/2 controller supports 2 PS/2 ports or if it's an older controller that only supports one PS/2 port
• It doesn't do the "interface tests" for either PS/2 ports
• It does send the "reset/self test" command to the device on either PS/2 port (device command 0xFF)
• It doesn't check if the device reported "OK" or "faulty" after the "reset/self test" command
• It doesn't check the device ID if the device reported "OK" after the "reset/self test" command; which is needed to determine if the device is a mouse or something else (keyboard, bar-code scanner, touch screen, whatever)
• When sending commands to the mouse; you don't check if it replies with 0xFA (OK), 0xFC (error) or 0xFE (resend)
• You don't poll periodically to determine if the device has been removed
• If the user unplugs the device and plugs in a difference device (e.g. keyboard) then the new device sends a "BAT completion code" and you ignore that and continue pretending a mouse is connected
• If the user unplugs the device and plugs the exact same device back in; then the device sends a "BAT completion code" and you ignore that and fail to reset/reinitialise the device
• If you get invalid data from the mouse for any reason, you don't ask the mouse to resend the packet and just silently discard information until you see a byte that may or may not be the first byte of a packet

Then please update the Wiki, i took the code example from here:
http://forum.osdev.org/viewtopic.php?t=10247
found here:
http://wiki.osdev.org/Resources

and the code example is quite popular, as you can see here:
https://github.com/klange/toaruos/blob/ ... ps2mouse.c

and for the "two keyboard/mice/whatever" problem. this is driven by a special device management. as i said, the driver here just takes the mousedata from ioport to a queue. keyboard data is also in a seperate queue. and there is a third device sitting on top (called INPUT) which collects the data.
so you can plug in a second, third, fourth mouse, keyboard, whatever and all is collected and correctly assigned to the system through the input processor, because all of them have a seperate queue. its kind of a NAMED PIPE under unix/linux for the drivers.
they deliver data, someone else fetches it.
The best thing is: where do you see anything that i do anything with the data that you see i have a mixed driver? i cut all "nonsense" code out of the code example.. here i just save the inputb in a variable mouse[]. And i believe all driver code has to do this at first or? But his is OFF TOPIC.. I guess.

----
So Back to topic. If your driver does all this, could you please check it with QEMU 2.x if it works? Dont want to sit hours on implementing this all and it wouldnt fix my problem.
Thank you for your help.

Brgds
C.

[Brendan]

Hi,

If you mean "arch_ps2m_init(void)" in your second post; then:

• It doesn't check if the PS/2 controller exists at all
• It doesn't do the PS/2 controller's self test (command 0xAA)
• It doesn't disable the silly "scancode set 2 to scancode set 1" translation for the first PS/2 port
• It doesn't check if the PS/2 controller supports 2 PS/2 ports or if it's an older controller that only supports one PS/2 port
• It doesn't do the "interface tests" for either PS/2 ports
• It does send the "reset/self test" command to the device on either PS/2 port (device command 0xFF)
• It doesn't check if the device reported "OK" or "faulty" after the "reset/self test" command
• It doesn't check the device ID if the device reported "OK" after the "reset/self test" command; which is needed to determine if the device is a mouse or something else (keyboard, bar-code scanner, touch screen, whatever)
• When sending commands to the mouse; you don't check if it replies with 0xFA (OK), 0xFC (error) or 0xFE (resend)
• You don't poll periodically to determine if the device has been removed
• If the user unplugs the device and plugs in a difference device (e.g. keyboard) then the new device sends a "BAT completion code" and you ignore that and continue pretending a mouse is connected
• If the user unplugs the device and plugs the exact same device back in; then the device sends a "BAT completion code" and you ignore that and fail to reset/reinitialise the device
• If you get invalid data from the mouse for any reason, you don't ask the mouse to resend the packet and just silently discard information until you see a byte that may or may not be the first byte of a packet

Then please update the Wiki,

This is the wiki page for the PS/2 mouse. Sadly, this page is missing a lot of information; and there's a different "mouse" page that should only contain generic information (and the PS/2 specific information, which includes information for both the PS/2 controller and the PS/2 mouse should be shifted/merged into the correct places). Mostly, you're right - the mouse pages do need work.

i took the code example from here:
http://forum.osdev.org/viewtopic.php?t=10247

found here:
http://wiki.osdev.org/Resources

and the code example is quite popular, as you can see here:
https://github.com/klange/toaruos/blob/ ... ps2mouse.c

For virtually everything; most of the code you'll find online is pure crud. Most of it is code that "works" (until/unless something is slightly strange) and not code that works (including handling corner cases and failure modes).

and for the "two keyboard/mice/whatever" problem. this is driven by a special device management. as i said, the driver here just takes the mousedata from ioport to a queue. keyboard data is also in a seperate queue. and there is a third device sitting on top (called INPUT) which collects the data.
so you can plug in a second, third, fourth mouse, keyboard, whatever and all is collected and correctly assigned to the system through the input processor, because all of them have a seperate queue. its kind of a NAMED PIPE under unix/linux for the drivers.
they deliver data, someone else fetches it.

You're deliberately trying to avoid the issues. If there are 2 mouses you can't just use the same mouse driver for both because it's hard-coded to use IRQ 12 and a specific PS/2 port. If the PS/2 controller is something very different you can't just recompile the mouse driver and use it without changing its source code. You also can't test it easily (e.g. by telling it to communicate with a dummy "unit test" driver instead of communicating with the PS/2 controller's driver).

For the code itself, it's not just a simple translator that converts raw bytes from wherever into mouse events for your named pipe. The "2 button mouse protocol" is just a default (for backward compatibility) and there are several other protocols ("3 button with scroll wheel", "5 button with scroll wheel", etc); and (eventually) your code should be attempting to enable the correct protocol for the device. In addition there's the stuff you've skipped (retries, error handling, hot-plug), plus stuff you're not using (mouse scaling, better sample rates, etc), plus stuff you're not doing on the opposite side (e.g. providing some sort of standardised "n button, n axis" pointer device abstraction for the GUI to use, including scaling the mouse movement to a normalised/device independent range).

The best thing is: where do you see anything that i do anything with the data that you see i have a mixed driver?

The PS/2 controller and the PS/2 mouse driver are 2 separate pieces of hardware (just like a USB controller and a USB device are a separate pieces of hardware). You should have a PS/2 controller driver and a separate PS/2 mouse driver, where these 2 drivers communicate (just like you'd have a USB controller driver and separate USB device drivers), possibly using your "named pipes" if that's how your OS does things. You've mixed the PS/2 controller driver and the PS/2 mouse driver together.

Dont want to sit hours on implementing this all and it wouldnt fix my problem.

The current problem (whatever it is) is just a symptom of the fact that your code is crap and needs to be redesigned then rewritten. If you found and fixed the current problem the code would still be crap that needs to be redesigned and rewritten. Finding and fixing the current problem changes nothing and is a complete waste of time.


Cheers,

Brendan