drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

Hi,

Here's a patch to add support for KMS debugging to Nouveau, along the style of the previous patches for Intel¹ and Radeon². I'm only able to test on nv50 here, so a test on nv04 would be much appreciated, and I've published instructions on how to test here³. Thanks!

- Chris.

¹:  http://lkml.org/lkml/2010/8/5/280
²:  http://dev.laptop.org/~cjb/kdb-radeon.patch
³:  http://blog.printf.net/articles/2010/08/19/kdb-kms-for-nouveau-radeon

[...]

There is a problem with this on pre-nv20 cards. Fbcon acceleration won't work properly with IRQs disabled because you miss the context switching interrupts: You'll get a locked up fbcon if you hit sysrq-g when there's some process using the GPU.

I'd suggest disabling acceleration while in debug mode (e.g. using nouveau_fbcon_save_disable_accel()). That aside the patch looks good to me.

Hi Francisco,

Quoted 5 lines

There is a problem with this on pre-nv20 cards. Fbcon acceleration won't work properly with IRQs disabled because you miss the context switching interrupts: You'll get a locked up fbcon if you hit sysrq-g when there's some process using the GPU.

Quoted 3 lines

I'd suggest disabling acceleration while in debug mode (e.g. using nouveau_fbcon_save_disable_accel()). That aside the patch looks good to me.

Thanks very much for this. Here's a (only compile-tested) patch for this, on top of jwessel's current kgdb-next branch. Jason, would you mind testing on pre-nv20 and applying?

Thanks,

- Chris.

From: Chris Ball <[email protected]>
Subject: [PATCH] drm/nouveau/kms: Avoid a hang entering KDB with VT accel on.

Francisco Jerez advises that pre-nv20 cards would hang if we entered kdb with accel on and IRQs disabled, so we now disable accel before entering kdb and re-enable it on the way back out.

Signed-off-by Chris Ball <[email protected]>

Cc Francisco Jerez <[email protected]>

Cc Jason Wessel <[email protected]>

---
 drivers/gpu/drm/nouveau/nv04_crtc.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

drivers/gpu/drm/nouveau/nv04_crtc.c +4 -0

index fb669dd..427f90e 100644
@@ -31,10 +31,11 @@
 #include "nouveau_connector.h"
 #include "nouveau_crtc.h"
 #include "nouveau_fb.h"
 #include "nouveau_hw.h"
 #include "nvreg.h"
+#include "nouveau_fbcon.h"
 
 static int
 nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 			struct drm_framebuffer *old_fb);
 

