Re: [PATCH v1 01/15] xen/riscv: introduce struct arch_vcpu

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

On 1/6/26 4:33 PM, Jan Beulich wrote:
> On 06.01.2026 16:05, Oleksii Kurochko wrote:
>> On 1/6/26 3:26 PM, Jan Beulich wrote:
>>> On 06.01.2026 15:19, Oleksii Kurochko wrote:
>>>> On 1/5/26 5:58 PM, Jan Beulich wrote:
>>>>> On 24.12.2025 18:03, Oleksii Kurochko wrote:
>>>>>> Introduce structure with VCPU's registers which describes its state.
>>>>>>
>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>> Since none of this is being used for the time being, I think the description
>>>>> wants to be a little less terse. Coming from the x86 (rather then the Arm)
>>>>> side, I find the arrangements irritating. And even when comparing to Arm, ...
>>>>>
>>>>>> --- a/xen/arch/riscv/include/asm/domain.h
>>>>>> +++ b/xen/arch/riscv/include/asm/domain.h
>>>>>> @@ -22,9 +22,63 @@ struct hvm_domain
>>>>>>     struct arch_vcpu_io {
>>>>>>     };
>>>>>>     
>>>>>> -struct arch_vcpu {
>>>>>> +struct arch_vcpu
>>>>>> +{
>>>>>>         struct vcpu_vmid vmid;
>>>>>> -};
>>>>>> +
>>>>>> +    /* Xen's state: Callee-saved registers and tp, gp, ra */
>>>>> ... I don't think the following structure describes "Xen's state". On Arm
>>>>> it's guest controlled register values which are being saved afaict. I
>>>>> would then expect the same to become the case for RISC-V.
>>>> I think this is not fully correct, because guest-controlled registers on
>>>> Arm are allocated on the stack [1][2].
>>> I'll admit that I should have said "possibly guest-controlled". Callee-
>>> saved registers may or may not be used in functions, and if one isn't
>>> used throughout the call-stack reaching __context_switch(), it would
>>> still hold whatever the guest had put there.
>> But the guest doesn't put there nothing, only Xen does that and it is a reason
>> why I am trying to call it Xen state. Guest works only with what is stored in
>> struct cpu_info->guest_cpu_user_regs.* ...
>>
>>>> Regarding|xen_saved_context| (or|saved_context| on Arm, which I used as a base),
>>>> I think|xen_saved_context| is a slightly better name. Looking at how the
>>>> |saved_context| structure is used on Arm [3], it can be concluded that
>>>> |__context_switch()| switches only Xen’s internal context. What actually happens is
>>>> that|__context_switch()| is called while running on the previous vCPU’s stack
>>>> and returns on the next vCPU’s stack. Therefore, it is necessary to have
>>>> the correct register values stored in the|saved_context| structure in order
>>>> to continue Xen’s execution when it later returns to the previous stack.
>>> For this and ...
>>>
>>>> Probably I need to introduce|__context_switch()| in this patch series for RISC-V
>>>> now; I hope this will clarify things better. At the moment, it looks like [4].
>>>>
>>>> [1] https://elixir.bootlin.com/xen/v4.21.0/source/xen/arch/arm/include/asm/arm64/processor.h#L14
>>>> [2] https://elixir.bootlin.com/xen/v4.21.0/source/xen/arch/arm/domain.c#L547
>>>>
>>>> [3] https://elixir.bootlin.com/xen/v4.21.0/source/xen/arch/arm/arm64/entry.S#L650
>>>>
>>>> [4] https://gitlab.com/xen-project/people/olkur/xen/-/blob/riscv-next-upstreaming/xen/arch/riscv/entry.S?ref_type=heads#L153
>>>>
>>>>>> +    struct
>>>>>> +    {
>>>>>> +        register_t s0;
>>>>>> +        register_t s1;
>>>>>> +        register_t s2;
>>>>>> +        register_t s3;
>>>>>> +        register_t s4;
>>>>>> +        register_t s5;
>>>>>> +        register_t s6;
>>>>>> +        register_t s7;
>>>>>> +        register_t s8;
>>>>>> +        register_t s9;
>>>>>> +        register_t s10;
>>>>>> +        register_t s11;
>>>>>> +
>>>>>> +        register_t sp;
>>>>>> +        register_t gp;
>>>>>> +
>>>>>> +        /* ra is used to jump to guest when creating new vcpu */
>>>>>> +        register_t ra;
>>>>>> +    } xen_saved_context;
>>>>> The xen_ prefix here also doesn't exist in Arm code.
>>>> I think it should be added for Arm too. I can send a patch.
>>> ... this, to reword my comment: What value does the xen_ prefix add?
>> ... because guest doesn't access saved_context and as I mentioned above
>> guest has "access" only to struct cpu_info->guest_cpu_user_regs.*.
> The guest has no access to anything in the hypervisor.

Of course, the guest doesn't have access. By "access" I meant guest context
stored in cpu_info->guest_cpu_user_regs.*.

>   That said, seeing
> that Andrew had asked for this, so be it then (albeit I remain unconvinced).

I will add some extra comments about xen_saved_context to make things more
clear.

~ Oleksii