* [PATCH 0/3] perf: precise mode and exclude_guest
@ 2012-09-10 16:40 David Ahern
2012-09-10 16:40 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: David Ahern @ 2012-09-10 16:40 UTC (permalink / raw)
To: acme, linux-kernel, peterz; +Cc: David Ahern
Hopefully thi wraps up the precise mode-exclude_guest dependency.
I'm sure someone will let me know if I screwed up the attribution
in the second patch.
David Ahern (3):
perf tool: precise mode requires exclude_guest
perf: require exclude_guest to use PEBS - kernel side enforcement
perf tool: give user better message if precise is not supported
arch/x86/kernel/cpu/perf_event.c | 6 ++++++
tools/perf/builtin-record.c | 5 +++++
tools/perf/builtin-top.c | 4 ++++
tools/perf/util/parse-events.c | 3 +++
4 files changed, 18 insertions(+)
--
1.7.10.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] perf tool: precise mode requires exclude_guest
2012-09-10 16:40 [PATCH 0/3] perf: precise mode and exclude_guest David Ahern
@ 2012-09-10 16:40 ` David Ahern
2012-09-10 17:23 ` Peter Zijlstra
2012-09-10 16:40 ` [PATCH 2/3] perf: require exclude_guest to use PEBS - kernel side enforcement David Ahern
` (2 subsequent siblings)
3 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2012-09-10 16:40 UTC (permalink / raw)
To: acme, linux-kernel, peterz
Cc: David Ahern, Ingo Molnar, Robert Richter, Gleb Natapov, Avi Kivity
PEBS cannot be used with guest mode. See:
https://lkml.org/lkml/2012/7/9/264
If user adds :p modifier set exclude_guest as well.
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Robert Richter <robert.richter@amd.com>
Cc: Gleb Natapov <gleb@redhat.com>
Cc: Avi Kivity <avi@redhat.com>
---
tools/perf/util/parse-events.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index a031ee1..f2c02b2 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -694,6 +694,9 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
eH = 0;
} else if (*str == 'p') {
precise++;
+ /* use of precise requires exclude_guest */
+ if (!exclude_GH)
+ eG = 1;
} else
break;
--
1.7.10.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] perf tool: precise mode requires exclude_guest
2012-09-10 16:40 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
@ 2012-09-10 17:23 ` Peter Zijlstra
0 siblings, 0 replies; 20+ messages in thread
From: Peter Zijlstra @ 2012-09-10 17:23 UTC (permalink / raw)
To: David Ahern
Cc: acme, linux-kernel, Ingo Molnar, Robert Richter, Gleb Natapov,
Avi Kivity
On Mon, 2012-09-10 at 10:40 -0600, David Ahern wrote:
> PEBS cannot be used with guest mode. See:
> https://lkml.org/lkml/2012/7/9/264
Expanding and not relying on external stuff is so much better.
So in particular you want something like:
"Intel PEBS in VT-x context uses the DS address as a guest linear
address, even though its programmed by the host as a host linear
address. This either results in guest memory corruption and or the
hardware faulting and 'crashing' the virtual machine
Therefore we have to disable PEBS on VT-x enter and re-enable on VT-x
exit, enforcing a strict exclude_guest.
AMB IBS does work but doesn't currently support exclude_* at all,
setting an exclude_* bit will make it fail.
... explain how one can use IBS if we auto-fallback to !precise ..."
Hmm ?
> If user adds :p modifier set exclude_guest as well.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Robert Richter <robert.richter@amd.com>
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Avi Kivity <avi@redhat.com>
> ---
> tools/perf/util/parse-events.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index a031ee1..f2c02b2 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -694,6 +694,9 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
> eH = 0;
> } else if (*str == 'p') {
> precise++;
> + /* use of precise requires exclude_guest */
> + if (!exclude_GH)
> + eG = 1;
> } else
> break;
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] perf: require exclude_guest to use PEBS - kernel side enforcement
2012-09-10 16:40 [PATCH 0/3] perf: precise mode and exclude_guest David Ahern
2012-09-10 16:40 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
@ 2012-09-10 16:40 ` David Ahern
2012-09-10 17:24 ` Peter Zijlstra
2012-09-10 16:40 ` [PATCH 3/3] perf tool: give user better message if precise is not supported David Ahern
2012-09-10 16:57 ` [PATCH 0/3] perf: precise mode and exclude_guest Peter Zijlstra
3 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2012-09-10 16:40 UTC (permalink / raw)
To: acme, linux-kernel, peterz
Cc: David Ahern, Ingo Molnar, Robert Richter, Gleb Natapov, Avi Kivity
From: Peter Zijlstra <peterz@infradead.org>
See https://lkml.org/lkml/2012/7/9/298
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Robert Richter <robert.richter@amd.com>
Cc: Gleb Natapov <gleb@redhat.com>
Cc: Avi Kivity <avi@redhat.com>
---
arch/x86/kernel/cpu/perf_event.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 915b876..4bc96c5 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -338,6 +338,9 @@ int x86_setup_perfctr(struct perf_event *event)
/* BTS is currently only allowed for user-mode. */
if (!attr->exclude_kernel)
return -EOPNOTSUPP;
+
+ if (!attr->exclude_guest)
+ return -EOPNOTSUPP;
}
hwc->config |= config;
@@ -380,6 +383,9 @@ int x86_pmu_hw_config(struct perf_event *event)
if (event->attr.precise_ip) {
int precise = 0;
+ if (!event->attr.exclude_guest)
+ return -EOPNOTSUPP;
+
/* Support for constant skid */
if (x86_pmu.pebs_active && !x86_pmu.pebs_broken) {
precise++;
--
1.7.10.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] perf tool: give user better message if precise is not supported
2012-09-10 16:40 [PATCH 0/3] perf: precise mode and exclude_guest David Ahern
2012-09-10 16:40 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
2012-09-10 16:40 ` [PATCH 2/3] perf: require exclude_guest to use PEBS - kernel side enforcement David Ahern
@ 2012-09-10 16:40 ` David Ahern
2012-09-11 9:20 ` Robert Richter
2012-09-10 16:57 ` [PATCH 0/3] perf: precise mode and exclude_guest Peter Zijlstra
3 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2012-09-10 16:40 UTC (permalink / raw)
To: acme, linux-kernel, peterz; +Cc: David Ahern, Ingo Molnar, Robert Richter
Platforms (e.g., VM's) without support for precise mode get a confusing
error message. e.g.,
$ perf record -e cycles:p -a -- sleep 1
Error: sys_perf_event_open() syscall returned with 95 (Operation not
supported). /bin/dmesg may provide additional information.
No hardware sampling interrupt available. No APIC? If so then you can
boot the kernel with the "lapic" boot parameter to force-enable it.
sleep: Terminated
which is not clear that precise mode might be the root problem. With this
patch:
$ perf record -e cycles:p -fo /tmp/perf.data -- sleep 1
Error:
'precise' request not supported. Try removing 'p' modifier
sleep: Terminated
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Robert Richter <robert.richter@amd.com>
---
tools/perf/builtin-record.c | 5 +++++
tools/perf/builtin-top.c | 4 ++++
2 files changed, 9 insertions(+)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 7b8b891..ccf529b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -294,6 +294,11 @@ try_again:
perf_evsel__name(pos));
rc = -err;
goto out;
+ } else if ((err == ENOTSUP) && (attr->precise_ip)) {
+ ui__error("\'precise\' request not supported. "
+ "Try removing 'p' modifier\n");
+ rc = -err;
+ goto out;
}
printf("\n");
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0513aaa..0d3653b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -975,6 +975,10 @@ try_again:
ui__error("Too many events are opened.\n"
"Try again after reducing the number of events\n");
goto out_err;
+ } else if ((err == ENOTSUP) && (attr->precise_ip)) {
+ ui__error("\'precise\' request not supported. "
+ "Try removing 'p' modifier\n");
+ goto out_err;
}
ui__error("The sys_perf_event_open() syscall "
--
1.7.10.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] perf tool: give user better message if precise is not supported
2012-09-10 16:40 ` [PATCH 3/3] perf tool: give user better message if precise is not supported David Ahern
@ 2012-09-11 9:20 ` Robert Richter
2012-09-11 13:22 ` David Ahern
0 siblings, 1 reply; 20+ messages in thread
From: Robert Richter @ 2012-09-11 9:20 UTC (permalink / raw)
To: David Ahern; +Cc: acme, linux-kernel, peterz, Ingo Molnar
On 10.09.12 10:40:16, David Ahern wrote:
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -294,6 +294,11 @@ try_again:
> perf_evsel__name(pos));
> rc = -err;
> goto out;
> + } else if ((err == ENOTSUP) && (attr->precise_ip)) {
It is EOPNOTSUPP, did you test this?
> + ui__error("\'precise\' request not supported. "
> + "Try removing 'p' modifier\n");
I would better print "... request may not be supported.", since you
don't know for sure if this is the real cause.
> + rc = -err;
> + goto out;
> }
>
> printf("\n");
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 0513aaa..0d3653b 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -975,6 +975,10 @@ try_again:
> ui__error("Too many events are opened.\n"
> "Try again after reducing the number of events\n");
> goto out_err;
> + } else if ((err == ENOTSUP) && (attr->precise_ip)) {
> + ui__error("\'precise\' request not supported. "
> + "Try removing 'p' modifier\n");
Same here.
> + goto out_err;
To avoid adding more duplicate code, maybe we should start to unify
the code by implementing this in a shared helper function.
-Robert
--
Advanced Micro Devices, Inc.
Operating System Research Center
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] perf tool: give user better message if precise is not supported
2012-09-11 9:20 ` Robert Richter
@ 2012-09-11 13:22 ` David Ahern
2012-09-11 14:01 ` Robert Richter
0 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2012-09-11 13:22 UTC (permalink / raw)
To: Robert Richter; +Cc: acme, linux-kernel, peterz, Ingo Molnar
On 9/11/12 3:20 AM, Robert Richter wrote:
> On 10.09.12 10:40:16, David Ahern wrote:
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -294,6 +294,11 @@ try_again:
>> perf_evsel__name(pos));
>> rc = -err;
>> goto out;
>> + } else if ((err == ENOTSUP) && (attr->precise_ip)) {
>
> It is EOPNOTSUPP, did you test this?
I do not post patches without testing them. This particular patch was
verified in a Virtual Machine (no PEBS) and using :pG modifier.
'egrep -r ENOTSUP tools/perf' shows hits in 3 other files, so I am not
the only one using the shortcut. I'll change it in the follow up with
better commit messages to make it consistent with patch 2.
>
>> + ui__error("\'precise\' request not supported. "
>> + "Try removing 'p' modifier\n");
>
> I would better print "... request may not be supported.", since you
> don't know for sure if this is the real cause.
Sure I'll add the 'may not be'.
>
>> + rc = -err;
>> + goto out;
>> }
>>
>> printf("\n");
>> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
>> index 0513aaa..0d3653b 100644
>> --- a/tools/perf/builtin-top.c
>> +++ b/tools/perf/builtin-top.c
>> @@ -975,6 +975,10 @@ try_again:
>> ui__error("Too many events are opened.\n"
>> "Try again after reducing the number of events\n");
>> goto out_err;
>> + } else if ((err == ENOTSUP) && (attr->precise_ip)) {
>> + ui__error("\'precise\' request not supported. "
>> + "Try removing 'p' modifier\n");
>
> Same here.
>
>> + goto out_err;
>
> To avoid adding more duplicate code, maybe we should start to unify
> the code by implementing this in a shared helper function.
Doing that requires additional modifications to not break perl and
python scripts. Adding it in both commands here is consistent with all
other open counter failures. Consolidation of those loops into a common
base is known to do item.
David
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] perf tool: give user better message if precise is not supported
2012-09-11 13:22 ` David Ahern
@ 2012-09-11 14:01 ` Robert Richter
2012-09-11 14:32 ` David Ahern
0 siblings, 1 reply; 20+ messages in thread
From: Robert Richter @ 2012-09-11 14:01 UTC (permalink / raw)
To: David Ahern; +Cc: acme, linux-kernel, peterz, Ingo Molnar
On 11.09.12 07:22:32, David Ahern wrote:
> On 9/11/12 3:20 AM, Robert Richter wrote:
> > On 10.09.12 10:40:16, David Ahern wrote:
> >> --- a/tools/perf/builtin-record.c
> >> +++ b/tools/perf/builtin-record.c
> >> @@ -294,6 +294,11 @@ try_again:
> >> perf_evsel__name(pos));
> >> rc = -err;
> >> goto out;
> >> + } else if ((err == ENOTSUP) && (attr->precise_ip)) {
> >
> > It is EOPNOTSUPP, did you test this?
Ok, wrong question. Better would have been: Did you run it on a
non-pebs Intel machine of an non-ibs AMD machine?
> I do not post patches without testing them. This particular patch was
> verified in a Virtual Machine (no PEBS) and using :pG modifier.
>
> 'egrep -r ENOTSUP tools/perf' shows hits in 3 other files, so I am not
> the only one using the shortcut. I'll change it in the follow up with
> better commit messages to make it consistent with patch 2.
For VM this might be valid. Don't know where ENOTSUP comes from. It is
neither in kernel/events/ nor arch/x86/kernel/cpu/perf.
If you run this bare-metal on older machines which do not support pebs
or ibs, the syscall returns EOPNOTSUPP. You can trigger the same
behaviour on newer systems with:
# perf record -e cycles:ppp -c 2097120 -R -a sleep 1
Error: sys_perf_event_open() syscall returned with 95 (Operation not supported). /bin/dmesg may provide additional information.
...
It should work in this case too.
> > To avoid adding more duplicate code, maybe we should start to unify
> > the code by implementing this in a shared helper function.
>
> Doing that requires additional modifications to not break perl and
> python scripts. Adding it in both commands here is consistent with all
> other open counter failures. Consolidation of those loops into a common
> base is known to do item.
I am fine with that too.
-Robert
--
Advanced Micro Devices, Inc.
Operating System Research Center
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] perf tool: give user better message if precise is not supported
2012-09-11 14:01 ` Robert Richter
@ 2012-09-11 14:32 ` David Ahern
2012-09-11 15:11 ` Robert Richter
0 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2012-09-11 14:32 UTC (permalink / raw)
To: Robert Richter; +Cc: acme, linux-kernel, peterz, Ingo Molnar
On 9/11/12 8:01 AM, Robert Richter wrote:
>
> Ok, wrong question. Better would have been: Did you run it on a
> non-pebs Intel machine of an non-ibs AMD machine?
Intel: yes. VM for example. All the servers I have now are Nehalem or
better - ie., with a PEBS.
AMD: no. I do not have any AMD-based servers.
>
>> I do not post patches without testing them. This particular patch was
>> verified in a Virtual Machine (no PEBS) and using :pG modifier.
>>
>> 'egrep -r ENOTSUP tools/perf' shows hits in 3 other files, so I am not
>> the only one using the shortcut. I'll change it in the follow up with
>> better commit messages to make it consistent with patch 2.
>
> For VM this might be valid. Don't know where ENOTSUP comes from. It is
> neither in kernel/events/ nor arch/x86/kernel/cpu/perf.
My guess would be /usr/include/bits/errno.h:
/* Linux has no ENOTSUP error code. */
# define ENOTSUP EOPNOTSUPP
>
> If you run this bare-metal on older machines which do not support pebs
> or ibs, the syscall returns EOPNOTSUPP. You can trigger the same
> behaviour on newer systems with:
>
> # perf record -e cycles:ppp -c 2097120 -R -a sleep 1
>
> Error: sys_perf_event_open() syscall returned with 95 (Operation not supported). /bin/dmesg may provide additional information.
> ...
>
> It should work in this case too.
The commit message was a copy and paste from the failure of both :p in a
VM (PEBS is not supported in a VM). I also ran the bare metal case with
:pG which per the second patch in this series generates the not
supported message.
David
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] perf tool: give user better message if precise is not supported
2012-09-11 14:32 ` David Ahern
@ 2012-09-11 15:11 ` Robert Richter
2012-09-12 14:59 ` David Ahern
0 siblings, 1 reply; 20+ messages in thread
From: Robert Richter @ 2012-09-11 15:11 UTC (permalink / raw)
To: David Ahern; +Cc: acme, linux-kernel, peterz, Ingo Molnar
On 11.09.12 08:32:55, David Ahern wrote:
> My guess would be /usr/include/bits/errno.h:
>
> /* Linux has no ENOTSUP error code. */
> # define ENOTSUP EOPNOTSUPP
Ok, so ENOTSUP is actually the same as EOPNOTSUPP. Since the syscall
returns a EOPNOTSUPP, I prefer this when checking perf_event_open()
return codes. ENOTSUP is not used in the kernel. Was there a reason
for choosing ENOTSUP?
> > If you run this bare-metal on older machines which do not support pebs
> > or ibs, the syscall returns EOPNOTSUPP. You can trigger the same
> > behaviour on newer systems with:
> >
> > # perf record -e cycles:ppp -c 2097120 -R -a sleep 1
> >
> > Error: sys_perf_event_open() syscall returned with 95 (Operation not supported). /bin/dmesg may provide additional information.
> > ...
> >
> > It should work in this case too.
>
> The commit message was a copy and paste from the failure of both :p in a
> VM (PEBS is not supported in a VM). I also ran the bare metal case with
> :pG which per the second patch in this series generates the not
> supported message.
Since the error codes are the same, your code should work also on
bare-metal. Can you test on a host using :ppp? This should trigger the
same error message as in a vm.
Thanks,
-Robert
--
Advanced Micro Devices, Inc.
Operating System Research Center
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] perf tool: give user better message if precise is not supported
2012-09-11 15:11 ` Robert Richter
@ 2012-09-12 14:59 ` David Ahern
0 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2012-09-12 14:59 UTC (permalink / raw)
To: Robert Richter; +Cc: acme, linux-kernel, peterz, Ingo Molnar
On 9/11/12 9:11 AM, Robert Richter wrote:
> On 11.09.12 08:32:55, David Ahern wrote:
>> My guess would be /usr/include/bits/errno.h:
>>
>> /* Linux has no ENOTSUP error code. */
>> # define ENOTSUP EOPNOTSUPP
>
> Ok, so ENOTSUP is actually the same as EOPNOTSUPP. Since the syscall
> returns a EOPNOTSUPP, I prefer this when checking perf_event_open()
> return codes. ENOTSUP is not used in the kernel. Was there a reason
> for choosing ENOTSUP?
poor memory? laziness? a habit I was not aware I had acquired with using
ENOTSUP? I mentioned in a prior response I would change it to EOPNOTSUPP
to be consistent with the 2nd patch -- what the kernel is returning.
>
>>> If you run this bare-metal on older machines which do not support pebs
>>> or ibs, the syscall returns EOPNOTSUPP. You can trigger the same
>>> behaviour on newer systems with:
>>>
>>> # perf record -e cycles:ppp -c 2097120 -R -a sleep 1
>>>
>>> Error: sys_perf_event_open() syscall returned with 95 (Operation not supported). /bin/dmesg may provide additional information.
>>> ...
>>>
>>> It should work in this case too.
>>
>> The commit message was a copy and paste from the failure of both :p in a
>> VM (PEBS is not supported in a VM). I also ran the bare metal case with
>> :pG which per the second patch in this series generates the not
>> supported message.
>
> Since the error codes are the same, your code should work also on
> bare-metal. Can you test on a host using :ppp? This should trigger the
> same error message as in a vm.
As expected:
$ perf record -e cycles:ppp -a
Error:
'precise' request not supported. Try removing 'p' modifier
Resending patchset in a few minutes.
David
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] perf: precise mode and exclude_guest
2012-09-10 16:40 [PATCH 0/3] perf: precise mode and exclude_guest David Ahern
` (2 preceding siblings ...)
2012-09-10 16:40 ` [PATCH 3/3] perf tool: give user better message if precise is not supported David Ahern
@ 2012-09-10 16:57 ` Peter Zijlstra
2012-09-10 17:01 ` David Ahern
3 siblings, 1 reply; 20+ messages in thread
From: Peter Zijlstra @ 2012-09-10 16:57 UTC (permalink / raw)
To: David Ahern; +Cc: acme, linux-kernel
On Mon, 2012-09-10 at 10:40 -0600, David Ahern wrote:
> Hopefully thi wraps up the precise mode-exclude_guest dependency.
> I'm sure someone will let me know if I screwed up the attribution
> in the second patch.
I'll wait with applying until we have the IBS stuff sorted, other than
that, thanks for collecting this..
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] perf: precise mode and exclude_guest
2012-09-10 16:57 ` [PATCH 0/3] perf: precise mode and exclude_guest Peter Zijlstra
@ 2012-09-10 17:01 ` David Ahern
2012-09-10 17:13 ` Peter Zijlstra
0 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2012-09-10 17:01 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: acme, linux-kernel, Robert Richter
On 9/10/12 10:57 AM, Peter Zijlstra wrote:
> On Mon, 2012-09-10 at 10:40 -0600, David Ahern wrote:
>> Hopefully thi wraps up the precise mode-exclude_guest dependency.
>> I'm sure someone will let me know if I screwed up the attribution
>> in the second patch.
>
>
> I'll wait with applying until we have the IBS stuff sorted, other than
> that, thanks for collecting this..
>
Robert recently confirmed that this series should not have an impact on
AMD as it currently stands -- exclude attributes are ignored. And then
his patch to require they be unset works fine with the first patch in
this set because of the existing EINVAL fallback handling in perf-record
and perf-top.
Did you have something else in mind that needs to be done?
David
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] perf: precise mode and exclude_guest
2012-09-10 17:01 ` David Ahern
@ 2012-09-10 17:13 ` Peter Zijlstra
0 siblings, 0 replies; 20+ messages in thread
From: Peter Zijlstra @ 2012-09-10 17:13 UTC (permalink / raw)
To: David Ahern; +Cc: acme, linux-kernel, Robert Richter
On Mon, 2012-09-10 at 11:01 -0600, David Ahern wrote:
> On 9/10/12 10:57 AM, Peter Zijlstra wrote:
> > On Mon, 2012-09-10 at 10:40 -0600, David Ahern wrote:
> >> Hopefully thi wraps up the precise mode-exclude_guest dependency.
> >> I'm sure someone will let me know if I screwed up the attribution
> >> in the second patch.
> >
> >
> > I'll wait with applying until we have the IBS stuff sorted, other than
> > that, thanks for collecting this..
> >
>
> Robert recently confirmed that this series should not have an impact on
> AMD as it currently stands -- exclude attributes are ignored. And then
> his patch to require they be unset works fine with the first patch in
> this set because of the existing EINVAL fallback handling in perf-record
> and perf-top.
>
> Did you have something else in mind that needs to be done?
Ah, no, just hadn't integrated all this various data yet. There seems to
be a lag between reading stuff and actually knowing it :/
OK, let me go through all this once again.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3 v2] perf: precise mode and exclude_guest
@ 2012-09-12 15:16 David Ahern
2012-09-12 15:16 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
0 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2012-09-12 15:16 UTC (permalink / raw)
To: acme, linux-kernel, peterz; +Cc: David Ahern
Hopefully this wraps up the precise mode-exclude_guest dependency.
I'm sure someone will let me know if I screwed up the attribution
in the second patch.
David Ahern (2):
perf tool: precise mode requires exclude_guest
perf tool: give user better message if precise is not supported
Peter Zijlstra (1):
perf: require exclude_guest to use PEBS - kernel side enforcement
v2: updated commit messages for patches 1 and 2
arch/x86/kernel/cpu/perf_event.c | 6 ++++++
tools/perf/builtin-record.c | 5 +++++
tools/perf/builtin-top.c | 4 ++++
tools/perf/util/parse-events.c | 3 +++
4 files changed, 18 insertions(+)
--
1.7.10.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] perf tool: precise mode requires exclude_guest
2012-09-12 15:16 [PATCH 0/3 v2] " David Ahern
@ 2012-09-12 15:16 ` David Ahern
2012-09-12 17:46 ` Robert Richter
0 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2012-09-12 15:16 UTC (permalink / raw)
To: acme, linux-kernel, peterz
Cc: David Ahern, Ingo Molnar, Robert Richter, Gleb Natapov, Avi Kivity
Summary of events per Peter:
"Intel PEBS in VT-x context uses the DS address as a guest linear address,
even though its programmed by the host as a host linear address. This
either results in guest memory corruption and or the hardware faulting and
'crashing' the virtual machine. Therefore we have to disable PEBS on VT-x
enter and re-enable on VT-x exit, enforcing a strict exclude_guest.
AMB IBS does work but doesn't currently support exclude_* at all,
setting an exclude_* bit will make it fail."
This patch handles userspace perf command, setting the exclude_guest
attribute if precise mode is requested, but only if a user has not
specified a request for guest or host only profiling (G or H attribute).
Kernel side AMD currently ignores all exclude_* bits, so there is no impact
to existing IBS code paths. Robert Richter has a patch where IBS code will
return EINVAL if an exclude_* bit is set. When this goes in it means use
of :p on AMD with IBS will first fail with EINVAL (because exclude_guest
will be set). Then the existing fallback code within perf will unset
exclude_guest and try again. The second attempt will succeed if the CPU
supports IBS profiling.
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Robert Richter <robert.richter@amd.com>
Cc: Gleb Natapov <gleb@redhat.com>
Cc: Avi Kivity <avi@redhat.com>
Link: https://lkml.org/lkml/2012/7/9/264
---
tools/perf/util/parse-events.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 44afcf4..696cc7e 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -694,6 +694,9 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
eH = 0;
} else if (*str == 'p') {
precise++;
+ /* use of precise requires exclude_guest */
+ if (!exclude_GH)
+ eG = 1;
} else
break;
--
1.7.10.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] perf tool: precise mode requires exclude_guest
2012-09-12 15:16 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
@ 2012-09-12 17:46 ` Robert Richter
2012-09-12 18:50 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 20+ messages in thread
From: Robert Richter @ 2012-09-12 17:46 UTC (permalink / raw)
To: David Ahern
Cc: acme, linux-kernel, peterz, Ingo Molnar, Gleb Natapov, Avi Kivity
On 12.09.12 09:16:28, David Ahern wrote:
> Summary of events per Peter:
>
> "Intel PEBS in VT-x context uses the DS address as a guest linear address,
> even though its programmed by the host as a host linear address. This
> either results in guest memory corruption and or the hardware faulting and
> 'crashing' the virtual machine. Therefore we have to disable PEBS on VT-x
> enter and re-enable on VT-x exit, enforcing a strict exclude_guest.
>
> AMB IBS does work but doesn't currently support exclude_* at all,
> setting an exclude_* bit will make it fail."
>
> This patch handles userspace perf command, setting the exclude_guest
> attribute if precise mode is requested, but only if a user has not
> specified a request for guest or host only profiling (G or H attribute).
>
> Kernel side AMD currently ignores all exclude_* bits, so there is no impact
> to existing IBS code paths. Robert Richter has a patch where IBS code will
> return EINVAL if an exclude_* bit is set. When this goes in it means use
> of :p on AMD with IBS will first fail with EINVAL (because exclude_guest
> will be set). Then the existing fallback code within perf will unset
> exclude_guest and try again. The second attempt will succeed if the CPU
> supports IBS profiling.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Robert Richter <robert.richter@amd.com>
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Avi Kivity <avi@redhat.com>
> Link: https://lkml.org/lkml/2012/7/9/264
> ---
> tools/perf/util/parse-events.c | 3 +++
> 1 file changed, 3 insertions(+)
Acked-by: Robert Richter <robert.richter@amd.com>
I tested the patch set with AMD IBS and it works fine.
-Robert
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 44afcf4..696cc7e 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -694,6 +694,9 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
> eH = 0;
> } else if (*str == 'p') {
> precise++;
> + /* use of precise requires exclude_guest */
> + if (!exclude_GH)
> + eG = 1;
> } else
> break;
>
> --
> 1.7.10.1
>
>
--
Advanced Micro Devices, Inc.
Operating System Research Center
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] perf tool: precise mode requires exclude_guest
2012-09-12 17:46 ` Robert Richter
@ 2012-09-12 18:50 ` Arnaldo Carvalho de Melo
2012-09-13 9:13 ` Robert Richter
0 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-12 18:50 UTC (permalink / raw)
To: Robert Richter
Cc: David Ahern, linux-kernel, peterz, Ingo Molnar, Gleb Natapov, Avi Kivity
Em Wed, Sep 12, 2012 at 07:46:55PM +0200, Robert Richter escreveu:
> On 12.09.12 09:16:28, David Ahern wrote:
> >
> > Kernel side AMD currently ignores all exclude_* bits, so there is no impact
> > to existing IBS code paths. Robert Richter has a patch where IBS code will
> > return EINVAL if an exclude_* bit is set. When this goes in it means use
> > of :p on AMD with IBS will first fail with EINVAL (because exclude_guest
> > will be set). Then the existing fallback code within perf will unset
> > exclude_guest and try again. The second attempt will succeed if the CPU
> > supports IBS profiling.
> >
> > Signed-off-by: David Ahern <dsahern@gmail.com>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Robert Richter <robert.richter@amd.com>
> > Cc: Gleb Natapov <gleb@redhat.com>
> > Cc: Avi Kivity <avi@redhat.com>
> > Link: https://lkml.org/lkml/2012/7/9/264
> > ---
> > tools/perf/util/parse-events.c | 3 +++
> > 1 file changed, 3 insertions(+)
>
> Acked-by: Robert Richter <robert.richter@amd.com>
>
> I tested the patch set with AMD IBS and it works fine.
Ok, I think that in this specific patchset I can add:
Tested-by: Robert Richter <robert.richter@amd.com>
Reviewed-by: Robert Richter <robert.richter@amd.com>
Instead of an Acked-by, may I?
- Arnaldo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] perf tool: precise mode requires exclude_guest
2012-09-12 18:50 ` Arnaldo Carvalho de Melo
@ 2012-09-13 9:13 ` Robert Richter
0 siblings, 0 replies; 20+ messages in thread
From: Robert Richter @ 2012-09-13 9:13 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: David Ahern, linux-kernel, peterz, Ingo Molnar, Gleb Natapov, Avi Kivity
On 12.09.12 11:50:57, Arnaldo Carvalho de Melo wrote:
> Em Wed, Sep 12, 2012 at 07:46:55PM +0200, Robert Richter escreveu:
> > On 12.09.12 09:16:28, David Ahern wrote:
> > >
> > > Kernel side AMD currently ignores all exclude_* bits, so there is no impact
> > > to existing IBS code paths. Robert Richter has a patch where IBS code will
> > > return EINVAL if an exclude_* bit is set. When this goes in it means use
> > > of :p on AMD with IBS will first fail with EINVAL (because exclude_guest
> > > will be set). Then the existing fallback code within perf will unset
> > > exclude_guest and try again. The second attempt will succeed if the CPU
> > > supports IBS profiling.
> > >
> > > Signed-off-by: David Ahern <dsahern@gmail.com>
> > > Cc: Ingo Molnar <mingo@kernel.org>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Robert Richter <robert.richter@amd.com>
> > > Cc: Gleb Natapov <gleb@redhat.com>
> > > Cc: Avi Kivity <avi@redhat.com>
> > > Link: https://lkml.org/lkml/2012/7/9/264
> > > ---
> > > tools/perf/util/parse-events.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> >
> > Acked-by: Robert Richter <robert.richter@amd.com>
> >
> > I tested the patch set with AMD IBS and it works fine.
>
> Ok, I think that in this specific patchset I can add:
>
> Tested-by: Robert Richter <robert.richter@amd.com>
> Reviewed-by: Robert Richter <robert.richter@amd.com>
>
> Instead of an Acked-by, may I?
I am fine with this.
Thanks,
-Robert
>
> - Arnaldo
>
--
Advanced Micro Devices, Inc.
Operating System Research Center
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3 v3] perf: precise mode and exclude_guest
@ 2012-09-13 20:59 David Ahern
2012-09-13 20:59 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
0 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2012-09-13 20:59 UTC (permalink / raw)
To: acme, linux-kernel, peterz; +Cc: David Ahern
Hopefully this wraps up the precise mode-exclude_guest dependency.
I'm sure someone will let me know if I screwed up the attribution
in the second patch.
David Ahern (2):
perf tool: precise mode requires exclude_guest
perf tool: give user better message if precise is not supported
Peter Zijlstra (1):
perf: require exclude_guest to use PEBS - kernel side enforcement
v3: removed extra tab in patch 2
v2: updated commit messages for patches 1 and 2
arch/x86/kernel/cpu/perf_event.c | 6 ++++++
tools/perf/builtin-record.c | 5 +++++
tools/perf/builtin-top.c | 4 ++++
tools/perf/util/parse-events.c | 3 +++
4 files changed, 18 insertions(+)
--
1.7.10.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] perf tool: precise mode requires exclude_guest
2012-09-13 20:59 [PATCH 0/3 v3] perf: precise mode and exclude_guest David Ahern
@ 2012-09-13 20:59 ` David Ahern
0 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2012-09-13 20:59 UTC (permalink / raw)
To: acme, linux-kernel, peterz
Cc: David Ahern, Ingo Molnar, Robert Richter, Gleb Natapov, Avi Kivity
Summary of events per Peter:
"Intel PEBS in VT-x context uses the DS address as a guest linear address,
even though its programmed by the host as a host linear address. This
either results in guest memory corruption and or the hardware faulting and
'crashing' the virtual machine. Therefore we have to disable PEBS on VT-x
enter and re-enable on VT-x exit, enforcing a strict exclude_guest.
AMB IBS does work but doesn't currently support exclude_* at all,
setting an exclude_* bit will make it fail."
This patch handles userspace perf command, setting the exclude_guest
attribute if precise mode is requested, but only if a user has not
specified a request for guest or host only profiling (G or H attribute).
Kernel side AMD currently ignores all exclude_* bits, so there is no impact
to existing IBS code paths. Robert Richter has a patch where IBS code will
return EINVAL if an exclude_* bit is set. When this goes in it means use
of :p on AMD with IBS will first fail with EINVAL (because exclude_guest
will be set). Then the existing fallback code within perf will unset
exclude_guest and try again. The second attempt will succeed if the CPU
supports IBS profiling.
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Robert Richter <robert.richter@amd.com>
Cc: Gleb Natapov <gleb@redhat.com>
Cc: Avi Kivity <avi@redhat.com>
Link: https://lkml.org/lkml/2012/7/9/264
---
tools/perf/util/parse-events.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 44afcf4..696cc7e 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -694,6 +694,9 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
eH = 0;
} else if (*str == 'p') {
precise++;
+ /* use of precise requires exclude_guest */
+ if (!exclude_GH)
+ eG = 1;
} else
break;
--
1.7.10.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2012-09-13 20:59 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-10 16:40 [PATCH 0/3] perf: precise mode and exclude_guest David Ahern
2012-09-10 16:40 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
2012-09-10 17:23 ` Peter Zijlstra
2012-09-10 16:40 ` [PATCH 2/3] perf: require exclude_guest to use PEBS - kernel side enforcement David Ahern
2012-09-10 17:24 ` Peter Zijlstra
2012-09-10 16:40 ` [PATCH 3/3] perf tool: give user better message if precise is not supported David Ahern
2012-09-11 9:20 ` Robert Richter
2012-09-11 13:22 ` David Ahern
2012-09-11 14:01 ` Robert Richter
2012-09-11 14:32 ` David Ahern
2012-09-11 15:11 ` Robert Richter
2012-09-12 14:59 ` David Ahern
2012-09-10 16:57 ` [PATCH 0/3] perf: precise mode and exclude_guest Peter Zijlstra
2012-09-10 17:01 ` David Ahern
2012-09-10 17:13 ` Peter Zijlstra
2012-09-12 15:16 [PATCH 0/3 v2] " David Ahern
2012-09-12 15:16 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
2012-09-12 17:46 ` Robert Richter
2012-09-12 18:50 ` Arnaldo Carvalho de Melo
2012-09-13 9:13 ` Robert Richter
2012-09-13 20:59 [PATCH 0/3 v3] perf: precise mode and exclude_guest David Ahern
2012-09-13 20:59 ` [PATCH 1/3] perf tool: precise mode requires exclude_guest David Ahern
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).