Re: [PATCH v20 14/14] KVM: selftests: arm64: Add GCS EXLOCK exception emulation test

[Leonardo Bras <leo.bras@arm.com>]

On Tue, Sep 01, 2026 at 10:47:12PM +0100, Mark Brown wrote:
> When running in vEL2 ERET is emulated, this means that we must emulate
> the behaviour of GCSCR_EL2.EXLOCKEN. Add a test that validates the
> behaviour.
> 
> As per DDI0487 R_WTXBY PSTATE.EXLOCK is set to the value of
> GCSCR_ELx.EXLOCKEN when taking an exception to the same exception level.
> As per DDI0487 R_TYTWB if on exception return to the same EL
> GCSCR_ELx.EXLOCKEN is 1 and PSTATE.EXLOCK is not 1 then the exception
> return is illegal.
> 
> We nest handling of two UDFs to allow us to both observe and modify
> PSTATE.EXLOCK, checking that expected exceptions are generated for ERET.
> We also repeat the UDF tests having enabled fine grained traps for ERET
> to try to ensure that the slow paths are taken.
> 
> We also attempt to write to both SPSR_ELx and ELR_ELx, again using a UDF
> handler to control PSTATE.EXLOCK. We do all this both with and without
> having enabled EXLOCKEN, though only for code running at EL2 due to
> framework limitations.
> 
> For completeness we also inject a data abort from EL0 to EL2 with
> GCSCR_EL2.EXLOCKEN set, ensuring that PSTATE.EXLOCK is not set.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  tools/testing/selftests/kvm/Makefile.kvm           |   1 +
>  tools/testing/selftests/kvm/arm64/gcs_exceptions.c | 346 +++++++++++++++++++++
>  2 files changed, 347 insertions(+)
> 

Hi Mark,

Maybe because I am new to this type of selftest (i.e. the helpers), or 
maybe because I don't understand some of the used mechanisms here, but this 
selftest felt hard to understand, even though the commit message made 
it sound very simple.

So I will try to cover what I could understand, so please let me know if I 
got something wrong.

From both tests, the lower_el() one is the simplest: if the features are 
there, you set TGE, EXLOCKEN and EXLOCK, and then do an KVM_SET_VCPU_EVENTS 
ioctl which injects a dabt and commit_pending_events(). It should change 
the PSTATE which you read and make sure EXLOCK=0.
(This should be about the last sentence in your commit msg)

For same_el(), 3 exception handlers are assigned to the vcpu: unknown, 
ilegal, and GCS. You then run the guest once, and exit. Then you check the 
exit reason.

In the guest code, you run a few tests for EXTLOCK={0,1} by basically 
counting and checking how many times each handler was called, and if the 
expected EXLOCK/EXLOCKEN states are correct. (Relying on R_WTXBY)

When you run test_udf_exception() it will cause an udf(COLLECT) that will 
itself cause an udf(VALIDATE), and if EXLOCKEN=1, it will cause an illegal 
exception, which will by itself cause an udf(REPAIR). On EXLOCKEN=0, 
setting EXLOCK=0 should not take an exception, and thus there should be no 
illegal++, an just 2 undef. None should cause a gcs exception.
That makes sense with the tests you do after the udf, as well as deciding 
to check IL based on EXLOCKEN status.

As for test_sysreg(), it will run for 4 tested registers with both EXLOCKEN 
states. What it does it take an udf(reg), which will read then write that 
read value to that register. If EXLOCKEN=1 it's expected to cause a gcs 
fault. That makes sense with the gcs and udf counters check in the bottom.

The part of re-running test_udf_exception() with HFGITR_EL2 set, IIUC, it 
that part of fine-grained traps. It seems correct, although I am not seeing 
any tests of the EC syndrome being 0x1A to make sure it's in use. 

There are also a lot of expect_exlock being used as an int, even if it's a 
bool. I understand the idea of summing it as we expect it to be 1 on true 
or 0 on false, but is it really ok?
Maybe we could change it to int, so we just set it when we enter 
test_sysreg() or test_udf_exception() like:

