From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BC27D2114; Thu, 5 Jun 2025 06:46:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749106010; cv=none; b=SEo8l0u8ms9Rs5pcCd6uOZ5jDluC/2KKHzax7FLvxINLu8XLUh+KQpYjprVztsyIswe1TsMpspMjWA/QAOso/+kkmfFKd+zv7Z8SIbXUTplXmfvT9OSEx3DiW8Bk6n+oEYTFiTSSniwiREfQkjNsmrU7c1fRymEDFJow2US62pY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749106010; c=relaxed/simple; bh=TkwPaC5LkiD2AzRfVu53V8vYeTTkIIo3sfS9ma0nNMo=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=MT/bkBVu9RHiABTI9rqEV3d26L+GJ+cMfeK8Q/W2e61E1fPIw/Am9qfeGcwB2HO8Cs95jwsPFFx8g7Vz4XUznurHQiKNHO1G4BuhKrAXOdoFJiJFb96pHB3W+eIumWtZuUeYhlzbXE5450zimPhL2qT6TqhPrH2Z3akGujvw6QA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=NLwSt/rX; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="NLwSt/rX" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 26139C4CEE7; Thu, 5 Jun 2025 06:46:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1749106010; bh=TkwPaC5LkiD2AzRfVu53V8vYeTTkIIo3sfS9ma0nNMo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=NLwSt/rXojX3cThmkkKEtcWLjAXAPd5COS5p+Snz0xS/Qt8dFWs9aFgZTSynHEUYt 0WOgzX/u8wBdR1j6HqyW778WPuvy9f3QYkxnimgdhuDMOygd9mYMAWGifExKCLCd2C 6Dpws77QthBmsN2YZHenwEBVBlrMaVwWZcxCwoBQVttY5qYr16STOTp/MFQjmBft6o tGT/3MadWrMWAQ7QZcFsjcmMNtj6NDoE6bqtXzRZGcm5zqbZBnI7ZgSiqHahi3K7az odhwsr187j3x+jQY/oGDRNaQDwepertrh9W+B4O7Xuvtv5v+l4mT0CU/n9uuSEVSAP pGnZ4w/x9+jyw== Received: from [149.88.19.236] (helo=lobster-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1uN4ND-0049uH-Cb; Thu, 05 Jun 2025 07:46:47 +0100 Date: Thu, 05 Jun 2025 07:46:45 +0100 Message-ID: <875xhaeld6.wl-maz@kernel.org> From: Marc Zyngier To: Sebastian Ott Cc: Zenghui Yu , Oliver Upton , Colton Lewis , Ricardo Koller , Joey Gouly , Suzuki K Poulose , Shuah Khan , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH v2 0/3] KVM: arm64: selftests: arch_timer_edge_cases fixes In-Reply-To: <9b9f7099-4e81-9b74-a1ac-37cd4965675b@redhat.com> References: <20250527142434.25209-1-sebott@redhat.com> <9b9f7099-4e81-9b74-a1ac-37cd4965675b@redhat.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Precedence: bulk X-Mailing-List: linux-kselftest@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 149.88.19.236 X-SA-Exim-Rcpt-To: sebott@redhat.com, yuzenghui@huawei.com, oliver.upton@linux.dev, coltonlewis@google.com, ricarkol@google.com, joey.gouly@arm.com, suzuki.poulose@arm.com, shuah@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Wed, 04 Jun 2025 21:58:48 +0100, Sebastian Ott wrote: > > Hi Zenghui, > > On Tue, 3 Jun 2025, Zenghui Yu wrote: > > On 2025/5/27 22:24, Sebastian Ott wrote: > >> Some small fixes for arch_timer_edge_cases that I stumbled upon > >> while debugging failures for this selftest on ampere-one. > >> > >> Changes since v1: modified patch 3 based on suggestions from Marc. > >> > >> I've done some tests with this on various machines - seems to be all > >> good, however on ampere-one I now hit this in 10% of the runs: > >> ==== Test Assertion Failure ==== > >> arm64/arch_timer_edge_cases.c:481: timer_get_cntct(timer) >= DEF_CNT + (timer_get_cntfrq() * (uint64_t)(delta_2_ms) / 1000) > >> pid=166657 tid=166657 errno=4 - Interrupted system call > >> 1 0x0000000000404db3: test_run at arch_timer_edge_cases.c:933 > >> 2 0x0000000000401f9f: main at arch_timer_edge_cases.c:1062 > >> 3 0x0000ffffaedd625b: ?? ??:0 > >> 4 0x0000ffffaedd633b: ?? ??:0 > >> 5 0x00000000004020af: _start at ??:? > >> timer_get_cntct(timer) >= DEF_CNT + msec_to_cycles(delta_2_ms) > >> > >> This is not new, it was just hidden behind the other failure. I'll > >> try to figure out what this is about (seems to be independent of > >> the wait time).. > > > > Not sure if you have figured it out. I can easily reproduce it on my box > > and I *guess* it is that we have some random XVAL values when we enable > > the timer.. > > Yes, I think so, too. > > > test_reprogramming_timer() > > { > > local_irq_disable(); > > reset_timer_state(timer, DEF_CNT); > > My first attempt was to also initialize cval here Note that CVAL and TVAL are two views of the same thing. It should be enough to initialise one of them (though TVAL is stupidly narrow). > > > > > /* Program the timer to DEF_CNT + delta_1_ms. */ > > set_tval_irq(timer, msec_to_cycles(delta_1_ms), CTL_ENABLE); > > > > [...] > > } > > > > set_tval_irq() > > { > > timer_set_ctl(timer, ctl); > > > > // There is a window that we enable the timer with *random* XVAL > > // values and we may get the unexpected interrupt.. And it's > > // unlikely that KVM can be aware of TVAL's change (and > > // re-evaluate the interrupt's pending state) before hitting the > > // GUEST_ASSERT(). > > > > timer_set_tval(timer, tval_cycles); > > Yes, I stumbled over this as well. I've always assumed that this order is > becauase of this from the architecture "If CNTV_CTL_EL0.ENABLE is 0, > the value returned is UNKNOWN." However re-reading that part today I > realized > that this only concerns register reads. > > Maybe somone on cc knows why it's in that order? I can't think of a valid reason. Enabling the timer without having set the deadline first is just silly. > I'm currently testing this with the above swapped and it's looking good, > so far. The swapped version (set xVAL, then enable the timer) is the only one that makes any sense. Thanks, M. -- Jazz isn't dead. It just smells funny.