Re: [PATCH v4 01/16] xen/riscv: implement arch_vcpu_{create,destroy}()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.02.2026 17:28, Oleksii Kurochko wrote:
> Introduce architecture-specific functions to create and destroy VCPUs.
> Note that arch_vcpu_create() currently returns -EOPNOTSUPP, as the virtual
> timer and interrupt controller are not yet implemented.
> 
> Add calle-saved registers to struct arch_vcpu which are  used to preserve
> Xen’s own execution context when switching between vCPU stacks.
> It is going to be used in the following way (pseudocode):
>   context_switch(prev_vcpu, next_vcpu):
>     ...
> 
>     /* Switch from previous stack to the next stack. */
>     __context_switch(prev_vcpu, next_vcpu);
> 
>     ...
>     schedule_tail(prev_vcpu):
>         Save and restore vCPU's CSRs.
> The Xen-saved context allows __context_switch() to switch execution
> from the previous vCPU’s stack to the next vCPU’s stack and later resume
> execution on the original stack when switching back.
> 
> During vCPU creation, the Xen-saved context is going to be initialized
> with:
>   - SP pointing to the newly allocated vCPU stack.
>   - RA pointing to a helper that performs final vCPU setup before
>     transferring control to the guest.
> 
> As part of this change, add continue_new_vcpu(), which will be used after
> the first context_switch() of a new vCPU. Since this functionality is not
> yet implemented, continue_new_vcpu() is currently provided as a stub.
> The prev argument is going to be set by RISC-V ABI (prev will be stored in
> a0) when __context_swtich() will be introduced and called from
> context_switch().
> 
> Update the STACK_SIZE definition and introduce STACK_ORDER (to align with
> other architectures) for allocating the vCPU stack.

Yet you don't really need STACK_ORDER, as you use vzalloc() (unlike in
particular Arm, but also x86). If there's no expected other use of the
constant, I'd suggest to omit it, to avoid the false impression that
RISC-V is like (again in particular) Arm in regards to how the stack is
being allocated.

> --- /dev/null
> +++ b/xen/arch/riscv/domain.c
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/init.h>
> +#include <xen/mm.h>
> +#include <xen/sched.h>
> +#include <xen/vmap.h>
> +
> +static void continue_new_vcpu(struct vcpu *prev)
> +{
> +    BUG_ON("unimplemented\n");
> +}
> +
> +static void __init __maybe_unused build_assertions(void)
> +{
> +    /*
> +     * Enforce the requirement documented in struct cpu_info that
> +     * guest_cpu_user_regs must be the first field.
> +     */
> +    BUILD_BUG_ON(offsetof(struct cpu_info, guest_cpu_user_regs) != 0);
> +}

Nit: Generally we have this kind of function at the bottom of source files.

> +int arch_vcpu_create(struct vcpu *v)
> +{
> +    int rc = 0;
> +    void *stack = vzalloc(STACK_SIZE);
> +
> +    if ( !stack )
> +        return -ENOMEM;
> +
> +    v->arch.cpu_info = stack + STACK_SIZE - sizeof(struct cpu_info);

Perhaps better sizeof(*v->arch.cpu_info), to connect lhs and rhs?

> +    v->arch.xen_saved_context.sp = (register_t)v->arch.cpu_info;
> +    v->arch.xen_saved_context.ra = (register_t)continue_new_vcpu;
> +
> +    /* Idle VCPUs don't need the rest of this setup */
> +    if ( is_idle_vcpu(v) )
> +        return rc;

I'd suggest "return 0" here to make clear it's a success path. Then
possible uses of "rc" earlier in the function also won't affect this.

With all of the adjustments (happy to carry out while committing, as long
as you agree)
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan