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