On 17.12.2025 17:54, Oleksii Kurochko wrote:
> This commit adds support for legacy SBI extensions (version 0.1) in Xen
> for guest domains.
>
> The changes include:
> 1. Define all legacy SBI extension IDs (0x0 to 0x8) for better clarity and
> completeness.
> 2. Implement handling of legacy SBI extensions, starting with support for
> SBI_EXT_0_1_CONSOLE_{PUT,GET}CHAR.
I can't spot any actual support for GETCHAR.
> --- /dev/null
> +++ b/xen/arch/riscv/vsbi/legacy-extension.c
> @@ -0,0 +1,65 @@
> +
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/console.h>
> +#include <xen/lib.h>
> +#include <xen/sched.h>
> +
> +#include <asm/processor.h>
> +#include <asm/vsbi.h>
> +
> +static void vsbi_print_line(char c)
Misleading function name? The parameter doesn't fit the name, and ...
> +{
> + struct domain *cd = current->domain;
I guess you copied this code from somewhere, but a variable of this type and
contents wants to be named "currd".
> + struct domain_console *cons = cd->console;
> +
> + if ( !is_console_printable(c) )
> + return;
> +
> + spin_lock(&cons->lock);
> + ASSERT(cons->idx < ARRAY_SIZE(cons->buf));
> + if ( c != '\n' )
> + cons->buf[cons->idx++] = c;
> + if ( (cons->idx == (ARRAY_SIZE(cons->buf) - 1)) || (c == '\n') )
> + {
> + cons->buf[cons->idx] = '\0';
> + guest_printk(cd, XENLOG_G_DEBUG "%s\n", cons->buf);
... you also only print a line under certain conditions.
> + cons->idx = 0;
> + }
> + spin_unlock(&cons->lock);
> +}
> +
> +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:
> + vsbi_print_line((char)regs->a0);
The cast isn't really needed, is it? And just to double-check: The spec demands
the upper bits to be ignored? (A link to the spec could have been useful, e.g.
in the cover letter.)
> + break;
> +
> + case SBI_EXT_0_1_CONSOLE_GETCHAR:
> + ret = SBI_ERR_NOT_SUPPORTED;
> + break;
> +
> + default:
> + /*
> + * TODO: 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.
> + */
Question then is why SBI_EXT_0_1_CONSOLE_GETCHAR gets a separate case block.
Jan
