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 8488D1F4CB1 for ; Wed, 2 Apr 2025 22:04:42 +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=1743631482; cv=none; b=R//j4NmHopBcYh7v40FT/Ae/Bt8dhDlcP+vGjFyMRADXwwd6btFRGdSJbHlTM6r7Zzx1P3LVP9NHrB4WHqVQZniARqpDMCarbsspm8owQ1V1tQGjnU6s4IO9ITraS7iD1JDsg7ZkqguyGi+daugN5orZC3VrFNOyDjSJdZrfUZs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1743631482; c=relaxed/simple; bh=kU7IBFQFjyFEhwSQwmwTIAIwV7Rkf7439jPSNOzP+BI=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=OXi5jSWOlB4fNMBvw4Y1xqBpZV80SzDuMijQevk6JGiHGF32YlwjhMYTJ36y7dOlzr7FofgNtZMDCPD/DVliabBjFvkk9DA8JTyDuDXy5MA/c0RYTJDMdUrrtE/lO775Bmzo2gp/SyYgugPObxLJT/BI98IEOXepmE9jCfjce94= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=OIsZY+wZ; 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="OIsZY+wZ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 033EAC4CEDD; Wed, 2 Apr 2025 22:04:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1743631482; bh=kU7IBFQFjyFEhwSQwmwTIAIwV7Rkf7439jPSNOzP+BI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=OIsZY+wZLwltxMq7Z6jFA17WE/pD9x0cnvdW8GHMbZdLRl9jtbA7uTWspfjZHAJTt ZwDQx9mnLjaOYzUvmqSErTAPugBz87i2eKzkjDecMhhqGZEp2RF5bUwvotEJIQTwm/ W4OFx6ecWwIWsB3vr+hr6Cs20/BkgKfjpeu+8qp7RhjNjvjL9FUQ2613adZ7/4ozgh ZG52rR2tdWlPclFO0LgEk0D2TiT5g35H1/xyoDcETz2nKL5lEsy6RGKoKdt+FrqkBB CdWR2NnAe5aeno+eQs7zoZiuvLsjCtDuQi7VAx0WJog6ngSJ3+nKhBSzbkwiuXZwqY uGU+9gWN/kwIw== 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 1u06CN-001nKm-Iy; Wed, 02 Apr 2025 23:04:39 +0100 Date: Wed, 02 Apr 2025 23:04:41 +0100 Message-ID: <87a58yz0cm.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: <20250402-arm-vdso-v1-1-2e7a12d75107@debian.org> References: <20250402-arm-vdso-v1-1-2e7a12d75107@debian.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 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. M. -- Jazz isn't dead. It just smells funny.