linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* fbdev sbuslib cleanups
@ 2020-10-07  7:44 Christoph Hellwig
  2020-10-07  7:44 ` [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers Christoph Hellwig
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Christoph Hellwig @ 2020-10-07  7:44 UTC (permalink / raw)
  To: Geert Uytterhoeven, David S. Miller, Bartlomiej Zolnierkiewicz
  Cc: Arnd Bergmann, linux-m68k, sparclinux, linux-fbdev

Hi all,

this series cleans up the fbdev sbuslib support, most importantly by
removing usage of the deprecated compat_alloc_user_space API.

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

* [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers
  2020-10-07  7:44 fbdev sbuslib cleanups Christoph Hellwig
@ 2020-10-07  7:44 ` Christoph Hellwig
  2020-10-07  8:05   ` Geert Uytterhoeven
  2020-10-07  8:54   ` Arnd Bergmann
  2020-10-07  7:44 ` [PATCH 2/3] fbdev/sbuslib: refactor sbusfb_ioctl_helper Christoph Hellwig
  2020-10-07  7:44 ` [PATCH 3/3] fbdev/sbuslib: avoid compat_alloc_user_space in fbiogetputcmap Christoph Hellwig
  2 siblings, 2 replies; 12+ messages in thread
From: Christoph Hellwig @ 2020-10-07  7:44 UTC (permalink / raw)
  To: Geert Uytterhoeven, David S. Miller, Bartlomiej Zolnierkiewicz
  Cc: Arnd Bergmann, linux-m68k, sparclinux, linux-fbdev

There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
don't bother with a compat handler for it, and remove the remaining
definitions as well.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/m68k/include/asm/fbio.h       | 29 -----------------------------
 arch/sparc/include/asm/fbio.h      | 13 -------------
 arch/sparc/include/uapi/asm/fbio.h | 15 ---------------
 drivers/video/fbdev/sbuslib.c      | 30 ------------------------------
 4 files changed, 87 deletions(-)

diff --git a/arch/m68k/include/asm/fbio.h b/arch/m68k/include/asm/fbio.h
index 590b923c470d3e..90ae409c6bdb4e 100644
--- a/arch/m68k/include/asm/fbio.h
+++ b/arch/m68k/include/asm/fbio.h
@@ -97,21 +97,6 @@ struct fbgattr {
 #define FBIOSVIDEO _IOW('F', 7, int)
 #define FBIOGVIDEO _IOR('F', 8, int)
 
-struct fbcursor {
-        short set;              /* what to set, choose from the list above */
-        short enable;           /* cursor on/off */
-        struct fbcurpos pos;    /* cursor position */
-        struct fbcurpos hot;    /* cursor hot spot */
-        struct fbcmap cmap;     /* color map info */
-        struct fbcurpos size;   /* cursor bit map size */
-        char __user *image;     /* cursor image bits */
-        char __user *mask;      /* cursor mask bits */
-};
-
-/* set/get cursor attributes/shape */
-#define FBIOSCURSOR     _IOW('F', 24, struct fbcursor)
-#define FBIOGCURSOR     _IOWR('F', 25, struct fbcursor)
- 
 /* set/get cursor position */
 #define FBIOSCURPOS     _IOW('F', 26, struct fbcurpos)
 #define FBIOGCURPOS     _IOW('F', 27, struct fbcurpos)
@@ -312,20 +297,6 @@ struct  fbcmap32 {
 
 #define FBIOPUTCMAP32	_IOW('F', 3, struct fbcmap32)
 #define FBIOGETCMAP32	_IOW('F', 4, struct fbcmap32)
-
-struct fbcursor32 {
-	short set;		/* what to set, choose from the list above */
-	short enable;		/* cursor on/off */
-	struct fbcurpos pos;	/* cursor position */
-	struct fbcurpos hot;	/* cursor hot spot */
-	struct fbcmap32 cmap;	/* color map info */
-	struct fbcurpos size;	/* cursor bit map size */
-	u32	image;		/* cursor image bits */
-	u32	mask;		/* cursor mask bits */
-};
-
-#define FBIOSCURSOR32	_IOW('F', 24, struct fbcursor32)
-#define FBIOGCURSOR32	_IOW('F', 25, struct fbcursor32)
 #endif
 
 #endif /* __LINUX_FBIO_H */
diff --git a/arch/sparc/include/asm/fbio.h b/arch/sparc/include/asm/fbio.h
index 02654cb95dec57..348994cc142973 100644
--- a/arch/sparc/include/asm/fbio.h
+++ b/arch/sparc/include/asm/fbio.h
@@ -57,17 +57,4 @@ struct  fbcmap32 {
 #define FBIOPUTCMAP32	_IOW('F', 3, struct fbcmap32)
 #define FBIOGETCMAP32	_IOW('F', 4, struct fbcmap32)
 
-struct fbcursor32 {
-	short set;		/* what to set, choose from the list above */
-	short enable;		/* cursor on/off */
-	struct fbcurpos pos;	/* cursor position */
-	struct fbcurpos hot;	/* cursor hot spot */
-	struct fbcmap32 cmap;	/* color map info */
-	struct fbcurpos size;	/* cursor bit map size */
-	u32	image;		/* cursor image bits */
-	u32	mask;		/* cursor mask bits */
-};
-
-#define FBIOSCURSOR32	_IOW('F', 24, struct fbcursor32)
-#define FBIOGCURSOR32	_IOW('F', 25, struct fbcursor32)
 #endif /* __LINUX_FBIO_H */
diff --git a/arch/sparc/include/uapi/asm/fbio.h b/arch/sparc/include/uapi/asm/fbio.h
index 0dafe2c1eab740..bda278c2a7d091 100644
--- a/arch/sparc/include/uapi/asm/fbio.h
+++ b/arch/sparc/include/uapi/asm/fbio.h
@@ -94,21 +94,6 @@ struct fbgattr {
 #define FBIOSVIDEO _IOW('F', 7, int)
 #define FBIOGVIDEO _IOR('F', 8, int)
 
-struct fbcursor {
-        short set;              /* what to set, choose from the list above */
-        short enable;           /* cursor on/off */
-        struct fbcurpos pos;    /* cursor position */
-        struct fbcurpos hot;    /* cursor hot spot */
-        struct fbcmap cmap;     /* color map info */
-        struct fbcurpos size;   /* cursor bit map size */
-        char __user *image;     /* cursor image bits */
-        char __user *mask;      /* cursor mask bits */
-};
-
-/* set/get cursor attributes/shape */
-#define FBIOSCURSOR     _IOW('F', 24, struct fbcursor)
-#define FBIOGCURSOR     _IOWR('F', 25, struct fbcursor)
- 
 /* set/get cursor position */
 #define FBIOSCURPOS     _IOW('F', 26, struct fbcurpos)
 #define FBIOGCURPOS     _IOW('F', 27, struct fbcurpos)
diff --git a/drivers/video/fbdev/sbuslib.c b/drivers/video/fbdev/sbuslib.c
index 01a7110e61a76a..176dbfb5d3efca 100644
--- a/drivers/video/fbdev/sbuslib.c
+++ b/drivers/video/fbdev/sbuslib.c
@@ -214,32 +214,6 @@ static int fbiogetputcmap(struct fb_info *info, unsigned int cmd, unsigned long
 			(unsigned long)p);
 }
 
-static int fbiogscursor(struct fb_info *info, unsigned long arg)
-{
-	struct fbcursor __user *p = compat_alloc_user_space(sizeof(*p));
-	struct fbcursor32 __user *argp =  (void __user *)arg;
-	compat_uptr_t addr;
-	int ret;
-
-	ret = copy_in_user(p, argp,
-			      2 * sizeof (short) + 2 * sizeof(struct fbcurpos));
-	ret |= copy_in_user(&p->size, &argp->size, sizeof(struct fbcurpos));
-	ret |= copy_in_user(&p->cmap, &argp->cmap, 2 * sizeof(int));
-	ret |= get_user(addr, &argp->cmap.red);
-	ret |= put_user(compat_ptr(addr), &p->cmap.red);
-	ret |= get_user(addr, &argp->cmap.green);
-	ret |= put_user(compat_ptr(addr), &p->cmap.green);
-	ret |= get_user(addr, &argp->cmap.blue);
-	ret |= put_user(compat_ptr(addr), &p->cmap.blue);
-	ret |= get_user(addr, &argp->mask);
-	ret |= put_user(compat_ptr(addr), &p->mask);
-	ret |= get_user(addr, &argp->image);
-	ret |= put_user(compat_ptr(addr), &p->image);
-	if (ret)
-		return -EFAULT;
-	return info->fbops->fb_ioctl(info, FBIOSCURSOR, (unsigned long)p);
-}
-
 int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long arg)
 {
 	switch (cmd) {
@@ -248,8 +222,6 @@ int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long ar
 	case FBIOGATTR:
 	case FBIOSVIDEO:
 	case FBIOGVIDEO:
-	case FBIOGCURSOR32:	/* This is not implemented yet.
-				   Later it should be converted... */
 	case FBIOSCURPOS:
 	case FBIOGCURPOS:
 	case FBIOGCURMAX:
@@ -258,8 +230,6 @@ int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long ar
 		return fbiogetputcmap(info, cmd, arg);
 	case FBIOGETCMAP32:
 		return fbiogetputcmap(info, cmd, arg);
-	case FBIOSCURSOR32:
-		return fbiogscursor(info, arg);
 	default:
 		return -ENOIOCTLCMD;
 	}
-- 
2.28.0

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

* [PATCH 2/3] fbdev/sbuslib: refactor sbusfb_ioctl_helper
  2020-10-07  7:44 fbdev sbuslib cleanups Christoph Hellwig
  2020-10-07  7:44 ` [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers Christoph Hellwig
@ 2020-10-07  7:44 ` Christoph Hellwig
  2020-10-07  7:44 ` [PATCH 3/3] fbdev/sbuslib: avoid compat_alloc_user_space in fbiogetputcmap Christoph Hellwig
  2 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2020-10-07  7:44 UTC (permalink / raw)
  To: Geert Uytterhoeven, David S. Miller, Bartlomiej Zolnierkiewicz
  Cc: Arnd Bergmann, linux-m68k, sparclinux, linux-fbdev

Refactor sbusfb_ioctl_helper into a bunch of self-contained helpers,
to prepare for improvements to the compat ioctl handler.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/video/fbdev/sbuslib.c | 157 ++++++++++++++++------------------
 1 file changed, 75 insertions(+), 82 deletions(-)

diff --git a/drivers/video/fbdev/sbuslib.c b/drivers/video/fbdev/sbuslib.c
index 176dbfb5d3efca..1c3bf1cb8dccd7 100644
--- a/drivers/video/fbdev/sbuslib.c
+++ b/drivers/video/fbdev/sbuslib.c
@@ -97,94 +97,87 @@ int sbusfb_mmap_helper(struct sbus_mmap_map *map,
 }
 EXPORT_SYMBOL(sbusfb_mmap_helper);
 
-int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,
-			struct fb_info *info,
-			int type, int fb_depth, unsigned long fb_size)
+static int sbufsfb_ioctl_gtype(struct fbtype __user *f, struct fb_info *info,
+		int type, int fb_depth, unsigned long fb_size)
 {
-	switch(cmd) {
-	case FBIOGTYPE: {
-		struct fbtype __user *f = (struct fbtype __user *) arg;
-
-		if (put_user(type, &f->fb_type) ||
-		    put_user(info->var.yres, &f->fb_height) ||
-		    put_user(info->var.xres, &f->fb_width) ||
-		    put_user(fb_depth, &f->fb_depth) ||
-		    put_user(0, &f->fb_cmsize) ||
-		    put_user(fb_size, &f->fb_cmsize))
-			return -EFAULT;
-		return 0;
-	}
-	case FBIOPUTCMAP_SPARC: {
-		struct fbcmap __user *c = (struct fbcmap __user *) arg;
-		struct fb_cmap cmap;
-		u16 red, green, blue;
-		u8 red8, green8, blue8;
-		unsigned char __user *ured;
-		unsigned char __user *ugreen;
-		unsigned char __user *ublue;
-		unsigned int index, count, i;
-
-		if (get_user(index, &c->index) ||
-		    get_user(count, &c->count) ||
-		    get_user(ured, &c->red) ||
-		    get_user(ugreen, &c->green) ||
-		    get_user(ublue, &c->blue))
+	if (put_user(type, &f->fb_type) ||
+	    put_user(info->var.yres, &f->fb_height) ||
+	    put_user(info->var.xres, &f->fb_width) ||
+	    put_user(fb_depth, &f->fb_depth) ||
+	    put_user(fb_size, &f->fb_cmsize))
+		return -EFAULT;
+	return 0;
+}
+
+static int sbusfb_ioctl_putcmap(struct fbcmap *c, struct fb_info *info)
+{
+	u8 red8, green8, blue8;
+	u16 red, green, blue;
+	struct fb_cmap cmap;
+	unsigned int i;
+	int err;
+
+	cmap.len = 1;
+	cmap.red = &red;
+	cmap.green = &green;
+	cmap.blue = &blue;
+	cmap.transp = NULL;
+	for (i = 0; i < c->count; i++) {
+		if (get_user(red8, c->red + i) ||
+		    get_user(green8, c->green + i) ||
+		    get_user(blue8, c->blue + i))
 			return -EFAULT;
 
-		cmap.len = 1;
-		cmap.red = &red;
-		cmap.green = &green;
-		cmap.blue = &blue;
-		cmap.transp = NULL;
-		for (i = 0; i < count; i++) {
-			int err;
-
-			if (get_user(red8, &ured[i]) ||
-			    get_user(green8, &ugreen[i]) ||
-			    get_user(blue8, &ublue[i]))
-				return -EFAULT;
-
-			red = red8 << 8;
-			green = green8 << 8;
-			blue = blue8 << 8;
-
-			cmap.start = index + i;
-			err = fb_set_cmap(&cmap, info);
-			if (err)
-				return err;
-		}
-		return 0;
+		red = red8 << 8;
+		green = green8 << 8;
+		blue = blue8 << 8;
+
+		cmap.start = c->index + i;
+		err = fb_set_cmap(&cmap, info);
+		if (err)
+			return err;
 	}
-	case FBIOGETCMAP_SPARC: {
-		struct fbcmap __user *c = (struct fbcmap __user *) arg;
-		unsigned char __user *ured;
-		unsigned char __user *ugreen;
-		unsigned char __user *ublue;
-		struct fb_cmap *cmap = &info->cmap;
-		unsigned int index, count, i;
-		u8 red, green, blue;
-
-		if (get_user(index, &c->index) ||
-		    get_user(count, &c->count) ||
-		    get_user(ured, &c->red) ||
-		    get_user(ugreen, &c->green) ||
-		    get_user(ublue, &c->blue))
-			return -EFAULT;
+	return 0;
+}
 
-		if (index > cmap->len || count > cmap->len - index)
-			return -EINVAL;
-
-		for (i = 0; i < count; i++) {
-			red = cmap->red[index + i] >> 8;
-			green = cmap->green[index + i] >> 8;
-			blue = cmap->blue[index + i] >> 8;
-			if (put_user(red, &ured[i]) ||
-			    put_user(green, &ugreen[i]) ||
-			    put_user(blue, &ublue[i]))
-				return -EFAULT;
-		}
-		return 0;
+static int sbusfb_ioctl_getcmap(struct fbcmap *c, struct fb_info *info)
+{
+	unsigned int i;
+
+	if (c->index > info->cmap.len || c->count > info->cmap.len - c->index)
+		return -EINVAL;
+
+	for (i = 0; i < c->count; i++) {
+		u8 red = info->cmap.red[c->index + i] >> 8;
+		u8 green = info->cmap.green[c->index + i] >> 8;
+		u8 blue = info->cmap.blue[c->index + i] >> 8;
+
+		if (put_user(red, c->red + i) ||
+		    put_user(green, c->green + i) ||
+		    put_user(blue, c->blue + i))
+			return -EFAULT;
 	}
+	return 0;
+}
+
+int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,
+			struct fb_info *info,
+			int type, int fb_depth, unsigned long fb_size)
+{
+	void __user *argp = (void __user *)arg;
+	struct fbcmap kc;
+
+	switch (cmd) {
+	case FBIOGTYPE:
+		return sbufsfb_ioctl_gtype(argp, info, type, fb_depth, fb_size);
+	case FBIOPUTCMAP_SPARC:
+		if (copy_from_user(&kc, argp, sizeof(kc)))
+			return -EFAULT;
+		return sbusfb_ioctl_putcmap(&kc, info);
+	case FBIOGETCMAP_SPARC:
+		if (copy_from_user(&kc, argp, sizeof(kc)))
+			return -EFAULT;
+		return sbusfb_ioctl_getcmap(&kc, info);
 	default:
 		return -EINVAL;
 	}
-- 
2.28.0

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

* [PATCH 3/3] fbdev/sbuslib: avoid compat_alloc_user_space in fbiogetputcmap
  2020-10-07  7:44 fbdev sbuslib cleanups Christoph Hellwig
  2020-10-07  7:44 ` [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers Christoph Hellwig
  2020-10-07  7:44 ` [PATCH 2/3] fbdev/sbuslib: refactor sbusfb_ioctl_helper Christoph Hellwig
@ 2020-10-07  7:44 ` Christoph Hellwig
  2020-10-07  9:14   ` Arnd Bergmann
  2 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2020-10-07  7:44 UTC (permalink / raw)
  To: Geert Uytterhoeven, David S. Miller, Bartlomiej Zolnierkiewicz
  Cc: Arnd Bergmann, linux-m68k, sparclinux, linux-fbdev

Rewrite fbiogetputcmap to call the low-level sbusfb_ioctl_{put,get}cmap
helpers directly and thus avoid usage of the deprecated
compat_alloc_user_space API.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/video/fbdev/sbuslib.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/sbuslib.c b/drivers/video/fbdev/sbuslib.c
index 1c3bf1cb8dccd7..ef74e3e6326a06 100644
--- a/drivers/video/fbdev/sbuslib.c
+++ b/drivers/video/fbdev/sbuslib.c
@@ -188,23 +188,23 @@ EXPORT_SYMBOL(sbusfb_ioctl_helper);
 static int fbiogetputcmap(struct fb_info *info, unsigned int cmd, unsigned long arg)
 {
 	struct fbcmap32 __user *argp = (void __user *)arg;
-	struct fbcmap __user *p = compat_alloc_user_space(sizeof(*p));
+	struct fbcmap kc;
 	u32 addr;
 	int ret;
 
-	ret = copy_in_user(p, argp, 2 * sizeof(int));
+	ret = get_user(kc.index, &argp->index);
+	ret |= get_user(kc.count, &argp->count);
 	ret |= get_user(addr, &argp->red);
-	ret |= put_user(compat_ptr(addr), &p->red);
+	kc.red = compat_ptr(addr);
 	ret |= get_user(addr, &argp->green);
-	ret |= put_user(compat_ptr(addr), &p->green);
+	kc.green = compat_ptr(addr);
 	ret |= get_user(addr, &argp->blue);
-	ret |= put_user(compat_ptr(addr), &p->blue);
+	kc.blue = compat_ptr(addr);
 	if (ret)
 		return -EFAULT;
-	return info->fbops->fb_ioctl(info,
-			(cmd = FBIOPUTCMAP32) ?
-			FBIOPUTCMAP_SPARC : FBIOGETCMAP_SPARC,
-			(unsigned long)p);
+	if (cmd = FBIOPUTCMAP32)
+		return sbusfb_ioctl_putcmap(&kc, info);
+	return sbusfb_ioctl_getcmap(&kc, info);
 }
 
 int sbusfb_compat_ioctl(struct fb_info *info, unsigned int cmd, unsigned long arg)
-- 
2.28.0

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

* Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers
  2020-10-07  7:44 ` [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers Christoph Hellwig
@ 2020-10-07  8:05   ` Geert Uytterhoeven
  2020-10-07  8:54   ` Arnd Bergmann
  1 sibling, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2020-10-07  8:05 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: David S. Miller, Bartlomiej Zolnierkiewicz, Arnd Bergmann,
	linux-m68k, sparclinux, Linux Fbdev development list

On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> don't bother with a compat handler for it, and remove the remaining
> definitions as well.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers
  2020-10-07  7:44 ` [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers Christoph Hellwig
  2020-10-07  8:05   ` Geert Uytterhoeven
@ 2020-10-07  8:54   ` Arnd Bergmann
  2020-10-07  8:59     ` Christoph Hellwig
  2020-10-07 15:41     ` Sam Ravnborg
  1 sibling, 2 replies; 12+ messages in thread
From: Arnd Bergmann @ 2020-10-07  8:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Geert Uytterhoeven, David S. Miller, Bartlomiej Zolnierkiewicz,
	linux-m68k, sparclinux, Linux Fbdev development list,
	Sam Ravnborg

On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
>
> There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> don't bother with a compat handler for it, and remove the remaining
> definitions as well.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
what the state is.

My version only removed the compat handling, not the data structures,
so I'm happy to see your version used instead if mine got lost.

Acked-by: Arnd Bergmann <arnd@arndb.de>

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

* Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers
  2020-10-07  8:54   ` Arnd Bergmann
@ 2020-10-07  8:59     ` Christoph Hellwig
  2020-10-07  9:28       ` Arnd Bergmann
  2020-10-07 15:41     ` Sam Ravnborg
  1 sibling, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2020-10-07  8:59 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, Geert Uytterhoeven, David S. Miller,
	Bartlomiej Zolnierkiewicz, linux-m68k, sparclinux,
	Linux Fbdev development list, Sam Ravnborg

On Wed, Oct 07, 2020 at 10:54:19AM +0200, Arnd Bergmann wrote:
> On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> > don't bother with a compat handler for it, and remove the remaining
> > definitions as well.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
> drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
> what the state is.
> 
> My version only removed the compat handling, not the data structures,
> so I'm happy to see your version used instead if mine got lost.

Oh, sorry.  I thought in your summary you decided to give up on
the sbuslib ones.

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

* Re: [PATCH 3/3] fbdev/sbuslib: avoid compat_alloc_user_space in fbiogetputcmap
  2020-10-07  7:44 ` [PATCH 3/3] fbdev/sbuslib: avoid compat_alloc_user_space in fbiogetputcmap Christoph Hellwig
@ 2020-10-07  9:14   ` Arnd Bergmann
  0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2020-10-07  9:14 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Geert Uytterhoeven, David S. Miller, Bartlomiej Zolnierkiewicz,
	linux-m68k, sparclinux, Linux Fbdev development list,
	Sam Ravnborg

On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Rewrite fbiogetputcmap to call the low-level sbusfb_ioctl_{put,get}cmap
> helpers directly and thus avoid usage of the deprecated
> compat_alloc_user_space API.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

This is another one that I had already addressed in a somewhat inferior way.
I attempted the same kind of cleanup that you did in patch 2/3 but failed to
convince myself that I had managed to avoid regressions, so my patch just
copy-pasted the native implementation into the compat handler with minor
changes.

Provided that your preparation patch is correct, this version is clearly better
than mine, but again I don't know the state of that patch after Sam had
said he applied it already.

     Arnd

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

* Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers
  2020-10-07  8:59     ` Christoph Hellwig
@ 2020-10-07  9:28       ` Arnd Bergmann
  2020-10-07 10:40         ` Christoph Hellwig
  0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2020-10-07  9:28 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Geert Uytterhoeven, David S. Miller, Bartlomiej Zolnierkiewicz,
	linux-m68k, sparclinux, Linux Fbdev development list,
	Sam Ravnborg

On Wed, Oct 7, 2020 at 10:59 AM Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Oct 07, 2020 at 10:54:19AM +0200, Arnd Bergmann wrote:
> > On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> > > don't bother with a compat handler for it, and remove the remaining
> > > definitions as well.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> >
> > I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
> > drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
> > what the state is.
> >
> > My version only removed the compat handling, not the data structures,
> > so I'm happy to see your version used instead if mine got lost.
>
> Oh, sorry.  I thought in your summary you decided to give up on
> the sbuslib ones.

Here is what I have pending at the moment for set_fs() and
compat_alloc_user_space():

https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=compat-alloc-user-space-2

The only one I have actually given up on is the atomisp staging driver,
which uses an awful hack, copying the x86 implementation of
copy_in_user()/compat_alloc_user_space() into the driver.

This is based on last week's linux-next, as I plan to resubmit after the
merge window.

     Arnd

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

* Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers
  2020-10-07  9:28       ` Arnd Bergmann
@ 2020-10-07 10:40         ` Christoph Hellwig
  2020-10-07 11:07           ` Arnd Bergmann
  0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2020-10-07 10:40 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, Geert Uytterhoeven, David S. Miller,
	Bartlomiej Zolnierkiewicz, linux-m68k, sparclinux,
	Linux Fbdev development list, Sam Ravnborg

On Wed, Oct 07, 2020 at 11:28:58AM +0200, Arnd Bergmann wrote:
> The only one I have actually given up on is the atomisp staging driver,
> which uses an awful hack, copying the x86 implementation of
> copy_in_user()/compat_alloc_user_space() into the driver.

That is gross.  Just mark the driver as broken for now.  Linus agreed
years ago that we don't need to work around staging drivers.  But
it would still be nice to ping the mainainers, as they often have
better ideas how to solve the problem.

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

* Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers
  2020-10-07 10:40         ` Christoph Hellwig
@ 2020-10-07 11:07           ` Arnd Bergmann
  0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2020-10-07 11:07 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Geert Uytterhoeven, David S. Miller, Bartlomiej Zolnierkiewicz,
	linux-m68k, sparclinux, Linux Fbdev development list,
	Sam Ravnborg

On Wed, Oct 7, 2020 at 12:40 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Oct 07, 2020 at 11:28:58AM +0200, Arnd Bergmann wrote:
> > The only one I have actually given up on is the atomisp staging driver,
> > which uses an awful hack, copying the x86 implementation of
> > copy_in_user()/compat_alloc_user_space() into the driver.
>
> That is gross.  Just mark the driver as broken for now.

Ah, it seems that Hans has already done that in commit 57e6b6f2303e
("media: atomisp_fops.c: disable atomisp_compat_ioctl32"), which
removes the only reference to this code. In this case, I think a one-line
change to stop building that file would be best, then if anyone ever
wants to fix the bug that Hans and Sakari found, they get to do also
deal with replacing compat_alloc_user_space().

I'll send a patch for that right away, the patch I have in my tree was
so evil that I hadn't dared submit that, but it was useful for
compile-testing the compat_alloc_user_space() removal patch.

   Arnd

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

* Re: [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers
  2020-10-07  8:54   ` Arnd Bergmann
  2020-10-07  8:59     ` Christoph Hellwig
@ 2020-10-07 15:41     ` Sam Ravnborg
  1 sibling, 0 replies; 12+ messages in thread
From: Sam Ravnborg @ 2020-10-07 15:41 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, Geert Uytterhoeven, David S. Miller,
	Bartlomiej Zolnierkiewicz, linux-m68k, sparclinux,
	Linux Fbdev development list

Hi Arnd.

On Wed, Oct 07, 2020 at 10:54:19AM +0200, Arnd Bergmann wrote:
> On Wed, Oct 7, 2020 at 9:44 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > There are no actual implementations of FBIOSCURSOR/FBIOGCURSOR left, so
> > don't bother with a compat handler for it, and remove the remaining
> > definitions as well.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> I had submitted a similar patch earlier, and Sam Ravnborg applied it to the
> drm-misc tree, but it doesn't seem to be in linux-next, so I don't know
> what the state is.
> 
> My version only removed the compat handling, not the data structures,
> so I'm happy to see your version used instead if mine got lost.
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>

I think the patches got applied to drm-misc-next *after* the final push
to -next for the current merge window.
So the patches are lingering in drm-misc-next waiting for the upcoming
merge window to comple before they are pushed to linux-next.
So for now the patches are only visible if one pulls drm-misc and checks
out drm-misc-next branch.

Pulling a fresh drm-misc tree just to be 100% sure .......

Yep, patches are there when I pull a fresh tree and mripard just
confirmed on irc that he sees them in his drm-misc-next repo.

I am working with git.freedesktop.org/git/drm/drm-misc.

	Sam

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

end of thread, other threads:[~2020-10-07 15:41 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07  7:44 fbdev sbuslib cleanups Christoph Hellwig
2020-10-07  7:44 ` [PATCH 1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers Christoph Hellwig
2020-10-07  8:05   ` Geert Uytterhoeven
2020-10-07  8:54   ` Arnd Bergmann
2020-10-07  8:59     ` Christoph Hellwig
2020-10-07  9:28       ` Arnd Bergmann
2020-10-07 10:40         ` Christoph Hellwig
2020-10-07 11:07           ` Arnd Bergmann
2020-10-07 15:41     ` Sam Ravnborg
2020-10-07  7:44 ` [PATCH 2/3] fbdev/sbuslib: refactor sbusfb_ioctl_helper Christoph Hellwig
2020-10-07  7:44 ` [PATCH 3/3] fbdev/sbuslib: avoid compat_alloc_user_space in fbiogetputcmap Christoph Hellwig
2020-10-07  9:14   ` Arnd Bergmann

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).