Skip to contentSkip to conversation

perf: precise mode and exclude_guest

PATCH Superseded David Ahern 4 participants 10 messages Sep 12, 2012 – Sep 13, 2012

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover perf: precise mode and exclude_guest

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(+)
Signature · 1 line
1.7.10.1
#1 lore raw

PATCH v2 1/3 perf tool: precise mode requires exclude_guest

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.

Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Robert Richter <[email protected]>
Cc Gleb Natapov <[email protected]>
Cc Avi Kivity <[email protected]>
---
 tools/perf/util/parse-events.c |    3 +++
 1 file changed, 3 insertions(+)
tools/perf/util/parse-events.c +3 -0
index 44afcf4..696cc7e 100644
@@ -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;
 
Signature · 1 line
1.7.10.1
Acked-by on v2 1/3
#2 lore raw

Re: [PATCH 1/3] perf tool: precise mode requires exclude_guest

Quoted 30 lines · David Ahern

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.

Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Robert Richter <[email protected]>
Cc Gleb Natapov <[email protected]>
Cc Avi Kivity <[email protected]>
Quoted 3 lines
---
 tools/perf/util/parse-events.c |    3 +++
 1 file changed, 3 insertions(+)

I tested the patch set with AMD IBS and it works fine.

-Robert

Quoted 19 lines
tools/perf/util/parse-events.c +3 -0
index 44afcf4..696cc7e 100644
@@ -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;
 
Signature · 1 line
1.7.10.1
Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center
Tested-by Robert Richter Reviewed-by Robert Richter
#3 lore raw

Re: [PATCH 1/3] perf tool: precise mode requires exclude_guest

Em Wed, Sep 12, 2012 at 07:46:55PM +0200, Robert Richter escreveu:

Quoted 21 lines · David Ahern
Quoted 16 lines

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.

Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Robert Richter <[email protected]>
Cc Gleb Natapov <[email protected]>
Cc Avi Kivity <[email protected]>
Quoted 3 lines
---
 tools/perf/util/parse-events.c |    3 +++
 1 file changed, 3 insertions(+)
Quoted 3 lines

I tested the patch set with AMD IBS and it works fine.

Ok, I think that in this specific patchset I can add:

Instead of an Acked-by, may I?

- Arnaldo

#4 lore raw

Re: [PATCH 1/3] perf tool: precise mode requires exclude_guest

Quoted 24 lines · Arnaldo Carvalho de Melo

Em Wed, Sep 12, 2012 at 07:46:55PM +0200, Robert Richter escreveu:

Quoted 23 lines · David Ahern
Quoted 16 lines

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.

Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Robert Richter <[email protected]>
Cc Gleb Natapov <[email protected]>
Cc Avi Kivity <[email protected]>
Quoted 3 lines
---
 tools/perf/util/parse-events.c |    3 +++
 1 file changed, 3 insertions(+)
Quoted 8 lines
Quoted 1 line

I tested the patch set with AMD IBS and it works fine.

Ok, I think that in this specific patchset I can add:

Instead of an Acked-by, may I?

I am fine with this.

Thanks,

-Robert

Quoted 3 lines

- Arnaldo

Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center
#5 lore raw

PATCH v2 2/3 perf: require exclude_guest to use PEBS - kernel side enforcement

In reply to David Ahern

From: Peter Zijlstra <[email protected]>

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.

This patch enforces exclude_guest kernel side.

Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Robert Richter <[email protected]>
Cc Gleb Natapov <[email protected]>
Cc Avi Kivity <[email protected]>
---
 arch/x86/kernel/cpu/perf_event.c |    6 ++++++
 1 file changed, 6 insertions(+)
arch/x86/kernel/cpu/perf_event.c +6 -0
index 915b876..4bc96c5 100644
@@ -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++;
Signature · 1 line
1.7.10.1
maintainer
#6 lore raw

Re: [PATCH 2/3] perf: require exclude_guest to use PEBS - kernel side enforcement

Hi David,

Quoted 11 lines · David Ahern

From: Peter Zijlstra <[email protected]>

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.

This patch enforces exclude_guest kernel side.

[snip]

arch/x86/kernel/cpu/perf_event.c int x86_pmu_hw_config(struct perf_event *event) · line 387

Quoted 6 lines
arch/x86/kernel/cpu/perf_event.c +2 -0
@@ -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;

I see a whitespace problem here. :)

Thanks,
Namhyung

Quoted 4 lines
arch/x86/kernel/cpu/perf_event.c +1 -0
+
 		/* Support for constant skid */
 		if (x86_pmu.pebs_active && !x86_pmu.pebs_broken) {
 			precise++;
#7 lore raw

Re: [PATCH 2/3] perf: require exclude_guest to use PEBS - kernel side enforcement

Quoted 15 lines · Namhyung Kim

Hi David,

Quoted 11 lines · David Ahern

From: Peter Zijlstra <[email protected]>

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.

This patch enforces exclude_guest kernel side.

[snip]

Quoted 8 lines

arch/x86/kernel/cpu/perf_event.c int x86_pmu_hw_config(struct perf_event *event) · line 387

Quoted 6 lines
arch/x86/kernel/cpu/perf_event.c +2 -0
@@ -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;

I see a whitespace problem here. :)

grr.... an extra freaking tab.

#8 lore raw

Re: [PATCH 2/3] perf: require exclude_guest to use PEBS - kernel side enforcement

Em Wed, Sep 12, 2012 at 10:33:36PM -0600, David Ahern escreveu:

Quoted 4 lines · Namhyung Kim
Quoted 1 line

I see a whitespace problem here. :)

grr.... an extra freaking tab.

As people say down here, it happens, even in the best families... :-P

- Arnaldo

$ shutdown now tho
#9 lore raw

PATCH v2 3/3 perf tool: give user better message if precise is not supported

In reply to David Ahern

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 may not be supported. Try removing 'p' modifier
  sleep: Terminated

v2: softened message to 'may not be' supported per Robert's suggestion

Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Robert Richter <[email protected]>
---
 tools/perf/builtin-record.c |    5 +++++
 tools/perf/builtin-top.c    |    4 ++++
 2 files changed, 9 insertions(+)
tools/perf/builtin-record.c +5 -0
index c643ed6..385e272 100644
@@ -294,6 +294,11 @@ try_again:
 					  perf_evsel__name(pos));
 				rc = -err;
 				goto out;
+			} else if ((err == EOPNOTSUPP) && (attr->precise_ip)) {
+				ui__error("\'precise\' request may not be supported. "
+					  "Try removing 'p' modifier\n");
+				rc = -err;
+				goto out;
 			}
 
 			printf("\n");
tools/perf/builtin-top.c +4 -0
index 5550754..1d1c98c 100644
@@ -976,6 +976,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 == EOPNOTSUPP) && (attr->precise_ip)) {
+				ui__error("\'precise\' request may not be supported. "
+					  "Try removing 'p' modifier\n");
+				goto out_err;
 			}
 
 			ui__error("The sys_perf_event_open() syscall "
Signature · 1 line
1.7.10.1