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 D0162C4338F for ; Mon, 9 Aug 2021 11:09:43 +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 72C0E60F8F for ; Mon, 9 Aug 2021 11:09:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 72C0E60F8F 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=KtQsS1Mwb0g/eyxL7cils8ehSk+sZwQkNfcUS8uyCzk=; b=WZGluge3/wEV20 UNa0o2ZluhstVI+PvS+EoBb97et4aR9KEktiswVb7S4wgt3BwH97hrc+8DpnzWO8gGQ7/d8ZHutPt XoD9CRhk2kXDKZwkuj89TmaXMkUMDIUequ8afSFgOz6JnWez67Zw0XkdX8gorc143Laonj03wgdjR B1TG2hrc32ZdVdP96a6htggAESKGx3sWJN7awbpkFcscEo6ss9CVkEnN2VXSn67IhAdE8xGB/oMbC sVw7oBjt6G9O5F8eeBXmM6YyM4HAjxldJOXCZDTJWmI/kkJ1UlMcSId4R84e5C9XOsw64wdqZA+5/ Q0PhyFHJjBYvlzhZywUA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mD38Q-000IT4-Jg; Mon, 09 Aug 2021 11:07:58 +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 1mD38M-000IS4-Va for linux-arm-kernel@lists.infradead.org; Mon, 09 Aug 2021 11:07:56 +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 9FA9360EE9; Mon, 9 Aug 2021 11:07:54 +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 1mD38K-003nhQ-LH; Mon, 09 Aug 2021 12:07:52 +0100 Date: Mon, 09 Aug 2021 12:07:52 +0100 Message-ID: <87lf5ac1vb.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 Subject: Re: [PATCH v2] clocksource/arm_arch_timer: Fix masking for high freq counters In-Reply-To: <20210807191428.3488948-1-oupton@google.com> References: <20210807191428.3488948-1-oupton@google.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/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 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-20210809_040755_097473_36150573 X-CRM114-Status: GOOD ( 38.53 ) 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 Sat, 07 Aug 2021 20:14:28 +0100, Oliver Upton wrote: > > 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 > --- > This patch was tested on QEMU, tweaked to provide a 1GHz system counter > frequency. The 'bp.refcounter.base_frequency' property does not seem to > have any affect on the 'ARMvA Base RevC AEM FVP', and instead provides a > 100MHz counter. > > Parent commit: 0c32706dac1b ("arm64: stacktrace: avoid tracing arch_stack_walk()") > > drivers/clocksource/arm_arch_timer.c | 32 ++++++++++++++++++++++++---- > 1 file changed, 28 insertions(+), 4 deletions(-) > > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c > index be6d741d404c..f4816b22213c 100644 > --- a/drivers/clocksource/arm_arch_timer.c > +++ b/drivers/clocksource/arm_arch_timer.c > @@ -52,6 +52,12 @@ > #define CNTV_TVAL 0x38 > #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; > > static void __iomem *arch_counter_base __ro_after_init; > @@ -205,13 +211,11 @@ static struct clocksource clocksource_counter = { > .id = CSID_ARM_ARCH_COUNTER, > .rating = 400, > .read = arch_counter_read, > - .mask = CLOCKSOURCE_MASK(56), > .flags = CLOCK_SOURCE_IS_CONTINUOUS, > }; > > static struct cyclecounter cyclecounter __ro_after_init = { > .read = arch_counter_read_cc, > - .mask = CLOCKSOURCE_MASK(56), > }; > > struct ate_acpi_oem_info { > @@ -1004,9 +1008,26 @@ struct arch_timer_kvm_info *arch_timer_get_kvm_info(void) > return &arch_timer_kvm_info; > } > > +/* > + * 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 __init 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), 56, 64); See my comment somewhere else in the thread about the potential wasted bit. > +} > + > static void __init arch_counter_register(unsigned type) > { > u64 start_count; > + int width; > > /* Register the CP15 based counter if we have one */ > if (type & ARCH_TIMER_TYPE_CP15) { > @@ -1031,6 +1052,10 @@ static void __init arch_counter_register(unsigned type) > arch_timer_read_counter = arch_counter_get_cntvct_mem; > } > > + width = arch_counter_get_width(); > + clocksource_counter.mask = CLOCKSOURCE_MASK(width); > + cyclecounter.mask = CLOCKSOURCE_MASK(width); > + > if (!arch_counter_suspend_stop) > clocksource_counter.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP; > start_count = arch_timer_read_counter(); > @@ -1040,8 +1065,7 @@ static void __init arch_counter_register(unsigned type) > timecounter_init(&arch_timer_kvm_info.timecounter, > &cyclecounter, start_count); > > - /* 56 bits minimum, so we assume worst case rollover */ > - sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate); > + sched_clock_register(arch_timer_read_counter, width, arch_timer_rate); For the record, there is one spot where the clockevent gets registered and configured that also needs addressing (there is a mask harcoded to 31 bits there, which is pretty odd). It cannot be fixed directly in this patch though. I'll probably take this patch on top of my series and adjust the relevant bits. 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