linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] console: Add console=auto option
@ 2018-08-16 14:10 Prarit Bhargava
  2018-08-16 15:09 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Prarit Bhargava @ 2018-08-16 14:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Prarit Bhargava, Mark Salter, Al Stone, Rafael J. Wysocki,
	Len Brown, Pavel Machek, x86, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt, Kees Cook, Greg Kroah-Hartman, linux-pm

ACPI may contain an SPCR table that defines a default system console.
On ARM if the table is present then the SPCR console is enabled by default.
On x86 the SPCR console is used if 'earlycon' (no parameters) is
specified as a kernel parameter and is used only as the early console.
To use the SPCR data as a console a user must boot with 'earlycon',
grep logs & specify a console= kernel parameter, and then reboot again.

Add 'console=auto' that enables a firmware or hardware console, and on
x86 enable the SPCR console if 'console=auto' is specified.

Tested on systems with and without an ACPI SPCR.  The following kernel
parameters were also tested:

console=ttyS0,115200    		works
earlycon                		works (early console only)
console=auto            		works (full console as expected)
no console or earlycon arguments	works (no output as expected)

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Al Stone <ahs3@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <len.brown@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: x86@kernel.org
Cc: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
---
 Documentation/admin-guide/kernel-parameters.txt |  1 +
 arch/x86/kernel/acpi/boot.c                     |  5 +++++
 include/linux/console.h                         |  1 +
 kernel/printk/printk.c                          | 10 ++++++++++
 4 files changed, 17 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a32f2a126791..dd057224f33b 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -635,6 +635,7 @@
 
 		hvc<n>	Use the hypervisor console device <n>. This is for
 			both Xen and PowerPC hypervisors.
+		auto	[X86] Enable ACPI SPCR console
 
 		If the device connected to the port is not a TTY but a braille
 		device, prepend "brl," before the device type, for instance
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 3b20607d581b..fb2616ba3b21 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1771,3 +1771,8 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size)
 	e820__range_add(addr, size, E820_TYPE_ACPI);
 	e820__update_table_print();
 }
+
+void __init arch_console_setup(void)
+{
+	acpi_parse_spcr(false, true);
+}
diff --git a/include/linux/console.h b/include/linux/console.h
index dfd6b0e97855..b802f62289a9 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -181,6 +181,7 @@ extern int is_console_locked(void);
 extern int braille_register_console(struct console *, int index,
 		char *console_options, char *braille_options);
 extern int braille_unregister_console(struct console *);
+int arch_console_setup(void);
 #ifdef CONFIG_TTY
 extern void console_sysfs_notify(void);
 #else
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 247808333ba4..b74eba44183e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2078,6 +2078,11 @@ static int __init console_msg_format_setup(char *str)
 }
 __setup("console_msg_format=", console_msg_format_setup);
 
+int __init __weak arch_console_setup(void)
+{
+	return 0;
+}
+
 /*
  * Set up a console.  Called via do_early_param() in init/main.c
  * for each "console=" parameter in the boot command line.
@@ -2088,6 +2093,11 @@ static int __init console_setup(char *str)
 	char *s, *options, *brl_options = NULL;
 	int idx;
 
+	if (!strcmp(str, "auto")) {
+		arch_console_setup();
+		return 1;
+	}
+
 	if (_braille_console_setup(&str, &brl_options))
 		return 1;
 
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] console: Add console=auto option
  2018-08-16 14:10 [PATCH] console: Add console=auto option Prarit Bhargava
@ 2018-08-16 15:09 ` Greg Kroah-Hartman
  2018-08-16 15:28   ` Prarit Bhargava
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2018-08-16 15:09 UTC (permalink / raw)
  To: Prarit Bhargava
  Cc: linux-kernel, Mark Salter, Al Stone, Rafael J. Wysocki,
	Len Brown, Pavel Machek, x86, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt, Kees Cook, linux-pm

On Thu, Aug 16, 2018 at 10:10:55AM -0400, Prarit Bhargava wrote:
> ACPI may contain an SPCR table that defines a default system console.
> On ARM if the table is present then the SPCR console is enabled by default.
> On x86 the SPCR console is used if 'earlycon' (no parameters) is
> specified as a kernel parameter and is used only as the early console.
> To use the SPCR data as a console a user must boot with 'earlycon',
> grep logs & specify a console= kernel parameter, and then reboot again.
> 
> Add 'console=auto' that enables a firmware or hardware console, and on
> x86 enable the SPCR console if 'console=auto' is specified.
> 
> Tested on systems with and without an ACPI SPCR.  The following kernel
> parameters were also tested:
> 
> console=ttyS0,115200    		works
> earlycon                		works (early console only)
> console=auto            		works (full console as expected)
> no console or earlycon arguments	works (no output as expected)
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Al Stone <ahs3@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: x86@kernel.org
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  1 +
>  arch/x86/kernel/acpi/boot.c                     |  5 +++++
>  include/linux/console.h                         |  1 +
>  kernel/printk/printk.c                          | 10 ++++++++++
>  4 files changed, 17 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a32f2a126791..dd057224f33b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -635,6 +635,7 @@
>  
>  		hvc<n>	Use the hypervisor console device <n>. This is for
>  			both Xen and PowerPC hypervisors.
> +		auto	[X86] Enable ACPI SPCR console
>  
>  		If the device connected to the port is not a TTY but a braille
>  		device, prepend "brl," before the device type, for instance
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 3b20607d581b..fb2616ba3b21 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -1771,3 +1771,8 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size)
>  	e820__range_add(addr, size, E820_TYPE_ACPI);
>  	e820__update_table_print();
>  }
> +
> +void __init arch_console_setup(void)
> +{
> +	acpi_parse_spcr(false, true);
> +}

<snip>

> +int arch_console_setup(void);

Function does not match its prototype :(

How did you build this successfully?


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] console: Add console=auto option
  2018-08-16 15:09 ` Greg Kroah-Hartman
@ 2018-08-16 15:28   ` Prarit Bhargava
  2018-08-16 15:33     ` Steven Rostedt
  0 siblings, 1 reply; 14+ messages in thread
From: Prarit Bhargava @ 2018-08-16 15:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Mark Salter, Al Stone, Rafael J. Wysocki,
	Len Brown, Pavel Machek, x86, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt, Kees Cook, linux-pm



On 08/16/2018 11:09 AM, Greg Kroah-Hartman wrote:
> On Thu, Aug 16, 2018 at 10:10:55AM -0400, Prarit Bhargava wrote:
>> ACPI may contain an SPCR table that defines a default system console.
>> On ARM if the table is present then the SPCR console is enabled by default.
>> On x86 the SPCR console is used if 'earlycon' (no parameters) is
>> specified as a kernel parameter and is used only as the early console.
>> To use the SPCR data as a console a user must boot with 'earlycon',
>> grep logs & specify a console= kernel parameter, and then reboot again.
>>
>> Add 'console=auto' that enables a firmware or hardware console, and on
>> x86 enable the SPCR console if 'console=auto' is specified.
>>
>> Tested on systems with and without an ACPI SPCR.  The following kernel
>> parameters were also tested:
>>
>> console=ttyS0,115200    		works
>> earlycon                		works (early console only)
>> console=auto            		works (full console as expected)
>> no console or earlycon arguments	works (no output as expected)
>>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Cc: Mark Salter <msalter@redhat.com>
>> Cc: Al Stone <ahs3@redhat.com>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Len Brown <len.brown@intel.com>
>> Cc: Pavel Machek <pavel@ucw.cz>
>> Cc: x86@kernel.org
>> Cc: Petr Mladek <pmladek@suse.com>
>> Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: linux-pm@vger.kernel.org
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt |  1 +
>>  arch/x86/kernel/acpi/boot.c                     |  5 +++++
>>  include/linux/console.h                         |  1 +
>>  kernel/printk/printk.c                          | 10 ++++++++++
>>  4 files changed, 17 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index a32f2a126791..dd057224f33b 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -635,6 +635,7 @@
>>  
>>  		hvc<n>	Use the hypervisor console device <n>. This is for
>>  			both Xen and PowerPC hypervisors.
>> +		auto	[X86] Enable ACPI SPCR console
>>  
>>  		If the device connected to the port is not a TTY but a braille
>>  		device, prepend "brl," before the device type, for instance
>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>> index 3b20607d581b..fb2616ba3b21 100644
>> --- a/arch/x86/kernel/acpi/boot.c
>> +++ b/arch/x86/kernel/acpi/boot.c
>> @@ -1771,3 +1771,8 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size)
>>  	e820__range_add(addr, size, E820_TYPE_ACPI);
>>  	e820__update_table_print();
>>  }
>> +
>> +void __init arch_console_setup(void)
>> +{
>> +	acpi_parse_spcr(false, true);
>> +}
> 
> <snip>
> 
>> +int arch_console_setup(void);
> 
> Function does not match its prototype :(
> 
> How did you build this successfully?

Weird, it compiles on my system?

I'll fix it though.

P.

> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] console: Add console=auto option
  2018-08-16 15:28   ` Prarit Bhargava
@ 2018-08-16 15:33     ` Steven Rostedt
  2018-08-16 17:23       ` Prarit Bhargava
  0 siblings, 1 reply; 14+ messages in thread
From: Steven Rostedt @ 2018-08-16 15:33 UTC (permalink / raw)
  To: Prarit Bhargava
  Cc: Greg Kroah-Hartman, linux-kernel, Mark Salter, Al Stone,
	Rafael J. Wysocki, Len Brown, Pavel Machek, x86, Petr Mladek,
	Sergey Senozhatsky, Kees Cook, linux-pm

On Thu, 16 Aug 2018 11:28:24 -0400
Prarit Bhargava <prarit@redhat.com> wrote:

> On 08/16/2018 11:09 AM, Greg Kroah-Hartman wrote:
> > On Thu, Aug 16, 2018 at 10:10:55AM -0400, Prarit Bhargava wrote:  
> >> ACPI may contain an SPCR table that defines a default system console.
> >> On ARM if the table is present then the SPCR console is enabled by default.
> >> On x86 the SPCR console is used if 'earlycon' (no parameters) is
> >> specified as a kernel parameter and is used only as the early console.
> >> To use the SPCR data as a console a user must boot with 'earlycon',
> >> grep logs & specify a console= kernel parameter, and then reboot again.
> >>
> >> Add 'console=auto' that enables a firmware or hardware console, and on
> >> x86 enable the SPCR console if 'console=auto' is specified.
> >>
> >> Tested on systems with and without an ACPI SPCR.  The following kernel
> >> parameters were also tested:
> >>
> >> console=ttyS0,115200    		works
> >> earlycon                		works (early console only)
> >> console=auto            		works (full console as expected)
> >> no console or earlycon arguments	works (no output as expected)
> >>
> >> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> >> Cc: Mark Salter <msalter@redhat.com>
> >> Cc: Al Stone <ahs3@redhat.com>
> >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> >> Cc: Len Brown <len.brown@intel.com>
> >> Cc: Pavel Machek <pavel@ucw.cz>
> >> Cc: x86@kernel.org
> >> Cc: Petr Mladek <pmladek@suse.com>
> >> Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> >> Cc: Steven Rostedt <rostedt@goodmis.org>
> >> Cc: Kees Cook <keescook@chromium.org>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> Cc: linux-pm@vger.kernel.org
> >> ---
> >>  Documentation/admin-guide/kernel-parameters.txt |  1 +
> >>  arch/x86/kernel/acpi/boot.c                     |  5 +++++
> >>  include/linux/console.h                         |  1 +
> >>  kernel/printk/printk.c                          | 10 ++++++++++
> >>  4 files changed, 17 insertions(+)
> >>
> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >> index a32f2a126791..dd057224f33b 100644
> >> --- a/Documentation/admin-guide/kernel-parameters.txt
> >> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >> @@ -635,6 +635,7 @@
> >>  
> >>  		hvc<n>	Use the hypervisor console device <n>. This is for
> >>  			both Xen and PowerPC hypervisors.
> >> +		auto	[X86] Enable ACPI SPCR console
> >>  
> >>  		If the device connected to the port is not a TTY but a braille
> >>  		device, prepend "brl," before the device type, for instance
> >> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> >> index 3b20607d581b..fb2616ba3b21 100644
> >> --- a/arch/x86/kernel/acpi/boot.c
> >> +++ b/arch/x86/kernel/acpi/boot.c
> >> @@ -1771,3 +1771,8 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size)
> >>  	e820__range_add(addr, size, E820_TYPE_ACPI);
> >>  	e820__update_table_print();
> >>  }
> >> +
> >> +void __init arch_console_setup(void)
> >> +{
> >> +	acpi_parse_spcr(false, true);
> >> +}  
> > 
> > <snip>
> >   
> >> +int arch_console_setup(void);  
> > 
> > Function does not match its prototype :(
> > 
> > How did you build this successfully?  
> 
> Weird, it compiles on my system?

Are you compiling with ACPI enabled?

It would still compile without it and use the properly prototyped
"weak" function.

-- Steve

> 
> I'll fix it though.
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] console: Add console=auto option
  2018-08-16 15:33     ` Steven Rostedt
@ 2018-08-16 17:23       ` Prarit Bhargava
  2018-08-16 17:39         ` [PATCH v2] " Prarit Bhargava
  0 siblings, 1 reply; 14+ messages in thread
From: Prarit Bhargava @ 2018-08-16 17:23 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Greg Kroah-Hartman, linux-kernel, Mark Salter, Al Stone,
	Rafael J. Wysocki, Len Brown, Pavel Machek, x86, Petr Mladek,
	Sergey Senozhatsky, Kees Cook, linux-pm



On 08/16/2018 11:33 AM, Steven Rostedt wrote:
> On Thu, 16 Aug 2018 11:28:24 -0400
> Prarit Bhargava <prarit@redhat.com> wrote:
> 
>> On 08/16/2018 11:09 AM, Greg Kroah-Hartman wrote:
>>> On Thu, Aug 16, 2018 at 10:10:55AM -0400, Prarit Bhargava wrote:  
>>>> ACPI may contain an SPCR table that defines a default system console.
>>>> On ARM if the table is present then the SPCR console is enabled by default.
>>>> On x86 the SPCR console is used if 'earlycon' (no parameters) is
>>>> specified as a kernel parameter and is used only as the early console.
>>>> To use the SPCR data as a console a user must boot with 'earlycon',
>>>> grep logs & specify a console= kernel parameter, and then reboot again.
>>>>
>>>> Add 'console=auto' that enables a firmware or hardware console, and on
>>>> x86 enable the SPCR console if 'console=auto' is specified.
>>>>
>>>> Tested on systems with and without an ACPI SPCR.  The following kernel
>>>> parameters were also tested:
>>>>
>>>> console=ttyS0,115200    		works
>>>> earlycon                		works (early console only)
>>>> console=auto            		works (full console as expected)
>>>> no console or earlycon arguments	works (no output as expected)
>>>>
>>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>>> Cc: Mark Salter <msalter@redhat.com>
>>>> Cc: Al Stone <ahs3@redhat.com>
>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>>> Cc: Len Brown <len.brown@intel.com>
>>>> Cc: Pavel Machek <pavel@ucw.cz>
>>>> Cc: x86@kernel.org
>>>> Cc: Petr Mladek <pmladek@suse.com>
>>>> Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
>>>> Cc: Steven Rostedt <rostedt@goodmis.org>
>>>> Cc: Kees Cook <keescook@chromium.org>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: linux-pm@vger.kernel.org
>>>> ---
>>>>  Documentation/admin-guide/kernel-parameters.txt |  1 +
>>>>  arch/x86/kernel/acpi/boot.c                     |  5 +++++
>>>>  include/linux/console.h                         |  1 +
>>>>  kernel/printk/printk.c                          | 10 ++++++++++
>>>>  4 files changed, 17 insertions(+)
>>>>
>>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>>> index a32f2a126791..dd057224f33b 100644
>>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>>> @@ -635,6 +635,7 @@
>>>>  
>>>>  		hvc<n>	Use the hypervisor console device <n>. This is for
>>>>  			both Xen and PowerPC hypervisors.
>>>> +		auto	[X86] Enable ACPI SPCR console
>>>>  
>>>>  		If the device connected to the port is not a TTY but a braille
>>>>  		device, prepend "brl," before the device type, for instance
>>>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>>>> index 3b20607d581b..fb2616ba3b21 100644
>>>> --- a/arch/x86/kernel/acpi/boot.c
>>>> +++ b/arch/x86/kernel/acpi/boot.c
>>>> @@ -1771,3 +1771,8 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size)
>>>>  	e820__range_add(addr, size, E820_TYPE_ACPI);
>>>>  	e820__update_table_print();
>>>>  }
>>>> +
>>>> +void __init arch_console_setup(void)
>>>> +{
>>>> +	acpi_parse_spcr(false, true);
>>>> +}  
>>>
>>> <snip>
>>>   
>>>> +int arch_console_setup(void);  
>>>
>>> Function does not match its prototype :(
>>>
>>> How did you build this successfully?  
>>
>> Weird, it compiles on my system?
> 
> Are you compiling with ACPI enabled?
> 

Yes, CONFIG_ACPI is enabled.

> It would still compile without it and use the properly prototyped
> "weak" function.

I did this:

1.  git co linux.git
2.  git am 0001-console-Add-console-auto-option.patch
3.  make -j224

and I have a clean compile with no warnings or errors.  My config is the
standard Fedora .config so it has CONFIG_ACPI enabled and CONFIG_ACPI_SPCR_TABLE
enabled.

IMO the compile should have warned or error'd out.  Is there a possibility that
some subtlety of weak functions that causes this to not be an error?

In any case, I've changed the prototype and other functions to void and am
finishing up retesting.

P.

> 
> -- Steve
> 
>>
>> I'll fix it though.
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] console: Add console=auto option
  2018-08-16 17:23       ` Prarit Bhargava
@ 2018-08-16 17:39         ` Prarit Bhargava
  2018-08-17  8:19           ` Petr Mladek
  2018-08-17  9:38           ` Sergey Senozhatsky
  0 siblings, 2 replies; 14+ messages in thread
From: Prarit Bhargava @ 2018-08-16 17:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: Prarit Bhargava, Mark Salter, Al Stone, Rafael J. Wysocki,
	Len Brown, Pavel Machek, x86, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt, Kees Cook, Greg Kroah-Hartman, linux-pm

ACPI may contain an SPCR table that defines a default system console.
On ARM if the table is present then the SPCR console is enabled by default.
On x86 the SPCR console is used if 'earlycon' (no parameters) is
specified as a kernel parameter and is used only as the early console.
To use the SPCR data as a console a user must boot with 'earlycon',
grep logs & specify a console= kernel parameter, and then reboot again.

Add 'console=auto' that enables a firmware or hardware console, and on
x86 enable the SPCR console if 'console=auto' is specified.

Tested on systems with and without an ACPI SPCR.  The following kernel
parameters were also tested:

console=ttyS0,115200    		works
earlycon                		works (early console only)
console=auto            		works (full console as expected)
no console or earlycon arguments	works (no output as expected)

v2: Fix prototype.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Al Stone <ahs3@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <len.brown@intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: x86@kernel.org
Cc: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
---
 Documentation/admin-guide/kernel-parameters.txt |  1 +
 arch/x86/kernel/acpi/boot.c                     |  5 +++++
 include/linux/console.h                         |  1 +
 kernel/printk/printk.c                          | 10 ++++++++++
 4 files changed, 17 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a32f2a126791..dd057224f33b 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -635,6 +635,7 @@
 
 		hvc<n>	Use the hypervisor console device <n>. This is for
 			both Xen and PowerPC hypervisors.
+		auto	[X86] Enable ACPI SPCR console
 
 		If the device connected to the port is not a TTY but a braille
 		device, prepend "brl," before the device type, for instance
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 3b20607d581b..fb2616ba3b21 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1771,3 +1771,8 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size)
 	e820__range_add(addr, size, E820_TYPE_ACPI);
 	e820__update_table_print();
 }
+
+void __init arch_console_setup(void)
+{
+	acpi_parse_spcr(false, true);
+}
diff --git a/include/linux/console.h b/include/linux/console.h
index dfd6b0e97855..c2a6dde9a799 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -181,6 +181,7 @@ extern int is_console_locked(void);
 extern int braille_register_console(struct console *, int index,
 		char *console_options, char *braille_options);
 extern int braille_unregister_console(struct console *);
+void arch_console_setup(void);
 #ifdef CONFIG_TTY
 extern void console_sysfs_notify(void);
 #else
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 247808333ba4..bdd53858664b 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2078,6 +2078,11 @@ static int __init console_msg_format_setup(char *str)
 }
 __setup("console_msg_format=", console_msg_format_setup);
 
+void __init __weak arch_console_setup(void)
+{
+	return;
+}
+
 /*
  * Set up a console.  Called via do_early_param() in init/main.c
  * for each "console=" parameter in the boot command line.
@@ -2088,6 +2093,11 @@ static int __init console_setup(char *str)
 	char *s, *options, *brl_options = NULL;
 	int idx;
 
+	if (!strcmp(str, "auto")) {
+		arch_console_setup();
+		return 1;
+	}
+
 	if (_braille_console_setup(&str, &brl_options))
 		return 1;
 
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] console: Add console=auto option
  2018-08-16 17:39         ` [PATCH v2] " Prarit Bhargava
@ 2018-08-17  8:19           ` Petr Mladek
  2018-08-17 10:26             ` Prarit Bhargava
  2018-08-17  9:38           ` Sergey Senozhatsky
  1 sibling, 1 reply; 14+ messages in thread
From: Petr Mladek @ 2018-08-17  8:19 UTC (permalink / raw)
  To: Prarit Bhargava
  Cc: linux-kernel, Mark Salter, Al Stone, Rafael J. Wysocki,
	Len Brown, Pavel Machek, x86, Sergey Senozhatsky, Steven Rostedt,
	Kees Cook, Greg Kroah-Hartman, linux-pm

On Thu 2018-08-16 13:39:01, Prarit Bhargava wrote:
> ACPI may contain an SPCR table that defines a default system console.
> On ARM if the table is present then the SPCR console is enabled by default.
> On x86 the SPCR console is used if 'earlycon' (no parameters) is
> specified as a kernel parameter and is used only as the early console.
> To use the SPCR data as a console a user must boot with 'earlycon',
> grep logs & specify a console= kernel parameter, and then reboot again.
> 
> Add 'console=auto' that enables a firmware or hardware console, and on
> x86 enable the SPCR console if 'console=auto' is specified.

I basically like the idea. Just one or two nits below.


> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a32f2a126791..dd057224f33b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -635,6 +635,7 @@
>  
>  		hvc<n>	Use the hypervisor console device <n>. This is for
>  			both Xen and PowerPC hypervisors.
> +		auto	[X86] Enable ACPI SPCR console

The "auto" option sounds reasonable. But earlycon does exactly this
when used without no extra options. I prefer to stay consistent
with the existing earlycon behavior.


>  		If the device connected to the port is not a TTY but a braille
>  		device, prepend "brl," before the device type, for instance
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 3b20607d581b..fb2616ba3b21 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -1771,3 +1771,8 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size)
>  	e820__range_add(addr, size, E820_TYPE_ACPI);
>  	e820__update_table_print();
>  }
> +
> +void __init arch_console_setup(void)
> +{
> +	acpi_parse_spcr(false, true);

Just for record. I was curious that this might be called twice
(also from acpi_boot_init(). But it looks safe after all.

The trick is in the two bool parameters. One call allows to
register/enable earlycon and ignores normal console. This other
call does exactly the opposite. I do not see any unwanted side
effects.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] console: Add console=auto option
  2018-08-16 17:39         ` [PATCH v2] " Prarit Bhargava
  2018-08-17  8:19           ` Petr Mladek
@ 2018-08-17  9:38           ` Sergey Senozhatsky
  2018-08-17 10:36             ` Prarit Bhargava
  1 sibling, 1 reply; 14+ messages in thread
From: Sergey Senozhatsky @ 2018-08-17  9:38 UTC (permalink / raw)
  To: Prarit Bhargava
  Cc: linux-kernel, Mark Salter, Al Stone, Rafael J. Wysocki,
	Len Brown, Pavel Machek, x86, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt, Kees Cook, Greg Kroah-Hartman, linux-pm

On (08/16/18 13:39), Prarit Bhargava wrote:
>
> +		auto	[X86] Enable ACPI SPCR console
			^^^^
			And arm64?


Any chance we can rename param to "spcr" or something more clear?
To explicitly state what exactly it's going to do. `auto' sounds
too general and doesn't tell me that much. I'm probably the only
here who can't see a connection between "auto" and "SPCR", but
still.

One more thing, as far as I can tell, acpi_parse_spcr() can fail
and return an error. arch_console_setup() hides all errors and
returns void. Should it return error code?

	int arch_console_setup(void)
	{
		return acpi_parse_spcr(false, true);
	}

Or maybe

	void arch_console_setup(void)
	{
		if (acpi_parse_spcr(false, true))
			pr_err(.........);
	}

There can be other consoles in the system, logging an error is not
such a useless thing.

	-ss

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] console: Add console=auto option
  2018-08-17  8:19           ` Petr Mladek
@ 2018-08-17 10:26             ` Prarit Bhargava
  0 siblings, 0 replies; 14+ messages in thread
From: Prarit Bhargava @ 2018-08-17 10:26 UTC (permalink / raw)
  To: Petr Mladek
  Cc: linux-kernel, Mark Salter, Al Stone, Rafael J. Wysocki,
	Len Brown, Pavel Machek, x86, Sergey Senozhatsky, Steven Rostedt,
	Kees Cook, Greg Kroah-Hartman, linux-pm



On 08/17/2018 04:19 AM, Petr Mladek wrote:
> On Thu 2018-08-16 13:39:01, Prarit Bhargava wrote:
>> ACPI may contain an SPCR table that defines a default system console.
>> On ARM if the table is present then the SPCR console is enabled by default.
>> On x86 the SPCR console is used if 'earlycon' (no parameters) is
>> specified as a kernel parameter and is used only as the early console.
>> To use the SPCR data as a console a user must boot with 'earlycon',
>> grep logs & specify a console= kernel parameter, and then reboot again.
>>
>> Add 'console=auto' that enables a firmware or hardware console, and on
>> x86 enable the SPCR console if 'console=auto' is specified.
> 
> I basically like the idea. Just one or two nits below.
> 
> 
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index a32f2a126791..dd057224f33b 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -635,6 +635,7 @@
>>  
>>  		hvc<n>	Use the hypervisor console device <n>. This is for
>>  			both Xen and PowerPC hypervisors.
>> +		auto	[X86] Enable ACPI SPCR console
> 
> The "auto" option sounds reasonable. But earlycon does exactly this
> when used without no extra options. I prefer to stay consistent
> with the existing earlycon behavior.

Hi Petr, on x86 earlycon still only enables the early console but does not
enable the console.

> 
> 
>>  		If the device connected to the port is not a TTY but a braille
>>  		device, prepend "brl," before the device type, for instance
>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>> index 3b20607d581b..fb2616ba3b21 100644
>> --- a/arch/x86/kernel/acpi/boot.c
>> +++ b/arch/x86/kernel/acpi/boot.c
>> @@ -1771,3 +1771,8 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size)
>>  	e820__range_add(addr, size, E820_TYPE_ACPI);
>>  	e820__update_table_print();
>>  }
>> +
>> +void __init arch_console_setup(void)
>> +{
>> +	acpi_parse_spcr(false, true);
> 
> Just for record. I was curious that this might be called twice
> (also from acpi_boot_init(). But it looks safe after all.
> 

Yes, the console code prevents the same console from being registered twice.

> The trick is in the two bool parameters. One call allows to
> register/enable earlycon and ignores normal console. This other
> call does exactly the opposite. I do not see any unwanted side
> effects.

Thanks.  I'd like to know if you (or anyone else) has strong feelings about
changing the behaviour of earlycon on x86?  I could make it so that specifying
just earlycon would also initialize the console.

P.

> 
> Best Regards,
> Petr
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] console: Add console=auto option
  2018-08-17  9:38           ` Sergey Senozhatsky
@ 2018-08-17 10:36             ` Prarit Bhargava
  2018-08-17 10:50               ` Sergey Senozhatsky
  0 siblings, 1 reply; 14+ messages in thread
From: Prarit Bhargava @ 2018-08-17 10:36 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: linux-kernel, Mark Salter, Al Stone, Rafael J. Wysocki,
	Len Brown, Pavel Machek, x86, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt, Kees Cook, Greg Kroah-Hartman, linux-pm



On 08/17/2018 05:38 AM, Sergey Senozhatsky wrote:
> On (08/16/18 13:39), Prarit Bhargava wrote:
>>
>> +		auto	[X86] Enable ACPI SPCR console
> 			^^^^
> 			And arm64?

Hi Sergey, on arm64 if an SPCR is present the early console and console are
initialized by default.  IOW no kernel parameter is necessary to initialize the
console in that case.

> 
> 
> Any chance we can rename param to "spcr" or something more clear?
> To explicitly state what exactly it's going to do. `auto' sounds
> too general and doesn't tell me that much. I'm probably the only
> here who can't see a connection between "auto" and "SPCR", but
> still.

I came up with "auto" because I think it is generic.  I also thought about
"console=fw", or just "console".  If in the future another arch wants to
optionally bring up a firmware or hardware defined console then they could use
auto too.

> 
> One more thing, as far as I can tell, acpi_parse_spcr() can fail
> and return an error. arch_console_setup() hides all errors and
> returns void. Should it return error code?
> 
> 	int arch_console_setup(void)
> 	{
> 		return acpi_parse_spcr(false, true);
> 	}
> 
> Or maybe
> 
> 	void arch_console_setup(void)
> 	{
> 		if (acpi_parse_spcr(false, true))
> 			pr_err(.........);
> 	}
> 
> There can be other consoles in the system, logging an error is not
> such a useless thing.

I can make the second change.  The problem (IIRC) with returning an error in an
setup fn is that the rest of the setup functions will not execute.  I don't want
to fail the setup callbacks because of an incorrect SPCR table.

Like I mentioned to Petr, I'd like to know if you (or anyone else) has strong
feelings about changing the behaviour of earlycon on x86?  I could make it so
that specifying just earlycon would also initialize the console.

P.

> 
> 	-ss
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] console: Add console=auto option
  2018-08-17 10:36             ` Prarit Bhargava
@ 2018-08-17 10:50               ` Sergey Senozhatsky
  2018-08-17 11:06                 ` Prarit Bhargava
  0 siblings, 1 reply; 14+ messages in thread
From: Sergey Senozhatsky @ 2018-08-17 10:50 UTC (permalink / raw)
  To: Prarit Bhargava
  Cc: Sergey Senozhatsky, linux-kernel, Mark Salter, Al Stone,
	Rafael J. Wysocki, Len Brown, Pavel Machek, x86, Petr Mladek,
	Sergey Senozhatsky, Steven Rostedt, Kees Cook,
	Greg Kroah-Hartman, linux-pm, Peter Zijlstra

Hello,

Cc-ing Peter Zijlstra

  lkml.kernel.org/r/728a8e68-ea4b-4040-a0fc-217df4f1928d@redhat.com
  lkml.kernel.org/r/20180817081947.m425gok2ugt7tglp@pathway.suse.cz
  lkml.kernel.org/r/00c60dca-60bc-8568-eaa3-d4b0c326cab4@redhat.com

On (08/17/18 06:36), Prarit Bhargava wrote:
> On 08/17/2018 05:38 AM, Sergey Senozhatsky wrote:
> > On (08/16/18 13:39), Prarit Bhargava wrote:
> >>
> >> +		auto	[X86] Enable ACPI SPCR console
> > 			^^^^
> > 			And arm64?
> 
> Hi Sergey, on arm64 if an SPCR is present the early console and console are
> initialized by default.  IOW no kernel parameter is necessary to initialize the
> console in that case.

OK, thanks.

> > Any chance we can rename param to "spcr" or something more clear?
> > To explicitly state what exactly it's going to do. `auto' sounds
> > too general and doesn't tell me that much. I'm probably the only
> > here who can't see a connection between "auto" and "SPCR", but
> > still.
> 
> I came up with "auto" because I think it is generic.  I also thought about
> "console=fw", or just "console".  If in the future another arch wants to
> optionally bring up a firmware or hardware defined console then they could use
> auto too.

Hmm, I see your point.
My [sort of a] problem with "auto" is that it tells me as much as "magic"
[and "magic" tells me almost nothing]. By the way, would be fun if we had
"magic" instead of "auto" all over the kernel

	echo "magic" > /sys/bus/usb/...../power/control

> > 	void arch_console_setup(void)
> > 	{
> > 		if (acpi_parse_spcr(false, true))
> > 			pr_err(.........);
> > 	}
> > 
> > There can be other consoles in the system, logging an error is not
> > such a useless thing.
> 
> I can make the second change.  The problem (IIRC) with returning an error in an
> setup fn is that the rest of the setup functions will not execute.  I don't want
> to fail the setup callbacks because of an incorrect SPCR table.

OK, fair enough.
Letting users know that SPCR is incorrect also makes sense, so option #2
I guess is what we want after all.

> Like I mentioned to Petr, I'd like to know if you (or anyone else) has strong
> feelings about changing the behaviour of earlycon on x86?  I could make it so
> that specifying just earlycon would also initialize the console.

x86 people and/or scheduler people might have strong opinions on this.
I Cc-ed Peter Zijlstra; he represents both groups and is known to be
a hardcore earlycon user.

	-ss

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] console: Add console=auto option
  2018-08-17 10:50               ` Sergey Senozhatsky
@ 2018-08-17 11:06                 ` Prarit Bhargava
  2018-08-17 12:57                   ` Petr Mladek
  0 siblings, 1 reply; 14+ messages in thread
From: Prarit Bhargava @ 2018-08-17 11:06 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: linux-kernel, Mark Salter, Al Stone, Rafael J. Wysocki,
	Len Brown, Pavel Machek, x86, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt, Kees Cook, Greg Kroah-Hartman, linux-pm,
	Peter Zijlstra



On 08/17/2018 06:50 AM, Sergey Senozhatsky wrote:
> Hello,
> 
> Cc-ing Peter Zijlstra
> 
>   lkml.kernel.org/r/728a8e68-ea4b-4040-a0fc-217df4f1928d@redhat.com
>   lkml.kernel.org/r/20180817081947.m425gok2ugt7tglp@pathway.suse.cz
>   lkml.kernel.org/r/00c60dca-60bc-8568-eaa3-d4b0c326cab4@redhat.com
> 
> On (08/17/18 06:36), Prarit Bhargava wrote:
>> On 08/17/2018 05:38 AM, Sergey Senozhatsky wrote:
>>> On (08/16/18 13:39), Prarit Bhargava wrote:
>>>>
>>>> +		auto	[X86] Enable ACPI SPCR console
>>> 			^^^^
>>> 			And arm64?
>>
>> Hi Sergey, on arm64 if an SPCR is present the early console and console are
>> initialized by default.  IOW no kernel parameter is necessary to initialize the
>> console in that case.
> 
> OK, thanks.
> 
>>> Any chance we can rename param to "spcr" or something more clear?
>>> To explicitly state what exactly it's going to do. `auto' sounds
>>> too general and doesn't tell me that much. I'm probably the only
>>> here who can't see a connection between "auto" and "SPCR", but
>>> still.
>>
>> I came up with "auto" because I think it is generic.  I also thought about
>> "console=fw", or just "console".  If in the future another arch wants to
>> optionally bring up a firmware or hardware defined console then they could use
>> auto too.
> 
> Hmm, I see your point.
> My [sort of a] problem with "auto" is that it tells me as much as "magic"
> [and "magic" tells me almost nothing]. By the way, would be fun if we had
> "magic" instead of "auto" all over the kernel
> 
> 	echo "magic" > /sys/bus/usb/...../power/control
> 
>>> 	void arch_console_setup(void)
>>> 	{
>>> 		if (acpi_parse_spcr(false, true))
>>> 			pr_err(.........);
>>> 	}
>>>
>>> There can be other consoles in the system, logging an error is not
>>> such a useless thing.
>>
>> I can make the second change.  The problem (IIRC) with returning an error in an
>> setup fn is that the rest of the setup functions will not execute.  I don't want
>> to fail the setup callbacks because of an incorrect SPCR table.
> 
> OK, fair enough.
> Letting users know that SPCR is incorrect also makes sense, so option #2
> I guess is what we want after all.
> 
>> Like I mentioned to Petr, I'd like to know if you (or anyone else) has strong
>> feelings about changing the behaviour of earlycon on x86?  I could make it so
>> that specifying just earlycon would also initialize the console.
> 
> x86 people and/or scheduler people might have strong opinions on this.
> I Cc-ed Peter Zijlstra; he represents both groups and is known to be
> a hardcore earlycon user.

Thanks, I'm a user of earlycon too, but only moderately.

peterz, to give you an overview:  Currently on x86, the SPCR information is only
interpreted by the early console code, and can be enabled with kernel parameter
"earlycon" (no arguments).  In this patch I'm proposing adding "console=auto"
that would enable the console based on the SPCR data.

There are two options on the table.  One, we could go with this patch which
would make users do "earlycon console=auto" or, I could just change the
behaviour of earlycon (no arguments) to also bring up the console.  In the
second case the kernel parameter would just be "earlycon".  There is precedent
for the second option as arm64 enables both the earlycon and console by default
if SPCR is present.  However, on x86, I know many users do not want the console
enabled by default so we should keep it on demand.

tl;dr: Pick one

Option 1: earlycon enables both the early console and console.
Option 2: earlycon only enables the early console, and console=auto enables the
console.

P.

> 
> 	-ss
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] console: Add console=auto option
  2018-08-17 11:06                 ` Prarit Bhargava
@ 2018-08-17 12:57                   ` Petr Mladek
  2018-08-29 12:54                     ` Prarit Bhargava
  0 siblings, 1 reply; 14+ messages in thread
From: Petr Mladek @ 2018-08-17 12:57 UTC (permalink / raw)
  To: Prarit Bhargava
  Cc: Sergey Senozhatsky, linux-kernel, Mark Salter, Al Stone,
	Rafael J. Wysocki, Len Brown, Pavel Machek, x86,
	Sergey Senozhatsky, Steven Rostedt, Kees Cook,
	Greg Kroah-Hartman, linux-pm, Peter Zijlstra

On Fri 2018-08-17 07:06:56, Prarit Bhargava wrote:
> On 08/17/2018 06:50 AM, Sergey Senozhatsky wrote:
> >> Like I mentioned to Petr, I'd like to know if you (or anyone else) has strong
> >> feelings about changing the behaviour of earlycon on x86?  I could make it so
> >> that specifying just earlycon would also initialize the console.
> > 
> > x86 people and/or scheduler people might have strong opinions on this.
> > I Cc-ed Peter Zijlstra; he represents both groups and is known to be
> > a hardcore earlycon user.
> 
> Thanks, I'm a user of earlycon too, but only moderately.
> 
> peterz, to give you an overview:  Currently on x86, the SPCR information is only
> interpreted by the early console code, and can be enabled with kernel parameter
> "earlycon" (no arguments).  In this patch I'm proposing adding "console=auto"
> that would enable the console based on the SPCR data.
> 
> There are two options on the table.  One, we could go with this patch which
> would make users do "earlycon console=auto" or, I could just change the
> behaviour of earlycon (no arguments) to also bring up the console.  In the
> second case the kernel parameter would just be "earlycon".  There is precedent
> for the second option as arm64 enables both the earlycon and console by default
> if SPCR is present.  However, on x86, I know many users do not want the console
> enabled by default so we should keep it on demand.
> 
> tl;dr: Pick one
> 
> Option 1: earlycon enables both the early console and console.

I am afraid that this is not acceptable. Users are sensitive
when a new kernel suddenly enables different consoles. For example,
see:

+ commit dac8bbbae1d0ccba9 ("Revert "printk: fix double printing with
  earlycon")

+ commit c6c7d83b9c9e6a8b3 ("Revert "console: don't prefer first
  registered if DT specifies stdout-path")


By other words, the new behavior must depend on a new option.


> Option 2: earlycon only enables the early console, and console=auto enables the
> console.

I suggest:

Option 3: "earlycon" enables early console defined by SPCR
	  "console" enables console defined by SPCR

I mean that "console" without extra options would enable the console
defined by ACPI SPCR. It would work the same as "earlycon" without
extra options for early consoles.

If you would want to make it explicit than I agree with Sergey
and would prefer "console=spcr" instead of "console=auto".

Note kernel tries to enable some consoles automatically even
when "console" parameter is not defined at all. Therefore "auto"
is somehow misleading.

Best Regards,
Petr

PS: JFYI, I have vacation the following week...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] console: Add console=auto option
  2018-08-17 12:57                   ` Petr Mladek
@ 2018-08-29 12:54                     ` Prarit Bhargava
  0 siblings, 0 replies; 14+ messages in thread
From: Prarit Bhargava @ 2018-08-29 12:54 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, linux-kernel, Mark Salter, Al Stone,
	Rafael J. Wysocki, Len Brown, Pavel Machek, x86,
	Sergey Senozhatsky, Steven Rostedt, Kees Cook,
	Greg Kroah-Hartman, linux-pm, Peter Zijlstra



On 08/17/2018 08:57 AM, Petr Mladek wrote:
> On Fri 2018-08-17 07:06:56, Prarit Bhargava wrote:
>> On 08/17/2018 06:50 AM, Sergey Senozhatsky wrote:
>>>> Like I mentioned to Petr, I'd like to know if you (or anyone else) has strong
>>>> feelings about changing the behaviour of earlycon on x86?  I could make it so
>>>> that specifying just earlycon would also initialize the console.
>>>
>>> x86 people and/or scheduler people might have strong opinions on this.
>>> I Cc-ed Peter Zijlstra; he represents both groups and is known to be
>>> a hardcore earlycon user.
>>
>> Thanks, I'm a user of earlycon too, but only moderately.
>>
>> peterz, to give you an overview:  Currently on x86, the SPCR information is only
>> interpreted by the early console code, and can be enabled with kernel parameter
>> "earlycon" (no arguments).  In this patch I'm proposing adding "console=auto"
>> that would enable the console based on the SPCR data.
>>
>> There are two options on the table.  One, we could go with this patch which
>> would make users do "earlycon console=auto" or, I could just change the
>> behaviour of earlycon (no arguments) to also bring up the console.  In the
>> second case the kernel parameter would just be "earlycon".  There is precedent
>> for the second option as arm64 enables both the earlycon and console by default
>> if SPCR is present.  However, on x86, I know many users do not want the console
>> enabled by default so we should keep it on demand.
>>
>> tl;dr: Pick one
>>
>> Option 1: earlycon enables both the early console and console.
> 
> I am afraid that this is not acceptable. Users are sensitive
> when a new kernel suddenly enables different consoles. For example,
> see:
> 
> + commit dac8bbbae1d0ccba9 ("Revert "printk: fix double printing with
>   earlycon")
> 
> + commit c6c7d83b9c9e6a8b3 ("Revert "console: don't prefer first
>   registered if DT specifies stdout-path")
> 
> 
> By other words, the new behavior must depend on a new option.
> 
> 
>> Option 2: earlycon only enables the early console, and console=auto enables the
>> console.
> 
> I suggest:
> 
> Option 3: "earlycon" enables early console defined by SPCR
> 	  "console" enables console defined by SPCR
> 
> I mean that "console" without extra options would enable the console
> defined by ACPI SPCR. It would work the same as "earlycon" without
> extra options for early consoles.
> 
> If you would want to make it explicit than I agree with Sergey
> and would prefer "console=spcr" instead of "console=auto".

I'm going with "console=spcr".  FYI.

P.

> 
> Note kernel tries to enable some consoles automatically even
> when "console" parameter is not defined at all. Therefore "auto"
> is somehow misleading.
> 
> Best Regards,
> Petr
> 
> PS: JFYI, I have vacation the following week...
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-08-29 12:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-16 14:10 [PATCH] console: Add console=auto option Prarit Bhargava
2018-08-16 15:09 ` Greg Kroah-Hartman
2018-08-16 15:28   ` Prarit Bhargava
2018-08-16 15:33     ` Steven Rostedt
2018-08-16 17:23       ` Prarit Bhargava
2018-08-16 17:39         ` [PATCH v2] " Prarit Bhargava
2018-08-17  8:19           ` Petr Mladek
2018-08-17 10:26             ` Prarit Bhargava
2018-08-17  9:38           ` Sergey Senozhatsky
2018-08-17 10:36             ` Prarit Bhargava
2018-08-17 10:50               ` Sergey Senozhatsky
2018-08-17 11:06                 ` Prarit Bhargava
2018-08-17 12:57                   ` Petr Mladek
2018-08-29 12:54                     ` Prarit Bhargava

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).