Re: [PATCH v2 03/39] xen/riscv: set the guest's XLEN explicitly in hstatus.VSXL

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/1/26 5:20 PM, Jan Beulich wrote:
> On 27.08.2026 17:20, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/domain.c
> > +++ b/xen/arch/riscv/domain.c
> > @@ -88,7 +88,13 @@ 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;
> > +    /*
> > +     * Xen supports 64-bit guests only, so set the guest's XLEN explicitly
> > +     * rather than leaving it to the WARL behaviour of hstatus.VSXL, which the
> > +     * decoding of a trapped instruction depends on.
> > +     */
> > +    vcpu_guest_cpu_user_regs(v)->hstatus =
> > +        HSTATUS_SPV | HSTATUS_SPVP | MASK_INSR(HSTATUS_VSXL_64, HSTATUS_VSXL);
> The comment is correct right now, but the situation better would change
> at some point. Can't you arrange for things to be correct here also for
> a future where 32- and 128-bit guests would also be supported?
I am not sure about 128-bit guests as H extension is dependent on RV32 
or RV64 but probably it will be changed:
```
The hypervisor extension depends on an "I" base integer ISA with 32 x 
registers (RV32I or RV64I), not RV32E or RV64E, which have only 16 x 
registers.
```

I will introduce the following (also it will be needed also to check if 
we could VSXL set at all as implmentation can make that field read-only 
and do VSXLLEN=HSXLEN):
/*
 * Return the hstatus.VSXL value encoding the guest's XLEN. 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.
 */
static unsigned int domain_vsxl(const struct domain *d)
{
    switch ( d->type )
    {
    case DOMAIN_32BIT:
        return HSTATUS_VSXL_32;

    case DOMAIN_64BIT:
        return HSTATUS_VSXL_64;
    }

    ASSERT_UNREACHABLE();

    return HSTATUS_VSXL_64;
}

It will also affect then common code as vcpu_csr_init() could be then 
called before domain type is set:
+++ b/xen/common/device-tree/dom0less-build.c
@@ -812,17 +812,18 @@ static int __init construct_domU(struct 
kernel_info *kinfo,
     else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") )
         kinfo->dom0less_feature = DOM0LESS_ENHANCED_NO_XS;

-    if ( vcpu_create(d, 0) == NULL )
-        return -ENOMEM;
-
     d->max_pages = ((paddr_t)mem * SZ_1K) >> PAGE_SHIFT;

     rc = kernel_probe(kinfo, node);
     if ( rc < 0 )
         return rc;

+    /* The domain type needs to be known before the first vCPU is 
created. */
     set_domain_type(d, kinfo);

+    if ( vcpu_create(d, 0) == NULL )
+        return -ENOMEM;


> > --- a/xen/arch/riscv/include/asm/riscv_encoding.h
> > +++ b/xen/arch/riscv/include/asm/riscv_encoding.h
> > @@ -68,6 +68,8 @@
> >   #if __riscv_xlen == 64
> >   #define HSTATUS_VSXL                  _UL(0x300000000)
> >   #define HSTATUS_VSXL_SHIFT            32
> > +#define HSTATUS_VSXL_64                        _UL(2)
> > +#define HSTATUS_VSXL_32                        _UL(1)
> >   #endif
> While adding the two #define-s, would you mind considering to remove the
> unused (and supposed to remain so) HSTATUS_VSXL_SHIFT?
Sure, I will drop that.

Thanks.

~ Oleksii