@@ -858,10 +859,18 @@ nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 static int
 nv04_crtc_mode_set_base_atomic(struct drm_crtc *crtc,
 			       struct drm_framebuffer *fb,
 			       int x, int y, int enter)
 {
+	struct drm_nouveau_private *dev_priv = crtc->dev->dev_private;
+	struct drm_device *dev = dev_priv->dev;
+

Hi Francisco,

Quoted 5 lines

There is a problem with this on pre-nv20 cards. Fbcon acceleration won't work properly with IRQs disabled because you miss the context switching interrupts: You'll get a locked up fbcon if you hit sysrq-g when there's some process using the GPU.

Quoted 3 lines

I'd suggest disabling acceleration while in debug mode (e.g. using nouveau_fbcon_save_disable_accel()). That aside the patch looks good to me.

Thanks very much for this. Here's a (only compile-tested) patch for this, on top of jwessel's current kgdb-next branch. Jason, would you mind testing on pre-nv20 and applying?

Hi,

Here's a patch to add support for KMS debugging to Nouveau, along the style of the previous patches for Intel¹ and Radeon². I'm only able to test on nv50 here, so a test on nv04 would be much appreciated, and I've published instructions on how to test here³. Thanks!

- Chris.

Quoted 9 lines · Chris Ball

Hi,

Here's a patch to add support for KMS debugging to Nouveau, along the style of the previous patches for Intel¹ and Radeon². I'm only able to test on nv50 here, so a test on nv04 would be much appreciated, and I've published instructions on how to test here³. Thanks!

- Chris.

Hi,

I just tried that patch, but unfortunately nether with nor without it kdb seems not to work.
It could be id10t error from my side, but I did test the kdb in the past with few KMS patches, and it seemed to work.

Now I can't even get its prompt on the console.

This is what I do:

echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc (also tried booting with kgdboc=kbd)

Quoted 23 lines · Maxim Levitsky

Quoted 9 lines · Chris Ball

Hi,

Here's a patch to add support for KMS debugging to Nouveau, along the style of the previous patches for Intel¹ and Radeon². I'm only able to test on nv50 here, so a test on nv04 would be much appreciated, and I've published instructions on how to test here³. Thanks!

- Chris.

Hi,

I just tried that patch, but unfortunately nether with nor without it kdb seems not to work.
It could be id10t error from my side, but I did test the kdb in the past with few KMS patches, and it seemed to work.

Now I can't even get its prompt on the console.

This is what I do:

Quoted 3 lines

echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc (also tried booting with kgdboc=kbd)

Try changing it to kgdboc=kms,kbd or the "echo kms,kbd"

When you use only the kbd, the kms feature is not activated.

Quoted 28 lines · Jason Wessel

Quoted 26 lines · Maxim Levitsky

Quoted 9 lines · Chris Ball

Hi,

Here's a patch to add support for KMS debugging to Nouveau, along the style of the previous patches for Intel¹ and Radeon². I'm only able to test on nv50 here, so a test on nv04 would be much appreciated, and I've published instructions on how to test here³. Thanks!

- Chris.

Hi,

I just tried that patch, but unfortunately nether with nor without it kdb seems not to work.
It could be id10t error from my side, but I did test the kdb in the past with few KMS patches, and it seemed to work.

Now I can't even get its prompt on the console.

This is what I do:

echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc (also tried booting with kgdboc=kbd)

Quoted 3 lines

Try changing it to kgdboc=kms,kbd or the "echo kms,kbd"

When you use only the kbd, the kms feature is not activated.

This doesn't help.

I am afraid that this bug isn't related to kms, but rather is generic.

Quoted 31 lines · Maxim Levitsky

Quoted 30 lines · Jason Wessel

Quoted 19 lines · Maxim Levitsky

Quoted 9 lines · Chris Ball

Hi,

Here's a patch to add support for KMS debugging to Nouveau, along the style of the previous patches for Intel¹ and Radeon². I'm only able to test on nv50 here, so a test on nv04 would be much appreciated, and I've published instructions on how to test here³. Thanks!

- Chris.

Hi,

I just tried that patch, but unfortunately nether with nor without it kdb seems not to work.
It could be id10t error from my side, but I did test the kdb in the past with few KMS patches, and it seemed to work.

Quoted 7 lines

Now I can't even get its prompt on the console.

This is what I do:

echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc (also tried booting with kgdboc=kbd)

Try changing it to kgdboc=kms,kbd or the "echo kms,kbd"

Quoted 4 lines

Quoted 1 line

When you use only the kbd, the kms feature is not activated.

This doesn't help.

I am afraid that this bug isn't related to kms, but rather is generic.

I turns out that it was the NMI watchdog that I had enabled. Without it kdb works very well, including the kms support.

It would be better if you were to detect kms instead of adding an explicit param to kgdboc cmd line.

Also found out that after a debug session with Alt+SysRQ+g and X running, these keys aren't released. I had to press on all of them to make them release.
It makes sense as kgdboc in that case reads directly from keyboard port.

Quoted 32 lines · Maxim Levitsky

Quoted 31 lines · Maxim Levitsky

Quoted 30 lines · Jason Wessel

Quoted 19 lines · Maxim Levitsky

Quoted 9 lines · Chris Ball

Hi,

Here's a patch to add support for KMS debugging to Nouveau, along the style of the previous patches for Intel¹ and Radeon². I'm only able to test on nv50 here, so a test on nv04 would be much appreciated, and I've published instructions on how to test here³. Thanks!

- Chris.

Hi,

I just tried that patch, but unfortunately nether with nor without it kdb seems not to work.
It could be id10t error from my side, but I did test the kdb in the past with few KMS patches, and it seemed to work.

Quoted 7 lines

Now I can't even get its prompt on the console.

This is what I do:

echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc (also tried booting with kgdboc=kbd)

Try changing it to kgdboc=kms,kbd or the "echo kms,kbd"

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

When you use only the kbd, the kms feature is not activated.

This doesn't help.

I am afraid that this bug isn't related to kms, but rather is generic.

I turns out that it was the NMI watchdog that I had enabled. Without it kdb works very well, including the kms support.

Please disregard this. kdb works with nmi watchdog now as well. Probably something was fixed, maybe unrelated to it.

Quoted 4 lines

It would be better if you were to detect kms instead of adding an explicit param to kgdboc cmd line.

Quoted 4 lines

Also found out that after a debug session with Alt+SysRQ+g and X running, these keys aren't released. I had to press on all of them to make them release.
It makes sense as kgdboc in that case reads directly from keyboard port.

And I see that kgdb actually has a code that works that around. I suspect that what happens is that keys are released before X continues running, and therefore it doesn't pick these events up.

Quoted 51 lines · Maxim Levitsky

Quoted 32 lines · Maxim Levitsky

Quoted 31 lines · Maxim Levitsky

Quoted 30 lines · Jason Wessel

Quoted 19 lines · Maxim Levitsky

Quoted 9 lines · Chris Ball

Hi,

Here's a patch to add support for KMS debugging to Nouveau, along the style of the previous patches for Intel¹ and Radeon². I'm only able to test on nv50 here, so a test on nv04 would be much appreciated, and I've published instructions on how to test here³. Thanks!

- Chris.

Hi,

I just tried that patch, but unfortunately nether with nor without it kdb seems not to work.
It could be id10t error from my side, but I did test the kdb in the past with few KMS patches, and it seemed to work.

Quoted 7 lines

Now I can't even get its prompt on the console.

This is what I do:

echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc (also tried booting with kgdboc=kbd)

Try changing it to kgdboc=kms,kbd or the "echo kms,kbd"

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

When you use only the kbd, the kms feature is not activated.

This doesn't help.

I am afraid that this bug isn't related to kms, but rather is generic.

I turns out that it was the NMI watchdog that I had enabled. Without it kdb works very well, including the kms support.

Please disregard this. kdb works with nmi watchdog now as well. Probably something was fixed, maybe unrelated to it.

Quoted 8 lines

It would be better if you were to detect kms instead of adding an explicit param to kgdboc cmd line.

Also found out that after a debug session with Alt+SysRQ+g and X running, these keys aren't released. I had to press on all of them to make them release.
It makes sense as kgdboc in that case reads directly from keyboard port.

Quoted 3 lines

And I see that kgdb actually has a code that works that around. I suspect that what happens is that keys are released before X continues running, and therefore it doesn't pick these events up.

However an evtest on input event running on kernel tty, still only picks release of the alt key.

Quoted 52 lines · Maxim Levitsky

Quoted 51 lines · Maxim Levitsky

Quoted 32 lines · Maxim Levitsky

Quoted 31 lines · Maxim Levitsky

Quoted 30 lines · Jason Wessel

Quoted 19 lines · Maxim Levitsky

Quoted 9 lines · Chris Ball

Hi,

Here's a patch to add support for KMS debugging to Nouveau, along the style of the previous patches for Intel¹ and Radeon². I'm only able to test on nv50 here, so a test on nv04 would be much appreciated, and I've published instructions on how to test here³. Thanks!

- Chris.

Hi,

I just tried that patch, but unfortunately nether with nor without it kdb seems not to work.
It could be id10t error from my side, but I did test the kdb in the past with few KMS patches, and it seemed to work.

Quoted 7 lines

Now I can't even get its prompt on the console.

This is what I do:

echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc (also tried booting with kgdboc=kbd)

Try changing it to kgdboc=kms,kbd or the "echo kms,kbd"

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

When you use only the kbd, the kms feature is not activated.

This doesn't help.

I am afraid that this bug isn't related to kms, but rather is generic.

I turns out that it was the NMI watchdog that I had enabled. Without it kdb works very well, including the kms support.

Please disregard this. kdb works with nmi watchdog now as well. Probably something was fixed, maybe unrelated to it.

Quoted 8 lines

It would be better if you were to detect kms instead of adding an explicit param to kgdboc cmd line.

Also found out that after a debug session with Alt+SysRQ+g and X running, these keys aren't released. I had to press on all of them to make them release.
It makes sense as kgdboc in that case reads directly from keyboard port.

Quoted 6 lines

Quoted 3 lines

And I see that kgdb actually has a code that works that around. I suspect that what happens is that keys are released before X continues running, and therefore it doesn't pick these events up.

However an evtest on input event running on kernel tty, still only picks release of the alt key.

[Dropped nouveau list, because this is offtopic there]

I pretty much got to the bottom of this.
There are 2 separate issues:

1. SysRq handler is now a input 'filter', which means that it can 'eat' input events, so they don't show up on input bus.
It does so while sysrq key is down.
So sysrq and 'g' events never reach the kernel kbd driver and therefore the hack to release them doesn't work.

2. The kbd_clear_keys_helper injects the keyup events alright, but it doesn't inject SYN events, and therefore X evdev driver doesn't pick these injected events untill next SYN event.

This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good, because now Alt+SysRQ causes a screen capture by default.
In my opinion the sysrq filter should stay.
We should just make kdb hook into atkbd and do the key release there.
This should both result in cleaner/more robust code, and make this issue disappear. I'll look at doing that.

drivers/char/keyboard.c +1 -0

index 0c6c641..7df6af5 100644
@@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
 {
 	unsigned int *keycode = data;
 	input_inject_event(handle, EV_KEY, *keycode, 0);
+	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	return 0;
 }
 

drivers/char/sysrq.c +1 -1

index ef31bb8..db1eb12 100644
@@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
 	}
 
 out:
