[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210601125703.GC28025@willie-the-truck>
Date: Tue, 1 Jun 2021 13:57:03 +0100
From: Will Deacon <will@...nel.org>
To: Rob Herring <robh@...nel.org>
Cc: Catalin Marinas <catalin.marinas@....com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Jiri Olsa <jolsa@...hat.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Ian Rogers <irogers@...gle.com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
honnappa.nagarahalli@....com, Zachary.Leaf@....com,
Raphael Gault <raphael.gault@....com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Namhyung Kim <namhyung@...nel.org>,
Itaru Kitayama <itaru.kitayama@...il.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 4/5] arm64: perf: Add userspace counter access disable
switch
Hi Rob,
On Mon, May 17, 2021 at 02:54:04PM -0500, Rob Herring wrote:
> Like x86, some users may want to disable userspace PMU counter
> altogether. Add a sysfs 'rdpmc' file to control userspace counter
> access. The default is '1' which is enabled. Writing '0' disables
> access.
>
> In the case of multiple PMUs (i.e. big.LITTLE), the control is per PMU
> and userspace must disable access on each PMU.
>
> Note that x86 also supports writing '2' to globally enable user access.
> As there's not existing userspace support to worry about, this shouldn't
> be necessary for Arm. It could be added later if the need arises.
>
> Signed-off-by: Rob Herring <robh@...nel.org>
> ---
> v8:
> - Adjust due to patch 3 changes
> v7:
> - New patch
> ---
> arch/arm64/kernel/perf_event.c | 64 ++++++++++++++++++++++++++++++++--
> include/linux/perf/arm_pmu.h | 4 ++-
> 2 files changed, 65 insertions(+), 3 deletions(-)
I understand you've tried to follow the x86 behaviour here, but I think it
might be better to implement this as a sysctl on arm64, with the default
behaviour being that userspace access is _disabled_. Having the attribute
per-PMU doesn't really make a lot of sense to me and we don't have any
compatibility issues to worry about given that we've not exposed this to
userspace yet.
That should also be straightforward to implement (famous last words... yell
if it isn't).
Will
Powered by blists - more mailing lists