From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CBA73FD8FD3 for ; Thu, 26 Feb 2026 16:48:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=3OtTVwfI5EdoTQbhG5XRtDZZgaRr1Iijt9vnK29bd14=; b=z5Zj/JNuWZkKuOTm9Lr7X1jAgT vYEW13Fvf+ljgFkq1NIcvsZbij+mWtaFiEFAv3dniVIxab0XqkzhHbd3fSETBPWc+Qwy2a6RI3jSp F4Ri5ejrnja6z1sBTL5bUkBifHh+TFByt2cQJuNBvJp1IxU4s/CpCYQM/MEPvoRMd3YnIPEZ4KAD6 T/5jqphSxW/wtuBz7EZuKc01928dzKrR+tm75HcoDclThReVHDBTRBaoN/F3o8pEMmMBNbxal0gij 9Ska5+A8h73WrTu32hDeD1y0wgTRZ+ignDywB7e/xCx7D7jpwah8ljqT1o50NeiHfp/OZtCIirr42 XdLnEdng==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vveX5-00000006llk-02Ac; Thu, 26 Feb 2026 16:48:11 +0000 Received: from tor.source.kernel.org ([2600:3c04:e001:324:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vveX2-00000006llO-3kgV for linux-arm-kernel@lists.infradead.org; Thu, 26 Feb 2026 16:48:09 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id F03BC60122; Thu, 26 Feb 2026 16:48:07 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A00F1C116C6; Thu, 26 Feb 2026 16:48:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1772124487; bh=6IgteOujw4QUNLN0dbDKU2Si6JJRwngrRv4WhbmtUw4=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=G04kmmUKEg7YMH7x7NLWpHD2Jy09Ob2y5LjCvy6cuzHVxPEJ9KoHotJUGSIefeZoL /2QK2CnrvBIPuyf7MOq6SxEdQ+y24Px66I0KF2SECbT3pCcNEuGL1dqkti426dAp3q KrTNzkD84ZbgsNQTtwI5aWbBWrsBXPeCvLTdBT2GAeK/B0DaSsVvH8OML2p9KRGA4k du5Fb9Q5It1ZZsuzvKLVQuSUD6xAHB2xSZ/OKTsOYFbIizQVlfKktu6bvxRw1GyRBz bjTf1+UG/rb2AMVzLIUbUm4Mvas5kW0w+VFMSYn3vz7zjIXvZgYaIqMvTC+fFgkYI3 ekVY1VfzEbGfQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1vveWz-0000000E8b8-1lOc; Thu, 26 Feb 2026 16:48:05 +0000 Date: Thu, 26 Feb 2026 16:48:05 +0000 Message-ID: <864in3a1tm.wl-maz@kernel.org> From: Marc Zyngier To: Ben Horgan Cc: linux-arm-kernel@lists.infradead.org, Will Deacon , Catalin Marinas , Mark Rutland , Daniel Lezcano Subject: Re: [PATCH 2/3] clocksource/drivers/arm_arch_timer: Expose a direct accessor for the virtual counter In-Reply-To: <4ab7b471-a67f-454e-aaf4-9e0b1f0d96e8@arm.com> References: <20260226082234.26707-1-maz@kernel.org> <20260226082234.26707-3-maz@kernel.org> <4ab7b471-a67f-454e-aaf4-9e0b1f0d96e8@arm.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) 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.219.108.64 X-SA-Exim-Rcpt-To: ben.horgan@arm.com, linux-arm-kernel@lists.infradead.org, will@kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com, daniel.lezcano@linaro.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, 26 Feb 2026 14:03:36 +0000, Ben Horgan wrote: > > > > On 2/26/26 13:48, Ben Horgan wrote: > > Hi Marc, > > > > On 2/26/26 08:22, Marc Zyngier wrote: > >> We allow access to the architected counter via arch_timer_read_counter(). > >> However, this accessor can either be the virtual or the physical > >> view of the counter, depending on how the kernel has been booted. > >> > >> At the same time, we have some architectural features (such as WFIT, > >> WFET) that rely on the virtual counter, and nothing else. > >> > >> If implementations were perfect, we'd rely on reading CNTVCT_EL0, > >> and be done with it. However, we have a bunch of broken implementations > >> in the wild, which rely on preemption being disabled and other > >> costly workarounds. > >> > >> In order to provide decent performance on non-broken HW while still > >> supporting the legacy horrors, expose arch_timer_read_vcounter() as > >> a new helper that hides this complexity. > >> > >> Signed-off-by: Marc Zyngier > >> --- > >> drivers/clocksource/arm_arch_timer.c | 5 +++++ > >> include/clocksource/arm_arch_timer.h | 1 + > >> 2 files changed, 6 insertions(+) > >> > >> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c > >> index 90aeff44a2764..4e4a62e1c9439 100644 > >> --- a/drivers/clocksource/arm_arch_timer.c > >> +++ b/drivers/clocksource/arm_arch_timer.c > >> @@ -137,6 +137,8 @@ static noinstr u64 arch_counter_get_cntvct(void) > >> u64 (*arch_timer_read_counter)(void) __ro_after_init = arch_counter_get_cntvct; > >> EXPORT_SYMBOL_GPL(arch_timer_read_counter); > >> > >> +u64 (*arch_timer_read_vcounter)(void) __ro_after_init = arch_counter_get_cntvct; > >> + > >> static u64 arch_counter_read(struct clocksource *cs) > >> { > >> return arch_timer_read_counter(); > >> @@ -931,6 +933,9 @@ static void __init arch_counter_register(void) > >> } > >> > >> arch_timer_read_counter = rd; > >> + arch_timer_read_vcounter = (arch_timer_counter_has_wa() ? > > > > This matches what is done for arch_timer_read_counter but it seems a bit > > surprising to me that arch_timer_counter_has_wa() is checking that the > > workaround is in use and not whether the workaround should be in use. Do > > we need to worry about what happens if the workaround fails to be enabled? > > Or is the point that if you haven't enabled a relevant workaround then > all cores are treated the same and so there is no need to disable > preemption? There are multiple things at play here: - we cannot fail to enable a workaround. If we find one, we enable it. - if no workaround are available, then there is no need to disable preemption, because the read of the counter is the same on all CPUs. However, this code is a bug nest, and I just re-discovered an interesting failure mode (boot on a sane CPU, keeping the broken CPUs offline, online a broken CPU late, enjoy the fireworks). Plus the fact that we don't indirect sched_clock(), which means we never really enable a workaround if the boot CPU is not affected. I have a small pile of hacks to address all of this, but I need to convince myself that this is actually correct. Stay tuned... M. -- Without deviation from the norm, progress is not possible.