Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1466114 > unrolled thread

[PATCH 3.12 098/100] Input: i8042 - break load dependency between atkbd/psmouse and i8042

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3.12 098/100] Input: i8042 - break load dependency between atkbd/psmouse and i8042 Jiri Slaby <jslaby@suse.cz> - 2016-08-19 09:20 +0200
    Re: [PATCH 3.12 098/100] Input: i8042 - break load dependency between  atkbd/psmouse and i8042 Jiri Slaby <jslaby@suse.cz> - 2016-08-19 12:10 +0200

#1466114 — [PATCH 3.12 098/100] Input: i8042 - break load dependency between atkbd/psmouse and i8042

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

3.12-stable review patch.  If anyone has any objections, please let me know.

===============

commit 4097461897df91041382ff6fcd2bfa7ee6b2448c upstream.

As explained in 1407814240-4275-1-git-send-email-decui@microsoft.com we
have a hard load dependency between i8042 and atkbd which prevents
keyboard from working on Gen2 Hyper-V VMs.

> hyperv_keyboard invokes serio_interrupt(), which needs a valid serio
> driver like atkbd.c.  atkbd.c depends on libps2.c because it invokes
> ps2_command().  libps2.c depends on i8042.c because it invokes
> i8042_check_port_owner().  As a result, hyperv_keyboard actually
> depends on i8042.c.
>
> For a Generation 2 Hyper-V VM (meaning no i8042 device emulated), if a
> Linux VM (like Arch Linux) happens to configure CONFIG_SERIO_I8042=m
> rather than =y, atkbd.ko can't load because i8042.ko can't load(due to
> no i8042 device emulated) and finally hyperv_keyboard can't work and
> the user can't input: https://bugs.archlinux.org/task/39820
> (Ubuntu/RHEL/SUSE aren't affected since they use CONFIG_SERIO_I8042=y)

To break the dependency we move away from using i8042_check_port_owner()
and instead allow serio port owner specify a mutex that clients should use
to serialize PS/2 command stream.

Reported-by: Mark Laws <mdl@60hz.org>
Tested-by: Mark Laws <mdl@60hz.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/input/serio/i8042.c  | 16 +---------------
 drivers/input/serio/libps2.c | 10 ++++------
 include/linux/i8042.h        |  6 ------
 include/linux/serio.h        | 24 +++++++++++++++++++-----
 4 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 42825216e83d..7ecca05bd7a5 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -1230,6 +1230,7 @@ static int __init i8042_create_kbd_port(void)
 	serio->start		= i8042_start;
 	serio->stop		= i8042_stop;
 	serio->close		= i8042_port_close;
+	serio->ps2_cmd_mutex	= &i8042_mutex;
 	serio->port_data	= port;
 	serio->dev.parent	= &i8042_platform_device->dev;
 	strlcpy(serio->name, "i8042 KBD port", sizeof(serio->name));
@@ -1321,21 +1322,6 @@ static void i8042_unregister_ports(void)
 	}
 }
 
-/*
- * Checks whether port belongs to i8042 controller.
- */
-bool i8042_check_port_owner(const struct serio *port)
-{
-	int i;
-
-	for (i = 0; i < I8042_NUM_PORTS; i++)
-		if (i8042_ports[i].serio == port)
-			return true;
-
-	return false;
-}
-EXPORT_SYMBOL(i8042_check_port_owner);
-
 static void i8042_free_irqs(void)
 {
 	if (i8042_aux_irq_registered)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index 07a8363f3c5c..b5ec313cb9c9 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -57,19 +57,17 @@ EXPORT_SYMBOL(ps2_sendbyte);
 
 void ps2_begin_command(struct ps2dev *ps2dev)
 {
-	mutex_lock(&ps2dev->cmd_mutex);
+	struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
 
-	if (i8042_check_port_owner(ps2dev->serio))
-		i8042_lock_chip();
+	mutex_lock(m);
 }
 EXPORT_SYMBOL(ps2_begin_command);
 
 void ps2_end_command(struct ps2dev *ps2dev)
 {
-	if (i8042_check_port_owner(ps2dev->serio))
-		i8042_unlock_chip();
+	struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex;
 
-	mutex_unlock(&ps2dev->cmd_mutex);
+	mutex_unlock(m);
 }
 EXPORT_SYMBOL(ps2_end_command);
 
diff --git a/include/linux/i8042.h b/include/linux/i8042.h
index 0f9bafa17a02..d98780ca9604 100644
--- a/include/linux/i8042.h
+++ b/include/linux/i8042.h
@@ -62,7 +62,6 @@ struct serio;
 void i8042_lock_chip(void);
 void i8042_unlock_chip(void);
 int i8042_command(unsigned char *param, int command);
-bool i8042_check_port_owner(const struct serio *);
 int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
 					struct serio *serio));
 int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char str,
@@ -83,11 +82,6 @@ static inline int i8042_command(unsigned char *param, int command)
 	return -ENODEV;
 }
 
-static inline bool i8042_check_port_owner(const struct serio *serio)
-{
-	return false;
-}
-
 static inline int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
 					struct serio *serio))
 {
diff --git a/include/linux/serio.h b/include/linux/serio.h
index 9f779c7a2da4..27ae809edd70 100644
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -29,7 +29,8 @@ struct serio {
 
 	struct serio_device_id id;
 
-	spinlock_t lock;		/* protects critical sections from port's interrupt handler */
+	/* Protects critical sections from port's interrupt handler */
+	spinlock_t lock;
 
 	int (*write)(struct serio *, unsigned char);
 	int (*open)(struct serio *);
@@ -38,16 +39,29 @@ struct serio {
 	void (*stop)(struct serio *);
 
 	struct serio *parent;
-	struct list_head child_node;	/* Entry in parent->children list */
+	/* Entry in parent->children list */
+	struct list_head child_node;
 	struct list_head children;
-	unsigned int depth;		/* level of nesting in serio hierarchy */
+	/* Level of nesting in serio hierarchy */
+	unsigned int depth;
 
-	struct serio_driver *drv;	/* accessed from interrupt, must be protected by serio->lock and serio->sem */
-	struct mutex drv_mutex;		/* protects serio->drv so attributes can pin driver */
+	/*
+	 * serio->drv is accessed from interrupt handlers; when modifying
+	 * caller should acquire serio->drv_mutex and serio->lock.
+	 */
+	struct serio_driver *drv;
+	/* Protects serio->drv so attributes can pin current driver */
+	struct mutex drv_mutex;
 
 	struct device dev;
 
 	struct list_head node;
+
+	/*
+	 * For use by PS/2 layer when several ports share hardware and
+	 * may get indigestion when exposed to concurrent access (i8042).
+	 */
+	struct mutex *ps2_cmd_mutex;
 };
 #define to_serio_port(d)	container_of(d, struct serio, dev)
 
-- 
2.9.3

[toc] | [next] | [standalone]

#1466263 — Re: [PATCH 3.12 098/100] Input: i8042 - break load dependency between atkbd/psmouse and i8042

On 08/19/2016, 09:11 AM, Jiri Slaby wrote:
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> 3.12-stable review patch.  If anyone has any objections, please let me know.
> 
> ===============
> 
> commit 4097461897df91041382ff6fcd2bfa7ee6b2448c upstream.
> 
> As explained in 1407814240-4275-1-git-send-email-decui@microsoft.com we
> have a hard load dependency between i8042 and atkbd which prevents
> keyboard from working on Gen2 Hyper-V VMs.
> 
>> hyperv_keyboard invokes serio_interrupt(), which needs a valid serio
>> driver like atkbd.c.  atkbd.c depends on libps2.c because it invokes
>> ps2_command().  libps2.c depends on i8042.c because it invokes
>> i8042_check_port_owner().  As a result, hyperv_keyboard actually
>> depends on i8042.c.
>>
>> For a Generation 2 Hyper-V VM (meaning no i8042 device emulated), if a
>> Linux VM (like Arch Linux) happens to configure CONFIG_SERIO_I8042=m
>> rather than =y, atkbd.ko can't load because i8042.ko can't load(due to
>> no i8042 device emulated) and finally hyperv_keyboard can't work and
>> the user can't input: https://bugs.archlinux.org/task/39820
>> (Ubuntu/RHEL/SUSE aren't affected since they use CONFIG_SERIO_I8042=y)
> 
> To break the dependency we move away from using i8042_check_port_owner()
> and instead allow serio port owner specify a mutex that clients should use
> to serialize PS/2 command stream.
> 
> Reported-by: Mark Laws <mdl@60hz.org>
> Tested-by: Mark Laws <mdl@60hz.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>

Dropped from 3.12 for the same reason as in
CAKdAkRS7-hrnMn-ibWT_+1eM859GuPZAg=fSqGQrY8bT7zC+rw@mail.gmail.com.

-- 
js
suse labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web