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 C20931EFFA0 for ; Wed, 2 Apr 2025 22:22:52 +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=1743632573; cv=none; b=Gaq8dPONqOJ5Tcj9JWdfgmQI/KJQKRVgStGNMtjIwEWYX0E8aS8qczR1jV6oMqf3RNbsI+zPJYOeqIJe920h2j0LNmjJHKENnJ7XDIgDqRvd1XvIVga6SNpw8vG8Q9EcQhTZARAvgb4VrwG3Bk7njF79inthhpJfaoLDAftGqCw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1743632573; c=relaxed/simple; bh=wp1wdBawXEYaoVvIG0zTvsPZwslUtgd90Up9ylLoJKE=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=f2fLj8VZUG2qXMjNLLssZcJmzD9rjnnrzvqbPW5ur6wPK/KgRwOkUbqTY8DwiSDNhC5oXeogjtDpzRTEBi5OEaGSm89IyI0TH/J8xED6K9FxhpmpCmcrQ2xp86kmoEX+6D0M+phqp2Nn4WuPfVF1zvB347b+vIsc1z6CjB+Mj6s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=MxSktIkD; 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="MxSktIkD" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2C482C4CEDD; Wed, 2 Apr 2025 22:22:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1743632572; bh=wp1wdBawXEYaoVvIG0zTvsPZwslUtgd90Up9ylLoJKE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=MxSktIkDHPtbNWJiFubM0d15o/9scY5bN5feGpcHkpsXS6YUuBS4GHhWSne4tb6Dq pA78BMJrMKcKwSuCuouwDzXeJIpw8rSyBG41B/mYWjdRNeodQgQmr6BBO5id4rhWcS AFi0RYgzmVVvqOpQ4s0WCKtP3KfLxOWc7g5PxIS2ol+5qzMiycNp5FeF6bFS/4frzN tGbNlxxzmvjwMjcaEJ35CsKwQS6WQoHsgL/aaAKSjoOPZjrhw/rH4mGjgu6xKcUaDx chs9kxh41RUXglUBHDwa1OOJz9wqf9CI/R1/gq8F0rG+ubG0UakxHuRZ/+ppGbc1u6 8zxPHxR2Kf/GA== Received: from ip-185-104-136-29.ptr.icomera.net ([185.104.136.29] 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 1u06Tx-001nWm-M6; Wed, 02 Apr 2025 23:22:49 +0100 Date: Wed, 02 Apr 2025 23:22:51 +0100 Message-ID: <878qoiyzic.wl-maz@kernel.org> From: Marc Zyngier To: Breno Leitao Cc: Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, arnd@arndb.de, kernel-team@meta.com, vincenzo.frascino@arm.com, anders.roxell@linaro.org Subject: Re: [PATCH RFC] arm64: vdso: Use __arch_counter_get_cntvct() In-Reply-To: <87a58yz0cm.wl-maz@kernel.org> References: <20250402-arm-vdso-v1-1-2e7a12d75107@debian.org> <87a58yz0cm.wl-maz@kernel.org> 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-kernel@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: 185.104.136.29 X-SA-Exim-Rcpt-To: leitao@debian.org, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, arnd@arndb.de, kernel-team@meta.com, vincenzo.frascino@arm.com, anders.roxell@linaro.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, 02 Apr 2025 23:04:41 +0100, Marc Zyngier wrote: > > On Wed, 02 Apr 2025 20:22:47 +0100, > Breno Leitao wrote: > > > > While reading how `cntvct_el0` was read in the kernel, I found that > > __arch_get_hw_counter() is doing something very similar to what > > __arch_counter_get_cntvct() is already doing. > > > > Use the existing __arch_counter_get_cntvct() function instead of > > duplicating similar inline assembly code in __arch_get_hw_counter(). > > > > Both functions were performing nearly identical operations to read the > > cntvct_el0 register. The only difference was that > > __arch_get_hw_counter() included a memory clobber in its inline > > assembly, which appears unnecessary in this context. > > > > This change simplifies the code by eliminating duplicate functionality > > and improves maintainability by centralizing the counter access logic in > > a single implementation. > > > > Signed-off-by: Breno Leitao > > --- > > I'm sharing this code as an RFC since I'm not intimately familiar with > > different arm platforms, and I want to double-check that I haven't > > missed anything subtle. > > --- > > arch/arm64/include/asm/vdso/gettimeofday.h | 22 ++-------------------- > > 1 file changed, 2 insertions(+), 20 deletions(-) > > > > diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h > > index 92a2b59a9f3df..417b5b41b877d 100644 > > --- a/arch/arm64/include/asm/vdso/gettimeofday.h > > +++ b/arch/arm64/include/asm/vdso/gettimeofday.h > > @@ -11,6 +11,7 @@ > > #include > > #include > > #include > > +#include > > > > #define VDSO_HAS_CLOCK_GETRES 1 > > > > @@ -69,8 +70,6 @@ int clock_getres_fallback(clockid_t _clkid, struct __kernel_timespec *_ts) > > static __always_inline u64 __arch_get_hw_counter(s32 clock_mode, > > const struct vdso_time_data *vd) > > { > > - u64 res; > > - > > /* > > * Core checks for mode already, so this raced against a concurrent > > * update. Return something. Core will do another round and then > > @@ -79,24 +78,7 @@ static __always_inline u64 __arch_get_hw_counter(s32 clock_mode, > > if (clock_mode == VDSO_CLOCKMODE_NONE) > > return 0; > > > > - /* > > - * If FEAT_ECV is available, use the self-synchronizing counter. > > - * Otherwise the isb is required to prevent that the counter value > > - * is speculated. > > - */ > > - asm volatile( > > - ALTERNATIVE("isb\n" > > - "mrs %0, cntvct_el0", > > - "nop\n" > > - __mrs_s("%0", SYS_CNTVCTSS_EL0), > > - ARM64_HAS_ECV) > > - : "=r" (res) > > - : > > - : "memory"); > > - > > - arch_counter_enforce_ordering(res); > > - > > - return res; > > + return __arch_counter_get_cntvct(); > > I won't pretend I understand it all, but you really want to have a > look at the link just above the arch_counter_enforce_ordering() > definition, pasted below for your convenience: > > https://lore.kernel.org/r/alpine.DEB.2.21.1902081950260.1662@nanos.tec.linutronix.de/ > > Dropping this ordering enforcement seems pretty adventurous unless you > have very strong guarantees about the context this executes in. Ah, I appear to have misread this patch, and __arch_counter_get_cntvct() does have the same ordering requirements. Apologies for the noise. M. -- Jazz isn't dead. It just smells funny.