2019-05-20 08:30:18

by Daniel Vetter

[permalink] [raw]
Subject: [PATCH 31/33] fbcon: Call con2fb_map functions directly

These are actually fbcon ioctls which just happen to be exposed
through /dev/fb*. They completely ignore which fb_info they're called
on, and I think the userspace tool even hardcodes to /dev/fb0.

Hence just forward the entire thing to fbcon.c wholesale.

Note that this patch drops the fb_lock/unlock on the set side. Since
the ioctl can operate on any fb (as passed in through
con2fb.framebuffer) this is bogus. Also note that fbcon.c in general
never calls fb_lock on anything, so this has been badly broken
already.

With this the last user of the fbcon notifier callback is gone, and we
can garbage collect that too.

Signed-off-by: Daniel Vetter <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Bartlomiej Zolnierkiewicz <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: Yisheng Xie <[email protected]>
Cc: "Michał Mirosław" <[email protected]>
Cc: Peter Rosin <[email protected]>
Cc: Mikulas Patocka <[email protected]>
---
drivers/video/fbdev/core/fbcon.c | 62 +++++++++++++++++++-------------
drivers/video/fbdev/core/fbmem.c | 34 ++----------------
include/linux/fbcon.h | 4 +++
3 files changed, 43 insertions(+), 57 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index fd604ffb3c05..b40b56702c61 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -3315,29 +3315,47 @@ void fbcon_get_requirement(struct fb_info *info,
}
}

-static int fbcon_event_notify(struct notifier_block *self,
- unsigned long action, void *data)
-{
- struct fb_event *event = data;
- struct fb_info *info = event->info;
- struct fb_con2fbmap *con2fb;
- int idx, ret = 0;
-
- switch(action) {
- case FB_EVENT_SET_CONSOLE_MAP:
- /* called with console lock held */
- con2fb = event->data;
- ret = set_con2fb_map(con2fb->console - 1,
- con2fb->framebuffer, 1);
- break;
- case FB_EVENT_GET_CONSOLE_MAP:
- con2fb = event->data;
- con2fb->framebuffer = con2fb_map[con2fb->console - 1];
- break;
+int fbcon_set_con2fb_map_ioctl(void __user *argp)
+{
+ struct fb_con2fbmap con2fb;
+ int ret;
+
+ if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
+ return -EFAULT;
+ if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES)
+ return -EINVAL;
+ if (con2fb.framebuffer >= FB_MAX)
+ return -EINVAL;
+ if (!registered_fb[con2fb.framebuffer])
+ request_module("fb%d", con2fb.framebuffer);
+ if (!registered_fb[con2fb.framebuffer]) {
+ return -EINVAL;
}
+
+ console_lock();
+ ret = set_con2fb_map(con2fb.console - 1,
+ con2fb.framebuffer, 1);
+ console_unlock();
+
return ret;
}

+int fbcon_get_con2fb_map_ioctl(void __user *argp)
+{
+ struct fb_con2fbmap con2fb;
+
+ if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
+ return -EFAULT;
+ if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES)
+ return -EINVAL;
+
+ console_lock();
+ con2fb.framebuffer = con2fb_map[con2fb.console - 1];
+ console_unlock();
+
+ return copy_to_user(argp, &con2fb, sizeof(con2fb)) ? -EFAULT : 0;
+}
+
/*
* The console `switch' structure for the frame buffer based console
*/
@@ -3369,10 +3387,6 @@ static const struct consw fb_con = {
.con_debug_leave = fbcon_debug_leave,
};

-static struct notifier_block fbcon_event_notifier = {
- .notifier_call = fbcon_event_notify,
-};
-
static ssize_t store_rotate(struct device *device,
struct device_attribute *attr, const char *buf,
size_t count)
@@ -3645,7 +3659,6 @@ void __init fb_console_init(void)
int i;

console_lock();
- fb_register_client(&fbcon_event_notifier);
fbcon_device = device_create(fb_class, NULL, MKDEV(0, 0), NULL,
"fbcon");

