Add support of virtual SBI base extension calls for RISC-V guests, delegating
hardware-specific queries to the underlying SBI and handling version and
firmware ID queries directly.
The changes include:
1. Define new SBI base extension function IDs (SBI_EXT_BASE_GET_MVENDORID,
SBI_EXT_BASE_GET_MARCHID, SBI_EXT_BASE_GET_MIMPID).
2. Introduce XEN_SBI_VER_MAJOR, XEN_SBI_VER_MINOR for imeplenataion of
SBI_EXT_BASE_GET_SPEC_VERSION.
4. Introduce VCPU_SBI_IMPID to implement SBI_EXT_BASE_GET_IMP_ID.
5. Implement handling of SBI base extension functions, including version,
firmware ID, and machine-specific queries.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v2:
- s/vsbi-base-extension.*/base-extension.*
- Introduce VCPU_SBI_IMPID, XEN_SBI_VER_MINOR and XEN_SBI_VER_MAJOR.
- Return VCPU_SBI_IMPID in the case of SBI_EXT_BASE_GET_IMP_ID.
- Return Xen version in the case of SBI_EXT_BASE_GET_IMP_VERSION.
- Use domain_crash() instead of panic() for default case.
- For SBI_EXT_BASE_GET_{MVENDORID,MARCHID,MIMPID} abd SBI_EXT_BASE_PROBE_EXT
add handling of a domain is h/w or not.
---
xen/arch/riscv/include/asm/sbi.h | 7 +++
xen/arch/riscv/vsbi/Makefile | 1 +
xen/arch/riscv/vsbi/base-extension.c | 71 ++++++++++++++++++++++++++++
3 files changed, 79 insertions(+)
create mode 100644 xen/arch/riscv/vsbi/base-extension.c
diff --git a/xen/arch/riscv/include/asm/sbi.h b/xen/arch/riscv/include/asm/sbi.h
index 751bae6d66..eb710950cc 100644
--- a/xen/arch/riscv/include/asm/sbi.h
+++ b/xen/arch/riscv/include/asm/sbi.h
@@ -14,6 +14,10 @@
#include <xen/cpumask.h>
+#define XEN_SBI_VER_MAJOR 0
+#define XEN_SBI_VER_MINOR 2
+#define XEN_SBI_IMPID 7
+
#define SBI_EXT_0_1_SET_TIMER 0x0
#define SBI_EXT_0_1_CONSOLE_PUTCHAR 0x1
#define SBI_EXT_0_1_CONSOLE_GETCHAR 0x2
@@ -32,6 +36,9 @@
#define SBI_EXT_BASE_GET_IMP_ID 0x1
#define SBI_EXT_BASE_GET_IMP_VERSION 0x2
#define SBI_EXT_BASE_PROBE_EXT 0x3
+#define SBI_EXT_BASE_GET_MVENDORID 0x4
+#define SBI_EXT_BASE_GET_MARCHID 0x5
+#define SBI_EXT_BASE_GET_MIMPID 0x6
/* SBI function IDs for RFENCE extension */
#define SBI_EXT_RFENCE_REMOTE_FENCE_I 0x0
diff --git a/xen/arch/riscv/vsbi/Makefile b/xen/arch/riscv/vsbi/Makefile
index bc5755cb13..8ce470f787 100644
--- a/xen/arch/riscv/vsbi/Makefile
+++ b/xen/arch/riscv/vsbi/Makefile
@@ -1,2 +1,3 @@
+obj-y += base-extension.o
obj-y += core.o
obj-y += legacy-extension.o
diff --git a/xen/arch/riscv/vsbi/base-extension.c b/xen/arch/riscv/vsbi/base-extension.c
new file mode 100644
index 0000000000..567429c715
--- /dev/null
+++ b/xen/arch/riscv/vsbi/base-extension.c
@@ -0,0 +1,71 @@
+
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/lib.h>
+#include <xen/sched.h>
+#include <xen/version.h>
+
+#include <asm/processor.h>
+#include <asm/sbi.h>
+#include <asm/vsbi.h>
+
+static int vsbi_base_ecall_handler(struct vcpu *vcpu, unsigned long eid,
+ unsigned long fid,
+ struct cpu_user_regs *regs)
+{
+ int ret = 0;
+ struct sbiret sbi_ret;
+
+ switch ( fid ) {
+ case SBI_EXT_BASE_GET_SPEC_VERSION:
+ regs->a1 = MASK_INSR(XEN_SBI_VER_MAJOR, SBI_SPEC_VERSION_MAJOR_MASK) |
+ XEN_SBI_VER_MINOR;
+ break;
+ case SBI_EXT_BASE_GET_IMP_ID:
+ regs->a1 = XEN_SBI_IMPID;
+ break;
+ case SBI_EXT_BASE_GET_IMP_VERSION:
+ regs->a1 = (xen_major_version() << 16) | xen_minor_version();
+ break;
+ case SBI_EXT_BASE_GET_MVENDORID:
+ case SBI_EXT_BASE_GET_MARCHID:
+ case SBI_EXT_BASE_GET_MIMPID:
+ if ( is_hardware_domain(vcpu->domain) )
+ {
+ sbi_ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0);
+ ret = sbi_ret.error;
+ regs->a1 = sbi_ret.value;
+ }
+ else
+ /*
+ * vSBI should present a consistent, virtualized view to guests.
+ * In particular, DomU-visible data must remain stable across
+ * migration and must not expose hardware-specific details.
+ *
+ * These register(s) must be readable in any implementation,
+ * but a value of 0 can be returned to indicate the field
+ * is not implemented.
+ */
+ regs->a1 = 0;
+
+ break;
+ case SBI_EXT_BASE_PROBE_EXT:
+ regs->a1 = vsbi_find_extension(regs->a0) ? 1 : 0;
+ 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.
+ */
+ domain_crash(vcpu->domain,
+ "%s: Unsupported ecall: FID: #%lx, EID: #%lx\n",
+ __func__, fid, eid);
+ break;
+ }
+
+ return ret;
+}
+
+VSBI_EXT(base, SBI_EXT_BASE, SBI_EXT_BASE, vsbi_base_ecall_handler)
--
2.52.0On 17.12.2025 17:54, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/include/asm/sbi.h
> +++ b/xen/arch/riscv/include/asm/sbi.h
> @@ -14,6 +14,10 @@
>
> #include <xen/cpumask.h>
>
> +#define XEN_SBI_VER_MAJOR 0
> +#define XEN_SBI_VER_MINOR 2
> +#define XEN_SBI_IMPID 7
Are these numbers part of the spec (sorry, lack of a reference makes me wonder,
plus if that were the case, I'd kind of expect the names to be SBI_XEN_..., not
XEN_SBI_...)?
> --- /dev/null
> +++ b/xen/arch/riscv/vsbi/base-extension.c
> @@ -0,0 +1,71 @@
> +
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/lib.h>
> +#include <xen/sched.h>
> +#include <xen/version.h>
> +
> +#include <asm/processor.h>
> +#include <asm/sbi.h>
> +#include <asm/vsbi.h>
> +
> +static int vsbi_base_ecall_handler(struct vcpu *vcpu, unsigned long eid,
> + unsigned long fid,
> + struct cpu_user_regs *regs)
> +{
> + int ret = 0;
> + struct sbiret sbi_ret;
ASSERT(eid == SBI_EXT_BASE);
> + switch ( fid ) {
Nit: Brace placement.
> + case SBI_EXT_BASE_GET_SPEC_VERSION:
> + regs->a1 = MASK_INSR(XEN_SBI_VER_MAJOR, SBI_SPEC_VERSION_MAJOR_MASK) |
> + XEN_SBI_VER_MINOR;
> + break;
> + case SBI_EXT_BASE_GET_IMP_ID:
> + regs->a1 = XEN_SBI_IMPID;
> + break;
> + case SBI_EXT_BASE_GET_IMP_VERSION:
> + regs->a1 = (xen_major_version() << 16) | xen_minor_version();
> + break;
Along those lines here - are we free to use an arbitrary layout (shifting major by
16 bits), or is this mandated by the spec? At least in the latter case, the 16 will
want to gain a #define.
Also - blank lines please between non-fall-through case blocks.
Jan
On 12/18/25 3:32 PM, Jan Beulich wrote:
> On 17.12.2025 17:54, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/include/asm/sbi.h
>> +++ b/xen/arch/riscv/include/asm/sbi.h
>> @@ -14,6 +14,10 @@
>>
>> #include <xen/cpumask.h>
>>
>> +#define XEN_SBI_VER_MAJOR 0
>> +#define XEN_SBI_VER_MINOR 2
>> +#define XEN_SBI_IMPID 7
> Are these numbers part of the spec (sorry, lack of a reference makes me wonder,
> plus if that were the case, I'd kind of expect the names to be SBI_XEN_..., not
> XEN_SBI_...)?
XEN_SBI_IMPID is a number defined by the SBI specification:
https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-base.adoc#sbi-implementation-ids
XEN_SBI_VER_MAJOR and XEN_SBI_VER_MINOR somehow also is a part of the spec, there is
no such defines explicitly, but it is real numbers of the SBI version.
I will rename the defines accordingly:
- s/XEN_SBI_VER_MAJOR/SBI_XEN_VER_MAJOR
- s/XEN_SBI_VER_MINOR/SBI_XEN_VER_MINOR
- s/XEN_SBI_IMPID/SBI_XEN_IMPID
>
>> --- /dev/null
>> +++ b/xen/arch/riscv/vsbi/base-extension.c
>> @@ -0,0 +1,71 @@
>> +
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#include <xen/lib.h>
>> +#include <xen/sched.h>
>> +#include <xen/version.h>
>> +
>> +#include <asm/processor.h>
>> +#include <asm/sbi.h>
>> +#include <asm/vsbi.h>
>> +
>> +static int vsbi_base_ecall_handler(struct vcpu *vcpu, unsigned long eid,
>> + unsigned long fid,
>> + struct cpu_user_regs *regs)
>> +{
>> + int ret = 0;
>> + struct sbiret sbi_ret;
> ASSERT(eid == SBI_EXT_BASE);
>
>> + switch ( fid ) {
> Nit: Brace placement.
>
>> + case SBI_EXT_BASE_GET_SPEC_VERSION:
>> + regs->a1 = MASK_INSR(XEN_SBI_VER_MAJOR, SBI_SPEC_VERSION_MAJOR_MASK) |
>> + XEN_SBI_VER_MINOR;
>> + break;
>> + case SBI_EXT_BASE_GET_IMP_ID:
>> + regs->a1 = XEN_SBI_IMPID;
>> + break;
>> + case SBI_EXT_BASE_GET_IMP_VERSION:
>> + regs->a1 = (xen_major_version() << 16) | xen_minor_version();
>> + break;
> Along those lines here - are we free to use an arbitrary layout (shifting major by
> 16 bits), or is this mandated by the spec? At least in the latter case, the 16 will
> want to gain a #define.
I would say that we are free to use an arbitrary layout. The specification says:
The encoding of this version number is specific to the SBI implementation.
(https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-base.adoc#function-get-sbi-implementation-version-fid-2)
So this fully depends on how a specific SBI implementation decides to encode the
version. For Xen, I simply copied the approach used by OpenSBI:
/**
* OpenSBI 32-bit version with:
* 1. upper 16-bits as major number
* 2. lower 16-bits as minor number
*/
#define OPENSBI_VERSION ((OPENSBI_VERSION_MAJOR << 16) | \
(OPENSBI_VERSION_MINOR))
Therefore, I think it is fine to keep Xen’s implementation as it is now, without
introducing additional defines.
Thanks.
~ Oleksii
On 19.12.2025 21:34, Oleksii Kurochko wrote: > On 12/18/25 3:32 PM, Jan Beulich wrote: >> On 17.12.2025 17:54, Oleksii Kurochko wrote: >>> --- a/xen/arch/riscv/include/asm/sbi.h >>> +++ b/xen/arch/riscv/include/asm/sbi.h >>> @@ -14,6 +14,10 @@ >>> >>> #include <xen/cpumask.h> >>> >>> +#define XEN_SBI_VER_MAJOR 0 >>> +#define XEN_SBI_VER_MINOR 2 >>> +#define XEN_SBI_IMPID 7 >> Are these numbers part of the spec (sorry, lack of a reference makes me wonder, >> plus if that were the case, I'd kind of expect the names to be SBI_XEN_..., not >> XEN_SBI_...)? > > XEN_SBI_IMPID is a number defined by the SBI specification: > https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-base.adoc#sbi-implementation-ids I see. For this the name change below is then appropriate; for ... > XEN_SBI_VER_MAJOR and XEN_SBI_VER_MINOR somehow also is a part of the spec, there is > no such defines explicitly, but it is real numbers of the SBI version. ... these two I'm unconvinced, otoh: If it is us to control the versions used here, the present names should imo remain. Brief comments may then also want adding to clarify the different origin of the numbers. Jan
© 2016 - 2026 Red Hat, Inc.