From: Will Deacon <will@kernel.org> To: Rob Herring <robh@kernel.org> Cc: Catalin Marinas <catalin.marinas@arm.com>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Jiri Olsa <jolsa@redhat.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Namhyung Kim <namhyung@kernel.org>, Raphael Gault <raphael.gault@arm.com>, Mark Rutland <mark.rutland@arm.com>, Jonathan Cameron <Jonathan.Cameron@huawei.com>, Ian Rogers <irogers@google.com>, Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Subject: Re: [PATCH v3 01/10] arm64: pmu: Add hook to handle pmu-related undefined instructions Date: Tue, 29 Sep 2020 18:49:34 +0100 [thread overview] Message-ID: <20200929174933.GF14317@willie-the-truck> (raw) In-Reply-To: <CAL_Jsq+pNrqBN9TghXxPi2kT_T=pfMXf89diGXqo-RauuLYRuA@mail.gmail.com> On Tue, Sep 29, 2020 at 08:46:46AM -0500, Rob Herring wrote: > On Mon, Sep 28, 2020 at 1:26 PM Will Deacon <will@kernel.org> wrote: > > On Fri, Sep 11, 2020 at 03:51:09PM -0600, Rob Herring wrote: > > > +static int emulate_pmu(struct pt_regs *regs, u32 insn) > > > +{ > > > + u32 rt; > > > + u32 pmuserenr; > > > + > > > + rt = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn); > > > + pmuserenr = read_sysreg(pmuserenr_el0); > > > + > > > + if ((pmuserenr & (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR)) != > > > + (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR)) > > > + return -EINVAL; > > > + > > > + > > > + /* > > > + * Userspace is expected to only use this in the context of the scheme > > > + * described in the struct perf_event_mmap_page comments. > > > + * > > > + * Given that context, we can only get here if we got migrated between > > > + * getting the register index and doing the MSR read. This in turn > > > + * implies we'll fail the sequence and retry, so any value returned is > > > + * 'good', all we need is to be non-fatal. > > > + * > > > + * The choice of the value 0 is comming from the fact that when > > > + * accessing a register which is not counting events but is accessible, > > > + * we get 0. > > > + */ > > > + pt_regs_write_reg(regs, rt, 0); > > > > Hmm... this feels pretty fragile since, although we may expect userspace only > > to trigger this in the context of the specific perf use-case, we don't have > > a way to detect that, so the ABI we're exposing is that EL0 accesses to > > non-existent counters will return 0. I don't really think that's something > > we want to commit to. > > > > When restartable sequences were added to the kernel, one of the proposed > > use-cases was to allow PMU access on big/little systems, because the > > sequence will abort on preemption. Taking that approach removes the need > > for this emulation hook entirely. Is that something we can rely on instead > > of this emulation hook? > > So back to the RFC version[1]!? That would mean pulling librseq into > the kernel based on the prior discussion. It doesn't look like that > has happened yet. Yeah, or just don't bother supporting heterogeneous systems with this for now. > Why not just drop the undef hook? For heterogeneous systems, we > require userspace to pin itself to cores for a specific PMU. See patch > 9. If userspace fails to do that, then it gets to keep the pieces. Dropping it works too! Will
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will@kernel.org> To: Rob Herring <robh@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com>, Ian Rogers <irogers@google.com>, Peter Zijlstra <peterz@infradead.org>, Catalin Marinas <catalin.marinas@arm.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Arnaldo Carvalho de Melo <acme@kernel.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Raphael Gault <raphael.gault@arm.com>, Ingo Molnar <mingo@redhat.com>, Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>, Jonathan Cameron <Jonathan.Cameron@huawei.com>, Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@redhat.com>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v3 01/10] arm64: pmu: Add hook to handle pmu-related undefined instructions Date: Tue, 29 Sep 2020 18:49:34 +0100 [thread overview] Message-ID: <20200929174933.GF14317@willie-the-truck> (raw) In-Reply-To: <CAL_Jsq+pNrqBN9TghXxPi2kT_T=pfMXf89diGXqo-RauuLYRuA@mail.gmail.com> On Tue, Sep 29, 2020 at 08:46:46AM -0500, Rob Herring wrote: > On Mon, Sep 28, 2020 at 1:26 PM Will Deacon <will@kernel.org> wrote: > > On Fri, Sep 11, 2020 at 03:51:09PM -0600, Rob Herring wrote: > > > +static int emulate_pmu(struct pt_regs *regs, u32 insn) > > > +{ > > > + u32 rt; > > > + u32 pmuserenr; > > > + > > > + rt = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn); > > > + pmuserenr = read_sysreg(pmuserenr_el0); > > > + > > > + if ((pmuserenr & (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR)) != > > > + (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR)) > > > + return -EINVAL; > > > + > > > + > > > + /* > > > + * Userspace is expected to only use this in the context of the scheme > > > + * described in the struct perf_event_mmap_page comments. > > > + * > > > + * Given that context, we can only get here if we got migrated between > > > + * getting the register index and doing the MSR read. This in turn > > > + * implies we'll fail the sequence and retry, so any value returned is > > > + * 'good', all we need is to be non-fatal. > > > + * > > > + * The choice of the value 0 is comming from the fact that when > > > + * accessing a register which is not counting events but is accessible, > > > + * we get 0. > > > + */ > > > + pt_regs_write_reg(regs, rt, 0); > > > > Hmm... this feels pretty fragile since, although we may expect userspace only > > to trigger this in the context of the specific perf use-case, we don't have > > a way to detect that, so the ABI we're exposing is that EL0 accesses to > > non-existent counters will return 0. I don't really think that's something > > we want to commit to. > > > > When restartable sequences were added to the kernel, one of the proposed > > use-cases was to allow PMU access on big/little systems, because the > > sequence will abort on preemption. Taking that approach removes the need > > for this emulation hook entirely. Is that something we can rely on instead > > of this emulation hook? > > So back to the RFC version[1]!? That would mean pulling librseq into > the kernel based on the prior discussion. It doesn't look like that > has happened yet. Yeah, or just don't bother supporting heterogeneous systems with this for now. > Why not just drop the undef hook? For heterogeneous systems, we > require userspace to pin itself to cores for a specific PMU. See patch > 9. If userspace fails to do that, then it gets to keep the pieces. Dropping it works too! Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-09-29 17:49 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-11 21:51 [PATCH v3 0/10] libperf and arm64 userspace counter access support Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-11 21:51 ` [PATCH v3 01/10] arm64: pmu: Add hook to handle pmu-related undefined instructions Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-28 18:26 ` Will Deacon 2020-09-28 18:26 ` Will Deacon 2020-09-29 13:46 ` Rob Herring 2020-09-29 13:46 ` Rob Herring 2020-09-29 17:49 ` Will Deacon [this message] 2020-09-29 17:49 ` Will Deacon 2020-09-29 20:46 ` Rob Herring 2020-09-29 20:46 ` Rob Herring 2020-09-11 21:51 ` [PATCH v3 02/10] arm64: pmu: Add function implementation to update event index in userpage Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-11 21:51 ` [PATCH v3 03/10] arm64: perf: Enable pmu counter direct access for perf event on armv8 Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-11 21:51 ` [PATCH v3 04/10] tools/include: Add an initial math64.h Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-11 21:51 ` [PATCH v3 05/10] libperf: Add libperf_evsel__mmap() Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-18 14:33 ` Jiri Olsa 2020-09-18 14:33 ` Jiri Olsa 2020-09-22 15:28 ` Rob Herring 2020-09-22 15:28 ` Rob Herring 2020-09-22 18:32 ` Jiri Olsa 2020-09-22 18:32 ` Jiri Olsa 2020-09-11 21:51 ` [PATCH v3 06/10] libperf: tests: Add support for verbose printing Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-11 21:51 ` [PATCH v3 07/10] libperf: Add support for user space counter access Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-11 21:51 ` [PATCH v3 08/10] libperf: Add arm64 support to perf_mmap__read_self() Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-11 21:51 ` [PATCH v3 09/10] perf: arm64: Add test for userspace counter access on heterogeneous systems Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-11 21:51 ` [PATCH v3 10/10] Documentation: arm64: Document PMU counters access from userspace Rob Herring 2020-09-11 21:51 ` Rob Herring 2020-09-12 20:53 ` [PATCH v3 0/10] libperf and arm64 userspace counter access support Jiri Olsa 2020-09-12 20:53 ` Jiri Olsa 2020-09-14 14:21 ` Rob Herring 2020-09-14 14:21 ` Rob Herring [not found] ` <CANW9uytmafiNb_8oua9QY7L9O5BQTBFQBOMS3ZgjQ7aWj8CD2Q@mail.gmail.com> 2020-09-16 2:50 ` Rob Herring 2020-09-16 2:50 ` Rob Herring 2020-09-19 7:22 ` Itaru Kitayama 2020-09-19 7:22 ` Itaru Kitayama 2020-09-22 15:23 ` Rob Herring 2020-09-22 15:23 ` Rob Herring
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200929174933.GF14317@willie-the-truck \ --to=will@kernel.org \ --cc=Jonathan.Cameron@huawei.com \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=catalin.marinas@arm.com \ --cc=honnappa.nagarahalli@arm.com \ --cc=irogers@google.com \ --cc=jolsa@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mingo@redhat.com \ --cc=namhyung@kernel.org \ --cc=peterz@infradead.org \ --cc=raphael.gault@arm.com \ --cc=robh@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.