On 2/17/26 3:23 PM, Jan Beulich wrote:
On 13.02.2026 17:28, Oleksii Kurochko wrote:
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -6,10 +6,74 @@
#include <xen/sched.h>
#include <xen/vmap.h>
+#include <asm/cpufeature.h>
+#include <asm/csr.h>
+#include <asm/riscv_encoding.h>
#include <asm/setup.h>
struct csr_masks __ro_after_init csr_masks;
+#define HEDELEG_DEFAULT (BIT(CAUSE_MISALIGNED_FETCH, U) | \
+ BIT(CAUSE_FETCH_ACCESS, U) | \
+ BIT(CAUSE_ILLEGAL_INSTRUCTION, U) | \
+ BIT(CAUSE_BREAKPOINT, U) | \
+ BIT(CAUSE_MISALIGNED_LOAD, U) | \
+ BIT(CAUSE_LOAD_ACCESS, U) | \
+ BIT(CAUSE_MISALIGNED_STORE, U) | \
+ BIT(CAUSE_STORE_ACCESS, U) | \
+ BIT(CAUSE_USER_ECALL, U) | \
+ BIT(CAUSE_FETCH_PAGE_FAULT, U) | \
+ BIT(CAUSE_LOAD_PAGE_FAULT, U) | \
+ BIT(CAUSE_STORE_PAGE_FAULT, U))
+
+#define HIDELEG_DEFAULT (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP)
+
+static void vcpu_csr_init(struct vcpu *v)
+{
+ v->arch.hedeleg = HEDELEG_DEFAULT & csr_masks.hedeleg;
+
+ vcpu_guest_cpu_user_regs(v)->hstatus = HSTATUS_SPV | HSTATUS_SPVP;
+
+ v->arch.hideleg = HIDELEG_DEFAULT & csr_masks.hideleg;
+
+ /*
+ * VS should access only the time counter directly.
+ * Everything else should trap.
+ */
+ v->arch.hcounteren = HCOUNTEREN_TM;
+
+ 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_smstateen) )
+ {
+ register_t hstateen0 = 0;
+
+ if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_ssaia) )
+ /*
+ * If the hypervisor extension is implemented, the same three
+ * bits are defined also in hypervisor CSR hstateen0 but concern
+ * only the state potentially accessible to a virtual machine
+ * executing in privilege modes VS and VU:
+ * bit 60 CSRs siselect and sireg (really vsiselect and
+ * vsireg)
+ * bit 59 CSRs siph and sieh (RV32 only) and stopi (really
+ * vsiph, vsieh, and vstopi)
+ * bit 58 all state of IMSIC guest interrupt files, including
+ * CSR stopei (really vstopei)
+ * If one of these bits is zero in hstateen0, and the same bit is
+ * one in mstateen0, then an attempt to access the corresponding
+ * state from VS or VU-mode raises a virtual instruction exception.
+ */
+ hstateen0 = SMSTATEEN0_AIA | SMSTATEEN0_IMSIC | SMSTATEEN0_SVSLCT;
+
+ /* Allow guest to access CSR_ENVCFG */
+ hstateen0 |= SMSTATEEN0_HSENVCFG;
I continue to be puzzled by the use of = vs |=. If you use |=, best do so
uniformly. Then inserting new code ahead of the one you have now is not a
problem. I wonder anyway why you don't do (omitting commentary):
register_t hstateen0 = SMSTATEEN0_HSENVCFG;
if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_ssaia) )
hstateen0 |= SMSTATEEN0_AIA | SMSTATEEN0_IMSIC | SMSTATEEN0_SVSLCT;
As to CSR_ENVCFG - what's that referring to? I'm aware of menvcfg, henvcfg,
and senvcfg. But I'm unaware of plain envcfg, and there's also no CSR_ENVCFG
constant in riscv_encoding.h afaics. I assume it's senvcfg that you mean
here.
I referred to CSR_SENVCFG, I just automatically applied the way how it is
defined
in Linux kernel and Linux kernel abstracts it as CSR_ENVCFG as it could be
booted
in M-mode or S-mode. There is no such definition in Xen as we don't use it.
And then - is this CSR unconditionally available? The "Supervisor ISA"
isn't called an extension, yet at the same time it's also part of the
separate "privileged" specification, not the general one.
I don't know, the available specs aren't precise here. Considering that
OpenSBI(menvcfg) or Linux(menvcfg or senvcfg) uses them unconditionally
then an assumption that CSR_*ENVCFG unconditionally exist could be done.
Anyway, a ENVCFG bit in hstateen0 depends only on RISCV_ISA_EXT_smstateen
(or Ssstateen extension) then it is okay to set that bit and even
CSR_*ENVCFG isn't implemented a trap to Xen will happen and it should be
handled somehow separately as I mentioned above Linux kernel uses them
unconditionally.
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -48,6 +48,12 @@ struct arch_vcpu {
} xen_saved_context;
struct cpu_info *cpu_info;
+
+ register_t hcounteren;
+ register_t hedeleg;
+ register_t henvcfg;
+ register_t hideleg;
+ register_t hstateen0;
};
One question about the ordering here: It looks to be alphabetically sorted
right now, yet I wonder whether that's optimal. Some CSRs might typically
be used together, in which case they may best live close together (for
chances to be good that they end up in the same cache line).
Make sense, I will group then hedeleg and hideleg.
Thanks.
~ Oleksii
