From patchwork Sat Sep 18 00:23:47 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Hogan X-Patchwork-Id: 65118 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id A753EB70AB for ; Sat, 18 Sep 2010 10:23:57 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752244Ab0IRAXz (ORCPT ); Fri, 17 Sep 2010 20:23:55 -0400 Received: from mail-wy0-f174.google.com ([74.125.82.174]:50904 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750782Ab0IRAXy (ORCPT ); Fri, 17 Sep 2010 20:23:54 -0400 Received: by wyf22 with SMTP id 22so2890917wyf.19 for ; Fri, 17 Sep 2010 17:23:53 -0700 (PDT) Received: by 10.227.152.18 with SMTP id e18mr4918528wbw.1.1284769432909; Fri, 17 Sep 2010 17:23:52 -0700 (PDT) Received: from gandalf.localnet (jahogan.plus.com [212.159.75.221]) by mx.google.com with ESMTPS id o49sm3115177wej.19.2010.09.17.17.23.50 (version=TLSv1/SSLv3 cipher=RC4-MD5); Fri, 17 Sep 2010 17:23:52 -0700 (PDT) To: sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org Subject: [PATCH] fbmem: Fix fb_read, fb_write unaligned accesses. Cc: "David S. Miller" , Dave Airlie , Andrew Morton , Marcin Slusarz , Florian Tobias Schandinat , Denys Vlasenko , Jesse Barnes , James Simmons From: James Hogan Date: Sat, 18 Sep 2010 01:23:47 +0100 MIME-Version: 1.0 Message-Id: <201009180123.48303.james@albanarts.com> Sender: sparclinux-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: sparclinux@vger.kernel.org Apologies for corrupted patch. I'll try again. Comments? I'd also appreciate if somebody familiar with sbus on sparc could check this patch is sane since I know virtually nothing about sbus and am not in a position to compile for sparc, let alone test on it: fb_{read,write} access the framebuffer using lots of fb_{read,write}l's but don't check that the file position is aligned which can cause problems on some architectures which do not support unaligned accesses. Since the operations are essentially memcpy_{from,to}io, new fb_memcpy_{from,to}fb macros have been defined and these are used instead. For Sparc, fb_{read,write} macros use sbus_{read,write}, so this defines new sbus_memcpy_{from,to}io functions the same as memcpy_{from,to}io but using sbus_{read,write}b instead of {read,write}b. Signed-off-by: James Hogan Acked-by: David S. Miller Acked-by: Florian Tobias Schandinat --- arch/sparc/include/asm/io_32.h | 31 +++++++++++++++++++++++++++ arch/sparc/include/asm/io_64.h | 31 +++++++++++++++++++++++++++ drivers/video/fbmem.c | 46 ++++++++++++--------------------------- include/linux/fb.h | 6 +++++ 4 files changed, 82 insertions(+), 32 deletions(-) diff --git a/arch/sparc/include/asm/io_32.h b/arch/sparc/include/asm/io_32.h index 2889574..c2ced21 100644 --- a/arch/sparc/include/asm/io_32.h +++ b/arch/sparc/include/asm/io_32.h @@ -208,6 +208,21 @@ _memset_io(volatile void __iomem *dst, int c, __kernel_size_t n) #define memset_io(d,c,sz) _memset_io(d,c,sz) static inline void +_sbus_memcpy_fromio(void *dst, const volatile void __iomem *src, + __kernel_size_t n) +{ + char *d = dst; + + while (n--) { + char tmp = sbus_readb(src); + *d++ = tmp; + src++; + } +} + +#define sbus_memcpy_fromio(d, s, sz) _sbus_memcpy_fromio(d, s, sz) + +static inline void _memcpy_fromio(void *dst, const volatile void __iomem *src, __kernel_size_t n) { char *d = dst; @@ -222,6 +237,22 @@ _memcpy_fromio(void *dst, const volatile void __iomem *src, __kernel_size_t n) #define memcpy_fromio(d,s,sz) _memcpy_fromio(d,s,sz) static inline void +_sbus_memcpy_toio(volatile void __iomem *dst, const void *src, + __kernel_size_t n) +{ + const char *s = src; + volatile void __iomem *d = dst; + + while (n--) { + char tmp = *s++; + sbus_writeb(tmp, d); + d++; + } +} + +#define sbus_memcpy_toio(d, s, sz) _sbus_memcpy_toio(d, s, sz) + +static inline void _memcpy_toio(volatile void __iomem *dst, const void *src, __kernel_size_t n) { const char *s = src; diff --git a/arch/sparc/include/asm/io_64.h b/arch/sparc/include/asm/io_64.h index 9517d06..9c89654 100644 --- a/arch/sparc/include/asm/io_64.h +++ b/arch/sparc/include/asm/io_64.h @@ -419,6 +419,21 @@ _memset_io(volatile void __iomem *dst, int c, __kernel_size_t n) #define memset_io(d,c,sz) _memset_io(d,c,sz) static inline void +_sbus_memcpy_fromio(void *dst, const volatile void __iomem *src, + __kernel_size_t n) +{ + char *d = dst; + + while (n--) { + char tmp = sbus_readb(src); + *d++ = tmp; + src++; + } +} + +#define sbus_memcpy_fromio(d, s, sz) _sbus_memcpy_fromio(d, s, sz) + +static inline void _memcpy_fromio(void *dst, const volatile void __iomem *src, __kernel_size_t n) { char *d = dst; @@ -433,6 +448,22 @@ _memcpy_fromio(void *dst, const volatile void __iomem *src, __kernel_size_t n) #define memcpy_fromio(d,s,sz) _memcpy_fromio(d,s,sz) static inline void +_sbus_memcpy_toio(volatile void __iomem *dst, const void *src, + __kernel_size_t n) +{ + const char *s = src; + volatile void __iomem *d = dst; + + while (n--) { + char tmp = *s++; + sbus_writeb(tmp, d); + d++; + } +} + +#define sbus_memcpy_toio(d, s, sz) _sbus_memcpy_toio(d, s, sz) + +static inline void _memcpy_toio(volatile void __iomem *dst, const void *src, __kernel_size_t n) { const char *s = src; diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c index b066475..925f25d 100644 --- a/drivers/video/fbmem.c +++ b/drivers/video/fbmem.c @@ -697,9 +697,9 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) struct inode *inode = file->f_path.dentry->d_inode; int fbidx = iminor(inode); struct fb_info *info = registered_fb[fbidx]; - u32 *buffer, *dst; - u32 __iomem *src; - int c, i, cnt = 0, err = 0; + u8 *buffer, *dst; + u8 __iomem *src; + int c, cnt = 0, err = 0; unsigned long total_size; if (!info || ! info->screen_base) @@ -730,7 +730,7 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) if (!buffer) return -ENOMEM; - src = (u32 __iomem *) (info->screen_base + p); + src = (u8 __iomem *) (info->screen_base + p); if (info->fbops->fb_sync) info->fbops->fb_sync(info); @@ -738,17 +738,9 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) while (count) { c = (count > PAGE_SIZE) ? PAGE_SIZE : count; dst = buffer; - for (i = c >> 2; i--; ) - *dst++ = fb_readl(src++); - if (c & 3) { - u8 *dst8 = (u8 *) dst; - u8 __iomem *src8 = (u8 __iomem *) src; - - for (i = c & 3; i--;) - *dst8++ = fb_readb(src8++); - - src = (u32 __iomem *) src8; - } + fb_memcpy_fromfb(dst, src, c); + dst += c; + src += c; if (copy_to_user(buf, buffer, c)) { err = -EFAULT; @@ -772,9 +764,9 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) struct inode *inode = file->f_path.dentry->d_inode; int fbidx = iminor(inode); struct fb_info *info = registered_fb[fbidx]; - u32 *buffer, *src; - u32 __iomem *dst; - int c, i, cnt = 0, err = 0; + u8 *buffer, *src; + u8 __iomem *dst; + int c, cnt = 0, err = 0; unsigned long total_size; if (!info || !info->screen_base) @@ -811,7 +803,7 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) if (!buffer) return -ENOMEM; - dst = (u32 __iomem *) (info->screen_base + p); + dst = (u8 __iomem *) (info->screen_base + p); if (info->fbops->fb_sync) info->fbops->fb_sync(info); @@ -825,19 +817,9 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) break; } - for (i = c >> 2; i--; ) - fb_writel(*src++, dst++); - - if (c & 3) { - u8 *src8 = (u8 *) src; - u8 __iomem *dst8 = (u8 __iomem *) dst; - - for (i = c & 3; i--; ) - fb_writeb(*src8++, dst8++); - - dst = (u32 __iomem *) dst8; - } - + fb_memcpy_tofb(dst, src, c); + dst += c; + src += c; *ppos += c; buf += c; cnt += c; diff --git a/include/linux/fb.h b/include/linux/fb.h index f0268de..7fca3dc 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -931,6 +931,8 @@ static inline struct apertures_struct *alloc_apertures(unsigned int max_num) { #define fb_writel sbus_writel #define fb_writeq sbus_writeq #define fb_memset sbus_memset_io +#define fb_memcpy_fromfb sbus_memcpy_fromio +#define fb_memcpy_tofb sbus_memcpy_toio #elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) || defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || defined(__avr32__) || defined(__bfin__) @@ -943,6 +945,8 @@ static inline struct apertures_struct *alloc_apertures(unsigned int max_num) { #define fb_writel __raw_writel #define fb_writeq __raw_writeq #define fb_memset memset_io +#define fb_memcpy_fromfb memcpy_fromio +#define fb_memcpy_tofb memcpy_toio #else @@ -955,6 +959,8 @@ static inline struct apertures_struct *alloc_apertures(unsigned int max_num) { #define fb_writel(b,addr) (*(volatile u32 *) (addr) = (b)) #define fb_writeq(b,addr) (*(volatile u64 *) (addr) = (b)) #define fb_memset memset +#define fb_memcpy_fromfb memcpy +#define fb_memcpy_tofb memcpy #endif