@@ -3681,7 +3694,6 @@ static void __exit fbcon_deinit_device(void)
void __exit fb_console_exit(void)
{
console_lock();
- fb_unregister_client(&fbcon_event_notifier);
fbcon_deinit_device();
device_destroy(fb_class, MKDEV(0, 0));
fbcon_exit();
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 55b88163edc2..c5cf02e68e25 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1084,10 +1084,8 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
struct fb_ops *fb;
struct fb_var_screeninfo var;
struct fb_fix_screeninfo fix;
- struct fb_con2fbmap con2fb;
struct fb_cmap cmap_from;
struct fb_cmap_user cmap;
- struct fb_event event;
void __user *argp = (void __user *)arg;
long ret = 0;

@@ -1149,38 +1147,10 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
ret = -EINVAL;
break;
case FBIOGET_CON2FBMAP:
- if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
- return -EFAULT;
- if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES)
- return -EINVAL;
- con2fb.framebuffer = -1;
- event.data = &con2fb;
- lock_fb_info(info);
- event.info = info;
- fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, &event);
- unlock_fb_info(info);
- ret = copy_to_user(argp, &con2fb, sizeof(con2fb)) ? -EFAULT : 0;
+ ret = fbcon_get_con2fb_map_ioctl(argp);
break;
case FBIOPUT_CON2FBMAP:
- if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
- return -EFAULT;
- if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES)
- return -EINVAL;
- if (con2fb.framebuffer >= FB_MAX)
- return -EINVAL;
- if (!registered_fb[con2fb.framebuffer])
- request_module("fb%d", con2fb.framebuffer);
- if (!registered_fb[con2fb.framebuffer]) {
- ret = -EINVAL;
- break;
- }
- event.data = &con2fb;
- console_lock();
- lock_fb_info(info);
- event.info = info;
- ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event);
- unlock_fb_info(info);
- console_unlock();
+ ret = fbcon_set_con2fb_map_ioctl(argp);
break;
case FBIOBLANK:
console_lock();
diff --git a/include/linux/fbcon.h b/include/linux/fbcon.h
index 3f854e803746..8dfd1aa40483 100644
--- a/include/linux/fbcon.h
+++ b/include/linux/fbcon.h
@@ -17,6 +17,8 @@ void fbcon_get_requirement(struct fb_info *info,
void fbcon_fb_blanked(struct fb_info *info, int blank);
void fbcon_update_vcs(struct fb_info *info, bool all);
void fbcon_remap_all(struct fb_info *info);
+int fbcon_set_con2fb_map_ioctl(void __user *argp);
+int fbcon_get_con2fb_map_ioctl(void __user *argp);
#else
static inline void fb_console_init(void) {}
static inline void fb_console_exit(void) {}
@@ -33,6 +35,8 @@ void fbcon_get_requirement(struct fb_info *info,
void fbcon_fb_blanked(struct fb_info *info, int blank) {}
void fbcon_update_vcs(struct fb_info *info, bool all) {}
void fbcon_remap_all(struct fb_info *info) {}
+int fbcon_set_con2fb_map_ioctl(void __user *argp) { return 0; }
+int fbcon_get_con2fb_map_ioctl(void __user *argp) { return 0; }
#endif

#endif /* _LINUX_FBCON_H */
--
2.20.1



2019-05-20 19:31:34

by kernel test robot

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH 31/33] fbcon: Call con2fb_map functions directly

Hi Daniel,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.2-rc1 next-20190520]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/fbcon-notifier-begone/20190521-021841
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=sparc64

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/video/fbdev/core/fbcon.c: In function 'fbcon_set_con2fb_map_ioctl':
>> drivers/video/fbdev/core/fbcon.c:3323:6: error: implicit declaration of function 'copy_from_user'; did you mean 'copy_creds'? [-Werror=implicit-function-declaration]
if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
^~~~~~~~~~~~~~
copy_creds
drivers/video/fbdev/core/fbcon.c: In function 'fbcon_get_con2fb_map_ioctl':
>> drivers/video/fbdev/core/fbcon.c:3356:9: error: implicit declaration of function 'copy_to_user'; did you mean 'cpu_to_mem'? [-Werror=implicit-function-declaration]
return copy_to_user(argp, &con2fb, sizeof(con2fb)) ? -EFAULT : 0;
^~~~~~~~~~~~
cpu_to_mem
cc1: some warnings being treated as errors

