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 54F9CC433F5 for ; Tue, 8 Feb 2022 13:14:10 +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=K+Q3Suz3N6cFCTDso1jsMmcbclqS8fbz56LjWg+wwP8=; b=4p/A0u5YMvnkfJ q9ITZ7CDmcehOLObVjXuzwLsN8szMUdNAf5fAgJrzmrcy8ZtNkkFJ80xgKX2egW0A2FZtpDRdMAWv q4VnwXbX4Di0IJ3i1rdoQAUEM5nRsOpr99drj4KDDbrnMS9LaW+d1/DSEmXYEmtyd90T/bWRqSXhP 424ULTU3ufjuPmQdZ64DBEMyqXp/hJ2nlug2ZgFu0hVZkUKKSRPnRLaL22APrNf7GW1IsqZZkxm9U LWrnrmtTOXX4HW3EX2FkKr4KbKKJn+VLWDLAKEN8qtNYtlPSTcGhokN3eur+H4YzJLMTLBjl8z+ZS TGTh5CQ7NCYzX7JmjeXw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nHQHy-00E0m0-Lc; Tue, 08 Feb 2022 13:12:13 +0000 Received: from mail-ej1-x633.google.com ([2a00:1450:4864:20::633]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nHQ76-00Dw7p-De for linux-arm-kernel@lists.infradead.org; Tue, 08 Feb 2022 13:00:58 +0000 Received: by mail-ej1-x633.google.com with SMTP id k25so51600806ejp.5 for ; Tue, 08 Feb 2022 05:00:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=sgE/xQ/+9cclDjXn3p9Poq5b+nUiXr8NosRhKU1WPOI=; b=u0NGyojcfUrrXm7SA9UB6FvjZCVTK2c3+k5JX8hnf13T0wAr7bHay4EuWIGxlqc7ht URWuXbXGijtB2CErJs54bRz+VolKQ5aI7p43JPRRALiy2RBrGmRD07kYrmVIPqkN5b3B qNVBR2AZlI4KTqy7Zyjsw7P3f9Tj6tNVhg2zZsQgUdWXOuUTyA8Cuv9UfzQOOGNUKap5 QHJ3coUKIzh13R9l5RWmisTrOhGrpst/lqjT5tf8Wpqy5wSALdEJNIyMJoYkrOjLBhA8 gAq0kwPTEbUNFLUTiCVxLit9RYIbDVVLXvceQWiAE/lHjj51dKDFtVvZrgnsqJxDTHxb tZmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=sgE/xQ/+9cclDjXn3p9Poq5b+nUiXr8NosRhKU1WPOI=; b=B7B/C+iY9I8IB+MdqkQiqFHZcSAQ2Pz8vSAqEJ8U+sx2SNHmZnxcs8za8WuupsjcKb iWSeBlyWA328ovr9maX82fNAO1Stri1Kdm+ERAtVXSVFjkvRjICjTOiQ32fKiXjZs2/w Mz6/G4AisfCj00nhtEOaj2tDpvdHzIA0SQaOSJouVINvLRAdFWLeBn8FOErr/wUDhaKV FF1TgT7F4YtRIQYGh6Ned0D4iAadDw8Sr7hwMddOEivU2MvttB7JtwCPb6D7c7qMKI85 x1qPfRT01utvOAE3pBnl41W8TWTqo2gcaRpjR/Ov4ykFv7gDWA9TYUGgDxj2GLrjrL87 8U2w== X-Gm-Message-State: AOAM5326IhhrzJYFsSP5hB56fJE3qokBSwEJAV34JfA1RSIDXRhYDzL9 d+WG05KwEeJGLYJ8Hop/wb1kVg== X-Google-Smtp-Source: ABdhPJx0Xa0/oSNN61iMZmiZDaLdK5fU7sB6YAOngEbNBgD+TUspwhEFssy+nEbHiA9adB0tdhR3eQ== X-Received: by 2002:a17:907:6d1e:: with SMTP id sa30mr2000503ejc.24.1644325253383; Tue, 08 Feb 2022 05:00:53 -0800 (PST) Received: from leoy-ThinkPad-X240s ([104.245.96.65]) by smtp.gmail.com with ESMTPSA id z2sm3032818ejr.68.2022.02.08.05.00.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 08 Feb 2022 05:00:52 -0800 (PST) Date: Tue, 8 Feb 2022 21:00:47 +0800 From: Leo Yan To: German Gomez Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, will@kernel.org, mark.rutland@arm.com, james.clark@arm.com Subject: Re: [RFC PATCH 1/2] perf: arm_spe: Fix consistency of PMSCR register bit CX Message-ID: <20220208130047.GA273989@leoy-ThinkPad-X240s> References: <20220117124432.3119132-1-german.gomez@arm.com> <20220117124432.3119132-2-german.gomez@arm.com> <20220205153940.GB391033@leoy-ThinkPad-X240s> <4d5951ee-d7d2-1e76-eb24-5f3c46d1662c@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4d5951ee-d7d2-1e76-eb24-5f3c46d1662c@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220208_050056_517557_06ACE444 X-CRM114-Status: GOOD ( 34.08 ) 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="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi German, On Mon, Feb 07, 2022 at 12:06:14PM +0000, German Gomez wrote: [...] > > I reviewed the code and also traced the backtrace for the function > > arm_spe_pmu_start(), I can confirm that every time perf session will > > execute below flow: > > > > evlist__enable() > > __evlist__enable() > > evlist__for_each_cpu() { -> call affinity__set() > > evsel__enable_cpu() > > } > > > > We can see the macro evlist__for_each_cpu() will extend to invoke > > evlist__cpu_begin() and affinity__set(); affinity__set() will set CPU > > affinity to the target CPU, thus perf process will firstly migrate to > > the target CPU and enable event on the target CPU. This means perf > > will not send remote IPI and it directly runs on target CPU, and the > > dd program will not interfere capabilities for perf session. > > Thank you for looking at this, > = > I re-tested on the N1SDP (previously I was using a graviton2 instance). > I had to adjust the command slightly with "-m,2" to get it consistently > this time: > = > $ taskset --cpu-list 0 sudo dd if=3D/dev/random of=3D/dev/null & > $ perf record -e arm_spe_0// -C0 -m,2 -- sleep 1 > $ perf report -D | grep CONTEXT | head > .=A0 0000000e:=A0 65 b5 6e 00 00=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 CONTEXT 0x6eb5= el2 > .=A0 0000004e:=A0 65 b5 6e 00 00=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 CONTEXT 0x6eb5= el2 > .=A0 0000008e:=A0 65 b5 6e 00 00=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 CONTEXT 0x6eb5= el2 > [...] Indeed! I can reproduce the issue now. And I can capture backtrace for arm_spe_pmu_start() with below commands: # cd /home/leoy/linux/tools/perf # ./perf probe --add "arm_spe_pmu_start" -s /home/leoy/linux/ -k /home/leoy= /linux/vmlinux # echo 1 > /sys/kernel/debug/tracing/events/probe/arm_spe_pmu_start/enable # echo stacktrace > /sys/kernel/debug/tracing/events/probe/arm_spe_pmu_star= t/trigger ... run your commands with non-root user ... # cat /sys/kernel/debug/tracing/trace dd-7697 [000] d.h2. 506.068700: arm_spe_pmu_start: (arm_s= pe_pmu_start+0x8/0xe0) dd-7697 [000] d.h3. 506.068701: =3D> kprobe_dispatcher =3D> kprobe_breakpoint_handler =3D> call_break_hook =3D> brk_handler =3D> do_debug_exception =3D> el1_dbg =3D> el1h_64_sync_handler =3D> el1h_64_sync =3D> arm_spe_pmu_start =3D> event_sched_in.isra.0 =3D> merge_sched_in =3D> visit_groups_merge.constprop.0 =3D> ctx_sched_in =3D> perf_event_sched_in =3D> ctx_resched =3D> __perf_event_enable =3D> event_function =3D> remote_function =3D> flush_smp_call_function_queue =3D> generic_smp_call_function_single_interrupt =3D> ipi_handler =3D> handle_percpu_devid_irq =3D> generic_handle_domain_irq =3D> gic_handle_irq =3D> call_on_irq_stack =3D> do_interrupt_handler =3D> el1_interrupt =3D> el1h_64_irq_handler =3D> el1h_64_irq =3D> _raw_spin_unlock_irqrestore =3D> urandom_read_nowarn.isra.0 =3D> random_read =3D> vfs_read =3D> ksys_read =3D> __arm64_sys_read =3D> invoke_syscall =3D> el0_svc_common.constprop.0 =3D> do_el0_svc =3D> el0_svc =3D> el0t_64_sync_handler =3D> el0t_64_sync The backtrace clearly shows the function arm_spe_pmu_start() is invoked in the 'dd' process (dd-7697); the flow is: - perf program sends IPI to CPU0; - 'dd' process is running on CPU0 and it's interrupted to handle IPI; - 'dd' process has root capabilities, so it will enable context tracing for non-root perf session. > >> One way to fix this is by caching the value of the CX bit during the > >> initialization of the PMU event, so that it remains consistent for the > >> duration of the session. > >> > >> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.gi= t/tree/drivers/perf/arm_spe_pmu.c?h=3Dv5.16#n275 > >> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.gi= t/tree/drivers/perf/arm_spe_pmu.c?h=3Dv5.16#n713 > >> [3]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.gi= t/tree/drivers/perf/arm_spe_pmu.c?h=3Dv5.16#n751 > >> > >> Signed-off-by: German Gomez > >> --- > >> drivers/perf/arm_spe_pmu.c | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c > >> index d44bcc29d..8515bf85c 100644 > >> --- a/drivers/perf/arm_spe_pmu.c > >> +++ b/drivers/perf/arm_spe_pmu.c > >> @@ -57,6 +57,7 @@ struct arm_spe_pmu { > >> u16 pmsver; > >> u16 min_period; > >> u16 counter_sz; > >> + bool pmscr_cx; So the patch makes sense to me. Just a minor comment: Here we can define a u64 for recording pmscr value rather than a bool value. struct arm_spe_pmu { ... u64 pmscr; }; > >> = > >> #define SPE_PMU_FEAT_FILT_EVT (1UL << 0) > >> #define SPE_PMU_FEAT_FILT_TYP (1UL << 1) > >> @@ -260,6 +261,7 @@ static const struct attribute_group *arm_spe_pmu_a= ttr_groups[] =3D { > >> static u64 arm_spe_event_to_pmscr(struct perf_event *event) > >> { > >> struct perf_event_attr *attr =3D &event->attr; > >> + struct arm_spe_pmu *spe_pmu =3D to_spe_pmu(event->pmu); > >> u64 reg =3D 0; > >> = > >> reg |=3D ATTR_CFG_GET_FLD(attr, ts_enable) << SYS_PMSCR_EL1_TS_SHIFT; > >> @@ -272,7 +274,7 @@ static u64 arm_spe_event_to_pmscr(struct perf_even= t *event) > >> if (!attr->exclude_kernel) > >> reg |=3D BIT(SYS_PMSCR_EL1_E1SPE_SHIFT); > >> = > >> - if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && perfmon_capable()) > >> + if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && spe_pmu->pmscr_cx) > >> reg |=3D BIT(SYS_PMSCR_EL1_CX_SHIFT); > >> = > >> return reg; > >> @@ -709,10 +711,10 @@ static int arm_spe_pmu_event_init(struct perf_ev= ent *event) > >> !(spe_pmu->features & SPE_PMU_FEAT_FILT_LAT)) > >> return -EOPNOTSUPP; > >> = > >> + spe_pmu->pmscr_cx =3D perfmon_capable(); > >> reg =3D arm_spe_event_to_pmscr(event); Thus here we can change as: spe_pmu->pmscr =3D arm_spe_event_to_pmscr(event); And then in the function arm_spe_pmu_start(), we can skip calling arm_spe_event_to_pmscr() and directly set PMSCR register: static void arm_spe_pmu_start(struct perf_event *event, int flags) { ... isb(); write_sysreg_s(spe_pmu->pmscr, SYS_PMSCR_EL1); } Thanks, Leo > >> if (!perfmon_capable() && > >> (reg & (BIT(SYS_PMSCR_EL1_PA_SHIFT) | > >> - BIT(SYS_PMSCR_EL1_CX_SHIFT) | > >> BIT(SYS_PMSCR_EL1_PCT_SHIFT)))) > >> return -EACCES; > >> = > >> -- = > >> 2.25.1 > >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel