Skip to contentSkip to conversation

vt: Drop K_OFF for VC_MUTE

PATCH In review Adam Jackson 4 participants 4 messages Nov 16, 2012 – Dec 12, 2012 TTY layer and serial drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 vt: Drop K_OFF for VC_MUTE

The "don't enqueue stuff" semantics of K_OFF shouldn't be a function of the keyboard map state; we should be able to switch among cooked/raw/ unicode without changing whether events are delivered. Otherwise - if changing to K_UNICODE undoes K_OFF - then suddenly Alt-F2 under Gnome will switch VT instead of summoning the "run command" dialog.

Drop the K_OFF handling and replace it with a new "mute" ioctl pair. Anybody using K_OFF would already need to be prepared to handle it throwing -EINVAL for old kernel compatibility, so userspace will degrade gracefully.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=859485

Cc Arthur Taylor <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
 drivers/tty/vt/keyboard.c | 40 +++++++++++++++++++++++++++++++++-------
 drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++
 include/linux/kbd_kern.h  |  6 +++---
 include/linux/vt_kern.h   |  2 ++
 include/uapi/linux/kd.h   |  5 +++++
 5 files changed, 56 insertions(+), 10 deletions(-)
drivers/tty/vt/keyboard.c +33 -7
index 681765b..08d1d57 100644
@@ -657,7 +657,7 @@ static void k_spec(struct vc_data *vc, unsigned char value, char up_flag)
 		return;
 	if ((kbd->kbdmode == VC_RAW ||
 	     kbd->kbdmode == VC_MEDIUMRAW ||
-	     kbd->kbdmode == VC_OFF) &&
+	     vc_kbd_mode(kbd, VC_MUTE)) &&
 	     value != KVAL(K_SAK))
 		return;		/* SAK is allowed even in raw mode */
 	fn_handler[value](vc);
@@ -1381,7 +1381,7 @@ static void kbd_keycode(unsigned int keycode, int down, int hw_raw)
 	if (rc == NOTIFY_STOP)
 		return;
 
-	if ((raw_mode || kbd->kbdmode == VC_OFF) && type != KT_SPEC && type != KT_SHIFT)
+	if ((raw_mode || vc_kbd_mode(kbd, VC_MUTE)) && type != KT_SPEC && type != KT_SHIFT)
 		return;
 
 	(*k_handler[type])(vc, keysym & 0xff, !down);
@@ -1731,9 +1731,6 @@ int vt_do_kdskbmode(int console, unsigned int arg)
 		kbd->kbdmode = VC_UNICODE;
 		do_compute_shiftstate();
 		break;
-	case K_OFF:
-		kbd->kbdmode = VC_OFF;
-		break;
 	default:
 		ret = -EINVAL;
 	}
@@ -1742,6 +1739,30 @@ int vt_do_kdskbmode(int console, unsigned int arg)
 }
 
 /**
+ *	vt_do_kdskbmute		-	set keyboard event mute
+ *	@console: the console to use
+ *	@arg: the requested mode
+ *
+ *	Update the keyboard mute state while holding the correct locks.
+ *	Return 0 for success or an error code.
+ */
+int vt_do_kdskbmute(int console, unsigned int arg)
+{
+	struct kbd_struct * kbd = kbd_table + console;
+	int ret = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&kbd_event_lock, flags);
+	if (arg)
+		set_vc_kbd_mode(kbd, VC_MUTE);
+	else
+		clr_vc_kbd_mode(kbd, VC_MUTE);
+	spin_unlock_irqrestore(&kbd_event_lock, flags);
+	return ret;
+}
+
+
+/**
  *	vt_do_kdskbmeta		-	set keyboard meta state
  *	@console: the console to use
  *	@arg: the requested meta state
@@ -2068,13 +2089,18 @@ int vt_do_kdgkbmode(int console)
 		return K_MEDIUMRAW;
 	case VC_UNICODE:
 		return K_UNICODE;
-	case VC_OFF:
-		return K_OFF;
 	default:
 		return K_XLATE;
 	}
 }
 
+int vt_do_kgdbmute(int console)
+{
+	struct kbd_struct * kbd = kbd_table + console;
+	/* This is a spot read so needs no locking */
+	return vc_kbd_mode(kbd, VC_MUTE);
+}
+
 /**
  *	vt_do_kdgkbmeta		-	report meta status
  *	@console: console to report
drivers/tty/vt/vt_ioctl.c +13 -0
index b841f56..f0951e2 100644
@@ -477,6 +477,19 @@ int vt_ioctl(struct tty_struct *tty,
 		ret = put_user(uival, (int __user *)arg);
 		break;
 
+	case KDSKBMUTE:
+		if (!perm)
+			return -EPERM;
+		ret = vt_do_kdskbmute(console, arg);
+		if (ret == 0)
+			tty_ldisc_flush(tty);
+		break;
+
+	case KDGKBMUTE:
+		uival = vt_do_kdgkbmute(console);
+		ret = put_user(uival, (int __user *)arg);
+		break;
+
 	/* this could be folded into KDSKBMODE, but for compatibility
 	   reasons it is not so easy to fold KDGKBMETA into KDGKBMODE */
 	case KDSKBMETA:
include/linux/kbd_kern.h +3 -3
index b7c8cdc..9386143 100644
@@ -48,19 +48,19 @@ struct kbd_struct {
 #define VC_CAPSLOCK	2	/* capslock mode */
 #define VC_KANALOCK	3	/* kanalock mode */
 
-	unsigned char kbdmode:3;	/* one 3-bit value */
+	unsigned char kbdmode:2;	/* one 2-bit value */
 #define VC_XLATE	0	/* translate keycodes using keymap */
 #define VC_MEDIUMRAW	1	/* medium raw (keycode) mode */
 #define VC_RAW		2	/* raw (scancode) mode */
 #define VC_UNICODE	3	/* Unicode mode */
-#define VC_OFF		4	/* disabled mode */
 
-	unsigned char modeflags:5;
+	unsigned char modeflags:6;
 #define VC_APPLIC	0	/* application key mode */
 #define VC_CKMODE	1	/* cursor key mode */
 #define VC_REPEAT	2	/* keyboard repeat */
 #define VC_CRLF		3	/* 0 - enter sends CR, 1 - enter sends CRLF */
 #define VC_META		4	/* 0 - meta, 1 - meta=prefix with ESC */
+#define VC_MUTE		5	/* don't generate events */
 };
 
 extern int kbd_init(void);
include/linux/vt_kern.h +2 -0
index 50ae7d0..a886915 100644
@@ -168,6 +168,7 @@ extern void hide_boot_cursor(bool hide);
 
 /* keyboard  provided interfaces */
 extern int vt_do_diacrit(unsigned int cmd, void __user *up, int eperm);
+extern int vt_do_kdskbmute(int console, unsigned int arg);
 extern int vt_do_kdskbmode(int console, unsigned int arg);
 extern int vt_do_kdskbmeta(int console, unsigned int arg);
 extern int vt_do_kbkeycode_ioctl(int cmd, struct kbkeycode __user *user_kbkc,
@@ -177,6 +178,7 @@ extern int vt_do_kdsk_ioctl(int cmd, struct kbentry __user *user_kbe,
 extern int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb,
                                         int perm);
 extern int vt_do_kdskled(int console, int cmd, unsigned long arg, int perm);
+extern int vt_do_kdgkbmute(int console);
 extern int vt_do_kdgkbmode(int console);
 extern int vt_do_kdgkbmeta(int console);
 extern void vt_reset_unicode(int console);
include/uapi/linux/kd.h +5 -0
index 87b7cc4..c3de63c 100644
@@ -81,6 +81,7 @@ struct unimapinit {
 #define		K_XLATE		0x01
 #define		K_MEDIUMRAW	0x02
 #define		K_UNICODE	0x03
+/* K_OFF is no longer implemented, but preserved for source compatibility */
 #define		K_OFF		0x04
 #define KDGKBMODE	0x4B44	/* gets current keyboard mode */
 #define KDSKBMODE	0x4B45	/* sets current keyboard mode */
@@ -150,6 +151,10 @@ struct kbd_repeat {
 			/* earlier this field was misnamed "rate" */
 };
 
+/* get/set event mute */
+#define KDGKBMUTE	0x4B50
+#define KDSKBMUTE	0x4B51
+
 #define KDKBDREP        0x4B52  /* set keyboard delay/repeat rate;
 				 * actually used values are returned */
 
Signature · 1 line
1.7.11.7
#1 lore raw
Quoted 11 lines · Adam Jackson

The "don't enqueue stuff" semantics of K_OFF shouldn't be a function of the keyboard map state; we should be able to switch among cooked/raw/ unicode without changing whether events are delivered. Otherwise - if changing to K_UNICODE undoes K_OFF - then suddenly Alt-F2 under Gnome will switch VT instead of summoning the "run command" dialog.

Drop the K_OFF handling and replace it with a new "mute" ioctl pair. Anybody using K_OFF would already need to be prepared to handle it throwing -EINVAL for old kernel compatibility, so userspace will degrade gracefully.

Quoted 5 lines

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=859485

Cc Arthur Taylor <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>

Well, I tested a build with this patch so the tested-by is accurate. It fixes the problem when it builds. But...

Quoted 4 lines
drivers/tty/vt/keyboard.c +0 -0
index 681765b..08d1d57 100644

<snip>

drivers/tty/vt/keyboard.c int vt_do_kdgkbmode(int console) · line 2103

Quoted 7 lines
drivers/tty/vt/keyboard.c +7 -0
+int vt_do_kgdbmute(int console)
+{
+       struct kbd_struct * kbd = kbd_table + console;
+       /* This is a spot read so needs no locking */
+       return vc_kbd_mode(kbd, VC_MUTE);
+}
+

That should be vt_do_kdgkbmute. I know kgdb is probably easier to type because 'gdb' is ingrained into every developer's finger memory, but the kernel gets really grumpy when function definitions don't match what callers actually call:

drivers/built-in.o: In function `vt_ioctl':
/home/jwboyer/kernel/drivers/tty/vt/vt_ioctl.c:489: undefined reference to `vt_do_kdgkbmute'

After that small fixup, the patch does build and does still fix the problem with a properly fixed up Xorg.

josh

#2 lore raw
In reply to Adam Jackson
Quoted 4 lines

Drop the K_OFF handling and replace it with a new "mute" ioctl pair. Anybody using K_OFF would already need to be prepared to handle it throwing -EINVAL for old kernel compatibility, so userspace will degrade gracefully.

Interesting theory. It may degrade but it'll degrade to a state worse than current.

I don't see why K_OFF needs to go away. If the mute ioctl handling also affects mute mode too then it should just work.

Alan

#3 lore raw
Quoted 7 lines
Quoted 4 lines

Drop the K_OFF handling and replace it with a new "mute" ioctl pair. Anybody using K_OFF would already need to be prepared to handle it throwing -EINVAL for old kernel compatibility, so userspace will degrade gracefully.

Interesting theory. It may degrade but it'll degrade to a state worse than current.

AFAIK the only consumer of K_OFF is Xorg, as I wrote both the patches, and Xorg will degrade nicely if K_OFF doesn't work.

I'm confused why g-c-c is trying to set the virtual console keyboard mode, as it is completely the wrong thing to do. X never wants its virtual console set to K_UNICODE, which is the same as K_XLATE, but in unicode. Doing so causes the kernel to buffer lines and handle things like CTRL-C and ALT-Fn.

As this is a per-virtual console setting, setting it on the current VT where X is running has no effect on other virtual consoles.

Modern X uses the input event subsystem directly and has no need for the virtual console to be involved with input. Ancient X required that the virtual console be in K_RAW.

-Art