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

Oleksii Kurochko posted 16 patches 2 weeks, 3 days ago
[PATCH v2 01/16] xen/riscv: introduce struct arch_vcpu
Posted by Oleksii Kurochko 2 weeks, 3 days ago
Introdce struct arch_vcpu to hold RISC-V vCPU-specific state.

The structure contains:
  - Guest-visible CSR state, used to save and restore vCPU execution
    state across context switches. hstatus isn't added here as it is
    already part of cpu_user_regs struct.
  - Callee-saved registers used to preserve Xen’s own execution context
    when switching between vCPU stacks.

It is going to be used in the following way (pseudocode):
  context_switch(prev_vcpu, next_vcpu):
    ...

    /* Switch from previous stack to the next stack. */
    __context_switch(prev_vcpu, next_vcpu);

    ...
    schedule_tail(prev_vcpu):
       Save and restore vCPU's CSRs.

The Xen-saved context allows __context_switch() to switch execution
from the previous vCPU’s stack to the next vCPU’s stack and later resume
execution on the original stack when switching back.

During vCPU creation, the Xen-saved context is going to be initialized
with:
  - SP pointing to the newly allocated vCPU stack
  - RA pointing to a helper that performs final vCPU setup before
    transferring control to the guest
After the first execution of __context_switch(), RA naturally points to
the instruction following the call site, and the remaining callee-saved
registers contain the Xen register state at the time of the switch.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v2:
 - Drop hstatus from struct arch_vcpu as it is stored in struct cpu_user_regs
   which will be stored on top of vCPU's stack.
 - Drop the comment above ra in xen_saved_context struct as it is potentially
   misleading.
---
 xen/arch/riscv/include/asm/domain.h | 57 ++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/xen/arch/riscv/include/asm/domain.h b/xen/arch/riscv/include/asm/domain.h
index 316e7c6c8448..0d9b4c4b525e 100644
--- 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.
+     */
+    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;
+    } xen_saved_context;
+
+    /* CSRs */
+    register_t hedeleg;
+    register_t hideleg;
+    register_t hvip;
+    register_t hip;
+    register_t hie;
+    register_t hgeie;
+    register_t henvcfg;
+    register_t hcounteren;
+    register_t htimedelta;
+    register_t htval;
+    register_t htinst;
+    register_t hstateen0;
+#ifdef CONFIG_RISCV_32
+    register_t henvcfgh;
+    register_t htimedeltah;
+#endif
+
+    /* VCSRs */
+    register_t vsstatus;
+    register_t vsip;
+    register_t vsie;
+    register_t vstvec;
+    register_t vsscratch;
+    register_t vscause;
+    register_t vstval;
+    register_t vsatp;
+    register_t vsepc;
+}  __cacheline_aligned;
 
 struct paging_domain {
     spinlock_t lock;
-- 
2.52.0


Re: [PATCH v2 01/16] xen/riscv: introduce struct arch_vcpu
Posted by Jan Beulich 1 week, 6 days ago
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"?

Also, what about tp? The 't' in there isn't the same as that in "t0", "t1", etc.

> +    } xen_saved_context;
> +
> +    /* CSRs */
> +    register_t hedeleg;
> +    register_t hideleg;
> +    register_t hvip;
> +    register_t hip;
> +    register_t hie;
> +    register_t hgeie;
> +    register_t henvcfg;
> +    register_t hcounteren;
> +    register_t htimedelta;
> +    register_t htval;
> +    register_t htinst;
> +    register_t hstateen0;
> +#ifdef CONFIG_RISCV_32
> +    register_t henvcfgh;
> +    register_t htimedeltah;
> +#endif
> +
> +    /* VCSRs */

Why the V? These are normal CSRs dedicated to VS use, aren't they?

> +    register_t vsstatus;
> +    register_t vsip;
> +    register_t vsie;
> +    register_t vstvec;
> +    register_t vsscratch;
> +    register_t vscause;
> +    register_t vstval;
> +    register_t vsatp;
> +    register_t vsepc;
> +}  __cacheline_aligned;

I continue to question the presence of this attribute.

Jan
Re: [PATCH v2 01/16] xen/riscv: introduce struct arch_vcpu
Posted by Oleksii Kurochko 1 week, 6 days ago
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().

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.
+     */

>
> 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.
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.

>
>> +    } xen_saved_context;
>> +
>> +    /* CSRs */
>> +    register_t hedeleg;
>> +    register_t hideleg;
>> +    register_t hvip;
>> +    register_t hip;
>> +    register_t hie;
>> +    register_t hgeie;
>> +    register_t henvcfg;
>> +    register_t hcounteren;
>> +    register_t htimedelta;
>> +    register_t htval;
>> +    register_t htinst;
>> +    register_t hstateen0;
>> +#ifdef CONFIG_RISCV_32
>> +    register_t henvcfgh;
>> +    register_t htimedeltah;
>> +#endif
>> +
>> +    /* VCSRs */
> Why the V? These are normal CSRs dedicated to VS use, aren't they?

I meant VSCSRs, yes, it is normal CSRs dedicated to VS use.
I'll drop the comment as from the name it is clear that VS-mode CSRs.

>
>> +    register_t vsstatus;
>> +    register_t vsip;
>> +    register_t vsie;
>> +    register_t vstvec;
>> +    register_t vsscratch;
>> +    register_t vscause;
>> +    register_t vstval;
>> +    register_t vsatp;
>> +    register_t vsepc;
>> +}  __cacheline_aligned;
> I continue to question the presence of this attribute.

I will drop it until I could really measure that it boosts performance.
At the moment, it is just an assumption.

~ Oleksii


Re: [PATCH v2 01/16] xen/riscv: introduce struct arch_vcpu
Posted by Jan Beulich 1 week, 6 days ago
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

Re: [PATCH v2 01/16] xen/riscv: introduce struct arch_vcpu
Posted by Oleksii Kurochko 1 week, 6 days ago
On 1/26/26 1:53 PM, Jan Beulich wrote:
> 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".

Yeah, but at time when I was writing the commit I thought only about one meaning
"far from where the stack starts when we enter the hypervisor".


>
>> 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)?

Totally agree, that why I mentioned in reply that it could (it would be better if
"must/should" were used) be dropped as it shouldn't be preserved across calls and
as you also notice that it ought to be stable across Xen as well (or not be used
at all).

~ Oleksii