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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

On 9/2/26 5:56 PM, Oleksii Kurochko wrote:
> 
> 
> On 9/2/26 5:17 PM, Oleksii Kurochko wrote:
>>
>>
>> On 9/2/26 4:31 PM, Jan Beulich wrote:
>>> On 02.09.2026 15:29, Oleksii Kurochko wrote:
>>>> On 9/2/26 3:07 PM, Jan Beulich wrote:
>>>>> On 02.09.2026 13:42, Oleksii Kurochko wrote:
>>>>>> 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;
>>>>>
>>>>> I don't understand the need for this, likely because I don't see why
>>>>> domain_vsxl() would need calling from underneath vcpu_create().
>>>>
>>>> The call trace will be the following:
>>>>     vcpu_create() -> arch_vcpu_create() -> vcpu_csr_init() ->
>>>> domain_vsxldomain_vsxl()
>>>>
>>>>       unsigned int vsxl = domain_vsxl(v->domain);
>>>>
>>>>       ...
>>>>
>>>>       vcpu_guest_cpu_user_regs(v)->hstatus =
>>>>           HSTATUS_SPV | HSTATUS_SPVP | MASK_INSR(vsxl, HSTATUS_VSXL);
>>>>
>>>> Without moving vcpu_create(d, 0) after set_domain_type(), domain_vsxl()
>>>> will return something wrong.
>>>
>>> Question is - do you need to set ->hstatus this early?
>>
>> Good point. I think that there is no really such need. It could be set 
>> (at least, VSXL) just before jumping to new vCPU where we know domain 
>> type for sure.
> 
> I planned to set VSXL bits here: https://lore.kernel.org/xen-devel/ 
> cover.1787838835.git.oleksii.kurochko@gmail.com/T/ 
> #m4144ea90815b48f2d281a2267cd28f50bd4cd54c
> 
> But it seems that I can't do that there as considering that platform can 
> do VSXLEN == HSXLEN == 64 but someone will try do run
> vCPU in 32bit mode we can't just crash domain in continue_new_vcpu().

I think that I can have a check that platform supports VSXLEN guest 
requested based on d->type in construct_domain() where d->type will be 
for sure properly set and reject construction of a domain VSXLEN of 
which isn't supported by a platform. And it looks a proper place for 
such check in general. Then in continue_new_vcpu() just set 
hstatus.VSXLEN without any issue as at that moment we will for sure now 
that requested VSXLEN is correct.

Doing in such way will continue to follow your suggestion (not ->hstatus 
so early in vcpu_csr_init()) and also ...

> 
> Then still to set it in arch_vcpu_create() will be better (or in the 
> construct_domain() if we want to avoid to change dom0less common code). 
> At this stage it is easier to reject to create such vCPU which violates 
> platform implementation.

...  help to void changing of common code.

~ Oleksii