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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4BAD3C433F5 for ; Fri, 1 Oct 2021 08:36:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3027E61507 for ; Fri, 1 Oct 2021 08:36:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1352806AbhJAIiX (ORCPT ); Fri, 1 Oct 2021 04:38:23 -0400 Received: from foss.arm.com ([217.140.110.172]:37874 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1352782AbhJAIiO (ORCPT ); Fri, 1 Oct 2021 04:38:14 -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 6FBFD101E; Fri, 1 Oct 2021 01:36:28 -0700 (PDT) Received: from [10.57.72.173] (unknown [10.57.72.173]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7778F3F793; Fri, 1 Oct 2021 01:36:26 -0700 (PDT) Subject: Re: [PATCH v2 03/17] coresight: trbe: Add a helper to calculate the trace generated To: Mathieu Poirier Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, maz@kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com, james.morse@arm.com, anshuman.khandual@arm.com, leo.yan@linaro.org, mike.leach@linaro.org, will@kernel.org, lcherian@marvell.com, coresight@lists.linaro.org References: <20210921134121.2423546-1-suzuki.poulose@arm.com> <20210921134121.2423546-4-suzuki.poulose@arm.com> <20210930175421.GB3047827@p14s> From: Suzuki K Poulose Message-ID: <60037d18-9d0e-68ce-2a34-aa84e7876fb8@arm.com> Date: Fri, 1 Oct 2021 09:36:24 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20210930175421.GB3047827@p14s> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 30/09/2021 18:54, Mathieu Poirier wrote: > Hi Suzuki, > > On Tue, Sep 21, 2021 at 02:41:07PM +0100, Suzuki K Poulose wrote: >> We collect the trace from the TRBE on FILL event from IRQ context >> and when via update_buffer(), when the event is stopped. Let us > > s/"and when via"/"and via" > >> consolidate how we calculate the trace generated into a helper. >> >> Cc: Mathieu Poirier >> Cc: Mike Leach >> Cc: Leo Yan >> Reviewed-by: Anshuman Khandual >> Signed-off-by: Suzuki K Poulose >> --- >> drivers/hwtracing/coresight/coresight-trbe.c | 48 ++++++++++++-------- >> 1 file changed, 30 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c >> index 63f7edd5fd1f..063c4505a203 100644 >> --- a/drivers/hwtracing/coresight/coresight-trbe.c >> +++ b/drivers/hwtracing/coresight/coresight-trbe.c >> @@ -527,6 +527,30 @@ static enum trbe_fault_action trbe_get_fault_act(u64 trbsr) >> return TRBE_FAULT_ACT_SPURIOUS; >> } >> >> +static unsigned long trbe_get_trace_size(struct perf_output_handle *handle, >> + struct trbe_buf *buf, >> + bool wrap) > > Stacking > Ack >> +{ >> + u64 write; >> + u64 start_off, end_off; >> + >> + /* >> + * If the TRBE has wrapped around the write pointer has >> + * wrapped and should be treated as limit. >> + */ >> + if (wrap) >> + write = get_trbe_limit_pointer(); >> + else >> + write = get_trbe_write_pointer(); >> + >> + end_off = write - buf->trbe_base; > > In both arm_trbe_alloc_buffer() and trbe_handle_overflow() the base address is > acquired using get_trbe_base_pointer() but here it is referenced directly - any > reason for that? It certainly makes reviewing this simple patch quite > difficult because I keep wondering if I am missing something subtle... Very good observation. So far, we always prgrammed the TRBBASER with the the VA(ring_buffer[0]). And thus reading the BASER and using the buf->trbe_base is all fine. But going forward, we are going to use different values for the TRBBASER to work around erratum. Thus to make the computation of the "offsets" within the ring buffer, it is always correct to use this field. I could move this to the patch where the work around is introduced, and put in a comment there. Thanks for the review Suzuki 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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C9CEFC433F5 for ; Fri, 1 Oct 2021 08:38:17 +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 8DE3461353 for ; Fri, 1 Oct 2021 08:38:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 8DE3461353 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com 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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=rbLaherZtP1njcfSj6JwQ76F8oQM+nObaKeSibOLgeM=; b=2MLKBs0DouTgCWQzwLeSclfy2r H3nIthxGcr2DNtvWTO7Frm/BfIsd3KGNFluy2aSq2N+qf8gn2HhaU2/vMUsq6CksDWthbgIpKI1Mn g7kGXSXXnWwPIAMutErO+B4uocPRC6rZX3/0ooSKd5F9k8bge7CfISUx+z+5UUK/T+JyX9KFOChgQ ZYhoaBEOg1cUgs1GixtBFfDznxKVo7JwjtyVy21hw9XjXfmuXurFDSq9cha7Sorskj7rC5DdvdbES slFpUA+2cpNhDZBrGyXkmZRhmOPAMI1dslGWEwGk56lzjbCqfDEDQaPlmq7BHRULpdIP4fNxOEnBO lX7S9o3w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mWE21-00H5Qa-DC; Fri, 01 Oct 2021 08:36:37 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mWE1x-00H5PX-LW for linux-arm-kernel@lists.infradead.org; Fri, 01 Oct 2021 08:36:35 +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 6FBFD101E; Fri, 1 Oct 2021 01:36:28 -0700 (PDT) Received: from [10.57.72.173] (unknown [10.57.72.173]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7778F3F793; Fri, 1 Oct 2021 01:36:26 -0700 (PDT) Subject: Re: [PATCH v2 03/17] coresight: trbe: Add a helper to calculate the trace generated To: Mathieu Poirier Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, maz@kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com, james.morse@arm.com, anshuman.khandual@arm.com, leo.yan@linaro.org, mike.leach@linaro.org, will@kernel.org, lcherian@marvell.com, coresight@lists.linaro.org References: <20210921134121.2423546-1-suzuki.poulose@arm.com> <20210921134121.2423546-4-suzuki.poulose@arm.com> <20210930175421.GB3047827@p14s> From: Suzuki K Poulose Message-ID: <60037d18-9d0e-68ce-2a34-aa84e7876fb8@arm.com> Date: Fri, 1 Oct 2021 09:36:24 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20210930175421.GB3047827@p14s> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211001_013633_814297_DD9FBA1A X-CRM114-Status: GOOD ( 22.44 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 30/09/2021 18:54, Mathieu Poirier wrote: > Hi Suzuki, > > On Tue, Sep 21, 2021 at 02:41:07PM +0100, Suzuki K Poulose wrote: >> We collect the trace from the TRBE on FILL event from IRQ context >> and when via update_buffer(), when the event is stopped. Let us > > s/"and when via"/"and via" > >> consolidate how we calculate the trace generated into a helper. >> >> Cc: Mathieu Poirier >> Cc: Mike Leach >> Cc: Leo Yan >> Reviewed-by: Anshuman Khandual >> Signed-off-by: Suzuki K Poulose >> --- >> drivers/hwtracing/coresight/coresight-trbe.c | 48 ++++++++++++-------- >> 1 file changed, 30 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c >> index 63f7edd5fd1f..063c4505a203 100644 >> --- a/drivers/hwtracing/coresight/coresight-trbe.c >> +++ b/drivers/hwtracing/coresight/coresight-trbe.c >> @@ -527,6 +527,30 @@ static enum trbe_fault_action trbe_get_fault_act(u64 trbsr) >> return TRBE_FAULT_ACT_SPURIOUS; >> } >> >> +static unsigned long trbe_get_trace_size(struct perf_output_handle *handle, >> + struct trbe_buf *buf, >> + bool wrap) > > Stacking > Ack >> +{ >> + u64 write; >> + u64 start_off, end_off; >> + >> + /* >> + * If the TRBE has wrapped around the write pointer has >> + * wrapped and should be treated as limit. >> + */ >> + if (wrap) >> + write = get_trbe_limit_pointer(); >> + else >> + write = get_trbe_write_pointer(); >> + >> + end_off = write - buf->trbe_base; > > In both arm_trbe_alloc_buffer() and trbe_handle_overflow() the base address is > acquired using get_trbe_base_pointer() but here it is referenced directly - any > reason for that? It certainly makes reviewing this simple patch quite > difficult because I keep wondering if I am missing something subtle... Very good observation. So far, we always prgrammed the TRBBASER with the the VA(ring_buffer[0]). And thus reading the BASER and using the buf->trbe_base is all fine. But going forward, we are going to use different values for the TRBBASER to work around erratum. Thus to make the computation of the "offsets" within the ring buffer, it is always correct to use this field. I could move this to the patch where the work around is introduced, and put in a comment there. Thanks for the review Suzuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel