diff mbox series

tty: hvc: Don't enable the RISC-V SBI console by default

Message ID 20240214153429.16484-2-palmer@rivosinc.com
State New
Headers
Series tty: hvc: Don't enable the RISC-V SBI console by default |

Commit Message

Palmer Dabbelt Feb. 14, 2024, 3:34 p.m. UTC 
  From: Palmer Dabbelt <palmer@rivosinc.com>

The new SBI console has the same problem as the old one: there's only
one shared backing hardware and no synchronization, so the two drivers
end up stepping on each other.  This was the same issue the old SBI-0.1
console drivers had, but that was disabled by default when SBI-0.1 was.

So just mark the new driver as nonportable.

Reported-by: Emil Renner Berthing <kernel@esmil.dk>
Fixes: 88ead68e764c ("tty: Add SBI debug console support to HVC SBI driver")
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 drivers/tty/hvc/Kconfig | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Anup Patel Feb. 14, 2024, 3:50 p.m. UTC | #1 
On Wed, Feb 14, 2024 at 9:06 PM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> From: Palmer Dabbelt <palmer@rivosinc.com>
>
> The new SBI console has the same problem as the old one: there's only
> one shared backing hardware and no synchronization, so the two drivers
> end up stepping on each other.  This was the same issue the old SBI-0.1
> console drivers had, but that was disabled by default when SBI-0.1 was.
>
> So just mark the new driver as nonportable.
>
> Reported-by: Emil Renner Berthing <kernel@esmil.dk>
> Fixes: 88ead68e764c ("tty: Add SBI debug console support to HVC SBI driver")
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>

LTGM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  drivers/tty/hvc/Kconfig | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
> index 6e05c5c7bca1..c2a4e88b328f 100644
> --- a/drivers/tty/hvc/Kconfig
> +++ b/drivers/tty/hvc/Kconfig
> @@ -108,13 +108,15 @@ config HVC_DCC_SERIALIZE_SMP
>
>  config HVC_RISCV_SBI
>         bool "RISC-V SBI console support"
> -       depends on RISCV_SBI
> +       depends on RISCV_SBI && NONPORTABLE
>         select HVC_DRIVER
>         help
>           This enables support for console output via RISC-V SBI calls, which
> -         is normally used only during boot to output printk.
> +         is normally used only during boot to output printk.  This driver
> +         conflicts with real console drivers and should not be enabled on
> +         systems that directly access the console.
>
> -         If you don't know what do to here, say Y.
> +         If you don't know what do to here, say N.
>
>  config HVCS
>         tristate "IBM Hypervisor Virtual Console Server support"
> --
> 2.43.0
>
>
Conor Dooley Feb. 14, 2024, 3:54 p.m. UTC | #2 
On Wed, Feb 14, 2024 at 07:34:30AM -0800, Palmer Dabbelt wrote:
> From: Palmer Dabbelt <palmer@rivosinc.com>
> 
> The new SBI console has the same problem as the old one: there's only
> one shared backing hardware and no synchronization, so the two drivers
> end up stepping on each other.  This was the same issue the old SBI-0.1
> console drivers had, but that was disabled by default when SBI-0.1 was.
> 
> So just mark the new driver as nonportable.
> 
> Reported-by: Emil Renner Berthing <kernel@esmil.dk>
> Fixes: 88ead68e764c ("tty: Add SBI debug console support to HVC SBI driver")
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>

As was brought up when we covered this earlier today, if you're going to
probe a driver based on an ecall, the same hardware should not remain
enabled in the DT passed to the kernel.
If you want to enable this driver in a multiplatform kernel alongside
"real" drivers, then the solution is simple, firmware needs implementation
needs to patch the DT and, at least, mark the uart as reserved if it is
using it to provide the debug console. Marking this nonportable so that
people only walk into this with their eyes open seems like a reasonable
action to me.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> ---
>  drivers/tty/hvc/Kconfig | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
> index 6e05c5c7bca1..c2a4e88b328f 100644
> --- a/drivers/tty/hvc/Kconfig
> +++ b/drivers/tty/hvc/Kconfig
> @@ -108,13 +108,15 @@ config HVC_DCC_SERIALIZE_SMP
>  
>  config HVC_RISCV_SBI
>  	bool "RISC-V SBI console support"
> -	depends on RISCV_SBI
> +	depends on RISCV_SBI && NONPORTABLE
>  	select HVC_DRIVER
>  	help
>  	  This enables support for console output via RISC-V SBI calls, which
> -	  is normally used only during boot to output printk.
> +	  is normally used only during boot to output printk.  This driver
> +	  conflicts with real console drivers and should not be enabled on
> +	  systems that directly access the console.
>  
> -	  If you don't know what do to here, say Y.
> +	  If you don't know what do to here, say N.
>  
>  config HVCS
>  	tristate "IBM Hypervisor Virtual Console Server support"
> -- 
> 2.43.0
> 
>
patchwork-bot+linux-riscv@kernel.org Feb. 21, 2024, 7:50 p.m. UTC | #3 
Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Wed, 14 Feb 2024 07:34:30 -0800 you wrote:
> From: Palmer Dabbelt <palmer@rivosinc.com>
> 
> The new SBI console has the same problem as the old one: there's only
> one shared backing hardware and no synchronization, so the two drivers
> end up stepping on each other.  This was the same issue the old SBI-0.1
> console drivers had, but that was disabled by default when SBI-0.1 was.
> 
> [...]

Here is the summary with links:
  - tty: hvc: Don't enable the RISC-V SBI console by default
    https://git.kernel.org/riscv/c/481860974faa

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
index 6e05c5c7bca1..c2a4e88b328f 100644
--- a/drivers/tty/hvc/Kconfig
+++ b/drivers/tty/hvc/Kconfig
@@ -108,13 +108,15 @@  config HVC_DCC_SERIALIZE_SMP
 
 config HVC_RISCV_SBI
 	bool "RISC-V SBI console support"
-	depends on RISCV_SBI
+	depends on RISCV_SBI && NONPORTABLE
 	select HVC_DRIVER
 	help
 	  This enables support for console output via RISC-V SBI calls, which
-	  is normally used only during boot to output printk.
+	  is normally used only during boot to output printk.  This driver
+	  conflicts with real console drivers and should not be enabled on
+	  systems that directly access the console.
 
-	  If you don't know what do to here, say Y.
+	  If you don't know what do to here, say N.
 
 config HVCS
 	tristate "IBM Hypervisor Virtual Console Server support"