A subsequent patch alters perf to perf_event_open with the
PERF_SAMPLE_ARCH_1 bit on.
This patch deals with the case where the kernel does not know about the
PERF_SAMPLE_ARCH_1 bit, and does not know to send the pointer
authentication masks. In this case the perf_event_open system call
returns -EINVAL (-22) and perf exits with an error.
This patch causes userspace process to re-attempt the perf_event_open
system call but without asking for the PERF_SAMPLE_ARCH_1 sample
field, allowing the perf_event_open system call to succeed.
The check is placed to disable PERF_SAMPLE_ARCH_1 as the first thing
to do in the try_fallback section of evsel__open_cpu() because it's the
most recent sample field added, so should probably be the first thing to
attempt to turn off.
Signed-off-by: Andrew Kilroy <[email protected]>
---
tools/include/uapi/linux/perf_event.h | 5 ++++-
tools/perf/util/evsel.c | 19 +++++++++++++++++++
tools/perf/util/evsel.h | 1 +
tools/perf/util/perf_event_attr_fprintf.c | 2 +-
4 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index d37629dbad72..821bf5ff6a19 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -162,12 +162,15 @@ enum perf_event_sample_format {
PERF_SAMPLE_DATA_PAGE_SIZE = 1U << 22,
PERF_SAMPLE_CODE_PAGE_SIZE = 1U << 23,
PERF_SAMPLE_WEIGHT_STRUCT = 1U << 24,
+ PERF_SAMPLE_ARCH_1 = 1U << 25,
- PERF_SAMPLE_MAX = 1U << 25, /* non-ABI */
+ PERF_SAMPLE_MAX = 1U << 26, /* non-ABI */
__PERF_SAMPLE_CALLCHAIN_EARLY = 1ULL << 63, /* non-ABI; internal use */
};
+#define PERF_SAMPLE_ARM64_PTRAUTH PERF_SAMPLE_ARCH_1
+
#define PERF_SAMPLE_WEIGHT_TYPE (PERF_SAMPLE_WEIGHT | PERF_SAMPLE_WEIGHT_STRUCT)
/*
* values to program into branch_sample_type when PERF_SAMPLE_BRANCH is set
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index ce499c5da8d7..25d8f804f49a 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1830,6 +1830,8 @@ static int __evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
static void evsel__disable_missing_features(struct evsel *evsel)
{
+ if (perf_missing_features.ptrauth)
+ evsel__reset_sample_bit(evsel, ARM64_PTRAUTH);
if (perf_missing_features.weight_struct) {
evsel__set_sample_bit(evsel, WEIGHT);
evsel__reset_sample_bit(evsel, WEIGHT_STRUCT);
@@ -1875,6 +1877,20 @@ int evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
return err;
}
+static bool evsel__detect_ptrauth_masks_missing(struct evsel *evsel __maybe_unused)
+{
+#if defined(__aarch64__)
+ if (!perf_missing_features.ptrauth &&
+ (evsel->core.attr.sample_type & PERF_SAMPLE_ARM64_PTRAUTH)) {
+ perf_missing_features.ptrauth = true;
+ pr_debug2_peo("switching off request for pointer authentication masks\n");
+ return true;
+ }
+#endif
+
+ return false;
+}
+
bool evsel__detect_missing_features(struct evsel *evsel)
{
/*
@@ -2114,6 +2130,9 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
return 0;
try_fallback:
+ if (evsel__detect_ptrauth_masks_missing(evsel))
+ goto fallback_missing_features;
+
if (evsel__precise_ip_fallback(evsel))
goto retry_open;
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 73ea48e94079..9690c35088bf 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -188,6 +188,7 @@ struct perf_missing_features {
bool data_page_size;
bool code_page_size;
bool weight_struct;
+ bool ptrauth;
};
extern struct perf_missing_features perf_missing_features;
diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
index 98af3fa4ea35..d18d5f2c6891 100644
--- a/tools/perf/util/perf_event_attr_fprintf.c
+++ b/tools/perf/util/perf_event_attr_fprintf.c
@@ -36,7 +36,7 @@ static void __p_sample_type(char *buf, size_t size, u64 value)
bit_name(IDENTIFIER), bit_name(REGS_INTR), bit_name(DATA_SRC),
bit_name(WEIGHT), bit_name(PHYS_ADDR), bit_name(AUX),
bit_name(CGROUP), bit_name(DATA_PAGE_SIZE), bit_name(CODE_PAGE_SIZE),
- bit_name(WEIGHT_STRUCT),
+ bit_name(WEIGHT_STRUCT), bit_name(ARCH_1),
{ .name = NULL, }
};
#undef bit_name
--
2.17.1
On Mon, 4 Jul 2022, Andrew Kilroy wrote:
> A subsequent patch alters perf to perf_event_open with the
> PERF_SAMPLE_ARCH_1 bit on.
>
> This patch deals with the case where the kernel does not know about the
> PERF_SAMPLE_ARCH_1 bit, and does not know to send the pointer
> authentication masks. In this case the perf_event_open system call
> returns -EINVAL (-22) and perf exits with an error.
>
> This patch causes userspace process to re-attempt the perf_event_open
> system call but without asking for the PERF_SAMPLE_ARCH_1 sample
> field, allowing the perf_event_open system call to succeed.
So in this case you are leaking ARM64-specific info into the generic
perf_event_open() call? Is there any way the kernel could implement this
without userspace having to deal with the issue?
There are a few recent ARM64 perf_event related patches that are pushing
ARM specific interfaces into the generic code, with the apparent
assumption that it will just be implemented in the userspace perf tool.
However there are a number of outside-the-kernel codebases that also use
perf_event_open() and it seems a bit onerous if all of them have to start
adding a lot of extra ARM64-specific code, especially because as far as I
can tell there haven't been any documentation patches included for the
Makefile.
The other recent change that's annoying for userspace is the addition of
the ARM-specific /proc/sys/kernel/perf_user_access that duplicates
functionality found in /sys/devices/cpu/rdpmc
Vince Weaver
[email protected]
On 06/07/2022 17:01, Vince Weaver wrote:
> On Mon, 4 Jul 2022, Andrew Kilroy wrote:
>
>> A subsequent patch alters perf to perf_event_open with the
>> PERF_SAMPLE_ARCH_1 bit on.
>>
>> This patch deals with the case where the kernel does not know about the
>> PERF_SAMPLE_ARCH_1 bit, and does not know to send the pointer
>> authentication masks. In this case the perf_event_open system call
>> returns -EINVAL (-22) and perf exits with an error.
>>
>> This patch causes userspace process to re-attempt the perf_event_open
>> system call but without asking for the PERF_SAMPLE_ARCH_1 sample
>> field, allowing the perf_event_open system call to succeed.
>
> So in this case you are leaking ARM64-specific info into the generic
> perf_event_open() call? Is there any way the kernel could implement this
> without userspace having to deal with the issue?
Hi Vince,
The alternative to this change is just to call it "PERF_SAMPLE_POINTER_AUTH_MASK"
and then it's not Arm specific, it's just that only Arm implements it for now.
This is definitely an option.
But if no platform ever implements something similar then that bit is wasted.
The intention of adding "PERF_SAMPLE_ARCH_1" was to prevent wasting that bit.
But as you say, maybe making it arch specific isn't the right way either.
I wouldn't say the perf_event_open call is currently generic though, lots of
it already requires knowledge of the current platform, and searching for 'x86'
in the docs for it gives 10 matches.
>
> There are a few recent ARM64 perf_event related patches that are pushing
> ARM specific interfaces into the generic code, with the apparent
> assumption that it will just be implemented in the userspace perf tool.
> However there are a number of outside-the-kernel codebases that also use
> perf_event_open() and it seems a bit onerous if all of them have to start
> adding a lot of extra ARM64-specific code, especially because as far as I
Because pointer auth is a hardware feature, other tools have no choice but
to implement this if they do Dwarf based stack unwinding. There is no way
around that. The pointers are stored mangled and they don't make sense
without masking them. GDB has already implemented support for it. If they
don't do Dwarf based stack unwinding then they can carry on as they are
and everything will still work.
> can tell there haven't been any documentation patches included for the
> Makefile.
We plan to update the docs for the syscall, but it's in another repo, and
we'll wait for this change to be finalised first. I'm not sure what you
mean about the Makefile?
Thanks
James
>
> The other recent change that's annoying for userspace is the addition of
> the ARM-specific /proc/sys/kernel/perf_user_access that duplicates
> functionality found in /sys/devices/cpu/rdpmc
>
> Vince Weaver
> [email protected]
On Mon, 11 Jul 2022, James Clark wrote:
> On 06/07/2022 17:01, Vince Weaver wrote:
> > So in this case you are leaking ARM64-specific info into the generic
> > perf_event_open() call? Is there any way the kernel could implement this
> > without userspace having to deal with the issue?
>
> The alternative to this change is just to call it "PERF_SAMPLE_POINTER_AUTH_MASK"
> and then it's not Arm specific, it's just that only Arm implements it for now.
> This is definitely an option.
>
> But if no platform ever implements something similar then that bit is wasted.
> The intention of adding "PERF_SAMPLE_ARCH_1" was to prevent wasting that bit.
> But as you say, maybe making it arch specific isn't the right way either.
I don't know what the current kernel policy is on this kind of thing, but
in the past perf_event_open was meant to be a generic as possible.
Having architecture-specific magic bits is best avoided.
However I'm not the maintainer for this so really my opinion doesn't
really matter.
I'm just speaking up as a userspace coder who is trying to write
cross-platform tools, and having to maintain obscure arch-specific code
paths in every single utility ends up being a huge pain. And isn't the
whole point of an operating system to abstract this away?
> > can tell there haven't been any documentation patches included for the
> > Makefile.
>
> We plan to update the docs for the syscall, but it's in another repo, and
> we'll wait for this change to be finalised first. I'm not sure what you
> mean about the Makefile?
sorry, that was a mis-type. I meant "manpage" not Makefile.
Vince Weaver
[email protected]