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 6DA2FC3DA4A for ; Fri, 9 Aug 2024 14:15:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:Cc:To:Subject: Message-ID:Date:From:In-Reply-To:References:MIME-Version:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Rp39z+u4lrroAv5FlQuGnxo/zwyflIlnYSEFg4kk+oY=; b=3k7km1w0QbLlW2bt50zcZRVuiN XbXLtXbrS0JdEcRTlksgFXrh+Uip0dj/f22mn9MfSdAJxF2K9YjtDSsLFvSrfsKr/tIxYAxzqj1L8 mhX8AUZmSUWMaTHYsOlQQ+6CXMkG/BBDbL8vLZ/DJBU4o+YV06rmRvmwClzWHydzp7UfnBUvMai8j ZVlA8bNEFR1qzPyUxLRfC3tZlnBXo59eOqVYrz23ebgrC3/Rt3cfoS+Zl/Q+o3TbnrZ+upzGFx+uo 6Lch2yGWnICBkjUbgbzVJve/YBnnCPjVRQ8azStTJCGJKKnneG5e22BVs8x5gGNX3C4VEKoWn+LCX ifOF50NQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1scQOI-0000000BTiZ-0KDK; Fri, 09 Aug 2024 14:14:50 +0000 Received: from mail-pl1-x631.google.com ([2607:f8b0:4864:20::631]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1scQNd-0000000BTTI-2GQh for linux-arm-kernel@lists.infradead.org; Fri, 09 Aug 2024 14:14:12 +0000 Received: by mail-pl1-x631.google.com with SMTP id d9443c01a7336-1fdd6d81812so21752545ad.1 for ; Fri, 09 Aug 2024 07:14:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1723212848; x=1723817648; darn=lists.infradead.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Rp39z+u4lrroAv5FlQuGnxo/zwyflIlnYSEFg4kk+oY=; b=UhLh9l7BetuO6ztDlpbQR7G8mD/WvSDa4EnXmJZ1Hsn2t0VAbb8y2jJLBQeT+a5eOa 4fe8FzeFeisNjLx3gSBF9XN6eC64Z2UBdgP8q3iiR5FPBa0kqeIkRWn17nqSSuyOOJHk iqXKO8eH86PxlGMATD7RBnOMCykuyUuKKP82vyAuROy9d6GGlwS/SQhfGDhSvpJ7aeRf LUAYbYXeF7wQdtwvtl21x071qCQ35be1Pbcm8EtojmLa4fIyXHZ4In+6JY2vfOMuugd8 dv+lQT4fFXiI6/CgtN4RHylMiPRjCgR20wgfltJIv4JuRNViJ2Fgg8Ng3ESzyO5PrMJX 1/cw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723212848; x=1723817648; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Rp39z+u4lrroAv5FlQuGnxo/zwyflIlnYSEFg4kk+oY=; b=aKtPDH+1dX/w/vvJUCweNjCDQNSIOS+H8x+CFudoEUHGezYzw0MwtIRbO2ipKU7Psn 70kbguEMV5ZVs4QFObNI/AKptopRei0cvhs3kyLv5pr6lqvIoszqAG/U+6DOMo65DBZC AP05kKlWLQ1wCczywhXCHkHX8c6X/bfbB5KT6kF05oxjcypujD9xMI5+9b/04ADELjwB IEH8uz4H6Y8HfVLUiPJVU0J3xris19eRIHxfBCWzq1jzhoTz3j9d0ZUD1n95+H2F2zAB vaBKYAsaqVdQgcM6QeAs5hH17ETjx8fxbODWb75CcraPwALNO1BhqE7hCPvj4ho7+kzC 1xNQ== X-Forwarded-Encrypted: i=1; AJvYcCW4MK6dTvNWGlJtheeHWxDA0keEij2VRtAPCK3Hkgyd+1zAVt0irESMvhr/MpXa45awYXnfSYm3P2bLqjf3sYnyVwJ+ucrsWh58FxleJDg1AQp5PSI= X-Gm-Message-State: AOJu0Yymqe8cWT1f1Tu9Hg4Y1EZo077NbmT8uKtQXO9shN+/LR6O3Y+R eLQB4fCoRwomTL8ZzsdUR2CHPNyqcgDuLsRL/Wo+lfOEjkocHigB/F9eK1GPVuuZB1NQke3XPOP PJZ2cmKOwGH1P9xxJBamUvwJcFrmMLZs0V2s+0A== X-Google-Smtp-Source: AGHT+IFyMQUTum0CqjZAJJvppxm1rAPilIvHqmOtoXaZfy5yrhQPNhXHNDxUbJ5rDqe1YrKyPxYvIictHLGJhJIcb/s= X-Received: by 2002:a17:902:e543:b0:1ff:4d2a:fe43 with SMTP id d9443c01a7336-200ae5e6a9bmr20147015ad.61.1723212848304; Fri, 09 Aug 2024 07:14:08 -0700 (PDT) MIME-Version: 1.0 References: <20240719092619.274730-1-gankulkarni@os.amperecomputing.com> <8f6f221b-4c9a-42e1-b8ce-1f492caee184@linaro.org> <0a697a54-5dd8-4351-a651-991724690db2@os.amperecomputing.com> <543813f6-cb1f-4759-b26f-75246750814d@linaro.org> <00fac24c-d664-4ebb-8c60-f4697b7f76c1@linaro.org> <8b53a424-19f7-4042-a2db-e1c5d051f9cc@os.amperecomputing.com> <6adf84fa-b755-4d7a-957a-9bf01e442238@linaro.org> <6f535bb6-2cee-48e6-93f1-ea19887bae74@os.amperecomputing.com> <027c76a9-9bd4-43e9-a170-8391a0037291@linaro.org> <3d7a6f93-0555-48fa-99cb-bf26b53c2da5@os.amperecomputing.com> <4dd7f210-c03e-4203-b8e9-1c26a7f8fe79@arm.com> In-Reply-To: From: Mike Leach Date: Fri, 9 Aug 2024 15:13:56 +0100 Message-ID: Subject: Re: [PATCH] perf scripts python arm-cs-trace-disasm.py: Skip disasm if address continuity is broken To: James Clark Cc: Leo Yan , Ganapatrao Kulkarni , scclevenger@os.amperecomputing.com, acme@redhat.com, coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, darren@os.amperecomputing.com, james.clark@arm.com, suzuki.poulose@arm.com, Al.Grant@arm.com Content-Type: text/plain; charset="UTF-8" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240809_071410_207848_F9A94484 X-CRM114-Status: GOOD ( 36.64 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi James On Thu, 8 Aug 2024 at 10:32, James Clark wrote: > > > > On 07/08/2024 5:48 pm, Leo Yan wrote: > > Hi all, > > > > On 8/7/2024 3:53 PM, James Clark wrote: > > > > A minor suggestion: if the discussion is too long, please delete the > > irrelevant message ;) > > > > [...] > > > >>> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py > >>> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py > >>> @@ -257,6 +257,11 @@ def process_event(param_dict): > >>> print("Stop address 0x%x is out of range [ 0x%x .. 0x%x > >>> ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) > >>> return > >>> > >>> + if (stop_addr < start_addr): > >>> + if (options.verbose == True): > >>> + print("Packet Dropped, Discontinuity detected > >>> [stop_add:0x%x start_addr:0x%x ] for dso %s" % (stop_addr, start_addr, > >>> dso)) > >>> + return > >>> + > >> > >> I suppose my only concern with this is that it hides real errors and > >> Perf shouldn't be outputting samples that go backwards. Considering that > >> fixing this in OpenCSD and Perf has a much wider benefit I think that > >> should be the ultimate goal. I'm putting this on my todo list for now > >> (including Steve's merging idea). > > > > In the perf's util/cs-etm.c file, it handles DISCONTINUITY with: > > > > case CS_ETM_DISCONTINUITY: > > /* > > * The trace is discontinuous, if the previous packet is > > * instruction packet, set flag PERF_IP_FLAG_TRACE_END > > * for previous packet. > > */ > > if (prev_packet->sample_type == CS_ETM_RANGE) > > prev_packet->flags |= PERF_IP_FLAG_BRANCH | > > PERF_IP_FLAG_TRACE_END; > > > > I am wandering if OpenCSD has passed the correct info so Perf decoder can > > detect the discontinuity. If yes, then the flag 'PERF_IP_FLAG_TRACE_END' will > > be set (it is a general flag in branch sample), then we can consider use it in > > the python script to handle discontinuous data. > > No OpenCSD isn't passing the correct info here. Higher up in the thread > I suggested an OpenCSD patch that makes it detect the error earlier and > fixes the issue. It also needs to output a discontinuity when the > address goes backwards. So two fixes and then the script works without > modifications. > Which address is going backwards here? - OpenCSD generates trace ranges only by walking forwards from the last known address till it hits a branch. Unless this wraps round 0x000000 this will never result in a backwards address as far as I can see. Do you have an example dump with OpenCSD outputting a range packet with backwards addresses? Mike > > > >> > >> But in the mean time what about having a force option? > >> > >>> + if (stop_addr < start_addr): > >>> + if (options.verbose == True or not options.force): > >>> + print("Packet Dropped, Discontinuity detected > >>> [stop_add:0x%x start_addr:0x%x ] for dso %s" % (stop_addr, start_addr, > >>> dso)) > >>> + if (not options.force): > >>> + return > > > > If the stop address is less than the start address, it must be something > > wrong. In this case, we can report a warning for discontinuity and directly > > return (also need to save the `addr` into global variable for next parsing). > > > > I prefer to not add force option for this case - eventually, this will consume > > much time for reporting this kind of failure and need to root causing it. A > > better way is we just print out the reasoning in the log and continue to dump. > > But in this case we've identified all the known issues that would cause > the script to fail and we can fix them in Perf and OpenCSD. There may > not even be any more issues that will cause the script to fail in the > future so there's no point in softening the error IMO. That will only > hide future issues (of which there may be none) and make root causing > harder when it hits some other tool. -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK