public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Alexandru Elisei <alexandru.elisei@arm.com>
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, 01 May 2026 15:21:19 +0100	[thread overview]
Message-ID: <87zf2jcisg.fsf@draig.linaro.org> (raw)
In-Reply-To: <afSnVJLe7HrB6ISS@raptor> (Alexandru Elisei's message of "Fri, 1 May 2026 14:15:14 +0100")

Alexandru Elisei <alexandru.elisei@arm.com> writes:

> 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.

So I'm using kvm-unit-test to exercise QEMU's TCG modelling of the
instructions, see:

  Message-ID: <20260430104434.1482407-1-alex.bennee@linaro.org>
  Date: Thu, 30 Apr 2026 11:44:27 +0100
  Subject: [PATCH v4 0/7] target/arm: fully model WFxT instructions for A-profile

So while I'm confident the current modelling doesn't cause issues
(because we basically treat it as a NOP) I wanted to check the various
modes behave as they should with the above patches.

I can limit the test to TCG only if it is likely to fail under KVM.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

      reply	other threads:[~2026-05-01 14:21 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
2026-05-01 14:21   ` Alex Bennée [this message]

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=87zf2jcisg.fsf@draig.linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=alexandru.elisei@arm.com \
    --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