public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Alexandru Elisei <alexandru.elisei@arm.com>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: qemu-devel@nongnu.org, Andrew Jones <andrew.jones@linux.dev>,
	Eric Auger <eric.auger@redhat.com>,
	"open list:ARM" <kvmarm@lists.linux.dev>,
	"open list:Default mailing list" <kvm@vger.kernel.org>
Subject: Re: [kvm-unit-tests PATCH] arm: add wfx test case
Date: Fri, 1 May 2026 14:15:14 +0100	[thread overview]
Message-ID: <afSnVJLe7HrB6ISS@raptor> (raw)
In-Reply-To: <20260427130045.3669851-1-alex.bennee@linaro.org>

Hi Alex,

On Mon, Apr 27, 2026 at 02:00:45PM +0100, Alex Bennée wrote:
> This is based on a similar test case I wrote for QEMU's tcg tests although
> obviously able to take advantage of kvm-unit-tests additional plumbing for
> dealing with the GIC and IRQs.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  arm/Makefile.arm64        |   1 +
>  lib/arm64/asm/processor.h |   7 ++
>  lib/arm64/asm/sysreg.h    |   3 +
>  arm/wfx.c                 | 137 ++++++++++++++++++++++++++++++++++++++
>  arm/unittests.cfg         |   5 ++
>  5 files changed, 153 insertions(+)
>  create mode 100644 arm/wfx.c
> 
> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> index a40c830d..52b3f35d 100644
> --- a/arm/Makefile.arm64
> +++ b/arm/Makefile.arm64
> @@ -64,6 +64,7 @@ tests += $(TEST_DIR)/cache.$(exe)
>  tests += $(TEST_DIR)/debug.$(exe)
>  tests += $(TEST_DIR)/fpu.$(exe)
>  tests += $(TEST_DIR)/mte.$(exe)
> +tests += $(TEST_DIR)/wfx.$(exe)
>  
>  include $(SRCDIR)/$(TEST_DIR)/Makefile.common
>  
> diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
> index 32ddc1b3..2104036d 100644
> --- a/lib/arm64/asm/processor.h
> +++ b/lib/arm64/asm/processor.h
> @@ -173,5 +173,12 @@ static inline bool system_supports_rndr(void)
>  	return ((id_aa64isar0_el1 >> ID_AA64ISAR0_EL1_RNDR_SHIFT) & 0xf) != 0;
>  }
>  
> +static inline bool system_supports_wfxt(void)
> +{
> +	u64 id_aa64isar2_el1 = read_sysreg_s(ID_AA64ISAR2_EL1);
> +
> +	return ((id_aa64isar2_el1 >> ID_AA64ISAR2_EL1_WFxT_SHIFT) & 0xf) != 0;
> +}
> +
>  #endif /* !__ASSEMBLER__ */
>  #endif /* _ASMARM64_PROCESSOR_H_ */
> diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h
> index f2d05018..cb96a649 100644
> --- a/lib/arm64/asm/sysreg.h
> +++ b/lib/arm64/asm/sysreg.h
> @@ -77,6 +77,9 @@ asm(
>  #define ID_AA64ISAR0_EL1_RNDR_SHIFT	60
>  #define ID_AA64PFR1_EL1_MTE_SHIFT	8
>  
> +#define ID_AA64ISAR2_EL1		sys_reg(3, 0, 0, 6, 2)
> +#define ID_AA64ISAR2_EL1_WFxT_SHIFT	0
> +
>  #define ID_AA64MMFR0_EL1_FGT_SHIFT	56
>  #define ID_AA64MMFR0_EL1_FGT_FGT2	0x2
>  
> diff --git a/arm/wfx.c b/arm/wfx.c
> new file mode 100644
> index 00000000..912e50e6
> --- /dev/null
> +++ b/arm/wfx.c
> @@ -0,0 +1,137 @@
> +/*
> + * WFX Instructions Test (WFI, WFE, WFIT, WFET)
> + *
> + * Copyright (c) 2026 Linaro Ltd
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include <libcflat.h>
> +#include <asm/processor.h>
> +#include <asm/gic.h>
> +#include <asm/timer.h>
> +#include <asm/io.h>
> +
> +#define TIMEOUT 200000
> +
> +#define sev() asm volatile("sev" : : : "memory")
> +#define sevl() asm volatile("sevl" : : : "memory")
> +#define wfi() asm volatile("wfi" : : : "memory")
> +#define wfe() asm volatile("wfe" : : : "memory")
> +
> +#define wfit(reg) \
> +	asm volatile(".arch armv8.7-a\n\twfit %0" : : "r" (reg) : "memory")
> +#define wfet(reg) \
> +	asm volatile(".arch armv8.7-a\n\twfet %0" : : "r" (reg) : "memory")
> +
> +static void timer_handler(struct pt_regs *regs)
> +{
> +	/* Disable timer to stop IRQ from re-firing */
> +	write_sysreg(0, cntv_ctl_el0);
> +}

It is customary for the interrupt handler to ack the interrupt at the GIC level.

> +
> +static bool check_elapsed(uint64_t start, uint64_t threshold, const char *test, bool more)
> +{
> +	uint64_t end = read_sysreg(cntvct_el0);
> +	uint64_t elapsed = end - start;
> +	bool pass = more ? elapsed >= threshold : elapsed <= threshold;
> +
> +	report(pass, "%s (%ld ticks)", test, elapsed);
> +
> +	if (!pass) {
> +		report_info("%s %s", test, more ? "woke too early" : "slept despite SEV");
> +	}
> +	return pass;
> +}
> +
> +static void test_wfi(void)
> +{
> +	uint64_t start;
> +
> +	report_info("Testing WFI...");
> +
> +	start = read_sysreg(cntvct_el0);
> +	write_sysreg(TIMEOUT, cntv_tval_el0);
> +	write_sysreg(1, cntv_ctl_el0); /* Enable timer, no mask */
> +	isb();
> +
> +	local_irq_enable();
> +	wfi();

Consider this scenario:

	local_irq_enable();
	// CPU takes the interrupt, handles it and returns.
	wfi(); <- CPU now waits forever for an interrupt that will never come

The proper way to do it on baremetal would be:

	local_irq_disable();
	// Program timer to fire
	wfi();
	// Timer fires, GIC asserts the interrupt, WFI completes
	local_irq_enable();
	// CPU handles the interrupt
	check_elapsed(..)

But this is not baremetal. KVM can decide not to trap WFI (look at
arch/arm64/kvm/arm.c::kvm_arch_vcpu_load() -> kvm_vpcu_should_clear_twi()). In
that case, the WFI might complete due a host interrupt, and check_elapsed() will
fail because the timer hasn't yet fired.

> +	local_irq_disable();
> +
> +	check_elapsed(start, TIMEOUT, "WFI", true);
> +}
> +
> +static void test_wfe(void)
> +{
> +	uint64_t start;
> +
> +	report_info("Testing WFE/SEV...");
> +	sev();
> +	start = read_sysreg(cntvct_el0);
> +	wfe();
> +	check_elapsed(start, TIMEOUT, "WFE/SEV", false);
> +
> +	report_info("Testing WFE/SEVL...");
> +	sevl();
> +	start = read_sysreg(cntvct_el0);
> +	wfe();
> +	check_elapsed(start, TIMEOUT, "WFE/SEVL", false);
> +}

I haven't thought about this too much, but it looks to me like the same
situation with WFI can happen with WFE

Also, kvm-unit-tests() makes use of WFE and SEV for multithreading, and the fact
that multithreaded tests work at all might be taken as proof enough that the two
instructions are correctly implemented.

Thanks,
Alex

> +
> +static void test_wfit(void)
> +{
> +	uint64_t start, timeout;
> +
> +	report_info("Testing WFIT...");
> +	start = read_sysreg(cntvct_el0);
> +	timeout = start + TIMEOUT;
> +	wfit(timeout);
> +	check_elapsed(start, TIMEOUT, "WFIT", true);
> +}
> +
> +static void test_wfet(void)
> +{
> +	uint64_t start, timeout;
> +
> +	report_info("Testing WFET...");
> +	/* Ensure no pending events */
> +	sev();
> +	wfe();
> +
> +	start = read_sysreg(cntvct_el0);
> +	timeout = start + TIMEOUT;
> +	wfet(timeout);
> +	check_elapsed(start, TIMEOUT, "WFET", true);
> +}
> +
> +int main(void)
> +{
> +
> +	if (gic_init() < 0) {
> +		report_abort("GIC init failed");
> +		return 1;
> +	}
> +
> +	/* Install timer handler for WFI wake-up */
> +	install_irq_handler(EL1H_IRQ, timer_handler);
> +	gic_enable_defaults();
> +
> +	/* Enable Virtual Timer PPI */
> +	gic_irq_set_clr_enable(27, true);
> +
> +	report_prefix_push("WFx");
> +	test_wfi();
> +	test_wfe();
> +	report_prefix_pop();
> +
> +	if (system_supports_wfxt()) {
> +		report_prefix_push("WFxT");
> +		test_wfit();
> +		test_wfet();
> +	} else {
> +		report_skip("WFxT instructions not supported");
> +	}
> +
> +	return report_summary();
> +}
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index 12fc4468..7adf96f2 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -339,3 +339,8 @@ groups = mte
>  test_args = asymm
>  qemu_params = -machine mte=on
>  arch = arm64
> +
> +[wfx]
> +file = wfx.flat
> +groups = wfx
> +arch = arm64
> -- 
> 2.47.3
> 

  parent reply	other threads:[~2026-05-01 13:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-27 13:00 [kvm-unit-tests PATCH] arm: add wfx test case Alex Bennée
2026-04-28 13:26 ` Joey Gouly
2026-04-28 14:56   ` Alex Bennée
2026-05-01 13:15 ` Alexandru Elisei [this message]
2026-05-01 14:21   ` Alex Bennée

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=afSnVJLe7HrB6ISS@raptor \
    --to=alexandru.elisei@arm.com \
    --cc=alex.bennee@linaro.org \
    --cc=andrew.jones@linux.dev \
    --cc=eric.auger@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=qemu-devel@nongnu.org \
    /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