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 <[email protected]>
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
