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=-8.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,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 5402DC48BD6 for ; Thu, 27 Jun 2019 11:41:50 +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 27B98204FD for ; Thu, 27 Jun 2019 11:41:50 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="llyuzd1Q" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 27B98204FD 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=lnFzeqxFNwmqMYTjmhZuvvs3l437CNg7vi0oDNj+TQo=; b=llyuzd1Qvtn/HQ sCmM64Qd7aR8Ms+efYi/2a9ibwAPHWuzcJ67jpw2ZAA7s7Iv1KbOJEWUiLflS1h3rYnIGQTMjV+Tj tZD1kJDwZLj3lEYzv5/xLxqY+eaWffDghQ0hPEtETK8xm+LUoClODTBj75gB+QPD3/zMyJvrHVX99 +8ToeebkUpzqee9HJqeVD+uh1x+QRxb33825PzpC3NNngCWYHGACtEk2wH3nRd2X6BDraucghYAfK kWAfteSTTg9unqEFLREZ27Y4/gyNqjP1s65+bEpkKddyimlKfEHqS1bQ/ujUJQ0tvzcEb9ZuTIMHM rYtVfJv9vM83ikR1nqqw==; 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 1hgSmd-0001ms-JM; Thu, 27 Jun 2019 11:41:43 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hgSma-0001mK-Id for linux-arm-kernel@lists.infradead.org; Thu, 27 Jun 2019 11:41:41 +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 E9AB62B; Thu, 27 Jun 2019 04:41:39 -0700 (PDT) Received: from localhost (unknown [10.37.6.20]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6B3C63F718; Thu, 27 Jun 2019 04:41:39 -0700 (PDT) Date: Thu, 27 Jun 2019 12:41:37 +0100 From: Andrew Murray To: Suzuki K Poulose Subject: Re: [PATCH v2 2/5] coresight: etm4x: use explicit barriers on enable/disable Message-ID: <20190627114137.GD34530@e119886-lin.cambridge.arm.com> References: <20190627083525.37463-1-andrew.murray@arm.com> <20190627083525.37463-3-andrew.murray@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1+81 (426a6c1) (2018-08-26) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190627_044140_710847_26005544 X-CRM114-Status: GOOD ( 23.85 ) 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: mathieu.poirier@linaro.org, alexander.shishkin@linux.intel.com, coresight@lists.linaro.org, stable@vger.kernel.org, Sudeep.Holla@arm.com, linux-arm-kernel@lists.infradead.org, mike.leach@linaro.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 On Thu, Jun 27, 2019 at 10:16:17AM +0100, Suzuki K Poulose wrote: > Hi Andrew, > > On 27/06/2019 09:35, Andrew Murray wrote: > > Synchronization is recommended before disabling the trace registers > > to prevent any start or stop points being speculative at the point > > of disabling the unit (section 7.3.77 of ARM IHI 0064D). > > > > Synchronization is also recommended after programming the trace > > registers to ensure all updates are committed prior to normal code > > resuming (section 4.3.7 of ARM IHI 0064D). > > > > Let's ensure these syncronization points are present in the code > > and clearly commented. > > Please could you also mention why we switched from mb() ? No problem. > > > > > Note that we could rely on the barriers in CS_LOCK and > > coresight_disclaim_device_unlocked or the context switch to user > > space - however coresight may be of use in the kernel. > > > > Signed-off-by: Andrew Murray > > CC: stable@vger.kernel.org > > > > > --- > > drivers/hwtracing/coresight/coresight-etm4x.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c > > index c89190d464ab..68e8e3954cef 100644 > > --- a/drivers/hwtracing/coresight/coresight-etm4x.c > > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c > > @@ -188,6 +188,10 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > > dev_err(etm_dev, > > "timeout while waiting for Idle Trace Status\n"); > > + /* As recommended by 4.3.7 of ARM IHI 0064D */ > > nit: It would be good to mention the "section name" to help the reader > find the same on a different version of the document. Also within the same > version, this is listed in the subsection: > "Synchronization when using the memory-mapped interface" > > Please could you update the comment to reflect the same ? > Yes sure. > > + dsb(sy); > > + isb(); > > + > > done: > > CS_LOCK(drvdata->base); > > @@ -454,7 +458,8 @@ static void etm4_disable_hw(void *info) > > control &= ~0x1; > > /* make sure everything completes before disabling */ > > - mb(); > > + /* As recommended by 7.3.77 of ARM IHI 0064D */ > > Nit: This refers to completely unrelated section. Shouldn't this be the same > as above ? Actually 4.3.7 relates to using dsb/isb after programming the trace unit registers and indicates this is to 'ensure that all updates are committed to the trace unit before normal code execution resumes'. Whereas 7.3.77 (hidden awawy in the SSTATUS description) relates to using dsb/isb before disabling the trace unit to 'prevent any start or stop points being specualtive at the point of disabling the trace unit'. Both sections suggest the same course of action - however I felt that the description in 7.3.77 better related to the context of etm4_disable_hw. Perhaps if I also add the section name, readers are more likely to find this text? > > With the above fixed: > > Reviewed-by: Suzuki K Poulose Thanks, Andrew Murray _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel