linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] Input: Add userio module
@ 2015-09-17 23:00 cpaul
  2015-09-19 17:39 ` Dmitry Torokhov
  2015-09-22 10:59 ` David Herrmann
  0 siblings, 2 replies; 19+ messages in thread
From: cpaul @ 2015-09-17 23:00 UTC (permalink / raw)
  To: David Herrmann, linux-kernel, open list:HID CORE LAYER, Linux API
  Cc: Dmitry Torokhov, Andrew Morton, Mauro Carvalho Chehab, Greg KH,
	Arnd Bergmann, Joe Perches, Jiri Slaby, Vishnu Patekar,
	Sebastian Ott, Benjamin Tissoires, Hans de Goede, linux-doc

From: Stephen Chandler Paul <cpaul@redhat.com>

Debugging input devices, specifically laptop touchpads, can be tricky
without having the physical device handy. Here we try to remedy that
with userio. This module allows an application to connect to a character
device provided by the kernel, and emulate any serio device. In
combination with userspace programs that can record PS/2 devices and
replay them through the /dev/userio device, this allows developers to
debug driver issues on the PS/2 level with devices simply by requesting
a recording from the user experiencing the issue without having to have
the physical hardware in front of them.

Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
---
				    Changes

* Correct "VIRTUAL PS/2 DEVICE" to "VIRTUAL SERIO DEVICE" in MAINTAINERS
* Remove uneccessary forward declaration in userio
* s/repeat repeat/repeat/
* Drop userio->serio checks, this struct is allocated right when we register
  the serio port so we don't need to worry about this ever being NULL
* Allocate a static minor and use that
* Add MODULE_ALIAS_MISCDEV() and MODULOE_ALIAS() lines to the module
* Improve the module description
* Change description for userio.h to "userio: virtual serio device support"
* Fix double space in comments in userio.h
* Change userio command types to enumerators

				   Responses
* Re: if (!userio) in userio_device_write(): it happens if we close the file
  descriptor while the input driver is trying to talk to the device, since we
  can't immediately bring down the driver. Removing the condition breaks the
  driver
* Re: ignoring trailing bytes: The main reason for that is if we need to add
  something else to the struct in the future. I doubt this will ever be done,
  but I figured I might as well let that behavior go since it can't harm
  anyone.
* Re: ignoring larger payloads + adding larger payloads: As of right now there
  really isn't one. serio_send() only allows you to send a single character at
  a time, and we can add the ability to have flags using this same packet
  format. I don't think userio is ever going to get much more complex then
  this, since I don't think serio is going to be changing very much in the
  future. This being said, if some unlikely circumstances happen and it does
  happen  to change in such a way we need to extend the payload, we're
  currently ignoring the rest of the payload so we could safely add additional
  data onto the end of the struct.

 Documentation/input/userio.txt |  70 +++++++++++
 MAINTAINERS                    |   6 +
 drivers/input/serio/Kconfig    |  11 ++
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/userio.c   | 277 +++++++++++++++++++++++++++++++++++++++++
 include/linux/miscdevice.h     |   1 +
 include/uapi/linux/userio.h    |  44 +++++++
 7 files changed, 410 insertions(+)
 create mode 100644 Documentation/input/userio.txt
 create mode 100644 drivers/input/serio/userio.c
 create mode 100644 include/uapi/linux/userio.h

diff --git a/Documentation/input/userio.txt b/Documentation/input/userio.txt
new file mode 100644
index 0000000..0880c0f
--- /dev/null
+++ b/Documentation/input/userio.txt
@@ -0,0 +1,70 @@
+			      The userio Protocol
+	     (c) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+			      Sponsored by Red Hat
+--------------------------------------------------------------------------------
+
+1. Introduction
+~~~~~~~~~~~~~~~
+  This module is intended to try to make the lives of input driver developers
+easier by allowing them to test various serio devices (mainly the various
+touchpads found on laptops) without having to have the physical device in front
+of them. userio accomplishes this by allowing any privileged userspace program
+to directly interact with the kernel's serio driver and control a virtual serio
+port from there.
+
+2. Usage overview
+~~~~~~~~~~~~~~~~~
+  In order to interact with the userio kernel module, one simply opens the
+/dev/userio character device in their applications. Commands are sent to the
+kernel module by writing to the device, and any data received from the serio
+driver is read as-is from the /dev/userio device. All of the structures and
+macros you need to interact with the device are defined in <linux/userio.h> and
+<linux/serio.h>.
+
+3. Command Structure
+~~~~~~~~~~~~~~~~~~~~
+  The struct used for sending commands to /dev/userio is as follows:
+
+	struct userio_cmd {
+		__u8 type;
+		__u8 data;
+	};
+
+  "type" describes the type of command that is being sent. This can be any one
+of the USERIO_CMD macros defined in <linux/userio.h>. "data" is the argument
+that goes along with the command. In the event that the command doesn't have an
+argument, this field can be left untouched and will be ignored by the kernel.
+Each command should be sent by writing the struct directly to the character
+device. In the event that the command you send is invalid, an error will be
+returned by the character device and a more descriptive error will be printed
+to the kernel log. Only one command can be sent at a time, any additional data
+written to the character device after the initial command will be ignored.
+  To close the virtual serio port, just close /dev/userio.
+
+4. Commands
+~~~~~~~~~~~
+
+4.1 USERIO_CMD_REGISTER
+~~~~~~~~~~~~~~~~~~~~~~~
+  Registers the port with the serio driver and begins transmitting data back and
+forth. Registration can only be performed once a port type is set with
+USERIO_CMD_SET_PORT_TYPE. Has no argument.
+
+4.2 USERIO_CMD_SET_PORT_TYPE
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sets the type of port we're emulating, where "data" is the port type being
+set. Can be any of the macros from <linux/serio.h>. For example: SERIO_8042
+would set the port type to be a normal PS/2 port.
+
+4.3 USERIO_CMD_SEND_INTERRUPT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sends an interrupt through the virtual serio port to the serio driver, where
+"data" is the interrupt data being sent.
+
+5. Userspace tools
+~~~~~~~~~~~~~~~~~~
+  The userio userspace tools are able to record PS/2 devices using some of the
+debugging information from i8042, and play back the devices on /dev/userio. The
+latest version of these tools can be found at:
+
+	https://github.com/Lyude/ps2emu
diff --git a/MAINTAINERS b/MAINTAINERS
index 7ba7ab7..0a0fe08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11086,6 +11086,12 @@ S:	Maintained
 F:	drivers/media/v4l2-core/videobuf2-*
 F:	include/media/videobuf2-*
 
+VIRTUAL SERIO DEVICE DRIVER
+M:	Stephen Chandler Paul <thatslyude@gmail.com>
+S:	Maintained
+F:	drivers/input/serio/ps2emu.c
+F:	include/uapi/linux/ps2emu.h
+
 VIRTIO CONSOLE DRIVER
 M:	Amit Shah <amit.shah@redhat.com>
 L:	virtualization@lists.linux-foundation.org
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 200841b..22b8b58 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,4 +292,15 @@ config SERIO_SUN4I_PS2
 	  To compile this driver as a module, choose M here: the
 	  module will be called sun4i-ps2.
 
+config USERIO
+	tristate "Virtual serio device support"
+	help
+	  Say Y here if you want to emulate serio devices using the userio
+	  kernel module.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called userio.
+
+	  If you are unsure, say N.
+
 endif
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index c600089..2374ef9 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_SERIO_APBPS2)	+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)	+= olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)	+= hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)	+= sun4i-ps2.o
+obj-$(CONFIG_USERIO)		+= userio.o
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
new file mode 100644
index 0000000..f32cb3b
--- /dev/null
+++ b/drivers/input/serio/userio.c
@@ -0,0 +1,277 @@
+/*
+ * userio kernel serio device emulation module
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ */
+#include <linux/circ_buf.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <linux/sched.h>
+#include <linux/poll.h>
+#include <uapi/linux/userio.h>
+
+#define USERIO_NAME "userio"
+#define USERIO_BUFSIZE 16
+
+static struct miscdevice userio_misc;
+
+struct userio_device {
+	struct serio *serio;
+	struct mutex lock;
+
+	bool running;
+
+	u8 head;
+	u8 tail;
+
+	unsigned char buf[USERIO_BUFSIZE];
+
+	wait_queue_head_t waitq;
+};
+
+/**
+ * userio_device_write - Write data from serio to a userio device in userspace
+ * @id: The serio port for the userio device
+ * @val: The data to write to the device
+ */
+static int userio_device_write(struct serio *id, unsigned char val)
+{
+	struct userio_device *userio = id->port_data;
+
+	if (!userio)
+		return -1;
+
+	mutex_lock(&userio->lock);
+
+	userio->buf[userio->head] = val;
+	userio->head = (userio->head + 1) % USERIO_BUFSIZE;
+
+	if (userio->head == userio->tail)
+		dev_warn(userio_misc.this_device,
+			 "Buffer overflowed, userio client isn't keeping up");
+
+	mutex_unlock(&userio->lock);
+
+	wake_up_interruptible(&userio->waitq);
+
+	return 0;
+}
+
+static int userio_char_open(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio;
+
+	userio = kzalloc(sizeof(struct userio_device), GFP_KERNEL);
+	if (!userio)
+		return -ENOMEM;
+
+	mutex_init(&userio->lock);
+	init_waitqueue_head(&userio->waitq);
+
+	userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
+	if (!userio->serio) {
+		kfree(userio);
+		return -ENOMEM;
+	}
+
+	userio->serio->write = userio_device_write;
+	userio->serio->port_data = userio;
+
+	file->private_data = userio;
+
+	return 0;
+}
+
+static int userio_char_release(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio = file->private_data;
+
+	/* Don't free the serio port here, serio_unregister_port() does
+	 * this for us */
+	if (userio->running)
+		serio_unregister_port(userio->serio);
+	else
+		kfree(userio->serio);
+
+	kfree(userio);
+
+	return 0;
+}
+
+static ssize_t userio_char_read(struct file *file, char __user *buffer,
+				size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	int ret;
+	size_t nonwrap_len, copylen;
+
+	if (!count)
+		return 0;
+
+	/*
+	 * By the time we get here, the data that was waiting might have been
+	 * taken by another thread. Grab the mutex and check if there's still
+	 * any data waiting, otherwise repeat repeat this process until we have
+	 * data (unless the file descriptor is non-blocking of course)
+	 */
+	for (;;) {
+		ret = mutex_lock_interruptible(&userio->lock);
+		if (ret)
+			return ret;
+
+		if (userio->head != userio->tail)
+			break;
+
+		mutex_unlock(&userio->lock);
+
+		if (file->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+
+		ret = wait_event_interruptible(userio->waitq,
+					       userio->head != userio->tail);
+		if (ret)
+			return ret;
+	}
+
+	nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
+				      USERIO_BUFSIZE);
+	copylen = min(nonwrap_len, count);
+
+	if (copy_to_user(buffer, &userio->buf[userio->tail], copylen)) {
+		mutex_unlock(&userio->lock);
+		return -EFAULT;
+	}
+
+	userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
+
+	mutex_unlock(&userio->lock);
+
+	return copylen;
+}
+
+static ssize_t userio_char_write(struct file *file, const char __user *buffer,
+				 size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	struct userio_cmd cmd;
+	int ret;
+
+	if (count < sizeof(cmd))
+		return -EINVAL;
+
+	if (copy_from_user(&cmd, buffer, sizeof(cmd)))
+		return -EFAULT;
+
+	ret = mutex_lock_interruptible(&userio->lock);
+	if (ret)
+		return ret;
+
+	switch (cmd.type) {
+	case USERIO_CMD_REGISTER:
+		if (!userio->serio->id.type) {
+			dev_warn(userio_misc.this_device,
+				 "No port type given on /dev/userio\n");
+
+			ret = -EINVAL;
+			goto out;
+		}
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Begin command sent, but we're already running\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->running = true;
+		serio_register_port(userio->serio);
+		break;
+
+	case USERIO_CMD_SET_PORT_TYPE:
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Can't change port type on an already running userio instance\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->serio->id.type = cmd.data;
+		break;
+
+	case USERIO_CMD_SEND_INTERRUPT:
+		if (!userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "The device must be registered before sending interrupts\n");
+
+			ret = -ENODEV;
+			goto out;
+		}
+
+		serio_interrupt(userio->serio, cmd.data, 0);
+		break;
+
+	default:
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	ret = sizeof(cmd);
+
+out:
+	mutex_unlock(&userio->lock);
+	return ret;
+}
+
+static unsigned int userio_char_poll(struct file *file, poll_table *wait)
+{
+	struct userio_device *userio = file->private_data;
+
+	poll_wait(file, &userio->waitq, wait);
+
+	if (userio->head != userio->tail)
+		return POLLIN | POLLRDNORM;
+
+	return 0;
+}
+
+static const struct file_operations userio_fops = {
+	.owner		= THIS_MODULE,
+	.open		= userio_char_open,
+	.release	= userio_char_release,
+	.read		= userio_char_read,
+	.write		= userio_char_write,
+	.poll		= userio_char_poll,
+	.llseek		= no_llseek,
+};
+
+static struct miscdevice userio_misc = {
+	.fops	= &userio_fops,
+	.minor	= USERIO_MINOR,
+	.name	= USERIO_NAME,
+};
+
+MODULE_ALIAS_MISCDEV(USERIO_MINOR);
+MODULE_ALIAS("devname:" USERIO_NAME);
+
+MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
+MODULE_DESCRIPTION("Virtual Serio Device Support");
+MODULE_LICENSE("GPL");
+
+module_driver(userio_misc, misc_register, misc_deregister);
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 81f6e42..5430374 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -49,6 +49,7 @@
 #define LOOP_CTRL_MINOR		237
 #define VHOST_NET_MINOR		238
 #define UHID_MINOR		239
+#define USERIO_MINOR		240
 #define MISC_DYNAMIC_MINOR	255
 
 struct device;
diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
new file mode 100644
index 0000000..37935b6
--- /dev/null
+++ b/include/uapi/linux/userio.h
@@ -0,0 +1,44 @@
+/*
+ * userio: virtual serio device support
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ *
+ * This is the public header used for user-space communication with the userio
+ * driver. __attribute__((__packed__)) is used for all structs to keep ABI
+ * compatibility between all architectures.
+ */
+
+#ifndef _USERIO_H
+#define _USERIO_H
+
+#include <linux/types.h>
+
+enum userio_cmd_type {
+	USERIO_CMD_REGISTER = 0,
+	USERIO_CMD_SET_PORT_TYPE = 1,
+	USERIO_CMD_SEND_INTERRUPT = 2
+};
+
+/*
+ * userio Commands
+ * All commands sent to /dev/userio are encoded using this structure. The type
+ * field should contain a USERIO_CMD* value that indicates what kind of command
+ * is being sent to userio. The data field should contain the accompanying
+ * argument for the command, if there is one.
+ */
+struct userio_cmd {
+	enum userio_cmd_type type;
+	__u8 data;
+} __attribute__((__packed__));
+
+#endif /* !_USERIO_H */
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v5] Input: Add userio module
  2015-09-17 23:00 [PATCH v5] Input: Add userio module cpaul
@ 2015-09-19 17:39 ` Dmitry Torokhov
  2015-09-22 10:59 ` David Herrmann
  1 sibling, 0 replies; 19+ messages in thread
From: Dmitry Torokhov @ 2015-09-19 17:39 UTC (permalink / raw)
  To: cpaul
  Cc: David Herrmann, linux-kernel, open list:HID CORE LAYER,
	Linux API, Andrew Morton, Mauro Carvalho Chehab, Greg KH,
	Arnd Bergmann, Joe Perches, Jiri Slaby, Vishnu Patekar,
	Sebastian Ott, Benjamin Tissoires, Hans de Goede, linux-doc

Hi Stephen,

On Thu, Sep 17, 2015 at 07:00:10PM -0400, cpaul@redhat.com wrote:
> 				   Responses
> * Re: if (!userio) in userio_device_write(): it happens if we close the file
>   descriptor while the input driver is trying to talk to the device, since we
>   can't immediately bring down the driver. Removing the condition breaks the
>   driver

...

> +/**
> + * userio_device_write - Write data from serio to a userio device in userspace
> + * @id: The serio port for the userio device
> + * @val: The data to write to the device
> + */
> +static int userio_device_write(struct serio *id, unsigned char val)
> +{
> +	struct userio_device *userio = id->port_data;
> +
> +	if (!userio)
> +		return -1;

1. I do not see where we reset port data.
2. I do not see what prevents object to which you now have a pointer to
from disappearing at any moment past your check.

> +
> +	mutex_lock(&userio->lock);

serio_write() must allow to be called from interrupt context, so you
can't use mutex here.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5] Input: Add userio module
  2015-09-17 23:00 [PATCH v5] Input: Add userio module cpaul
  2015-09-19 17:39 ` Dmitry Torokhov
@ 2015-09-22 10:59 ` David Herrmann
  2015-09-23 17:49   ` [PATCH v6 1/1] " cpaul
  1 sibling, 1 reply; 19+ messages in thread
From: David Herrmann @ 2015-09-22 10:59 UTC (permalink / raw)
  To: Stephen Chandler Paul
  Cc: linux-kernel, open list:HID CORE LAYER, Linux API,
	Dmitry Torokhov, Andrew Morton, Mauro Carvalho Chehab, Greg KH,
	Arnd Bergmann, Joe Perches, Jiri Slaby, Vishnu Patekar,
	Sebastian Ott, Benjamin Tissoires, Hans de Goede, linux-doc

Hi

On Fri, Sep 18, 2015 at 1:00 AM,  <cpaul@redhat.com> wrote:
> From: Stephen Chandler Paul <cpaul@redhat.com>
>
> Debugging input devices, specifically laptop touchpads, can be tricky
> without having the physical device handy. Here we try to remedy that
> with userio. This module allows an application to connect to a character
> device provided by the kernel, and emulate any serio device. In
> combination with userspace programs that can record PS/2 devices and
> replay them through the /dev/userio device, this allows developers to
> debug driver issues on the PS/2 level with devices simply by requesting
> a recording from the user experiencing the issue without having to have
> the physical hardware in front of them.
>
> Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
> ---
>                                     Changes
>
> * Correct "VIRTUAL PS/2 DEVICE" to "VIRTUAL SERIO DEVICE" in MAINTAINERS
> * Remove uneccessary forward declaration in userio
> * s/repeat repeat/repeat/
> * Drop userio->serio checks, this struct is allocated right when we register
>   the serio port so we don't need to worry about this ever being NULL
> * Allocate a static minor and use that
> * Add MODULE_ALIAS_MISCDEV() and MODULOE_ALIAS() lines to the module
> * Improve the module description
> * Change description for userio.h to "userio: virtual serio device support"
> * Fix double space in comments in userio.h
> * Change userio command types to enumerators
>
>                                    Responses
> * Re: if (!userio) in userio_device_write(): it happens if we close the file
>   descriptor while the input driver is trying to talk to the device, since we
>   can't immediately bring down the driver. Removing the condition breaks the
>   driver
> * Re: ignoring trailing bytes: The main reason for that is if we need to add
>   something else to the struct in the future. I doubt this will ever be done,
>   but I figured I might as well let that behavior go since it can't harm
>   anyone.
> * Re: ignoring larger payloads + adding larger payloads: As of right now there
>   really isn't one. serio_send() only allows you to send a single character at
>   a time, and we can add the ability to have flags using this same packet
>   format. I don't think userio is ever going to get much more complex then
>   this, since I don't think serio is going to be changing very much in the
>   future. This being said, if some unlikely circumstances happen and it does
>   happen  to change in such a way we need to extend the payload, we're
>   currently ignoring the rest of the payload so we could safely add additional
>   data onto the end of the struct.

Please always respond inline to the previous emails. Otherwise, you
break the thread and make it hard to track your responses. Nobody
minds multiple emails.

>  Documentation/input/userio.txt |  70 +++++++++++
>  MAINTAINERS                    |   6 +
>  drivers/input/serio/Kconfig    |  11 ++
>  drivers/input/serio/Makefile   |   1 +
>  drivers/input/serio/userio.c   | 277 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/miscdevice.h     |   1 +
>  include/uapi/linux/userio.h    |  44 +++++++
>  7 files changed, 410 insertions(+)
>  create mode 100644 Documentation/input/userio.txt
>  create mode 100644 drivers/input/serio/userio.c
>  create mode 100644 include/uapi/linux/userio.h

[snip]

> --- /dev/null
> +++ b/drivers/input/serio/userio.c
> @@ -0,0 +1,277 @@
> +/*
> + * userio kernel serio device emulation module
> + * Copyright (C) 2015 Red Hat
> + * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
> + * details.
> + */
> +#include <linux/circ_buf.h>
> +#include <linux/mutex.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/serio.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/miscdevice.h>
> +#include <linux/sched.h>
> +#include <linux/poll.h>
> +#include <uapi/linux/userio.h>
> +
> +#define USERIO_NAME "userio"
> +#define USERIO_BUFSIZE 16
> +
> +static struct miscdevice userio_misc;
> +
> +struct userio_device {
> +       struct serio *serio;
> +       struct mutex lock;
> +
> +       bool running;
> +
> +       u8 head;
> +       u8 tail;
> +
> +       unsigned char buf[USERIO_BUFSIZE];
> +
> +       wait_queue_head_t waitq;
> +};
> +
> +/**
> + * userio_device_write - Write data from serio to a userio device in userspace
> + * @id: The serio port for the userio device
> + * @val: The data to write to the device
> + */
> +static int userio_device_write(struct serio *id, unsigned char val)
> +{
> +       struct userio_device *userio = id->port_data;
> +
> +       if (!userio)
> +               return -1;

I still cannot see why this is necessary. Care to elaborate?

> +
> +       mutex_lock(&userio->lock);
> +
> +       userio->buf[userio->head] = val;
> +       userio->head = (userio->head + 1) % USERIO_BUFSIZE;
> +
> +       if (userio->head == userio->tail)
> +               dev_warn(userio_misc.this_device,
> +                        "Buffer overflowed, userio client isn't keeping up");
> +
> +       mutex_unlock(&userio->lock);

As Dmitry mentioned, just add a spinlock to "struct userio_device"
(supplementary to the mutex) which just protects buf, head and tail.

> +
> +       wake_up_interruptible(&userio->waitq);
> +
> +       return 0;
> +}
> +
> +static int userio_char_open(struct inode *inode, struct file *file)
> +{
> +       struct userio_device *userio;
> +
> +       userio = kzalloc(sizeof(struct userio_device), GFP_KERNEL);
> +       if (!userio)
> +               return -ENOMEM;
> +
> +       mutex_init(&userio->lock);
> +       init_waitqueue_head(&userio->waitq);
> +
> +       userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
> +       if (!userio->serio) {
> +               kfree(userio);
> +               return -ENOMEM;
> +       }
> +
> +       userio->serio->write = userio_device_write;
> +       userio->serio->port_data = userio;
> +
> +       file->private_data = userio;
> +
> +       return 0;
> +}
> +
> +static int userio_char_release(struct inode *inode, struct file *file)
> +{
> +       struct userio_device *userio = file->private_data;
> +
> +       /* Don't free the serio port here, serio_unregister_port() does
> +        * this for us */
> +       if (userio->running)
> +               serio_unregister_port(userio->serio);
> +       else
> +               kfree(userio->serio);
> +
> +       kfree(userio);
> +
> +       return 0;
> +}
> +
> +static ssize_t userio_char_read(struct file *file, char __user *buffer,
> +                               size_t count, loff_t *ppos)
> +{
> +       struct userio_device *userio = file->private_data;
> +       int ret;
> +       size_t nonwrap_len, copylen;
> +
> +       if (!count)
> +               return 0;
> +
> +       /*
> +        * By the time we get here, the data that was waiting might have been
> +        * taken by another thread. Grab the mutex and check if there's still
> +        * any data waiting, otherwise repeat repeat this process until we have
> +        * data (unless the file descriptor is non-blocking of course)
> +        */
> +       for (;;) {
> +               ret = mutex_lock_interruptible(&userio->lock);
> +               if (ret)
> +                       return ret;

Here it will be enough to take the spinlock rather than the mutex. The
buffer is empty by default, so no reason to synchronize against
write() calls.

> +
> +               if (userio->head != userio->tail)
> +                       break;
> +
> +               mutex_unlock(&userio->lock);
> +
> +               if (file->f_flags & O_NONBLOCK)
> +                       return -EAGAIN;
> +
> +               ret = wait_event_interruptible(userio->waitq,
> +                                              userio->head != userio->tail);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
> +                                     USERIO_BUFSIZE);
> +       copylen = min(nonwrap_len, count);
> +
> +       if (copy_to_user(buffer, &userio->buf[userio->tail], copylen)) {
> +               mutex_unlock(&userio->lock);
> +               return -EFAULT;
> +       }
> +
> +       userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
> +
> +       mutex_unlock(&userio->lock);
> +
> +       return copylen;
> +}
> +
> +static ssize_t userio_char_write(struct file *file, const char __user *buffer,
> +                                size_t count, loff_t *ppos)
> +{
> +       struct userio_device *userio = file->private_data;
> +       struct userio_cmd cmd;
> +       int ret;
> +
> +       if (count < sizeof(cmd))
> +               return -EINVAL;
> +
> +       if (copy_from_user(&cmd, buffer, sizeof(cmd)))
> +               return -EFAULT;
> +
> +       ret = mutex_lock_interruptible(&userio->lock);
> +       if (ret)
> +               return ret;
> +
> +       switch (cmd.type) {
> +       case USERIO_CMD_REGISTER:
> +               if (!userio->serio->id.type) {
> +                       dev_warn(userio_misc.this_device,
> +                                "No port type given on /dev/userio\n");
> +
> +                       ret = -EINVAL;
> +                       goto out;
> +               }
> +               if (userio->running) {
> +                       dev_warn(userio_misc.this_device,
> +                                "Begin command sent, but we're already running\n");
> +
> +                       ret = -EBUSY;
> +                       goto out;
> +               }
> +
> +               userio->running = true;
> +               serio_register_port(userio->serio);
> +               break;
> +
> +       case USERIO_CMD_SET_PORT_TYPE:
> +               if (userio->running) {
> +                       dev_warn(userio_misc.this_device,
> +                                "Can't change port type on an already running userio instance\n");
> +
> +                       ret = -EBUSY;
> +                       goto out;
> +               }
> +
> +               userio->serio->id.type = cmd.data;
> +               break;
> +
> +       case USERIO_CMD_SEND_INTERRUPT:
> +               if (!userio->running) {
> +                       dev_warn(userio_misc.this_device,
> +                                "The device must be registered before sending interrupts\n");
> +
> +                       ret = -ENODEV;
> +                       goto out;
> +               }
> +
> +               serio_interrupt(userio->serio, cmd.data, 0);
> +               break;
> +
> +       default:
> +               ret = -EOPNOTSUPP;
> +               goto out;
> +       }
> +
> +       ret = sizeof(cmd);
> +
> +out:
> +       mutex_unlock(&userio->lock);
> +       return ret;
> +}
> +
> +static unsigned int userio_char_poll(struct file *file, poll_table *wait)
> +{
> +       struct userio_device *userio = file->private_data;
> +
> +       poll_wait(file, &userio->waitq, wait);
> +
> +       if (userio->head != userio->tail)
> +               return POLLIN | POLLRDNORM;
> +
> +       return 0;
> +}
> +
> +static const struct file_operations userio_fops = {
> +       .owner          = THIS_MODULE,
> +       .open           = userio_char_open,
> +       .release        = userio_char_release,
> +       .read           = userio_char_read,
> +       .write          = userio_char_write,
> +       .poll           = userio_char_poll,
> +       .llseek         = no_llseek,
> +};
> +
> +static struct miscdevice userio_misc = {
> +       .fops   = &userio_fops,
> +       .minor  = USERIO_MINOR,
> +       .name   = USERIO_NAME,
> +};
> +
> +MODULE_ALIAS_MISCDEV(USERIO_MINOR);
> +MODULE_ALIAS("devname:" USERIO_NAME);
> +
> +MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
> +MODULE_DESCRIPTION("Virtual Serio Device Support");
> +MODULE_LICENSE("GPL");
> +
> +module_driver(userio_misc, misc_register, misc_deregister);
> diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
> index 81f6e42..5430374 100644
> --- a/include/linux/miscdevice.h
> +++ b/include/linux/miscdevice.h
> @@ -49,6 +49,7 @@
>  #define LOOP_CTRL_MINOR                237
>  #define VHOST_NET_MINOR                238
>  #define UHID_MINOR             239
> +#define USERIO_MINOR           240
>  #define MISC_DYNAMIC_MINOR     255
>
>  struct device;
> diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
> new file mode 100644
> index 0000000..37935b6
> --- /dev/null
> +++ b/include/uapi/linux/userio.h
> @@ -0,0 +1,44 @@
> +/*
> + * userio: virtual serio device support
> + * Copyright (C) 2015 Red Hat
> + * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
> + * details.
> + *
> + * This is the public header used for user-space communication with the userio
> + * driver. __attribute__((__packed__)) is used for all structs to keep ABI
> + * compatibility between all architectures.
> + */
> +
> +#ifndef _USERIO_H
> +#define _USERIO_H
> +
> +#include <linux/types.h>
> +
> +enum userio_cmd_type {
> +       USERIO_CMD_REGISTER = 0,
> +       USERIO_CMD_SET_PORT_TYPE = 1,
> +       USERIO_CMD_SEND_INTERRUPT = 2
> +};
> +
> +/*
> + * userio Commands
> + * All commands sent to /dev/userio are encoded using this structure. The type
> + * field should contain a USERIO_CMD* value that indicates what kind of command
> + * is being sent to userio. The data field should contain the accompanying
> + * argument for the command, if there is one.
> + */
> +struct userio_cmd {
> +       enum userio_cmd_type type;

Here I'd stick to "__u8". Sorry, that wasn't clear in my previous
response. I just meant changing the definition to an enum.

Otherwise looks good to me.

Thanks
David

> +       __u8 data;
> +} __attribute__((__packed__));
> +
> +#endif /* !_USERIO_H */
> --
> 2.4.3
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v6 1/1] Input: Add userio module
  2015-09-22 10:59 ` David Herrmann
@ 2015-09-23 17:49   ` cpaul
  2015-09-23 17:54     ` [PATCH v6.1 " cpaul
  0 siblings, 1 reply; 19+ messages in thread
From: cpaul @ 2015-09-23 17:49 UTC (permalink / raw)
  To: David Herrmann, Dmitry Torokhov
  Cc: linux-kernel, linux-input, linux-api, Andrew Morton,
	Mauro Carvalho Chehab, Greg KH, Arnd Bergmann, Joe Perches,
	Jiri Slaby, Vishnu Patekar, Sebastian Ott, Benjamin Tissoires,
	Hans de Goede, linux-doc

From: Stephen Chandler Paul <cpaul@redhat.com>

Debugging input devices, specifically laptop touchpads, can be tricky
without having the physical device handy. Here we try to remedy that
with userio. This module allows an application to connect to a character
device provided by the kernel, and emulate any serio device. In
combination with userspace programs that can record PS/2 devices and
replay them through the /dev/userio device, this allows developers to
debug driver issues on the PS/2 level with devices simply by requesting
a recording from the user experiencing the issue without having to have
the physical hardware in front of them.

Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
---
				    Changes
* Change type in userio.h struct to __u8
* Add a spin lock that is used in addition to the mutex, no more completion struct
* Fix "repeat repeat" typo again (not sure how this got here?)
* Use USERIO_BUFSIZE for size of buf array in userio_char_read()
* Rename buffer in userio_char_read() to user_buffer (having both buffer and
  buf could be confusing)

 Documentation/input/userio.txt |  70 ++++++++++
 MAINTAINERS                    |   6 +
 drivers/input/serio/Kconfig    |  11 ++
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/userio.c   | 289 +++++++++++++++++++++++++++++++++++++++++
 include/linux/miscdevice.h     |   1 +
 include/uapi/linux/userio.h    |  44 +++++++
 7 files changed, 422 insertions(+)
 create mode 100644 Documentation/input/userio.txt
 create mode 100644 drivers/input/serio/userio.c
 create mode 100644 include/uapi/linux/userio.h

diff --git a/Documentation/input/userio.txt b/Documentation/input/userio.txt
new file mode 100644
index 0000000..0880c0f
--- /dev/null
+++ b/Documentation/input/userio.txt
@@ -0,0 +1,70 @@
+			      The userio Protocol
+	     (c) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+			      Sponsored by Red Hat
+--------------------------------------------------------------------------------
+
+1. Introduction
+~~~~~~~~~~~~~~~
+  This module is intended to try to make the lives of input driver developers
+easier by allowing them to test various serio devices (mainly the various
+touchpads found on laptops) without having to have the physical device in front
+of them. userio accomplishes this by allowing any privileged userspace program
+to directly interact with the kernel's serio driver and control a virtual serio
+port from there.
+
+2. Usage overview
+~~~~~~~~~~~~~~~~~
+  In order to interact with the userio kernel module, one simply opens the
+/dev/userio character device in their applications. Commands are sent to the
+kernel module by writing to the device, and any data received from the serio
+driver is read as-is from the /dev/userio device. All of the structures and
+macros you need to interact with the device are defined in <linux/userio.h> and
+<linux/serio.h>.
+
+3. Command Structure
+~~~~~~~~~~~~~~~~~~~~
+  The struct used for sending commands to /dev/userio is as follows:
+
+	struct userio_cmd {
+		__u8 type;
+		__u8 data;
+	};
+
+  "type" describes the type of command that is being sent. This can be any one
+of the USERIO_CMD macros defined in <linux/userio.h>. "data" is the argument
+that goes along with the command. In the event that the command doesn't have an
+argument, this field can be left untouched and will be ignored by the kernel.
+Each command should be sent by writing the struct directly to the character
+device. In the event that the command you send is invalid, an error will be
+returned by the character device and a more descriptive error will be printed
+to the kernel log. Only one command can be sent at a time, any additional data
+written to the character device after the initial command will be ignored.
+  To close the virtual serio port, just close /dev/userio.
+
+4. Commands
+~~~~~~~~~~~
+
+4.1 USERIO_CMD_REGISTER
+~~~~~~~~~~~~~~~~~~~~~~~
+  Registers the port with the serio driver and begins transmitting data back and
+forth. Registration can only be performed once a port type is set with
+USERIO_CMD_SET_PORT_TYPE. Has no argument.
+
+4.2 USERIO_CMD_SET_PORT_TYPE
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sets the type of port we're emulating, where "data" is the port type being
+set. Can be any of the macros from <linux/serio.h>. For example: SERIO_8042
+would set the port type to be a normal PS/2 port.
+
+4.3 USERIO_CMD_SEND_INTERRUPT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sends an interrupt through the virtual serio port to the serio driver, where
+"data" is the interrupt data being sent.
+
+5. Userspace tools
+~~~~~~~~~~~~~~~~~~
+  The userio userspace tools are able to record PS/2 devices using some of the
+debugging information from i8042, and play back the devices on /dev/userio. The
+latest version of these tools can be found at:
+
+	https://github.com/Lyude/ps2emu
diff --git a/MAINTAINERS b/MAINTAINERS
index 274f854..44740de 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11086,6 +11086,12 @@ S:	Maintained
 F:	drivers/media/v4l2-core/videobuf2-*
 F:	include/media/videobuf2-*
 
+VIRTUAL SERIO DEVICE DRIVER
+M:	Stephen Chandler Paul <thatslyude@gmail.com>
+S:	Maintained
+F:	drivers/input/serio/ps2emu.c
+F:	include/uapi/linux/ps2emu.h
+
 VIRTIO CONSOLE DRIVER
 M:	Amit Shah <amit.shah@redhat.com>
 L:	virtualization@lists.linux-foundation.org
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 200841b..22b8b58 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,4 +292,15 @@ config SERIO_SUN4I_PS2
 	  To compile this driver as a module, choose M here: the
 	  module will be called sun4i-ps2.
 
+config USERIO
+	tristate "Virtual serio device support"
+	help
+	  Say Y here if you want to emulate serio devices using the userio
+	  kernel module.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called userio.
+
+	  If you are unsure, say N.
+
 endif
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index c600089..2374ef9 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_SERIO_APBPS2)	+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)	+= olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)	+= hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)	+= sun4i-ps2.o
+obj-$(CONFIG_USERIO)		+= userio.o
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
new file mode 100644
index 0000000..f0e1bd4
--- /dev/null
+++ b/drivers/input/serio/userio.c
@@ -0,0 +1,289 @@
+/*
+ * userio kernel serio device emulation module
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ */
+#include <linux/circ_buf.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <linux/sched.h>
+#include <linux/poll.h>
+#include <uapi/linux/userio.h>
+
+#define USERIO_NAME "userio"
+#define USERIO_BUFSIZE 16
+
+static struct miscdevice userio_misc;
+
+struct userio_device {
+	struct serio *serio;
+	struct mutex lock;
+
+	bool running;
+
+	u8 head;
+	u8 tail;
+
+	spinlock_t buf_lock;
+	unsigned char buf[USERIO_BUFSIZE];
+
+	wait_queue_head_t waitq;
+};
+
+/**
+ * userio_device_write - Write data from serio to a userio device in userspace
+ * @id: The serio port for the userio device
+ * @val: The data to write to the device
+ */
+static int userio_device_write(struct serio *id, unsigned char val)
+{
+	struct userio_device *userio = id->port_data;
+	unsigned long flags;
+
+	if (!userio)
+		return -1;
+
+	spin_lock_irqsave(&userio->buf_lock, flags);
+
+	userio->buf[userio->head] = val;
+	userio->head = (userio->head + 1) % USERIO_BUFSIZE;
+
+	if (userio->head == userio->tail)
+		dev_warn(userio_misc.this_device,
+			 "Buffer overflowed, userio client isn't keeping up");
+
+	spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+	wake_up_interruptible(&userio->waitq);
+
+	return 0;
+}
+
+static int userio_char_open(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio;
+
+	userio = kzalloc(sizeof(struct userio_device), GFP_KERNEL);
+	if (!userio)
+		return -ENOMEM;
+
+	mutex_init(&userio->lock);
+	spin_lock_init(&userio->buf_lock);
+	init_waitqueue_head(&userio->waitq);
+
+	userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
+	if (!userio->serio) {
+		kfree(userio);
+		return -ENOMEM;
+	}
+
+	userio->serio->write = userio_device_write;
+	userio->serio->port_data = userio;
+
+	file->private_data = userio;
+
+	return 0;
+}
+
+static int userio_char_release(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio = file->private_data;
+
+	/* Don't free the serio port here, serio_unregister_port() does
+	 * this for us */
+	if (userio->running)
+		serio_unregister_port(userio->serio);
+	else
+		kfree(userio->serio);
+
+	kfree(userio);
+
+	return 0;
+}
+
+static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
+				size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	int ret;
+	size_t nonwrap_len, copylen;
+	unsigned char buf[USERIO_BUFSIZE];
+	unsigned long flags;
+
+	if (!count)
+		return 0;
+
+	ret = mutex_lock_interruptible(&userio->lock);
+	if (ret)
+		return ret;
+
+	/*
+	 * By the time we get here, the data that was waiting might have been
+	 * taken by another thread. Grab the mutex and check if there's still
+	 * any data waiting, otherwise repeat this process until we have data
+	 * (unless the file descriptor is non-blocking of course)
+	 */
+	for (;;) {
+		spin_lock_irqsave(&userio->buf_lock, flags);
+
+		if (userio->head != userio->tail)
+			break;
+
+		spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+		if (file->f_flags & O_NONBLOCK) {
+			ret = -EAGAIN;
+			goto out;
+		}
+
+		ret = wait_event_interruptible(userio->waitq,
+					       userio->head != userio->tail);
+		if (ret)
+			goto out;
+	}
+
+	nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
+				      USERIO_BUFSIZE);
+	copylen = min(nonwrap_len, count);
+	memcpy(&buf, &userio->buf[userio->tail], copylen);
+
+	userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
+
+	spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+	if (copy_to_user(user_buffer, &buf, copylen))
+		ret = -EFAULT;
+	else
+		ret = copylen;
+
+out:
+	mutex_unlock(&userio->lock);
+	return ret;
+}
+
+static ssize_t userio_char_write(struct file *file, const char __user *buffer,
+				 size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	struct userio_cmd cmd;
+	int ret;
+
+	if (count < sizeof(cmd))
+		return -EINVAL;
+
+	if (copy_from_user(&cmd, buffer, sizeof(cmd)))
+		return -EFAULT;
+
+	ret = mutex_lock_interruptible(&userio->lock);
+	if (ret)
+		return ret;
+
+	switch (cmd.type) {
+	case USERIO_CMD_REGISTER:
+		if (!userio->serio->id.type) {
+			dev_warn(userio_misc.this_device,
+				 "No port type given on /dev/userio\n");
+
+			ret = -EINVAL;
+			goto out;
+		}
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Begin command sent, but we're already running\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->running = true;
+		serio_register_port(userio->serio);
+		break;
+
+	case USERIO_CMD_SET_PORT_TYPE:
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Can't change port type on an already running userio instance\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->serio->id.type = cmd.data;
+		break;
+
+	case USERIO_CMD_SEND_INTERRUPT:
+		if (!userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "The device must be registered before sending interrupts\n");
+
+			ret = -ENODEV;
+			goto out;
+		}
+
+		serio_interrupt(userio->serio, cmd.data, 0);
+		break;
+
+	default:
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	ret = sizeof(cmd);
+
+out:
+	mutex_unlock(&userio->lock);
+	return ret;
+}
+
+static unsigned int userio_char_poll(struct file *file, poll_table *wait)
+{
+	struct userio_device *userio = file->private_data;
+
+	poll_wait(file, &userio->waitq, wait);
+
+	if (userio->head != userio->tail)
+		return POLLIN | POLLRDNORM;
+
+	return 0;
+}
+
+static const struct file_operations userio_fops = {
+	.owner		= THIS_MODULE,
+	.open		= userio_char_open,
+	.release	= userio_char_release,
+	.read		= userio_char_read,
+	.write		= userio_char_write,
+	.poll		= userio_char_poll,
+	.llseek		= no_llseek,
+};
+
+static struct miscdevice userio_misc = {
+	.fops	= &userio_fops,
+	.minor	= USERIO_MINOR,
+	.name	= USERIO_NAME,
+};
+
+MODULE_ALIAS_MISCDEV(USERIO_MINOR);
+MODULE_ALIAS("devname:" USERIO_NAME);
+
+MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
+MODULE_DESCRIPTION("Virtual Serio Device Support");
+MODULE_LICENSE("GPL");
+
+module_driver(userio_misc, misc_register, misc_deregister);
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 81f6e42..5430374 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -49,6 +49,7 @@
 #define LOOP_CTRL_MINOR		237
 #define VHOST_NET_MINOR		238
 #define UHID_MINOR		239
+#define USERIO_MINOR		240
 #define MISC_DYNAMIC_MINOR	255
 
 struct device;
diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
new file mode 100644
index 0000000..37d147f
--- /dev/null
+++ b/include/uapi/linux/userio.h
@@ -0,0 +1,44 @@
+/*
+ * userio: virtual serio device support
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ *
+ * This is the public header used for user-space communication with the userio
+ * driver. __attribute__((__packed__)) is used for all structs to keep ABI
+ * compatibility between all architectures.
+ */
+
+#ifndef _USERIO_H
+#define _USERIO_H
+
+#include <linux/types.h>
+
+enum userio_cmd_type {
+	USERIO_CMD_REGISTER = 0,
+	USERIO_CMD_SET_PORT_TYPE = 1,
+	USERIO_CMD_SEND_INTERRUPT = 2
+};
+
+/*
+ * userio Commands
+ * All commands sent to /dev/userio are encoded using this structure. The type
+ * field should contain a USERIO_CMD* value that indicates what kind of command
+ * is being sent to userio. The data field should contain the accompanying
+ * argument for the command, if there is one.
+ */
+struct userio_cmd {
+	__u8 type;
+	__u8 data;
+} __attribute__((__packed__));
+
+#endif /* !_USERIO_H */
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6.1 1/1] Input: Add userio module
  2015-09-23 17:49   ` [PATCH v6 1/1] " cpaul
@ 2015-09-23 17:54     ` cpaul
  2015-10-02 17:37       ` Dmitry Torokhov
  0 siblings, 1 reply; 19+ messages in thread
From: cpaul @ 2015-09-23 17:54 UTC (permalink / raw)
  To: David Herrmann, Dmitry Torokhov
  Cc: linux-kernel, linux-input, linux-api, Andrew Morton,
	Mauro Carvalho Chehab, Greg KH, Arnd Bergmann, Joe Perches,
	Jiri Slaby, Vishnu Patekar, Sebastian Ott, Benjamin Tissoires,
	Hans de Goede, linux-doc

From: Stephen Chandler Paul <cpaul@redhat.com>

Debugging input devices, specifically laptop touchpads, can be tricky
without having the physical device handy. Here we try to remedy that
with userio. This module allows an application to connect to a character
device provided by the kernel, and emulate any serio device. In
combination with userspace programs that can record PS/2 devices and
replay them through the /dev/userio device, this allows developers to
debug driver issues on the PS/2 level with devices simply by requesting
a recording from the user experiencing the issue without having to have
the physical hardware in front of them.

Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
---
 Documentation/input/userio.txt |  70 ++++++++++
 MAINTAINERS                    |   6 +
 drivers/input/serio/Kconfig    |  11 ++
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/userio.c   | 289 +++++++++++++++++++++++++++++++++++++++++
 include/linux/miscdevice.h     |   1 +
 include/uapi/linux/userio.h    |  44 +++++++
 7 files changed, 422 insertions(+)
 create mode 100644 Documentation/input/userio.txt
 create mode 100644 drivers/input/serio/userio.c
 create mode 100644 include/uapi/linux/userio.h

				    Changes
* Fixed the files listed in MAINTAINERS, surprised no one (myself included)
  noticed this earlier.

diff --git a/Documentation/input/userio.txt b/Documentation/input/userio.txt
new file mode 100644
index 0000000..0880c0f
--- /dev/null
+++ b/Documentation/input/userio.txt
@@ -0,0 +1,70 @@
+			      The userio Protocol
+	     (c) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+			      Sponsored by Red Hat
+--------------------------------------------------------------------------------
+
+1. Introduction
+~~~~~~~~~~~~~~~
+  This module is intended to try to make the lives of input driver developers
+easier by allowing them to test various serio devices (mainly the various
+touchpads found on laptops) without having to have the physical device in front
+of them. userio accomplishes this by allowing any privileged userspace program
+to directly interact with the kernel's serio driver and control a virtual serio
+port from there.
+
+2. Usage overview
+~~~~~~~~~~~~~~~~~
+  In order to interact with the userio kernel module, one simply opens the
+/dev/userio character device in their applications. Commands are sent to the
+kernel module by writing to the device, and any data received from the serio
+driver is read as-is from the /dev/userio device. All of the structures and
+macros you need to interact with the device are defined in <linux/userio.h> and
+<linux/serio.h>.
+
+3. Command Structure
+~~~~~~~~~~~~~~~~~~~~
+  The struct used for sending commands to /dev/userio is as follows:
+
+	struct userio_cmd {
+		__u8 type;
+		__u8 data;
+	};
+
+  "type" describes the type of command that is being sent. This can be any one
+of the USERIO_CMD macros defined in <linux/userio.h>. "data" is the argument
+that goes along with the command. In the event that the command doesn't have an
+argument, this field can be left untouched and will be ignored by the kernel.
+Each command should be sent by writing the struct directly to the character
+device. In the event that the command you send is invalid, an error will be
+returned by the character device and a more descriptive error will be printed
+to the kernel log. Only one command can be sent at a time, any additional data
+written to the character device after the initial command will be ignored.
+  To close the virtual serio port, just close /dev/userio.
+
+4. Commands
+~~~~~~~~~~~
+
+4.1 USERIO_CMD_REGISTER
+~~~~~~~~~~~~~~~~~~~~~~~
+  Registers the port with the serio driver and begins transmitting data back and
+forth. Registration can only be performed once a port type is set with
+USERIO_CMD_SET_PORT_TYPE. Has no argument.
+
+4.2 USERIO_CMD_SET_PORT_TYPE
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sets the type of port we're emulating, where "data" is the port type being
+set. Can be any of the macros from <linux/serio.h>. For example: SERIO_8042
+would set the port type to be a normal PS/2 port.
+
+4.3 USERIO_CMD_SEND_INTERRUPT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sends an interrupt through the virtual serio port to the serio driver, where
+"data" is the interrupt data being sent.
+
+5. Userspace tools
+~~~~~~~~~~~~~~~~~~
+  The userio userspace tools are able to record PS/2 devices using some of the
+debugging information from i8042, and play back the devices on /dev/userio. The
+latest version of these tools can be found at:
+
+	https://github.com/Lyude/ps2emu
diff --git a/MAINTAINERS b/MAINTAINERS
index 274f854..7613654 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11086,6 +11086,12 @@ S:	Maintained
 F:	drivers/media/v4l2-core/videobuf2-*
 F:	include/media/videobuf2-*
 
+VIRTUAL SERIO DEVICE DRIVER
+M:	Stephen Chandler Paul <thatslyude@gmail.com>
+S:	Maintained
+F:	drivers/input/serio/userio.c
+F:	include/uapi/linux/userio.h
+
 VIRTIO CONSOLE DRIVER
 M:	Amit Shah <amit.shah@redhat.com>
 L:	virtualization@lists.linux-foundation.org
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 200841b..22b8b58 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,4 +292,15 @@ config SERIO_SUN4I_PS2
 	  To compile this driver as a module, choose M here: the
 	  module will be called sun4i-ps2.
 
+config USERIO
+	tristate "Virtual serio device support"
+	help
+	  Say Y here if you want to emulate serio devices using the userio
+	  kernel module.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called userio.
+
+	  If you are unsure, say N.
+
 endif
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index c600089..2374ef9 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_SERIO_APBPS2)	+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)	+= olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)	+= hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)	+= sun4i-ps2.o
+obj-$(CONFIG_USERIO)		+= userio.o
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
new file mode 100644
index 0000000..f0e1bd4
--- /dev/null
+++ b/drivers/input/serio/userio.c
@@ -0,0 +1,289 @@
+/*
+ * userio kernel serio device emulation module
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ */
+#include <linux/circ_buf.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <linux/sched.h>
+#include <linux/poll.h>
+#include <uapi/linux/userio.h>
+
+#define USERIO_NAME "userio"
+#define USERIO_BUFSIZE 16
+
+static struct miscdevice userio_misc;
+
+struct userio_device {
+	struct serio *serio;
+	struct mutex lock;
+
+	bool running;
+
+	u8 head;
+	u8 tail;
+
+	spinlock_t buf_lock;
+	unsigned char buf[USERIO_BUFSIZE];
+
+	wait_queue_head_t waitq;
+};
+
+/**
+ * userio_device_write - Write data from serio to a userio device in userspace
+ * @id: The serio port for the userio device
+ * @val: The data to write to the device
+ */
+static int userio_device_write(struct serio *id, unsigned char val)
+{
+	struct userio_device *userio = id->port_data;
+	unsigned long flags;
+
+	if (!userio)
+		return -1;
+
+	spin_lock_irqsave(&userio->buf_lock, flags);
+
+	userio->buf[userio->head] = val;
+	userio->head = (userio->head + 1) % USERIO_BUFSIZE;
+
+	if (userio->head == userio->tail)
+		dev_warn(userio_misc.this_device,
+			 "Buffer overflowed, userio client isn't keeping up");
+
+	spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+	wake_up_interruptible(&userio->waitq);
+
+	return 0;
+}
+
+static int userio_char_open(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio;
+
+	userio = kzalloc(sizeof(struct userio_device), GFP_KERNEL);
+	if (!userio)
+		return -ENOMEM;
+
+	mutex_init(&userio->lock);
+	spin_lock_init(&userio->buf_lock);
+	init_waitqueue_head(&userio->waitq);
+
+	userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
+	if (!userio->serio) {
+		kfree(userio);
+		return -ENOMEM;
+	}
+
+	userio->serio->write = userio_device_write;
+	userio->serio->port_data = userio;
+
+	file->private_data = userio;
+
+	return 0;
+}
+
+static int userio_char_release(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio = file->private_data;
+
+	/* Don't free the serio port here, serio_unregister_port() does
+	 * this for us */
+	if (userio->running)
+		serio_unregister_port(userio->serio);
+	else
+		kfree(userio->serio);
+
+	kfree(userio);
+
+	return 0;
+}
+
+static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
+				size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	int ret;
+	size_t nonwrap_len, copylen;
+	unsigned char buf[USERIO_BUFSIZE];
+	unsigned long flags;
+
+	if (!count)
+		return 0;
+
+	ret = mutex_lock_interruptible(&userio->lock);
+	if (ret)
+		return ret;
+
+	/*
+	 * By the time we get here, the data that was waiting might have been
+	 * taken by another thread. Grab the mutex and check if there's still
+	 * any data waiting, otherwise repeat this process until we have data
+	 * (unless the file descriptor is non-blocking of course)
+	 */
+	for (;;) {
+		spin_lock_irqsave(&userio->buf_lock, flags);
+
+		if (userio->head != userio->tail)
+			break;
+
+		spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+		if (file->f_flags & O_NONBLOCK) {
+			ret = -EAGAIN;
+			goto out;
+		}
+
+		ret = wait_event_interruptible(userio->waitq,
+					       userio->head != userio->tail);
+		if (ret)
+			goto out;
+	}
+
+	nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
+				      USERIO_BUFSIZE);
+	copylen = min(nonwrap_len, count);
+	memcpy(&buf, &userio->buf[userio->tail], copylen);
+
+	userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
+
+	spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+	if (copy_to_user(user_buffer, &buf, copylen))
+		ret = -EFAULT;
+	else
+		ret = copylen;
+
+out:
+	mutex_unlock(&userio->lock);
+	return ret;
+}
+
+static ssize_t userio_char_write(struct file *file, const char __user *buffer,
+				 size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	struct userio_cmd cmd;
+	int ret;
+
+	if (count < sizeof(cmd))
+		return -EINVAL;
+
+	if (copy_from_user(&cmd, buffer, sizeof(cmd)))
+		return -EFAULT;
+
+	ret = mutex_lock_interruptible(&userio->lock);
+	if (ret)
+		return ret;
+
+	switch (cmd.type) {
+	case USERIO_CMD_REGISTER:
+		if (!userio->serio->id.type) {
+			dev_warn(userio_misc.this_device,
+				 "No port type given on /dev/userio\n");
+
+			ret = -EINVAL;
+			goto out;
+		}
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Begin command sent, but we're already running\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->running = true;
+		serio_register_port(userio->serio);
+		break;
+
+	case USERIO_CMD_SET_PORT_TYPE:
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Can't change port type on an already running userio instance\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->serio->id.type = cmd.data;
+		break;
+
+	case USERIO_CMD_SEND_INTERRUPT:
+		if (!userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "The device must be registered before sending interrupts\n");
+
+			ret = -ENODEV;
+			goto out;
+		}
+
+		serio_interrupt(userio->serio, cmd.data, 0);
+		break;
+
+	default:
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	ret = sizeof(cmd);
+
+out:
+	mutex_unlock(&userio->lock);
+	return ret;
+}
+
+static unsigned int userio_char_poll(struct file *file, poll_table *wait)
+{
+	struct userio_device *userio = file->private_data;
+
+	poll_wait(file, &userio->waitq, wait);
+
+	if (userio->head != userio->tail)
+		return POLLIN | POLLRDNORM;
+
+	return 0;
+}
+
+static const struct file_operations userio_fops = {
+	.owner		= THIS_MODULE,
+	.open		= userio_char_open,
+	.release	= userio_char_release,
+	.read		= userio_char_read,
+	.write		= userio_char_write,
+	.poll		= userio_char_poll,
+	.llseek		= no_llseek,
+};
+
+static struct miscdevice userio_misc = {
+	.fops	= &userio_fops,
+	.minor	= USERIO_MINOR,
+	.name	= USERIO_NAME,
+};
+
+MODULE_ALIAS_MISCDEV(USERIO_MINOR);
+MODULE_ALIAS("devname:" USERIO_NAME);
+
+MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
+MODULE_DESCRIPTION("Virtual Serio Device Support");
+MODULE_LICENSE("GPL");
+
+module_driver(userio_misc, misc_register, misc_deregister);
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 81f6e42..5430374 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -49,6 +49,7 @@
 #define LOOP_CTRL_MINOR		237
 #define VHOST_NET_MINOR		238
 #define UHID_MINOR		239
+#define USERIO_MINOR		240
 #define MISC_DYNAMIC_MINOR	255
 
 struct device;
diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
new file mode 100644
index 0000000..37d147f
--- /dev/null
+++ b/include/uapi/linux/userio.h
@@ -0,0 +1,44 @@
+/*
+ * userio: virtual serio device support
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ *
+ * This is the public header used for user-space communication with the userio
+ * driver. __attribute__((__packed__)) is used for all structs to keep ABI
+ * compatibility between all architectures.
+ */
+
+#ifndef _USERIO_H
+#define _USERIO_H
+
+#include <linux/types.h>
+
+enum userio_cmd_type {
+	USERIO_CMD_REGISTER = 0,
+	USERIO_CMD_SET_PORT_TYPE = 1,
+	USERIO_CMD_SEND_INTERRUPT = 2
+};
+
+/*
+ * userio Commands
+ * All commands sent to /dev/userio are encoded using this structure. The type
+ * field should contain a USERIO_CMD* value that indicates what kind of command
+ * is being sent to userio. The data field should contain the accompanying
+ * argument for the command, if there is one.
+ */
+struct userio_cmd {
+	__u8 type;
+	__u8 data;
+} __attribute__((__packed__));
+
+#endif /* !_USERIO_H */
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v6.1 1/1] Input: Add userio module
  2015-09-23 17:54     ` [PATCH v6.1 " cpaul
@ 2015-10-02 17:37       ` Dmitry Torokhov
  2015-10-05 14:06         ` Stephen Chandler Paul
  2015-10-05 15:55         ` [PATCH v7] " cpaul
  0 siblings, 2 replies; 19+ messages in thread
From: Dmitry Torokhov @ 2015-10-02 17:37 UTC (permalink / raw)
  To: cpaul
  Cc: David Herrmann, linux-kernel, linux-input, linux-api,
	Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
	Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott,
	Benjamin Tissoires, Hans de Goede, linux-doc

On Wed, Sep 23, 2015 at 01:54:59PM -0400, cpaul@redhat.com wrote:
> +static int userio_device_write(struct serio *id, unsigned char val)
> +{
> +	struct userio_device *userio = id->port_data;
> +	unsigned long flags;
> +
> +	if (!userio)
> +		return -1;

I still have the same question: how can this happen? Where do we reset
port data to NULL? What happens if we set to NULL after checking?

> +
> +	spin_lock_irqsave(&userio->buf_lock, flags);
> +
> +	userio->buf[userio->head] = val;
> +	userio->head = (userio->head + 1) % USERIO_BUFSIZE;
> +
> +	if (userio->head == userio->tail)
> +		dev_warn(userio_misc.this_device,
> +			 "Buffer overflowed, userio client isn't keeping up");
> +
> +	spin_unlock_irqrestore(&userio->buf_lock, flags);
> +
> +	wake_up_interruptible(&userio->waitq);
> +
> +	return 0;
> +}

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6.1 1/1] Input: Add userio module
  2015-10-02 17:37       ` Dmitry Torokhov
@ 2015-10-05 14:06         ` Stephen Chandler Paul
  2015-10-23 20:47           ` [PATCH v6.2 " cpaul
  2015-10-05 15:55         ` [PATCH v7] " cpaul
  1 sibling, 1 reply; 19+ messages in thread
From: Stephen Chandler Paul @ 2015-10-05 14:06 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: David Herrmann, linux-kernel, linux-input, linux-api,
	Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
	Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott,
	Benjamin Tissoires, Hans de Goede, linux-doc

On Fri, 2015-10-02 at 10:37 -0700, Dmitry Torokhov wrote:
> On Wed, Sep 23, 2015 at 01:54:59PM -0400, cpaul@redhat.com wrote:
> > +static int userio_device_write(struct serio *id, unsigned char
> > val)
> > +{
> > +	struct userio_device *userio = id->port_data;
> > +	unsigned long flags;
> > +
> > +	if (!userio)
> > +		return -1;
> 
> I still have the same question: how can this happen? Where do we
> reset
> port data to NULL? What happens if we set to NULL after checking?
> 

Actually David pointed out this line was not needed, somehow it still
got left in the patch. I'll send an updated version in just a minute
with the changes.

Cheers,
	Lyude
> > +
> > +	spin_lock_irqsave(&userio->buf_lock, flags);
> > +
> > +	userio->buf[userio->head] = val;
> > +	userio->head = (userio->head + 1) % USERIO_BUFSIZE;
> > +
> > +	if (userio->head == userio->tail)
> > +		dev_warn(userio_misc.this_device,
> > +			 "Buffer overflowed, userio client isn't
> > keeping up");
> > +
> > +	spin_unlock_irqrestore(&userio->buf_lock, flags);
> > +
> > +	wake_up_interruptible(&userio->waitq);
> > +
> > +	return 0;
> > +}
> 
> Thanks.
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v7] Input: Add userio module
  2015-10-02 17:37       ` Dmitry Torokhov
  2015-10-05 14:06         ` Stephen Chandler Paul
@ 2015-10-05 15:55         ` cpaul
  2015-10-08 17:20           ` David Herrmann
  1 sibling, 1 reply; 19+ messages in thread
From: cpaul @ 2015-10-05 15:55 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: David Herrmann, linux-kernel, linux-api, Andrew Morton,
	Mauro Carvalho Chehab, Greg KH, Arnd Bergmann, Joe Perches,
	Jiri Slaby, Vishnu Patekar, Sebastian Ott, Benjamin Tissoires,
	Hans de Goede, linux-doc

From: Stephen Chandler Paul <cpaul@redhat.com>

Debugging input devices, specifically laptop touchpads, can be tricky
without having the physical device handy. Here we try to remedy that
with userio. This module allows an application to connect to a character
device provided by the kernel, and emulate any serio device. In
combination with userspace programs that can record PS/2 devices and
replay them through the /dev/userio device, this allows developers to
debug driver issues on the PS/2 level with devices simply by requesting
a recording from the user experiencing the issue without having to have
the physical hardware in front of them.

Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
---
				    Changes
* Removed useless if (!userio) check in userio_device_write()

 Documentation/input/userio.txt |  70 ++++++++++
 MAINTAINERS                    |   6 +
 drivers/input/serio/Kconfig    |  11 ++
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/userio.c   | 286 +++++++++++++++++++++++++++++++++++++++++
 include/linux/miscdevice.h     |   1 +
 include/uapi/linux/userio.h    |  44 +++++++
 7 files changed, 419 insertions(+)
 create mode 100644 Documentation/input/userio.txt
 create mode 100644 drivers/input/serio/userio.c
 create mode 100644 include/uapi/linux/userio.h

diff --git a/Documentation/input/userio.txt b/Documentation/input/userio.txt
new file mode 100644
index 0000000..0880c0f
--- /dev/null
+++ b/Documentation/input/userio.txt
@@ -0,0 +1,70 @@
+			      The userio Protocol
+	     (c) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+			      Sponsored by Red Hat
+--------------------------------------------------------------------------------
+
+1. Introduction
+~~~~~~~~~~~~~~~
+  This module is intended to try to make the lives of input driver developers
+easier by allowing them to test various serio devices (mainly the various
+touchpads found on laptops) without having to have the physical device in front
+of them. userio accomplishes this by allowing any privileged userspace program
+to directly interact with the kernel's serio driver and control a virtual serio
+port from there.
+
+2. Usage overview
+~~~~~~~~~~~~~~~~~
+  In order to interact with the userio kernel module, one simply opens the
+/dev/userio character device in their applications. Commands are sent to the
+kernel module by writing to the device, and any data received from the serio
+driver is read as-is from the /dev/userio device. All of the structures and
+macros you need to interact with the device are defined in <linux/userio.h> and
+<linux/serio.h>.
+
+3. Command Structure
+~~~~~~~~~~~~~~~~~~~~
+  The struct used for sending commands to /dev/userio is as follows:
+
+	struct userio_cmd {
+		__u8 type;
+		__u8 data;
+	};
+
+  "type" describes the type of command that is being sent. This can be any one
+of the USERIO_CMD macros defined in <linux/userio.h>. "data" is the argument
+that goes along with the command. In the event that the command doesn't have an
+argument, this field can be left untouched and will be ignored by the kernel.
+Each command should be sent by writing the struct directly to the character
+device. In the event that the command you send is invalid, an error will be
+returned by the character device and a more descriptive error will be printed
+to the kernel log. Only one command can be sent at a time, any additional data
+written to the character device after the initial command will be ignored.
+  To close the virtual serio port, just close /dev/userio.
+
+4. Commands
+~~~~~~~~~~~
+
+4.1 USERIO_CMD_REGISTER
+~~~~~~~~~~~~~~~~~~~~~~~
+  Registers the port with the serio driver and begins transmitting data back and
+forth. Registration can only be performed once a port type is set with
+USERIO_CMD_SET_PORT_TYPE. Has no argument.
+
+4.2 USERIO_CMD_SET_PORT_TYPE
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sets the type of port we're emulating, where "data" is the port type being
+set. Can be any of the macros from <linux/serio.h>. For example: SERIO_8042
+would set the port type to be a normal PS/2 port.
+
+4.3 USERIO_CMD_SEND_INTERRUPT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sends an interrupt through the virtual serio port to the serio driver, where
+"data" is the interrupt data being sent.
+
+5. Userspace tools
+~~~~~~~~~~~~~~~~~~
+  The userio userspace tools are able to record PS/2 devices using some of the
+debugging information from i8042, and play back the devices on /dev/userio. The
+latest version of these tools can be found at:
+
+	https://github.com/Lyude/ps2emu
diff --git a/MAINTAINERS b/MAINTAINERS
index 797236b..ba59bd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11100,6 +11100,12 @@ S:	Maintained
 F:	drivers/media/v4l2-core/videobuf2-*
 F:	include/media/videobuf2-*
 
+VIRTUAL SERIO DEVICE DRIVER
+M:	Stephen Chandler Paul <thatslyude@gmail.com>
+S:	Maintained
+F:	drivers/input/serio/userio.c
+F:	include/uapi/linux/userio.h
+
 VIRTIO CONSOLE DRIVER
 M:	Amit Shah <amit.shah@redhat.com>
 L:	virtualization@lists.linux-foundation.org
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 200841b..22b8b58 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,4 +292,15 @@ config SERIO_SUN4I_PS2
 	  To compile this driver as a module, choose M here: the
 	  module will be called sun4i-ps2.
 
+config USERIO
+	tristate "Virtual serio device support"
+	help
+	  Say Y here if you want to emulate serio devices using the userio
+	  kernel module.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called userio.
+
+	  If you are unsure, say N.
+
 endif
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index c600089..2374ef9 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_SERIO_APBPS2)	+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)	+= olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)	+= hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)	+= sun4i-ps2.o
+obj-$(CONFIG_USERIO)		+= userio.o
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
new file mode 100644
index 0000000..0ada7cf
--- /dev/null
+++ b/drivers/input/serio/userio.c
@@ -0,0 +1,286 @@
+/*
+ * userio kernel serio device emulation module
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ */
+#include <linux/circ_buf.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <linux/sched.h>
+#include <linux/poll.h>
+#include <uapi/linux/userio.h>
+
+#define USERIO_NAME "userio"
+#define USERIO_BUFSIZE 16
+
+static struct miscdevice userio_misc;
+
+struct userio_device {
+	struct serio *serio;
+	struct mutex lock;
+
+	bool running;
+
+	u8 head;
+	u8 tail;
+
+	spinlock_t buf_lock;
+	unsigned char buf[USERIO_BUFSIZE];
+
+	wait_queue_head_t waitq;
+};
+
+/**
+ * userio_device_write - Write data from serio to a userio device in userspace
+ * @id: The serio port for the userio device
+ * @val: The data to write to the device
+ */
+static int userio_device_write(struct serio *id, unsigned char val)
+{
+	struct userio_device *userio = id->port_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&userio->buf_lock, flags);
+
+	userio->buf[userio->head] = val;
+	userio->head = (userio->head + 1) % USERIO_BUFSIZE;
+
+	if (userio->head == userio->tail)
+		dev_warn(userio_misc.this_device,
+			 "Buffer overflowed, userio client isn't keeping up");
+
+	spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+	wake_up_interruptible(&userio->waitq);
+
+	return 0;
+}
+
+static int userio_char_open(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio;
+
+	userio = kzalloc(sizeof(struct userio_device), GFP_KERNEL);
+	if (!userio)
+		return -ENOMEM;
+
+	mutex_init(&userio->lock);
+	spin_lock_init(&userio->buf_lock);
+	init_waitqueue_head(&userio->waitq);
+
+	userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
+	if (!userio->serio) {
+		kfree(userio);
+		return -ENOMEM;
+	}
+
+	userio->serio->write = userio_device_write;
+	userio->serio->port_data = userio;
+
+	file->private_data = userio;
+
+	return 0;
+}
+
+static int userio_char_release(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio = file->private_data;
+
+	/* Don't free the serio port here, serio_unregister_port() does
+	 * this for us */
+	if (userio->running)
+		serio_unregister_port(userio->serio);
+	else
+		kfree(userio->serio);
+
+	kfree(userio);
+
+	return 0;
+}
+
+static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
+				size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	int ret;
+	size_t nonwrap_len, copylen;
+	unsigned char buf[USERIO_BUFSIZE];
+	unsigned long flags;
+
+	if (!count)
+		return 0;
+
+	ret = mutex_lock_interruptible(&userio->lock);
+	if (ret)
+		return ret;
+
+	/*
+	 * By the time we get here, the data that was waiting might have been
+	 * taken by another thread. Grab the mutex and check if there's still
+	 * any data waiting, otherwise repeat this process until we have data
+	 * (unless the file descriptor is non-blocking of course)
+	 */
+	for (;;) {
+		spin_lock_irqsave(&userio->buf_lock, flags);
+
+		if (userio->head != userio->tail)
+			break;
+
+		spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+		if (file->f_flags & O_NONBLOCK) {
+			ret = -EAGAIN;
+			goto out;
+		}
+
+		ret = wait_event_interruptible(userio->waitq,
+					       userio->head != userio->tail);
+		if (ret)
+			goto out;
+	}
+
+	nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
+				      USERIO_BUFSIZE);
+	copylen = min(nonwrap_len, count);
+	memcpy(&buf, &userio->buf[userio->tail], copylen);
+
+	userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
+
+	spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+	if (copy_to_user(user_buffer, &buf, copylen))
+		ret = -EFAULT;
+	else
+		ret = copylen;
+
+out:
+	mutex_unlock(&userio->lock);
+	return ret;
+}
+
+static ssize_t userio_char_write(struct file *file, const char __user *buffer,
+				 size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	struct userio_cmd cmd;
+	int ret;
+
+	if (count < sizeof(cmd))
+		return -EINVAL;
+
+	if (copy_from_user(&cmd, buffer, sizeof(cmd)))
+		return -EFAULT;
+
+	ret = mutex_lock_interruptible(&userio->lock);
+	if (ret)
+		return ret;
+
+	switch (cmd.type) {
+	case USERIO_CMD_REGISTER:
+		if (!userio->serio->id.type) {
+			dev_warn(userio_misc.this_device,
+				 "No port type given on /dev/userio\n");
+
+			ret = -EINVAL;
+			goto out;
+		}
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Begin command sent, but we're already running\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->running = true;
+		serio_register_port(userio->serio);
+		break;
+
+	case USERIO_CMD_SET_PORT_TYPE:
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Can't change port type on an already running userio instance\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->serio->id.type = cmd.data;
+		break;
+
+	case USERIO_CMD_SEND_INTERRUPT:
+		if (!userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "The device must be registered before sending interrupts\n");
+
+			ret = -ENODEV;
+			goto out;
+		}
+
+		serio_interrupt(userio->serio, cmd.data, 0);
+		break;
+
+	default:
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	ret = sizeof(cmd);
+
+out:
+	mutex_unlock(&userio->lock);
+	return ret;
+}
+
+static unsigned int userio_char_poll(struct file *file, poll_table *wait)
+{
+	struct userio_device *userio = file->private_data;
+
+	poll_wait(file, &userio->waitq, wait);
+
+	if (userio->head != userio->tail)
+		return POLLIN | POLLRDNORM;
+
+	return 0;
+}
+
+static const struct file_operations userio_fops = {
+	.owner		= THIS_MODULE,
+	.open		= userio_char_open,
+	.release	= userio_char_release,
+	.read		= userio_char_read,
+	.write		= userio_char_write,
+	.poll		= userio_char_poll,
+	.llseek		= no_llseek,
+};
+
+static struct miscdevice userio_misc = {
+	.fops	= &userio_fops,
+	.minor	= USERIO_MINOR,
+	.name	= USERIO_NAME,
+};
+
+MODULE_ALIAS_MISCDEV(USERIO_MINOR);
+MODULE_ALIAS("devname:" USERIO_NAME);
+
+MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
+MODULE_DESCRIPTION("Virtual Serio Device Support");
+MODULE_LICENSE("GPL");
+
+module_driver(userio_misc, misc_register, misc_deregister);
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 81f6e42..5430374 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -49,6 +49,7 @@
 #define LOOP_CTRL_MINOR		237
 #define VHOST_NET_MINOR		238
 #define UHID_MINOR		239
+#define USERIO_MINOR		240
 #define MISC_DYNAMIC_MINOR	255
 
 struct device;
diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
new file mode 100644
index 0000000..37d147f
--- /dev/null
+++ b/include/uapi/linux/userio.h
@@ -0,0 +1,44 @@
+/*
+ * userio: virtual serio device support
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ *
+ * This is the public header used for user-space communication with the userio
+ * driver. __attribute__((__packed__)) is used for all structs to keep ABI
+ * compatibility between all architectures.
+ */
+
+#ifndef _USERIO_H
+#define _USERIO_H
+
+#include <linux/types.h>
+
+enum userio_cmd_type {
+	USERIO_CMD_REGISTER = 0,
+	USERIO_CMD_SET_PORT_TYPE = 1,
+	USERIO_CMD_SEND_INTERRUPT = 2
+};
+
+/*
+ * userio Commands
+ * All commands sent to /dev/userio are encoded using this structure. The type
+ * field should contain a USERIO_CMD* value that indicates what kind of command
+ * is being sent to userio. The data field should contain the accompanying
+ * argument for the command, if there is one.
+ */
+struct userio_cmd {
+	__u8 type;
+	__u8 data;
+} __attribute__((__packed__));
+
+#endif /* !_USERIO_H */
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v7] Input: Add userio module
  2015-10-05 15:55         ` [PATCH v7] " cpaul
@ 2015-10-08 17:20           ` David Herrmann
  2015-10-09 14:30             ` [PATCH v8] " cpaul
  2015-10-24 22:26             ` [PATCH v7] " Dmitry Torokhov
  0 siblings, 2 replies; 19+ messages in thread
From: David Herrmann @ 2015-10-08 17:20 UTC (permalink / raw)
  To: Stephen Chandler Paul
  Cc: Dmitry Torokhov, linux-kernel, Linux API, Andrew Morton,
	Mauro Carvalho Chehab, Greg KH, Arnd Bergmann, Joe Perches,
	Jiri Slaby, Vishnu Patekar, Sebastian Ott, Benjamin Tissoires,
	Hans de Goede, linux-doc

Hi

On Mon, Oct 5, 2015 at 5:55 PM,  <cpaul@redhat.com> wrote:
> From: Stephen Chandler Paul <cpaul@redhat.com>
>
> Debugging input devices, specifically laptop touchpads, can be tricky
> without having the physical device handy. Here we try to remedy that
> with userio. This module allows an application to connect to a character
> device provided by the kernel, and emulate any serio device. In
> combination with userspace programs that can record PS/2 devices and
> replay them through the /dev/userio device, this allows developers to
> debug driver issues on the PS/2 level with devices simply by requesting
> a recording from the user experiencing the issue without having to have
> the physical hardware in front of them.
>
> Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>

Apart from the serio-unregistration, all I have left is bike-shedding.
Regardless of those changes, this is:

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

See below for some details..

> ---
>                                     Changes
> * Removed useless if (!userio) check in userio_device_write()
>
>  Documentation/input/userio.txt |  70 ++++++++++
>  MAINTAINERS                    |   6 +
>  drivers/input/serio/Kconfig    |  11 ++
>  drivers/input/serio/Makefile   |   1 +
>  drivers/input/serio/userio.c   | 286 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/miscdevice.h     |   1 +
>  include/uapi/linux/userio.h    |  44 +++++++
>  7 files changed, 419 insertions(+)
>  create mode 100644 Documentation/input/userio.txt
>  create mode 100644 drivers/input/serio/userio.c
>  create mode 100644 include/uapi/linux/userio.h
>
> diff --git a/Documentation/input/userio.txt b/Documentation/input/userio.txt
> new file mode 100644
> index 0000000..0880c0f
> --- /dev/null
> +++ b/Documentation/input/userio.txt
> @@ -0,0 +1,70 @@
> +                             The userio Protocol
> +            (c) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
> +                             Sponsored by Red Hat
> +--------------------------------------------------------------------------------
> +
> +1. Introduction
> +~~~~~~~~~~~~~~~
> +  This module is intended to try to make the lives of input driver developers
> +easier by allowing them to test various serio devices (mainly the various
> +touchpads found on laptops) without having to have the physical device in front
> +of them. userio accomplishes this by allowing any privileged userspace program
> +to directly interact with the kernel's serio driver and control a virtual serio
> +port from there.
> +
> +2. Usage overview
> +~~~~~~~~~~~~~~~~~
> +  In order to interact with the userio kernel module, one simply opens the
> +/dev/userio character device in their applications. Commands are sent to the
> +kernel module by writing to the device, and any data received from the serio
> +driver is read as-is from the /dev/userio device. All of the structures and
> +macros you need to interact with the device are defined in <linux/userio.h> and
> +<linux/serio.h>.
> +
> +3. Command Structure
> +~~~~~~~~~~~~~~~~~~~~
> +  The struct used for sending commands to /dev/userio is as follows:
> +
> +       struct userio_cmd {
> +               __u8 type;
> +               __u8 data;
> +       };
> +
> +  "type" describes the type of command that is being sent. This can be any one
> +of the USERIO_CMD macros defined in <linux/userio.h>. "data" is the argument
> +that goes along with the command. In the event that the command doesn't have an
> +argument, this field can be left untouched and will be ignored by the kernel.
> +Each command should be sent by writing the struct directly to the character
> +device. In the event that the command you send is invalid, an error will be
> +returned by the character device and a more descriptive error will be printed
> +to the kernel log. Only one command can be sent at a time, any additional data
> +written to the character device after the initial command will be ignored.
> +  To close the virtual serio port, just close /dev/userio.
> +
> +4. Commands
> +~~~~~~~~~~~
> +
> +4.1 USERIO_CMD_REGISTER
> +~~~~~~~~~~~~~~~~~~~~~~~
> +  Registers the port with the serio driver and begins transmitting data back and
> +forth. Registration can only be performed once a port type is set with
> +USERIO_CMD_SET_PORT_TYPE. Has no argument.
> +
> +4.2 USERIO_CMD_SET_PORT_TYPE
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +  Sets the type of port we're emulating, where "data" is the port type being
> +set. Can be any of the macros from <linux/serio.h>. For example: SERIO_8042
> +would set the port type to be a normal PS/2 port.
> +
> +4.3 USERIO_CMD_SEND_INTERRUPT
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +  Sends an interrupt through the virtual serio port to the serio driver, where
> +"data" is the interrupt data being sent.
> +
> +5. Userspace tools
> +~~~~~~~~~~~~~~~~~~
> +  The userio userspace tools are able to record PS/2 devices using some of the
> +debugging information from i8042, and play back the devices on /dev/userio. The
> +latest version of these tools can be found at:
> +
> +       https://github.com/Lyude/ps2emu
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 797236b..ba59bd7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11100,6 +11100,12 @@ S:     Maintained
>  F:     drivers/media/v4l2-core/videobuf2-*
>  F:     include/media/videobuf2-*
>
> +VIRTUAL SERIO DEVICE DRIVER
> +M:     Stephen Chandler Paul <thatslyude@gmail.com>
> +S:     Maintained
> +F:     drivers/input/serio/userio.c
> +F:     include/uapi/linux/userio.h
> +
>  VIRTIO CONSOLE DRIVER
>  M:     Amit Shah <amit.shah@redhat.com>
>  L:     virtualization@lists.linux-foundation.org
> diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
> index 200841b..22b8b58 100644
> --- a/drivers/input/serio/Kconfig
> +++ b/drivers/input/serio/Kconfig
> @@ -292,4 +292,15 @@ config SERIO_SUN4I_PS2
>           To compile this driver as a module, choose M here: the
>           module will be called sun4i-ps2.
>
> +config USERIO
> +       tristate "Virtual serio device support"
> +       help
> +         Say Y here if you want to emulate serio devices using the userio
> +         kernel module.
> +
> +         To compile this driver as a module, choose M here: the module will be
> +         called userio.
> +
> +         If you are unsure, say N.
> +
>  endif
> diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
> index c600089..2374ef9 100644
> --- a/drivers/input/serio/Makefile
> +++ b/drivers/input/serio/Makefile
> @@ -30,3 +30,4 @@ obj-$(CONFIG_SERIO_APBPS2)    += apbps2.o
>  obj-$(CONFIG_SERIO_OLPC_APSP)  += olpc_apsp.o
>  obj-$(CONFIG_HYPERV_KEYBOARD)  += hyperv-keyboard.o
>  obj-$(CONFIG_SERIO_SUN4I_PS2)  += sun4i-ps2.o
> +obj-$(CONFIG_USERIO)           += userio.o
> diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
> new file mode 100644
> index 0000000..0ada7cf
> --- /dev/null
> +++ b/drivers/input/serio/userio.c
> @@ -0,0 +1,286 @@
> +/*
> + * userio kernel serio device emulation module
> + * Copyright (C) 2015 Red Hat
> + * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
> + * details.
> + */

We put an empty line here, usually.

> +#include <linux/circ_buf.h>
> +#include <linux/mutex.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/serio.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/miscdevice.h>
> +#include <linux/sched.h>
> +#include <linux/poll.h>
> +#include <uapi/linux/userio.h>
> +
> +#define USERIO_NAME "userio"
> +#define USERIO_BUFSIZE 16
> +
> +static struct miscdevice userio_misc;
> +
> +struct userio_device {
> +       struct serio *serio;
> +       struct mutex lock;
> +
> +       bool running;
> +
> +       u8 head;
> +       u8 tail;
> +
> +       spinlock_t buf_lock;
> +       unsigned char buf[USERIO_BUFSIZE];
> +
> +       wait_queue_head_t waitq;
> +};
> +
> +/**
> + * userio_device_write - Write data from serio to a userio device in userspace
> + * @id: The serio port for the userio device
> + * @val: The data to write to the device
> + */
> +static int userio_device_write(struct serio *id, unsigned char val)
> +{
> +       struct userio_device *userio = id->port_data;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&userio->buf_lock, flags);
> +
> +       userio->buf[userio->head] = val;
> +       userio->head = (userio->head + 1) % USERIO_BUFSIZE;
> +
> +       if (userio->head == userio->tail)
> +               dev_warn(userio_misc.this_device,
> +                        "Buffer overflowed, userio client isn't keeping up");
> +
> +       spin_unlock_irqrestore(&userio->buf_lock, flags);
> +
> +       wake_up_interruptible(&userio->waitq);
> +
> +       return 0;
> +}
> +
> +static int userio_char_open(struct inode *inode, struct file *file)
> +{
> +       struct userio_device *userio;
> +
> +       userio = kzalloc(sizeof(struct userio_device), GFP_KERNEL);
> +       if (!userio)
> +               return -ENOMEM;
> +
> +       mutex_init(&userio->lock);
> +       spin_lock_init(&userio->buf_lock);
> +       init_waitqueue_head(&userio->waitq);
> +
> +       userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
> +       if (!userio->serio) {
> +               kfree(userio);
> +               return -ENOMEM;
> +       }
> +
> +       userio->serio->write = userio_device_write;
> +       userio->serio->port_data = userio;
> +
> +       file->private_data = userio;
> +
> +       return 0;
> +}
> +
> +static int userio_char_release(struct inode *inode, struct file *file)
> +{
> +       struct userio_device *userio = file->private_data;
> +
> +       /* Don't free the serio port here, serio_unregister_port() does
> +        * this for us */
> +       if (userio->running)
> +               serio_unregister_port(userio->serio);
> +       else
> +               kfree(userio->serio);
> +
> +       kfree(userio);

I'm not very familiar with the serio-subsystem, but is there a
guarantee that .write() will not be called once unregistered? I can
see that it's not scheduled by userio anymore, but maybe there's an
async path that can trigger .write() callbacks. Dunno.. I'll leave
that to Dmitry.

> +
> +       return 0;
> +}
> +
> +static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
> +                               size_t count, loff_t *ppos)
> +{
> +       struct userio_device *userio = file->private_data;
> +       int ret;
> +       size_t nonwrap_len, copylen;
> +       unsigned char buf[USERIO_BUFSIZE];
> +       unsigned long flags;
> +
> +       if (!count)
> +               return 0;
> +
> +       ret = mutex_lock_interruptible(&userio->lock);
> +       if (ret)
> +               return ret;

I cannot see why you need to lock the mutex here. The spin-lock should
be perfectly fine, given that you don't check the device-state,
anyway. Also, given that you don't have a shutdown-commands, except
for close(), I don't see why we'd ever need the mutex-lock here, ever.

> +
> +       /*
> +        * By the time we get here, the data that was waiting might have been
> +        * taken by another thread. Grab the mutex and check if there's still
> +        * any data waiting, otherwise repeat this process until we have data
> +        * (unless the file descriptor is non-blocking of course)
> +        */
> +       for (;;) {
> +               spin_lock_irqsave(&userio->buf_lock, flags);
> +
> +               if (userio->head != userio->tail)
> +                       break;
> +
> +               spin_unlock_irqrestore(&userio->buf_lock, flags);
> +
> +               if (file->f_flags & O_NONBLOCK) {
> +                       ret = -EAGAIN;
> +                       goto out;
> +               }
> +
> +               ret = wait_event_interruptible(userio->waitq,
> +                                              userio->head != userio->tail);
> +               if (ret)
> +                       goto out;
> +       }
> +
> +       nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
> +                                     USERIO_BUFSIZE);
> +       copylen = min(nonwrap_len, count);
> +       memcpy(&buf, &userio->buf[userio->tail], copylen);
> +
> +       userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
> +
> +       spin_unlock_irqrestore(&userio->buf_lock, flags);
> +
> +       if (copy_to_user(user_buffer, &buf, copylen))

No need for '&' before 'buf'.

> +               ret = -EFAULT;
> +       else
> +               ret = copylen;
> +
> +out:
> +       mutex_unlock(&userio->lock);
> +       return ret;
> +}
> +
> +static ssize_t userio_char_write(struct file *file, const char __user *buffer,
> +                                size_t count, loff_t *ppos)
> +{
> +       struct userio_device *userio = file->private_data;
> +       struct userio_cmd cmd;
> +       int ret;
> +
> +       if (count < sizeof(cmd))
> +               return -EINVAL;
> +
> +       if (copy_from_user(&cmd, buffer, sizeof(cmd)))
> +               return -EFAULT;

I know we discussed that before, but I'm still slightly uncomfortable
that we ignore trailing bytes. How about adding a check to each case
below? (see below for an example)

> +
> +       ret = mutex_lock_interruptible(&userio->lock);
> +       if (ret)
> +               return ret;
> +
> +       switch (cmd.type) {
> +       case USERIO_CMD_REGISTER:

                if (count != sizeof(cmd)) {
                        ret = -EINVAL;
                        goto out;
                }

And same for the other 2 commands. This guarantees that each cmd gets
the correct payload size.

> +               if (!userio->serio->id.type) {
> +                       dev_warn(userio_misc.this_device,
> +                                "No port type given on /dev/userio\n");
> +
> +                       ret = -EINVAL;
> +                       goto out;
> +               }
> +               if (userio->running) {
> +                       dev_warn(userio_misc.this_device,
> +                                "Begin command sent, but we're already running\n");
> +
> +                       ret = -EBUSY;
> +                       goto out;
> +               }
> +
> +               userio->running = true;
> +               serio_register_port(userio->serio);
> +               break;
> +
> +       case USERIO_CMD_SET_PORT_TYPE:
> +               if (userio->running) {
> +                       dev_warn(userio_misc.this_device,
> +                                "Can't change port type on an already running userio instance\n");
> +
> +                       ret = -EBUSY;
> +                       goto out;
> +               }
> +
> +               userio->serio->id.type = cmd.data;
> +               break;
> +
> +       case USERIO_CMD_SEND_INTERRUPT:
> +               if (!userio->running) {
> +                       dev_warn(userio_misc.this_device,
> +                                "The device must be registered before sending interrupts\n");
> +
> +                       ret = -ENODEV;
> +                       goto out;
> +               }
> +
> +               serio_interrupt(userio->serio, cmd.data, 0);
> +               break;
> +
> +       default:
> +               ret = -EOPNOTSUPP;
> +               goto out;
> +       }
> +
> +       ret = sizeof(cmd);
> +
> +out:
> +       mutex_unlock(&userio->lock);
> +       return ret;
> +}
> +
> +static unsigned int userio_char_poll(struct file *file, poll_table *wait)
> +{
> +       struct userio_device *userio = file->private_data;
> +
> +       poll_wait(file, &userio->waitq, wait);
> +
> +       if (userio->head != userio->tail)
> +               return POLLIN | POLLRDNORM;
> +
> +       return 0;
> +}
> +
> +static const struct file_operations userio_fops = {
> +       .owner          = THIS_MODULE,
> +       .open           = userio_char_open,
> +       .release        = userio_char_release,
> +       .read           = userio_char_read,
> +       .write          = userio_char_write,
> +       .poll           = userio_char_poll,
> +       .llseek         = no_llseek,
> +};
> +
> +static struct miscdevice userio_misc = {
> +       .fops   = &userio_fops,
> +       .minor  = USERIO_MINOR,
> +       .name   = USERIO_NAME,
> +};
> +
> +MODULE_ALIAS_MISCDEV(USERIO_MINOR);
> +MODULE_ALIAS("devname:" USERIO_NAME);
> +
> +MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
> +MODULE_DESCRIPTION("Virtual Serio Device Support");
> +MODULE_LICENSE("GPL");
> +
> +module_driver(userio_misc, misc_register, misc_deregister);
> diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
> index 81f6e42..5430374 100644
> --- a/include/linux/miscdevice.h
> +++ b/include/linux/miscdevice.h
> @@ -49,6 +49,7 @@
>  #define LOOP_CTRL_MINOR                237
>  #define VHOST_NET_MINOR                238
>  #define UHID_MINOR             239
> +#define USERIO_MINOR           240
>  #define MISC_DYNAMIC_MINOR     255
>
>  struct device;
> diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
> new file mode 100644
> index 0000000..37d147f
> --- /dev/null
> +++ b/include/uapi/linux/userio.h
> @@ -0,0 +1,44 @@
> +/*
> + * userio: virtual serio device support
> + * Copyright (C) 2015 Red Hat
> + * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
> + * details.
> + *
> + * This is the public header used for user-space communication with the userio
> + * driver. __attribute__((__packed__)) is used for all structs to keep ABI
> + * compatibility between all architectures.
> + */
> +
> +#ifndef _USERIO_H
> +#define _USERIO_H
> +
> +#include <linux/types.h>
> +
> +enum userio_cmd_type {
> +       USERIO_CMD_REGISTER = 0,
> +       USERIO_CMD_SET_PORT_TYPE = 1,
> +       USERIO_CMD_SEND_INTERRUPT = 2
> +};
> +
> +/*
> + * userio Commands
> + * All commands sent to /dev/userio are encoded using this structure. The type
> + * field should contain a USERIO_CMD* value that indicates what kind of command
> + * is being sent to userio. The data field should contain the accompanying
> + * argument for the command, if there is one.
> + */
> +struct userio_cmd {
> +       __u8 type;
> +       __u8 data;
> +} __attribute__((__packed__));
> +
> +#endif /* !_USERIO_H */

Otherwise, looks all good.

Thanks
David

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v8] Input: Add userio module
  2015-10-08 17:20           ` David Herrmann
@ 2015-10-09 14:30             ` cpaul
  2015-10-09 14:52               ` LABBE Corentin
  2015-10-24 22:26             ` [PATCH v7] " Dmitry Torokhov
  1 sibling, 1 reply; 19+ messages in thread
From: cpaul @ 2015-10-09 14:30 UTC (permalink / raw)
  To: David Herrmann, Dmitry Torokhov
  Cc: linux-kernel, Linux API, Andrew Morton, Mauro Carvalho Chehab,
	Greg KH, Arnd Bergmann, Joe Perches, Jiri Slaby, Vishnu Patekar,
	Sebastian Ott, Benjamin Tissoires, Hans de Goede, linux-doc

From: Stephen Chandler Paul <cpaul@redhat.com>

Debugging input devices, specifically laptop touchpads, can be tricky
without having the physical device handy. Here we try to remedy that
with userio. This module allows an application to connect to a character
device provided by the kernel, and emulate any serio device. In
combination with userspace programs that can record PS/2 devices and
replay them through the /dev/userio device, this allows developers to
debug driver issues on the PS/2 level with devices simply by requesting
a recording from the user experiencing the issue without having to have
the physical hardware in front of them.

Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
---
				    Changes
* Add empty line under GPL notice
* Remove mutex lock in userio_char_read()
* Also remove the goto in userio_char_read(), we only needed it before to
  ensure the mutex got unlocked at the end of the function
* Remove & in front of buf in userio_char_read()
* Return error if count != sizeof(cmd)
  (changed the top of the function instead of the change you suggested, does
   the same thing with fewer lines)
* Added a warning message if we get an invalid payload header. We do this for
  everything else but segfaults so I figured it would be appropriate

Re: write() after serio; originally I thought that there was no guarantee of
this (hence the ineffective `if (!userio)` checks before), however going off
Dmitry I think serio_unregister_port() gives us the guarantee this will never
happen. Of course it's up to him to clarify this.

 Documentation/input/userio.txt |  70 +++++++++++
 MAINTAINERS                    |   6 +
 drivers/input/serio/Kconfig    |  11 ++
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/userio.c   | 279 +++++++++++++++++++++++++++++++++++++++++
 include/linux/miscdevice.h     |   1 +
 include/uapi/linux/userio.h    |  44 +++++++
 7 files changed, 412 insertions(+)
 create mode 100644 Documentation/input/userio.txt
 create mode 100644 drivers/input/serio/userio.c
 create mode 100644 include/uapi/linux/userio.h

diff --git a/Documentation/input/userio.txt b/Documentation/input/userio.txt
new file mode 100644
index 0000000..0880c0f
--- /dev/null
+++ b/Documentation/input/userio.txt
@@ -0,0 +1,70 @@
+			      The userio Protocol
+	     (c) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+			      Sponsored by Red Hat
+--------------------------------------------------------------------------------
+
+1. Introduction
+~~~~~~~~~~~~~~~
+  This module is intended to try to make the lives of input driver developers
+easier by allowing them to test various serio devices (mainly the various
+touchpads found on laptops) without having to have the physical device in front
+of them. userio accomplishes this by allowing any privileged userspace program
+to directly interact with the kernel's serio driver and control a virtual serio
+port from there.
+
+2. Usage overview
+~~~~~~~~~~~~~~~~~
+  In order to interact with the userio kernel module, one simply opens the
+/dev/userio character device in their applications. Commands are sent to the
+kernel module by writing to the device, and any data received from the serio
+driver is read as-is from the /dev/userio device. All of the structures and
+macros you need to interact with the device are defined in <linux/userio.h> and
+<linux/serio.h>.
+
+3. Command Structure
+~~~~~~~~~~~~~~~~~~~~
+  The struct used for sending commands to /dev/userio is as follows:
+
+	struct userio_cmd {
+		__u8 type;
+		__u8 data;
+	};
+
+  "type" describes the type of command that is being sent. This can be any one
+of the USERIO_CMD macros defined in <linux/userio.h>. "data" is the argument
+that goes along with the command. In the event that the command doesn't have an
+argument, this field can be left untouched and will be ignored by the kernel.
+Each command should be sent by writing the struct directly to the character
+device. In the event that the command you send is invalid, an error will be
+returned by the character device and a more descriptive error will be printed
+to the kernel log. Only one command can be sent at a time, any additional data
+written to the character device after the initial command will be ignored.
+  To close the virtual serio port, just close /dev/userio.
+
+4. Commands
+~~~~~~~~~~~
+
+4.1 USERIO_CMD_REGISTER
+~~~~~~~~~~~~~~~~~~~~~~~
+  Registers the port with the serio driver and begins transmitting data back and
+forth. Registration can only be performed once a port type is set with
+USERIO_CMD_SET_PORT_TYPE. Has no argument.
+
+4.2 USERIO_CMD_SET_PORT_TYPE
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sets the type of port we're emulating, where "data" is the port type being
+set. Can be any of the macros from <linux/serio.h>. For example: SERIO_8042
+would set the port type to be a normal PS/2 port.
+
+4.3 USERIO_CMD_SEND_INTERRUPT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sends an interrupt through the virtual serio port to the serio driver, where
+"data" is the interrupt data being sent.
+
+5. Userspace tools
+~~~~~~~~~~~~~~~~~~
+  The userio userspace tools are able to record PS/2 devices using some of the
+debugging information from i8042, and play back the devices on /dev/userio. The
+latest version of these tools can be found at:
+
+	https://github.com/Lyude/ps2emu
diff --git a/MAINTAINERS b/MAINTAINERS
index 797236b..ba59bd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11100,6 +11100,12 @@ S:	Maintained
 F:	drivers/media/v4l2-core/videobuf2-*
 F:	include/media/videobuf2-*
 
+VIRTUAL SERIO DEVICE DRIVER
+M:	Stephen Chandler Paul <thatslyude@gmail.com>
+S:	Maintained
+F:	drivers/input/serio/userio.c
+F:	include/uapi/linux/userio.h
+
 VIRTIO CONSOLE DRIVER
 M:	Amit Shah <amit.shah@redhat.com>
 L:	virtualization@lists.linux-foundation.org
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 200841b..22b8b58 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,4 +292,15 @@ config SERIO_SUN4I_PS2
 	  To compile this driver as a module, choose M here: the
 	  module will be called sun4i-ps2.
 
+config USERIO
+	tristate "Virtual serio device support"
+	help
+	  Say Y here if you want to emulate serio devices using the userio
+	  kernel module.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called userio.
+
+	  If you are unsure, say N.
+
 endif
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index c600089..2374ef9 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_SERIO_APBPS2)	+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)	+= olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)	+= hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)	+= sun4i-ps2.o
+obj-$(CONFIG_USERIO)		+= userio.o
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
new file mode 100644
index 0000000..7a89371
--- /dev/null
+++ b/drivers/input/serio/userio.c
@@ -0,0 +1,279 @@
+/*
+ * userio kernel serio device emulation module
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ */
+
+#include <linux/circ_buf.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <linux/sched.h>
+#include <linux/poll.h>
+#include <uapi/linux/userio.h>
+
+#define USERIO_NAME "userio"
+#define USERIO_BUFSIZE 16
+
+static struct miscdevice userio_misc;
+
+struct userio_device {
+	struct serio *serio;
+	struct mutex lock;
+
+	bool running;
+
+	u8 head;
+	u8 tail;
+
+	spinlock_t buf_lock;
+	unsigned char buf[USERIO_BUFSIZE];
+
+	wait_queue_head_t waitq;
+};
+
+/**
+ * userio_device_write - Write data from serio to a userio device in userspace
+ * @id: The serio port for the userio device
+ * @val: The data to write to the device
+ */
+static int userio_device_write(struct serio *id, unsigned char val)
+{
+	struct userio_device *userio = id->port_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&userio->buf_lock, flags);
+
+	userio->buf[userio->head] = val;
+	userio->head = (userio->head + 1) % USERIO_BUFSIZE;
+
+	if (userio->head == userio->tail)
+		dev_warn(userio_misc.this_device,
+			 "Buffer overflowed, userio client isn't keeping up");
+
+	spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+	wake_up_interruptible(&userio->waitq);
+
+	return 0;
+}
+
+static int userio_char_open(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio;
+
+	userio = kzalloc(sizeof(struct userio_device), GFP_KERNEL);
+	if (!userio)
+		return -ENOMEM;
+
+	mutex_init(&userio->lock);
+	spin_lock_init(&userio->buf_lock);
+	init_waitqueue_head(&userio->waitq);
+
+	userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
+	if (!userio->serio) {
+		kfree(userio);
+		return -ENOMEM;
+	}
+
+	userio->serio->write = userio_device_write;
+	userio->serio->port_data = userio;
+
+	file->private_data = userio;
+
+	return 0;
+}
+
+static int userio_char_release(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio = file->private_data;
+
+	/* Don't free the serio port here, serio_unregister_port() does
+	 * this for us */
+	if (userio->running)
+		serio_unregister_port(userio->serio);
+	else
+		kfree(userio->serio);
+
+	kfree(userio);
+
+	return 0;
+}
+
+static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
+				size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	int ret;
+	size_t nonwrap_len, copylen;
+	unsigned char buf[USERIO_BUFSIZE];
+	unsigned long flags;
+
+	if (!count)
+		return 0;
+
+	/*
+	 * By the time we get here, the data that was waiting might have been
+	 * taken by another thread. Grab the mutex and check if there's still
+	 * any data waiting, otherwise repeat this process until we have data
+	 * (unless the file descriptor is non-blocking of course)
+	 */
+	for (;;) {
+		spin_lock_irqsave(&userio->buf_lock, flags);
+
+		if (userio->head != userio->tail)
+			break;
+
+		spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+		if (file->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+
+		ret = wait_event_interruptible(userio->waitq,
+					       userio->head != userio->tail);
+		if (ret)
+			return ret;
+	}
+
+	nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
+				      USERIO_BUFSIZE);
+	copylen = min(nonwrap_len, count);
+	memcpy(buf, &userio->buf[userio->tail], copylen);
+
+	userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
+
+	spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+	if (copy_to_user(user_buffer, buf, copylen))
+		return -EFAULT;
+
+	return copylen;
+}
+
+static ssize_t userio_char_write(struct file *file, const char __user *buffer,
+				 size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	struct userio_cmd cmd;
+	int ret;
+
+	if (count != sizeof(cmd)) {
+		dev_warn(userio_misc.this_device, "Invalid payload size\n");
+		return -EINVAL;
+	}
+
+	if (copy_from_user(&cmd, buffer, sizeof(cmd)))
+		return -EFAULT;
+
+	ret = mutex_lock_interruptible(&userio->lock);
+	if (ret)
+		return ret;
+
+	switch (cmd.type) {
+	case USERIO_CMD_REGISTER:
+		if (!userio->serio->id.type) {
+			dev_warn(userio_misc.this_device,
+				 "No port type given on /dev/userio\n");
+
+			ret = -EINVAL;
+			goto out;
+		}
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Begin command sent, but we're already running\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->running = true;
+		serio_register_port(userio->serio);
+		break;
+
+	case USERIO_CMD_SET_PORT_TYPE:
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Can't change port type on an already running userio instance\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->serio->id.type = cmd.data;
+		break;
+
+	case USERIO_CMD_SEND_INTERRUPT:
+		if (!userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "The device must be registered before sending interrupts\n");
+
+			ret = -ENODEV;
+			goto out;
+		}
+
+		serio_interrupt(userio->serio, cmd.data, 0);
+		break;
+
+	default:
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	ret = sizeof(cmd);
+
+out:
+	mutex_unlock(&userio->lock);
+	return ret;
+}
+
+static unsigned int userio_char_poll(struct file *file, poll_table *wait)
+{
+	struct userio_device *userio = file->private_data;
+
+	poll_wait(file, &userio->waitq, wait);
+
+	if (userio->head != userio->tail)
+		return POLLIN | POLLRDNORM;
+
+	return 0;
+}
+
+static const struct file_operations userio_fops = {
+	.owner		= THIS_MODULE,
+	.open		= userio_char_open,
+	.release	= userio_char_release,
+	.read		= userio_char_read,
+	.write		= userio_char_write,
+	.poll		= userio_char_poll,
+	.llseek		= no_llseek,
+};
+
+static struct miscdevice userio_misc = {
+	.fops	= &userio_fops,
+	.minor	= USERIO_MINOR,
+	.name	= USERIO_NAME,
+};
+
+MODULE_ALIAS_MISCDEV(USERIO_MINOR);
+MODULE_ALIAS("devname:" USERIO_NAME);
+
+MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
+MODULE_DESCRIPTION("Virtual Serio Device Support");
+MODULE_LICENSE("GPL");
+
+module_driver(userio_misc, misc_register, misc_deregister);
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 81f6e42..5430374 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -49,6 +49,7 @@
 #define LOOP_CTRL_MINOR		237
 #define VHOST_NET_MINOR		238
 #define UHID_MINOR		239
+#define USERIO_MINOR		240
 #define MISC_DYNAMIC_MINOR	255
 
 struct device;
diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
new file mode 100644
index 0000000..37d147f
--- /dev/null
+++ b/include/uapi/linux/userio.h
@@ -0,0 +1,44 @@
+/*
+ * userio: virtual serio device support
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ *
+ * This is the public header used for user-space communication with the userio
+ * driver. __attribute__((__packed__)) is used for all structs to keep ABI
+ * compatibility between all architectures.
+ */
+
+#ifndef _USERIO_H
+#define _USERIO_H
+
+#include <linux/types.h>
+
+enum userio_cmd_type {
+	USERIO_CMD_REGISTER = 0,
+	USERIO_CMD_SET_PORT_TYPE = 1,
+	USERIO_CMD_SEND_INTERRUPT = 2
+};
+
+/*
+ * userio Commands
+ * All commands sent to /dev/userio are encoded using this structure. The type
+ * field should contain a USERIO_CMD* value that indicates what kind of command
+ * is being sent to userio. The data field should contain the accompanying
+ * argument for the command, if there is one.
+ */
+struct userio_cmd {
+	__u8 type;
+	__u8 data;
+} __attribute__((__packed__));
+
+#endif /* !_USERIO_H */
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v8] Input: Add userio module
  2015-10-09 14:30             ` [PATCH v8] " cpaul
@ 2015-10-09 14:52               ` LABBE Corentin
  2015-10-09 14:59                 ` Stephen Chandler Paul
  0 siblings, 1 reply; 19+ messages in thread
From: LABBE Corentin @ 2015-10-09 14:52 UTC (permalink / raw)
  To: cpaul
  Cc: David Herrmann, Dmitry Torokhov, linux-kernel, Linux API,
	Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
	Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott,
	Benjamin Tissoires, Hans de Goede, linux-doc

On Fri, Oct 09, 2015 at 10:30:53AM -0400, cpaul@redhat.com wrote:
> From: Stephen Chandler Paul <cpaul@redhat.com>
> +
> +static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
> +				size_t count, loff_t *ppos)
> +{
> +	struct userio_device *userio = file->private_data;
> +	int ret;
> +	size_t nonwrap_len, copylen;
> +	unsigned char buf[USERIO_BUFSIZE];
> +	unsigned long flags;
> +
> +	if (!count)
> +		return 0;
> +
> +	/*
> +	 * By the time we get here, the data that was waiting might have been
> +	 * taken by another thread. Grab the mutex and check if there's still
> +	 * any data waiting, otherwise repeat this process until we have data
> +	 * (unless the file descriptor is non-blocking of course)
> +	 */
> +	for (;;) {
> +		spin_lock_irqsave(&userio->buf_lock, flags);
> +
> +		if (userio->head != userio->tail)
> +			break;
> +
> +		spin_unlock_irqrestore(&userio->buf_lock, flags);
> +
> +		if (file->f_flags & O_NONBLOCK)
> +			return -EAGAIN;
> +
> +		ret = wait_event_interruptible(userio->waitq,
> +					       userio->head != userio->tail);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
> +				      USERIO_BUFSIZE);
> +	copylen = min(nonwrap_len, count);
> +	memcpy(buf, &userio->buf[userio->tail], copylen);
> +
> +	userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
> +
> +	spin_unlock_irqrestore(&userio->buf_lock, flags);
> +
> +	if (copy_to_user(user_buffer, buf, copylen))
> +		return -EFAULT;
> +
> +	return copylen;
> +}

Hello

I think you could simplify by just return copy_to_user() since it return copylen in case of success.

Regards


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8] Input: Add userio module
  2015-10-09 14:52               ` LABBE Corentin
@ 2015-10-09 14:59                 ` Stephen Chandler Paul
  0 siblings, 0 replies; 19+ messages in thread
From: Stephen Chandler Paul @ 2015-10-09 14:59 UTC (permalink / raw)
  To: LABBE Corentin
  Cc: David Herrmann, Dmitry Torokhov, linux-kernel, Linux API,
	Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
	Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott,
	Benjamin Tissoires, Hans de Goede, linux-doc

On Fri, 2015-10-09 at 16:52 +0200, LABBE Corentin wrote:
> On Fri, Oct 09, 2015 at 10:30:53AM -0400, cpaul@redhat.com wrote:
> > From: Stephen Chandler Paul <cpaul@redhat.com>
> > +
> > +static ssize_t userio_char_read(struct file *file, char __user
> > *user_buffer,
> > +				size_t count, loff_t *ppos)
> > +{
> > +	struct userio_device *userio = file->private_data;
> > +	int ret;
> > +	size_t nonwrap_len, copylen;
> > +	unsigned char buf[USERIO_BUFSIZE];
> > +	unsigned long flags;
> > +
> > +	if (!count)
> > +		return 0;
> > +
> > +	/*
> > +	 * By the time we get here, the data that was waiting
> > might have been
> > +	 * taken by another thread. Grab the mutex and check if
> > there's still
> > +	 * any data waiting, otherwise repeat this process until
> > we have data
> > +	 * (unless the file descriptor is non-blocking of course)
> > +	 */
> > +	for (;;) {
> > +		spin_lock_irqsave(&userio->buf_lock, flags);
> > +
> > +		if (userio->head != userio->tail)
> > +			break;
> > +
> > +		spin_unlock_irqrestore(&userio->buf_lock, flags);
> > +
> > +		if (file->f_flags & O_NONBLOCK)
> > +			return -EAGAIN;
> > +
> > +		ret = wait_event_interruptible(userio->waitq,
> > +					       userio->head !=
> > userio->tail);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
> > +				      USERIO_BUFSIZE);
> > +	copylen = min(nonwrap_len, count);
> > +	memcpy(buf, &userio->buf[userio->tail], copylen);
> > +
> > +	userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
> > +
> > +	spin_unlock_irqrestore(&userio->buf_lock, flags);
> > +
> > +	if (copy_to_user(user_buffer, buf, copylen))
> > +		return -EFAULT;
> > +
> > +	return copylen;
> > +}
> 
> Hello
> 
> I think you could simplify by just return copy_to_user() since it
> return copylen in case of success.
> 
> Regards

Hi! You're mistaken here actually:
https://www.fsl.cs.sunysb.edu/kernel-api/re256.html
"Returns number of bytes that could not be copied. On success, this
will be zero". Plus, we need to check if the copy failed or not since
if it did we have to indicate -EFAULT to signal that the memory address
the user's buffer is pointed to isn't actually accessible.

Cheers,
	Lyude

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v6.2 1/1] Input: Add userio module
  2015-10-05 14:06         ` Stephen Chandler Paul
@ 2015-10-23 20:47           ` cpaul
  2015-10-24 22:40             ` Dmitry Torokhov
  0 siblings, 1 reply; 19+ messages in thread
From: cpaul @ 2015-10-23 20:47 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: David Herrmann, linux-kernel, linux-input, linux-api,
	Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
	Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott,
	Benjamin Tissoires, Hans de Goede, linux-doc

From: Stephen Chandler Paul <cpaul@redhat.com>

Debugging input devices, specifically laptop touchpads, can be tricky
without having the physical device handy. Here we try to remedy that
with userio. This module allows an application to connect to a character
device provided by the kernel, and emulate any serio device. In
combination with userspace programs that can record PS/2 devices and
replay them through the /dev/userio device, this allows developers to
debug driver issues on the PS/2 level with devices simply by requesting
a recording from the user experiencing the issue without having to have
the physical hardware in front of them.

Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
---
				    Changes
* Remove the if (!userio) { return -1; } check that somehow got left in.

Sorry this took so long! I was wondering why you hadn't replied yet, only to
notice I only made this change on my own tree and never sent out a response
patch. Oops.

 Documentation/input/userio.txt |  70 +++++++++++
 MAINTAINERS                    |   6 +
 drivers/input/serio/Kconfig    |  11 ++
 drivers/input/serio/Makefile   |   1 +
 drivers/input/serio/userio.c   | 279 +++++++++++++++++++++++++++++++++++++++++
 include/linux/miscdevice.h     |   1 +
 include/uapi/linux/userio.h    |  44 +++++++
 7 files changed, 412 insertions(+)
 create mode 100644 Documentation/input/userio.txt
 create mode 100644 drivers/input/serio/userio.c
 create mode 100644 include/uapi/linux/userio.h

diff --git a/Documentation/input/userio.txt b/Documentation/input/userio.txt
new file mode 100644
index 0000000..0880c0f
--- /dev/null
+++ b/Documentation/input/userio.txt
@@ -0,0 +1,70 @@
+			      The userio Protocol
+	     (c) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+			      Sponsored by Red Hat
+--------------------------------------------------------------------------------
+
+1. Introduction
+~~~~~~~~~~~~~~~
+  This module is intended to try to make the lives of input driver developers
+easier by allowing them to test various serio devices (mainly the various
+touchpads found on laptops) without having to have the physical device in front
+of them. userio accomplishes this by allowing any privileged userspace program
+to directly interact with the kernel's serio driver and control a virtual serio
+port from there.
+
+2. Usage overview
+~~~~~~~~~~~~~~~~~
+  In order to interact with the userio kernel module, one simply opens the
+/dev/userio character device in their applications. Commands are sent to the
+kernel module by writing to the device, and any data received from the serio
+driver is read as-is from the /dev/userio device. All of the structures and
+macros you need to interact with the device are defined in <linux/userio.h> and
+<linux/serio.h>.
+
+3. Command Structure
+~~~~~~~~~~~~~~~~~~~~
+  The struct used for sending commands to /dev/userio is as follows:
+
+	struct userio_cmd {
+		__u8 type;
+		__u8 data;
+	};
+
+  "type" describes the type of command that is being sent. This can be any one
+of the USERIO_CMD macros defined in <linux/userio.h>. "data" is the argument
+that goes along with the command. In the event that the command doesn't have an
+argument, this field can be left untouched and will be ignored by the kernel.
+Each command should be sent by writing the struct directly to the character
+device. In the event that the command you send is invalid, an error will be
+returned by the character device and a more descriptive error will be printed
+to the kernel log. Only one command can be sent at a time, any additional data
+written to the character device after the initial command will be ignored.
+  To close the virtual serio port, just close /dev/userio.
+
+4. Commands
+~~~~~~~~~~~
+
+4.1 USERIO_CMD_REGISTER
+~~~~~~~~~~~~~~~~~~~~~~~
+  Registers the port with the serio driver and begins transmitting data back and
+forth. Registration can only be performed once a port type is set with
+USERIO_CMD_SET_PORT_TYPE. Has no argument.
+
+4.2 USERIO_CMD_SET_PORT_TYPE
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sets the type of port we're emulating, where "data" is the port type being
+set. Can be any of the macros from <linux/serio.h>. For example: SERIO_8042
+would set the port type to be a normal PS/2 port.
+
+4.3 USERIO_CMD_SEND_INTERRUPT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  Sends an interrupt through the virtual serio port to the serio driver, where
+"data" is the interrupt data being sent.
+
+5. Userspace tools
+~~~~~~~~~~~~~~~~~~
+  The userio userspace tools are able to record PS/2 devices using some of the
+debugging information from i8042, and play back the devices on /dev/userio. The
+latest version of these tools can be found at:
+
+	https://github.com/Lyude/ps2emu
diff --git a/MAINTAINERS b/MAINTAINERS
index 797236b..ba59bd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11100,6 +11100,12 @@ S:	Maintained
 F:	drivers/media/v4l2-core/videobuf2-*
 F:	include/media/videobuf2-*
 
+VIRTUAL SERIO DEVICE DRIVER
+M:	Stephen Chandler Paul <thatslyude@gmail.com>
+S:	Maintained
+F:	drivers/input/serio/userio.c
+F:	include/uapi/linux/userio.h
+
 VIRTIO CONSOLE DRIVER
 M:	Amit Shah <amit.shah@redhat.com>
 L:	virtualization@lists.linux-foundation.org
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 200841b..22b8b58 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,4 +292,15 @@ config SERIO_SUN4I_PS2
 	  To compile this driver as a module, choose M here: the
 	  module will be called sun4i-ps2.
 
+config USERIO
+	tristate "Virtual serio device support"
+	help
+	  Say Y here if you want to emulate serio devices using the userio
+	  kernel module.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called userio.
+
+	  If you are unsure, say N.
+
 endif
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index c600089..2374ef9 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_SERIO_APBPS2)	+= apbps2.o
 obj-$(CONFIG_SERIO_OLPC_APSP)	+= olpc_apsp.o
 obj-$(CONFIG_HYPERV_KEYBOARD)	+= hyperv-keyboard.o
 obj-$(CONFIG_SERIO_SUN4I_PS2)	+= sun4i-ps2.o
+obj-$(CONFIG_USERIO)		+= userio.o
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
new file mode 100644
index 0000000..7a89371
--- /dev/null
+++ b/drivers/input/serio/userio.c
@@ -0,0 +1,279 @@
+/*
+ * userio kernel serio device emulation module
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ */
+
+#include <linux/circ_buf.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <linux/sched.h>
+#include <linux/poll.h>
+#include <uapi/linux/userio.h>
+
+#define USERIO_NAME "userio"
+#define USERIO_BUFSIZE 16
+
+static struct miscdevice userio_misc;
+
+struct userio_device {
+	struct serio *serio;
+	struct mutex lock;
+
+	bool running;
+
+	u8 head;
+	u8 tail;
+
+	spinlock_t buf_lock;
+	unsigned char buf[USERIO_BUFSIZE];
+
+	wait_queue_head_t waitq;
+};
+
+/**
+ * userio_device_write - Write data from serio to a userio device in userspace
+ * @id: The serio port for the userio device
+ * @val: The data to write to the device
+ */
+static int userio_device_write(struct serio *id, unsigned char val)
+{
+	struct userio_device *userio = id->port_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&userio->buf_lock, flags);
+
+	userio->buf[userio->head] = val;
+	userio->head = (userio->head + 1) % USERIO_BUFSIZE;
+
+	if (userio->head == userio->tail)
+		dev_warn(userio_misc.this_device,
+			 "Buffer overflowed, userio client isn't keeping up");
+
+	spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+	wake_up_interruptible(&userio->waitq);
+
+	return 0;
+}
+
+static int userio_char_open(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio;
+
+	userio = kzalloc(sizeof(struct userio_device), GFP_KERNEL);
+	if (!userio)
+		return -ENOMEM;
+
+	mutex_init(&userio->lock);
+	spin_lock_init(&userio->buf_lock);
+	init_waitqueue_head(&userio->waitq);
+
+	userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
+	if (!userio->serio) {
+		kfree(userio);
+		return -ENOMEM;
+	}
+
+	userio->serio->write = userio_device_write;
+	userio->serio->port_data = userio;
+
+	file->private_data = userio;
+
+	return 0;
+}
+
+static int userio_char_release(struct inode *inode, struct file *file)
+{
+	struct userio_device *userio = file->private_data;
+
+	/* Don't free the serio port here, serio_unregister_port() does
+	 * this for us */
+	if (userio->running)
+		serio_unregister_port(userio->serio);
+	else
+		kfree(userio->serio);
+
+	kfree(userio);
+
+	return 0;
+}
+
+static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
+				size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	int ret;
+	size_t nonwrap_len, copylen;
+	unsigned char buf[USERIO_BUFSIZE];
+	unsigned long flags;
+
+	if (!count)
+		return 0;
+
+	/*
+	 * By the time we get here, the data that was waiting might have been
+	 * taken by another thread. Grab the mutex and check if there's still
+	 * any data waiting, otherwise repeat this process until we have data
+	 * (unless the file descriptor is non-blocking of course)
+	 */
+	for (;;) {
+		spin_lock_irqsave(&userio->buf_lock, flags);
+
+		if (userio->head != userio->tail)
+			break;
+
+		spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+		if (file->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+
+		ret = wait_event_interruptible(userio->waitq,
+					       userio->head != userio->tail);
+		if (ret)
+			return ret;
+	}
+
+	nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
+				      USERIO_BUFSIZE);
+	copylen = min(nonwrap_len, count);
+	memcpy(buf, &userio->buf[userio->tail], copylen);
+
+	userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
+
+	spin_unlock_irqrestore(&userio->buf_lock, flags);
+
+	if (copy_to_user(user_buffer, buf, copylen))
+		return -EFAULT;
+
+	return copylen;
+}
+
+static ssize_t userio_char_write(struct file *file, const char __user *buffer,
+				 size_t count, loff_t *ppos)
+{
+	struct userio_device *userio = file->private_data;
+	struct userio_cmd cmd;
+	int ret;
+
+	if (count != sizeof(cmd)) {
+		dev_warn(userio_misc.this_device, "Invalid payload size\n");
+		return -EINVAL;
+	}
+
+	if (copy_from_user(&cmd, buffer, sizeof(cmd)))
+		return -EFAULT;
+
+	ret = mutex_lock_interruptible(&userio->lock);
+	if (ret)
+		return ret;
+
+	switch (cmd.type) {
+	case USERIO_CMD_REGISTER:
+		if (!userio->serio->id.type) {
+			dev_warn(userio_misc.this_device,
+				 "No port type given on /dev/userio\n");
+
+			ret = -EINVAL;
+			goto out;
+		}
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Begin command sent, but we're already running\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->running = true;
+		serio_register_port(userio->serio);
+		break;
+
+	case USERIO_CMD_SET_PORT_TYPE:
+		if (userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "Can't change port type on an already running userio instance\n");
+
+			ret = -EBUSY;
+			goto out;
+		}
+
+		userio->serio->id.type = cmd.data;
+		break;
+
+	case USERIO_CMD_SEND_INTERRUPT:
+		if (!userio->running) {
+			dev_warn(userio_misc.this_device,
+				 "The device must be registered before sending interrupts\n");
+
+			ret = -ENODEV;
+			goto out;
+		}
+
+		serio_interrupt(userio->serio, cmd.data, 0);
+		break;
+
+	default:
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	ret = sizeof(cmd);
+
+out:
+	mutex_unlock(&userio->lock);
+	return ret;
+}
+
+static unsigned int userio_char_poll(struct file *file, poll_table *wait)
+{
+	struct userio_device *userio = file->private_data;
+
+	poll_wait(file, &userio->waitq, wait);
+
+	if (userio->head != userio->tail)
+		return POLLIN | POLLRDNORM;
+
+	return 0;
+}
+
+static const struct file_operations userio_fops = {
+	.owner		= THIS_MODULE,
+	.open		= userio_char_open,
+	.release	= userio_char_release,
+	.read		= userio_char_read,
+	.write		= userio_char_write,
+	.poll		= userio_char_poll,
+	.llseek		= no_llseek,
+};
+
+static struct miscdevice userio_misc = {
+	.fops	= &userio_fops,
+	.minor	= USERIO_MINOR,
+	.name	= USERIO_NAME,
+};
+
+MODULE_ALIAS_MISCDEV(USERIO_MINOR);
+MODULE_ALIAS("devname:" USERIO_NAME);
+
+MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
+MODULE_DESCRIPTION("Virtual Serio Device Support");
+MODULE_LICENSE("GPL");
+
+module_driver(userio_misc, misc_register, misc_deregister);
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index 81f6e42..5430374 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -49,6 +49,7 @@
 #define LOOP_CTRL_MINOR		237
 #define VHOST_NET_MINOR		238
 #define UHID_MINOR		239
+#define USERIO_MINOR		240
 #define MISC_DYNAMIC_MINOR	255
 
 struct device;
diff --git a/include/uapi/linux/userio.h b/include/uapi/linux/userio.h
new file mode 100644
index 0000000..37d147f
--- /dev/null
+++ b/include/uapi/linux/userio.h
@@ -0,0 +1,44 @@
+/*
+ * userio: virtual serio device support
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ *
+ * This is the public header used for user-space communication with the userio
+ * driver. __attribute__((__packed__)) is used for all structs to keep ABI
+ * compatibility between all architectures.
+ */
+
+#ifndef _USERIO_H
+#define _USERIO_H
+
+#include <linux/types.h>
+
+enum userio_cmd_type {
+	USERIO_CMD_REGISTER = 0,
+	USERIO_CMD_SET_PORT_TYPE = 1,
+	USERIO_CMD_SEND_INTERRUPT = 2
+};
+
+/*
+ * userio Commands
+ * All commands sent to /dev/userio are encoded using this structure. The type
+ * field should contain a USERIO_CMD* value that indicates what kind of command
+ * is being sent to userio. The data field should contain the accompanying
+ * argument for the command, if there is one.
+ */
+struct userio_cmd {
+	__u8 type;
+	__u8 data;
+} __attribute__((__packed__));
+
+#endif /* !_USERIO_H */
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v7] Input: Add userio module
  2015-10-08 17:20           ` David Herrmann
  2015-10-09 14:30             ` [PATCH v8] " cpaul
@ 2015-10-24 22:26             ` Dmitry Torokhov
  1 sibling, 0 replies; 19+ messages in thread
From: Dmitry Torokhov @ 2015-10-24 22:26 UTC (permalink / raw)
  To: David Herrmann
  Cc: Stephen Chandler Paul, linux-kernel, Linux API, Andrew Morton,
	Mauro Carvalho Chehab, Greg KH, Arnd Bergmann, Joe Perches,
	Jiri Slaby, Vishnu Patekar, Sebastian Ott, Benjamin Tissoires,
	Hans de Goede, linux-doc

On Thu, Oct 08, 2015 at 07:20:59PM +0200, David Herrmann wrote:
> On Mon, Oct 5, 2015 at 5:55 PM,  <cpaul@redhat.com> wrote:
> > +static int userio_char_release(struct inode *inode, struct file *file)
> > +{
> > +       struct userio_device *userio = file->private_data;
> > +
> > +       /* Don't free the serio port here, serio_unregister_port() does
> > +        * this for us */
> > +       if (userio->running)
> > +               serio_unregister_port(userio->serio);
> > +       else
> > +               kfree(userio->serio);
> > +
> > +       kfree(userio);
> 
> I'm not very familiar with the serio-subsystem, but is there a
> guarantee that .write() will not be called once unregistered? I can
> see that it's not scheduled by userio anymore, but maybe there's an
> async path that can trigger .write() callbacks. Dunno.. I'll leave
> that to Dmitry.

I think this is OK. When port is unregistered we the driver will be
disconnected from it, and is not supposed to call serio_write() anymore.
In fact, it should not access port after calling serio_close(). If it
keeps the pointer and uses it is a bug in the other driver; we can't
really do much about it.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6.2 1/1] Input: Add userio module
  2015-10-23 20:47           ` [PATCH v6.2 " cpaul
@ 2015-10-24 22:40             ` Dmitry Torokhov
  2015-10-26 13:51               ` Lyude
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Torokhov @ 2015-10-24 22:40 UTC (permalink / raw)
  To: cpaul
  Cc: David Herrmann, linux-kernel, linux-input, linux-api,
	Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
	Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott,
	Benjamin Tissoires, Hans de Goede, linux-doc

Hi Stephen,

On Fri, Oct 23, 2015 at 04:47:46PM -0400, cpaul@redhat.com wrote:
> From: Stephen Chandler Paul <cpaul@redhat.com>
> 
> Debugging input devices, specifically laptop touchpads, can be tricky
> without having the physical device handy. Here we try to remedy that
> with userio. This module allows an application to connect to a character
> device provided by the kernel, and emulate any serio device. In
> combination with userspace programs that can record PS/2 devices and
> replay them through the /dev/userio device, this allows developers to
> debug driver issues on the PS/2 level with devices simply by requesting
> a recording from the user experiencing the issue without having to have
> the physical hardware in front of them.
> 
> Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> 				    Changes
> * Remove the if (!userio) { return -1; } check that somehow got left in.
> 
> Sorry this took so long! I was wondering why you hadn't replied yet, only to
> notice I only made this change on my own tree and never sent out a response
> patch. Oops.

Thank you for making all the changes.

> +
> +static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
> +				size_t count, loff_t *ppos)
> +{
> +	struct userio_device *userio = file->private_data;
> +	int ret;
> +	size_t nonwrap_len, copylen;
> +	unsigned char buf[USERIO_BUFSIZE];
> +	unsigned long flags;
> +
> +	if (!count)
> +		return 0;

This is not quite right: read of size 0 should still check if there is
data in the buffer and return -EAGAIN for non-blocking descriptors.

I cooked a patch (below) that should adjust the read behavior (_+ a
coupe of formatting changes), please take a look.

Thanks!

-- 
Dmitry

userio - misc fixups

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

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/Kconfig  |    9 ++-
 drivers/input/serio/userio.c |  122 ++++++++++++++++++++++--------------------
 2 files changed, 70 insertions(+), 61 deletions(-)

diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 22b8b58..c3d05b4 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -293,10 +293,13 @@ config SERIO_SUN4I_PS2
 	  module will be called sun4i-ps2.
 
 config USERIO
-	tristate "Virtual serio device support"
+	tristate "User space serio port driver support"
 	help
-	  Say Y here if you want to emulate serio devices using the userio
-	  kernel module.
+	  Say Y here if you want to support user level drivers for serio
+	  subsystem accessible under char device 10:240 - /dev/userio. Using
+	  this facility userspace programs can implement serio ports that
+	  will be used by the standard in-kernel serio consumer drivers,
+	  such as psmouse and atkbd.
 
 	  To compile this driver as a module, choose M here: the module will be
 	  called userio.
diff --git a/drivers/input/serio/userio.c b/drivers/input/serio/userio.c
index 7a89371..df1fd41 100644
--- a/drivers/input/serio/userio.c
+++ b/drivers/input/serio/userio.c
@@ -4,14 +4,14 @@
  * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
  *
  * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU Lesser General Public License as published by the
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
  *
- * This program is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
- * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
- * details.
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
+ * General Public License for more details.
  */
 
 #include <linux/circ_buf.h>
@@ -27,14 +27,14 @@
 #include <linux/poll.h>
 #include <uapi/linux/userio.h>
 
-#define USERIO_NAME "userio"
-#define USERIO_BUFSIZE 16
+#define USERIO_NAME		"userio"
+#define USERIO_BUFSIZE		16
 
 static struct miscdevice userio_misc;
 
 struct userio_device {
 	struct serio *serio;
-	struct mutex lock;
+	struct mutex mutex;
 
 	bool running;
 
@@ -81,7 +81,7 @@ static int userio_char_open(struct inode *inode, struct file *file)
 	if (!userio)
 		return -ENOMEM;
 
-	mutex_init(&userio->lock);
+	mutex_init(&userio->mutex);
 	spin_lock_init(&userio->buf_lock);
 	init_waitqueue_head(&userio->waitq);
 
@@ -103,12 +103,15 @@ static int userio_char_release(struct inode *inode, struct file *file)
 {
 	struct userio_device *userio = file->private_data;
 
-	/* Don't free the serio port here, serio_unregister_port() does
-	 * this for us */
-	if (userio->running)
+	if (userio->running) {
+		/*
+		 * Don't free the serio port here, serio_unregister_port()
+		 * does it for us.
+		 */
 		serio_unregister_port(userio->serio);
-	else
+	} else {
 		kfree(userio->serio);
+	}
 
 	kfree(userio);
 
@@ -119,48 +122,56 @@ static ssize_t userio_char_read(struct file *file, char __user *user_buffer,
 				size_t count, loff_t *ppos)
 {
 	struct userio_device *userio = file->private_data;
-	int ret;
+	int error;
 	size_t nonwrap_len, copylen;
 	unsigned char buf[USERIO_BUFSIZE];
 	unsigned long flags;
 
-	if (!count)
-		return 0;
-
 	/*
-	 * By the time we get here, the data that was waiting might have been
-	 * taken by another thread. Grab the mutex and check if there's still
-	 * any data waiting, otherwise repeat this process until we have data
-	 * (unless the file descriptor is non-blocking of course)
+	 * By the time we get here, the data that was waiting might have
+	 * been taken by another thread. Grab the buffer lock and check if
+	 * there's still any data waiting, otherwise repeat this process
+	 * until we have data (unless the file descriptor is non-blocking
+	 * of course).
 	 */
 	for (;;) {
 		spin_lock_irqsave(&userio->buf_lock, flags);
 
-		if (userio->head != userio->tail)
-			break;
+		nonwrap_len = CIRC_CNT_TO_END(userio->head,
+					      userio->tail,
+					      USERIO_BUFSIZE);
+		copylen = min(nonwrap_len, count);
+		if (copylen) {
+			memcpy(buf, &userio->buf[userio->tail], copylen);
+			userio->tail = (userio->tail + copylen) %
+							USERIO_BUFSIZE;
+		}
 
 		spin_unlock_irqrestore(&userio->buf_lock, flags);
 
+		if (nonwrap_len)
+			break;
+
+		/* buffer was/is empty */
 		if (file->f_flags & O_NONBLOCK)
 			return -EAGAIN;
 
-		ret = wait_event_interruptible(userio->waitq,
-					       userio->head != userio->tail);
-		if (ret)
-			return ret;
+		/*
+		 * count == 0 is special - no IO is done but we check
+		 * for error conditions (see above).
+		 */
+		if (count == 0)
+			return 0;
+
+		error = wait_event_interruptible(userio->waitq,
+						 userio->head != userio->tail);
+		if (error)
+			return error;
 	}
 
-	nonwrap_len = CIRC_CNT_TO_END(userio->head, userio->tail,
-				      USERIO_BUFSIZE);
-	copylen = min(nonwrap_len, count);
-	memcpy(buf, &userio->buf[userio->tail], copylen);
-
-	userio->tail = (userio->tail + copylen) % USERIO_BUFSIZE;
-
-	spin_unlock_irqrestore(&userio->buf_lock, flags);
-
-	if (copy_to_user(user_buffer, buf, copylen))
-		return -EFAULT;
+	if (copylen)
+		if (copy_to_user(user_buffer, buf, copylen))
+			return -EFAULT;
 
 	return copylen;
 }
@@ -170,7 +181,7 @@ static ssize_t userio_char_write(struct file *file, const char __user *buffer,
 {
 	struct userio_device *userio = file->private_data;
 	struct userio_cmd cmd;
-	int ret;
+	int error;
 
 	if (count != sizeof(cmd)) {
 		dev_warn(userio_misc.this_device, "Invalid payload size\n");
@@ -180,9 +191,9 @@ static ssize_t userio_char_write(struct file *file, const char __user *buffer,
 	if (copy_from_user(&cmd, buffer, sizeof(cmd)))
 		return -EFAULT;
 
-	ret = mutex_lock_interruptible(&userio->lock);
-	if (ret)
-		return ret;
+	error = mutex_lock_interruptible(&userio->mutex);
+	if (error)
+		return error;
 
 	switch (cmd.type) {
 	case USERIO_CMD_REGISTER:
@@ -190,14 +201,14 @@ static ssize_t userio_char_write(struct file *file, const char __user *buffer,
 			dev_warn(userio_misc.this_device,
 				 "No port type given on /dev/userio\n");
 
-			ret = -EINVAL;
+			error = -EINVAL;
 			goto out;
 		}
+
 		if (userio->running) {
 			dev_warn(userio_misc.this_device,
 				 "Begin command sent, but we're already running\n");
-
-			ret = -EBUSY;
+			error = -EBUSY;
 			goto out;
 		}
 
@@ -209,8 +220,7 @@ static ssize_t userio_char_write(struct file *file, const char __user *buffer,
 		if (userio->running) {
 			dev_warn(userio_misc.this_device,
 				 "Can't change port type on an already running userio instance\n");
-
-			ret = -EBUSY;
+			error = -EBUSY;
 			goto out;
 		}
 
@@ -221,8 +231,7 @@ static ssize_t userio_char_write(struct file *file, const char __user *buffer,
 		if (!userio->running) {
 			dev_warn(userio_misc.this_device,
 				 "The device must be registered before sending interrupts\n");
-
-			ret = -ENODEV;
+			error = -ENODEV;
 			goto out;
 		}
 
@@ -230,15 +239,13 @@ static ssize_t userio_char_write(struct file *file, const char __user *buffer,
 		break;
 
 	default:
-		ret = -EOPNOTSUPP;
+		error = -EOPNOTSUPP;
 		goto out;
 	}
 
-	ret = sizeof(cmd);
-
 out:
-	mutex_unlock(&userio->lock);
-	return ret;
+	mutex_unlock(&userio->mutex);
+	return error ?: count;
 }
 
 static unsigned int userio_char_poll(struct file *file, poll_table *wait)
@@ -268,6 +275,7 @@ static struct miscdevice userio_misc = {
 	.minor	= USERIO_MINOR,
 	.name	= USERIO_NAME,
 };
+module_driver(userio_misc, misc_register, misc_deregister);
 
 MODULE_ALIAS_MISCDEV(USERIO_MINOR);
 MODULE_ALIAS("devname:" USERIO_NAME);
@@ -275,5 +283,3 @@ MODULE_ALIAS("devname:" USERIO_NAME);
 MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
 MODULE_DESCRIPTION("Virtual Serio Device Support");
 MODULE_LICENSE("GPL");
-
-module_driver(userio_misc, misc_register, misc_deregister);

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v6.2 1/1] Input: Add userio module
  2015-10-24 22:40             ` Dmitry Torokhov
@ 2015-10-26 13:51               ` Lyude
  2015-10-26 23:26                 ` Dmitry Torokhov
  0 siblings, 1 reply; 19+ messages in thread
From: Lyude @ 2015-10-26 13:51 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: David Herrmann, linux-kernel, linux-input, linux-api,
	Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
	Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott,
	Benjamin Tissoires, Hans de Goede, linux-doc

Hi! So, I'm guessing you want me to merge this with the patch I've got
right now and post the new version? (Just making sure since I'm a bit
new to this :)

On Sat, 2015-10-24 at 15:40 -0700, Dmitry Torokhov wrote:
> Hi Stephen,
> 
> On Fri, Oct 23, 2015 at 04:47:46PM -0400, cpaul@redhat.com wrote:
> > From: Stephen Chandler Paul <cpaul@redhat.com>
> > 
> > Debugging input devices, specifically laptop touchpads, can be
> > tricky
> > without having the physical device handy. Here we try to remedy
> > that
> > with userio. This module allows an application to connect to a
> > character
> > device provided by the kernel, and emulate any serio device. In
> > combination with userspace programs that can record PS/2 devices
> > and
> > replay them through the /dev/userio device, this allows developers
> > to
> > debug driver issues on the PS/2 level with devices simply by
> > requesting
> > a recording from the user experiencing the issue without having to
> > have
> > the physical hardware in front of them.
> > 
> > Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
> > Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> > ---
> > 				    Changes
> > * Remove the if (!userio) { return -1; } check that somehow got
> > left in.
> > 
> > Sorry this took so long! I was wondering why you hadn't replied
> > yet, only to
> > notice I only made this change on my own tree and never sent out a
> > response
> > patch. Oops.
> 
> Thank you for making all the changes.
> 
> > +
> > +static ssize_t userio_char_read(struct file *file, char __user
> > *user_buffer,
> > +				size_t count, loff_t *ppos)
> > +{
> > +	struct userio_device *userio = file->private_data;
> > +	int ret;
> > +	size_t nonwrap_len, copylen;
> > +	unsigned char buf[USERIO_BUFSIZE];
> > +	unsigned long flags;
> > +
> > +	if (!count)
> > +		return 0;
> 
> This is not quite right: read of size 0 should still check if there
> is
> data in the buffer and return -EAGAIN for non-blocking descriptors.
> 
> I cooked a patch (below) that should adjust the read behavior (_+ a
> coupe of formatting changes), please take a look.
> 
> Thanks!
> 
-- 
Cheers,
	Lyude


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6.2 1/1] Input: Add userio module
  2015-10-26 13:51               ` Lyude
@ 2015-10-26 23:26                 ` Dmitry Torokhov
  2015-10-27 18:10                   ` Lyude
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Torokhov @ 2015-10-26 23:26 UTC (permalink / raw)
  To: Lyude
  Cc: David Herrmann, linux-kernel, linux-input, linux-api,
	Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
	Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott,
	Benjamin Tissoires, Hans de Goede, linux-doc

Hi,

On Mon, Oct 26, 2015 at 09:51:57AM -0400, Lyude wrote:
> Hi! So, I'm guessing you want me to merge this with the patch I've got
> right now and post the new version? (Just making sure since I'm a bit
> new to this :)
> 

If you apply my patch on top of yours and it still works you do not need
to resend anything. If I messed up and you need to fix something ideally
just send me an incremental patch so that I can see where exactly I
screwed up and I'll fold it all together.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6.2 1/1] Input: Add userio module
  2015-10-26 23:26                 ` Dmitry Torokhov
@ 2015-10-27 18:10                   ` Lyude
  2015-10-28  2:01                     ` Dmitry Torokhov
  0 siblings, 1 reply; 19+ messages in thread
From: Lyude @ 2015-10-27 18:10 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: David Herrmann, linux-kernel, linux-input, linux-api,
	Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
	Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott,
	Benjamin Tissoires, Hans de Goede, linux-doc

Gotcha, tested it and took a look at the changes, looks good to me.

On Mon, 2015-10-26 at 16:26 -0700, Dmitry Torokhov wrote:
> Hi,
> 
> On Mon, Oct 26, 2015 at 09:51:57AM -0400, Lyude wrote:
> > Hi! So, I'm guessing you want me to merge this with the patch I've got
> > right now and post the new version? (Just making sure since I'm a bit
> > new to this :)
> > 
> 
> If you apply my patch on top of yours and it still works you do not need
> to resend anything. If I messed up and you need to fix something ideally
> just send me an incremental patch so that I can see where exactly I
> screwed up and I'll fold it all together.
> 
> Thanks.
> 
-- 
Cheers,
	Lyude


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6.2 1/1] Input: Add userio module
  2015-10-27 18:10                   ` Lyude
@ 2015-10-28  2:01                     ` Dmitry Torokhov
  0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Torokhov @ 2015-10-28  2:01 UTC (permalink / raw)
  To: Lyude
  Cc: David Herrmann, linux-kernel, linux-input, linux-api,
	Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
	Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott,
	Benjamin Tissoires, Hans de Goede, linux-doc

On Tue, Oct 27, 2015 at 02:10:42PM -0400, Lyude wrote:
> Gotcha, tested it and took a look at the changes, looks good to me.

Excellent, thank you, applied.

> 
> On Mon, 2015-10-26 at 16:26 -0700, Dmitry Torokhov wrote:
> > Hi,
> > 
> > On Mon, Oct 26, 2015 at 09:51:57AM -0400, Lyude wrote:
> > > Hi! So, I'm guessing you want me to merge this with the patch I've got
> > > right now and post the new version? (Just making sure since I'm a bit
> > > new to this :)
> > > 
> > 
> > If you apply my patch on top of yours and it still works you do not need
> > to resend anything. If I messed up and you need to fix something ideally
> > just send me an incremental patch so that I can see where exactly I
> > screwed up and I'll fold it all together.
> > 
> > Thanks.
> > 
> -- 
> Cheers,
> 	Lyude
> 

-- 
Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-10-28  2:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-17 23:00 [PATCH v5] Input: Add userio module cpaul
2015-09-19 17:39 ` Dmitry Torokhov
2015-09-22 10:59 ` David Herrmann
2015-09-23 17:49   ` [PATCH v6 1/1] " cpaul
2015-09-23 17:54     ` [PATCH v6.1 " cpaul
2015-10-02 17:37       ` Dmitry Torokhov
2015-10-05 14:06         ` Stephen Chandler Paul
2015-10-23 20:47           ` [PATCH v6.2 " cpaul
2015-10-24 22:40             ` Dmitry Torokhov
2015-10-26 13:51               ` Lyude
2015-10-26 23:26                 ` Dmitry Torokhov
2015-10-27 18:10                   ` Lyude
2015-10-28  2:01                     ` Dmitry Torokhov
2015-10-05 15:55         ` [PATCH v7] " cpaul
2015-10-08 17:20           ` David Herrmann
2015-10-09 14:30             ` [PATCH v8] " cpaul
2015-10-09 14:52               ` LABBE Corentin
2015-10-09 14:59                 ` Stephen Chandler Paul
2015-10-24 22:26             ` [PATCH v7] " Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).