Ftrace: arm/arm64: Define a new arm/arm64 trace clock source based on CNTPCT/CNTPCT_EL0 register.

PATCH New Amit 2 participants 6 messages Oct 26, 2015 – Oct 29, 2015

Subsystems (5)

Documentation Arm (32-bit) arm64 Tracing +1 more subsystems

lore

Share & tools

lore

Amit

author

2015-10-26 10:15 UTC #0 lore raw

PATCH 1/1 Ftrace: arm/arm64: Define a new arm/arm64 trace clock source based on CNTPCT/CNTPCT_EL0 register.

From: Amit Singh Tomar <[email protected]>

Define new arm/arm64 specific trace clock using CNTPCT/CNTPCT_EL0 register, similar to x86-tsc. It can be used to correlate trace events across hosts and guest that can be useful for debugging purpose.

Signed-off-by Amit Singh Tomar <[email protected]>

---
Changes since RFC:
* Replaced define with static inline
* Revert few changes done in ftrace.txt
* Made it common between ARM and ARM64.

---
 Documentation/trace/ftrace.txt       | 12 ++++++++++++
 arch/arm/include/asm/Kbuild          |  1 -
 arch/arm/include/asm/trace_clock.h   | 21 +++++++++++++++++++++
 arch/arm/kernel/Makefile             |  2 ++
 arch/arm/kernel/trace_clock.c        | 33 +++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/Kbuild        |  1 -
 arch/arm64/include/asm/trace_clock.h | 21 +++++++++++++++++++++
 arch/arm64/kernel/Makefile           |  2 ++
 8 files changed, 91 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/include/asm/trace_clock.h
 create mode 100644 arch/arm/kernel/trace_clock.c
 create mode 100644 arch/arm64/include/asm/trace_clock.h

Documentation/trace/ftrace.txt +12 -0

index ef621d3..13f3737 100644
@@ -351,6 +351,18 @@ of ftrace. Here is a list of some of the key files:
 		  to correlate events across hypervisor/guest if
 		  tb_offset is known.
 
+	 arm-pct: This uses ARM Physical Count register value.This
+	 	  is different from default "local" clock which
+		  usese Virtual Count register.This is consistent
+		  across processors and can be used to correlate
+		  events across host/guest.
+
+	 arm64-pct: This uses ARM64 Physical Timer Count register
+                   value. This is different from default "local"
+                   clock which usese Virtual Timer Count register.
+                   This is consistent across processors and can be
+                   used to correlate events across host/guest.
+
 	To set a clock, simply echo the clock name into this file.
 
 	  echo global > trace_clock

arch/arm/include/asm/Kbuild +0 -1

index be648eb..bc3964b 100644
@@ -34,5 +34,4 @@ generic-y += sockios.h
 generic-y += termbits.h
 generic-y += termios.h
 generic-y += timex.h
-generic-y += trace_clock.h
 generic-y += unaligned.h

arch/arm/include/asm/trace_clock.h new file +21 -0

new file mode 100644
index 0000000..dc41f75
@@ -0,0 +1,21 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * Copyright (C) 2015 Amit Singh Tomar, amittomer25 at gmail.com
+*/
+
+#ifndef _ASM_ARM_TRACE_CLOCK_H
+#define _ASM_ARM_TRACE_CLOCK_H
+
+#include <linux/compiler.h>
+#include <linux/types.h>
+
+extern u64 notrace trace_clock_arm_pct(void);
+
+# define ARCH_TRACE_CLOCKS \
+	{ trace_clock_arm_pct,  "arm-pct",  0},
+
+#endif  /* _ASM_ARM_TRACE_CLOCK_H */
+

arch/arm/kernel/Makefile +2 -0

index af9e59b..c3abf7c 100644
@@ -92,4 +92,6 @@ obj-y				+= psci-call.o
 obj-$(CONFIG_SMP)		+= psci_smp.o
 endif
 
+obj-$(CONFIG_TRACING)           += trace_clock.o
+
 extra-y := $(head-y) vmlinux.lds

arch/arm/kernel/trace_clock.c new file +33 -0

