[PATCH v4 23/42] xen/riscv: set the guest's XLEN explicitly in hstatus.VSXL

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

The reset value of hstatus.VSXL isn't architecturally defined, so it
has to be programmed explicitly from the domain's type instead of
relying on whatever the hardware happens to leave there.

This matters beyond the guest's own view of itself: decoding of a
trapped instruction depends on the effective XLEN of the guest, as the
encodings which exist for XLEN=64 only must not be recognized for a
32-bit one.

The field is WARL: an implementation is permitted to make it read-only
with a value that always keeps VSXLEN == HSXLEN. Probe at boot which
encodings can really be set, and refuse to construct a domain whose
width the hardware cannot provide, rather than silently running it at
the wrong XLEN.

The field occupies bits 33:32 and hence doesn't exist when HSXLEN=32,
where VSXLEN is always 32 and HSTATUS_VSXL isn't even defined. Hence
the split on CONFIG_RISCV_32, even though RV32 isn't buildable yet.

vCPU0 is created before kernel_probe() reads the guest kernel header,
from which set_domain_type() derives the domain's type, and hence the
guest's XLEN, so the field can't be set when the vCPU is created:
vcpu_csr_init() leaves it zero. vcpu_set_vsxl() is to fill it in from
continue_new_vcpu() instead, on the vCPU's first entry, which every
vCPU goes through whichever way its domain is built.
continue_new_vcpu() isn't implemented yet, so for now it only
references vcpu_set_vsxl() to keep the compiler from complaining about
it being unused.

By then the domain is built and about to run, so it is too late to
reject a width the hardware cannot provide. construct_domain()
therefore checks it up front, before any of the domain's images are
loaded.

domain_vsxl() only maps a domain type onto the encoding, and
hstatus_vsxl_settable() tells whether the hardware accepts it. The
switch() in domain_vsxl() deliberately has no default case, so that
adding a 128-bit domain type fails to build until the mapping is
updated. Should it nevertheless be left without a mapping, it returns
the reserved encoding, named XLEN_FIELD_RESERVED, which
hstatus_vsxl_settable() never accepts.

While at it drop HSTATUS_VSXL_SHIFT, which has no users and isn't
expected to gain any.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Changes in v4:
 - Move ahead of "xen/riscv: implement continue_new_vcpu()", so that
   hstatus is never loaded with the reserved VSXL encoding of 0.
 - Move the explanation of the missing default case into the switch() of
   domain_vsxl(), saying it is -Wswitch which fails the build.
 - Introduce XLEN_FIELD_RESERVED for the value domain_vsxl() returns
   when it can't map the domain's type.
 - Name kernel_probe() in the comment in vcpu_csr_init() and in the
   commit message, as "probe" could be taken for the probing of the
   settable hstatus.VSXL values.
---
Changes in v3:
 - Derive the width from the domain's type instead of hardcoding
   HSTATUS_VSXL_64, now that the preceding patches make d->type correct.
 - Probe at boot which VSXL encodings the hardware really accepts, and
   refuse in construct_domain() a domain whose width it cannot provide
   instead of silently running the guest at the wrong XLEN.
 - Move the write out of vcpu_csr_init(), which runs before the domain's
   type is known, into vcpu_set_vsxl() called from continue_new_vcpu().
   As a consequence the patch is moved after the one implementing
   continue_new_vcpu().
 - Handle RV32, where the field doesn't exist at all and VSXLEN is
   always 32.
 - Use XLEN_FIELD_{32,64} introduced earlier in the series instead of
   adding HSTATUS_VSXL_{32,64}: it is the same encoding, and unlike the
   latter it isn't hidden behind #if __riscv_xlen == 64, which the RV32
   paths need.
 - Drop HSTATUS_VSXL_SHIFT, which has no users.
 - Rework the commit message accordingly.
---
Changes in v2:
 - New patch.
---
---
 xen/arch/riscv/domain-build.c               |   8 ++
 xen/arch/riscv/domain.c                     | 123 ++++++++++++++++++++
 xen/arch/riscv/include/asm/domain.h         |   3 +
 xen/arch/riscv/include/asm/riscv_encoding.h |   2 +-
 xen/arch/riscv/include/asm/setup.h          |   2 +
 xen/arch/riscv/setup.c                      |   2 +
 6 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/xen/arch/riscv/domain-build.c b/xen/arch/riscv/domain-build.c
