Re: [PATCH] goldfish: fix goldfish_pipe driver BUG
Hi yalin, [auto build test ERROR on staging/staging-testing] [also build test ERROR on v4.3 next-20151106] url: https://github.com/0day-ci/linux/commits/yalin-wang/goldfish-fix-goldfish_pipe-driver-BUG/20151106-235541 config: i386-allmodconfig (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/platform/goldfish/goldfish_pipe.c: In function 'goldfish_pipe_read_write': >> drivers/platform/goldfish/goldfish_pipe.c:340:7: error: implicit declaration >> of function 'get_user_pages_unlocked' [-Werror=implicit-function-declaration] if (get_user_pages_unlocked(current, current->active_mm, ^ >> drivers/platform/goldfish/goldfish_pipe.c:343:36: error: implicit >> declaration of function 'offset_in_page' >> [-Werror=implicit-function-declaration] phys_addr = page_to_phys(page) + offset_in_page(address); ^ >> drivers/platform/goldfish/goldfish_pipe.c:355:3: error: implicit declaration >> of function 'put_page' [-Werror=implicit-function-declaration] put_page(page); ^ cc1: some warnings being treated as errors vim +/get_user_pages_unlocked +340 drivers/platform/goldfish/goldfish_pipe.c 334 if (__put_user(0, (char __user *)address)) { 335 if (!ret) 336 ret = -EFAULT; 337 break; 338 } 339 } > 340 if (get_user_pages_unlocked(current, current->active_mm, 341 address, 1, !is_write, 0, ) != 1) 342 return -EINVAL; > 343 phys_addr = page_to_phys(page) + > offset_in_page(address); 344 /* Now, try to transfer the bytes in the current page */ 345 spin_lock_irqsave(>lock, irq_flags); 346 gf_write_ptr(pipe, dev->base + PIPE_REG_CHANNEL, 347 dev->base + PIPE_REG_CHANNEL_HIGH); 348 writel(avail, dev->base + PIPE_REG_SIZE); 349 gf_write_ptr((void *)phys_addr, dev->base + PIPE_REG_ADDRESS, 350 dev->base + PIPE_REG_ADDRESS_HIGH); 351 writel(CMD_WRITE_BUFFER + cmd_offset, 352 dev->base + PIPE_REG_COMMAND); 353 status = readl(dev->base + PIPE_REG_STATUS); 354 spin_unlock_irqrestore(>lock, irq_flags); > 355 put_page(page); 356 if (status > 0) { /* Correct transfer */ 357 ret += status; 358 address += status; --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
[PATCH] goldfish: fix goldfish_pipe driver BUG
goldfish_pipe_read_write() should pass the buffer's physical address to qemu, so that host can copy access data correctly, currently, the drier write a virtual address into address register, host can not get correct data, then adbd daemon can not work in guest. Also I comment off access_with_param() function, seems not used, we don't need use this function in goldfish_pipe_read_write(). Signed-off-by: yalin wang --- drivers/platform/goldfish/goldfish_pipe.c | 56 +++ 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c index 55b6d7c..bdf6f11 100644 --- a/drivers/platform/goldfish/goldfish_pipe.c +++ b/drivers/platform/goldfish/goldfish_pipe.c @@ -112,16 +112,27 @@ #define PIPE_WAKE_READ (1 << 1) /* pipe can now be read from */ #define PIPE_WAKE_WRITE(1 << 2) /* pipe can now be written to */ +#ifdef CONFIG_64BIT struct access_params { - unsigned long channel; - u32 size; - unsigned long address; - u32 cmd; - u32 result; + uint64_t channel; /* 0x00 */ + uint32_t size; /* 0x08 */ + uint64_t address; /* 0x0c */ + uint32_t cmd; /* 0x14 */ + uint32_t result;/* 0x18 */ /* reserved for future extension */ - u32 flags; + uint32_t flags; /* 0x1c */ }; - +#else +struct access_params { + uint32_t channel; /* 0x00 */ + uint32_t size; /* 0x04 */ + uint32_t address; /* 0x08 */ + uint32_t cmd; /* 0x0c */ + uint32_t result;/* 0x10 */ + /* reserved for future extension */ + uint32_t flags; /* 0x14 */ +}; +#endif /* The global driver data. Holds a reference to the i/o page used to * communicate with the emulator, and a wake queue for blocked tasks * waiting to be awoken. @@ -237,6 +248,7 @@ static int setup_access_params_addr(struct platform_device *pdev, return -1; } +#if 0 /* A value that will not be set by qemu emulator */ #define INITIAL_BATCH_RESULT (0xdeadbeaf) static int access_with_param(struct goldfish_pipe_dev *dev, const int cmd, @@ -263,6 +275,7 @@ static int access_with_param(struct goldfish_pipe_dev *dev, const int cmd, *status = aps->result; return 0; } +#endif /* This function is used for both reading from and writing to a given * pipe. @@ -304,6 +317,8 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer, : address_end; unsigned long avail= next - address; int status, wakeBit; + struct page *page; + phys_addr_t phys_addr; /* Ensure that the corresponding page is properly mapped */ /* FIXME: this isn't safe or sufficient - use get_user_pages */ @@ -323,23 +338,22 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer, break; } } - + if (get_user_pages_unlocked(current, current->active_mm, + address, 1, !is_write, 0, ) != 1) + return -EINVAL; + phys_addr = page_to_phys(page) + offset_in_page(address); /* Now, try to transfer the bytes in the current page */ spin_lock_irqsave(>lock, irq_flags); - if (access_with_param(dev, CMD_WRITE_BUFFER + cmd_offset, - address, avail, pipe, )) { - 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, -dev->base + PIPE_REG_ADDRESS, -dev->base + PIPE_REG_ADDRESS_HIGH); - writel(CMD_WRITE_BUFFER + cmd_offset, - dev->base + PIPE_REG_COMMAND); - status = readl(dev->base + PIPE_REG_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 *)phys_addr, dev->base + PIPE_REG_ADDRESS, +dev->base + PIPE_REG_ADDRESS_HIGH); + writel(CMD_WRITE_BUFFER + cmd_offset, + dev->base + PIPE_REG_COMMAND); + status = readl(dev->base + PIPE_REG_STATUS); spin_unlock_irqrestore(>lock, irq_flags); - + put_page(page); if (status > 0) { /* Correct transfer */ ret += status;
[PATCH] goldfish: fix goldfish_pipe driver BUG
goldfish_pipe_read_write() should pass the buffer's physical address to qemu, so that host can copy access data correctly, currently, the drier write a virtual address into address register, host can not get correct data, then adbd daemon can not work in guest. Also I comment off access_with_param() function, seems not used, we don't need use this function in goldfish_pipe_read_write(). Signed-off-by: yalin wang--- drivers/platform/goldfish/goldfish_pipe.c | 56 +++ 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c index 55b6d7c..bdf6f11 100644 --- a/drivers/platform/goldfish/goldfish_pipe.c +++ b/drivers/platform/goldfish/goldfish_pipe.c @@ -112,16 +112,27 @@ #define PIPE_WAKE_READ (1 << 1) /* pipe can now be read from */ #define PIPE_WAKE_WRITE(1 << 2) /* pipe can now be written to */ +#ifdef CONFIG_64BIT struct access_params { - unsigned long channel; - u32 size; - unsigned long address; - u32 cmd; - u32 result; + uint64_t channel; /* 0x00 */ + uint32_t size; /* 0x08 */ + uint64_t address; /* 0x0c */ + uint32_t cmd; /* 0x14 */ + uint32_t result;/* 0x18 */ /* reserved for future extension */ - u32 flags; + uint32_t flags; /* 0x1c */ }; - +#else +struct access_params { + uint32_t channel; /* 0x00 */ + uint32_t size; /* 0x04 */ + uint32_t address; /* 0x08 */ + uint32_t cmd; /* 0x0c */ + uint32_t result;/* 0x10 */ + /* reserved for future extension */ + uint32_t flags; /* 0x14 */ +}; +#endif /* The global driver data. Holds a reference to the i/o page used to * communicate with the emulator, and a wake queue for blocked tasks * waiting to be awoken. @@ -237,6 +248,7 @@ static int setup_access_params_addr(struct platform_device *pdev, return -1; } +#if 0 /* A value that will not be set by qemu emulator */ #define INITIAL_BATCH_RESULT (0xdeadbeaf) static int access_with_param(struct goldfish_pipe_dev *dev, const int cmd, @@ -263,6 +275,7 @@ static int access_with_param(struct goldfish_pipe_dev *dev, const int cmd, *status = aps->result; return 0; } +#endif /* This function is used for both reading from and writing to a given * pipe. @@ -304,6 +317,8 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer, : address_end; unsigned long avail= next - address; int status, wakeBit; + struct page *page; + phys_addr_t phys_addr; /* Ensure that the corresponding page is properly mapped */ /* FIXME: this isn't safe or sufficient - use get_user_pages */ @@ -323,23 +338,22 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer, break; } } - + if (get_user_pages_unlocked(current, current->active_mm, + address, 1, !is_write, 0, ) != 1) + return -EINVAL; + phys_addr = page_to_phys(page) + offset_in_page(address); /* Now, try to transfer the bytes in the current page */ spin_lock_irqsave(>lock, irq_flags); - if (access_with_param(dev, CMD_WRITE_BUFFER + cmd_offset, - address, avail, pipe, )) { - 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, -dev->base + PIPE_REG_ADDRESS, -dev->base + PIPE_REG_ADDRESS_HIGH); - writel(CMD_WRITE_BUFFER + cmd_offset, - dev->base + PIPE_REG_COMMAND); - status = readl(dev->base + PIPE_REG_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 *)phys_addr, dev->base + PIPE_REG_ADDRESS, +dev->base + PIPE_REG_ADDRESS_HIGH); + writel(CMD_WRITE_BUFFER + cmd_offset, + dev->base + PIPE_REG_COMMAND); + status = readl(dev->base + PIPE_REG_STATUS); spin_unlock_irqrestore(>lock, irq_flags); - + put_page(page); if (status > 0) { /* Correct transfer */ ret +=
Re: [PATCH] goldfish: fix goldfish_pipe driver BUG
Hi yalin, [auto build test ERROR on staging/staging-testing] [also build test ERROR on v4.3 next-20151106] url: https://github.com/0day-ci/linux/commits/yalin-wang/goldfish-fix-goldfish_pipe-driver-BUG/20151106-235541 config: i386-allmodconfig (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/platform/goldfish/goldfish_pipe.c: In function 'goldfish_pipe_read_write': >> drivers/platform/goldfish/goldfish_pipe.c:340:7: error: implicit declaration >> of function 'get_user_pages_unlocked' [-Werror=implicit-function-declaration] if (get_user_pages_unlocked(current, current->active_mm, ^ >> drivers/platform/goldfish/goldfish_pipe.c:343:36: error: implicit >> declaration of function 'offset_in_page' >> [-Werror=implicit-function-declaration] phys_addr = page_to_phys(page) + offset_in_page(address); ^ >> drivers/platform/goldfish/goldfish_pipe.c:355:3: error: implicit declaration >> of function 'put_page' [-Werror=implicit-function-declaration] put_page(page); ^ cc1: some warnings being treated as errors vim +/get_user_pages_unlocked +340 drivers/platform/goldfish/goldfish_pipe.c 334 if (__put_user(0, (char __user *)address)) { 335 if (!ret) 336 ret = -EFAULT; 337 break; 338 } 339 } > 340 if (get_user_pages_unlocked(current, current->active_mm, 341 address, 1, !is_write, 0, ) != 1) 342 return -EINVAL; > 343 phys_addr = page_to_phys(page) + > offset_in_page(address); 344 /* Now, try to transfer the bytes in the current page */ 345 spin_lock_irqsave(>lock, irq_flags); 346 gf_write_ptr(pipe, dev->base + PIPE_REG_CHANNEL, 347 dev->base + PIPE_REG_CHANNEL_HIGH); 348 writel(avail, dev->base + PIPE_REG_SIZE); 349 gf_write_ptr((void *)phys_addr, dev->base + PIPE_REG_ADDRESS, 350 dev->base + PIPE_REG_ADDRESS_HIGH); 351 writel(CMD_WRITE_BUFFER + cmd_offset, 352 dev->base + PIPE_REG_COMMAND); 353 status = readl(dev->base + PIPE_REG_STATUS); 354 spin_unlock_irqrestore(>lock, irq_flags); > 355 put_page(page); 356 if (status > 0) { /* Correct transfer */ 357 ret += status; 358 address += status; --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data