new file mode 100644
index 0000000..3bf040d
@@ -0,0 +1,33 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * Copyright (C) 2015 Amit Singh Tomar, amittomer25 at gmail.com
+ *
+ * Thanks to Steven Rostedt and Andre Przywara!
+*/
+
+#include <asm/trace_clock.h>
+#include <asm/barrier.h>
+#include <asm/arch_timer.h>
+
+static inline u64 get_cntpct(void)
+{
+	u64 cycles;
+
+	isb();
+	asm volatile("mrs %0, cntpct_el0" : "=r" (cycles));
+
+	return cycles;
+}
+
+u64 notrace trace_clock_arm_pct(void)
+{
+	return arch_counter_get_cntpct();
+}
+
+#ifdef CONFIG_ARM64
+u64 notrace trace_clock_arm64_pct(void)
+{
+	return get_cntpct();

+}
+#endif

arch/arm64/include/asm/Kbuild +0 -1

index 70fd9ff..8608f9e 100644
@@ -50,7 +50,6 @@ generic-y += switch_to.h
 generic-y += termbits.h
 generic-y += termios.h
 generic-y += topology.h
-generic-y += trace_clock.h
 generic-y += types.h
 generic-y += unaligned.h
 generic-y += user.h

arch/arm64/include/asm/trace_clock.h new file +21 -0

new file mode 100644
index 0000000..aef4ccd
@@ -0,0 +1,21 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * Copyright (C) 2015 Amit Singh Tomar, amittomer25 at gmail.com
+*/
+
+#ifndef _ASM_ARM_TRACE_CLOCK_H
+#define _ASM_ARM_TRACE_CLOCK_H
+
+#include <linux/compiler.h>
+#include <linux/types.h>
+
+extern u64 notrace trace_clock_arm64_pct(void);
+
+# define ARCH_TRACE_CLOCKS \
+	{ trace_clock_arm64_pct,  "arm64-pct",  0},
+
+#endif  /* _ASM_ARM_TRACE_CLOCK_H */
+

arch/arm64/kernel/Makefile +2 -0

index 22dc9bc..b23a149 100644
@@ -10,6 +10,7 @@ CFLAGS_armv8_deprecated.o := -I$(src)
 CFLAGS_REMOVE_ftrace.o = -pg
 CFLAGS_REMOVE_insn.o = -pg
 CFLAGS_REMOVE_return_address.o = -pg
+ARM=../../../arch/arm/kernel
 
 # Object file lists.
 arm64-obj-y		:= debug-monitors.o entry.o irq.o fpsimd.o		\

@@ -36,6 +37,7 @@ arm64-obj-$(CONFIG_EFI)			+= efi.o efi-stub.o efi-entry.o
 arm64-obj-$(CONFIG_PCI)			+= pci.o
 arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
 arm64-obj-$(CONFIG_ACPI)		+= acpi.o
+arm64-obj-$(CONFIG_TRACING)             += $(ARM)/trace_clock.o
 
 obj-y					+= $(arm64-obj-y) vdso/
 obj-m					+= $(arm64-obj-m)

Signature · 1 line

1.9.1

Mark Rutland

reviewer

2015-10-28 12:47 UTC #1 lore raw

PATCH 1/1 Ftrace: arm/arm64: Define a new arm/arm64 trace clock source based on CNTPCT/CNTPCT_EL0 register.

Quoted 35 lines · Amit

From: Amit Singh Tomar <[email protected]>
Define new arm/arm64 specific trace clock using CNTPCT/CNTPCT_EL0 register, similar to x86-tsc. It can be used to correlate trace events across hosts and guest that can be useful for debugging purpose.
Signed-off-by Amit Singh Tomar <[email protected]>
---
Changes since RFC:
* Replaced define with static inline
* Revert few changes done in ftrace.txt
* Made it common between ARM and ARM64.
---
 Documentation/trace/ftrace.txt       | 12 ++++++++++++
 arch/arm/include/asm/Kbuild          |  1 -
 arch/arm/include/asm/trace_clock.h   | 21 +++++++++++++++++++++
 arch/arm/kernel/Makefile             |  2 ++
 arch/arm/kernel/trace_clock.c        | 33 +++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/Kbuild        |  1 -
 arch/arm64/include/asm/trace_clock.h | 21 +++++++++++++++++++++
 arch/arm64/kernel/Makefile           |  2 ++
 8 files changed, 91 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/include/asm/trace_clock.h
 create mode 100644 arch/arm/kernel/trace_clock.c
 create mode 100644 arch/arm64/include/asm/trace_clock.h
Documentation/trace/ftrace.txt +1 -0
index ef621d3..13f3737 100644
@@ -351,6 +351,18 @@ of ftrace. Here is a list of some of the key files:
 		  to correlate events across hypervisor/guest if
 		  tb_offset is known.
 
+	 arm-pct: This uses ARM Physical Count register value.This

Documentation/trace/ftrace.txt of ftrace. Here is a list of some of the key files: · line 364

Quoted 10 lines

Documentation/trace/ftrace.txt +10 -0

+	 	  is different from default "local" clock which
+		  usese Virtual Count register.This is consistent
+		  across processors and can be used to correlate
+		  events across host/guest.
+
+	 arm64-pct: This uses ARM64 Physical Timer Count register
+                   value. This is different from default "local"
+                   clock which usese Virtual Timer Count register.
+                   This is consistent across processors and can be
+                   used to correlate events across host/guest.

The above is wrong. Both the physical and virtual counters have the same guarantees w.r.t. consistency across CPUs for the same VM image. Neither is more local than the other.

In the host you will have access to CNTVOFF, so you can use that to correlate the virtual counter.

Additionally, it's not safe to read the physical counter register unless you know that access has not been denied by the hypervisor; so any Linux image not booted at hyp cannot use this. If the hypervisor emulates the access, it will also be horrifically slow.

I don't think this makes sense.

Quoted 19 lines

arch/arm/kernel/trace_clock.c new file +13 -0

new file mode 100644
index 0000000..3bf040d
@@ -0,0 +1,33 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * Copyright (C) 2015 Amit Singh Tomar, amittomer25 at gmail.com
+ *
+ * Thanks to Steven Rostedt and Andre Przywara!
+*/
+
+#include <asm/trace_clock.h>
+#include <asm/barrier.h>
+#include <asm/arch_timer.h>

arch/arm/kernel/trace_clock.c line 23

Quoted 10 lines

arch/arm/kernel/trace_clock.c new file +10 -0

+
+static inline u64 get_cntpct(void)
+{
+	u64 cycles;
+
+	isb();
+	asm volatile("mrs %0, cntpct_el0" : "=r" (cycles));
+
+	return cycles;
+}

This is arch/arm. This cannot build for 32-bit.

Regardless, this doesn't belong here.

Thanks,
Mark.

Amit Tomer

author

2015-10-28 19:02 UTC #2 lore raw

PATCH 1/1 Ftrace: arm/arm64: Define a new arm/arm64 trace clock source based on CNTPCT/CNTPCT_EL0 register.

Quoted 35 lines · Mark Rutland

Quoted 34 lines · Amit
From: Amit Singh Tomar <[email protected]>
Define new arm/arm64 specific trace clock using CNTPCT/CNTPCT_EL0 register, similar to x86-tsc. It can be used to correlate trace events across hosts and guest that can be useful for debugging purpose.
Signed-off-by Amit Singh Tomar <[email protected]>
---
Changes since RFC:
* Replaced define with static inline
* Revert few changes done in ftrace.txt
* Made it common between ARM and ARM64.
---
 Documentation/trace/ftrace.txt       | 12 ++++++++++++
 arch/arm/include/asm/Kbuild          |  1 -
 arch/arm/include/asm/trace_clock.h   | 21 +++++++++++++++++++++
 arch/arm/kernel/Makefile             |  2 ++
 arch/arm/kernel/trace_clock.c        | 33 +++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/Kbuild        |  1 -
 arch/arm64/include/asm/trace_clock.h | 21 +++++++++++++++++++++
 arch/arm64/kernel/Makefile           |  2 ++
 8 files changed, 91 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/include/asm/trace_clock.h
 create mode 100644 arch/arm/kernel/trace_clock.c
 create mode 100644 arch/arm64/include/asm/trace_clock.h
Documentation/trace/ftrace.txt +0 -0
index ef621d3..13f3737 100644
@@ -351,6 +351,18 @@ of ftrace. Here is a list of some of the key files:
                to correlate events across hypervisor/guest if
                tb_offset is known.
 

Quoted 15 lines

Documentation/trace/ftrace.txt of ftrace. Here is a list of some of the key files: · line 364
Quoted 11 lines
Documentation/trace/ftrace.txt +11 -0
+      arm-pct: This uses ARM Physical Count register value.This
+               is different from default "local" clock which
+               usese Virtual Count register.This is consistent
+               across processors and can be used to correlate
+               events across host/guest.
+
+      arm64-pct: This uses ARM64 Physical Timer Count register
+                   value. This is different from default "local"
+                   clock which usese Virtual Timer Count register.
+                   This is consistent across processors and can be
+                   used to correlate events across host/guest.
The above is wrong. Both the physical and virtual counters have the same guarantees w.r.t. consistency across CPUs for the same VM image. Neither is more local than the other.

I never said virtual counters are not consistent across CPU's but pointed out that Values of Virtual counter seen by host and guest are different(but even in Host context
I have seen VCT is not exactly equal to PCT).

Quoted 2 lines

In the host you will have access to CNTVOFF, so you can use that to correlate the virtual counter.

I don't think CNTVOFF can be accessible from normal el1 host code and do you think reading it from el2 and then feed it to virtual counter is good idea?

Quoted 5 lines

Additionally, it's not safe to read the physical counter register unless you know that access has not been denied by the hypervisor; so any Linux image not booted at hyp cannot use this. If the hypervisor emulates the access, it will also be horrifically slow.

Sorry but are you saying if kernel doesn't boot in HYP mode , I have no way to read Physical counter in Guest?

Quoted 3 lines

This is arch/arm. This cannot build for 32-bit.

I did build it on ARM(32 bit) and for the same made a check using "#ifdef CONFIG_ARM64", so would you relook at it again.

Thanks
Amit.

Mark Rutland

reviewer

2015-10-29 11:38 UTC #3 lore raw

PATCH 1/1 Ftrace: arm/arm64: Define a new arm/arm64 trace clock source based on CNTPCT/CNTPCT_EL0 register.

[adding Marc and Christoffer]

Quoted 52 lines · Amit Tomer

Quoted 35 lines · Mark Rutland
Quoted 34 lines · Amit
From: Amit Singh Tomar <[email protected]>
Define new arm/arm64 specific trace clock using CNTPCT/CNTPCT_EL0 register, similar to x86-tsc. It can be used to correlate trace events across hosts and guest that can be useful for debugging purpose.
Signed-off-by Amit Singh Tomar <[email protected]>
---
Changes since RFC:
* Replaced define with static inline
* Revert few changes done in ftrace.txt
* Made it common between ARM and ARM64.
---
 Documentation/trace/ftrace.txt       | 12 ++++++++++++
 arch/arm/include/asm/Kbuild          |  1 -
 arch/arm/include/asm/trace_clock.h   | 21 +++++++++++++++++++++
 arch/arm/kernel/Makefile             |  2 ++
 arch/arm/kernel/trace_clock.c        | 33 +++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/Kbuild        |  1 -
 arch/arm64/include/asm/trace_clock.h | 21 +++++++++++++++++++++
 arch/arm64/kernel/Makefile           |  2 ++
 8 files changed, 91 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/include/asm/trace_clock.h
 create mode 100644 arch/arm/kernel/trace_clock.c
 create mode 100644 arch/arm64/include/asm/trace_clock.h
Documentation/trace/ftrace.txt +0 -0
index ef621d3..13f3737 100644
@@ -351,6 +351,18 @@ of ftrace. Here is a list of some of the key files:
                to correlate events across hypervisor/guest if
                tb_offset is known.
 
Quoted 15 lines
Documentation/trace/ftrace.txt of ftrace. Here is a list of some of the key files: · line 364
Quoted 11 lines
Documentation/trace/ftrace.txt +11 -0
+      arm-pct: This uses ARM Physical Count register value.This
+               is different from default "local" clock which
+               usese Virtual Count register.This is consistent
+               across processors and can be used to correlate
+               events across host/guest.
+
+      arm64-pct: This uses ARM64 Physical Timer Count register
+                   value. This is different from default "local"
+                   clock which usese Virtual Timer Count register.
+                   This is consistent across processors and can be
+                   used to correlate events across host/guest.
The above is wrong. Both the physical and virtual counters have the same guarantees w.r.t. consistency across CPUs for the same VM image. Neither is more local than the other.

Quoted 3 lines

I never said virtual counters are not consistent across CPU's but pointed out that Values of Virtual counter seen by host and guest are different(but even in Host context

Ok.

Quoted 1 line

I have seen VCT is not exactly equal to PCT).

