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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2041 bytes --]


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

[-- Attachment #2: Type: text/html, Size: 3069 bytes --]