xen/riscv: introduce vtimer related things.

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

Posted by Oleksii Kurochko 2 weeks ago

Introduce structure with VCPU's registers which describes its state.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/include/asm/domain.h | 58 ++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/xen/arch/riscv/include/asm/domain.h b/xen/arch/riscv/include/asm/domain.h
index 316e7c6c8448..639cafdade99 100644
--- 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 */
+    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;
+
+    /* CSRs */
+    register_t hstatus;
+    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 v1 01/15] xen/riscv: introduce struct arch_vcpu

Posted by Jan Beulich 2 days, 19 hours ago

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.

> +    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. Nor is there a
similar, partly potentially misleading comment on "pc" there
comparable to the one that you added for "ra". ("Potentially
misleading" because what is being described is, aiui, not the only
and not even the main purpose of the field.)

> +    /* CSRs */
> +    register_t hstatus;
> +    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;

Why this attribute?

Jan

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

Posted by Oleksii Kurochko 1 day, 22 hours ago

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

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.

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.

> Nor is there a
> similar, partly potentially misleading comment on "pc" there
> comparable to the one that you added for "ra". ("Potentially
> misleading" because what is being described is, aiui, not the only
> and not even the main purpose of the field.)

Yes, the purpose of|ra| here is not just to jump to the new vCPU code
(|continue_new_vcpu()|). It is used that way only the first time;
afterwards,|ra| will simply point to the next instruction after the
call to|__context_switch()| in|context_switch()| [5].

[5] https://gitlab.com/xen-project/people/olkur/xen/-/blob/riscv-next-upstreaming/xen/arch/riscv/domain.c?ref_type=heads#L463

>
>> +    /* CSRs */
>> +    register_t hstatus;
>> +    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;
> Why this attribute?

As arch_vcpu structure is accessed pretty often I thought it would
be nice to have it cache-aligned so some accesses would be faster
and something like false sharing won't happen.

~ Oleksii

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

Posted by Jan Beulich 1 day, 22 hours ago

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.

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

>> Nor is there a
>> similar, partly potentially misleading comment on "pc" there
>> comparable to the one that you added for "ra". ("Potentially
>> misleading" because what is being described is, aiui, not the only
>> and not even the main purpose of the field.)
> 
> Yes, the purpose of|ra| here is not just to jump to the new vCPU code
> (|continue_new_vcpu()|). It is used that way only the first time;
> afterwards,|ra| will simply point to the next instruction after the
> call to|__context_switch()| in|context_switch()| [5].
> 
> [5] https://gitlab.com/xen-project/people/olkur/xen/-/blob/riscv-next-upstreaming/xen/arch/riscv/domain.c?ref_type=heads#L463
> 
>>
>>> +    /* CSRs */
>>> +    register_t hstatus;
>>> +    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;
>> Why this attribute?
> 
> As arch_vcpu structure is accessed pretty often I thought it would
> be nice to have it cache-aligned so some accesses would be faster
> and something like false sharing won't happen.

I think you would want to prove that this actually makes a difference.
I notice Arm has such an attribute (and maybe indeed you merely copied
it), but x86 doesn't.

Jan

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

Posted by Oleksii Kurochko 1 day, 21 hours ago

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

>
>>> Nor is there a
>>> similar, partly potentially misleading comment on "pc" there
>>> comparable to the one that you added for "ra". ("Potentially
>>> misleading" because what is being described is, aiui, not the only
>>> and not even the main purpose of the field.)
>> Yes, the purpose of|ra| here is not just to jump to the new vCPU code
>> (|continue_new_vcpu()|). It is used that way only the first time;
>> afterwards,|ra| will simply point to the next instruction after the
>> call to|__context_switch()| in|context_switch()| [5].
>>
>> [5] https://gitlab.com/xen-project/people/olkur/xen/-/blob/riscv-next-upstreaming/xen/arch/riscv/domain.c?ref_type=heads#L463
>>
>>>> +    /* CSRs */
>>>> +    register_t hstatus;
>>>> +    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;
>>> Why this attribute?
>> As arch_vcpu structure is accessed pretty often I thought it would
>> be nice to have it cache-aligned so some accesses would be faster
>> and something like false sharing won't happen.
> I think you would want to prove that this actually makes a difference.
> I notice Arm has such an attribute (and maybe indeed you merely copied
> it), but x86 doesn't.

I haven't measured, but I saw that Arm has and it was my explanation to
myself to put it for RISC-V too.

~ Oleksii

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

Posted by Jan Beulich 1 day, 20 hours ago

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. That said, seeing
that Andrew had asked for this, so be it then (albeit I remain unconvinced).

Jan

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

Posted by Oleksii Kurochko 1 day, 20 hours ago

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

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

Posted by Andrew Cooper 1 day, 21 hours ago

On 06/01/2026 2:26 pm, Jan Beulich wrote:
> On 06.01.2026 15:19, Oleksii Kurochko wrote:
>>>> +    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?

This was my recommendation after reverse engineering how ARM worked to
explain it to Oleksii.  But I also thought I said to write a real
comment too.

This is arbitrary *Xen* state, not guest state like you'd expect to find
in struct vcpu.  The guest GPR state is at the base of the vCPU stack.

I suggested that this property be made clearer for the benefit of anyone
trying to decipher the context switching logic.

~Andrew