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.
The implementation uses the existing virtual SBI framework to handle legacy
SBI ecalls, ensuring compatibility with older SBI specifications in
RISC-V guests.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
xen/arch/riscv/include/asm/sbi.h | 11 ++++--
xen/arch/riscv/vsbi/Makefile | 1 +
xen/arch/riscv/vsbi/vsbi-legacy-extension.c | 37 +++++++++++++++++++++
3 files changed, 47 insertions(+), 2 deletions(-)
create mode 100644 xen/arch/riscv/vsbi/vsbi-legacy-extension.c
diff --git a/xen/arch/riscv/include/asm/sbi.h b/xen/arch/riscv/include/asm/sbi.h
index ade24a572d..e7d5d707b1 100644
--- a/xen/arch/riscv/include/asm/sbi.h
+++ b/xen/arch/riscv/include/asm/sbi.h
@@ -14,8 +14,15 @@
#include <xen/cpumask.h>
-#define SBI_EXT_0_1_CONSOLE_PUTCHAR 0x1
-#define SBI_EXT_0_1_SHUTDOWN 0x8
+#define SBI_EXT_0_1_SET_TIMER 0x0
+#define SBI_EXT_0_1_CONSOLE_PUTCHAR 0x1
+#define SBI_EXT_0_1_CONSOLE_GETCHAR 0x2
+#define SBI_EXT_0_1_CLEAR_IPI 0x3
+#define SBI_EXT_0_1_SEND_IPI 0x4
+#define SBI_EXT_0_1_REMOTE_FENCE_I 0x5
+#define SBI_EXT_0_1_REMOTE_SFENCE_VMA 0x6
+#define SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID 0x7
+#define SBI_EXT_0_1_SHUTDOWN 0x8
#define SBI_EXT_BASE 0x10
#define SBI_EXT_RFENCE 0x52464E43
diff --git a/xen/arch/riscv/vsbi/Makefile b/xen/arch/riscv/vsbi/Makefile
index 574c8ff78d..4da625db9a 100644
--- a/xen/arch/riscv/vsbi/Makefile
+++ b/xen/arch/riscv/vsbi/Makefile
@@ -1 +1,2 @@
obj-y += vsbi.o
+obj-y += vsbi-legacy-extension.o
diff --git a/xen/arch/riscv/vsbi/vsbi-legacy-extension.c b/xen/arch/riscv/vsbi/vsbi-legacy-extension.c
new file mode 100644
index 0000000000..39d65931b1
--- /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);
+ break;
+
+ case SBI_EXT_0_1_CONSOLE_GETCHAR:
+ regs->a0 = SBI_ERR_NOT_SUPPORTED;
+ break;
+
+ default:
+ panic("%s: Unsupported ecall: FID: #%lx, EID: #%lx\n",
+ __func__, fid, eid);
+ break;
+ }
+
+ return ret;
+}
+
+VSBI_EXT_START(legacy, SBI_EXT_0_1_SET_TIMER, SBI_EXT_0_1_SHUTDOWN,
+ vsbi_legacy_ecall_handler)
+VSBI_EXT_END
--
2.52.0On 01.12.2025 11:24, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/include/asm/sbi.h
> +++ b/xen/arch/riscv/include/asm/sbi.h
> @@ -14,8 +14,15 @@
>
> #include <xen/cpumask.h>
>
> -#define SBI_EXT_0_1_CONSOLE_PUTCHAR 0x1
> -#define SBI_EXT_0_1_SHUTDOWN 0x8
> +#define SBI_EXT_0_1_SET_TIMER 0x0
> +#define SBI_EXT_0_1_CONSOLE_PUTCHAR 0x1
Why the padding adjustment when ...
> +#define SBI_EXT_0_1_CONSOLE_GETCHAR 0x2
> +#define SBI_EXT_0_1_CLEAR_IPI 0x3
> +#define SBI_EXT_0_1_SEND_IPI 0x4
> +#define SBI_EXT_0_1_REMOTE_FENCE_I 0x5
> +#define SBI_EXT_0_1_REMOTE_SFENCE_VMA 0x6
> +#define SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID 0x7
... you immediately have one that doesn't fit?
> --- a/xen/arch/riscv/vsbi/Makefile
> +++ b/xen/arch/riscv/vsbi/Makefile
> @@ -1 +1,2 @@
> obj-y += vsbi.o
> +obj-y += vsbi-legacy-extension.o
No vsbi- prefixes please underneath vsbi/.
> --- /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().
> + break;
> +
> + case SBI_EXT_0_1_CONSOLE_GETCHAR:
> + regs->a0 = SBI_ERR_NOT_SUPPORTED;
This will be overwritten with the return value you pass to the caller (i.e. 0),
by that caller (i.e. vsbi_handle_ecall()).
> + break;
> +
> + 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.
> + break;
Bad indentation.
Jan
On 12/8/25 4:05 PM, Jan Beulich wrote:
> On 01.12.2025 11:24, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/include/asm/sbi.h
>> +++ b/xen/arch/riscv/include/asm/sbi.h
>> @@ -14,8 +14,15 @@
>>
>> #include <xen/cpumask.h>
>>
>> -#define SBI_EXT_0_1_CONSOLE_PUTCHAR 0x1
>> -#define SBI_EXT_0_1_SHUTDOWN 0x8
>> +#define SBI_EXT_0_1_SET_TIMER 0x0
>> +#define SBI_EXT_0_1_CONSOLE_PUTCHAR 0x1
> Why the padding adjustment when ...
>
>> +#define SBI_EXT_0_1_CONSOLE_GETCHAR 0x2
>> +#define SBI_EXT_0_1_CLEAR_IPI 0x3
>> +#define SBI_EXT_0_1_SEND_IPI 0x4
>> +#define SBI_EXT_0_1_REMOTE_FENCE_I 0x5
>> +#define SBI_EXT_0_1_REMOTE_SFENCE_VMA 0x6
>> +#define SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID 0x7
> ... you immediately have one that doesn't fit?
IDK, the padding adjustment shouldn't be done in this way.
I will correct it.
>> --- /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?
>> + break;
>> +
>> + case SBI_EXT_0_1_CONSOLE_GETCHAR:
>> + regs->a0 = SBI_ERR_NOT_SUPPORTED;
> This will be overwritten with the return value you pass to the caller (i.e. 0),
> by that caller (i.e. vsbi_handle_ecall()).
Oh, thanks. It should be "ret = SBI_ERR_NOT_SUPPORTED;" here.
>
>> + break;
>> +
>> + 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.
Thanks.
~ Oleksii
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
On 12/11/25 12:02 PM, Jan Beulich wrote:
> 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.
I don’t recall anything like that for ARM. The only thing related to character
buffering that I remember is in vpl011_write_data_xen()
(https://elixir.bootlin.com/xen/v4.21.0/source/xen/arch/arm/vpl011.c#L76), but it
does not use the buf declared in struct domain_console. Instead, it provides a
separate structure for vpl011:
struct vpl011_xen_backend {
char in[SBSA_UART_FIFO_SIZE];
char out[SBSA_UART_OUT_BUF_SIZE];
XENCONS_RING_IDX in_cons, in_prod;
XENCONS_RING_IDX out_prod;
};
I don’t see that ARM uses struct domain_console.
By the way, I can’t find a counterpart of hvm_print_line() for reading a character(s).
Is domain_console->buf intended to be used only for writing characters?
>
>>>> + 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.
Agree, I will create a task in my Xen's repo to not forget to drop panic()/domain_crash().
~ Oleksii
On 11.12.2025 13:52, Oleksii Kurochko wrote:
> On 12/11/25 12:02 PM, Jan Beulich wrote:
>> 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.
>
> I don’t recall anything like that for ARM. The only thing related to character
> buffering that I remember is in vpl011_write_data_xen()
> (https://elixir.bootlin.com/xen/v4.21.0/source/xen/arch/arm/vpl011.c#L76), but it
> does not use the buf declared in struct domain_console. Instead, it provides a
> separate structure for vpl011:
> struct vpl011_xen_backend {
> char in[SBSA_UART_FIFO_SIZE];
> char out[SBSA_UART_OUT_BUF_SIZE];
> XENCONS_RING_IDX in_cons, in_prod;
> XENCONS_RING_IDX out_prod;
> };
>
> I don’t see that ARM uses struct domain_console.
>
> By the way, I can’t find a counterpart of hvm_print_line() for reading a character(s).
> Is domain_console->buf intended to be used only for writing characters?
I don't think there's any particular intention, but of course you can use it
only for one of the two.
Jan
© 2016 - 2025 Red Hat, Inc.