-	return sysrq_down;
+	return 0;
 }
 
 static int sysrq_connect(struct input_handler *handler,

Best regards,
Maxim Levitsky

Quoted 49 lines

[Dropped nouveau list, because this is offtopic there]

I pretty much got to the bottom of this.
There are 2 separate issues:

1. SysRq handler is now a input 'filter', which means that it can 'eat' input events, so they don't show up on input bus.
It does so while sysrq key is down.
So sysrq and 'g' events never reach the kernel kbd driver and therefore the hack to release them doesn't work.

2. The kbd_clear_keys_helper injects the keyup events alright, but it doesn't inject SYN events, and therefore X evdev driver doesn't pick these injected events untill next SYN event.

This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good, because now Alt+SysRQ causes a screen capture by default.
In my opinion the sysrq filter should stay.
We should just make kdb hook into atkbd and do the key release there.
This should both result in cleaner/more robust code, and make this issue disappear. I'll look at doing that.

drivers/char/keyboard.c +1 -0

index 0c6c641..7df6af5 100644
@@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
 {
 	unsigned int *keycode = data;
 	input_inject_event(handle, EV_KEY, *keycode, 0);
+	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	return 0;
 }
 

drivers/char/sysrq.c +1 -1

index ef31bb8..db1eb12 100644
@@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
 	}
 
 out:
-	return sysrq_down;
+	return 0;
 }
 

Quoted 7 lines

drivers/char/sysrq.c +0 -0

 static int sysrq_connect(struct input_handler *handler,

Best regards,
Maxim Levitsky

Quoted 49 lines · Maxim Levitsky

Quoted 49 lines

[Dropped nouveau list, because this is offtopic there]

I pretty much got to the bottom of this.
There are 2 separate issues:

1. SysRq handler is now a input 'filter', which means that it can 'eat' input events, so they don't show up on input bus.
It does so while sysrq key is down.
So sysrq and 'g' events never reach the kernel kbd driver and therefore the hack to release them doesn't work.

2. The kbd_clear_keys_helper injects the keyup events alright, but it doesn't inject SYN events, and therefore X evdev driver doesn't pick these injected events untill next SYN event.

This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good, because now Alt+SysRQ causes a screen capture by default.
In my opinion the sysrq filter should stay.
We should just make kdb hook into atkbd and do the key release there.
This should both result in cleaner/more robust code, and make this issue disappear. I'll look at doing that.

drivers/char/keyboard.c +1 -0

index 0c6c641..7df6af5 100644
@@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
 {
 	unsigned int *keycode = data;
 	input_inject_event(handle, EV_KEY, *keycode, 0);
+	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	return 0;
 }
 

drivers/char/sysrq.c +1 -1

index ef31bb8..db1eb12 100644
@@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
 	}
 
 out:
-	return sysrq_down;
+	return 0;
 }
 

Quoted 7 lines

Quoted 7 lines

drivers/char/sysrq.c +0 -0

 static int sysrq_connect(struct input_handler *handler,

Best regards,
Maxim Levitsky

Hi Maxim,

Is it the case that this patch takes care of masking the sysrq event from user space?

Originally before it became an input filter I had a patch to the keyboard.c to completely consume the keypress of the sysrq-g.

I have been less than successful at getting the keyboard changes upstreamed, and I was probably going to ping AKPM, because on that front I had not received any kind of response either (linux-input that is).

Certainly I can carry your patch in my branch until we can determine a final upstream design.

Quoted 66 lines · Jason Wessel

Quoted 49 lines · Maxim Levitsky

Quoted 49 lines

[Dropped nouveau list, because this is offtopic there]

I pretty much got to the bottom of this.
There are 2 separate issues:

1. SysRq handler is now a input 'filter', which means that it can 'eat' input events, so they don't show up on input bus.
It does so while sysrq key is down.
So sysrq and 'g' events never reach the kernel kbd driver and therefore the hack to release them doesn't work.

2. The kbd_clear_keys_helper injects the keyup events alright, but it doesn't inject SYN events, and therefore X evdev driver doesn't pick these injected events untill next SYN event.

This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good, because now Alt+SysRQ causes a screen capture by default.
In my opinion the sysrq filter should stay.
We should just make kdb hook into atkbd and do the key release there.
This should both result in cleaner/more robust code, and make this issue disappear. I'll look at doing that.

drivers/char/keyboard.c +1 -0

index 0c6c641..7df6af5 100644
@@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
 {
 	unsigned int *keycode = data;
 	input_inject_event(handle, EV_KEY, *keycode, 0);
+	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	return 0;
 }
 

drivers/char/sysrq.c +1 -1

index ef31bb8..db1eb12 100644
@@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
 	}
 
 out:
-	return sysrq_down;
+	return 0;
 }
 

Quoted 7 lines

Quoted 7 lines

drivers/char/sysrq.c +0 -0

 static int sysrq_connect(struct input_handler *handler,

Best regards,
Maxim Levitsky

Hi Maxim,

Is it the case that this patch takes care of masking the sysrq event from user space?

Originally before it became an input filter I had a patch to the keyboard.c to completely consume the keypress of the sysrq-g.

Quoted 6 lines

I have been less than successful at getting the keyboard changes upstreamed, and I was probably going to ping AKPM, because on that front I had not received any kind of response either (linux-input that is).

Certainly I can carry your patch in my branch until we can determine a final upstream design.

Its an option, but bear in the mind that my 'patch' causes very unpleasant regression.
The regression is that now Alt+SysRQ is reported to userspace and treated as a PrintScreen which in gnome is treated as a launcher for screenshot application.
So after you pressed it, you will end up with an army of gnome-screenshots poping up.
So I think that tty kbd driver is wrong place for key release.

The right solution in my opinion is to make atkbd register with kgdb and provide to it, the polling keyboard IO services, and also take care of releasing the keys as soon as debug mode is entered or exited.

Btw, can a driver register a hook into kgdb to be executed on enter/leave of the debug mode?
If so the atkbd driver should just do that.

I am also aware of the fact that atkbd doesn't talk to hardware directly.
Thus 'the' proper fix is also to add a polled serio handlers, make atkbd use them
if available and if so, register with the kgdb.

Quoted 30 lines · Maxim Levitsky

Quoted 28 lines · Jason Wessel

Quoted 26 lines · Maxim Levitsky

Quoted 26 lines

[Dropped nouveau list, because this is offtopic there]

I pretty much got to the bottom of this.
There are 2 separate issues:

1. SysRq handler is now a input 'filter', which means that it can 'eat' input events, so they don't show up on input bus.
It does so while sysrq key is down.
So sysrq and 'g' events never reach the kernel kbd driver and therefore the hack to release them doesn't work.

2. The kbd_clear_keys_helper injects the keyup events alright, but it doesn't inject SYN events, and therefore X evdev driver doesn't pick these injected events untill next SYN event.

This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good, because now Alt+SysRQ causes a screen capture by default.
In my opinion the sysrq filter should stay.
We should just make kdb hook into atkbd and do the key release there.
This should both result in cleaner/more robust code, and make this issue disappear. I'll look at doing that.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

drivers/char/keyboard.c +0 -0

index 0c6c641..7df6af5 100644

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

drivers/char/keyboard.c static int kbd_clear_keys_helper(struct input_handle *handle, void *data) · line 374

Quoted 8 lines

drivers/char/keyboard.c +1 -0

@@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
 {
 	unsigned int *keycode = data;
 	input_inject_event(handle, EV_KEY, *keycode, 0);
+	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	return 0;
 }
 

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

drivers/char/sysrq.c +0 -0

index ef31bb8..db1eb12 100644
@@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
 	}
 

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

drivers/char/sysrq.c static bool sysrq_filter(struct input_handle *handle, unsigned int type, · line 606

Quoted 5 lines

drivers/char/sysrq.c +1 -1

 out:
-	return sysrq_down;
+	return 0;
 }
 

Quoted 5 lines

The right solution in my opinion is to make atkbd register with kgdb and provide to it, the polling keyboard IO services, and also take care of releasing the keys as soon as debug mode is entered or exited.

Disagree. We may support different keyboard devices with kdb or use one keyboard (USB) to drop into debugger and then switch to atkbd...

I think we need to move Jason's code from drivers/char/keyboard.c to drivers/char/sysrq.c and have it track keys that have been kept pressed before entering sysrq mode and release them when leaving the mode.

We also need to teach sysrq that some handlers need resetting of the keyboard state.

From ed6fb201ca864e977c6a4fcf345014fd1d4ebed4 Mon Sep 17 00:00:00 2001

From 43eb157cdf4213e8b7792746e7d11afec2309205 Mon Sep 17 00:00:00 2001

From 33dea0e730e03814f0cd71c604185ba1c7fad51d Mon Sep 17 00:00:00 2001

Quoted 24 lines · Dmitry Torokhov

Quoted 22 lines · Maxim Levitsky

Quoted 20 lines · Jason Wessel

Quoted 18 lines · Maxim Levitsky

Quoted 18 lines

[Dropped nouveau list, because this is offtopic there]

I pretty much got to the bottom of this.
There are 2 separate issues:

1. SysRq handler is now a input 'filter', which means that it can 'eat' input events, so they don't show up on input bus.
It does so while sysrq key is down.
So sysrq and 'g' events never reach the kernel kbd driver and therefore the hack to release them doesn't work.

2. The kbd_clear_keys_helper injects the keyup events alright, but it doesn't inject SYN events, and therefore X evdev driver doesn't pick these injected events untill next SYN event.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good, because now Alt+SysRQ causes a screen capture by default.
In my opinion the sysrq filter should stay.
We should just make kdb hook into atkbd and do the key release there.
This should both result in cleaner/more robust code, and make this issue disappear. I'll look at doing that.

Nope, still same problem.
Maybe more keys were released, but still
pressing Alt+SysRQ+g second time doesn't break to the debugger.

Quoted 6 lines · Maxim Levitsky

Nope, still same problem.
Maybe more keys were released, but still
pressing Alt+SysRQ+g second time doesn't break to the debugger.

Thanks for trying this out Maxim.

I have to wonder if part of the problem is the way that raw events can propagate directly via the old keyboard interface vs the input layer. I had noticed something somewhat similar with an xorg.conf which appeared to be using both the raw keyboard and the input layer for input. I was having all sorts of problems until I changed the xorg.conf settings to use the input layer. I noticed that even pressing the up arrow in an xterm was invoking the screen shot mechanism for example.

Perhaps you can provide some more data to see if this is a similar problem or not.

Jason, I already found the root cause of the problem. First of all, note that I use xorg's evdev driver. The in-kernel tty kbd driver is only used for virtual consoles.

The root cause is that in-kernel kbd driver beeing a client of the input subsystem just doesn't see the sysrq keys because they are filtered, therefore it can't release them.

The right solution is to somehow hook at the atkbd driver , tell it that kbd tampered with its hardware, and make it release the keys.