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=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT 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 1DD92C48BE9 for ; Mon, 24 Jun 2019 13:36:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 013BA212F5 for ; Mon, 24 Jun 2019 13:36:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731180AbfFXNgN (ORCPT ); Mon, 24 Jun 2019 09:36:13 -0400 Received: from foss.arm.com ([217.140.110.172]:50588 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725562AbfFXNgN (ORCPT ); Mon, 24 Jun 2019 09:36:13 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1D0E0344; Mon, 24 Jun 2019 06:36:12 -0700 (PDT) Received: from fuggles.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 859783F71E; Mon, 24 Jun 2019 06:36:09 -0700 (PDT) Date: Mon, 24 Jun 2019 14:36:07 +0100 From: Will Deacon To: Vincenzo Frascino Cc: linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org, linux-kselftest@vger.kernel.org, Catalin Marinas , Arnd Bergmann , Russell King , Ralf Baechle , Paul Burton , Daniel Lezcano , Thomas Gleixner , Mark Salyzyn , Peter Collingbourne , Shuah Khan , Dmitry Safonov <0x7f454c46@gmail.com>, Rasmus Villemoes , Huw Davies , Shijith Thotton , Andre Przywara Subject: Re: [PATCH v7 04/25] arm64: Substitute gettimeofday with C implementation Message-ID: <20190624133607.GI29497@fuggles.cambridge.arm.com> References: <20190621095252.32307-1-vincenzo.frascino@arm.com> <20190621095252.32307-5-vincenzo.frascino@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190621095252.32307-5-vincenzo.frascino@arm.com> User-Agent: Mutt/1.11.1+86 (6f28e57d73f2) () Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Vincenzo, On Fri, Jun 21, 2019 at 10:52:31AM +0100, Vincenzo Frascino wrote: > To take advantage of the commonly defined vdso interface for > gettimeofday the architectural code requires an adaptation. > > Re-implement the gettimeofday vdso in C in order to use lib/vdso. > > With the new implementation arm64 gains support for CLOCK_BOOTTIME > and CLOCK_TAI. > > Cc: Catalin Marinas > Cc: Will Deacon > Signed-off-by: Vincenzo Frascino > Tested-by: Shijith Thotton > Tested-by: Andre Przywara > --- > arch/arm64/Kconfig | 2 + > arch/arm64/include/asm/vdso/gettimeofday.h | 86 ++++++ > arch/arm64/include/asm/vdso/vsyscall.h | 53 ++++ > arch/arm64/include/asm/vdso_datapage.h | 48 --- > arch/arm64/kernel/asm-offsets.c | 33 +- > arch/arm64/kernel/vdso.c | 51 +--- > arch/arm64/kernel/vdso/Makefile | 34 ++- > arch/arm64/kernel/vdso/gettimeofday.S | 334 --------------------- > arch/arm64/kernel/vdso/vgettimeofday.c | 28 ++ I'm concerned about an apparent semantic change introduced by your patch: > +static __always_inline u64 __arch_get_hw_counter(s32 clock_mode) > +{ > + u64 res; > + > + asm volatile("mrs %0, cntvct_el0" : "=r" (res) :: "memory"); > + > + return res; > +} vs: > - .macro get_clock_shifted_nsec res, cycle_last, mult > - /* Read the virtual counter. */ > - isb > - mrs x_tmp, cntvct_el0 > - /* Calculate cycle delta and convert to ns. */ > - sub \res, x_tmp, \cycle_last > - /* We can only guarantee 56 bits of precision. */ > - movn x_tmp, #0xff00, lsl #48 > - and \res, x_tmp, \res > - mul \res, \res, \mult > - /* > - * Fake address dependency from the value computed from the counter > - * register to subsequent data page accesses so that the sequence > - * locking also orders the read of the counter. > - */ > - and x_tmp, \res, xzr > - add vdso_data, vdso_data, x_tmp > - .endm It looks like you're dropping both the preceding ISB (allowing the counter value to be speculated) and also the subsequent dependency (allowing the seq lock to be speculated). If I've missed them, apologies, but I couldn't spot them elsewhere in this patch. __arch_get_hw_counter should probably be identical to __arch_counter_get_cntvct to avoid these problems. I guess we don't need to care about the case where the counter is unstable, since we'll just disable the vDSO altogether on such systems? Will 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=-5.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT 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 78608C43613 for ; Mon, 24 Jun 2019 13:36:20 +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 4CE4A2133F for ; Mon, 24 Jun 2019 13:36:20 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="HgRaE5jA" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4CE4A2133F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=mvV/PgiYAuS6vWyF7TF4nzp023+0ZKUUSqREoIMUAZ0=; b=HgRaE5jAraACDH SlPYSbIrnMpObfFz7kWDvpLqwEx90yIM8hViAKsnZcT5eIdN2cQLmBMrSbhKgEf6+4/LVZcY+/HKS L42iVZFZW+XrlMg+Ogk9k4PQFvQ9dHq29Fr6GG53FE4CZvfRylHesOJuER9lPOkTLoBObFRyYGyHV 2FiDapYpUblZvdRS/NyBdDgGD1TV4jbwDlPaj0AgB23UlH3OL0Kmn6F+cp3zVoe4qT8c0WKsElLLV rHRGBPk+AeSWE9qJ/a2KsHfrw7d7OKMf2gt0l6cMMU7moK3WXUReV4koPbuAgfRTLS5m3S85G7FUy AxIp/0iZw7wilZD6SPqg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hfP8t-0004Yq-J1; Mon, 24 Jun 2019 13:36:19 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hfP8q-0004Xr-EM for linux-arm-kernel@lists.infradead.org; Mon, 24 Jun 2019 13:36:17 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1D0E0344; Mon, 24 Jun 2019 06:36:12 -0700 (PDT) Received: from fuggles.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 859783F71E; Mon, 24 Jun 2019 06:36:09 -0700 (PDT) Date: Mon, 24 Jun 2019 14:36:07 +0100 From: Will Deacon To: Vincenzo Frascino Subject: Re: [PATCH v7 04/25] arm64: Substitute gettimeofday with C implementation Message-ID: <20190624133607.GI29497@fuggles.cambridge.arm.com> References: <20190621095252.32307-1-vincenzo.frascino@arm.com> <20190621095252.32307-5-vincenzo.frascino@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190621095252.32307-5-vincenzo.frascino@arm.com> User-Agent: Mutt/1.11.1+86 (6f28e57d73f2) () X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190624_063616_575774_A1B306E9 X-CRM114-Status: GOOD ( 15.75 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arch@vger.kernel.org, Shuah Khan , Shijith Thotton , Andre Przywara , Arnd Bergmann , Huw Davies , Catalin Marinas , Daniel Lezcano , Dmitry Safonov <0x7f454c46@gmail.com>, linux-kernel@vger.kernel.org, Ralf Baechle , linux-mips@vger.kernel.org, Paul Burton , linux-kselftest@vger.kernel.org, Rasmus Villemoes , Russell King , Thomas Gleixner , Mark Salyzyn , Peter Collingbourne , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Vincenzo, On Fri, Jun 21, 2019 at 10:52:31AM +0100, Vincenzo Frascino wrote: > To take advantage of the commonly defined vdso interface for > gettimeofday the architectural code requires an adaptation. > > Re-implement the gettimeofday vdso in C in order to use lib/vdso. > > With the new implementation arm64 gains support for CLOCK_BOOTTIME > and CLOCK_TAI. > > Cc: Catalin Marinas > Cc: Will Deacon > Signed-off-by: Vincenzo Frascino > Tested-by: Shijith Thotton > Tested-by: Andre Przywara > --- > arch/arm64/Kconfig | 2 + > arch/arm64/include/asm/vdso/gettimeofday.h | 86 ++++++ > arch/arm64/include/asm/vdso/vsyscall.h | 53 ++++ > arch/arm64/include/asm/vdso_datapage.h | 48 --- > arch/arm64/kernel/asm-offsets.c | 33 +- > arch/arm64/kernel/vdso.c | 51 +--- > arch/arm64/kernel/vdso/Makefile | 34 ++- > arch/arm64/kernel/vdso/gettimeofday.S | 334 --------------------- > arch/arm64/kernel/vdso/vgettimeofday.c | 28 ++ I'm concerned about an apparent semantic change introduced by your patch: > +static __always_inline u64 __arch_get_hw_counter(s32 clock_mode) > +{ > + u64 res; > + > + asm volatile("mrs %0, cntvct_el0" : "=r" (res) :: "memory"); > + > + return res; > +} vs: > - .macro get_clock_shifted_nsec res, cycle_last, mult > - /* Read the virtual counter. */ > - isb > - mrs x_tmp, cntvct_el0 > - /* Calculate cycle delta and convert to ns. */ > - sub \res, x_tmp, \cycle_last > - /* We can only guarantee 56 bits of precision. */ > - movn x_tmp, #0xff00, lsl #48 > - and \res, x_tmp, \res > - mul \res, \res, \mult > - /* > - * Fake address dependency from the value computed from the counter > - * register to subsequent data page accesses so that the sequence > - * locking also orders the read of the counter. > - */ > - and x_tmp, \res, xzr > - add vdso_data, vdso_data, x_tmp > - .endm It looks like you're dropping both the preceding ISB (allowing the counter value to be speculated) and also the subsequent dependency (allowing the seq lock to be speculated). If I've missed them, apologies, but I couldn't spot them elsewhere in this patch. __arch_get_hw_counter should probably be identical to __arch_counter_get_cntvct to avoid these problems. I guess we don't need to care about the case where the counter is unstable, since we'll just disable the vDSO altogether on such systems? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel