[kvm-unit-tests PATCH 1/2] riscv: Allow SBI_CONSOLE with no uart in device tree

[kvm-unit-tests PATCH 1/2] riscv: Allow SBI_CONSOLE with no uart in device tree

[Jesse Taube]

When CONFIG_SBI_CONSOLE is enabled and there is no uart defined in the
device tree kvm-unit-tests fails to start.

Only check if uart exists in device tree if SBI_CONSOLE is false.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
 lib/riscv/io.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/riscv/io.c b/lib/riscv/io.c
index fb40adb7..96a3c048 100644
--- a/lib/riscv/io.c
+++ b/lib/riscv/io.c
@@ -104,6 +104,7 @@ static void uart0_init_acpi(void)
 
 void io_init(void)
 {
+#ifndef CONFIG_SBI_CONSOLE
 	if (dt_available())
 		uart0_init_fdt();
 	else
@@ -114,6 +115,7 @@ void io_init(void)
 		       "Found uart at %p, but early base is %p.\n",
 		       uart0_base, UART_EARLY_BASE);
 	}
+#endif
 }
 
 #ifdef CONFIG_SBI_CONSOLE
-- 
2.43.0

Re: [kvm-unit-tests PATCH 1/2] riscv: Allow SBI_CONSOLE with no uart in device tree

[Andrew Jones]

On Thu, Jun 12, 2025 at 01:07:47PM -0700, Jesse Taube wrote:
> When CONFIG_SBI_CONSOLE is enabled and there is no uart defined in the
> device tree kvm-unit-tests fails to start.
> 
> Only check if uart exists in device tree if SBI_CONSOLE is false.
> 
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> ---
>  lib/riscv/io.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/riscv/io.c b/lib/riscv/io.c
> index fb40adb7..96a3c048 100644
> --- a/lib/riscv/io.c
> +++ b/lib/riscv/io.c
> @@ -104,6 +104,7 @@ static void uart0_init_acpi(void)
>  
>  void io_init(void)
>  {
> +#ifndef CONFIG_SBI_CONSOLE
>  	if (dt_available())
>  		uart0_init_fdt();
>  	else
> @@ -114,6 +115,7 @@ void io_init(void)
>  		       "Found uart at %p, but early base is %p.\n",
>  		       uart0_base, UART_EARLY_BASE);
>  	}
> +#endif

Doesn't this generate uart0_init_fdt/acpi defined but not used types of
warnings? We need to put everything unused under the #ifndef
CONFIG_SBI_CONSOLE, just as uart0_read/write already are. Alternatively,
we can keep everything out of the #ifndef and export
 
void sbi_puts(const char *s);
void uart0_puts(const char *s);

then just have a single use of the #ifdef,

void puts(const char *s)
{
#ifdef CONFIG_SBI_CONSOLE
    sbi_puts(s);
#else
    uart0_puts(s);
#endif
}

I think I like that better since it will ensure all code gets compile
tested all the time and allow an SBI console using unit test to also
access its uart if it has one, just because...

Thanks,
drew


>  }
>  
>  #ifdef CONFIG_SBI_CONSOLE
> -- 
> 2.43.0
> 
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [kvm-unit-tests PATCH 1/2] riscv: Allow SBI_CONSOLE with no uart in device tree

[Jesse Taube]

On Fri, Jun 13, 2025 at 12:53 AM Andrew Jones <andrew.jones@linux.dev> wrote:
>
> On Thu, Jun 12, 2025 at 01:07:47PM -0700, Jesse Taube wrote:
> > When CONFIG_SBI_CONSOLE is enabled and there is no uart defined in the
> > device tree kvm-unit-tests fails to start.
> >
> > Only check if uart exists in device tree if SBI_CONSOLE is false.
> >
> > Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> > ---
> >  lib/riscv/io.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/riscv/io.c b/lib/riscv/io.c
> > index fb40adb7..96a3c048 100644
> > --- a/lib/riscv/io.c
> > +++ b/lib/riscv/io.c
> > @@ -104,6 +104,7 @@ static void uart0_init_acpi(void)
> >
> >  void io_init(void)
> >  {
> > +#ifndef CONFIG_SBI_CONSOLE
> >       if (dt_available())
> >               uart0_init_fdt();
> >       else
> > @@ -114,6 +115,7 @@ void io_init(void)
> >                      "Found uart at %p, but early base is %p.\n",
> >                      uart0_base, UART_EARLY_BASE);
> >       }
> > +#endif
>
> Doesn't this generate uart0_init_fdt/acpi defined but not used types of
> warnings?

No, but it should have, I'll fix it.

> We need to put everything unused under the #ifndef
> CONFIG_SBI_CONSOLE, just as uart0_read/write already are. Alternatively,
> we can keep everything out of the #ifndef and export.

The problem is uart0_init_fdt panics if no compatible uart was found.

Thanks,
Jesse Taube
> void sbi_puts(const char *s);
> void uart0_puts(const char *s);
>
> then just have a single use of the #ifdef,
>
> void puts(const char *s)
> {
> #ifdef CONFIG_SBI_CONSOLE
>     sbi_puts(s);
> #else
>     uart0_puts(s);
> #endif
> }
>
> I think I like that better since it will ensure all code gets compile
> tested all the time and allow an SBI console using unit test to also
> access its uart if it has one, just because...
>
> Thanks,
> drew
>
>
> >  }
> >
> >  #ifdef CONFIG_SBI_CONSOLE
> > --
> > 2.43.0
> >
> >
> > --
> > kvm-riscv mailing list
> > kvm-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [kvm-unit-tests PATCH 1/2] riscv: Allow SBI_CONSOLE with no uart in device tree

[Andrew Jones]

On Fri, Jun 13, 2025 at 05:11:19AM -0700, Jesse Taube wrote:
> On Fri, Jun 13, 2025 at 12:53 AM Andrew Jones <andrew.jones@linux.dev> wrote:
> >
> > On Thu, Jun 12, 2025 at 01:07:47PM -0700, Jesse Taube wrote:
> > > When CONFIG_SBI_CONSOLE is enabled and there is no uart defined in the
> > > device tree kvm-unit-tests fails to start.
> > >
> > > Only check if uart exists in device tree if SBI_CONSOLE is false.
> > >
> > > Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> > > ---
> > >  lib/riscv/io.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/lib/riscv/io.c b/lib/riscv/io.c
> > > index fb40adb7..96a3c048 100644
> > > --- a/lib/riscv/io.c
> > > +++ b/lib/riscv/io.c
> > > @@ -104,6 +104,7 @@ static void uart0_init_acpi(void)
> > >
> > >  void io_init(void)
> > >  {
> > > +#ifndef CONFIG_SBI_CONSOLE
> > >       if (dt_available())
> > >               uart0_init_fdt();
> > >       else
> > > @@ -114,6 +115,7 @@ void io_init(void)
> > >                      "Found uart at %p, but early base is %p.\n",
> > >                      uart0_base, UART_EARLY_BASE);
> > >       }
> > > +#endif
> >
> > Doesn't this generate uart0_init_fdt/acpi defined but not used types of
> > warnings?
> 
> No, but it should have, I'll fix it.
> 
> > We need to put everything unused under the #ifndef
> > CONFIG_SBI_CONSOLE, just as uart0_read/write already are. Alternatively,
> > we can keep everything out of the #ifndef and export.
> 
> The problem is uart0_init_fdt panics if no compatible uart was found.

Let's do

#ifndef CONFIG_SBI_CONSOLE
   if (ret) {
       printf("%s: Compatible uart not found in the device tree, aborting...\n",__func__);
       abort();
   }
#else
   uart0_base = NULL;
   return;
#endif

And then in uart0_puts() do a 

  assert(uart0_base);

This rules out mapping a uart at physical address zero, but hopefully
that's not a thing. If it ever does appear to be a thing, then we can
just add a boolean to track whether the base address is valid or not.
But we'll burn that bridge when we cross it later, if ever.

Thanks,
drew

> 
> Thanks,
> Jesse Taube
> > void sbi_puts(const char *s);
> > void uart0_puts(const char *s);
> >
> > then just have a single use of the #ifdef,
> >
> > void puts(const char *s)
> > {
> > #ifdef CONFIG_SBI_CONSOLE
> >     sbi_puts(s);
> > #else
> >     uart0_puts(s);
> > #endif
> > }
> >
> > I think I like that better since it will ensure all code gets compile
> > tested all the time and allow an SBI console using unit test to also
> > access its uart if it has one, just because...
> >
> > Thanks,
> > drew
> >
> >
> > >  }
> > >
> > >  #ifdef CONFIG_SBI_CONSOLE
> > > --
> > > 2.43.0
> > >
> > >
> > > --
> > > kvm-riscv mailing list
> > > kvm-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/kvm-riscv
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv