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 X-Spam-Level: X-Spam-Status: No, score=-14.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7BFA5C4338F for ; Tue, 10 Aug 2021 08:49:46 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 3B05761019 for ; Tue, 10 Aug 2021 08:49:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 3B05761019 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=gBRk98JNnrIDqyu7mRG7ITNKD6xolqSsiEaYUsCxonU=; b=gEDvzk8Cb8eAxN ZvmHDJnK4FRVqkDc1WZ4C6+tX4SSay6ThUZHOI6jNVL3FeTbfx5vokwe4r472FOKyEAKer+AN3FRU dK3nDZ/MKkCoN8MHXD7VxDs/SRtRZb5LuFbel5bh/OzmBC35+kyQabT7KKYFb7+Hf7XxR+hErUUAd nf8SK8XmuVQ4ha63j0oBKduLHVUvUEpR2TnjxAcb+taKDR6ZnblulvDfHfoVwTAg1SlMNy+RBky0A 24M+MieBQlGTNj5c8WTqVfmp1IL4bUh0IWuE+v1/kV1EzSRYt/FTi5FBGCVE40iERhDeUU2DHGTXn pXW8iEyH9YtdjrYMuL7Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mDNPO-0037rE-6t; Tue, 10 Aug 2021 08:46:51 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mDNJP-0035VY-Qz for linux-arm-kernel@lists.infradead.org; Tue, 10 Aug 2021 08:40:41 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 6A03C6056C; Tue, 10 Aug 2021 08:40:39 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mDNJN-0040aK-E2; Tue, 10 Aug 2021 09:40:37 +0100 Date: Tue, 10 Aug 2021 09:40:37 +0100 Message-ID: <87eeb1bsl6.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Mark Rutland , Daniel Lezcano , Thomas Gleixner , Peter Shier , Raghavendra Rao Ananta , Ricardo Koller , Will Deacon , Catalin Marinas , Linus Walleij , kernel-team@android.com Subject: Re: [PATCH 11/13] clocksource/arm_arch_timer: Fix masking for high freq counters In-Reply-To: References: <20210809152651.2297337-1-maz@kernel.org> <20210809152651.2297337-12-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/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: oupton@google.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, mark.rutland@arm.com, daniel.lezcano@linaro.org, tglx@linutronix.de, pshier@google.com, rananta@google.com, ricarkol@google.com, will@kernel.org, catalin.marinas@arm.com, linus.walleij@linaro.org, kernel-team@android.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210810_014039_961051_724DCDD1 X-CRM114-Status: GOOD ( 44.47 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 09 Aug 2021 17:45:28 +0100, Oliver Upton wrote: > > On Mon, Aug 9, 2021 at 8:48 AM Marc Zyngier wrote: > > > > From: Oliver Upton > > > > Unfortunately, the architecture provides no means to determine the bit > > width of the system counter. However, we do know the following from the > > specification: > > > > - the system counter is at least 56 bits wide > > - Roll-over time of not less than 40 years > > > > To date, the arch timer driver has depended on the first property, > > assuming any system counter to be 56 bits wide and masking off the rest. > > However, combining a narrow clocksource mask with a high frequency > > counter could result in prematurely wrapping the system counter by a > > significant margin. For example, a 56 bit wide, 1GHz system counter > > would wrap in a mere 2.28 years! > > > > This is a problem for two reasons: v8.6+ implementations are required to > > provide a 64 bit, 1GHz system counter. Furthermore, before v8.6, > > implementers may select a counter frequency of their choosing. > > > > Fix the issue by deriving a valid clock mask based on the second > > property from above. Set the floor at 56 bits, since we know no system > > counter is narrower than that. > > > > Suggested-by: Marc Zyngier > > Signed-off-by: Oliver Upton > > Reviewed-by: Linus Walleij > > [maz: fixed width computation not to lose the last bit, added > > max delta generation for the timer] > > Signed-off-by: Marc Zyngier > > Link: https://lore.kernel.org/r/20210807191428.3488948-1-oupton@google.com > > --- > > drivers/clocksource/arm_arch_timer.c | 34 ++++++++++++++++++++++++---- > > 1 file changed, 29 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c > > index fa09952b94bf..74eca831d0d9 100644 > > --- a/drivers/clocksource/arm_arch_timer.c > > +++ b/drivers/clocksource/arm_arch_timer.c > > @@ -52,6 +52,12 @@ > > #define CNTV_CVAL_LO 0x30 > > #define CNTV_CTL 0x3c > > > > +/* > > + * The minimum amount of time a generic counter is guaranteed to not roll over > > + * (40 years) > > + */ > > +#define MIN_ROLLOVER_SECS (40ULL * 365 * 24 * 3600) > > + > > static unsigned arch_timers_present __initdata; > > > > struct arch_timer { > > @@ -95,6 +101,22 @@ static int __init early_evtstrm_cfg(char *buf) > > } > > early_param("clocksource.arm_arch_timer.evtstrm", early_evtstrm_cfg); > > > > +/* > > + * Makes an educated guess at a valid counter width based on the Generic Timer > > + * specification. Of note: > > + * 1) the system counter is at least 56 bits wide > > + * 2) a roll-over time of not less than 40 years > > + * > > + * See 'ARM DDI 0487G.a D11.1.2 ("The system counter")' for more details. > > + */ > > +static int arch_counter_get_width(void) > > +{ > > + u64 min_cycles = MIN_ROLLOVER_SECS * arch_timer_rate; > > + > > + /* guarantee the returned width is within the valid range */ > > + return clamp_val(ilog2(min_cycles - 1) + 1, 56, 64); > > +} > > Reposting thoughts from the original patch: > > Reading the ARM ARM > D11.1.2 'The system counter', I did not find any language that > suggested the counter saturates the register width before rolling > over. So, it may be paranoid, but I presumed it to be safer to wrap > within the guaranteed interval rather (40 years) than assume the > sanity of the system counter implementation. I really don't think that would be a likely implementation. The fact that the ARM ARM only talks about the width of the counter makes it a strong case that there is no 'ceiling' other than the natural saturation of the counter, IMO. If a rollover was allowed to occur before, it would definitely be mentioned. Think about it: you'd need to implement an extra comparator to drive the reset of the counter. It would also make the implementation of CVAL stupidly complicated: how do you handle the set of values that fit in the counter width, but are out of the counter range? Even though the architecture is not the clearest thing, I'm expecting the CPU designers to try and save gates, rather than trying to implement a GOTCHA, expensive counter... ;-) Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel