Re: [PATCH v1 2/3] xen/riscv: add RISC-V legacy SBI extension support for guests

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.12.2025 11:29, Oleksii Kurochko wrote:
> On 12/8/25 4:05 PM, Jan Beulich wrote:
>> On 01.12.2025 11:24, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/vsbi/vsbi-legacy-extension.c
>>> @@ -0,0 +1,37 @@
>>> +
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +
>>> +#include <xen/lib.h>
>>> +#include <xen/sched.h>
>>> +
>>> +#include <asm/processor.h>
>>> +#include <asm/vsbi.h>
>>> +
>>> +static int vsbi_legacy_ecall_handler(struct vcpu *vcpu, unsigned long eid,
>>> +                                     unsigned long fid,
>>> +                                     struct cpu_user_regs *regs)
>>> +{
>>> +    int ret = 0;
>>> +
>>> +    switch ( eid )
>>> +    {
>>> +    case SBI_EXT_0_1_CONSOLE_PUTCHAR:
>>> +        printk("%c", (char)regs->a0);
>> This is guest output, so shouldn't use plain printk().
> 
> I think that I don't know what should be used instead. Could you suggest me 
> something
> or point to the code in other arch-s?
> 
> Or do you mean that guest_printk() should be used?

No direct replacement will do what you want, as they all prefix something to the
string passed (which isn't what you want). You may need to buffer characters and
emit them in batches (full lines unless overly long). For x86 see 
hvm_print_line(),
but I think Arm also has something like this.

>>> +    default:
>>> +        panic("%s: Unsupported ecall: FID: #%lx, EID: #%lx\n",
>>> +              __func__, fid, eid);
>> Please don't. domain_crash() may be okay to use here, but crashing the 
>> hypervisor
>> because of unexpected guest input isn't okay.
> 
> |domain_crash()| is better. I also considered just 
> returning|SBI_ERR_NOT_SUPPORTED|,
> but it wasn’t too convenient for debugging which FID/EID the guest was called,
> so I started using|panic()| instead.

FTAOD - domain_crash() is acceptable here while things are still under 
development.
It shouldn't stay like this in the end though: Guests should not be punished 
like
this for something Xen hasn't implemented.

Jan