index 1e3abe259ccd..1acaf0946130 100644
--- a/xen/arch/riscv/domain-build.c
+++ b/xen/arch/riscv/domain-build.c
@@ -15,9 +15,17 @@ int __init construct_domain(struct domain *d, struct 
kernel_info *kinfo)
 {
     struct vcpu *v = d->vcpu[0];
     struct cpu_user_regs *regs = vcpu_guest_cpu_user_regs(v);
+    unsigned int vsxl = domain_vsxl(d);
 
     BUG_ON(v->is_initialised);
 
+    /* Refuse the domain before any of its images are loaded. */
+    if ( !hstatus_vsxl_settable(vsxl) )
+    {
+        printk(XENLOG_ERR "%pd: hstatus.VSXL cannot be set to %u\n", d, vsxl);
+        return -EOPNOTSUPP;
+    }
+
     /*
      * At the moment *_load() don't return value and will just panic()
      * inside.
diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
index 7be019e2ad12..66fa14c89eaf 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -51,6 +51,121 @@ static struct csr_masks __ro_after_init csr_masks;
 #define HENVCFG_VALID_MASK 0xe0000003000000ffUL
 #define HSTATEEN0_VALID_MASK 0xde00000000000007UL
 
+/*
+ * Return the hstatus.VSXL value encoding the guest's XLEN. Whether the
+ * hardware can actually provide that width is a separate question, answered
+ * by hstatus_vsxl_settable().
+ */
+unsigned int domain_vsxl(const struct domain *d)
+{
+    switch ( d->type )
+    {
+    case DOMAIN_32BIT:
+        return XLEN_FIELD_32;
+
+    case DOMAIN_64BIT:
+        return XLEN_FIELD_64;
+
+    /*
+     * No default case: -Wswitch then fails the build should a new
+     * domain_type (a 128-bit one, in particular) be added without
+     * updating this mapping.
+     */
+    }
+
+    ASSERT_UNREACHABLE();
+
+    /* Never accepted by hstatus_vsxl_settable(). */
+    return XLEN_FIELD_RESERVED;
+}
+
+#ifdef CONFIG_RISCV_32
+
+/*
+ * hstatus.VSXL occupies bits 33:32 and hence does not exist when HSXLEN=32.
+ * VSXLEN is then always 32, so there is nothing to probe and no width other
+ * than 32 to accept.
+ */
+void __init init_hstatus_vsxl_settable_mask(void)
+{
+}
+
+bool hstatus_vsxl_settable(unsigned int vsxl)
+{
+    return vsxl == XLEN_FIELD_32;
+}
+
+static void vcpu_set_vsxl(struct vcpu *v)
+{
+    ASSERT(is_32bit_domain(v->domain));
+}
+
+#else /* CONFIG_RISCV_64 */
+
+/*
+ * Bitmap of the hstatus.VSXL values this hardware accepts, indexed by field
+ * value. Not one of csr_masks: those are bit masks ANDed into a CSR write,
+ * this is a set of legal values of a single WARL field.
+ */
+static unsigned int __ro_after_init hstatus_vsxl_settable_mask;
+
+/*
+ * hstatus.VSXL is WARL and, in particular, an implementation is permitted to
+ * make it read-only with a value that always keeps VSXLEN == HSXLEN. Probe
+ * once which values can really be set, so that a domain whose width the
+ * hardware cannot provide is refused rather than silently run at the wrong
+ * XLEN.
+ *
+ * Every encoding the field can hold is probed, XLEN_FIELD_128 included, so
+ * that a guest width added later needs no change here.
+ *
+ * Like the CSR masks below this assumes all harts behave the same way.
+ */
+void __init init_hstatus_vsxl_settable_mask(void)
+{
+    register_t hstatus = csr_read(CSR_HSTATUS);
+
+    /* Encoding 0 is reserved, hence starting at the narrowest width. */
+    for ( unsigned int vsxl = XLEN_FIELD_32; vsxl <= XLEN_FIELD_128; vsxl++ )
+    {
+        register_t test = (hstatus & ~HSTATUS_VSXL) |
+                          MASK_INSR(vsxl, HSTATUS_VSXL);
+
+        csr_write(CSR_HSTATUS, test);
+
+        if ( MASK_EXTR(csr_read(CSR_HSTATUS), HSTATUS_VSXL) == vsxl )
+            hstatus_vsxl_settable_mask |= BIT(vsxl, U);
+    }
+
+    csr_write(CSR_HSTATUS, hstatus);
+}
+
+/*
+ * Whether the hardware really accepts the given hstatus.VSXL value, i.e.
+ * whether a guest of that XLEN can be run at all.
+ */
+bool hstatus_vsxl_settable(unsigned int vsxl)
+{
+    return hstatus_vsxl_settable_mask & BIT(vsxl, U);
+}
+
+/*
+ * Set the guest's XLEN explicitly rather than leaving it to the WARL
+ * behaviour of hstatus.VSXL.
+ */
+static void vcpu_set_vsxl(struct vcpu *v)
+{
+    struct cpu_user_regs *regs = vcpu_guest_cpu_user_regs(v);
+    unsigned int vsxl = domain_vsxl(v->domain);
+
+    ASSERT(hstatus_vsxl_settable(vsxl));
+
+    regs->hstatus = (regs->hstatus & ~HSTATUS_VSXL) |
+                    MASK_INSR(vsxl, HSTATUS_VSXL);
+}
+
+#endif /* CONFIG_RISCV_32 */
+
 void __init init_csr_masks(void)
 {
     /*
@@ -91,6 +206,11 @@ static void vcpu_csr_init(struct vcpu *v)
 {
     v->arch.hedeleg = HEDELEG_DEFAULT & csr_masks.hedeleg;
 
+    /*
+     * hstatus.VSXL is left zero here: vCPU0 is created before
+     * kernel_probe() reads the guest kernel header, so the domain's type,
+     * and hence the guest's XLEN, is not known yet.
+     */
     vcpu_guest_cpu_user_regs(v)->hstatus = HSTATUS_SPV | HSTATUS_SPVP;
 
     v->arch.hideleg = HIDELEG_DEFAULT & csr_masks.hideleg;
@@ -134,6 +254,9 @@ static void vcpu_csr_init(struct vcpu *v)
 
 static void continue_new_vcpu(struct vcpu *prev)
 {
+    /* Avoid an unused-function error until this function is implemented. */
+    (void)vcpu_set_vsxl;
+
     BUG_ON("unimplemented\n");
 }
 
diff --git a/xen/arch/riscv/include/asm/domain.h 
b/xen/arch/riscv/include/asm/domain.h
index c424e2428f60..005553648a3b 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -144,6 +144,9 @@ int vcpu_unset_interrupt(struct vcpu *v, unsigned int irq);
 void vcpu_sync_interrupts(struct vcpu *curr);
 void vcpu_flush_interrupts(struct vcpu *curr);
 
+bool hstatus_vsxl_settable(unsigned int vsxl);
+unsigned int domain_vsxl(const struct domain *d);
+
 #endif /* RISCV_DOMAIN_H */
 
 /*
diff --git a/xen/arch/riscv/include/asm/riscv_encoding.h 
b/xen/arch/riscv/include/asm/riscv_encoding.h
index 4529bef856b1..e909e1742f51 100644
--- a/xen/arch/riscv/include/asm/riscv_encoding.h
+++ b/xen/arch/riscv/include/asm/riscv_encoding.h
@@ -69,13 +69,13 @@
  * Width encoded by the MXL, SXL, UXL and VSXL fields, all of which share one
  * encoding. 0 is reserved.
  */
+#define XLEN_FIELD_RESERVED            _UL(0)
 #define XLEN_FIELD_32                  _UL(1)
 #define XLEN_FIELD_64                  _UL(2)
 #define XLEN_FIELD_128                 _UL(3)
 
 #if __riscv_xlen == 64
 #define HSTATUS_VSXL                   _UL(0x300000000)
-#define HSTATUS_VSXL_SHIFT             32
 #endif
 #define HSTATUS_VTSR                   _UL(0x00400000)
 #define HSTATUS_VTW                    _UL(0x00200000)
diff --git a/xen/arch/riscv/include/asm/setup.h 
b/xen/arch/riscv/include/asm/setup.h
index 8eee8a746335..7e6e81751ddb 100644
--- a/xen/arch/riscv/include/asm/setup.h
+++ b/xen/arch/riscv/include/asm/setup.h
@@ -11,6 +11,8 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long 
len);
 
 void init_csr_masks(void);
 
+void init_hstatus_vsxl_settable_mask(void);
+
 #endif /* RISCV_SETUP_H */
 
 /*
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index 05f93a74c9d3..fd699defcfc3 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -149,6 +149,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
 
     init_csr_masks();
 
+    init_hstatus_vsxl_settable_mask();
+
     preinit_xen_time();
 
     intc_preinit();
-- 
2.56.1