[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v3 38/39] xen/riscv: set the guest's XLEN explicitly in hstatus.VSXL



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.

The domain's type is only established by set_domain_type(), that is
once its kernel image has been probed, whereas vCPU0 is created before
that. vcpu_csr_init() therefore leaves the field zero, and
vcpu_set_vsxl() fills it in from continue_new_vcpu() instead: that runs
once per vCPU with the type already settled, right before
return_to_new_vcpu() loads the saved hstatus into the CSR.

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.

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 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                     | 119 ++++++++++++++++++++
 xen/arch/riscv/include/asm/domain.h         |   3 +
 xen/arch/riscv/include/asm/riscv_encoding.h |   1 -
 xen/arch/riscv/include/asm/setup.h          |   2 +
 xen/arch/riscv/setup.c                      |   2 +
 6 files changed, 134 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 04be628dcdb4..9155ff4234e8 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -51,6 +51,119 @@ 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().
+ *
+ * The switch() deliberately has no default case, so that adding a new
+ * domain_type (a 128-bit one, in particular) fails to build until this
+ * mapping is updated.
+ */
+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;
+    }
+
+    ASSERT_UNREACHABLE();
+
+    /* A reserved encoding, which hstatus_vsxl_settable() never accepts. */
+    return 0;
+}
+
+#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 +204,10 @@ static void vcpu_csr_init(struct vcpu *v)
 {
     v->arch.hedeleg = HEDELEG_DEFAULT & csr_masks.hedeleg;
 
+    /*
+     * hstatus.VSXL is left zero here: the domain's type, and hence the
+     * guest's XLEN, is only known once its kernel image has been probed.
+     */
     vcpu_guest_cpu_user_regs(v)->hstatus = HSTATUS_SPV | HSTATUS_SPVP;
 
     v->arch.hideleg = HIDELEG_DEFAULT & csr_masks.hideleg;
@@ -149,6 +266,8 @@ static void continue_new_vcpu(struct vcpu *prev)
     if ( is_idle_vcpu(current) )
         reset_stack_and_jump(idle_loop);
 
+    vcpu_set_vsxl(current);
+
     /*
      * return_to_new_vcpu() sets up hstatus.SPV, sstatus.SPP and sepc so
      * that sret enters the guest in VS-mode. A trap taken in HS-mode
diff --git a/xen/arch/riscv/include/asm/domain.h 
b/xen/arch/riscv/include/asm/domain.h
index a3fa637b40da..f8fd5964fd74 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -146,6 +146,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 /* ASM__RISCV__DOMAIN_H */
 
 /*
diff --git a/xen/arch/riscv/include/asm/riscv_encoding.h 
b/xen/arch/riscv/include/asm/riscv_encoding.h
index 1af309aa4ad5..578d772d23df 100644
--- a/xen/arch/riscv/include/asm/riscv_encoding.h
+++ b/xen/arch/riscv/include/asm/riscv_encoding.h
@@ -75,7 +75,6 @@
 
 #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 73ce2f293348..ec36cecad50f 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 /* ASM__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.55.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.