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=-12.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 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 F37A7C433E0 for ; Thu, 30 Jul 2020 14:42:57 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 C17C120829 for ; Thu, 30 Jul 2020 14:42:57 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="iI4Z1o7s" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C17C120829 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+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=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=fRVjufL8XEfP43PdxW7pU5QfIGLD39QpMkIWJGLYXSI=; b=iI4Z1o7s+HtTCJDzqnJwWZ32N m5QEg55QfAoXlVCnLq8WEQrDv6X1i0Of1SukdSnjOI7GA6T4FDsTtHR+dF+ByJF8zlUsef917emB1 z5qWuREt2kH9PMjJp2KwMbeb0tFcWKq3nTuTbspDvnuKhRmW4Pw8kL0AIV5q/rbC122LanvDoVzUz YYLYHpP0rji3wIVBQGs+ivFr7S5qmc0fHi8uCL3TZsHnaX/oGWRzFDdNY+IhHbZce0bIpRsLLYsDS w2/S+D/8QekDVZRldfI3qeoxtVpkncmtUlE6puEzQbv/jpIV5ShxTC94pnJ6KOVj3CCONdwi9sRgJ txN02wW/g==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k19k6-0005iC-Id; Thu, 30 Jul 2020 14:41:10 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k19k3-0005hO-9R for linux-arm-kernel@lists.infradead.org; Thu, 30 Jul 2020 14:41:08 +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 D42F61FB; Thu, 30 Jul 2020 07:41:01 -0700 (PDT) Received: from [10.37.12.83] (unknown [10.37.12.83]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6E2313F71F; Thu, 30 Jul 2020 07:41:00 -0700 (PDT) Subject: Re: [RFC PATCH 01/14] coresight: etm4x: Skip save/restore before device registration To: mathieu.poirier@linaro.org References: <20200722172040.1299289-1-suzuki.poulose@arm.com> <20200722172040.1299289-2-suzuki.poulose@arm.com> <20200729180128.GA3073178@xps15> From: Suzuki K Poulose Message-ID: Date: Thu, 30 Jul 2020 15:45:45 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20200729180128.GA3073178@xps15> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200730_104107_460441_1871666A X-CRM114-Status: GOOD ( 29.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: coresight@lists.linaro.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, mike.leach@linaro.org 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 07/29/2020 07:01 PM, Mathieu Poirier wrote: > Hi Suzuki, > > I have starte to review this - comments will be scattered over a few days. > > On Wed, Jul 22, 2020 at 06:20:27PM +0100, Suzuki K Poulose wrote: >> Skip cpu save/restore before the coresight device is registered. >> >> Cc: Mathieu Poirier >> Cc: Mike Leach >> Signed-off-by: Suzuki K Poulose >> --- >> drivers/hwtracing/coresight/coresight-etm4x.c | 16 +++++++++++++++- >> 1 file changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c >> index 6d7d2169bfb2..cb83fb77ded6 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm4x.c >> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c >> @@ -1135,7 +1135,13 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) >> { >> int i, ret = 0; >> struct etmv4_save_state *state; >> - struct device *etm_dev = &drvdata->csdev->dev; >> + struct coresight_device *csdev = drvdata->csdev; >> + struct device *etm_dev; >> + >> + if (WARN_ON(!csdev)) >> + return -ENODEV; >> + >> + etm_dev = &csdev->dev; >> >> /* >> * As recommended by 3.4.1 ("The procedure when powering down the PE") >> @@ -1261,6 +1267,10 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) >> { >> int i; >> struct etmv4_save_state *state = drvdata->save_state; >> + struct coresight_device *csdev = drvdata->csdev; >> + >> + if (WARN_ON(!csdev)) >> + return; > > Restore and save operations are only called from etm4_cpu_pm_notify() where the > check for a valid drvdata->csdev is already done. > Correct, this is just an enforcement as we are going to rely on the availability of drvdata->csdev to access the device with the introduction of abstraction. This is why we WARN_ON() as we should never hit this case. >> >> CS_UNLOCK(drvdata->base); >> >> @@ -1368,6 +1378,10 @@ static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, >> >> drvdata = etmdrvdata[cpu]; >> >> + /* If we have not registered the device there is nothing to do */ >> + if (!drvdata->csdev) >> + return NOTIFY_OK; > > Can you describe the scenario you've seen this happening in? Probably best to > add it to the changelog. The CPU PM notifier is registered with the probing of the first ETM device. Now, another ETM device could be probed (on a different CPU than the parent of this ETM). Now, there is a very narrow window of time between : 1) Initialise etmdrvdata[cpu] 2) Register the coresight_device for the ETM.(i.e, coresight_register()). If the CPU is put on idle, after (1) and before (2), we end up with drvdata->csdev == NULL. This is unacceptable and there is no need to take an action in such case. This patch fixes the potential problem, also making sure that we have the access methods available when we need it. (drvdata->csdev->access) I will add it to the commit message. Cheers Suzuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel