[PATCH v1 02/15] xen/riscv: implement arch_vcpu_{create,destroy}()

Oleksii Kurochko posted 15 patches 2 weeks ago
[PATCH v1 02/15] xen/riscv: implement arch_vcpu_{create,destroy}()
Posted by Oleksii Kurochko 2 weeks ago
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.

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.

Update the STACK_SIZE definition and introduce STACK_ORDER (to align with
other architectures) for allocating the vCPU stack.

Introduce struct cpu_info, which will be allocated in arch_vcpu_create()
and declare cpu_info inside arch_vcpu structure.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/Makefile              |  1 +
 xen/arch/riscv/domain.c              | 56 ++++++++++++++++++++++++++++
 xen/arch/riscv/include/asm/config.h  |  3 +-
 xen/arch/riscv/include/asm/current.h |  6 +++
 xen/arch/riscv/include/asm/domain.h  |  3 ++
 xen/arch/riscv/stubs.c               | 10 -----
 6 files changed, 68 insertions(+), 11 deletions(-)
 create mode 100644 xen/arch/riscv/domain.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 87c1148b0010..8863d4b15605 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -1,5 +1,6 @@
 obj-y += aplic.o
 obj-y += cpufeature.o
+obj-y += domain.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-y += entry.o
 obj-y += imsic.o
diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
new file mode 100644
index 000000000000..e5fda1af4ee9
--- /dev/null
+++ b/xen/arch/riscv/domain.c
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/mm.h>
+#include <xen/sched.h>
+
+static void continue_new_vcpu(struct vcpu *prev)
+{
+    BUG_ON("unimplemented\n");
+}
+
+int arch_vcpu_create(struct vcpu *v)
+{
+    int rc = 0;
+
+    BUILD_BUG_ON(sizeof(struct cpu_info) > STACK_SIZE);
+
+    v->arch.stack = alloc_xenheap_pages(STACK_ORDER, MEMF_node(vcpu_to_node(v)));
+    if ( !v->arch.stack )
+        return -ENOMEM;
+
+    v->arch.cpu_info = (struct cpu_info *)(v->arch.stack
+                                           + STACK_SIZE
+                                           - sizeof(struct cpu_info));
+    memset(v->arch.cpu_info, 0, sizeof(*v->arch.cpu_info));
+
+    v->arch.xen_saved_context.sp = (register_t)v->arch.cpu_info;
+    v->arch.xen_saved_context.ra = (register_t)continue_new_vcpu;
+
+    printk("Create vCPU with sp=%#lx, pc=%#lx, cpu_info(%#lx)\n",
+           v->arch.xen_saved_context.sp, v->arch.xen_saved_context.ra,
+           (unsigned long)v->arch.cpu_info);
+
+    /* Idle VCPUs don't need the rest of this setup */
+    if ( is_idle_vcpu(v) )
+        return rc;
+
+    /*
+     * As the vtimer and interrupt controller (IC) are not yet implemented,
+     * return an error.
+     *
+     * TODO: Drop this once the vtimer and IC are implemented.
+     */
+    rc = -EOPNOTSUPP;
+    goto fail;
+
+    return rc;
+
+ fail:
+    arch_vcpu_destroy(v);
+    return rc;
+}
+
+void arch_vcpu_destroy(struct vcpu *v)
+{
+    free_xenheap_pages(v->arch.stack, STACK_ORDER);
+}
diff --git a/xen/arch/riscv/include/asm/config.h b/xen/arch/riscv/include/asm/config.h
index 1e08d3bf78be..86a95df018b5 100644
--- a/xen/arch/riscv/include/asm/config.h
+++ b/xen/arch/riscv/include/asm/config.h
@@ -143,7 +143,8 @@
 
 #define SMP_CACHE_BYTES (1 << 6)
 
-#define STACK_SIZE PAGE_SIZE
+#define STACK_ORDER 3
+#define STACK_SIZE (PAGE_SIZE << STACK_ORDER)
 
 #define IDENT_AREA_SIZE 64
 
diff --git a/xen/arch/riscv/include/asm/current.h b/xen/arch/riscv/include/asm/current.h
index 0c3ea70c2ec8..58c9f1506b7c 100644
--- a/xen/arch/riscv/include/asm/current.h
+++ b/xen/arch/riscv/include/asm/current.h
@@ -21,6 +21,12 @@ struct pcpu_info {
 /* tp points to one of these */
 extern struct pcpu_info pcpu_info[NR_CPUS];
 
+/* Per-VCPU state that lives at the top of the stack */
+struct cpu_info {
+    /* This should be the first member. */
+    struct cpu_user_regs guest_cpu_user_regs;
+};
+
 #define set_processor_id(id)    do { \
     tp->processor_id = (id);         \
 } while (0)
diff --git a/xen/arch/riscv/include/asm/domain.h b/xen/arch/riscv/include/asm/domain.h
index 639cafdade99..a0ffbbc09c6f 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -49,6 +49,9 @@ struct arch_vcpu
         register_t ra;
     } xen_saved_context;
 
+    struct cpu_info *cpu_info;
+    void *stack;
+
     /* CSRs */
     register_t hstatus;
     register_t hedeleg;
diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
index 164fc091b28a..eab826e8c3ae 100644
--- a/xen/arch/riscv/stubs.c
+++ b/xen/arch/riscv/stubs.c
@@ -126,16 +126,6 @@ void free_vcpu_struct(struct vcpu *v)
     BUG_ON("unimplemented");
 }
 
-int arch_vcpu_create(struct vcpu *v)
-{
-    BUG_ON("unimplemented");
-}
-
-void arch_vcpu_destroy(struct vcpu *v)
-{
-    BUG_ON("unimplemented");
-}
-
 void vcpu_switch_to_aarch64_mode(struct vcpu *v)
 {
     BUG_ON("unimplemented");
-- 
2.52.0
Re: [PATCH v1 02/15] xen/riscv: implement arch_vcpu_{create,destroy}()
Posted by Jan Beulich 1 day, 20 hours ago
(some or even all of the comments may also apply to present Arm code)

On 24.12.2025 18:03, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/domain.c
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/mm.h>
> +#include <xen/sched.h>
> +
> +static void continue_new_vcpu(struct vcpu *prev)
> +{
> +    BUG_ON("unimplemented\n");
> +}
> +
> +int arch_vcpu_create(struct vcpu *v)
> +{
> +    int rc = 0;
> +
> +    BUILD_BUG_ON(sizeof(struct cpu_info) > STACK_SIZE);

I fear you're in trouble also when == or when only a few bytes are left on
the stack. IOW I'm unconvinced that this is a useful check to have.

> +    v->arch.stack = alloc_xenheap_pages(STACK_ORDER, MEMF_node(vcpu_to_node(v)));
> +    if ( !v->arch.stack )
> +        return -ENOMEM;

You don't really need contiguous memory, do you? In which case why not
vmalloc()? This would then also use the larger domheap.

> +    v->arch.cpu_info = (struct cpu_info *)(v->arch.stack
> +                                           + STACK_SIZE
> +                                           - sizeof(struct cpu_info));

Why the cast?

> +    memset(v->arch.cpu_info, 0, sizeof(*v->arch.cpu_info));
> +
> +    v->arch.xen_saved_context.sp = (register_t)v->arch.cpu_info;
> +    v->arch.xen_saved_context.ra = (register_t)continue_new_vcpu;
> +
> +    printk("Create vCPU with sp=%#lx, pc=%#lx, cpu_info(%#lx)\n",
> +           v->arch.xen_saved_context.sp, v->arch.xen_saved_context.ra,
> +           (unsigned long)v->arch.cpu_info);

Please don't, as this is going to get pretty noisy. (And if this wanted
keeping, use %p for pointers rather than casting to unsigned long.)

> +    /* Idle VCPUs don't need the rest of this setup */
> +    if ( is_idle_vcpu(v) )
> +        return rc;
> +
> +    /*
> +     * As the vtimer and interrupt controller (IC) are not yet implemented,
> +     * return an error.
> +     *
> +     * TODO: Drop this once the vtimer and IC are implemented.
> +     */
> +    rc = -EOPNOTSUPP;
> +    goto fail;
> +
> +    return rc;
> +
> + fail:
> +    arch_vcpu_destroy(v);
> +    return rc;
> +}
> +
> +void arch_vcpu_destroy(struct vcpu *v)
> +{
> +    free_xenheap_pages(v->arch.stack, STACK_ORDER);
> +}

Better to use FREE_XENHEAP_PAGES() here, I think, to make the function
idempotent.

> --- a/xen/arch/riscv/include/asm/current.h
> +++ b/xen/arch/riscv/include/asm/current.h
> @@ -21,6 +21,12 @@ struct pcpu_info {
>  /* tp points to one of these */
>  extern struct pcpu_info pcpu_info[NR_CPUS];
>  
> +/* Per-VCPU state that lives at the top of the stack */
> +struct cpu_info {
> +    /* This should be the first member. */
> +    struct cpu_user_regs guest_cpu_user_regs;
> +};

You may want to enforce what the comment says by way of a BUILD_BUG_ON().

> --- a/xen/arch/riscv/include/asm/domain.h
> +++ b/xen/arch/riscv/include/asm/domain.h
> @@ -49,6 +49,9 @@ struct arch_vcpu
>          register_t ra;
>      } xen_saved_context;
>  
> +    struct cpu_info *cpu_info;
> +    void *stack;

Do you really need both fields, when one is derived from the other?

Jan