| Message ID | 20201007074447.797968-2-hch@lst.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] fbdev/sbuslib: remove FBIOSCURSOR/FBIOGCURSOR leftovers | expand |
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
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>
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.
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
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.
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
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
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; }
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(-)