That should only be the case when running a guest (or running at hyp).

Accross CPUS, CNTVCT should be consistent for the same OS image (i.e. those CPUs in the same VM will have the same value at the same point in time).

Quoted 6 lines

Quoted 2 lines
In the host you will have access to CNTVOFF, so you can use that to correlate the virtual counter.
I don't think CNTVOFF can be accessible from normal el1 host code and do you think reading it from el2 and then feed it to virtual counter is good idea?

There are KVM APIs for acquring and configuring CNTVOFF for the guest. The host kernel knows what the virtual offset is.

You could even configure the offset to be zero if you wanted, which would make the guest and host virtual counters uniform.

Quoted 7 lines

Quoted 4 lines
Additionally, it's not safe to read the physical counter register unless you know that access has not been denied by the hypervisor; so any Linux image not booted at hyp cannot use this. If the hypervisor emulates the access, it will also be horrifically slow.
Sorry but are you saying if kernel doesn't boot in HYP mode , I have no way to read Physical counter in Guest?

Regardless of the host, you cannot know whether it is safe to access in a guest. It could bring down the system.

The host kernel could safely access the physical counter were it booted at EL2, because it could grant itself access.

Thanks,
Mark.

Amit Tomer

author

2015-10-29 13:47 UTC #4 lore raw

PATCH 1/1 Ftrace: arm/arm64: Define a new arm/arm64 trace clock source based on CNTPCT/CNTPCT_EL0 register.

Quoted 5 lines

Regardless of the host, you cannot know whether it is safe to access in a guest. It could bring down the system.
The host kernel could safely access the physical counter were it booted at EL2, because it could grant itself access.

May be I am totally wrong here but it doesn't look to be good . if Linux image does not boot at hyp mode,
there is no way to use arch timers, Not true?

But I could see even if Linux image booted in EL1 has the access to Physical timer/counters that is
been allowed by firmware it self.

ENTRY(armv8_switch_to_el1)

        switch_el x0, 0f, 1f, 0f

0: ret
1:
/* Initialize Generic Timers */
mrs x0, cnthctl_el2
orr x0, x0, #0x3 /* Enable EL1 access to timers */
msr cnthctl_el2, x0
msr cntvoff_el2, x0
mrs x0, cntkctl_el1
orr x0, x0, #0x3 /* Enable EL0 access to timers */
msr cntkctl_el1, x0

Sorry, if it's just all stupid.

Thanks,
Amit.

Mark Rutland

reviewer

2015-10-29 14:19 UTC #5 lore raw

PATCH 1/1 Ftrace: arm/arm64: Define a new arm/arm64 trace clock source based on CNTPCT/CNTPCT_EL0 register.

Quoted 9 lines · Amit Tomer

Quoted 5 lines
Regardless of the host, you cannot know whether it is safe to access in a guest. It could bring down the system.
The host kernel could safely access the physical counter were it booted at EL2, because it could grant itself access.
May be I am totally wrong here but it doesn't look to be good . if Linux image does not boot at hyp mode,
there is no way to use arch timers, Not true?

The _virtual_ counter and timer registers are _always_ accessible at EL1.

The _physical_ counter and timer registers are not.

The kernel can safely use the virtual counter & timer alone, but cannot use the physical counter & timer. It has no way of determining if the physical registers are accessible (and whether they will remain so).

Even if they were accessible, the physical values can change arbitrarily (think suspend+resuming a VM), so the physical values are generally useless to a VM.

Quoted 3 lines

But I could see even if Linux image booted in EL1 has the access to Physical timer/counters that is
been allowed by firmware it self.

The kernel has no way of knowing whether the physical counter & timer are accessible, even if firmware left them accessible.

Their accessibility could change at a moment's notice (e.g. the state of the control registers could be lost over idle).

Quoted 12 lines

ENTRY(armv8_switch_to_el1)
        switch_el x0, 0f, 1f, 0f
0: ret
1:
/* Initialize Generic Timers */
mrs x0, cnthctl_el2
orr x0, x0, #0x3 /* Enable EL1 access to timers */
msr cnthctl_el2, x0
msr cntvoff_el2, x0
mrs x0, cntkctl_el1
orr x0, x0, #0x3 /* Enable EL0 access to timers */
msr cntkctl_el1, x0

I'm not sure how the above code is relevant.

Mark.