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