-	expect_exlock = enable_exlock;
+	expect_exlock = enable_exlock ? 1 : 0;

If I got the workings right, and both above suggestions (0x1A and 
bool->int) are either fixed or considered not an issue, then:

Reviewed-by: Leonardo Bras <leo.bras@arm.com>

Thanks!
Leo

> diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
> index 96bab7002d39..0286453ab223 100644
> --- a/tools/testing/selftests/kvm/Makefile.kvm
> +++ b/tools/testing/selftests/kvm/Makefile.kvm
> @@ -179,6 +179,7 @@ TEST_GEN_PROGS_arm64 += arm64/hello_el2
>  TEST_GEN_PROGS_arm64 += arm64/host_sve
>  TEST_GEN_PROGS_arm64 += arm64/hypercalls
>  TEST_GEN_PROGS_arm64 += arm64/external_aborts
> +TEST_GEN_PROGS_arm64 += arm64/gcs_exceptions
>  TEST_GEN_PROGS_arm64 += arm64/mmio_sign_ext
>  TEST_GEN_PROGS_arm64 += arm64/page_fault_test
>  TEST_GEN_PROGS_arm64 += arm64/psci_test
> diff --git a/tools/testing/selftests/kvm/arm64/gcs_exceptions.c b/tools/testing/selftests/kvm/arm64/gcs_exceptions.c
> new file mode 100644
> index 000000000000..de5bc6590727
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/arm64/gcs_exceptions.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Test KVM's handling of Guarded Control Stack exception state.
> + */
> +
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "test_util.h"
> +#include "ucall.h"
> +
> +#include <asm/ptrace.h>
> +#include <asm/sysreg.h>
> +
> +#define PSTATE_IL_BIT	BIT(20)
> +
> +static void require_guest_gcs(struct kvm_vcpu *vcpu)
> +{
> +	u64 pfr1 = vcpu_get_reg(vcpu,
> +				KVM_ARM64_SYS_REG(SYS_ID_AA64PFR1_EL1));
> +
> +	TEST_REQUIRE(SYS_FIELD_GET(ID_AA64PFR1_EL1, GCS, pfr1) >=
> +		     ID_AA64PFR1_EL1_GCS_IMP);
> +}
> +
> +static bool expect_exlock;
> +static unsigned int undef_count;
> +static unsigned int illegal_count;
> +static unsigned int gcs_count;
> +static u64 handler_pstate;
> +static u64 illegal_pstate;
> +
> +static u64 expected_illegal_pc;
> +static enum udf_mode {
> +	UDF_COLLECT,	/* Initial UDF we collect PSTATE from */
> +	UDF_VALIDATE,	/* Secondary UDF that collects and validates */
> +	UDF_REPAIR,	/* UDF to clear EXLOCK */
> +	UDF_ELR_EL1,	/* Write to ELR_EL1 with EXLOCK */
> +	UDF_ELR_EL2,	/* Write to ELR_EL2 with EXLOCK */
> +	UDF_SPSR_EL1,	/* Write to SPSR_EL1 with EXLOCK */
> +	UDF_SPSR_EL2,	/* Write to SPSR_EL2 with EXLOCK */
> +} udf_mode;
> +
> +static void guest_undef_handler(struct ex_regs *regs)
> +{
> +	u64 esr = read_sysreg(esr_el2);
> +	u64 val;
> +
> +	undef_count++;
> +
> +	/* Just skip the instruction either way. */
> +	GUEST_ASSERT_EQ(esr, ESR_ELx_IL);
> +	regs->pc += 4;
> +
> +	switch (udf_mode) {
> +	case UDF_COLLECT:
> +		GUEST_ASSERT_EQ(read_sysreg_s(SYS_GCSCR_EL2) & GCSCR_ELx_EXLOCKEN,
> +				expect_exlock ? GCSCR_ELx_EXLOCKEN : 0);
> +
> +		/*
> +		 * If we want the nested exception handler to change
> +		 * PSTATE it can't have EXLOCK set.
> +		 */
> +		write_sysreg_s(0, SYS_GCSCR_EL2);
> +		isb();
> +
> +		/*
> +		 * Take another exception without returning so we can
> +		 * collect the EXLOCK state we're seeing here and make
> +		 * our return illegal.
> +		 */
> +		expected_illegal_pc = regs->pc;
> +		udf_mode = UDF_VALIDATE;
> +		asm volatile("udf #0" ::: "memory");
> +
> +		/* Reenable so PSTATE.EXLOCK is checked */
> +		write_sysreg_s(expect_exlock ? GCSCR_ELx_EXLOCKEN : 0,
> +			       SYS_GCSCR_EL2);
> +		isb();
> +		break;
> +
> +	case UDF_VALIDATE:
> +		GUEST_ASSERT_EQ(read_sysreg_s(SYS_GCSCR_EL2) & GCSCR_ELx_EXLOCKEN,
> +				0);
> +
> +		/* Validate the EXLOCK of the original exception. */
> +		handler_pstate = regs->pstate;
> +		GUEST_ASSERT_EQ(!!(regs->pstate & PSR_EXLOCK_BIT),
> +				expect_exlock);
> +
> +		/* Clearing PSTATE.EXLOCK triggers an illegal ERET. */
> +		regs->pstate &= ~PSR_EXLOCK_BIT;
> +		break;
> +
> +	case UDF_REPAIR:
> +		GUEST_ASSERT_EQ(read_sysreg_s(SYS_GCSCR_EL2) & GCSCR_ELx_EXLOCKEN,
> +				0);
> +
> +		/* Disable EXLOCK for the requesting context */
> +		regs->pstate &= ~PSR_EXLOCK_BIT;
> +		break;
> +
> +	case UDF_ELR_EL1:
> +		GUEST_ASSERT_EQ(read_sysreg_s(SYS_GCSCR_EL2) & GCSCR_ELx_EXLOCKEN,
> +				expect_exlock ? GCSCR_ELx_EXLOCKEN : 0);
> +
> +		asm volatile("mrs %0, elr_el1   \n\
> +	                      msr elr_el1, %0"
> +			     : "=r"(val) :: "memory");
> +		break;
> +
> +	case UDF_ELR_EL2:
> +		GUEST_ASSERT_EQ(read_sysreg_s(SYS_GCSCR_EL2) & GCSCR_ELx_EXLOCKEN,
> +				expect_exlock ? GCSCR_ELx_EXLOCKEN : 0);
> +
> +		asm volatile("mrs %0, elr_el2   \n\
> +	                      msr elr_el2, %0"
> +			     : "=r"(val) :: "memory");
> +		break;
> +
> +	case UDF_SPSR_EL1:
> +		GUEST_ASSERT_EQ(read_sysreg_s(SYS_GCSCR_EL2) & GCSCR_ELx_EXLOCKEN,
> +				expect_exlock ? GCSCR_ELx_EXLOCKEN : 0);
> +
> +		asm volatile("mrs %0, spsr_el1   \n\
> +			      msr spsr_el1, %0"
> +			     : "=r"(val) :: "memory");
> +		break;
> +
> +	case UDF_SPSR_EL2:
> +		GUEST_ASSERT_EQ(read_sysreg_s(SYS_GCSCR_EL2) & GCSCR_ELx_EXLOCKEN,
> +				expect_exlock ? GCSCR_ELx_EXLOCKEN : 0);
> +
> +		asm volatile("mrs %0, spsr_el2   \n\
> +			      msr spsr_el2, %0"
> +			     : "=r"(val) :: "memory");
> +		break;
> +
> +	default:
> +		GUEST_FAIL("Invalid udf_mode");
> +	}
> +}
> +
> +static void guest_illegal_handler(struct ex_regs *regs)
> +{
> +	u64 esr = read_sysreg(esr_el2);
> +
> +	illegal_count++;
> +	illegal_pstate = regs->pstate;
> +
> +	GUEST_ASSERT_EQ(ESR_ELx_EC(esr), ESR_ELx_EC_ILL);
> +	GUEST_ASSERT(esr & ESR_ELx_IL);
> +	GUEST_ASSERT(regs->pstate & PSTATE_IL_BIT);
> +	GUEST_ASSERT(!(regs->pstate & PSR_EXLOCK_BIT));
> +	GUEST_ASSERT_EQ(regs->pc, expected_illegal_pc);
> +
> +	/* Disable exception locking so we can repair the return state. */
> +	write_sysreg_s(0, SYS_GCSCR_EL2);
> +	isb();
> +
> +	/* We need our own EXLOCK clearing so we can repair the original */
> +	udf_mode = UDF_REPAIR;
> +	asm volatile("udf #0" ::: "memory");
> +
> +	/* Disable EXLOCK for the generating context, making things legal */
> +	regs->pstate &= ~(PSTATE_IL_BIT | PSR_EXLOCK_BIT);
> +}
> +
> +static void guest_gcs_handler(struct ex_regs *regs)
> +{
> +	u64 esr = read_sysreg(esr_el2);
> +
> +	gcs_count++;
> +
> +	GUEST_ASSERT_EQ(ESR_ELx_EC(esr), ESR_ELx_EC_GCS);
> +	GUEST_ASSERT_EQ(FIELD_GET(ESR_ELx_ExType_MASK, esr),
> +			ESR_ELx_ExType_EXLOCK);
> +
> +	/* Disabling EXLOCKEN makes sysreg writes legal. */
> +	write_sysreg_s(0, SYS_GCSCR_EL2);
> +	isb();
> +}
> +
> +static noinline void test_udf_exception(bool enable_exlock)
> +{
> +	unsigned int initial_undef = undef_count;
> +	unsigned int initial_illegal = illegal_count;
> +	unsigned int initial_gcs = gcs_count;
> +
> +	expect_exlock = enable_exlock;
> +	write_sysreg_s(enable_exlock ? GCSCR_ELx_EXLOCKEN : 0,
> +		       SYS_GCSCR_EL2);
> +	isb();
> +
> +	udf_mode = UDF_COLLECT;
> +	asm volatile("udf #0" ::: "memory");
> +
> +	/* Leave GCS disabled even if a broken ERET skipped the ILL handler. */
> +	write_sysreg_s(0, SYS_GCSCR_EL2);
> +	isb();
> +
> +	GUEST_ASSERT_EQ(undef_count, initial_undef + 2 + enable_exlock);
> +	GUEST_ASSERT_EQ(illegal_count, initial_illegal + enable_exlock);
> +	GUEST_ASSERT_EQ(!!(handler_pstate & PSR_EXLOCK_BIT), enable_exlock);
> +	if (enable_exlock)
> +		GUEST_ASSERT(illegal_pstate & PSTATE_IL_BIT);
> +	GUEST_ASSERT_EQ(initial_gcs, gcs_count);
> +}
> +
> +static noinline void test_sysreg(enum udf_mode reg, bool enable_exlock)
> +{
> +	unsigned int initial_undef = undef_count;
> +	unsigned int initial_gcs = gcs_count;
> +
> +	expect_exlock = enable_exlock;
> +
> +	write_sysreg_s(enable_exlock ? GCSCR_ELx_EXLOCKEN : 0,
> +		       SYS_GCSCR_EL2);
> +	isb();
> +
> +	udf_mode = reg;
> +	asm volatile("udf #0" ::: "memory");
> +
> +	/* Ensure EXLOCK is disabled */
> +	write_sysreg_s(0, SYS_GCSCR_EL2);
> +	isb();
> +
> +	GUEST_ASSERT_EQ(gcs_count, initial_gcs + expect_exlock);
> +	GUEST_ASSERT_EQ(undef_count, initial_undef + 1);
> +}
> +
> +static void guest_code(void)
> +{
> +	GUEST_ASSERT_EQ(get_current_el(), 2);
> +
> +	/*
> +	 * Try straightforwardly generating an exception without and
> +	 * with EXLOCK.
> +	 */
> +	test_udf_exception(false);
> +	test_sysreg(UDF_ELR_EL1, false);
> +	test_sysreg(UDF_ELR_EL2, false);
> +	test_sysreg(UDF_SPSR_EL1, false);]
> +	test_sysreg(UDF_SPSR_EL2, false);
> +
> +	test_udf_exception(true);
> +	test_sysreg(UDF_ELR_EL1, true);
> +	test_sysreg(UDF_ELR_EL2, true);
> +	test_sysreg(UDF_SPSR_EL1, true);
> +	test_sysreg(UDF_SPSR_EL2, true);
> +
> +	/* Force ERET directly through the slow emulation path as well. */
> +	sysreg_clear_set_s(SYS_HFGITR_EL2, 0, HFGITR_EL2_ERET);
> +	isb();
> +
> +	test_udf_exception(false);
> +
> +	test_udf_exception(true);
> +
> +	sysreg_clear_set_s(SYS_HFGITR_EL2, HFGITR_EL2_ERET, 0);
> +	isb();
> +	GUEST_DONE();
> +}
> +
> +static void test_exception_from_same_el(void)
> +{
> +	struct kvm_vcpu_init init;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	struct ucall uc;
> +
> +	vm = vm_create(1);
> +	kvm_get_default_vcpu_target(vm, &init);
> +	init.features[0] |= BIT(KVM_ARM_VCPU_HAS_EL2);
> +	vcpu = aarch64_vcpu_add(vm, 0, &init, guest_code);
> +	require_guest_gcs(vcpu);
> +
> +	vm_init_descriptor_tables(vm);
> +	vcpu_init_descriptor_tables(vcpu);
> +	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
> +				ESR_ELx_EC_UNKNOWN, guest_undef_handler);
> +	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
> +				ESR_ELx_EC_ILL, guest_illegal_handler);
> +	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
> +				ESR_ELx_EC_GCS, guest_gcs_handler);
> +	kvm_arch_vm_finalize_vcpus(vm);
> +
> +	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);
> +}
> +
> +static void test_exception_from_lower_el(void)
> +{
> +	struct kvm_vcpu_events events = {};
> +	struct kvm_vcpu_init init;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	u64 pstate;
> +
> +	vm = vm_create(1);
> +	kvm_get_default_vcpu_target(vm, &init);
> +	init.features[0] |= BIT(KVM_ARM_VCPU_HAS_EL2);
> +	vcpu = aarch64_vcpu_add(vm, 0, &init, NULL);
> +	require_guest_gcs(vcpu);
> +	kvm_arch_vm_finalize_vcpus(vm);
> +
> +	/*
> +	 * Inject an EL0 data abort to vEL2 with EXLOCK enabled for
> +	 * vEL2.
> +	 */
> +	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_HCR_EL2),
> +		     vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_HCR_EL2)) |
> +		     HCR_EL2_TGE);
> +	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_GCSCR_EL2),
> +		     GCSCR_ELx_EXLOCKEN);
> +	vcpu_set_reg(vcpu, ARM64_CORE_REG(regs.pstate),
> +		     PSR_MODE_EL0t | PSR_EXLOCK_BIT);
> +
> +	events.exception.ext_dabt_pending = true;
> +	vcpu_events_set(vcpu, &events);
> +
> +	pstate = vcpu_get_reg(vcpu, ARM64_CORE_REG(regs.pstate));
> +	TEST_ASSERT_EQ(pstate & PSR_MODE_MASK, PSR_MODE_EL2h);
> +	TEST_ASSERT(!(pstate & PSR_EXLOCK_BIT),
> +		    "EXLOCK set on exception from a lower EL");
> +
> +	kvm_vm_free(vm);
> +}
> +
> +int main(void)
> +{
> +	TEST_REQUIRE(kvm_check_cap(KVM_CAP_ARM_EL2));
> +	test_exception_from_same_el();
> +	test_exception_from_lower_el();
> +
> +	return 0;
> +}
> 
> -- 
> 2.47.3
>