Re: [PATCH v3] x86: Do not write into VGA memory space if there is no VGA device in the system

[Guenter Roeck <guenter.roeck@ericsson.com>]

On Mon, Apr 26, 2010 at 05:36:05AM -0400, Pekka Enberg wrote:
> Guenter Roeck kirjoitti:
> > Ensure that early_printk() does not write into VGA memory space
> > if there is not VGA device in the system.
> > 
> > Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> > ---
> > v3:
> > Changes are now limited to early_printk.c. Functionality is retained, meaning
> > there will still be output on the VGA console before setup_early_printk()
> > is called, but only if a VGA device is known to exist in the system.
> > 
> >  arch/x86/kernel/early_printk.c |   17 +++++++++++++----
> >  1 files changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
> > index b9c830c..783cb25 100644
> > --- a/arch/x86/kernel/early_printk.c
> > +++ b/arch/x86/kernel/early_printk.c
> > @@ -170,10 +170,13 @@ asmlinkage void early_printk(const char *fmt, ...)
> >  	int n;
> >  	va_list ap;
> >  
> > -	va_start(ap, fmt);
> > -	n = vscnprintf(buf, sizeof(buf), fmt, ap);
> > -	early_console->write(early_console, buf, n);
> > -	va_end(ap);
> > +	if (early_console && (early_console_initialized ||
> > +			      boot_params.screen_info.orig_video_isVGA == 1)) {
> > +		va_start(ap, fmt);
> > +		n = vscnprintf(buf, sizeof(buf), fmt, ap);
> > +		early_console->write(early_console, buf, n);
> > +		va_end(ap);
> > +	}
> >  }
> >  
> >  static inline void early_console_register(struct console *con, int keep_early)
> > @@ -233,6 +236,12 @@ static int __init setup_early_printk(char *buf)
> >  #endif
> >  		buf++;
> >  	}
> > +
> > +	/* If there is no VGA device, don't try to use it as early console. */
> > +	if (early_console == &early_vga_console &&
> > +	    boot_params.screen_info.orig_video_isVGA != 1)
> > +		early_console = NULL;
> 
> Wouldn't it be better to switch to, say, serial console here to avoid 
> the NULL check in early_printk()?
> 
This would be another possibility.

However, since the serial console was not explicitly selected, it would possibly
not be initialized. My understanding is that Peter had a problem with that.

One can argue that it would still be better to select the serial console in that case,
even though it might not be initialized. Actually, most likely it is (if it exists),
since the system must presumably have some means to communicate with the world.

I am pretty much open to either option, if that results in the patch being accepted. 
Just let me know which way to go.

Guenter