Re: [PATCH v2 01/16] xen/riscv: introduce struct arch_vcpu

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.01.2026 13:30, Oleksii Kurochko wrote:
> On 1/26/26 12:41 PM, Jan Beulich wrote:
>> On 22.01.2026 17:47, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/include/asm/domain.h
>>> +++ b/xen/arch/riscv/include/asm/domain.h
>>> @@ -22,9 +22,62 @@ struct hvm_domain
>>>   struct arch_vcpu_io {
>>>   };
>>>   
>>> -struct arch_vcpu {
>>> +struct arch_vcpu
>>> +{
>>>       struct vcpu_vmid vmid;
>>> -};
>>> +
>>> +    /*
>>> +     * Callee saved registers for Xen's state deep in the callframe used to
>>> +     * switch from prev's stack to the next's stack during context switch.
>>> +     */
>> What is "deep in the callframe" intended to convey? I'm in particular 
>> wondering
>> about ...
>>
>>> +    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;
>>> +        register_t ra;
>> ... sp and ra, which presumably don't live anywhere "deep"?
> 
> context_switch() is invoked relatively deep in the call stack, so the stack
> pointer in use when context_switch() executes can also be considered to be
> deep in the call frame. The same applies to RA: after the first
> __context_switch() call, RA will point to the next instruction within
> context_switch().

While writing, did you maybe notice that "deep" can have two entirely distinct
meanings here? It could be "far from where the stack starts when we enter the
hypervisor" or "far from present top of stack".

> I can update the comment and drop the wording about being “deep in the call
> frame” to avoid confusion. In that case it would simply read:
> 
> +    /*
> +     * Callee saved registers for Xen's state used to
> +     * switch from prev's stack to the next's stack during context switch.
> +     */

Yes please.

>> Also, what about tp? The 't' in there isn't the same as that in "t0", "t1", 
>> etc.
> 
> tp stores pcpu_info[] and it isn't expected to be changed during (or between) 
> function
> calls.

Oh, right, I forgot about that aspect. However, the more that you reference ...

> In this structure we are dealing only with registers which should be saved 
> according
> to RISC-V ABI convention:
>   [1] 
> https://riscv-non-isa.github.io/riscv-elf-psabi-doc/#_integer_register_convention
> The exception is for RA (as it is also used to jump to continue_to_new_vcpu() 
> when vcpu is scheduled
> first time). During a review of the [1], I think that GP could be dropped as 
> it shouldn't
> be preserved across calls.

... this - why would gp then need saving? That ought to be stable across Xen as
well (or not be used at all)?

Jan