Re: [PATCH v1] xen/riscv: add initialization support for virtual SBI UART (vSBI UART)

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 5/15/25 12:08 PM, Jan Beulich wrote:

> On 12.05.2025 17:55, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/Makefile
> > +++ b/xen/arch/riscv/Makefile
> > @@ -1,5 +1,6 @@
> >  obj-y += aplic.o
> >  obj-y += cpufeature.o
> > +obj-y += dom0less-build.o
> >       
> Arm uses
>
> obj-$(CONFIG_DOM0LESS_BOOT) += dom0less-build.init.o
>
> Why the two differences?
>     
Missed that. I have to understand why Arm uses *.init.o, but likely I should do the same for RISC-V.

> > --- /dev/null
> > +++ b/xen/arch/riscv/dom0less-build.c
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#include <xen/bug.h>
> > +#include <xen/device_tree.h>
> > +#include <xen/errno.h>
> > +#include <xen/fdt-kernel.h>
> > +#include <xen/init.h>
> > +#include <xen/sched.h>
> > +
> > +#include <asm/vsbi-uart.h>
> > +
> > +int __init init_vuart(struct domain *d, struct kernel_info *kinfo,
> > +                      const struct dt_device_node *node)
> > +{
> > +    int rc = -EINVAL;
> > +
> > +    kinfo->arch.vsbi_uart = dt_property_read_bool(node, "vsbi_uart");
> > +
> > +    if ( kinfo->arch.vsbi_uart )
> > +    {
> > +        rc = domain_vsbi_uart_init(d, NULL);
> > +        if ( rc < 0 )
> > +            return rc;
> > +    }
> > +
> > +    if ( rc )
> > +        panic("%s: what a domain should use as an UART?\n", __func__);
> >       
> Is this a reason to panic()? Isn't it possible for domains to be fine
> without any UART?
>     
Good point.
I think that it is possible to leave without UART. At this stage,
of development I need UART for debug, so automatically the panic() was added.
But I agree, it should be dropped.

> > +    domain_vsbi_uart_deinit(d);
> > +
> > +    return rc;
> > +}
> > +
> > +void domain_vsbi_uart_deinit(struct domain *d)
> > +{
> > +    struct vsbi_uart *vsbi_uart = &d->arch.vsbi_uart;
> > +
> > +    if ( vsbi_uart->backend_in_domain )
> > +        printk("%s: backed in a domain isn't supported\n", __func__);
> >       
> Is this relevant in a de-init function?
>     
Probably not, it was just added to not forget to update domain_vsbi_uart_deinit().

Thanks.

~ Oleksii