From bd2f348db5033b88b8b81caf58e7bdabf0b6961d Mon Sep 17 00:00:00 2001 From: Greg Hackmann Date: Wed, 6 Jan 2016 14:04:13 +0000 Subject: goldfish: refactor goldfish platform configs 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 Signed-off-by: Jin Qian [Corrected a tristate to bool] Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/platform/goldfish/Kconfig | 18 ++++++++++++++++++ drivers/platform/goldfish/Makefile | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) (limited to 'drivers/platform/goldfish') diff --git a/drivers/platform/goldfish/Kconfig b/drivers/platform/goldfish/Kconfig index 635ef25cc722..2be762743592 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 a0022395eee9..d3487125838c 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 -- cgit v1.2.1 From 23c5ee21c46ba694681e2378e64a475b5c68f02c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 6 Jan 2016 14:04:31 +0000 Subject: goldfish_pipe: don't be clever with #define offsets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It just makes it harder to figure out which commands are being used. Signed-off-by: Alex BennĂ©e Signed-off-by: Jin Qian Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/platform/goldfish/goldfish_pipe.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) (limited to 'drivers/platform/goldfish') diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c index e7a29e2750c6..0fb3a34f5991 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); -- cgit v1.2.1 From 2f3be88237a301ab83b8ba4a0fdf0381bbb3f6ac Mon Sep 17 00:00:00 2001 From: Christoffer Dall Date: Wed, 6 Jan 2016 14:05:15 +0000 Subject: goldfish_pipe: Pin pages to memory while copying and other cleanups 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 Signed-off-by: Jin Qian Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/platform/goldfish/goldfish_pipe.c | 129 +++++++++++++++++------------- 1 file changed, 72 insertions(+), 57 deletions(-) (limited to 'drivers/platform/goldfish') diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c index 0fb3a34f5991..20a933726ac1 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 #include #include +#include /* * 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 */ -- cgit v1.2.1 From 91a18a414185a2b937e2cc8bf0cc32de1863d11a Mon Sep 17 00:00:00 2001 From: Greg Hackmann Date: Wed, 6 Jan 2016 14:05:36 +0000 Subject: platform: goldfish: pipe: add devicetree bindings 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 Signed-off-by: Jin Qian Acked-by: Rob Herring Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/platform/goldfish/goldfish_pipe.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'drivers/platform/goldfish') diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c index 20a933726ac1..0b187ff7a35b 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, } }; -- cgit v1.2.1 From 25dd0f407307c54de5025250ca1dfbd4bdbb2fba Mon Sep 17 00:00:00 2001 From: Greg Hackmann Date: Wed, 6 Jan 2016 14:05:55 +0000 Subject: platform: goldfish: pipe: don't log when dropping PIPE_ERROR_AGAIN 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 Signed-off-by: Jin Qian Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/platform/goldfish/goldfish_pipe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/platform/goldfish') diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c index 0b187ff7a35b..7a56be9c9432 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; -- cgit v1.2.1 From 2e5fc89ac5a0b1460316f19b21c38284949daf38 Mon Sep 17 00:00:00 2001 From: Miodrag Dinic Date: Wed, 6 Jan 2016 14:06:08 +0000 Subject: Enable platform support for Goldfish virtual devices Enable CONFIG_GOLDFISH for MIPS platforms. Signed-off-by: Miodrag Dinic Signed-off-by: Jin Qian Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/platform/goldfish/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/platform/goldfish') diff --git a/drivers/platform/goldfish/Kconfig b/drivers/platform/goldfish/Kconfig index 2be762743592..50331e3e54f3 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. -- cgit v1.2.1 From 4f42071c943977e91e7fda8230e4f85bc3ba117a Mon Sep 17 00:00:00 2001 From: Yu Ning Date: Wed, 6 Jan 2016 14:06:40 +0000 Subject: goldfish_pipe: Pass physical addresses to the device if supported 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 Signed-off-by: Jin Qian Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/platform/goldfish/goldfish_pipe.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) (limited to 'drivers/platform/goldfish') diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c index 7a56be9c9432..c214434e8811 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(¤t->mm->mmap_sem); ret = get_user_pages(current, current->mm, address, 1, !is_write, 0, &page, NULL); + up_read(¤t->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: -- cgit v1.2.1 From d62f324b0ac80c3923ebbf897735c7c24ba887b8 Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Wed, 6 Jan 2016 14:06:55 +0000 Subject: goldfish: Enable ACPI-based enumeration for android pipe Add ACPI binding to the android pipe driver Signed-off-by: Jason Hu Signed-off-by: Jin Qian Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/platform/goldfish/goldfish_pipe.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/platform/goldfish') diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c index c214434e8811..e3fab9a1d9f7 100644 --- a/drivers/platform/goldfish/goldfish_pipe.c +++ b/drivers/platform/goldfish/goldfish_pipe.c @@ -59,6 +59,7 @@ #include #include #include +#include /* * 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), } }; -- cgit v1.2.1 From 5fae054c2b5a7c10475d2e425dfea0c567c1c5dd Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 2 Feb 2016 12:48:09 +0300 Subject: goldfish: locking bugs in goldfish_pipe_read_write() We recently messed up the error handling here. We can return with the pipe->lock held or sometimes we unlock twice by mistake. Fixes: 2f3be88237a3 ('goldfish_pipe: Pin pages to memory while copying and other cleanups') Signed-off-by: Dan Carpenter Reviewed-by: Christoffer Dall Signed-off-by: Greg Kroah-Hartman --- drivers/platform/goldfish/goldfish_pipe.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) (limited to 'drivers/platform/goldfish') diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c index e3fab9a1d9f7..839df4aace76 100644 --- a/drivers/platform/goldfish/goldfish_pipe.c +++ b/drivers/platform/goldfish/goldfish_pipe.c @@ -313,7 +313,7 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer, !is_write, 0, &page, NULL); up_read(¤t->mm->mmap_sem); if (ret < 0) - return ret; + break; if (dev->version) { /* Device version 1 or newer (qemu-android) expects the @@ -400,22 +400,16 @@ 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))) { - ret = -ERESTARTSYS; - break; - } - - if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags)) { - ret = -EIO; - break; - } + !test_bit(wakeBit, &pipe->flags))) + return -ERESTARTSYS; + + if (test_bit(BIT_CLOSED_ON_HOST, &pipe->flags)) + return -EIO; } /* Try to re-acquire the lock */ - if (mutex_lock_interruptible(&pipe->lock)) { - ret = -ERESTARTSYS; - break; - } + if (mutex_lock_interruptible(&pipe->lock)) + return -ERESTARTSYS; } mutex_unlock(&pipe->lock); -- cgit v1.2.1