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(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
On 1/6/26 4:56 PM, Jan Beulich wrote:
> (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.
There is really no need for contiguous memory, and|vmalloc()| could be used.
I expect that|vmalloc()| is more expensive and may make hardware prefetching less
effective, with more TLB pressure since it allocates 4 KB pages.
However, the latter two points do not really matter in this case, as only a
single 4 KB page is allocated, so we are unlikely to see any performance issues.
>
>> + v->arch.cpu_info = (struct cpu_info *)(v->arch.stack
>> + + STACK_SIZE
>> + - sizeof(struct cpu_info));
> Why the cast?
Just for readability, from compiler point of view it could be just dropped.
>
>> + 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.)
I didn’t consider the case where a large number of vCPUs are created, as
I have only tested with 2 vCPUs. However, if the number of vCPUs is large,
this could indeed get quite noisy.
I will keep these lines of code in downstream for debugging purposes and
drop them from upstream version of this patch.
>> --- 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().
Makes sense, I will add:
BUILD_BUG_ON(offsetof(struct cpu_info, guest_cpu_user_regs) != 0);
in|arch_vcpu_create()|, somewhere around the initialization of|v->arch.cpu_info = ... . |I noticed that there is no|BUILD_BUG_ON()| variant that can be used outside
of a function, or does such a variant exist and I’m just missing it? Or there
is no such sense at all for such variant?
>
>> --- 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?
No, I don't need. I think we can just keep cpu_info and it would be
enough. Thanks. ~ Oleksii
On 12.01.2026 11:19, Oleksii Kurochko wrote:
> On 1/6/26 4:56 PM, Jan Beulich wrote:
>> (some or even all of the comments may also apply to present Arm code)
>>
>> On 24.12.2025 18:03, Oleksii Kurochko wrote:
>>> + v->arch.cpu_info = (struct cpu_info *)(v->arch.stack
>>> + + STACK_SIZE
>>> + - sizeof(struct cpu_info));
>> Why the cast?
>
> Just for readability, from compiler point of view it could be just dropped.
Sorry, for me readability suffers from the cast and the then necessary
parentheses. Plus I've been keeping to tell you that casts can be dangerous,
and hence they would better only ever be used when really unavoidable.
>>> --- 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().
>
> Makes sense, I will add:
> BUILD_BUG_ON(offsetof(struct cpu_info, guest_cpu_user_regs) != 0);
> in|arch_vcpu_create()|, somewhere around the initialization of|v->arch.cpu_info = ... . |I noticed that there is no|BUILD_BUG_ON()| variant that can be used outside
> of a function, or does such a variant exist and I’m just missing it? Or there
> is no such sense at all for such variant?
There's none, correct. hence why in a few places we have build_assertions()
functions.
Jan
© 2016 - 2026 Red Hat, Inc.