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
