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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id E1DA5E7AD78 for ; Tue, 3 Oct 2023 16:44:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Lbx2hZD9PmlVH84S04zK/sQUv9yO6BE3lzTh13xxZ4k=; b=g/c03m+kEMCmdw mhpTKlSxhsBDG7ggxxs7qr+KWkIQCA9nb5Jlw5tIZY73EKy0SquDGhPa89MfS1stgAtFe0Nx84rm+ bK0i3ANFT7ynO8mo9luAnSRUKtFVwu/vJy/DnDxWJUr6CpEgwWN08KQ6xw7jq3Z8XLWDy0OqJmwM0 ZX/EGJV240FN8nN2eOMOmorhMe/xOL5iMG4+RbtJBunuu1FxTi2r/Od+Ad+TrHI8BBElvmuG9iMOr 5o/7xRitecg2wCOTTyIZTf1n60q1MwBdacEBYyoVl8P74XgimrdaJwu0n9p7g/ldoCUnNp24vcKGo 8ibfRwE5HnHmL4pbHciw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qniUo-00EzB3-2d; Tue, 03 Oct 2023 16:43:42 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qniUm-00Ez9q-0A for linux-arm-kernel@lists.infradead.org; Tue, 03 Oct 2023 16:43: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 D04B8C15; Tue, 3 Oct 2023 09:44:15 -0700 (PDT) Received: from [192.168.1.3] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 722483F762; Tue, 3 Oct 2023 09:43:35 -0700 (PDT) Message-ID: Date: Tue, 3 Oct 2023 17:43:34 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH 5/7] coresight: tmc: Add support for reading tracedata from previous boot Content-Language: en-US To: Linu Cherian , suzuki.poulose@arm.com, mike.leach@linaro.org, leo.yan@linaro.org 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 , Tanmay Jagdale References: <20230929133754.857678-1-lcherian@marvell.com> <20230929133754.857678-6-lcherian@marvell.com> From: James Clark In-Reply-To: <20230929133754.857678-6-lcherian@marvell.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231003_094340_185166_CA497361 X-CRM114-Status: GOOD ( 28.76 ) 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 29/09/2023 14:37, Linu Cherian wrote: > * Introduce a new mode CS_MODE_READ_PREVBOOT for reading tracedata > captured in previous boot. > > * Add special handlers for preparing ETR/ETF for this special mode > > * User can read the trace data as below > > For example, for reading trace data from tmc_etf sink > > 1. cd /sys/bus/coresight/devices/tmc_etfXX/ > > 2. Change mode to READ_PREVBOOT > > #echo 1 > read_prevboot > > 3. Dump trace buffer data to a file, > > #dd if=/dev/tmc_etrXX of=~/cstrace.bin > > 4. Reset back to normal mode > > #echo 0 > read_prevboot > > Signed-off-by: Anil Kumar Reddy > Signed-off-by: Tanmay Jagdale > Signed-off-by: Linu Cherian > --- > .../coresight/coresight-etm4x-core.c | 1 + > .../hwtracing/coresight/coresight-tmc-core.c | 81 +++++++++- > .../hwtracing/coresight/coresight-tmc-etf.c | 62 ++++++++ > .../hwtracing/coresight/coresight-tmc-etr.c | 145 +++++++++++++++++- > drivers/hwtracing/coresight/coresight-tmc.h | 6 + > include/linux/coresight.h | 13 ++ > 6 files changed, 306 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 77b0271ce6eb..513baf681280 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -1010,6 +1010,7 @@ static void etm4_disable(struct coresight_device *csdev, > > switch (mode) { > case CS_MODE_DISABLED: > + case CS_MODE_READ_PREVBOOT: > break; > case CS_MODE_SYSFS: > etm4_disable_sysfs(csdev); > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index 6658ce76777b..65c15c9f821b 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -103,6 +103,45 @@ u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata) > return mask; > } > > +int tmc_read_prepare_prevboot(struct tmc_drvdata *drvdata) > +{ > + int ret = 0; > + struct tmc_register_snapshot *reg_ptr; > + struct coresight_device *csdev = drvdata->csdev; > + > + if (!drvdata->metadata.vaddr) { > + ret = -ENOMEM; > + goto out; > + } > + > + reg_ptr = drvdata->metadata.vaddr; > + if (!reg_ptr->valid) { > + dev_err(&drvdata->csdev->dev, > + "Invalid metadata captured from previous boot\n"); > + ret = -EINVAL; > + goto out; > + } I'm wondering if a more robust check is needed than the valid flag, like a checksum or something. I didn't debug it yet but I ended up with an invalid set of metadata after a panic reboot, see below. I'm not sure if it's just a logic bug or something got lost during the reboot, I didn't debug it yet. But I suppose unless you assume the panic didn't affect writing the metadata, then it could be partially written and shouldn't be trusted? [...] > + > +static int tmc_etr_sync_prevboot_buf(struct tmc_drvdata *drvdata) > +{ > + u32 status; > + u64 rrp, rwp, dba; > + struct tmc_register_snapshot *reg_ptr; > + struct etr_buf *etr_buf = drvdata->prevboot_buf; > + > + reg_ptr = drvdata->metadata.vaddr; > + > + rrp = reg_ptr->rrp; > + rwp = reg_ptr->rwp; > + dba = reg_ptr->dba; > + status = reg_ptr->sts; > + > + etr_buf->full = !!(status & TMC_STS_FULL); > + > + /* Sync the buffer pointers */ > + etr_buf->offset = rrp - dba; > + if (etr_buf->full) > + etr_buf->len = etr_buf->size; > + else > + etr_buf->len = rwp - rrp; > + > + /* Sanity checks for validating metadata */ > + if ((etr_buf->offset > etr_buf->size) || > + (etr_buf->len > etr_buf->size)) > + return -EINVAL; The values I got here are 0x781b67182aa346f9 0x8000000 0x8000000 for offset, size and len respectively. This fails the first check. It would also be nice to have a dev_dbg here as well, it's basically the same as the valid check above which does have one. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel