Received-SPF: pass (google.com: domain of linux-kernel+bounces-142445-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <3c5c85d4-8657-4ee0-88fa-ee47dce4cc7c@arm.com>
Date: Fri, 12 Apr 2024 10:57:29 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 2/7] coresight: tmc-etr: Add support to use reserved
 trace memory
To: Linu Cherian <lcherian@marvell.com>, suzuki.poulose@arm.com
Cc: linux-arm-kernel@lists.infradead.org, coresight@lists.linaro.org,
 linux-kernel@vger.kernel.org, robh+dt@kernel.org,
 krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
 devicetree@vger.kernel.org, sgoutham@marvell.com, gcherian@marvell.com,
 Anil Kumar Reddy <areddy3@marvell.com>, mike.leach@linaro.org,
 leo.yan@linaro.org
References: <20240307033625.325058-1-lcherian@marvell.com>
 <20240307033625.325058-3-lcherian@marvell.com>
Content-Language: en-US
From: James Clark <james.clark@arm.com>
In-Reply-To: <20240307033625.325058-3-lcherian@marvell.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 07/03/2024 03:36, Linu Cherian wrote:
> Add support to use reserved memory for coresight ETR trace buffer.
> 
> Introduce a new ETR buffer mode called ETR_MODE_RESRV, which
> becomes available when ETR device tree node is supplied with a valid
> reserved memory region.
> 
> ETR_MODE_RESRV can be selected only by explicit user request.
> 
> $ echo resrv >/sys/bus/coresight/devices/tmc_etr<N>/buf_mode_preferred
> 
> Signed-off-by: Anil Kumar Reddy <areddy3@marvell.com>
> Signed-off-by: Linu Cherian <lcherian@marvell.com>
> ---
> Changelog from v6:
> * Removed redundant goto statements
> * Setting of etr_buf->size to the reserved memory size is done
>   after successful dma map inside the alloc function
> * Removed the special casing for ETR_MODE_RESRV 
> * Fixed the tab spacing in struct tmc_drvdata 
> 
>  .../hwtracing/coresight/coresight-tmc-core.c  | 47 +++++++++++
>  .../hwtracing/coresight/coresight-tmc-etr.c   | 82 ++++++++++++++++++-
>  drivers/hwtracing/coresight/coresight-tmc.h   | 27 ++++++
>  3 files changed, 153 insertions(+), 3 deletions(-)
> 

[...]

>  static bool etr_can_use_flat_mode(struct etr_buf_hw *buf_hw, ssize_t etr_buf_size)
> @@ -874,13 +947,10 @@ static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
>  	if (!etr_buf)
>  		return ERR_PTR(-ENOMEM);
>  
> -	etr_buf->size = size;
> -

Hi Linu,

Not sure if this was left in by mistake? It's not mentioned in the
commit message and it doesn't seem to match the description.

Please make sure the current tests pass both with and without a reserved
buffer defined in the DT. I get lots of failures with this patchset
applied on N1SDP. ETF seems to work but ETR doesn't:

  $ sudo perf test -vvv "arm coresight"

  Recording trace (only user mode) with path: CPU1 => tmc_etf0
  CoreSight path testing (CPU1 -> tmc_etf0): PASS

  Recording trace (only user mode) with path: CPU1 => tmc_etr0
  CoreSight path testing (CPU1 -> tmc_etr0): FAIL
  ...

Dmesg:
  [ 1938.622091] coresight tmc_etr0: Unable to allocate ETR buffer

>  	/* If there is user directive for buffer mode, try that first */
>  	if (drvdata->etr_mode != ETR_MODE_AUTO)
>  		rc = tmc_etr_mode_alloc_buf(drvdata->etr_mode, drvdata,
>  					    etr_buf, node, pages);
> -

Whitespace change.