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
