Re: [PATCH 8/8] KVM: selftests: Add nested pre-fault test for arm64

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Fri, Sep 11, 2026 at 01:42:57PM +0100, Wei-Lin Chang wrote:
> On Fri, Sep 11, 2026 at 10:32:17AM +0100, Lorenzo Stoakes (ARM) wrote:
> > +cc Wei-Lin
> >
> > On Fri, Sep 11, 2026 at 03:30:41PM +0900, Itaru Kitayama wrote:
> > > Hi Lorenzo,
> > >
> > > On Tue, Aug 25, 2026 at 05:00:42PM +0100, Lorenzo Stoakes (ARM) wrote:
> > > > From: Jack Thomson <jackabt@amazon.com>
> > > >
> > > > Add an arm64 nested-virt selftest for KVM_PRE_FAULT_MEMORY. The guest
> > > > enters vEL1 and exits to userspace with a nested/shadow stage-2 MMU as
> > > > the vCPU's last-run context.
> > > >
> > > > Before prefaulting, userspace enables HCR_EL2.VM and points VTTBR_EL2 at
> > > > an empty nested stage-2 root. A prefault implementation that incorrectly
> > > > treats the userspace GPA as an L2 IPA will fail the ioctl; the correct
> > > > path targets the canonical stage-2 and succeeds.
> > > >
> > > > Restore the original nested state before resuming the guest, then touch
> > > > the prefaulted range to check that vEL1 still runs correctly.
> > > >
> > > > Signed-off-by: Jack Thomson <jackabt@amazon.com>
> > > > [ljs: partial progress, >4 KiB pgsize, commit msg, comment fixups]
> > > > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > > > ---
> > > >  tools/testing/selftests/kvm/Makefile.kvm           |   1 +
> > > >  .../selftests/kvm/arm64/nv_pre_fault_memory_test.c | 206 +++++++++++++++++++++
> > > >  2 files changed, 207 insertions(+)
> > >
> > > I wonder whether this selftest can use functions Wei-Lin proposed
> > > some time ago [1]: 20260325003620.2214766-1-weilin.chang@arm.com
> > > or would you prefer this test as propose, since it is clear as
> > > to what needs to be done to enter L2.
> >
> > I think if these aren't merged yet then that could be a follow up?
>
> This selftest doesn't require setting up L1's stage-2 for L2, so to me
> the current approach is okay.



>
> [...]
>
> > > > +static void guest_code(void)
> > > > +{
> > > > +	u64 sp;
> > > > +
> > > > +	GUEST_ASSERT_EQ(get_current_el(), 2);
> > > > +
> > > > +	/*
> > > > +	 * Mirror the EL2 translation regime into the real EL1 registers so
> > > > +	 * that vEL1 runs on the test's stage-1 page tables. With E2H=1, the
> > > > +	 * _EL1 accessors read the EL2 registers, and the _EL12 accessors
> > > > +	 * write the real EL1 registers.
> > > > +	 */
> > > > +	write_sysreg_s(read_sysreg(sctlr_el1), SYS_SCTLR_EL12);
> > > > +	write_sysreg_s(read_sysreg(tcr_el1), SYS_TCR_EL12);
> > > > +	write_sysreg_s(read_sysreg(ttbr0_el1), SYS_TTBR0_EL12);
> > > > +	write_sysreg_s(read_sysreg(mair_el1), SYS_MAIR_EL12);
> > > > +	write_sysreg_s(read_sysreg(cpacr_el1), SYS_CPACR_EL12);
>
> Just FYI, the at.c selftest uses a macro for these.

Ack will check and see if I can use same approach here.

>
> > > > +
> > > > +	/* Run vEL1 on the same stack. */
> > > > +	asm volatile("mov %0, sp" : "=r"(sp));
> > > > +	write_sysreg(sp, sp_el1);
> > > > +
> > > > +	/*
> > > > +	 * Drop TGE so that vEL1 is a nested context rather than host EL0.
> > > > +	 * KVM backs it with a shadow stage-2 MMU even though vstage-2 is
> > > > +	 * disabled (HCR_EL2.VM=0).
> > > > +	 */
> > > > +	write_sysreg(read_sysreg(hcr_el2) & ~HCR_EL2_TGE, hcr_el2);
> > > > +	isb();
> > > > +
> > > > +	write_sysreg(PSR_MODE_EL1h | PSR_F_BIT | PSR_I_BIT | PSR_A_BIT |
> > > > +		     PSR_D_BIT, spsr_el2);
> > > > +	write_sysreg((u64)guest_el1_code, elr_el2);
> > > > +	asm volatile("eret");
> > > > +
> > > > +	GUEST_ASSERT(false);
> > > > +}
> > > > +
>
> [...]
>
> > > > +
> > > > +static struct nested_s2_state enable_empty_nested_s2(struct kvm_vcpu *vcpu)
> > > > +{
> > > > +	struct nested_s2_state state = {
> > > > +		.hcr_el2 = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_HCR_EL2)),
> > > > +		.vttbr_el2 = vcpu_get_reg(vcpu,
> > > > +					   KVM_ARM64_SYS_REG(SYS_VTTBR_EL2)),
> > > > +	};
> > > > +
> > > > +	TEST_ASSERT(!(state.hcr_el2 & HCR_EL2_TGE),
> > > > +		    "vCPU should be in nested/vEL1 context");
> > > > +
> > > > +	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_VTTBR_EL2),
> > > > +		     NESTED_S2_ROOT_GPA);
> > > > +	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_HCR_EL2),
> > > > +		     state.hcr_el2 | HCR_EL2_VM);
> > > > +
> > > > +	return state;
> > > > +}
> > > > +
> > > > +static void restore_nested_s2(struct kvm_vcpu *vcpu,
> > > > +			      struct nested_s2_state *state)
> > > > +{
> > > > +	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_HCR_EL2), state->hcr_el2);
> > > > +	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_VTTBR_EL2),
> > > > +		     state->vttbr_el2);
> > > > +}
> > > > +
> > > > +int main(void)
> > > > +{
> > > > +	struct nested_s2_state s2;
> > > > +	struct kvm_vcpu_init init;
> > > > +	struct kvm_vcpu *vcpu;
> > > > +	struct kvm_vm *vm;
> > > > +	struct ucall uc;
> > > > +	u64 npages;
> > > > +
>
> [...]
>
> > > > +
> > > > +	/*
> > > > +	 * The vCPU's last-run context is vEL1, backed by a shadow stage-2
> > > > +	 * MMU. Enable nested stage-2 with an empty root so that the ioctl
> > > > +	 * fails if it tries to interpret the userspace GPA as an L2 IPA.
> > > > +	 *
> > > > +	 * Prefault in two halves so that the second ioctl exercises a
> > > > +	 * repeated shadow-MMU attach and canonical stage-2 swap.
> > > > +	 *
> > > > +	 * (Note that an implementation that wrongly populates shadow
> > > > +	 * stage-2 page tables would not be caught as userland can't
> > > > +	 * inspect these.)
> > > > +	 */
> > > > +	s2 = enable_empty_nested_s2(vcpu);
> > > > +	pre_fault(vcpu, TEST_MEM_GPA, TEST_MEM_SIZE / 2);
> > > > +	pre_fault(vcpu, TEST_MEM_GPA + TEST_MEM_SIZE / 2, TEST_MEM_SIZE / 2);
> > > > +	restore_nested_s2(vcpu, &s2);
>
> Sorry I don't get what enable_empty_nested_s2(), restore_nested_s2()
> brings to the test?
>
> After quickly going through the prefault implementation there isn't
> anything about it that is nested related, how would giving L2 an empty
> stage-2 translation affect the outcome if it misinterprets the GPA as an
> L2 IPA?

Well the prefaulting intentionally does not interact with nested and only
prefaults for the canonical s2 page tables.

I guess the intention here was to make sure that's the case.

But I think you're right in that it's a bit silly - the code doesn't try to
interpret anything in terms of a nested guest, and nobody's going to change it
to do so.

>
> In other words would there be potential false positives if we don't call
> these functions?

I think really think this is actually asserting anything meaningful.

I'll drop it and simplify - have it just assert that the last-run vCPU was a
stage2 vCPU and prefault doesn't do anything weird with that.

The only way it could be broken is if it set a non-canonical mmu in
__pre_fault_s2() or set s2fd->nested, neither of which, of course, it does.

>
> Thanks,
> Wei-Lin Chang
>
> > > > +
> > > > +	/* Resume at vEL1 and touch the prefaulted range. */
> > > > +	vcpu_run(vcpu);
> > > > +	switch (get_ucall(vcpu, &uc)) {
> > > > +	case UCALL_DONE:
> > > > +		break;
> > > > +	case UCALL_ABORT:
> > > > +		REPORT_GUEST_ASSERT(uc);
> > > > +		break;
> > > > +	default:
> > > > +		TEST_FAIL("Unhandled ucall: %ld", uc.cmd);
> > > > +	}
> > > > +
> > > > +	kvm_vm_free(vm);
> > > > +	return 0;
> > > > +}
> > > >
> > > > --
> > > > 2.55.0
> > > >
> >
> > --
> > Cheers, Lorenzo

--
Cheers, Lorenzo