From: Joey Gouly <joey.gouly@arm.com>
To: Jing Zhang <jingzhangos@google.com>
Cc: KVM <kvm@vger.kernel.org>, KVMARM <kvmarm@lists.linux.dev>,
Marc Zyngier <maz@kernel.org>,
Wei-Lin Chang <weilin.chang@arm.com>,
Yao Yuan <yaoyuan@linux.alibaba.com>,
Oliver Upton <oliver.upton@linux.dev>,
Andrew Jones <andrew.jones@linux.dev>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Mingwei Zhang <mizhang@google.com>,
Raghavendra Rao Ananta <rananta@google.com>,
Colton Lewis <coltonlewis@google.com>
Subject: Re: [kvm-unit-tests PATCH v2 4/7] lib: arm64: Add foundational guest execution framework
Date: Thu, 16 Apr 2026 17:16:43 +0100 [thread overview]
Message-ID: <20260416161643.GC3142999@e124191.cambridge.arm.com> (raw)
In-Reply-To: <20260413204630.1149038-5-jingzhangos@google.com>
Hi,
First-look comments below,
On Mon, Apr 13, 2026 at 01:46:27PM -0700, Jing Zhang wrote:
> Introduce the infrastructure to manage and execute guest when running
> at EL2. This provides the basis for testing advanced features like
> nested virtualization and GICv4 direct interrupt injection.
>
> The framework includes:
> - 'struct guest': Encapsulates vCPU state (GPRs, EL1/EL2 sysregs) and
> Stage-2 MMU context.
> - guest_create() / guest_destroy(): Handle lifecycle management and
> Stage-2 MMU setup, including identity mappings for guest code,
> stack, and UART.
> - guest_run(): Assembly entry point that saves host callee-saved
> registers, caches the guest context pointer in TPIDR_EL2, and
> performs the exception return (ERET) to the guest.
>
> Signed-off-by: Jing Zhang <jingzhangos@google.com>
> ---
> arm/Makefile.arm64 | 2 +
> lib/arm64/asm-offsets.c | 13 ++++++
> lib/arm64/asm/guest.h | 46 +++++++++++++++++++++
> lib/arm64/asm/sysreg.h | 6 +++
> lib/arm64/guest.c | 89 +++++++++++++++++++++++++++++++++++++++++
> lib/arm64/guest_arch.S | 59 +++++++++++++++++++++++++++
> 6 files changed, 215 insertions(+)
> create mode 100644 lib/arm64/asm/guest.h
> create mode 100644 lib/arm64/guest.c
> create mode 100644 lib/arm64/guest_arch.S
>
> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> index 5e50f5ba..9026fd71 100644
> --- a/arm/Makefile.arm64
> +++ b/arm/Makefile.arm64
> @@ -41,6 +41,8 @@ cflatobjs += lib/arm64/processor.o
> cflatobjs += lib/arm64/spinlock.o
> cflatobjs += lib/arm64/gic-v3-its.o lib/arm64/gic-v3-its-cmd.o
> cflatobjs += lib/arm64/stage2_mmu.o
> +cflatobjs += lib/arm64/guest.o
> +cflatobjs += lib/arm64/guest_arch.o
>
> ifeq ($(CONFIG_EFI),y)
> cflatobjs += lib/acpi.o
> diff --git a/lib/arm64/asm-offsets.c b/lib/arm64/asm-offsets.c
> index 80de023c..ceeecce5 100644
> --- a/lib/arm64/asm-offsets.c
> +++ b/lib/arm64/asm-offsets.c
> @@ -8,6 +8,7 @@
> #include <libcflat.h>
> #include <kbuild.h>
> #include <asm/ptrace.h>
> +#include <asm/guest.h>
>
> int main(void)
> {
> @@ -30,5 +31,17 @@ int main(void)
> DEFINE(S_FP, sizeof(struct pt_regs));
> DEFINE(S_FRAME_SIZE, (sizeof(struct pt_regs) + 16));
>
> + OFFSET(GUEST_X_OFFSET, guest, x);
> + OFFSET(GUEST_ELR_OFFSET, guest, elr_el2);
> + OFFSET(GUEST_SPSR_OFFSET, guest, spsr_el2);
> + OFFSET(GUEST_HCR_OFFSET, guest, hcr_el2);
> + OFFSET(GUEST_VTTBR_OFFSET, guest, vttbr_el2);
> + OFFSET(GUEST_SCTLR_OFFSET, guest, sctlr_el1);
> + OFFSET(GUEST_SP_EL1_OFFSET, guest, sp_el1);
> + OFFSET(GUEST_ESR_OFFSET, guest, esr_el2);
> + OFFSET(GUEST_FAR_OFFSET, guest, far_el2);
> + OFFSET(GUEST_HPFAR_OFFSET, guest, hpfar_el2);
> + OFFSET(GUEST_EXIT_CODE_OFFSET, guest, exit_code);
> +
> return 0;
> }
> diff --git a/lib/arm64/asm/guest.h b/lib/arm64/asm/guest.h
> new file mode 100644
> index 00000000..826c44f8
> --- /dev/null
> +++ b/lib/arm64/asm/guest.h
> @@ -0,0 +1,46 @@
> +/*
> + * Copyright (C) 2026, Google LLC.
> + * Author: Jing Zhang <jingzhangos@google.com>
> + *
> + * SPDX-License-Identifier: LGPL-2.0-or-later
> + */
> +#ifndef _ASMARM64_GUEST_H_
> +#define _ASMARM64_GUEST_H_
> +
> +#include <libcflat.h>
> +#include <asm/processor.h>
> +#include <asm/stage2_mmu.h>
> +
> +#define HCR_GUEST_FLAGS (HCR_EL2_VM | HCR_EL2_FMO | HCR_EL2_IMO | \
> + HCR_EL2_AMO | HCR_EL2_RW | HCR_EL2_E2H)
> +/* Guest stack size */
> +#define GUEST_STACK_SIZE SZ_64K
> +
> +struct guest {
> + /* General Purpose Registers */
> + unsigned long x[31]; /* x0..x30 */
> +
> + /* Execution State */
> + unsigned long elr_el2;
> + unsigned long spsr_el2;
> +
> + /* Control Registers */
> + unsigned long hcr_el2;
> + unsigned long vttbr_el2;
> + unsigned long sctlr_el1;
> + unsigned long sp_el1;
> +
> + /* Exit Information */
> + unsigned long esr_el2;
> + unsigned long far_el2;
> + unsigned long hpfar_el2;
> + unsigned long exit_code;
> +
> + struct s2_mmu *s2mmu;
> +};
> +
> +struct guest *guest_create(int vmid, void (*guest_func)(void), enum s2_granule granule);
> +void guest_destroy(struct guest *guest);
> +void guest_run(struct guest *guest);
> +
> +#endif /* _ASMARM64_GUEST_H_ */
> diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h
> index f2d05018..857bee98 100644
> --- a/lib/arm64/asm/sysreg.h
> +++ b/lib/arm64/asm/sysreg.h
> @@ -118,6 +118,10 @@ asm(
> #define SCTLR_EL1_TCF0_SHIFT 38
> #define SCTLR_EL1_TCF0_MASK GENMASK_ULL(39, 38)
>
> +#define HCR_EL2_VM _BITULL(0)
> +#define HCR_EL2_FMO _BITULL(3)
> +#define HCR_EL2_IMO _BITULL(4)
> +#define HCR_EL2_AMO _BITULL(5)
> #define HCR_EL2_TGE _BITULL(27)
> #define HCR_EL2_RW _BITULL(31)
> #define HCR_EL2_E2H _BITULL(34)
> @@ -132,6 +136,8 @@ asm(
> #define SYS_HFGWTR2_EL2 sys_reg(3, 4, 3, 1, 3)
> #define SYS_HFGITR2_EL2 sys_reg(3, 4, 3, 1, 7)
>
> +#define SYS_SCTLR_EL1 sys_reg(3, 5, 1, 0, 0)
This isn't needed, the asm below can use `msr sctlr_el1` directly.
> +
> #define INIT_SCTLR_EL1_MMU_OFF \
> (SCTLR_EL1_ITD | SCTLR_EL1_SED | SCTLR_EL1_EOS | \
> SCTLR_EL1_TSCXT | SCTLR_EL1_EIS | SCTLR_EL1_SPAN | \
> diff --git a/lib/arm64/guest.c b/lib/arm64/guest.c
> new file mode 100644
> index 00000000..68dd449d
> --- /dev/null
> +++ b/lib/arm64/guest.c
> @@ -0,0 +1,89 @@
> +/*
> + * Copyright (C) 2026, Google LLC.
> + * Author: Jing Zhang <jingzhangos@google.com>
> + *
> + * SPDX-License-Identifier: LGPL-2.0-or-later
> + */
> +#include <libcflat.h>
> +#include <asm/guest.h>
> +#include <asm/io.h>
> +#include <asm/sysreg.h>
> +#include <asm/barrier.h>
> +#include <alloc_page.h>
> +#include <alloc.h>
> +
> +static struct guest *__guest_create(struct s2_mmu *s2_ctx, void *entry_point)
> +{
> + struct guest *guest = calloc(1, sizeof(struct guest));
Missing check if `guest` is NULL.
> +
> + guest->elr_el2 = (unsigned long)entry_point;
> + guest->spsr_el2 = 0x3C5; /* M=EL1h, DAIF=Masked */
Check lib/arm64/asm/ptrace.h, we can build this with macros, PSR_MODE_EL1h etc.
> + guest->hcr_el2 = HCR_GUEST_FLAGS;
> +
> + if (s2_ctx) {
> + guest->vttbr_el2 = virt_to_phys(s2_ctx->pgd);
> + guest->vttbr_el2 |= ((unsigned long)s2_ctx->vmid << 48);
Maybe this should be extracted into a helper in stage2_mmu, to build vttbr.
> + } else {
> + printf("Stage 2 MMU context missing!");
> + }
Can probably just assert(s2_ctx) instead of this conditional, don't think you
can use the guest without one?
> +
> + guest->sctlr_el1 = read_sysreg(sctlr_el1);
> + /* Disable guest stage 1 translation */
> + guest->sctlr_el1 &= ~(SCTLR_EL1_M | SCTLR_EL1_C);
> + guest->sctlr_el1 |= SCTLR_EL1_I;
> +
> + guest->s2mmu = s2_ctx;
> +
> + return guest;
> +}
> +
> +struct guest *guest_create(int vmid, void (*guest_func)(void), enum s2_granule granule)
> +{
> + unsigned long guest_pa, code_base, stack_pa;
> + unsigned long *stack_page;
> + struct guest *guest;
> + struct s2_mmu *ctx;
> +
> + ctx = s2mmu_init(vmid, granule, true);
> + /*
> + * Map the Host's code segment Identity Mapped (IPA=PA).
> + * To be safe, we map a large chunk (e.g., 2MB) around the function
> + * to capture any helper functions the compiler might generate calls to.
> + */
> + guest_pa = virt_to_phys((void *)guest_func);
> + code_base = guest_pa & ~(SZ_2M - 1);
> + s2mmu_map(ctx, code_base, code_base, SZ_2M, S2_MAP_RW);
> +
> + /*
> + * Map Stack
> + * Allocate 16 pages (64K) in Host, get its PA, and map it for Guest.
It's not always 16 pages (depends on PAGE_SIZE), can just reword to say
`Allocate pages in Host..`
> + */
> + stack_page = alloc_pages(get_order(GUEST_STACK_SIZE >> PAGE_SHIFT));
> + stack_pa = virt_to_phys(stack_page);
> + /* Identity Map it (IPA = PA) */
> + s2mmu_map(ctx, stack_pa, stack_pa, GUEST_STACK_SIZE, S2_MAP_RW);
> +
> + s2mmu_enable(ctx);
> +
> + /* Create Guest */
> + /* Entry point is the PA of the function (Identity Mapped) */
> + guest = __guest_create(ctx, (void *)guest_pa);
> +
> + /*
> + * Setup Guest Stack Pointer
> + * Must match where we mapped the stack + Offset
> + */
> + guest->sp_el1 = stack_pa + GUEST_STACK_SIZE;
> +
> + /* Map UART identity mapped, printf() available to guest */
> + s2mmu_map(ctx, 0x09000000, 0x09000000, PAGE_SIZE, S2_MAP_DEVICE);
This is not portable.
> +
> + return guest;
> +}
I'm actually not sure what I think of this function as a whole, maybe it does
too much and the caller should be expected to map stuff in. Different types of
tests could have different specific guest_create helpers.
> +
> +void guest_destroy(struct guest *guest)
> +{
> + s2mmu_disable(guest->s2mmu);
> + s2mmu_destroy(guest->s2mmu);
> + free(guest);
> +}
> diff --git a/lib/arm64/guest_arch.S b/lib/arm64/guest_arch.S
> new file mode 100644
> index 00000000..70c19507
> --- /dev/null
> +++ b/lib/arm64/guest_arch.S
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright (C) 2026, Google LLC.
> + * Author: Jing Zhang <jingzhangos@google.com>
> + *
> + * SPDX-License-Identifier: LGPL-2.0-or-later
> + */
> +#define __ASSEMBLY__
> +#include <asm/asm-offsets.h>
> +#include <asm/sysreg.h>
> +
> +.global guest_run
> +guest_run:
> + /* x0 = struct guest pointer */
> +
> + /* Save Host Callee-Saved Regs */
> + stp x29, x30, [sp, #-16]!
> + stp x27, x28, [sp, #-16]!
> + stp x25, x26, [sp, #-16]!
> + stp x23, x24, [sp, #-16]!
> + stp x21, x22, [sp, #-16]!
> + stp x19, x20, [sp, #-16]!
> +
> + /* Cache Guest Pointer in TPIDR_EL2 */
> + msr tpidr_el2, x0
> +
> + /* Load Guest System Registers */
> + ldr x1, [x0, #GUEST_ELR_OFFSET]
> + msr elr_el2, x1
> + ldr x1, [x0, #GUEST_SPSR_OFFSET]
> + msr spsr_el2, x1
> + ldr x1, [x0, #GUEST_HCR_OFFSET]
> + msr hcr_el2, x1
> + ldr x1, [x0, #GUEST_VTTBR_OFFSET]
> + msr vttbr_el2, x1
> + ldr x1, [x0, #GUEST_SCTLR_OFFSET]
> + msr_s SYS_SCTLR_EL1, x1
This part, can use `msr sctlr_el1`.
> + ldr x1, [x0, #GUEST_SP_EL1_OFFSET]
> + msr sp_el1, x1
> +
> + /* Load Guest GPRs */
> + ldp x1, x2, [x0, #8]
> + ldp x3, x4, [x0, #24]
> + ldp x5, x6, [x0, #40]
> + ldp x7, x8, [x0, #56]
> + ldp x9, x10, [x0, #72]
> + ldp x11, x12, [x0, #88]
> + ldp x13, x14, [x0, #104]
> + ldp x15, x16, [x0, #120]
> + ldp x17, x18, [x0, #136]
> + ldp x19, x20, [x0, #152]
> + ldp x21, x22, [x0, #168]
> + ldp x23, x24, [x0, #184]
> + ldp x25, x26, [x0, #200]
> + ldp x27, x28, [x0, #216]
> + ldp x29, x30, [x0, #232]
> + ldr x0, [x0, #0]
> +
> + isb
> + eret
> --
> 2.53.0.1213.gd9a14994de-goog
>
Thanks,
Joey
next prev parent reply other threads:[~2026-04-16 16:16 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-13 20:46 [kvm-unit-tests PATCH v2 0/7] arm64: Add Stage-2 MMU and Nested Guest Framework Jing Zhang
2026-04-13 20:46 ` [kvm-unit-tests PATCH v2 1/7] lib: arm64: Generalize ESR exception class definitions for EL2 support Jing Zhang
2026-04-16 15:27 ` Joey Gouly
2026-04-13 20:46 ` [kvm-unit-tests PATCH v2 2/7] lib: arm64: Add stage2 page table management library Jing Zhang
2026-04-16 15:19 ` Joey Gouly
2026-04-13 20:46 ` [kvm-unit-tests PATCH v2 3/7] lib: arm64: Generalize exception vector definitions for EL2 support Jing Zhang
2026-04-13 20:46 ` [kvm-unit-tests PATCH v2 4/7] lib: arm64: Add foundational guest execution framework Jing Zhang
2026-04-16 16:16 ` Joey Gouly [this message]
2026-04-13 20:46 ` [kvm-unit-tests PATCH v2 5/7] lib: arm64: Add support for guest exit exception handling Jing Zhang
2026-04-13 20:46 ` [kvm-unit-tests PATCH v2 6/7] lib: arm64: Add guest-internal exception handling (EL1) Jing Zhang
2026-04-13 20:46 ` [kvm-unit-tests PATCH v2 7/7] arm64: Add Stage-2 MMU demand paging test Jing Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260416161643.GC3142999@e124191.cambridge.arm.com \
--to=joey.gouly@arm.com \
--cc=alexandru.elisei@arm.com \
--cc=andrew.jones@linux.dev \
--cc=coltonlewis@google.com \
--cc=jingzhangos@google.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=maz@kernel.org \
--cc=mizhang@google.com \
--cc=oliver.upton@linux.dev \
--cc=rananta@google.com \
--cc=weilin.chang@arm.com \
--cc=yaoyuan@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox