[PATCH] tty: Don't force RISCV SBI console as preferred console

[PATCH] tty: Don't force RISCV SBI console as preferred console

[Anup Patel]

The Linux kernel will auto-disables all boot consoles whenever it
gets a preferred real console.

Currently on RISC-V systems, if we have a real console which is not
RISCV SBI console then boot consoles (such as earlycon=sbi) are not
auto-disabled when a real console (ttyS0 or ttySIF0) is available.
This results in duplicate prints at boot-time after kernel starts
using real console (i.e. ttyS0 or ttySIF0) if "earlycon=" kernel
parameter was passed by bootloader.

The reason for above issue is that RISCV SBI console always adds
itself as preferred console which is causing other real consoles
to be not used as preferred console.

Ideally "console=" kernel parameter passed by bootloaders should
be the one selecting a preferred real console.

This patch fixes above issue by not forcing RISCV SBI console as
preferred console.

Fixes: afa6b1ccfad5 ("tty: New RISC-V SBI console driver")
Cc: stable@vger.kernel.org
Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 drivers/tty/hvc/hvc_riscv_sbi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
index 75155bde2b88..31f53fa77e4a 100644
--- a/drivers/tty/hvc/hvc_riscv_sbi.c
+++ b/drivers/tty/hvc/hvc_riscv_sbi.c
@@ -53,7 +53,6 @@ device_initcall(hvc_sbi_init);
 static int __init hvc_sbi_console_init(void)
 {
 	hvc_instantiate(0, 0, &hvc_sbi_ops);
-	add_preferred_console("hvc", 0, NULL);
 
 	return 0;
 }
-- 
2.17.1

Re: [PATCH] tty: Don't force RISCV SBI console as preferred console

[Atish Patra]

On 4/25/19 6:35 AM, Anup Patel wrote:
> The Linux kernel will auto-disables all boot consoles whenever it
> gets a preferred real console.
> 
> Currently on RISC-V systems, if we have a real console which is not
> RISCV SBI console then boot consoles (such as earlycon=sbi) are not
> auto-disabled when a real console (ttyS0 or ttySIF0) is available.
> This results in duplicate prints at boot-time after kernel starts
> using real console (i.e. ttyS0 or ttySIF0) if "earlycon=" kernel
> parameter was passed by bootloader.
> 
> The reason for above issue is that RISCV SBI console always adds
> itself as preferred console which is causing other real consoles
> to be not used as preferred console.
> 

Do we even need HVC_SBI console to be enabled by default? Disabling 
CONFIG_HVC_RISCV_SBI seems to be fine while running in QEMU.

If we don't need it, I suggest we should remove the config option from 
defconfig in addition to this patch.

Regards,
Atish
> Ideally "console=" kernel parameter passed by bootloaders should
> be the one selecting a preferred real console.
> 
> This patch fixes above issue by not forcing RISCV SBI console as
> preferred console.
> 
> Fixes: afa6b1ccfad5 ("tty: New RISC-V SBI console driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>   drivers/tty/hvc/hvc_riscv_sbi.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
> index 75155bde2b88..31f53fa77e4a 100644
> --- a/drivers/tty/hvc/hvc_riscv_sbi.c
> +++ b/drivers/tty/hvc/hvc_riscv_sbi.c
> @@ -53,7 +53,6 @@ device_initcall(hvc_sbi_init);
>   static int __init hvc_sbi_console_init(void)
>   {
>   	hvc_instantiate(0, 0, &hvc_sbi_ops);
> -	add_preferred_console("hvc", 0, NULL);
>   
>   	return 0;
>   }
> 

Re: [PATCH] tty: Don't force RISCV SBI console as preferred console

[Anup Patel]

On Fri, Apr 26, 2019 at 10:11 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> On 4/25/19 6:35 AM, Anup Patel wrote:
> > The Linux kernel will auto-disables all boot consoles whenever it
> > gets a preferred real console.
> >
> > Currently on RISC-V systems, if we have a real console which is not
> > RISCV SBI console then boot consoles (such as earlycon=sbi) are not
> > auto-disabled when a real console (ttyS0 or ttySIF0) is available.
> > This results in duplicate prints at boot-time after kernel starts
> > using real console (i.e. ttyS0 or ttySIF0) if "earlycon=" kernel
> > parameter was passed by bootloader.
> >
> > The reason for above issue is that RISCV SBI console always adds
> > itself as preferred console which is causing other real consoles
> > to be not used as preferred console.
> >
>
> Do we even need HVC_SBI console to be enabled by default? Disabling
> CONFIG_HVC_RISCV_SBI seems to be fine while running in QEMU.

Actually, HVC_SBI console is useful on boards (such as SiFive Unleashed)
lacking upstream serial driver. It allows us to boot upstream kernel to prompt
on such boards with just timer driver (and probably irqchip driver).

Also, we should be able to use same kernel image on QEMU and SiFive
Unleashed board so disabling CONFIG_HVC_RISCV_SBI for QEMU is
a temporary solution.

>
> If we don't need it, I suggest we should remove the config option from
> defconfig in addition to this patch.

Like mentioned above, HVC_SBI is useful for newer SOCs and boards
where serial driver is not yet up-streamed.

Regards,
Anup

>
> Regards,
> Atish
> > Ideally "console=" kernel parameter passed by bootloaders should
> > be the one selecting a preferred real console.
> >
> > This patch fixes above issue by not forcing RISCV SBI console as
> > preferred console.
> >
> > Fixes: afa6b1ccfad5 ("tty: New RISC-V SBI console driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> >   drivers/tty/hvc/hvc_riscv_sbi.c | 1 -
> >   1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
> > index 75155bde2b88..31f53fa77e4a 100644
> > --- a/drivers/tty/hvc/hvc_riscv_sbi.c
> > +++ b/drivers/tty/hvc/hvc_riscv_sbi.c
> > @@ -53,7 +53,6 @@ device_initcall(hvc_sbi_init);
> >   static int __init hvc_sbi_console_init(void)
> >   {
> >       hvc_instantiate(0, 0, &hvc_sbi_ops);
> > -     add_preferred_console("hvc", 0, NULL);
> >
> >       return 0;
> >   }
> >
>

Re: [PATCH] tty: Don't force RISCV SBI console as preferred console

[Atish Patra]

On 4/25/19 11:21 PM, Anup Patel wrote:
> On Fri, Apr 26, 2019 at 10:11 AM Atish Patra <atish.patra@wdc.com> wrote:
>>
>> On 4/25/19 6:35 AM, Anup Patel wrote:
>>> The Linux kernel will auto-disables all boot consoles whenever it
>>> gets a preferred real console.
>>>
>>> Currently on RISC-V systems, if we have a real console which is not
>>> RISCV SBI console then boot consoles (such as earlycon=sbi) are not
>>> auto-disabled when a real console (ttyS0 or ttySIF0) is available.
>>> This results in duplicate prints at boot-time after kernel starts
>>> using real console (i.e. ttyS0 or ttySIF0) if "earlycon=" kernel
>>> parameter was passed by bootloader.
>>>
>>> The reason for above issue is that RISCV SBI console always adds
>>> itself as preferred console which is causing other real consoles
>>> to be not used as preferred console.
>>>
>>
>> Do we even need HVC_SBI console to be enabled by default? Disabling
>> CONFIG_HVC_RISCV_SBI seems to be fine while running in QEMU.
> 
> Actually, HVC_SBI console is useful on boards (such as SiFive Unleashed)
> lacking upstream serial driver. It allows us to boot upstream kernel to prompt
> on such boards with just timer driver (and probably irqchip driver).
> 
> Also, we should be able to use same kernel image on QEMU and SiFive
> Unleashed board so disabling CONFIG_HVC_RISCV_SBI for QEMU is
> a temporary solution.
> 
>>
>> If we don't need it, I suggest we should remove the config option from
>> defconfig in addition to this patch.
> 
> Like mentioned above, HVC_SBI is useful for newer SOCs and boards
> where serial driver is not yet up-streamed.
> 

Ok. Lets keep it then.

> Regards,
> Anup
> 
>>
>> Regards,
>> Atish
>>> Ideally "console=" kernel parameter passed by bootloaders should
>>> be the one selecting a preferred real console.
>>>
>>> This patch fixes above issue by not forcing RISCV SBI console as
>>> preferred console.
>>>
>>> Fixes: afa6b1ccfad5 ("tty: New RISC-V SBI console driver")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Anup Patel <anup.patel@wdc.com>
>>> ---
>>>    drivers/tty/hvc/hvc_riscv_sbi.c | 1 -
>>>    1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
>>> index 75155bde2b88..31f53fa77e4a 100644
>>> --- a/drivers/tty/hvc/hvc_riscv_sbi.c
>>> +++ b/drivers/tty/hvc/hvc_riscv_sbi.c
>>> @@ -53,7 +53,6 @@ device_initcall(hvc_sbi_init);
>>>    static int __init hvc_sbi_console_init(void)
>>>    {
>>>        hvc_instantiate(0, 0, &hvc_sbi_ops);
>>> -     add_preferred_console("hvc", 0, NULL);
>>>
>>>        return 0;
>>>    }
>>>
>>
> 

Reviewed-by: Atish Patra <atish.patra@wdc.com>

Regards,
Atish

Re: [PATCH] tty: Don't force RISCV SBI console as preferred console

[Christoph Hellwig]

On Thu, Apr 25, 2019 at 09:41:21PM -0700, Atish Patra wrote:
> Do we even need HVC_SBI console to be enabled by default? Disabling
> CONFIG_HVC_RISCV_SBI seems to be fine while running in QEMU.
> 
> If we don't need it, I suggest we should remove the config option from
> defconfig in addition to this patch.

I think the whole concept of the SBI console is a little dangerous.
It means that for one piece of physical hardware (usually the uart)
we have two entiries (the M-mode firmware and the OS) in control,
which tends to rarely end well.

Re: [PATCH] tty: Don't force RISCV SBI console as preferred console

[Anup Patel]

On Fri, Apr 26, 2019 at 11:51 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Apr 25, 2019 at 09:41:21PM -0700, Atish Patra wrote:
> > Do we even need HVC_SBI console to be enabled by default? Disabling
> > CONFIG_HVC_RISCV_SBI seems to be fine while running in QEMU.
> >
> > If we don't need it, I suggest we should remove the config option from
> > defconfig in addition to this patch.
>
> I think the whole concept of the SBI console is a little dangerous.
> It means that for one piece of physical hardware (usually the uart)
> we have two entiries (the M-mode firmware and the OS) in control,
> which tends to rarely end well.

I think the SBI console is only useful for early SOC bringup and early
SOC debugging when most drivers are not available in upstream
kernel. It cannot (and should not) be used in production deployments.

Regards,
Anup

Re: [PATCH] tty: Don't force RISCV SBI console as preferred console

[Paul Walmsley]

On Fri, 26 Apr 2019, Anup Patel wrote:

> On Fri, Apr 26, 2019 at 11:51 AM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Thu, Apr 25, 2019 at 09:41:21PM -0700, Atish Patra wrote:
> > > Do we even need HVC_SBI console to be enabled by default? Disabling
> > > CONFIG_HVC_RISCV_SBI seems to be fine while running in QEMU.
> > >
> > > If we don't need it, I suggest we should remove the config option from
> > > defconfig in addition to this patch.
> >
> > I think the whole concept of the SBI console is a little dangerous.
> > It means that for one piece of physical hardware (usually the uart)
> > we have two entiries (the M-mode firmware and the OS) in control,
> > which tends to rarely end well.
> 
> I think the SBI console is only useful for early SOC bringup and early
> SOC debugging when most drivers are not available in upstream
> kernel. It cannot (and should not) be used in production deployments.

Usually the primary use-case for an abstract console interface is for 
desktop and server users.  Usually Linux distributions want a hardware 
platform-specific bootloader or BIOS to specify and control the console.  

Originally I suspect this was implemented in the SBI for semi-hosting 
purposes, but that's no longer really applicable.


- Paul

Re: [PATCH] tty: Don't force RISCV SBI console as preferred console

[Palmer Dabbelt]

On Thu, 25 Apr 2019 06:35:06 PDT (-0700), Anup Patel wrote:
> The Linux kernel will auto-disables all boot consoles whenever it
> gets a preferred real console.
> 
> Currently on RISC-V systems, if we have a real console which is not
> RISCV SBI console then boot consoles (such as earlycon=sbi) are not
> auto-disabled when a real console (ttyS0 or ttySIF0) is available.
> This results in duplicate prints at boot-time after kernel starts
> using real console (i.e. ttyS0 or ttySIF0) if "earlycon=" kernel
> parameter was passed by bootloader.
> 
> The reason for above issue is that RISCV SBI console always adds
> itself as preferred console which is causing other real consoles
> to be not used as preferred console.
> 
> Ideally "console=" kernel parameter passed by bootloaders should
> be the one selecting a preferred real console.
> 
> This patch fixes above issue by not forcing RISCV SBI console as
> preferred console.
> 
> Fixes: afa6b1ccfad5 ("tty: New RISC-V SBI console driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  drivers/tty/hvc/hvc_riscv_sbi.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/tty/hvc/hvc_riscv_sbi.c b/drivers/tty/hvc/hvc_riscv_sbi.c
> index 75155bde2b88..31f53fa77e4a 100644
> --- a/drivers/tty/hvc/hvc_riscv_sbi.c
> +++ b/drivers/tty/hvc/hvc_riscv_sbi.c
> @@ -53,7 +53,6 @@ device_initcall(hvc_sbi_init);
>  static int __init hvc_sbi_console_init(void)
>  {
>  	hvc_instantiate(0, 0, &hvc_sbi_ops);
> -	add_preferred_console("hvc", 0, NULL);
>  
>  	return 0;
>  }
> -- 
> 2.17.1
> 

I merged this.  Also, it looks like Exchange is doing something to your patches
that makes them a bit difficult to merge.  If you don't have a way of fixing
that, can you include a pointer to a git tree with a signed commit/tag?