vim +3323 drivers/video/fbdev/core/fbcon.c

3317
3318 int fbcon_set_con2fb_map_ioctl(void __user *argp)
3319 {
3320 struct fb_con2fbmap con2fb;
3321 int ret;
3322
> 3323 if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
3324 return -EFAULT;
3325 if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES)
3326 return -EINVAL;
3327 if (con2fb.framebuffer >= FB_MAX)
3328 return -EINVAL;
3329 if (!registered_fb[con2fb.framebuffer])
3330 request_module("fb%d", con2fb.framebuffer);
3331 if (!registered_fb[con2fb.framebuffer]) {
3332 return -EINVAL;
3333 }
3334
3335 console_lock();
3336 ret = set_con2fb_map(con2fb.console - 1,
3337 con2fb.framebuffer, 1);
3338 console_unlock();
3339
3340 return ret;
3341 }
3342
3343 int fbcon_get_con2fb_map_ioctl(void __user *argp)
3344 {
3345 struct fb_con2fbmap con2fb;
3346
3347 if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
3348 return -EFAULT;
3349 if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES)
3350 return -EINVAL;
3351
3352 console_lock();
3353 con2fb.framebuffer = con2fb_map[con2fb.console - 1];
3354 console_unlock();
3355
> 3356 return copy_to_user(argp, &con2fb, sizeof(con2fb)) ? -EFAULT : 0;
3357 }
3358

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.07 kB)
.config.gz (57.16 kB)
Download all attachments

2019-05-20 19:36:42

by kernel test robot

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH 31/33] fbcon: Call con2fb_map functions directly

Hi Daniel,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.2-rc1 next-20190520]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/fbcon-notifier-begone/20190521-021841
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=alpha

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/video/fbdev/core/fbcon.c: In function 'fbcon_set_con2fb_map_ioctl':
>> drivers/video/fbdev/core/fbcon.c:3323:6: error: implicit declaration of function 'copy_from_user'; did you mean 'sg_copy_from_buffer'? [-Werror=implicit-function-declaration]
if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
^~~~~~~~~~~~~~
sg_copy_from_buffer
drivers/video/fbdev/core/fbcon.c: In function 'fbcon_get_con2fb_map_ioctl':
drivers/video/fbdev/core/fbcon.c:3356:9: error: implicit declaration of function 'copy_to_user'; did you mean 'cpu_to_mem'? [-Werror=implicit-function-declaration]
return copy_to_user(argp, &con2fb, sizeof(con2fb)) ? -EFAULT : 0;
^~~~~~~~~~~~
cpu_to_mem
cc1: some warnings being treated as errors

vim +3323 drivers/video/fbdev/core/fbcon.c

3317
3318 int fbcon_set_con2fb_map_ioctl(void __user *argp)
3319 {
3320 struct fb_con2fbmap con2fb;
3321 int ret;
3322
> 3323 if (copy_from_user(&con2fb, argp, sizeof(con2fb)))
3324 return -EFAULT;
3325 if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES)
3326 return -EINVAL;
3327 if (con2fb.framebuffer >= FB_MAX)
3328 return -EINVAL;
3329 if (!registered_fb[con2fb.framebuffer])
3330 request_module("fb%d", con2fb.framebuffer);
3331 if (!registered_fb[con2fb.framebuffer]) {
3332 return -EINVAL;
3333 }
3334
3335 console_lock();
3336 ret = set_con2fb_map(con2fb.console - 1,
3337 con2fb.framebuffer, 1);
3338 console_unlock();
3339
3340 return ret;
3341 }
3342

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.57 kB)
.config.gz (56.55 kB)
Download all attachments