Re: [kvm-unit-tests PATCH v2 4/7] lib: arm64: Add foundational guest execution framework

[Joey Gouly <joey.gouly@arm.com>]

On Thu, Apr 16, 2026 at 05:16:43PM +0100, Joey Gouly wrote:
> 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.

Ok this *is* needed, but this is SYS_SCTLR_EL12, not SYS_SCTLR_EL1. Same for
VBAR_EL1 in the next patch.

Thanks,
Joey

> 
> > +
> >  #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