[PATCH] hvc_vio: Do not override preferred console set by kernel parameter

[PATCH] hvc_vio: Do not override preferred console set by kernel parameter

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]

The original version of this was done by Bastian Blank, who wrote:

> The problem is the following:
> - Architecture specific code sets preferred console to something bogus.
> - Command line handling tries to set preferred console but is overruled
>   by the old setting.
> 
> The udbg0 console is a boot console and independant.

References: http://bugs.debian.org/492703
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
We've been carrying this in Debian for 5 years now, so it's about time
it got reviewed.

I'm not convinced strstr() is the right way to check the command line
(what if there's also a 'netconsole='?).

Ben.

--- a/drivers/tty/hvc/hvc_vio.c
+++ b/drivers/tty/hvc/hvc_vio.c
@@ -48,6 +48,7 @@
 #include <asm/prom.h>
 #include <asm/hvsi.h>
 #include <asm/udbg.h>
+#include <asm/machdep.h>
 
 #include "hvc_console.h"
 
@@ -440,7 +441,9 @@
 	if (hvterm_priv0.proto == HV_PROTOCOL_HVSI)
 		goto out;
 #endif
-	add_preferred_console("hvc", 0, NULL);
+	/* Check whether the user has requested a different console. */
+	if (!strstr(cmd_line, "console="))
+		add_preferred_console("hvc", 0, NULL);
 	hvc_instantiate(0, 0, ops);
 out:
 	of_node_put(stdout_node);

-- 
Ben Hutchings
The most exhausting thing in life is being insincere. - Anne Morrow Lindberg

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] hvc_vio: Do not override preferred console set by kernel parameter

[Benjamin Herrenschmidt]

On Sun, 2013-09-01 at 17:24 +0100, Ben Hutchings wrote:
> The original version of this was done by Bastian Blank, who wrote:
> 
> > The problem is the following:
> > - Architecture specific code sets preferred console to something bogus.
> > - Command line handling tries to set preferred console but is overruled
> >   by the old setting.
> > 
> > The udbg0 console is a boot console and independant.
> 
> References: http://bugs.debian.org/492703
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> We've been carrying this in Debian for 5 years now, so it's about time
> it got reviewed.
> 
> I'm not convinced strstr() is the right way to check the command line
> (what if there's also a 'netconsole='?).

Also I think the problem should be solved elsewhere :-)

In the end, what that code is trying to do (as are all the other similar
instances) is to set "this is a good default in case nothing is
specified *or* what is specified doesn't actually exist".

Of course "doesn't exist" is tricky since the console could be provided
by a module loaded god knows when ... but in that case, maybe it does
make sense to stick to one of the known good defaults. After all, init
will fail without a tty ...

So I'm thinking we should in kernel/printk.c keep track of all those
"arch defaults" when console= is specified as "latent" consoles, and
right before starting init, if the specified one didn't work out (we
have no console with an associated tty), then go through those latent
ones and pick one that works.

Cheers,
Ben.

Re: [PATCH] hvc_vio: Do not override preferred console set by kernel parameter

[Greg Kroah-Hartman]

On Mon, Sep 02, 2013 at 09:55:40AM +1000, Benjamin Herrenschmidt wrote:
> On Sun, 2013-09-01 at 17:24 +0100, Ben Hutchings wrote:
> > The original version of this was done by Bastian Blank, who wrote:
> > 
> > > The problem is the following:
> > > - Architecture specific code sets preferred console to something bogus.
> > > - Command line handling tries to set preferred console but is overruled
> > >   by the old setting.
> > > 
> > > The udbg0 console is a boot console and independant.
> > 
> > References: http://bugs.debian.org/492703
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > ---
> > We've been carrying this in Debian for 5 years now, so it's about time
> > it got reviewed.
> > 
> > I'm not convinced strstr() is the right way to check the command line
> > (what if there's also a 'netconsole='?).
> 
> Also I think the problem should be solved elsewhere :-)
> 
> In the end, what that code is trying to do (as are all the other similar
> instances) is to set "this is a good default in case nothing is
> specified *or* what is specified doesn't actually exist".
> 
> Of course "doesn't exist" is tricky since the console could be provided
> by a module loaded god knows when ... but in that case, maybe it does
> make sense to stick to one of the known good defaults. After all, init
> will fail without a tty ...
> 
> So I'm thinking we should in kernel/printk.c keep track of all those
> "arch defaults" when console= is specified as "latent" consoles, and
> right before starting init, if the specified one didn't work out (we
> have no console with an associated tty), then go through those latent
> ones and pick one that works.

So, I shouldn't apply this patch?  We should do something to fix this,
if Debian has to drag this patch on for 5 years, that's an indication
that this might be one solution we should use, right?

thanks,

greg k-h

Re: [PATCH] hvc_vio: Do not override preferred console set by kernel parameter

[Benjamin Herrenschmidt]

On Thu, 2013-09-26 at 14:22 -0700, Greg Kroah-Hartman wrote:

> So, I shouldn't apply this patch?  We should do something to fix this,
> if Debian has to drag this patch on for 5 years, that's an indication
> that this might be one solution we should use, right?

Ah sorry, dropped the ball on that one. Yes the patch is an acceptable
band-aid but somebody should work on a better solution :-)

Cheers,
Ben.