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 E79D1C433F5 for ; Sat, 5 Mar 2022 19:05:24 +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:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/UILo0I69mgp7RrKPlWvEFKIRQ2HjWwJ30IKB8Jd6SY=; b=X3N5b/bwKqY45S mHzbyAjuFoMeXqdHPIAP+rJN4iWbV3BOg1Fhdnakna76GD4GOUgZbdvVY3o6g47ZdiLzZhX/GAqLH 4rrHKGHrDjonhApTZZIeFk3MPRJeUfuFxzWU5FfrnEd5LRKgW0tjtm5JoAH4MnyKNcjQAOJbe6/yD jhx65/aKJ3CpC/9z2grIEDDoU+P3AJCe2s91hWXCPHqO0+tpIwQjGaz8154dqlFJkXHa0Jeu96fGd qpyXms1QBqzcy/7tEvl/SvKc5kt93tqZKixIjr1wtqy23kCtPB87/ZUu7zz+rZUSmB/2xPcSXf8iv J9R9vHq/wAVCjkdf+HXg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nQZhJ-00DfX4-5f; Sat, 05 Mar 2022 19:04:09 +0000 Received: from casper.infradead.org ([2001:8b0:10b:1236::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nQZhH-00DfWj-3A for linux-arm-kernel@bombadil.infradead.org; Sat, 05 Mar 2022 19:04:07 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=ZoEgsj0zPHbStnA0hjCe37/zmu+TNtci8uXzf1GISX8=; b=pBi4/k9VPo+XERK3vrjyww3Zax S4m+C+iPKy12APtnU0yrakGWuWuiPjkFNShU21htvl0OiM9f1s4vMWy2ID/FXTyRnMGU5yJ+I7j7U O/inFPCOsXKrDtIS4+yKwaRAHTghiVL5fI7LTnvZtCVVa51JgxJII25TxoXFvbvYgIHWS9hA0lspI Ou074A4y1KM7jgo7e1iUlNAfzqv3AFGW4fny9fYfNdIWBVGgb5rrB+EcPEuWuFmzbdS2bB3Pzy4Av tAPuDKCFDRUKL5gW+6Ys8VAwDC6kuBSgmt08kIYc2JZNf17Tig+eHg9LluaOnTn9c7pEcy4gbKUy8 XMnQx88Q==; Received: from [179.97.37.151] (helo=quaco.ghostprotocols.net) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1nQZhF-00DnJO-8H; Sat, 05 Mar 2022 19:04:05 +0000 Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id BB5BE403C8; Sat, 5 Mar 2022 16:04:02 -0300 (-03) Date: Sat, 5 Mar 2022 16:04:02 -0300 From: Arnaldo Carvalho de Melo To: James Clark Cc: Anshuman Khandual , linux-kernel@vger.kernel.org, Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Thomas Gleixner , Will Deacon , linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org Subject: Re: [PATCH] perf: Add irq and exception return branch types Message-ID: References: <1645681014-3346-1-git-send-email-anshuman.khandual@arm.com> <10aac3a9-6912-6e47-400e-5c7d8ca9dbcd@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <10aac3a9-6912-6e47-400e-5c7d8ca9dbcd@arm.com> X-Url: http://acmel.wordpress.com 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 Em Mon, Feb 28, 2022 at 03:45:25PM +0000, James Clark escreveu: > > > On 24/02/2022 05:36, Anshuman Khandual wrote: > > This expands generic branch type classification by adding two more entries > > there in i.e irq and exception return. Also updates the x86 implementation > > to process X86_BR_IRET and X86_BR_IRQ records as appropriate. This changes > > branch types reported to user space on x86 platform but it should not be a > > problem. The possible scenarios and impacts are enumerated here. > > > > -------------------------------------------------------------------------- > > | kernel | perf tool | Impact | > > -------------------------------------------------------------------------- > > | old | old | Works as before | > > -------------------------------------------------------------------------- > > | old | new | PERF_BR_UNKNOWN is processed | > > -------------------------------------------------------------------------- > > | new | old | PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX | > > -------------------------------------------------------------------------- > > | new | new | PERF_BR_ERET/IRQ are recognized | > > -------------------------------------------------------------------------- > > > > When PERF_BR_ERET/IRQ are blocked via old PERF_BR_MAX (new kernel with old > > perf tool) the user space might throw up an warning complaining about some > > unrecognized branch types being reported, but it is expected. PERF_BR_ERET > > and PERF_BR_IRQ branch types will be used for BRBE implementation on arm64 > > platform. > > > > Cc: Peter Zijlstra > > Cc: Ingo Molnar > > Cc: Arnaldo Carvalho de Melo > > Cc: Mark Rutland > > Cc: Alexander Shishkin > > Cc: Jiri Olsa > > Cc: Namhyung Kim > > Cc: Thomas Gleixner > > Cc: Will Deacon > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: linux-perf-users@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: Anshuman Khandual > > --- > > This applies on v5.17-rc5 > > > > These two new branch types expands generic branch type classification but > > still leaves another three entries in 'type' field for later. Please refer > > a previous discussion [1] for some further context. > > > > [1] https://lore.kernel.org/all/1643348653-24367-1-git-send-email-anshuman.khandual@arm.com/ > > > > arch/x86/events/intel/lbr.c | 4 ++-- > > include/uapi/linux/perf_event.h | 2 ++ Please try to avoid lockstep development of kernel and tools/, submit patches to the kernel maintainers for the kernel parts, and to the perf tools maintainer in separate patches. It is important that changes to the API are flagged, for instance via tools/perf/check-headers.sh so that opportunity is given for the various people involved in perf (u/k) development to see what is going on. Thanks, - Arnaldo > > tools/include/uapi/linux/perf_event.h | 2 ++ > > tools/perf/util/branch.c | 4 +++- > > 4 files changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c > > index 669c2be14784..fe1742c4ca49 100644 > > --- a/arch/x86/events/intel/lbr.c > > +++ b/arch/x86/events/intel/lbr.c > > @@ -1329,10 +1329,10 @@ static int branch_map[X86_BR_TYPE_MAP_MAX] = { > > PERF_BR_SYSCALL, /* X86_BR_SYSCALL */ > > PERF_BR_SYSRET, /* X86_BR_SYSRET */ > > PERF_BR_UNKNOWN, /* X86_BR_INT */ > > - PERF_BR_UNKNOWN, /* X86_BR_IRET */ > > + PERF_BR_ERET, /* X86_BR_IRET */ > > PERF_BR_COND, /* X86_BR_JCC */ > > PERF_BR_UNCOND, /* X86_BR_JMP */ > > - PERF_BR_UNKNOWN, /* X86_BR_IRQ */ > > + PERF_BR_IRQ, /* X86_BR_IRQ */ > Hi Anshuman, > > I couldn't verify if these changes to the lbr map matched up to the spec because > I couldn't find the right section. I suppose there might need to be a comment > about why BR_ERET == BR_IRET or what those abbreviations are. > > I think it could also be possible to leave these entries as unknowns if we don't know > if they're even being used. It will always be possible to go back and update these > lbr mappings after you've added the new types. > > > > PERF_BR_IND_CALL, /* X86_BR_IND_CALL */ > > PERF_BR_UNKNOWN, /* X86_BR_ABORT */ > > PERF_BR_UNKNOWN, /* X86_BR_IN_TX */ > > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h > > index 82858b697c05..d37629dbad72 100644 > > --- a/include/uapi/linux/perf_event.h > > +++ b/include/uapi/linux/perf_event.h > > @@ -251,6 +251,8 @@ enum { > > PERF_BR_SYSRET = 8, /* syscall return */ > > PERF_BR_COND_CALL = 9, /* conditional function call */ > > PERF_BR_COND_RET = 10, /* conditional function return */ > > + PERF_BR_ERET = 11, /* exception return */ > > + PERF_BR_IRQ = 12, /* irq */ > > PERF_BR_MAX,> }; > > > > diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h > > index 82858b697c05..d37629dbad72 100644 > > --- a/tools/include/uapi/linux/perf_event.h > > +++ b/tools/include/uapi/linux/perf_event.h > > @@ -251,6 +251,8 @@ enum { > > PERF_BR_SYSRET = 8, /* syscall return */ > > PERF_BR_COND_CALL = 9, /* conditional function call */ > > PERF_BR_COND_RET = 10, /* conditional function return */ > > + PERF_BR_ERET = 11, /* exception return */ > > + PERF_BR_IRQ = 12, /* irq */ > > PERF_BR_MAX, > > }; > > > > diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c > > index 2285b1eb3128..a9a909db8cc7 100644 > > --- a/tools/perf/util/branch.c > > +++ b/tools/perf/util/branch.c > > @@ -49,7 +49,9 @@ const char *branch_type_name(int type) > > "SYSCALL", > > "SYSRET", > > "COND_CALL", > > - "COND_RET" > > + "COND_RET", > > + "ERET", > > + "IRQ" > > }; > > > Otherwise the new entries look good to me, so without the lbr changes: > > Reviewed-by: James Clark > > If we're keeping the lbr mapping changes, then I will defer to someone else > to review. > > Thanks > James > > > if (type >= 0 && type < PERF_BR_MAX) -- - Arnaldo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel