All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] Goldfish: partial resync with Google tree
@ 2016-01-06 14:03 Alan
  2016-01-06 14:04 ` [PATCH 1/8] goldfish: refactor goldfish platform configs Alan
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Alan @ 2016-01-06 14:03 UTC (permalink / raw)
  To: greg, linux-kernel

The Goldfish virtual platform has been aligning itself a bit more with 
convenional interfaces. It now uses things like virtio, it uses ACPI in 
preference to a magic goldfish bus interface and slowly gets closer to
upstream qemu.
   
These patches pull relevant changes back from the Goldfish Google codebase
back into upstream.

---

Alex Bennée (1):
      android_pipe: don't be clever with #define offsets

Christoffer Dall (1):
      android_pipe: Pin pages to memory while copying and other cleanups

Greg Hackmann (3):
      goldfish: refactor goldfish platform configs
      platform: goldfish: pipe: add devicetree bindings
      platform: goldfish: pipe: don't log when dropping PIPE_ERROR_AGAIN

Jason Hu (1):
      goldfish: Enable ACPI-based enumeration for android pipe

Miodrag Dinic (1):
      [MIPS] Enable platform support for Goldfish virtual devices

Yu Ning (1):
      goldfish_pipe: Pass physical addresses to the device if supported


 .../devicetree/bindings/goldfish/pipe.txt          |   17 ++
 drivers/platform/Kconfig                           |    3 
 drivers/platform/goldfish/Kconfig                  |   18 ++
 drivers/platform/goldfish/Makefile                 |    2 
 drivers/platform/goldfish/goldfish_pipe.c          |  189 +++++++++++++-------
 5 files changed, 157 insertions(+), 72 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/goldfish/pipe.txt

--
	"CMS is like a porcupine in a balloon factory"
					--Alan Altmark

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

* [PATCH 1/8] goldfish: refactor goldfish platform configs
  2016-01-06 14:03 [PATCH 0/8] Goldfish: partial resync with Google tree Alan
@ 2016-01-06 14:04 ` Alan
  2016-01-06 14:04 ` [PATCH 2/8] android_pipe: don't be clever with #define offsets Alan
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Alan @ 2016-01-06 14:04 UTC (permalink / raw)
  To: greg, linux-kernel

From: Greg Hackmann <ghackmann@google.com>

On new virtual devices, the goldfish virtual bus can be replaced with
autoprobing infrastructure like Device Tree.  Refactor the goldfish
kernel configs to better accommodate this.

Move the goldfish platform into a menuconfig in the style of the chrome
platform, and separate the goldfish bus into its own config option.

Signed-off-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Jin Qian <jinqian@android.com>
[Corrected a tristate to bool]
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/platform/Kconfig           |    3 +--
 drivers/platform/goldfish/Kconfig  |   18 ++++++++++++++++++
 drivers/platform/goldfish/Makefile |    2 +-
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index 0adccbf..c11db8b 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -4,8 +4,7 @@ endif
 if MIPS
 source "drivers/platform/mips/Kconfig"
 endif
-if GOLDFISH
+
 source "drivers/platform/goldfish/Kconfig"
-endif
 
 source "drivers/platform/chrome/Kconfig"
diff --git a/drivers/platform/goldfish/Kconfig b/drivers/platform/goldfish/Kconfig
index 635ef25..2be7627 100644
--- a/drivers/platform/goldfish/Kconfig
+++ b/drivers/platform/goldfish/Kconfig
@@ -1,5 +1,23 @@
+menuconfig GOLDFISH
+	bool "Platform support for Goldfish virtual devices"
+	depends on X86_32 || X86_64 || ARM || ARM64
+	---help---
+	  Say Y here to get to see options for the Goldfish virtual platform.
+	  This option alone does not add any kernel code.
+
+	  Unless you are building for the Android Goldfish emulator say N here.
+
+if GOLDFISH
+
+config GOLDFISH_BUS
+	bool "Goldfish platform bus"
+	---help---
+	  This is a virtual bus to host Goldfish Android Virtual Devices.
+
 config GOLDFISH_PIPE
 	tristate "Goldfish virtual device for QEMU pipes"
 	---help---
 	  This is a virtual device to drive the QEMU pipe interface used by
 	  the Goldfish Android Virtual Device.
+
+endif # GOLDFISH
diff --git a/drivers/platform/goldfish/Makefile b/drivers/platform/goldfish/Makefile
index a002239..d348712 100644
--- a/drivers/platform/goldfish/Makefile
+++ b/drivers/platform/goldfish/Makefile
@@ -1,5 +1,5 @@
 #
 # Makefile for Goldfish platform specific drivers
 #
-obj-$(CONFIG_GOLDFISH)	+=	pdev_bus.o
+obj-$(CONFIG_GOLDFISH_BUS)	+= pdev_bus.o
 obj-$(CONFIG_GOLDFISH_PIPE)	+= goldfish_pipe.o


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

* [PATCH 2/8] android_pipe: don't be clever with #define offsets
  2016-01-06 14:03 [PATCH 0/8] Goldfish: partial resync with Google tree Alan
  2016-01-06 14:04 ` [PATCH 1/8] goldfish: refactor goldfish platform configs Alan
@ 2016-01-06 14:04 ` Alan
  2016-01-06 14:05 ` [PATCH 3/8] android_pipe: Pin pages to memory while copying and other cleanups Alan
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Alan @ 2016-01-06 14:04 UTC (permalink / raw)
  To: greg, linux-kernel

From: Alex Bennée <alex.bennee@linaro.org>

It just makes it harder to figure out which commands are being used.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Jin Qian <jinqian@android.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/platform/goldfish/goldfish_pipe.c |   16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index e7a29e2..0fb3a34 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -90,12 +90,6 @@
 #define CMD_WRITE_BUFFER	4  /* send a user buffer to the emulator */
 #define CMD_WAKE_ON_WRITE	5  /* tell the emulator to wake us when writing
 				     is possible */
-
-/* The following commands are related to read operations, they must be
- * listed in the same order than the corresponding write ones, since we
- * will use (CMD_READ_BUFFER - CMD_WRITE_BUFFER) as a special offset
- * in goldfish_pipe_read_write() below.
- */
 #define CMD_READ_BUFFER        6  /* receive a user buffer from the emulator */
 #define CMD_WAKE_ON_READ       7  /* tell the emulator to wake us when reading
 				   * is possible */
@@ -272,8 +266,6 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 	unsigned long irq_flags;
 	struct goldfish_pipe *pipe = filp->private_data;
 	struct goldfish_pipe_dev *dev = pipe->dev;
-	const int cmd_offset = is_write ? 0
-					: (CMD_READ_BUFFER - CMD_WRITE_BUFFER);
 	unsigned long address, address_end;
 	int ret = 0;
 
@@ -325,7 +317,8 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 
 		/* Now, try to transfer the bytes in the current page */
 		spin_lock_irqsave(&dev->lock, irq_flags);
-		if (access_with_param(dev, CMD_WRITE_BUFFER + cmd_offset,
+		if (access_with_param(dev,
+				is_write ? CMD_WRITE_BUFFER : CMD_READ_BUFFER,
 				address, avail, pipe, &status)) {
 			gf_write_ptr(pipe, dev->base + PIPE_REG_CHANNEL,
 				     dev->base + PIPE_REG_CHANNEL_HIGH);
@@ -333,7 +326,7 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 			gf_write_ptr((void *)address,
 				     dev->base + PIPE_REG_ADDRESS,
 				     dev->base + PIPE_REG_ADDRESS_HIGH);
-			writel(CMD_WRITE_BUFFER + cmd_offset,
+			writel(is_write ? CMD_WRITE_BUFFER : CMD_READ_BUFFER,
 					dev->base + PIPE_REG_COMMAND);
 			status = readl(dev->base + PIPE_REG_STATUS);
 		}
@@ -370,7 +363,8 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 		set_bit(wakeBit, &pipe->flags);
 
 		/* Tell the emulator we're going to wait for a wake event */
-		goldfish_cmd(pipe, CMD_WAKE_ON_WRITE + cmd_offset);
+		goldfish_cmd(pipe,
+			is_write ? CMD_WAKE_ON_WRITE : CMD_WAKE_ON_READ);
 
 		/* Unlock the pipe, then wait for the wake signal */
 		mutex_unlock(&pipe->lock);


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

* [PATCH 3/8] android_pipe: Pin pages to memory while copying and other cleanups
  2016-01-06 14:03 [PATCH 0/8] Goldfish: partial resync with Google tree Alan
  2016-01-06 14:04 ` [PATCH 1/8] goldfish: refactor goldfish platform configs Alan
  2016-01-06 14:04 ` [PATCH 2/8] android_pipe: don't be clever with #define offsets Alan
@ 2016-01-06 14:05 ` Alan
  2016-01-06 14:05 ` [PATCH 4/8] platform: goldfish: pipe: add devicetree bindings Alan
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Alan @ 2016-01-06 14:05 UTC (permalink / raw)
  To: greg, linux-kernel

From: Christoffer Dall <christoffer.dall@linaro.org>

The existing code had a troubling TODO statement concerning the fact
that it just did a check if the page that the QEMU backend was going to
read from / write to was there before the call to the QEMU backend and
then relying on the fact that the page stayed around, even in a
preemptible SMP kernel.  Obviously the page could go away or be
reassigned, and strange things may happen.

Further, writes were not tracked, so any use of COW or KSM-like
features would break completely.  Probably that was never used by adbd
(the only current active user of the pipe), but could prove much more
dangerous for the GPU passthrough mechanism.

Instead, use get_user_pages() as the comment suggested and cleanup the
error path and add the set_page_dirt() call on a successful read
operation.

Also clarify the count used to return from successful read/write calls
and use Linux style commentary in various places of the file.

Note: The "just ignore error and return whatever we read so far" error
handling is really quite horrific.  I cannot change it without a more
careful study of all user space ABIs reliance on this 'feature'.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Jin Qian <jinqian@android.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/platform/goldfish/goldfish_pipe.c |  129 ++++++++++++++++-------------
 1 file changed, 72 insertions(+), 57 deletions(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index 0fb3a34..20a9337 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -2,6 +2,7 @@
  * Copyright (C) 2011 Google, Inc.
  * Copyright (C) 2012 Intel, Inc.
  * Copyright (C) 2013 Intel, Inc.
+ * Copyright (C) 2014 Linaro Limited
  *
  * This software is licensed under the terms of the GNU General Public
  * License version 2, as published by the Free Software Foundation, and
@@ -57,6 +58,7 @@
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/goldfish.h>
+#include <linux/mm.h>
 
 /*
  * IMPORTANT: The following constants must match the ones used and defined
@@ -257,17 +259,14 @@ static int access_with_param(struct goldfish_pipe_dev *dev, const int cmd,
 	return 0;
 }
 
-/* This function is used for both reading from and writing to a given
- * pipe.
- */
 static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
-				    size_t bufflen, int is_write)
+				       size_t bufflen, int is_write)
 {
 	unsigned long irq_flags;
 	struct goldfish_pipe *pipe = filp->private_data;
 	struct goldfish_pipe_dev *dev = pipe->dev;
 	unsigned long address, address_end;
-	int ret = 0;
+	int count = 0, ret = -EINVAL;
 
 	/* If the emulator already closed the pipe, no need to go further */
 	if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags))
@@ -290,30 +289,23 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 	address_end = address + bufflen;
 
 	while (address < address_end) {
-		unsigned long  page_end = (address & PAGE_MASK) + PAGE_SIZE;
-		unsigned long  next     = page_end < address_end ? page_end
-								 : address_end;
-		unsigned long  avail    = next - address;
+		unsigned long page_end = (address & PAGE_MASK) + PAGE_SIZE;
+		unsigned long next     = page_end < address_end ? page_end
+								: address_end;
+		unsigned long avail    = next - address;
 		int status, wakeBit;
-
-		/* Ensure that the corresponding page is properly mapped */
-		/* FIXME: this isn't safe or sufficient - use get_user_pages */
-		if (is_write) {
-			char c;
-			/* Ensure that the page is mapped and readable */
-			if (__get_user(c, (char __user *)address)) {
-				if (!ret)
-					ret = -EFAULT;
-				break;
-			}
-		} else {
-			/* Ensure that the page is mapped and writable */
-			if (__put_user(0, (char __user *)address)) {
-				if (!ret)
-					ret = -EFAULT;
-				break;
-			}
-		}
+		struct page *page;
+
+		/*
+		 * We grab the pages on a page-by-page basis in case user
+		 * space gives us a potentially huge buffer but the read only
+		 * returns a small amount, then there's no need to pin that
+		 * much memory to the process.
+		 */
+		ret = get_user_pages(current, current->mm, address, 1,
+				     !is_write, 0, &page, NULL);
+		if (ret < 0)
+			return ret;
 
 		/* Now, try to transfer the bytes in the current page */
 		spin_lock_irqsave(&dev->lock, irq_flags);
@@ -332,33 +324,48 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 		}
 		spin_unlock_irqrestore(&dev->lock, irq_flags);
 
+		if (status > 0 && !is_write)
+			set_page_dirty(page);
+		put_page(page);
+
 		if (status > 0) { /* Correct transfer */
-			ret += status;
+			count += status;
 			address += status;
 			continue;
-		}
-
-		if (status == 0)  /* EOF */
+		} else if (status == 0) { /* EOF */
+			ret = 0;
 			break;
-
-		/* An error occured. If we already transfered stuff, just
-		* return with its count. We expect the next call to return
-		* an error code */
-		if (ret > 0)
+		} else if (status < 0 && count > 0) {
+			/*
+			 * An error occurred and we already transferred
+			 * something on one of the previous pages.
+			 * Just return what we already copied and log this
+			 * err.
+			 *
+			 * Note: This seems like an incorrect approach but
+			 * cannot change it until we check if any user space
+			 * ABI relies on this behavior.
+			 */
+			pr_info_ratelimited("android_pipe: backend returned error %d on %s\n",
+					status, is_write ? "write" : "read");
+			ret = 0;
 			break;
+		}
 
-		/* If the error is not PIPE_ERROR_AGAIN, or if we are not in
-		* non-blocking mode, just return the error code.
-		*/
+		/*
+		 * If the error is not PIPE_ERROR_AGAIN, or if we are not in
+		 * non-blocking mode, just return the error code.
+		 */
 		if (status != PIPE_ERROR_AGAIN ||
 			(filp->f_flags & O_NONBLOCK) != 0) {
 			ret = goldfish_pipe_error_convert(status);
 			break;
 		}
 
-		/* We will have to wait until more data/space is available.
-		* First, mark the pipe as waiting for a specific wake signal.
-		*/
+		/*
+		 * The backend blocked the read/write, wait until the backend
+		 * tells us it's ready to process more data.
+		 */
 		wakeBit = is_write ? BIT_WAKE_ON_WRITE : BIT_WAKE_ON_READ;
 		set_bit(wakeBit, &pipe->flags);
 
@@ -372,22 +379,29 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 		while (test_bit(wakeBit, &pipe->flags)) {
 			if (wait_event_interruptible(
 					pipe->wake_queue,
-					!test_bit(wakeBit, &pipe->flags)))
-				return -ERESTARTSYS;
+					!test_bit(wakeBit, &pipe->flags))) {
+				ret = -ERESTARTSYS;
+				break;
+			}
 
-			if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags))
-				return -EIO;
+			if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags)) {
+				ret = -EIO;
+				break;
+			}
 		}
 
 		/* Try to re-acquire the lock */
-		if (mutex_lock_interruptible(&pipe->lock))
-			return -ERESTARTSYS;
-
-		/* Try the transfer again */
-		continue;
+		if (mutex_lock_interruptible(&pipe->lock)) {
+			ret = -ERESTARTSYS;
+			break;
+		}
 	}
 	mutex_unlock(&pipe->lock);
-	return ret;
+
+	if (ret < 0)
+		return ret;
+	else
+		return count;
 }
 
 static ssize_t goldfish_pipe_read(struct file *filp, char __user *buffer,
@@ -440,10 +454,11 @@ static irqreturn_t goldfish_pipe_interrupt(int irq, void *dev_id)
 	unsigned long irq_flags;
 	int count = 0;
 
-	/* We're going to read from the emulator a list of (channel,flags)
-	* pairs corresponding to the wake events that occured on each
-	* blocked pipe (i.e. channel).
-	*/
+	/*
+	 * We're going to read from the emulator a list of (channel,flags)
+	 * pairs corresponding to the wake events that occurred on each
+	 * blocked pipe (i.e. channel).
+	 */
 	spin_lock_irqsave(&dev->lock, irq_flags);
 	for (;;) {
 		/* First read the channel, 0 means the end of the list */


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

* [PATCH 4/8] platform: goldfish: pipe: add devicetree bindings
  2016-01-06 14:03 [PATCH 0/8] Goldfish: partial resync with Google tree Alan
                   ` (2 preceding siblings ...)
  2016-01-06 14:05 ` [PATCH 3/8] android_pipe: Pin pages to memory while copying and other cleanups Alan
@ 2016-01-06 14:05 ` Alan
  2016-01-06 14:05 ` [PATCH 5/8] platform: goldfish: pipe: don't log when dropping PIPE_ERROR_AGAIN Alan
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Alan @ 2016-01-06 14:05 UTC (permalink / raw)
  To: greg, linux-kernel

From: Greg Hackmann <ghackmann@google.com>

Add bindings so we don't need to rely on goldfish virtual bus for
probing any more, which means we don't need ARM and MIPS goldfish
board code for instantiating the bus.

In the long term we would like to move towards replacing the Android
pipe with virtio-vsock that is currently under development.

Signed-off-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Jin Qian <jinqian@android.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 .../devicetree/bindings/goldfish/pipe.txt          |   17 +++++++++++++++++
 drivers/platform/goldfish/goldfish_pipe.c          |   10 +++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/goldfish/pipe.txt

diff --git a/Documentation/devicetree/bindings/goldfish/pipe.txt b/Documentation/devicetree/bindings/goldfish/pipe.txt
new file mode 100644
index 0000000..e417a31
--- /dev/null
+++ b/Documentation/devicetree/bindings/goldfish/pipe.txt
@@ -0,0 +1,17 @@
+Android Goldfish QEMU Pipe
+
+Andorid pipe virtual device generated by android emulator.
+
+Required properties:
+
+- compatible : should contain "google,android-pipe" to match emulator
+- reg        : <registers mapping>
+- interrupts : <interrupt mapping>
+
+Example:
+
+	android_pipe@a010000 {
+		compatible = "google,android-pipe";
+		reg = <ff018000 0x2000>;
+		interrupts = <0x12>;
+	};
diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index 20a9337..0b187ff 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -624,11 +624,19 @@ static int goldfish_pipe_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id goldfish_pipe_of_match[] = {
+	{ .compatible = "google,android-pipe", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, goldfish_pipe_of_match);
+
 static struct platform_driver goldfish_pipe = {
 	.probe = goldfish_pipe_probe,
 	.remove = goldfish_pipe_remove,
 	.driver = {
-		.name = "goldfish_pipe"
+		.name = "goldfish_pipe",
+		.owner = THIS_MODULE,
+		.of_match_table = goldfish_pipe_of_match,
 	}
 };
 


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

* [PATCH 5/8] platform: goldfish: pipe: don't log when dropping PIPE_ERROR_AGAIN
  2016-01-06 14:03 [PATCH 0/8] Goldfish: partial resync with Google tree Alan
                   ` (3 preceding siblings ...)
  2016-01-06 14:05 ` [PATCH 4/8] platform: goldfish: pipe: add devicetree bindings Alan
@ 2016-01-06 14:05 ` Alan
  2016-01-06 14:06 ` [PATCH 6/8] [MIPS] Enable platform support for Goldfish virtual devices Alan
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Alan @ 2016-01-06 14:05 UTC (permalink / raw)
  To: greg, linux-kernel

From: Greg Hackmann <ghackmann@google.com>

On PIPE_ERROR_AGAIN, just stopping in the middle of a transfer and
returning the number of bytes actually handled is the right behavior.

Other errors should be returned on the next read() or write() call.
Continue logging those until we confirm nothing actually relies on the
existing (wrong) behavior of dropping errors on the floor.

Signed-off-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Jin Qian <jinqian@android.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/platform/goldfish/goldfish_pipe.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index 0b187ff..7a56be9 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -346,7 +346,8 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 			 * cannot change it until we check if any user space
 			 * ABI relies on this behavior.
 			 */
-			pr_info_ratelimited("android_pipe: backend returned error %d on %s\n",
+			if (status != PIPE_ERROR_AGAIN)
+				pr_info_ratelimited("goldfish_pipe: backend returned error %d on %s\n",
 					status, is_write ? "write" : "read");
 			ret = 0;
 			break;


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

* [PATCH 6/8] [MIPS] Enable platform support for Goldfish virtual devices
  2016-01-06 14:03 [PATCH 0/8] Goldfish: partial resync with Google tree Alan
                   ` (4 preceding siblings ...)
  2016-01-06 14:05 ` [PATCH 5/8] platform: goldfish: pipe: don't log when dropping PIPE_ERROR_AGAIN Alan
@ 2016-01-06 14:06 ` Alan
  2016-01-06 14:06 ` [PATCH 7/8] goldfish_pipe: Pass physical addresses to the device if supported Alan
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Alan @ 2016-01-06 14:06 UTC (permalink / raw)
  To: greg, linux-kernel

From: Miodrag Dinic <miodrag.dinic@imgtec.com>

Enable CONFIG_GOLDFISH for MIPS platforms.

Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Jin Qian <jinqian@android.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/platform/goldfish/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/goldfish/Kconfig b/drivers/platform/goldfish/Kconfig
index 2be7627..50331e3 100644
--- a/drivers/platform/goldfish/Kconfig
+++ b/drivers/platform/goldfish/Kconfig
@@ -1,6 +1,6 @@
 menuconfig GOLDFISH
 	bool "Platform support for Goldfish virtual devices"
-	depends on X86_32 || X86_64 || ARM || ARM64
+	depends on X86_32 || X86_64 || ARM || ARM64 || MIPS
 	---help---
 	  Say Y here to get to see options for the Goldfish virtual platform.
 	  This option alone does not add any kernel code.


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

* [PATCH 7/8] goldfish_pipe: Pass physical addresses to the device if supported
  2016-01-06 14:03 [PATCH 0/8] Goldfish: partial resync with Google tree Alan
                   ` (5 preceding siblings ...)
  2016-01-06 14:06 ` [PATCH 6/8] [MIPS] Enable platform support for Goldfish virtual devices Alan
@ 2016-01-06 14:06 ` Alan
  2016-01-06 14:06 ` [PATCH 8/8] goldfish: Enable ACPI-based enumeration for android pipe Alan
  2016-01-25 22:56 ` [PATCH 0/8] Goldfish: partial resync with Google tree Kees Cook
  8 siblings, 0 replies; 12+ messages in thread
From: Alan @ 2016-01-06 14:06 UTC (permalink / raw)
  To: greg, linux-kernel

From: Yu Ning <yu.ning@intel.com>

For reading and writing guest user space buffers, currently the kernel
sends the guest virtual address of the buffer to the pipe device. This
virtual address has to be first converted to a guest physical address.
Doing this translation on the QEMU side is inefficient and requires
additional handling when KVM is enabled, whose implementation would
either incur intrusive changes to QEMU's KVM support code or suffer
from poor performance, see commit 08c7228c50f8 ("x86-kvm: only sync
SREGS when doing address translation") of $AOSP/external/qemu for
details, and thus should be avoided if possible.

There is a TODO comment in hw/misc/android_pipe.c in the new Android
emulator source tree ($AOSP/external/qemu-android) which requests that
the translation be done on the kernel side and that physical addresses
be passed to the device instead of virtual ones. Once the QEMU-side
implementation is done, the kernel will need to support both the new
paddr-based pipe device and the old vaddr-based one (which will
continue to be used by the classic emulator). This patch achieves that
by leveraging the device version register available in the new device.

See https://android-review.googlesource.com/128280 for the QEMU-side
patch.

In addition, use the mmap semaphore (in read mode) to safeguard the
call to get_user_pages().

Signed-off-by: Yu Ning <yu.ning@intel.com>
Signed-off-by: Jin Qian <jinqian@android.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/platform/goldfish/goldfish_pipe.c |   29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index 7a56be9..c214434 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -77,6 +77,7 @@
 #define PIPE_REG_PARAMS_ADDR_LOW	0x18  /* read/write: batch data address */
 #define PIPE_REG_PARAMS_ADDR_HIGH	0x1c  /* read/write: batch data address */
 #define PIPE_REG_ACCESS_PARAMS		0x20  /* write: batch access */
+#define PIPE_REG_VERSION		0x24  /* read: device version */
 
 /* list of commands for PIPE_REG_COMMAND */
 #define CMD_OPEN			1  /* open new channel */
@@ -126,6 +127,7 @@ struct goldfish_pipe_dev {
 	unsigned char __iomem *base;
 	struct access_params *aps;
 	int irq;
+	u32 version;
 };
 
 static struct goldfish_pipe_dev   pipe_dev[1];
@@ -296,26 +298,43 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 		int status, wakeBit;
 		struct page *page;
 
+		/* Either vaddr or paddr depending on the device version */
+		unsigned long xaddr;
+
 		/*
 		 * We grab the pages on a page-by-page basis in case user
 		 * space gives us a potentially huge buffer but the read only
 		 * returns a small amount, then there's no need to pin that
 		 * much memory to the process.
 		 */
+		down_read(&current->mm->mmap_sem);
 		ret = get_user_pages(current, current->mm, address, 1,
 				     !is_write, 0, &page, NULL);
+		up_read(&current->mm->mmap_sem);
 		if (ret < 0)
 			return ret;
 
+		if (dev->version) {
+			/* Device version 1 or newer (qemu-android) expects the
+			 * physical address.
+			 */
+			xaddr = page_to_phys(page) | (address & ~PAGE_MASK);
+		} else {
+			/* Device version 0 (classic emulator) expects the
+			 * virtual address.
+			 */
+			xaddr = address;
+		}
+
 		/* Now, try to transfer the bytes in the current page */
 		spin_lock_irqsave(&dev->lock, irq_flags);
 		if (access_with_param(dev,
 				is_write ? CMD_WRITE_BUFFER : CMD_READ_BUFFER,
-				address, avail, pipe, &status)) {
+				xaddr, avail, pipe, &status)) {
 			gf_write_ptr(pipe, dev->base + PIPE_REG_CHANNEL,
 				     dev->base + PIPE_REG_CHANNEL_HIGH);
 			writel(avail, dev->base + PIPE_REG_SIZE);
-			gf_write_ptr((void *)address,
+			gf_write_ptr((void *)xaddr,
 				     dev->base + PIPE_REG_ADDRESS,
 				     dev->base + PIPE_REG_ADDRESS_HIGH);
 			writel(is_write ? CMD_WRITE_BUFFER : CMD_READ_BUFFER,
@@ -610,6 +629,12 @@ static int goldfish_pipe_probe(struct platform_device *pdev)
 		goto error;
 	}
 	setup_access_params_addr(pdev, dev);
+
+	/* Although the pipe device in the classic Android emulator does not
+	 * recognize the 'version' register, it won't treat this as an error
+	 * either and will simply return 0, which is fine.
+	 */
+	dev->version = readl(dev->base + PIPE_REG_VERSION);
 	return 0;
 
 error:


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

* [PATCH 8/8] goldfish: Enable ACPI-based enumeration for android pipe
  2016-01-06 14:03 [PATCH 0/8] Goldfish: partial resync with Google tree Alan
                   ` (6 preceding siblings ...)
  2016-01-06 14:06 ` [PATCH 7/8] goldfish_pipe: Pass physical addresses to the device if supported Alan
@ 2016-01-06 14:06 ` Alan
  2016-01-25 22:56 ` [PATCH 0/8] Goldfish: partial resync with Google tree Kees Cook
  8 siblings, 0 replies; 12+ messages in thread
From: Alan @ 2016-01-06 14:06 UTC (permalink / raw)
  To: greg, linux-kernel

From: Jason Hu <jia-cheng.hu@intel.com>

Add ACPI binding to the android pipe driver

Signed-off-by: Jason Hu <jia-cheng.hu@intel.com>
Signed-off-by: Jin Qian <jinqian@android.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
 drivers/platform/goldfish/goldfish_pipe.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index c214434..e3fab9a 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -59,6 +59,7 @@
 #include <linux/io.h>
 #include <linux/goldfish.h>
 #include <linux/mm.h>
+#include <linux/acpi.h>
 
 /*
  * IMPORTANT: The following constants must match the ones used and defined
@@ -650,6 +651,12 @@ static int goldfish_pipe_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct acpi_device_id goldfish_pipe_acpi_match[] = {
+	{ "GFSH0003", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, goldfish_pipe_acpi_match);
+
 static const struct of_device_id goldfish_pipe_of_match[] = {
 	{ .compatible = "google,android-pipe", },
 	{},
@@ -663,6 +670,7 @@ static struct platform_driver goldfish_pipe = {
 		.name = "goldfish_pipe",
 		.owner = THIS_MODULE,
 		.of_match_table = goldfish_pipe_of_match,
+		.acpi_match_table = ACPI_PTR(goldfish_pipe_acpi_match),
 	}
 };
 


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

* Re: [PATCH 0/8] Goldfish: partial resync with Google tree
  2016-01-06 14:03 [PATCH 0/8] Goldfish: partial resync with Google tree Alan
                   ` (7 preceding siblings ...)
  2016-01-06 14:06 ` [PATCH 8/8] goldfish: Enable ACPI-based enumeration for android pipe Alan
@ 2016-01-25 22:56 ` Kees Cook
  2016-01-25 23:55   ` Greg Kroah-Hartman
  8 siblings, 1 reply; 12+ messages in thread
From: Kees Cook @ 2016-01-25 22:56 UTC (permalink / raw)
  To: Alan; +Cc: Greg Kroah-Hartman, LKML

On Wed, Jan 6, 2016 at 6:03 AM, Alan <gnomes@lxorguk.ukuu.org.uk> wrote:
> The Goldfish virtual platform has been aligning itself a bit more with
> convenional interfaces. It now uses things like virtio, it uses ACPI in
> preference to a magic goldfish bus interface and slowly gets closer to
> upstream qemu.
>
> These patches pull relevant changes back from the Goldfish Google codebase
> back into upstream.

Consider the series:

Reviewed-by: Kees Cook <keescook@chromium.org>

Greg, can you pull these in? It reduces the Android delta. :)

-Kees

>
> ---
>
> Alex Bennée (1):
>       android_pipe: don't be clever with #define offsets
>
> Christoffer Dall (1):
>       android_pipe: Pin pages to memory while copying and other cleanups
>
> Greg Hackmann (3):
>       goldfish: refactor goldfish platform configs
>       platform: goldfish: pipe: add devicetree bindings
>       platform: goldfish: pipe: don't log when dropping PIPE_ERROR_AGAIN
>
> Jason Hu (1):
>       goldfish: Enable ACPI-based enumeration for android pipe
>
> Miodrag Dinic (1):
>       [MIPS] Enable platform support for Goldfish virtual devices
>
> Yu Ning (1):
>       goldfish_pipe: Pass physical addresses to the device if supported
>
>
>  .../devicetree/bindings/goldfish/pipe.txt          |   17 ++
>  drivers/platform/Kconfig                           |    3
>  drivers/platform/goldfish/Kconfig                  |   18 ++
>  drivers/platform/goldfish/Makefile                 |    2
>  drivers/platform/goldfish/goldfish_pipe.c          |  189 +++++++++++++-------
>  5 files changed, 157 insertions(+), 72 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/goldfish/pipe.txt
>
> --
>         "CMS is like a porcupine in a balloon factory"
>                                         --Alan Altmark
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Kees Cook
Chrome OS & Brillo Security

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

* Re: [PATCH 0/8] Goldfish: partial resync with Google tree
  2016-01-25 22:56 ` [PATCH 0/8] Goldfish: partial resync with Google tree Kees Cook
@ 2016-01-25 23:55   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2016-01-25 23:55 UTC (permalink / raw)
  To: Kees Cook; +Cc: Alan, LKML

On Mon, Jan 25, 2016 at 02:56:37PM -0800, Kees Cook wrote:
> On Wed, Jan 6, 2016 at 6:03 AM, Alan <gnomes@lxorguk.ukuu.org.uk> wrote:
> > The Goldfish virtual platform has been aligning itself a bit more with
> > convenional interfaces. It now uses things like virtio, it uses ACPI in
> > preference to a magic goldfish bus interface and slowly gets closer to
> > upstream qemu.
> >
> > These patches pull relevant changes back from the Goldfish Google codebase
> > back into upstream.
> 
> Consider the series:
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 
> Greg, can you pull these in? It reduces the Android delta. :)

Yes, will do, they are part of my patch queue to dig through this week.
USB is finished, many more to go...

thanks,

greg k-h

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

* [PATCH 5/8] platform: goldfish: pipe: don't log when dropping PIPE_ERROR_AGAIN
       [not found] <1448413812-24289-1-git-send-email-jinqian@android.com>
@ 2015-11-25  1:10 ` Jin Qian
  0 siblings, 0 replies; 12+ messages in thread
From: Jin Qian @ 2015-11-25  1:10 UTC (permalink / raw)
  To: Greg Hackmann, Greg Kroah-Hartman, Alex Bennée, Yu Ning,
	Dan Carpenter, Jason Hu, Joe Perches, Christoffer Dall,
	Peter Senna Tschudin, linux-kernel
  Cc: Jin Qian

From: Greg Hackmann <ghackmann@google.com>

On PIPE_ERROR_AGAIN, just stopping in the middle of a transfer and
returning the number of bytes actually handled is the right behavior.

Other errors should be returned on the next read() or write() call.
Continue logging those until we confirm nothing actually relies on the
existing (wrong) behavior of dropping errors on the floor.

Signed-off-by: Greg Hackmann <ghackmann@google.com>
(cherry picked from commit 1bebc767c41766fc00787473e364db83d5fe6989)
Signed-off-by: Jin Qian <jinqian@android.com>
---
 drivers/platform/goldfish/goldfish_pipe.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index 86cc57f..afc6f8d 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -346,7 +346,8 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
 			 * cannot change it until we check if any user space
 			 * ABI relies on this behavior.
 			 */
-			pr_info_ratelimited("android_pipe: backend returned error %d on %s\n",
+			if (status != PIPE_ERROR_AGAIN)
+				pr_info_ratelimited("goldfish_pipe: backend returned error %d on %s\n",
 					status, is_write ? "write" : "read");
 			ret = 0;
 			break;
-- 
2.6.0.rc2.230.g3dd15c0


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

end of thread, other threads:[~2016-01-25 23:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-06 14:03 [PATCH 0/8] Goldfish: partial resync with Google tree Alan
2016-01-06 14:04 ` [PATCH 1/8] goldfish: refactor goldfish platform configs Alan
2016-01-06 14:04 ` [PATCH 2/8] android_pipe: don't be clever with #define offsets Alan
2016-01-06 14:05 ` [PATCH 3/8] android_pipe: Pin pages to memory while copying and other cleanups Alan
2016-01-06 14:05 ` [PATCH 4/8] platform: goldfish: pipe: add devicetree bindings Alan
2016-01-06 14:05 ` [PATCH 5/8] platform: goldfish: pipe: don't log when dropping PIPE_ERROR_AGAIN Alan
2016-01-06 14:06 ` [PATCH 6/8] [MIPS] Enable platform support for Goldfish virtual devices Alan
2016-01-06 14:06 ` [PATCH 7/8] goldfish_pipe: Pass physical addresses to the device if supported Alan
2016-01-06 14:06 ` [PATCH 8/8] goldfish: Enable ACPI-based enumeration for android pipe Alan
2016-01-25 22:56 ` [PATCH 0/8] Goldfish: partial resync with Google tree Kees Cook
2016-01-25 23:55   ` Greg Kroah-Hartman
     [not found] <1448413812-24289-1-git-send-email-jinqian@android.com>
2015-11-25  1:10 ` [PATCH 5/8] platform: goldfish: pipe: don't log when dropping PIPE_ERROR_AGAIN Jin Qian

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.