x86: Do not write to VGA memory space if CONFIG_VGA_CONSOLE is undefined
in [Kernel]
|
From: H. Peter Anvin on 31 Mar 2010 14:40 On 03/31/2010 07:41 AM, Guenter Roeck wrote: > Current early_printk code writes into VGA memory space even > if CONFIG_VGA_CONSOLE is undefined. This can cause problems > if there is no VGA device in the system, especially if the memory > is used by another device. > > Fix problem by redirecting output to early_serial_console > if CONFIG_VGA_CONSOLE is undefined. > > Signed-off-by: Guenter Roeck <guenter.roeck(a)ericsson.com> > > asmlinkage void early_printk(const char *fmt, ...) > @@ -216,7 +224,7 @@ static int __init setup_early_printk(char *buf) > early_serial_init(buf + 4); > early_console_register(&early_serial_console, keep); > } > - if (!strncmp(buf, "vga", 3) && > + if (have_vga_console && !strncmp(buf, "vga", 3) && > boot_params.screen_info.orig_video_isVGA == 1) { > max_xpos = boot_params.screen_info.orig_video_cols; > max_ypos = boot_params.screen_info.orig_video_lines; I'm confused in a big way about how you could end up with a system where: a) there is no VGA; b) VGA memory is used by another device(!!!); c) boot_params.screen_info.orig_video_isVGA == 1? -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 5 Apr 2010 14:50 On 04/05/2010 11:10 AM, Guenter Roeck wrote: > On Wed, 2010-03-31 at 11:32 -0400, Pekka Enberg wrote: >> On Wed, Mar 31, 2010 at 4:41 PM, Guenter Roeck >> <guenter.roeck(a)ericsson.com> wrote: >>> Current early_printk code writes into VGA memory space even >>> if CONFIG_VGA_CONSOLE is undefined. This can cause problems >>> if there is no VGA device in the system, especially if the memory >>> is used by another device. >>> >>> Fix problem by redirecting output to early_serial_console >>> if CONFIG_VGA_CONSOLE is undefined. >>> >>> Signed-off-by: Guenter Roeck <guenter.roeck(a)ericsson.com> >> >> Reviewed-by: Pekka Enberg <penberg(a)cs.helsinki.fi> >> > What will it take to get this patch into the tree ? > > If there are coding style issues or some other unresolved concerns, > please let me know. > You didn't answer my question (c). I want to know how you ended up with boot_params.screen_info.orig_video_isVGA == 1 on a system with no VGA, which seems like it would have resolved this. I am *not* inclined to add a compile-time test for what should have been handed with a runtime test already. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 5 Apr 2010 16:30 On 04/05/2010 01:02 PM, Guenter Roeck wrote: >> >> You didn't answer my question (c). >> >> I want to know how you ended up with >> boot_params.screen_info.orig_video_isVGA == 1 on a system with no VGA, >> which seems like it would have resolved this. >> >> I am *not* inclined to add a compile-time test for what should have been >> handed with a runtime test already. >> > Sorry, I thought I did answer it. > You didn't. You still haven't! > The problem is that early_printk() can be called prior to the call to > setup_early_printk(). Since early_console is currently pre-initialized > with early_vga_console, output can be written to VGA memory space even > if there is no VGA controller in the system (and even if > boot_params.screen_info.orig_video_isVGA == 0). This happens for all > early_printk() calls executed prior to the call to setup_early_printk(). If boot_params.screen_info.orig_video_isVGA == 0, at least this bit of your patch has no effect: > > - if (!strncmp(buf, "vga", 3) && > > + if (have_vga_console && !strncmp(buf, "vga", 3) && > > boot_params.screen_info.orig_video_isVGA == 1) { Now, we have at least two ways to report a non-VGA console at runtime: boot_params.screen_info.orig_video_isVGA != 1 boot_params.screen_info.orig_video_lines == 0 The former is zero for CGA/MDA/EGA, but early_vga_write() doesn't work right for MDA at least, so keying on isVGA is probably right. early_printk() being called before setup_early_printk() is a problem, and it's not immediately obvious to me how to fix it. We can of course make early_vga_write() simply return if boot_params.screen_info.isVGA == 0, of course, but it really is a bigger problem than that in many ways. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 5 Apr 2010 17:20 On 04/05/2010 02:04 PM, Guenter Roeck wrote: > > Would you accept a minimized patch like this ? > > /* Direct interface for emergencies */ > +#ifdef CONFIG_VGA_CONSOLE > static struct console *early_console = &early_vga_console; > +#else > +static struct console *early_console = &early_serial_console; > +#endif > static int __initdata early_console_initialized; > > This would prevent the problem while minimizing changes, and at the same > time permit early messages to be written to the serial console. > I'm unhappy about it, because *those early messages shouldn't exist in the first place*. It seems to be an indication that we're invoking setup_early_printk() too late. The whole playing around with max_xpos and max_ypos instead of using boot_params.screen_info directly is particularly bleacherous. I would at least like to see if the improper invocation of early_printk() can be avoided. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 5 Apr 2010 17:20
On 04/05/2010 02:11 PM, H. Peter Anvin wrote: > On 04/05/2010 02:04 PM, Guenter Roeck wrote: >> >> Would you accept a minimized patch like this ? >> >> /* Direct interface for emergencies */ >> +#ifdef CONFIG_VGA_CONSOLE >> static struct console *early_console = &early_vga_console; >> +#else >> +static struct console *early_console = &early_serial_console; >> +#endif >> static int __initdata early_console_initialized; >> >> This would prevent the problem while minimizing changes, and at the same >> time permit early messages to be written to the serial console. >> > > I'm unhappy about it, because *those early messages shouldn't exist in > the first place*. It seems to be an indication that we're invoking > setup_early_printk() too late. The whole playing around with max_xpos > and max_ypos instead of using boot_params.screen_info directly is > particularly bleacherous. > > I would at least like to see if the improper invocation of > early_printk() can be avoided. > Specifically: is this anything other than "Kernel alive\n"? If it's just "Kernel alive\n" I say drop it on the floor. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |