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=-15.5 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,USER_AGENT_SANE_1 autolearn=ham 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 CCFACC433C1 for ; Tue, 30 Mar 2021 11:10:39 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 3E12F614A7 for ; Tue, 30 Mar 2021 11:10:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3E12F614A7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc: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=BnrBS+WUxaWw6BA1q+wEAVP7CQZTpCkGl0gcFfpUWl4=; b=UWktQMYoajJTQ47SzM0tMGNZ0 pWXo14FjGph1DTmcJyBL5puX2AScJAKnlq5jsjveC4P9YLAdZEHGLy681X+y4F/tdlR2FkJyG+MmF 7RGy7vJsk8H9WbctU64rF5yozoJiGM5J4z/eLHBcL0pxk7WhB0aZSYMyC5Gd16PJEnme/OPJJENff ultyHWns3RExwNR1T9TAzY40sui2R4B/Z3bXU6ZrEvaLZT+hdto+pCIKZ17uJSaLsBw/DQmg7a5LC j/qoqiPIgVN/jedzAZ7/EktiJW7WWWzRIlrO76z25rmYU7WJrRDulrUg70UjaKYsvzj0FRGiFWOE5 10I2MSm1A==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lRCEh-003WOc-3L; Tue, 30 Mar 2021 11:08:39 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lRCEb-003WNY-Mo for linux-arm-kernel@lists.infradead.org; Tue, 30 Mar 2021 11:08:36 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id B685E61987; Tue, 30 Mar 2021 11:08:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1617102512; bh=MyBh8Fl6Gd3n+0tcc+2pQ9PQONvP53mmMZ1pVMQdgrs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=EJn3iqzch6T75zfM0EzAKi4ep3AiKWJ94obYJzu2i8G99CZRVmwlItw5vnF6t4O8n gh7M4XkOOcEatXdbQCk6gv/Zrd18Q4r0Dy8SvD9N6EyFGXu6s0y4vF02HddW/PmfjB CB4E2oeOLgMmxq+fm45x0TCiVwyhDDJu0FpTxZ9me1hMilwVdFIhrz8WFggUEZDY/Q VVI3mCEp7CeCRCz6seJxbKpdB/8dnhSYrwXBzqMuY56Yk3a77d5JSaWlvulURU1RZM j44duCZ0DvlZswHD7OPJDr8hepyVIL+wikY2alsWGmKcjechuW4EDV2eAFRHTJDvjs ZwvW26B2F+eQQ== Date: Tue, 30 Mar 2021 12:08:27 +0100 From: Will Deacon To: Pingfan Liu Cc: linux-arm-kernel@lists.infradead.org, Catalin Marinas , Vincenzo Frascino , Thomas Gleixner , Mark Rutland , Andrei Vagin , Marc Zyngier Subject: Re: [PATCH 1/2] arm64/gettimeofday: correct the note about isb in __arch_get_hw_counter() Message-ID: <20210330110826.GB5707@willie-the-truck> References: <20210330105719.47760-1-kernelfans@gmail.com> <20210330105719.47760-2-kernelfans@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210330105719.47760-2-kernelfans@gmail.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210330_120834_768455_8F0BDB27 X-CRM114-Status: GOOD ( 23.43 ) 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 Tue, Mar 30, 2021 at 06:57:18PM +0800, Pingfan Liu wrote: > The seq lock in vdso_read_retry() is behind smp_rmb(), and can not be > speculated before the counter value due to the read dependency. Hence > the original note is misleading. > > The description of getting counter value is not very clear. [1] > 'mrs Xt, cntpct' may execute out of program order, either forward or > backward. > > Hence this isb is still required to protect against backward speculation. > Correct the note around the code to show the motivation. > > [1]: AArch64 Programmer's Guides Generic Timer: 3.1. Count and frequency > Signed-off-by: Pingfan Liu > Cc: Catalin Marinas > Cc: Will Deacon > Cc: Vincenzo Frascino > Cc: Thomas Gleixner > Cc: Mark Rutland > Cc: Andrei Vagin > Cc: Marc Zyngier > To: linux-arm-kernel@lists.infradead.org > --- > arch/arm64/include/asm/vdso/compat_gettimeofday.h | 9 ++++++--- > arch/arm64/include/asm/vdso/gettimeofday.h | 9 ++++++--- > 2 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h > index 7508b0ac1d21..b5dfda25a5dc 100644 > --- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h > +++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h > @@ -123,10 +123,13 @@ static __always_inline u64 __arch_get_hw_counter(s32 clock_mode, > isb(); > asm volatile("mrrc p15, 1, %Q0, %R0, c14" : "=r" (res)); > /* > - * This isb() is required to prevent that the seq lock is > - * speculated. > + * Getting count value may execute out of program order, either forward > + * or backward. Although the caller has a read dependency on @res, but > + * it can not protect backward speculation against no dependency > + * instruction. Beside this purpose, this isb also severs as a > + * compiler barrier for this __always_inline function. I agree that the existing comment is pretty rubbish, but I don't think this is really much better. > diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h > index 631ab1281633..6988a730b878 100644 > --- a/arch/arm64/include/asm/vdso/gettimeofday.h > +++ b/arch/arm64/include/asm/vdso/gettimeofday.h > @@ -84,10 +84,13 @@ static __always_inline u64 __arch_get_hw_counter(s32 clock_mode, > isb(); > asm volatile("mrs %0, cntvct_el0" : "=r" (res) :: "memory"); > /* > - * This isb() is required to prevent that the seq lock is > - * speculated.# > + * Getting count value may execute out of program order, either forward > + * or backward. Although the caller has a read dependency on @res, but > + * it can not protect backward speculation against no dependency > + * instruction. Beside this purpose, this isb also severs as a > + * compiler barrier for this __always_inline function. > */ > - isb(); > + isb(); This ISB doesn't exist in linux-next (I've changed it to use the dependency trick, which you seem to have doubts about). Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel