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

Groups > linux.kernel > #1465534 > unrolled thread

[PATCH 4.7 172/186] Input: i8042 - break load dependency between atkbd/psmouse and i8042

Started byGreg Kroah-Hartman <gregkh@linuxfoundation.org>
First post2016-08-18 16:30 +0200
Last post2016-08-19 19:20 +0200
Articles 4 — 2 participants

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 4.7 172/186] Input: i8042 - break load dependency between atkbd/psmouse and i8042 Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-18 16:30 +0200
    Re: [PATCH 4.7 172/186] Input: i8042 - break load dependency between  atkbd/psmouse and i8042 Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-08-19 07:00 +0200
      Re: [PATCH 4.7 172/186] Input: i8042 - break load dependency between  atkbd/psmouse and i8042 Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-19 09:50 +0200
        Re: [PATCH 4.7 172/186] Input: i8042 - break load dependency between  atkbd/psmouse and i8042 Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-08-19 19:20 +0200

#1465534 — [PATCH 4.7 172/186] Input: i8042 - break load dependency between atkbd/psmouse and i8042

FromGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Date2016-08-18 16:30 +0200
Subject[PATCH 4.7 172/186] Input: i8042 - break load dependency between atkbd/psmouse and i8042
Message-ID<s7wuv-WK-67@gated-at.bofh.it>
4.7-stable review patch.  If anyone has any objections, please let me know.

------------------

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

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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 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(-)

--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -1277,6 +1277,7 @@ static int __init i8042_create_kbd_port(
 	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));
@@ -1373,21 +1374,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)
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -56,19 +56,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);
 
--- 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
 	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))
 {
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -31,7 +31,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 *);
@@ -40,16 +41,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)

[toc] | [next] | [standalone]

#1465988 — Re: [PATCH 4.7 172/186] Input: i8042 - break load dependency between atkbd/psmouse and i8042

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2016-08-19 07:00 +0200
SubjectRe: [PATCH 4.7 172/186] Input: i8042 - break load dependency between atkbd/psmouse and i8042
Message-ID<s7K4p-11j-7@gated-at.bofh.it>
In reply to#1465534
On Thu, Aug 18, 2016 at 6:59 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> 4.7-stable review patch.  If anyone has any objections, please let me know.

Greg, please hold this patch off till your next cycle. I will be
sending a fixup for it to Linus today or tomorrow.

Thanks!

>
> ------------------
>
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> 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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> ---
>  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(-)
>
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -1277,6 +1277,7 @@ static int __init i8042_create_kbd_port(
>         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));
> @@ -1373,21 +1374,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)
> --- a/drivers/input/serio/libps2.c
> +++ b/drivers/input/serio/libps2.c
> @@ -56,19 +56,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);
>
> --- 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
>         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))
>  {
> --- a/include/linux/serio.h
> +++ b/include/linux/serio.h
> @@ -31,7 +31,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 *);
> @@ -40,16 +41,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)
>
>
>



-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1466196 — Re: [PATCH 4.7 172/186] Input: i8042 - break load dependency between atkbd/psmouse and i8042

FromGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Date2016-08-19 09:50 +0200
SubjectRe: [PATCH 4.7 172/186] Input: i8042 - break load dependency between atkbd/psmouse and i8042
Message-ID<s7MIW-2K9-11@gated-at.bofh.it>
In reply to#1465988
On Thu, Aug 18, 2016 at 09:29:37AM -0700, Dmitry Torokhov wrote:
> On Thu, Aug 18, 2016 at 6:59 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > 4.7-stable review patch.  If anyone has any objections, please let me know.
> 
> Greg, please hold this patch off till your next cycle. I will be
> sending a fixup for it to Linus today or tomorrow.

I've now removed this from all of the queues, thanks.  I'll add it to
the next round of releases, hopefully you tagged the "fixup" for stable
so I'll see it when it lands in Linus's tree.  If not, can you send me
the git commit id?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1466575 — Re: [PATCH 4.7 172/186] Input: i8042 - break load dependency between atkbd/psmouse and i8042

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2016-08-19 19:20 +0200
SubjectRe: [PATCH 4.7 172/186] Input: i8042 - break load dependency between atkbd/psmouse and i8042
Message-ID<s7VCy-8rz-23@gated-at.bofh.it>
In reply to#1466196
On Fri, Aug 19, 2016 at 09:37:21AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Aug 18, 2016 at 09:29:37AM -0700, Dmitry Torokhov wrote:
> > On Thu, Aug 18, 2016 at 6:59 AM, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > > 4.7-stable review patch.  If anyone has any objections, please let me know.
> > 
> > Greg, please hold this patch off till your next cycle. I will be
> > sending a fixup for it to Linus today or tomorrow.
> 
> I've now removed this from all of the queues, thanks.  I'll add it to
> the next round of releases, hopefully you tagged the "fixup" for stable
> so I'll see it when it lands in Linus's tree.  If not, can you send me
> the git commit id?

It is here (in my tree ATM but the commit ID should be stable) and
it is marked for stable:

commit 47af45d684b5f3ae000ad448db02ce4f13f73273
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date:   Tue Aug 16 17:38:54 2016 -0700

    Input: i8042 - set up shared ps2_cmd_mutex for AUX ports

    The commit 4097461897df ("Input: i8042 - break load dependency ...")
    correctly set up ps2_cmd_mutex pointer for the KBD port but forgot to do
    the same for AUX port(s), which results in communication on KBD and AUX
    ports to clash with each other.

    Fixes: 4097461897df ("Input: i8042 - break load dependency ...")
    Reported-by: Bruno Wolff III <bruno@wolff.to>
    Tested-by: Bruno Wolff III <bruno@wolff.to>
    Cc: stable@vger.kernel.org
    Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

-- 
Dmitry

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web