[PATCH v1 0/4] RISCV: Intrdouce SSTC support in Xen

[PATCH v1 0/4] RISCV: Intrdouce SSTC support in Xen

[Oleksii Kurochko]

This patch series is created based on [1] and the aim is to make Xen properly
works when SSTC extension is available.

It is needed to do in this way as OpenSBI doesn't pass that it supports SSTC
by DTS to Xen and there is no easy way to turn off support of SSTC support.

Also, as a part of this patch series intrdouce suggested [2] by Jan B.
improvements of init_csr_masks().

[1] https://lore.kernel.org/xen-devel/cover.1772814110.git.oleksii.kurochko@gmail.com/T/#mc14576ef43a83b344c5f31626005b995e2ccbeaa
[2] https://lore.kernel.org/xen-devel/cover.1772814110.git.oleksii.kurochko@gmail.com/T/#m9c18d2d7a98958befec16419f5deccc40f6c8c3e

Oleksii Kurochko (4):
  xen/riscv: add exception table support
  xen/riscv: add csr_allowed_read() helper
  xen/riscv: allow Xen to use SSTC while hiding it from guests
  xen/riscv: init_csr_masks()-related improvements

 xen/arch/riscv/Kconfig                      |  1 +
 xen/arch/riscv/Makefile                     |  1 +
 xen/arch/riscv/cpufeature.c                 | 33 ++++++++
 xen/arch/riscv/domain.c                     | 32 ++++----
 xen/arch/riscv/extables.c                   | 85 +++++++++++++++++++++
 xen/arch/riscv/include/asm/cpufeature.h     |  1 +
 xen/arch/riscv/include/asm/csr.h            | 34 ++++++++-
 xen/arch/riscv/include/asm/extables.h       | 72 +++++++++++++++++
 xen/arch/riscv/include/asm/riscv_encoding.h |  2 +
 xen/arch/riscv/setup.c                      |  3 +
 xen/arch/riscv/time.c                       | 36 +++++----
 xen/arch/riscv/traps.c                      |  3 +
 xen/arch/riscv/vtimer.c                     |  7 +-
 xen/arch/riscv/xen.lds.S                    |  3 +
 xen/arch/x86/xen.lds.S                      |  6 +-
 xen/include/xen/xen.lds.h                   | 10 +++
 16 files changed, 294 insertions(+), 35 deletions(-)
 create mode 100644 xen/arch/riscv/extables.c
 create mode 100644 xen/arch/riscv/include/asm/extables.h

-- 
2.53.0

[PATCH v1 1/4] xen/riscv: add exception table support

[Oleksii Kurochko]

Introduce exception table handling for RISC-V so faults from selected
instructions can be recovered via fixup handlers instead of being
treated as fatal.

Add the RISC-V exception table format, sorting at boot to allow binary
search used furthuer, and lookup from the trap handler. Update the
linker script to emit the .ex_table section using introduced common
EX_TABLE macro shared with other architectures.

Also, the __start___ext_table is aligned now by POINTER_ALIGN instead
of just using hard-coded 8 as there is no too much sense to align
__start___ext_table by 8 for 32-bit systems.

This implementation is based on Linux 6.16.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Open question:

With some renaming the following could be generic, at least, between
x86 and RISC-V:
 - ASM_EXTABLE() definition
 - All what is conencted with sort_extable().
 - With some change of how x86 searchs an extension this cmp_ex_search()
   could also go to common file.

Does it make sense to introduce xen/extable.h and common/extable.c?
---
 xen/arch/riscv/Kconfig                |  1 +
 xen/arch/riscv/Makefile               |  1 +
 xen/arch/riscv/extables.c             | 85 +++++++++++++++++++++++++++
 xen/arch/riscv/include/asm/extables.h | 72 +++++++++++++++++++++++
 xen/arch/riscv/setup.c                |  3 +
 xen/arch/riscv/traps.c                |  3 +
 xen/arch/riscv/xen.lds.S              |  3 +
 xen/arch/x86/xen.lds.S                |  6 +-
 xen/include/xen/xen.lds.h             | 10 ++++
 9 files changed, 179 insertions(+), 5 deletions(-)
 create mode 100644 xen/arch/riscv/extables.c
 create mode 100644 xen/arch/riscv/include/asm/extables.h

diff --git a/xen/arch/riscv/Kconfig b/xen/arch/riscv/Kconfig
index 89876b32175d..a5e87c1757f7 100644
--- a/xen/arch/riscv/Kconfig
+++ b/xen/arch/riscv/Kconfig
@@ -4,6 +4,7 @@ config RISCV
 	select GENERIC_BUG_FRAME
 	select GENERIC_UART_INIT
 	select HAS_DEVICE_TREE_DISCOVERY
+	select HAS_EX_TABLE
 	select HAS_PMAP
 	select HAS_UBSAN
 	select HAS_VMAP
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index ffbd7062e214..6b3f3ed90bdb 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -3,6 +3,7 @@ obj-y += cpufeature.o
 obj-y += domain.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-y += entry.o
+obj-$(CONFIG_HAS_EX_TABLE) += extables.o
 obj-y += imsic.o
 obj-y += intc.o
 obj-y += irq.o
diff --git a/xen/arch/riscv/extables.c b/xen/arch/riscv/extables.c
new file mode 100644
index 000000000000..5e6e453ead29
--- /dev/null
+++ b/xen/arch/riscv/extables.c
@@ -0,0 +1,85 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/init.h>
+#include <xen/bsearch.h>
+#include <xen/lib.h>
+#include <xen/sort.h>
+#include <xen/virtual_region.h>
+
+#include <asm/extables.h>
+#include <asm/processor.h>
+
+#define EX_FIELD(ptr, field) ((unsigned long)&(ptr)->field + (ptr)->field)
+
+static inline unsigned long ex_insn(const struct exception_table_entry *ex)
+{
+    return EX_FIELD(ex, insn);
+}
+
+static inline unsigned long ex_fixup(const struct exception_table_entry *ex)
+{
+    return EX_FIELD(ex, fixup);
+}
+
+static void __init cf_check swap_ex(void *a, void *b)
+{
+    struct exception_table_entry *x = a, *y = b, tmp;
+    int delta = b - a;
+
+    tmp = *x;
+    x->insn = y->insn + delta;
+    y->insn = tmp.insn - delta;
+
+    x->fixup = y->fixup + delta;
+    y->fixup = tmp.fixup - delta;
+}
+
+static int __init cf_check cmp_ex_sort(const void *a, const void *b)
+{
+    const unsigned long l = ex_insn(a);
+    const unsigned long r = ex_insn(b);
+
+    /* avoid overflow */
+    return (l > r) - (l < r);
+}
+
+void __init sort_extable(void)
+{
+    sort(__start___ex_table,  __stop___ex_table - __start___ex_table,
+         sizeof(struct exception_table_entry), cmp_ex_sort, swap_ex);
+}
+
+static int cf_check cmp_ex_search(const void *key, const void *elt)
+{
+    const unsigned long k = *(const unsigned long *)key;
+    const unsigned long insn = ex_insn(elt);
+
+    /* avoid overflow */
+    return (k > insn) - (k < insn);
+}
+
+static bool ex_handler_fixup(const struct exception_table_entry *ex,
+			                 struct cpu_user_regs *regs)
+{
+	regs->sepc = ex_fixup(ex);
+
+	return true;
+}
+
+bool fixup_exception(struct cpu_user_regs *regs)
+{
+    unsigned long pc = regs->sepc;
+    const struct virtual_region *region = find_text_region(pc);
+    const struct exception_table_entry *ex;
+
+    if ( !region || !region->ex )
+        return false;
+
+    ex = bsearch(&pc, region->ex, region->ex_end - region->ex,
+                 sizeof(struct exception_table_entry), cmp_ex_search);
+
+    if ( !ex )
+        return false;
+
+    return ex_handler_fixup(ex, regs);
+}
diff --git a/xen/arch/riscv/include/asm/extables.h b/xen/arch/riscv/include/asm/extables.h
new file mode 100644
index 000000000000..139764f3808d
--- /dev/null
+++ b/xen/arch/riscv/include/asm/extables.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef ASM__RISCV__ASM_EXTABLES_H
+#define ASM__RISCV__ASM_EXTABLES_H
+
+#ifdef __ASSEMBLER__
+
+#define ASM_EXTABLE(insn, fixup)    \
+    .pushsection .ex_table, "a";    \
+    .balign     4;                  \
+    .long		((insn) - .);       \
+    .long		((fixup) - .);      \
+    .popsection;
+.endm
+
+#else /* __ASSEMBLER__ */
+
+#include <xen/bug.h>
+#include <xen/stringify.h>
+
+struct cpu_user_regs;
+
+#define ASM_EXTABLE(insn, fixup)        \
+    ".pushsection .ex_table, \"a\"\n"   \
+    ".balign    4\n"                    \
+    ".long      ((" #insn ") - .)\n"     \
+    ".long      ((" #fixup ") - .)\n"    \
+    ".popsection\n"
+
+/*
+ * The exception table consists of pairs of relative offsets: the first
+ * is the relative offset to an instruction that is allowed to fault,
+ * and the second is the relative offset at which the program should
+ * continue. No registers are modified, so it is entirely up to the
+ * continuation code to figure out what to do.
+ *
+ * All the routines below use bits of fixup code that are out of line
+ * with the main instruction path.  This means when everything is well,
+ * we don't even have to jump over them.  Further, they do not intrude
+ * on our cache or tlb entries.
+ */
+struct exception_table_entry {
+	int32_t insn, fixup;
+};
+
+extern struct exception_table_entry __start___ex_table[];
+extern struct exception_table_entry __stop___ex_table[];
+
+#ifdef CONFIG_HAS_EX_TABLE
+
+void sort_extable(void);
+bool fixup_exception(struct cpu_user_regs *regs);
+
+#else /* CONFIG_HAS_EX_TABLE */
+
+static inline void sort_extable(void)
+{
+    printk("%s: We don't support .ex_table\n", __func__);
+}
+
+static inline bool fixup_exception(struct cpu_user_regs *regs)
+{
+    dprintk("%s: We don't support .ex_table\n", __func__);
+
+    return false;
+}
+
+#endif /* CONFIG_HAS_EX_TABLE */
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* ASM__RISCV__ASM_EXTABLES_H */
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index cae49bb29626..4be6aa5a434e 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -19,6 +19,7 @@
 
 #include <public/version.h>
 
+#include <asm/extables.h>
 #include <asm/cpufeature.h>
 #include <asm/early_printk.h>
 #include <asm/fixmap.h>
@@ -81,6 +82,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
 
     smp_prepare_boot_cpu();
 
+    sort_extable();
+
     set_cpuid_to_hartid(0, bootcpu_id);
 
     trap_init();
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index 326f2be62823..242af0a7a5f3 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -12,6 +12,7 @@
 #include <xen/sched.h>
 #include <xen/softirq.h>
 
+#include <asm/extables.h>
 #include <asm/cpufeature.h>
 #include <asm/intc.h>
 #include <asm/processor.h>
@@ -217,6 +218,8 @@ void do_trap(struct cpu_user_regs *cpu_regs)
 
             break;
         }
+        else if ( fixup_exception(cpu_regs) )
+            break;
         fallthrough;
     default:
         if ( cause & CAUSE_IRQ_FLAG )
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index 331a7d63d3c9..65f136dce9f7 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -74,6 +74,9 @@ SECTIONS
     .data.ro_after_init : {
         __ro_after_init_start = .;
         *(.data.ro_after_init)
+
+        EX_TABLE
+
         . = ALIGN(PAGE_SIZE);
         __ro_after_init_end = .;
     } : text
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index c326538ebbb2..b9e888e5962f 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -113,11 +113,7 @@ SECTIONS
        __ro_after_init_start = .;
        *(.data.ro_after_init)
 
-       . = ALIGN(8);
-       /* Exception table */
-       __start___ex_table = .;
-       *(.ex_table)
-       __stop___ex_table = .;
+       EX_TABLE
 
        . = ALIGN(PAGE_SIZE);
        __ro_after_init_end = .;
diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
index 136849ecd515..85800f942aae 100644
--- a/xen/include/xen/xen.lds.h
+++ b/xen/include/xen/xen.lds.h
@@ -219,4 +219,14 @@
 #define VPCI_ARRAY
 #endif
 
+#ifdef CONFIG_HAS_EX_TABLE
+#define EX_TABLE                  \
+        . = ALIGN(POINTER_ALIGN); \
+        __start___ex_table = .;   \
+        *(.ex_table)              \
+        __stop___ex_table = .;
+#else
+#define EX_TABLE
+#endif
+
 #endif /* __XEN_LDS_H__ */
-- 
2.53.0

[PATCH v1 2/4] xen/riscv: add csr_allowed_read() helper

[Oleksii Kurochko]

Accessing some CSRs may trap when the corresponding extension is not
implemented or enabled. Introduce csr_allowed_read() which attempts to
read a CSR and relies on the exception table mechanism to safely recover
if the access faults.

This helper allows Xen to probe CSR availability without taking a fatal
trap and will be used for feature detection during early boot as we
can't always rely on what is in riscv,isa string in DTS.

While touching the header, reorder the include directives to follow the
usual Xen style.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/include/asm/csr.h | 34 +++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h
index 01876f828981..b9bee3d25d21 100644
--- a/xen/arch/riscv/include/asm/csr.h
+++ b/xen/arch/riscv/include/asm/csr.h
@@ -6,8 +6,10 @@
 #ifndef ASM__RISCV__CSR_H
 #define ASM__RISCV__CSR_H
 
-#include <asm/asm.h>
 #include <xen/const.h>
+
+#include <asm/asm.h>
+#include <asm/extables.h>
 #include <asm/riscv_encoding.h>
 
 #ifndef __ASSEMBLER__
@@ -78,6 +80,36 @@
                            : "memory" );                        \
 })
 
+static always_inline bool csr_allowed_read(unsigned long csr,
+                                           unsigned long *val)
+{
+    bool error = false;
+
+    /*
+     * Use "+" as a constraint instead of "=" to ensure the compiler passes the
+     * initial value into the asm volatile block. Otherwise, if the instruction
+     * (at label 1) faults, the variable 'error' may contain an undefined value
+     * instead of 0.
+     * If reading of CSR register was failed, we don't care about val, so "="
+     * constraint could be used in asm volatile block to not force always init.
+     * val argument before being passed to csr_allowed_read() functions.
+     *
+     * This avoids the need for an additional instruction inside the asm block
+     * to explicitly initialize 'error' to 0 before executing the potentially
+     * faulting instruction.
+     */
+    asm volatile (
+        "1: csrr %[val], %[csr]\n"
+        "   li %[err], 1\n"
+        "2:\n"
+        ASM_EXTABLE(1b, 2b)
+        : [val] "=&r" (*val), [err] "+&r" (error)
+        : [csr] "i" (csr)
+        : "memory" );
+
+    return error;
+}
+
 #endif /* __ASSEMBLER__ */
 
 #endif /* ASM__RISCV__CSR_H */
-- 
2.53.0

[PATCH v1 3/4] xen/riscv: allow Xen to use SSTC while hiding it from guests

[Oleksii Kurochko]

OpenSBI currently does not advertise the SSTC extension via the device
tree. Additionally, SSTC can no longer be reliably disabled by removing
the "sstc" string from riscv,isa, as OpenSBI probes support by attempting
to access CSR_STIMECMP.

Introduce a runtime probe in Xen to determine whether SSTC is available.
The probe attempts to read CSR_STIMECMP using csr_allowed_read(). If the
access succeeds, SSTC is considered available; if a trap occurs, it is
treated as unsupported.

When SSTC is detected, Xen may use it internally to program timers.
However, the extension is not exposed to guests because the required
context switch handling for the SSTC CSRs is not yet implemented.

To prevent guests from using SSTC, RISCV_ISA_EXT_sstc is cleared from the
riscv_isa bitmap. As a result, the corresponding HENVCFG bit is not set
and guests fall back to the SBI timer interface. Timer requests are then
handled by Xen via the usual SBI interception path.

Introduce set_xen_timer() to abstract how the timer is programmed,
either via the SSTC extension or an SBI call. This also reduces the
number of if statements in reprogram_timer().

The set_xen_timer function pointer is selected based on
csr_allowed_read() rather than riscv_isa_extension(). The latter
reflects features supported by both Xen and the guest, while SSTC is
currently only supported for Xen. Therefore, relying solely on
riscv_isa_extension() would not reliably determine whether SSTC can be
used.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/cpufeature.c                 | 33 +++++++++++++++++++
 xen/arch/riscv/domain.c                     |  3 ++
 xen/arch/riscv/include/asm/cpufeature.h     |  1 +
 xen/arch/riscv/include/asm/riscv_encoding.h |  2 ++
 xen/arch/riscv/time.c                       | 36 +++++++++++++--------
 xen/arch/riscv/vtimer.c                     |  7 +++-
 6 files changed, 67 insertions(+), 15 deletions(-)

diff --git a/xen/arch/riscv/cpufeature.c b/xen/arch/riscv/cpufeature.c
index 03e27b037be0..a7aa2358b73b 100644
--- a/xen/arch/riscv/cpufeature.c
+++ b/xen/arch/riscv/cpufeature.c
@@ -17,6 +17,7 @@
 #include <xen/sections.h>
 
 #include <asm/cpufeature.h>
+#include <asm/csr.h>
 
 #ifdef CONFIG_ACPI
 # error "cpufeature.c functions should be updated to support ACPI"
@@ -139,6 +140,7 @@ const struct riscv_isa_ext_data __initconst riscv_isa_ext[] = {
     RISCV_ISA_EXT_DATA(smaia),
     RISCV_ISA_EXT_DATA(smstateen),
     RISCV_ISA_EXT_DATA(ssaia),
+    RISCV_ISA_EXT_DATA(sstc),
     RISCV_ISA_EXT_DATA(svade),
     RISCV_ISA_EXT_DATA(svpbmt),
 };
@@ -483,6 +485,7 @@ void __init riscv_fill_hwcap(void)
     unsigned int i;
     const size_t req_extns_amount = ARRAY_SIZE(required_extensions);
     bool all_extns_available = true;
+    unsigned long tmp;
 
     riscv_fill_hwcap_from_isa_string();
 
@@ -495,6 +498,36 @@ void __init riscv_fill_hwcap(void)
         panic("HW capabilities parsing failed: %s\n", failure_msg);
     }
 
+    if ( csr_allowed_read(CSR_STIMECMP, &tmp) )
+    {
+        printk("SSTC is detected but is supported only for Xen usage not for "
+               "a guest.\n");
+
+        /*
+         * As SSTC for guest isn't supported it is needed temprorary to:
+         *
+         * 1. Clear bit RISCV_ISA_EXT_sstc in riscv_isa as theoretuically it
+         *    could be that OpenSBI (it doesn't pass it now) or whatever ran
+         *    before Xen will add SSTC to riscv,isa string. This bit  clear
+         *    willn't allow guest to use SSTC extension as vtimer context
+         *    switch and restore isn't ready for that.
+         */
+        __clear_bit(RISCV_ISA_EXT_sstc, riscv_isa);
+
+        /*
+         * 2. A VS-timer interrupt becomes pending whenever the value of
+         *    (time + htimedelta) is greater than or equal to vstimecmp CSR.
+         *    Thereby to avoid spurious VS-timer irqs set vstimecmp CSR to
+         *    -1.
+         *
+         * It should be dropped when SSTC for guests will be supported.
+         */
+        csr_write(CSR_VSTIMECMP, ULONG_MAX);
+#ifdef CONFIG_RISCV_32
+        csr_write(CSR_VSTIMECMPH, ULONG_MAX);
+#endif
+    }
+
     for ( i = 0; i < req_extns_amount; i++ )
     {
         const struct riscv_isa_ext_data ext = required_extensions[i];
diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
index c327f44d07ca..5f15dda88c8e 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -99,6 +99,9 @@ static void vcpu_csr_init(struct vcpu *v)
     if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svpbmt) )
         v->arch.henvcfg = ENVCFG_PBMTE & csr_masks.henvcfg;
 
+    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_sstc) )
+        v->arch.henvcfg |= ENVCFG_STCE & csr_masks.henvcfg;
+
     if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_smstateen) )
     {
         /* Allow guest to access CSR_SENVCFG */
diff --git a/xen/arch/riscv/include/asm/cpufeature.h b/xen/arch/riscv/include/asm/cpufeature.h
index ef02a3e26d2c..0c48d57a03bb 100644
--- a/xen/arch/riscv/include/asm/cpufeature.h
+++ b/xen/arch/riscv/include/asm/cpufeature.h
@@ -38,6 +38,7 @@ enum riscv_isa_ext_id {
     RISCV_ISA_EXT_smaia,
     RISCV_ISA_EXT_smstateen,
     RISCV_ISA_EXT_ssaia,
+    RISCV_ISA_EXT_sstc,
     RISCV_ISA_EXT_svade,
     RISCV_ISA_EXT_svpbmt,
     RISCV_ISA_EXT_MAX
diff --git a/xen/arch/riscv/include/asm/riscv_encoding.h b/xen/arch/riscv/include/asm/riscv_encoding.h
index dd15731a86fa..7fc379cab588 100644
--- a/xen/arch/riscv/include/asm/riscv_encoding.h
+++ b/xen/arch/riscv/include/asm/riscv_encoding.h
@@ -396,6 +396,8 @@
 #define CSR_VSTVAL			0x243
 #define CSR_VSIP			0x244
 #define CSR_VSATP			0x280
+#define CSR_VSTIMECMP		0x24D
+#define CSR_VSTIMECMPH		0x25D
 
 /* Virtual Interrupts and Interrupt Priorities (H-extension with AIA) */
 #define CSR_HVIEN			0x608
diff --git a/xen/arch/riscv/time.c b/xen/arch/riscv/time.c
index 7efa76fdbcb1..98eca2887d5c 100644
--- a/xen/arch/riscv/time.c
+++ b/xen/arch/riscv/time.c
@@ -13,6 +13,20 @@
 unsigned long __ro_after_init cpu_khz; /* CPU clock frequency in kHz. */
 uint64_t __ro_after_init boot_clock_cycles;
 
+static int cf_check sstc_set_xen_timer(uint64_t deadline)
+{
+#ifdef CONFIG_RISCV_32
+    csr_write(CSR_STIMECMP, deadline & 0xFFFFFFFF);
+    csr_write(CSR_STIMECMPH, deadline >> 32);
+#else
+    csr_write(CSR_STIMECMP, deadline);
+#endif
+
+    return 0;
+}
+
+int (* __ro_after_init set_xen_timer)(uint64_t deadline);
+
 s_time_t get_s_time(void)
 {
     uint64_t ticks = get_cycles() - boot_clock_cycles;
@@ -61,20 +75,7 @@ int reprogram_timer(s_time_t timeout)
     if ( deadline <= now )
         return 0;
 
-    /*
-     * TODO: When the SSTC extension is supported, it would be preferable to
-     *       use the supervisor timer registers directly here for better
-     *       performance, since an SBI call and mode switch would no longer
-     *       be required.
-     *
-     *       This would also reduce reliance on a specific SBI implementation.
-     *       For example, it is not ideal to panic() if sbi_set_timer() returns
-     *       a non-zero value. Currently it can return 0 or -ENOSUPP, and
-     *       without SSTC we still need an implementation because only the
-     *       M-mode timer is available, and it can only be programmed in
-     *       M-mode.
-     */
-    if ( (rc = sbi_set_timer(deadline)) )
+    if ( (rc = set_xen_timer(deadline)) )
         panic("%s: timer wasn't set because: %d\n", __func__, rc);
 
     /* Enable timer interrupt */
@@ -85,10 +86,17 @@ int reprogram_timer(s_time_t timeout)
 
 void __init preinit_xen_time(void)
 {
+    unsigned long tmp;
+
     if ( acpi_disabled )
         preinit_dt_xen_time();
     else
         panic("%s: ACPI isn't supported\n", __func__);
 
     boot_clock_cycles = get_cycles();
+
+    if ( csr_allowed_read(CSR_STIMECMP, &tmp) )
+        set_xen_timer = sstc_set_xen_timer;
+    else
+        set_xen_timer = sbi_set_timer;
 }
diff --git a/xen/arch/riscv/vtimer.c b/xen/arch/riscv/vtimer.c
index afd8a53a7387..c065052afeb7 100644
--- a/xen/arch/riscv/vtimer.c
+++ b/xen/arch/riscv/vtimer.c
@@ -4,6 +4,7 @@
 #include <xen/sched.h>
 #include <xen/timer.h>
 
+#include <asm/cpufeature.h>
 #include <asm/vtimer.h>
 
 static void vtimer_expired(void *data)
@@ -75,12 +76,16 @@ void vtimer_ctxt_switch_from(struct vcpu *p)
 {
     ASSERT(!is_idle_vcpu(p));
 
-    /* Nothing to do at the moment as SSTC isn't supported now. */
+    BUG_ON(riscv_isa_extension_available(NULL, RISCV_ISA_EXT_sstc));
+
+    /* Nothing to do at the moment as SSTC for guests isn't supported now */
 }
 
 void vtimer_ctxt_switch_to(struct vcpu *n)
 {
     ASSERT(!is_idle_vcpu(n));
 
+    BUG_ON(riscv_isa_extension_available(NULL, RISCV_ISA_EXT_sstc));
+
     migrate_timer(&n->arch.vtimer.timer, n->processor);
 }
-- 
2.53.0

[PATCH v1 4/4] xen/riscv: init_csr_masks()-related improvements

[Oleksii Kurochko]

There is no reason to use _UL() in define-s sitting in C file hence use UL
prefix instead.

Drop 3d argument of INIT_CSR_MASK() and INIT_RO_ONE_MASK() to reduce risk
of incomplete editing after copy-and-paste, or other typo-ing.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/domain.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
index 5f15dda88c8e..70d0e55ed1bc 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -42,10 +42,10 @@ struct csr_masks {
 
 static struct csr_masks __ro_after_init csr_masks;
 
-#define HEDELEG_AVAIL_MASK ULONG_MAX
-#define HIDELEG_AVAIL_MASK ULONG_MAX
-#define HENVCFG_AVAIL_MASK _UL(0xE0000003000000FF)
-#define HSTATEEN0_AVAIL_MASK _UL(0xDE00000000000007)
+#define HEDELEG_VALID_MASK ULONG_MAX
+#define HIDELEG_VALID_MASK ULONG_MAX
+#define HENVCFG_VALID_MASK (0xE0000003000000FFUL)
+#define HSTATEEN0_VALID_MASK (0xDE00000000000007UL)
 
 void __init init_csr_masks(void)
 {
@@ -57,25 +57,26 @@ void __init init_csr_masks(void)
      * fields that must be preserved. Any write to the full register must
      * therefore retain the original values of those fields.
      */
-#define INIT_CSR_MASK(csr, field, mask) do { \
-        register_t old = csr_read_set(CSR_ ## csr, mask); \
+#define INIT_CSR_MASK(csr, field) do { \
+        register_t old = csr_read_set(CSR_ ## csr, csr ## _VALID_MASK); \
         csr_masks.field = csr_swap(CSR_ ## csr, old); \
     } while (0)
 
-#define INIT_RO_ONE_MASK(csr, field, mask) do { \
-        register_t old = csr_read_clear(CSR_ ## csr, mask); \
-        csr_masks.ro_one.field = csr_swap(CSR_ ## csr, old) & mask; \
+#define INIT_RO_ONE_MASK(csr, field) do { \
+        register_t old = csr_read_clear(CSR_ ## csr, csr ## _VALID_MASK); \
+        csr_masks.ro_one.field = csr_swap(CSR_ ## csr, old) & \
+                                 csr ## _VALID_MASK; \
     } while (0)
 
-    INIT_CSR_MASK(HEDELEG, hedeleg, HEDELEG_AVAIL_MASK);
-    INIT_CSR_MASK(HIDELEG, hideleg, HIDELEG_AVAIL_MASK);
+    INIT_CSR_MASK(HEDELEG, hedeleg);
+    INIT_CSR_MASK(HIDELEG, hideleg);
 
-    INIT_CSR_MASK(HENVCFG, henvcfg, HENVCFG_AVAIL_MASK);
+    INIT_CSR_MASK(HENVCFG, henvcfg);
 
     if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_smstateen) )
     {
-        INIT_CSR_MASK(HSTATEEN0, hstateen0, HSTATEEN0_AVAIL_MASK);
-        INIT_RO_ONE_MASK(HSTATEEN0, hstateen0, HSTATEEN0_AVAIL_MASK);
+        INIT_CSR_MASK(HSTATEEN0, hstateen0);
+        INIT_RO_ONE_MASK(HSTATEEN0, hstateen0);
     }
 
 #undef INIT_CSR_MASK
-- 
2.53.0

Re: [PATCH v1 1/4] xen/riscv: add exception table support

[Jan Beulich]

On 13.03.2026 17:44, Oleksii Kurochko wrote:
> Introduce exception table handling for RISC-V so faults from selected
> instructions can be recovered via fixup handlers instead of being
> treated as fatal.
> 
> Add the RISC-V exception table format, sorting at boot to allow binary
> search used furthuer, and lookup from the trap handler. Update the
> linker script to emit the .ex_table section using introduced common
> EX_TABLE macro shared with other architectures.
> 
> Also, the __start___ext_table is aligned now by POINTER_ALIGN instead
> of just using hard-coded 8 as there is no too much sense to align
> __start___ext_table by 8 for 32-bit systems.

Nit: The identifier named here twice isn't correct (extra 't').

> This implementation is based on Linux 6.16.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Open question:
> 
> With some renaming the following could be generic, at least, between
> x86 and RISC-V:
>  - ASM_EXTABLE() definition
>  - All what is conencted with sort_extable().
>  - With some change of how x86 searchs an extension this cmp_ex_search()
>    could also go to common file.
> 
> Does it make sense to introduce xen/extable.h and common/extable.c?

Maybe, but not right here. Already the introduction of EX_TABLE for
linker script use might better have been broken out.

Seeing the names you suggest here, ...

> ---
>  xen/arch/riscv/Kconfig                |  1 +
>  xen/arch/riscv/Makefile               |  1 +
>  xen/arch/riscv/extables.c             | 85 +++++++++++++++++++++++++++
>  xen/arch/riscv/include/asm/extables.h | 72 +++++++++++++++++++++++
>  xen/arch/riscv/setup.c                |  3 +
>  xen/arch/riscv/traps.c                |  3 +
>  xen/arch/riscv/xen.lds.S              |  3 +
>  xen/arch/x86/xen.lds.S                |  6 +-
>  xen/include/xen/xen.lds.h             | 10 ++++
>  9 files changed, 179 insertions(+), 5 deletions(-)
>  create mode 100644 xen/arch/riscv/extables.c
>  create mode 100644 xen/arch/riscv/include/asm/extables.h

... is there a reason you use plural in the name here?

> --- a/xen/arch/riscv/Makefile
> +++ b/xen/arch/riscv/Makefile
> @@ -3,6 +3,7 @@ obj-y += cpufeature.o
>  obj-y += domain.o
>  obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>  obj-y += entry.o
> +obj-$(CONFIG_HAS_EX_TABLE) += extables.o

Simply obj-y please as long as the select is unconditional.

> --- /dev/null
> +++ b/xen/arch/riscv/extables.c
> @@ -0,0 +1,85 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/init.h>
> +#include <xen/bsearch.h>
> +#include <xen/lib.h>
> +#include <xen/sort.h>
> +#include <xen/virtual_region.h>
> +
> +#include <asm/extables.h>
> +#include <asm/processor.h>
> +
> +#define EX_FIELD(ptr, field) ((unsigned long)&(ptr)->field + (ptr)->field)
> +
> +static inline unsigned long ex_insn(const struct exception_table_entry *ex)
> +{
> +    return EX_FIELD(ex, insn);
> +}
> +
> +static inline unsigned long ex_fixup(const struct exception_table_entry *ex)
> +{
> +    return EX_FIELD(ex, fixup);
> +}
> +
> +static void __init cf_check swap_ex(void *a, void *b)
> +{
> +    struct exception_table_entry *x = a, *y = b, tmp;
> +    int delta = b - a;

Better play safe and use "long" (as we have it for x86)?

> +    tmp = *x;
> +    x->insn = y->insn + delta;
> +    y->insn = tmp.insn - delta;
> +
> +    x->fixup = y->fixup + delta;
> +    y->fixup = tmp.fixup - delta;
> +}
> +
> +static int __init cf_check cmp_ex_sort(const void *a, const void *b)
> +{
> +    const unsigned long l = ex_insn(a);
> +    const unsigned long r = ex_insn(b);
> +
> +    /* avoid overflow */
> +    return (l > r) - (l < r);
> +}
> +
> +void __init sort_extable(void)

Better account for live-patching right away (see corresponding x86 code)?

> +{
> +    sort(__start___ex_table,  __stop___ex_table - __start___ex_table,
> +         sizeof(struct exception_table_entry), cmp_ex_sort, swap_ex);
> +}
> +
> +static int cf_check cmp_ex_search(const void *key, const void *elt)
> +{
> +    const unsigned long k = *(const unsigned long *)key;

The deref here looks to be needed solely because you pass &pc into bsearch().
Generally I'd expect both search functions to be pretty similar (if already
distinct ones are needed, which indeed looks to make things easier here).

> +    const unsigned long insn = ex_insn(elt);
> +
> +    /* avoid overflow */
> +    return (k > insn) - (k < insn);
> +}
> +
> +static bool ex_handler_fixup(const struct exception_table_entry *ex,
> +			                 struct cpu_user_regs *regs)

Nit: Bad indentation.

> +{
> +	regs->sepc = ex_fixup(ex);
> +
> +	return true;

Nit: Bad use of hard tabs.

And then - why the boolean return type, when this can't fail anyway?

> +}
> +
> +bool fixup_exception(struct cpu_user_regs *regs)
> +{
> +    unsigned long pc = regs->sepc;
> +    const struct virtual_region *region = find_text_region(pc);
> +    const struct exception_table_entry *ex;
> +
> +    if ( !region || !region->ex )
> +        return false;
> +
> +    ex = bsearch(&pc, region->ex, region->ex_end - region->ex,
> +                 sizeof(struct exception_table_entry), cmp_ex_search);

Please prefer sizeof(<expression>) over sizeof(<type>) (also in the sort()
invocation further up, as I notice only now).

> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/extables.h
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef ASM__RISCV__ASM_EXTABLES_H
> +#define ASM__RISCV__ASM_EXTABLES_H
> +
> +#ifdef __ASSEMBLER__
> +
> +#define ASM_EXTABLE(insn, fixup)    \
> +    .pushsection .ex_table, "a";    \
> +    .balign     4;                  \
> +    .long		((insn) - .);       \
> +    .long		((fixup) - .);      \

Nit: More uses of hard tabs. Maybe that alone is the reason for the mis-aligned
trailing backslashes.

> +    .popsection;
> +.endm

I can't spot the corresponding .macro. What's going on here?

> +#else /* __ASSEMBLER__ */
> +
> +#include <xen/bug.h>
> +#include <xen/stringify.h>
> +
> +struct cpu_user_regs;
> +
> +#define ASM_EXTABLE(insn, fixup)        \
> +    ".pushsection .ex_table, \"a\"\n"   \
> +    ".balign    4\n"                    \
> +    ".long      ((" #insn ") - .)\n"     \
> +    ".long      ((" #fixup ") - .)\n"    \

More misaligned backslashes.

> +    ".popsection\n"
> +
> +/*
> + * The exception table consists of pairs of relative offsets: the first
> + * is the relative offset to an instruction that is allowed to fault,
> + * and the second is the relative offset at which the program should
> + * continue. No registers are modified, so it is entirely up to the
> + * continuation code to figure out what to do.

And the program counter is not a register?

> + * All the routines below use bits of fixup code that are out of line
> + * with the main instruction path.  This means when everything is well,
> + * we don't even have to jump over them.  Further, they do not intrude
> + * on our cache or tlb entries.

What is this paragraph about? There's nothing "below" which I can
associate this with.

> + */
> +struct exception_table_entry {
> +	int32_t insn, fixup;
> +};
> +
> +extern struct exception_table_entry __start___ex_table[];
> +extern struct exception_table_entry __stop___ex_table[];
> +
> +#ifdef CONFIG_HAS_EX_TABLE

Why, when this is a RISC-V specific header and HAS_EX_TABLE is selected
unconditionally?

> --- a/xen/arch/riscv/traps.c
> +++ b/xen/arch/riscv/traps.c
> @@ -12,6 +12,7 @@
>  #include <xen/sched.h>
>  #include <xen/softirq.h>
>  
> +#include <asm/extables.h>
>  #include <asm/cpufeature.h>
>  #include <asm/intc.h>
>  #include <asm/processor.h>
> @@ -217,6 +218,8 @@ void do_trap(struct cpu_user_regs *cpu_regs)
>  
>              break;
>          }
> +        else if ( fixup_exception(cpu_regs) )
> +            break;

Instead od the "else" better put a blank line ahead of the if(), to
visually separate the set of checks.

> --- a/xen/include/xen/xen.lds.h
> +++ b/xen/include/xen/xen.lds.h
> @@ -219,4 +219,14 @@
>  #define VPCI_ARRAY
>  #endif
>  
> +#ifdef CONFIG_HAS_EX_TABLE

No real need for this?

> +#define EX_TABLE                  \
> +        . = ALIGN(POINTER_ALIGN); \

Strictly speaking the original 8 (in x86 code) as much as this is more
than we need - each element is a struct of 2 4-byte entities, after all.

Jan

Re: [PATCH v1 2/4] xen/riscv: add csr_allowed_read() helper

[Jan Beulich]

On 13.03.2026 17:44, Oleksii Kurochko wrote:
> Accessing some CSRs may trap when the corresponding extension is not
> implemented or enabled. Introduce csr_allowed_read() which attempts to
> read a CSR and relies on the exception table mechanism to safely recover
> if the access faults.
> 
> This helper allows Xen to probe CSR availability without taking a fatal
> trap and will be used for feature detection during early boot as we
> can't always rely on what is in riscv,isa string in DTS.
> 
> While touching the header, reorder the include directives to follow the
> usual Xen style.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
>  xen/arch/riscv/include/asm/csr.h | 34 +++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h
> index 01876f828981..b9bee3d25d21 100644
> --- a/xen/arch/riscv/include/asm/csr.h
> +++ b/xen/arch/riscv/include/asm/csr.h
> @@ -6,8 +6,10 @@
>  #ifndef ASM__RISCV__CSR_H
>  #define ASM__RISCV__CSR_H
>  
> -#include <asm/asm.h>
>  #include <xen/const.h>
> +
> +#include <asm/asm.h>
> +#include <asm/extables.h>
>  #include <asm/riscv_encoding.h>
>  
>  #ifndef __ASSEMBLER__
> @@ -78,6 +80,36 @@
>                             : "memory" );                        \
>  })
>  
> +static always_inline bool csr_allowed_read(unsigned long csr,
> +                                           unsigned long *val)
> +{
> +    bool error = false;

Wrong polarity or wrong name? You set this ...

> +    /*
> +     * Use "+" as a constraint instead of "=" to ensure the compiler passes the
> +     * initial value into the asm volatile block. Otherwise, if the instruction
> +     * (at label 1) faults, the variable 'error' may contain an undefined value
> +     * instead of 0.
> +     * If reading of CSR register was failed, we don't care about val, so "="
> +     * constraint could be used in asm volatile block to not force always init.
> +     * val argument before being passed to csr_allowed_read() functions.
> +     *
> +     * This avoids the need for an additional instruction inside the asm block
> +     * to explicitly initialize 'error' to 0 before executing the potentially
> +     * faulting instruction.
> +     */
> +    asm volatile (
> +        "1: csrr %[val], %[csr]\n"
> +        "   li %[err], 1\n"

... to true in the success case.

Please can all of the commentary be dropped? You're re-stating what the compiler
doc has, and (imo) such doesn't belong here. If you want to comment on anything,
then the (again imo) less obvious need to use always_inline.

> +        "2:\n"
> +        ASM_EXTABLE(1b, 2b)
> +        : [val] "=&r" (*val), [err] "+&r" (error)

You don't clobber [err] early, so & isn't needed here.

> +        : [csr] "i" (csr)
> +        : "memory" );

The memory clobber, if you think you really need it, wants to come with
a comment.

Jan

Re: [PATCH v1 2/4] xen/riscv: add csr_allowed_read() helper

[Andrew Cooper]

On 13/03/2026 4:44 pm, Oleksii Kurochko wrote:
> diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h
> index 01876f828981..b9bee3d25d21 100644
> --- a/xen/arch/riscv/include/asm/csr.h
> +++ b/xen/arch/riscv/include/asm/csr.h
> @@ -78,6 +80,36 @@
>                             : "memory" );                        \
>  })
>  
> +static always_inline bool csr_allowed_read(unsigned long csr,
> +                                           unsigned long *val)

$FOO_safe() is the common nomenclature.  So csr_read_safe() in this case.

csr_allowed_read() reads as if it's a predicate, which this is not.

> +{
> +    bool error = false;
> +
> +    /*
> +     * Use "+" as a constraint instead of "=" to ensure the compiler passes the
> +     * initial value into the asm volatile block. Otherwise, if the instruction
> +     * (at label 1) faults, the variable 'error' may contain an undefined value
> +     * instead of 0.
> +     * If reading of CSR register was failed, we don't care about val, so "="
> +     * constraint could be used in asm volatile block to not force always init.
> +     * val argument before being passed to csr_allowed_read() functions.
> +     *
> +     * This avoids the need for an additional instruction inside the asm block
> +     * to explicitly initialize 'error' to 0 before executing the potentially
> +     * faulting instruction.

Honestly, this is mostly tutorial level "how to asm", and isn't really
appropriate.

> +     */
> +    asm volatile (

asm_inline.  Especially important for inlining decisions when using
ASM_EXTABLE().

> +        "1: csrr %[val], %[csr]\n"
> +        "   li %[err], 1\n"
> +        "2:\n"
> +        ASM_EXTABLE(1b, 2b)
> +        : [val] "=&r" (*val), [err] "+&r" (error)

err needs & dropping.

> +        : [csr] "i" (csr)
> +        : "memory" );
> +
> +    return error;

You should write a second form with CONFIG_CC_HAS_ASM_GOTO_OUTPUT right
away.  See x86's rdmsr_safe() for the equivalent of this function.

Code generation with ASM_EXTABLE() is far better when the fixup label
can be used.

~Andrew

Re: [PATCH v1 3/4] xen/riscv: allow Xen to use SSTC while hiding it from guests

[Jan Beulich]

On 13.03.2026 17:44, Oleksii Kurochko wrote:
> OpenSBI currently does not advertise the SSTC extension via the device
> tree. Additionally, SSTC can no longer be reliably disabled by removing
> the "sstc" string from riscv,isa, as OpenSBI probes support by attempting
> to access CSR_STIMECMP.

Still don't yopu need to remove that string from what guests get to see, ...

> Introduce a runtime probe in Xen to determine whether SSTC is available.
> The probe attempts to read CSR_STIMECMP using csr_allowed_read(). If the
> access succeeds, SSTC is considered available; if a trap occurs, it is
> treated as unsupported.
> 
> When SSTC is detected, Xen may use it internally to program timers.
> However, the extension is not exposed to guests because the required
> context switch handling for the SSTC CSRs is not yet implemented.
> 
> To prevent guests from using SSTC, RISCV_ISA_EXT_sstc is cleared from the
> riscv_isa bitmap. As a result, the corresponding HENVCFG bit is not set
> and guests fall back to the SBI timer interface. Timer requests are then
> handled by Xen via the usual SBI interception path.

... alongside the riscv_isa adjustment?

> --- a/xen/arch/riscv/cpufeature.c
> +++ b/xen/arch/riscv/cpufeature.c
> @@ -17,6 +17,7 @@
>  #include <xen/sections.h>
>  
>  #include <asm/cpufeature.h>
> +#include <asm/csr.h>
>  
>  #ifdef CONFIG_ACPI
>  # error "cpufeature.c functions should be updated to support ACPI"
> @@ -139,6 +140,7 @@ const struct riscv_isa_ext_data __initconst riscv_isa_ext[] = {
>      RISCV_ISA_EXT_DATA(smaia),
>      RISCV_ISA_EXT_DATA(smstateen),
>      RISCV_ISA_EXT_DATA(ssaia),
> +    RISCV_ISA_EXT_DATA(sstc),
>      RISCV_ISA_EXT_DATA(svade),
>      RISCV_ISA_EXT_DATA(svpbmt),
>  };
> @@ -483,6 +485,7 @@ void __init riscv_fill_hwcap(void)
>      unsigned int i;
>      const size_t req_extns_amount = ARRAY_SIZE(required_extensions);
>      bool all_extns_available = true;
> +    unsigned long tmp;
>  
>      riscv_fill_hwcap_from_isa_string();
>  
> @@ -495,6 +498,36 @@ void __init riscv_fill_hwcap(void)
>          panic("HW capabilities parsing failed: %s\n", failure_msg);
>      }
>  
> +    if ( csr_allowed_read(CSR_STIMECMP, &tmp) )
> +    {
> +        printk("SSTC is detected but is supported only for Xen usage not for "
> +               "a guest.\n");

No full stops please in log messages.

> +        /*
> +         * As SSTC for guest isn't supported it is needed temprorary to:
> +         *
> +         * 1. Clear bit RISCV_ISA_EXT_sstc in riscv_isa as theoretuically it
> +         *    could be that OpenSBI (it doesn't pass it now) or whatever ran
> +         *    before Xen will add SSTC to riscv,isa string. This bit  clear

Nit: Stray double blanks.

> +         *    willn't allow guest to use SSTC extension as vtimer context

Nit: won't

> +         *    switch and restore isn't ready for that.
> +         */
> +        __clear_bit(RISCV_ISA_EXT_sstc, riscv_isa);
> +
> +        /*
> +         * 2. A VS-timer interrupt becomes pending whenever the value of
> +         *    (time + htimedelta) is greater than or equal to vstimecmp CSR.
> +         *    Thereby to avoid spurious VS-timer irqs set vstimecmp CSR to
> +         *    -1.

-1 is misleading here, as any unsigned value is greater than -1. You mean
UINT64_MAX or e.g. ~0U here.

> +         * It should be dropped when SSTC for guests will be supported.
> +         */
> +        csr_write(CSR_VSTIMECMP, ULONG_MAX);
> +#ifdef CONFIG_RISCV_32
> +        csr_write(CSR_VSTIMECMPH, ULONG_MAX);
> +#endif
> +    }
> +
>      for ( i = 0; i < req_extns_amount; i++ )
>      {
>          const struct riscv_isa_ext_data ext = required_extensions[i];
> --- a/xen/arch/riscv/domain.c
> +++ b/xen/arch/riscv/domain.c
> @@ -99,6 +99,9 @@ static void vcpu_csr_init(struct vcpu *v)
>      if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svpbmt) )
>          v->arch.henvcfg = ENVCFG_PBMTE & csr_masks.henvcfg;
>  
> +    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_sstc) )
> +        v->arch.henvcfg |= ENVCFG_STCE & csr_masks.henvcfg;

Wouldn't this better be part of the (future) patch enabling SSTC for guests?

> --- a/xen/arch/riscv/include/asm/riscv_encoding.h
> +++ b/xen/arch/riscv/include/asm/riscv_encoding.h
> @@ -396,6 +396,8 @@
>  #define CSR_VSTVAL			0x243
>  #define CSR_VSIP			0x244
>  #define CSR_VSATP			0x280
> +#define CSR_VSTIMECMP		0x24D
> +#define CSR_VSTIMECMPH		0x25D

I think it would be nice if throughout the CSR definitions you settled on
using upper case hex digits uniformly, or all lower case ones (personally
I'd prefer the latter).

> --- a/xen/arch/riscv/time.c
> +++ b/xen/arch/riscv/time.c
> @@ -13,6 +13,20 @@
>  unsigned long __ro_after_init cpu_khz; /* CPU clock frequency in kHz. */
>  uint64_t __ro_after_init boot_clock_cycles;
>  
> +static int cf_check sstc_set_xen_timer(uint64_t deadline)
> +{
> +#ifdef CONFIG_RISCV_32
> +    csr_write(CSR_STIMECMP, deadline & 0xFFFFFFFF);

The "& 0x..." isn't needed here, is it? I.e. the whole function could be ...

> +    csr_write(CSR_STIMECMPH, deadline >> 32);
> +#else
> +    csr_write(CSR_STIMECMP, deadline);
> +#endif
> +
> +    return 0;
> +}

static int cf_check sstc_set_xen_timer(uint64_t deadline)
{
    csr_write(CSR_STIMECMP, deadline);
#ifdef CONFIG_RISCV_32
    csr_write(CSR_STIMECMPH, deadline >> 32);
#endif

    return 0;
}

> +int (* __ro_after_init set_xen_timer)(uint64_t deadline);

static?

Jan

Re: [PATCH v1 4/4] xen/riscv: init_csr_masks()-related improvements

[Jan Beulich]

On 13.03.2026 17:44, Oleksii Kurochko wrote:
> There is no reason to use _UL() in define-s sitting in C file hence use UL
> prefix instead.
> 
> Drop 3d argument of INIT_CSR_MASK() and INIT_RO_ONE_MASK() to reduce risk
> of incomplete editing after copy-and-paste, or other typo-ing.

Another brief sentence about the AVAIL -> VALID transformation? Then ...

> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH v1 4/4] xen/riscv: init_csr_masks()-related improvements

[Jan Beulich]

On 13.03.2026 17:44, Oleksii Kurochko wrote:
> There is no reason to use _UL() in define-s sitting in C file hence use UL
> prefix instead.

Actually, only after sending the earlier reply, I noticed: s/prefix/suffix/.

> --- a/xen/arch/riscv/domain.c
> +++ b/xen/arch/riscv/domain.c
> @@ -42,10 +42,10 @@ struct csr_masks {
>  
>  static struct csr_masks __ro_after_init csr_masks;
>  
> -#define HEDELEG_AVAIL_MASK ULONG_MAX
> -#define HIDELEG_AVAIL_MASK ULONG_MAX
> -#define HENVCFG_AVAIL_MASK _UL(0xE0000003000000FF)
> -#define HSTATEEN0_AVAIL_MASK _UL(0xDE00000000000007)
> +#define HEDELEG_VALID_MASK ULONG_MAX
> +#define HIDELEG_VALID_MASK ULONG_MAX
> +#define HENVCFG_VALID_MASK (0xE0000003000000FFUL)
> +#define HSTATEEN0_VALID_MASK (0xDE00000000000007UL)

With UL in upper case (for Misra), imo hex digits would better be lower case,
to visually better separate number from suffix. Also, not need for parentheses
around literal numbers.

Jan

Re: [PATCH v1 1/4] xen/riscv: add exception table support

[Oleksii Kurochko]

On 3/24/26 3:04 PM, Jan Beulich wrote:
> On 13.03.2026 17:44, Oleksii Kurochko wrote:
>> Introduce exception table handling for RISC-V so faults from selected
>> instructions can be recovered via fixup handlers instead of being
>> treated as fatal.
>>
>> Add the RISC-V exception table format, sorting at boot to allow binary
>> search used furthuer, and lookup from the trap handler. Update the
>> linker script to emit the .ex_table section using introduced common
>> EX_TABLE macro shared with other architectures.
>>
>> Also, the __start___ext_table is aligned now by POINTER_ALIGN instead
>> of just using hard-coded 8 as there is no too much sense to align
>> __start___ext_table by 8 for 32-bit systems.
> 
> Nit: The identifier named here twice isn't correct (extra 't').
> 
>> This implementation is based on Linux 6.16.
>>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> ---
>> Open question:
>>
>> With some renaming the following could be generic, at least, between
>> x86 and RISC-V:
>>   - ASM_EXTABLE() definition
>>   - All what is conencted with sort_extable().
>>   - With some change of how x86 searchs an extension this cmp_ex_search()
>>     could also go to common file.
>>
>> Does it make sense to introduce xen/extable.h and common/extable.c?
> 
> Maybe, but not right here. Already the introduction of EX_TABLE for
> linker script use might better have been broken out.
> 
> Seeing the names you suggest here, ...
> 
>> ---
>>   xen/arch/riscv/Kconfig                |  1 +
>>   xen/arch/riscv/Makefile               |  1 +
>>   xen/arch/riscv/extables.c             | 85 +++++++++++++++++++++++++++
>>   xen/arch/riscv/include/asm/extables.h | 72 +++++++++++++++++++++++
>>   xen/arch/riscv/setup.c                |  3 +
>>   xen/arch/riscv/traps.c                |  3 +
>>   xen/arch/riscv/xen.lds.S              |  3 +
>>   xen/arch/x86/xen.lds.S                |  6 +-
>>   xen/include/xen/xen.lds.h             | 10 ++++
>>   9 files changed, 179 insertions(+), 5 deletions(-)
>>   create mode 100644 xen/arch/riscv/extables.c
>>   create mode 100644 xen/arch/riscv/include/asm/extables.h
> 
> ... is there a reason you use plural in the name here?

No, I'll use singular form.

I called it tables as potentially I will need a different types of 
exception table, so I counted different types as different exception tables.

> 
>> --- a/xen/arch/riscv/Makefile
>> +++ b/xen/arch/riscv/Makefile
>> @@ -3,6 +3,7 @@ obj-y += cpufeature.o
>>   obj-y += domain.o
>>   obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>>   obj-y += entry.o
>> +obj-$(CONFIG_HAS_EX_TABLE) += extables.o
> 
> Simply obj-y please as long as the select is unconditional.

I see your point and at the moment there is no also other options how
to handle case(s) for which exception table is introduced now. But if 
potentially another mechanism will be introduced what will be the point 
to have extable.o code in the final binary?

> 
>> --- /dev/null
>> +++ b/xen/arch/riscv/extables.c
>> @@ -0,0 +1,85 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#include <xen/init.h>
>> +#include <xen/bsearch.h>
>> +#include <xen/lib.h>
>> +#include <xen/sort.h>
>> +#include <xen/virtual_region.h>
>> +
>> +#include <asm/extables.h>
>> +#include <asm/processor.h>
>> +
>> +#define EX_FIELD(ptr, field) ((unsigned long)&(ptr)->field + (ptr)->field)
>> +
>> +static inline unsigned long ex_insn(const struct exception_table_entry *ex)
>> +{
>> +    return EX_FIELD(ex, insn);
>> +}
>> +
>> +static inline unsigned long ex_fixup(const struct exception_table_entry *ex)
>> +{
>> +    return EX_FIELD(ex, fixup);
>> +}
>> +
>> +static void __init cf_check swap_ex(void *a, void *b)
>> +{
>> +    struct exception_table_entry *x = a, *y = b, tmp;
>> +    int delta = b - a;
> 
> Better play safe and use "long" (as we have it for x86)?

It makes sense. Lets switch to "long".

> 
>> +    tmp = *x;
>> +    x->insn = y->insn + delta;
>> +    y->insn = tmp.insn - delta;
>> +
>> +    x->fixup = y->fixup + delta;
>> +    y->fixup = tmp.fixup - delta;
>> +}
>> +
>> +static int __init cf_check cmp_ex_sort(const void *a, const void *b)
>> +{
>> +    const unsigned long l = ex_insn(a);
>> +    const unsigned long r = ex_insn(b);
>> +
>> +    /* avoid overflow */
>> +    return (l > r) - (l < r);
>> +}
>> +
>> +void __init sort_extable(void)
> 
> Better account for live-patching right away (see corresponding x86 code)?

I will introduce then void init_or_livepatch sort_exception_table(...) 
and re-use it inside sort_extable() with renaming it to 
sort_exception_tables() to take into account live-patching which 
requires sort_exception_table().

> 
>> +{
>> +    sort(__start___ex_table,  __stop___ex_table - __start___ex_table,
>> +         sizeof(struct exception_table_entry), cmp_ex_sort, swap_ex);
>> +}
>> +
>> +static int cf_check cmp_ex_search(const void *key, const void *elt)
>> +{
>> +    const unsigned long k = *(const unsigned long *)key;
> 
> The deref here looks to be needed solely because you pass &pc into bsearch().
> Generally I'd expect both search functions to be pretty similar (if already
> distinct ones are needed, which indeed looks to make things easier here).

The stuff is easier with such implementation.

We could really drop cmp_ex_search() if to pass struct 
exception_table_entry instead of (unsigned long *) so the following will 
allow to drop cmp_ex_search():

@@ -78,12 +78,15 @@ bool fixup_exception(struct cpu_user_regs *regs)
      unsigned long pc = regs->sepc;
      const struct virtual_region *region = find_text_region(pc);
      const struct exception_table_entry *ex;
+    struct exception_table_entry key;

      if ( !region || !region->ex )
          return false;

-    ex = bsearch(&pc, region->ex, region->ex_end - region->ex,
-                 sizeof(struct exception_table_entry), cmp_ex_search);
+    key.insn = pc - (unsigned long)&key.insn;
+
+    ex = bsearch(&key, region->ex, region->ex_end - region->ex,
+                 sizeof(struct exception_table_entry), cmp_ex_sort 
/*cmp_ex_search*/);

Also, then I will rename l and r variable inside cmp_ex_sort() to
insn_a and insn_b.

> 
>> +    const unsigned long insn = ex_insn(elt);
>> +
>> +    /* avoid overflow */
>> +    return (k > insn) - (k < insn);
>> +}
>> +
>> +static bool ex_handler_fixup(const struct exception_table_entry *ex,
>> +			                 struct cpu_user_regs *regs)
> 
> Nit: Bad indentation.
> 
>> +{
>> +	regs->sepc = ex_fixup(ex);
>> +
>> +	return true;
> 
> Nit: Bad use of hard tabs.
> 
> And then - why the boolean return type, when this can't fail anyway?

As potentially we could have other handlers which might return not only 
true, so it will be easier to handle return type inside fixup_exception().

But if you think there is no any sense to have for handlers the same 
signature then I am also return void instead of bool for 
ex_handler_fixup().

> 
>> --- /dev/null
>> +++ b/xen/arch/riscv/include/asm/extables.h
>> @@ -0,0 +1,72 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#ifndef ASM__RISCV__ASM_EXTABLES_H
>> +#define ASM__RISCV__ASM_EXTABLES_H
>> +
>> +#ifdef __ASSEMBLER__
>> +
>> +#define ASM_EXTABLE(insn, fixup)    \
>> +    .pushsection .ex_table, "a";    \
>> +    .balign     4;                  \
>> +    .long		((insn) - .);       \
>> +    .long		((fixup) - .);      \
> 
> Nit: More uses of hard tabs. Maybe that alone is the reason for the mis-aligned
> trailing backslashes.
> 
>> +    .popsection;
>> +.endm
> 
> I can't spot the corresponding .macro. What's going on here?

Good question... It was:

.macro asm_extable, insn, fixup
     ASM_EXTABLE(\insn, \fixup)
.endm

>> +
>> +/*
>> + * The exception table consists of pairs of relative offsets: the first
>> + * is the relative offset to an instruction that is allowed to fault,
>> + * and the second is the relative offset at which the program should
>> + * continue. No registers are modified, so it is entirely up to the
>> + * continuation code to figure out what to do.
> 
> And the program counter is not a register?
It is. "No register" meant no general purpose register. I will reprase 
this part to:

No general-purpose registers are modified by the exception handling 
mechanism itself, so it is up to the fixup code to handle any necessary 
state cleanup.

Or it could be just dropped.

> 
>> + * All the routines below use bits of fixup code that are out of line
>> + * with the main instruction path.  This means when everything is well,
>> + * we don't even have to jump over them.  Further, they do not intrude
>> + * on our cache or tlb entries.
> 
> What is this paragraph about? There's nothing "below" which I can
> associate this with.

It is orphaned from Linux (generally it is about that some functions 
from uaccess.h are using ASM_EXTABLE, the similar for Xen has in 
x86/uaccess.h). I'll rephrase it to:
  * The exception table and fixup code live out of line with the main
  * instruction path. This means when everything is well, we don't even
  * have to jump over them. Further, they do not intrude on our cache or
  * tlb entries.

> 
>> + */
>> +struct exception_table_entry {
>> +	int32_t insn, fixup;
>> +};
>> +
>> +extern struct exception_table_entry __start___ex_table[];
>> +extern struct exception_table_entry __stop___ex_table[];
>> +
>> +#ifdef CONFIG_HAS_EX_TABLE
> 
> Why, when this is a RISC-V specific header and HAS_EX_TABLE is selected
> unconditionally?

To handle the potential in future case that CONFIG_HAS_EX_TABLE will 
become conditional.
I thought that it makes sense to be in sync with common/virtual_region.c 
also uses ifdef around exception table related information.

> 
>> --- a/xen/include/xen/xen.lds.h
>> +++ b/xen/include/xen/xen.lds.h
>> @@ -219,4 +219,14 @@
>>   #define VPCI_ARRAY
>>   #endif
>>   
>> +#ifdef CONFIG_HAS_EX_TABLE
> 
> No real need for this?

Here I can agree that there is not reason as if CONFIG_HAS_EX_TABLE is n
then no one is expected to use exception table so the section is empty 
and don't occupy any extra space in binary (except potentially some 
space because of alignment).


> 
>> +#define EX_TABLE                  \
>> +        . = ALIGN(POINTER_ALIGN); \
> 
> Strictly speaking the original 8 (in x86 code) as much as this is more
> than we need - each element is a struct of 2 4-byte entities, after all.

For the  current struct - yes, we can do . = ALIGN(4) but if the 
architecture will add uint64_t inside (or unsigned long) shouldn't we 
then have ALIGN(POINTER_ALIGN)?

Thanks.

~ Oleksii

Re: [PATCH v1 1/4] xen/riscv: add exception table support

[Jan Beulich]

On 27.03.2026 13:47, Oleksii Kurochko wrote:
> On 3/24/26 3:04 PM, Jan Beulich wrote:
>> On 13.03.2026 17:44, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/Makefile
>>> +++ b/xen/arch/riscv/Makefile
>>> @@ -3,6 +3,7 @@ obj-y += cpufeature.o
>>>   obj-y += domain.o
>>>   obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>>>   obj-y += entry.o
>>> +obj-$(CONFIG_HAS_EX_TABLE) += extables.o
>>
>> Simply obj-y please as long as the select is unconditional.
> 
> I see your point and at the moment there is no also other options how
> to handle case(s) for which exception table is introduced now. But if 
> potentially another mechanism will be introduced what will be the point 
> to have extable.o code in the final binary?

I'd like to suggest to keep things simple as long as they are simple. It
might be different if you firmly knew the other variant is going to be
needed pretty soon.

>>> +static bool ex_handler_fixup(const struct exception_table_entry *ex,
>>> +			                 struct cpu_user_regs *regs)
>>
>> Nit: Bad indentation.
>>
>>> +{
>>> +	regs->sepc = ex_fixup(ex);
>>> +
>>> +	return true;
>>
>> Nit: Bad use of hard tabs.
>>
>> And then - why the boolean return type, when this can't fail anyway?
> 
> As potentially we could have other handlers which might return not only 
> true, so it will be easier to handle return type inside fixup_exception().
> 
> But if you think there is no any sense to have for handlers the same 
> signature then I am also return void instead of bool for 
> ex_handler_fixup().

It's not like there's no sense in that at all, but again - let's keep things
simple as long as they are simple.

>>> +struct exception_table_entry {
>>> +	int32_t insn, fixup;
>>> +};
>>> +
>>> +extern struct exception_table_entry __start___ex_table[];
>>> +extern struct exception_table_entry __stop___ex_table[];
>>> +
>>> +#ifdef CONFIG_HAS_EX_TABLE
>>
>> Why, when this is a RISC-V specific header and HAS_EX_TABLE is selected
>> unconditionally?
> 
> To handle the potential in future case that CONFIG_HAS_EX_TABLE will 
> become conditional.
> I thought that it makes sense to be in sync with common/virtual_region.c 
> also uses ifdef around exception table related information.

But that's common code, which has to deal with mixed needs of the various
architectures.

>>> --- a/xen/include/xen/xen.lds.h
>>> +++ b/xen/include/xen/xen.lds.h
>>> @@ -219,4 +219,14 @@
>>>   #define VPCI_ARRAY
>>>   #endif
>>>   
>>> +#ifdef CONFIG_HAS_EX_TABLE
>>
>> No real need for this?
> 
> Here I can agree that there is not reason as if CONFIG_HAS_EX_TABLE is n
> then no one is expected to use exception table so the section is empty 
> and don't occupy any extra space in binary (except potentially some 
> space because of alignment).
> 
> 
>>
>>> +#define EX_TABLE                  \
>>> +        . = ALIGN(POINTER_ALIGN); \
>>
>> Strictly speaking the original 8 (in x86 code) as much as this is more
>> than we need - each element is a struct of 2 4-byte entities, after all.
> 
> For the  current struct - yes, we can do . = ALIGN(4) but if the 
> architecture will add uint64_t inside (or unsigned long) shouldn't we 
> then have ALIGN(POINTER_ALIGN)?

Along the lines of what I said above, here things want to be consistent.
The alignment effected should be possible to justify by just what is in
the tree, without resorting to any unknown future.

Jan

Re: [PATCH v1 3/4] xen/riscv: allow Xen to use SSTC while hiding it from guests

[Oleksii Kurochko]

On 3/24/26 3:32 PM, Jan Beulich wrote:
> On 13.03.2026 17:44, Oleksii Kurochko wrote:
>> OpenSBI currently does not advertise the SSTC extension via the device
>> tree. Additionally, SSTC can no longer be reliably disabled by removing
>> the "sstc" string from riscv,isa, as OpenSBI probes support by attempting
>> to access CSR_STIMECMP.
> 
> Still don't yopu need to remove that string from what guests get to see, ...
> 
>> Introduce a runtime probe in Xen to determine whether SSTC is available.
>> The probe attempts to read CSR_STIMECMP using csr_allowed_read(). If the
>> access succeeds, SSTC is considered available; if a trap occurs, it is
>> treated as unsupported.
>>
>> When SSTC is detected, Xen may use it internally to program timers.
>> However, the extension is not exposed to guests because the required
>> context switch handling for the SSTC CSRs is not yet implemented.
>>
>> To prevent guests from using SSTC, RISCV_ISA_EXT_sstc is cleared from the
>> riscv_isa bitmap. As a result, the corresponding HENVCFG bit is not set
>> and guests fall back to the SBI timer interface. Timer requests are then
>> handled by Xen via the usual SBI interception path.
> 
> ... alongside the riscv_isa adjustment?

Right, I have to mentioned that in the commit message. It will be 
skipped for riscv_isa property here:
  
https://lore.kernel.org/xen-devel/cover.1773157782.git.oleksii.kurochko@gmail.com/T/#m6e45279d24258fe78c6aebf29379fa9135ec9f1c

(what will require to add SSTC extension to guest_unsupp_exts.)

I will add to commit message that except HEVFCFG bit shouldn't be set to 
not let a guest try to access VSTIMECMP register, it should be droppped 
(or not added) from riscv,isa property.

>> --- a/xen/arch/riscv/domain.c
>> +++ b/xen/arch/riscv/domain.c
>> @@ -99,6 +99,9 @@ static void vcpu_csr_init(struct vcpu *v)
>>       if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svpbmt) )
>>           v->arch.henvcfg = ENVCFG_PBMTE & csr_masks.henvcfg;
>>   
>> +    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_sstc) )
>> +        v->arch.henvcfg |= ENVCFG_STCE & csr_masks.henvcfg;
> 
> Wouldn't this better be part of the (future) patch enabling SSTC for guests?

Probably, it will be better. Lets drop these changes and re-introduce 
later when guests will support SSTC.

> 
>> --- a/xen/arch/riscv/include/asm/riscv_encoding.h
>> +++ b/xen/arch/riscv/include/asm/riscv_encoding.h
>> @@ -396,6 +396,8 @@
>>   #define CSR_VSTVAL			0x243
>>   #define CSR_VSIP			0x244
>>   #define CSR_VSATP			0x280
>> +#define CSR_VSTIMECMP		0x24D
>> +#define CSR_VSTIMECMPH		0x25D
> 
> I think it would be nice if throughout the CSR definitions you settled on
> using upper case hex digits uniformly, or all lower case ones (personally
> I'd prefer the latter).
> 
>> --- a/xen/arch/riscv/time.c
>> +++ b/xen/arch/riscv/time.c
>> @@ -13,6 +13,20 @@
>>   unsigned long __ro_after_init cpu_khz; /* CPU clock frequency in kHz. */
>>   uint64_t __ro_after_init boot_clock_cycles;
>>   
>> +static int cf_check sstc_set_xen_timer(uint64_t deadline)
>> +{
>> +#ifdef CONFIG_RISCV_32
>> +    csr_write(CSR_STIMECMP, deadline & 0xFFFFFFFF);
> 
> The "& 0x..." isn't needed here, is it? I.e. the whole function could be ...

I think you are right, it "& 0x..." could be dropped as it anyway will 
be truncated by (unsigned long) cast inside csr_write().

Thanks.

~ Oleksii

Re: [PATCH v1 4/4] xen/riscv: init_csr_masks()-related improvements

[Oleksii Kurochko]

On 3/24/26 3:36 PM, Jan Beulich wrote:
> On 13.03.2026 17:44, Oleksii Kurochko wrote:
>> There is no reason to use _UL() in define-s sitting in C file hence use UL
>> prefix instead.
>>
>> Drop 3d argument of INIT_CSR_MASK() and INIT_RO_ONE_MASK() to reduce risk
>> of incomplete editing after copy-and-paste, or other typo-ing.
> 
> Another brief sentence about the AVAIL -> VALID transformation? Then ...

Sure, I will add then:
  Use _VALID_ infix instead of _AVAIL_ as the mask identifies 
architecturally defined bits, not bits available for software use.

> 
>> Suggested-by: Jan Beulich <jbeulich@suse.com>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

~ Oleksii