HWPMC tracing support (1) -- main

[br]

Summary

This adds two new counters modes:

1. PMC_MODE_ST -- system-wide tracing
2. PMC_MODE_TT -- thread virtual tracing

Four new PMC ioctl calls:

1. LOG_KERNEL_MAP

Used to request kernel mappings to be send over pmc log. This is required in order to configure address ranges filter prior to starting system-wide PMC trace.

2. THREAD_UNSUSPEND

Used to release the thread. As user thread can call mmap() any time, we put thread on suspend, then we ship mmap result over PMC log, so user can get idea about mapping location (for libc etc) and reconfigure PMC address ranges prior to thread execution.

3. TRACE_READ

Used to get current fill of the trace buffers.

4. TRACE_CONFIG

Used to reconfigure tracing address ranges after PMC allocation and prior starting PMC.

We now also create /dev/pmcX per each cpu core, so user can access trace buffers.

Example usage:

• pmctrace -u -e branches uname
• pmctrace -u -e branches -i libc.so.7 -f memset sleep 1
• sudo pmctrace -s -e branches -i kernel -f cpu_switch

Other parts of patch:
part 2
part 3
part 4
part 5
part 6
part 7
part 8
part 9

Test Plan

Tested on ThinkPad X1 Carbon (Kabylake)
Tested on DragonBoard 410c (ARM64)

[kib]

sys/dev/hwpmc/hwpmc_cs.c

158 ↗

(On Diff #39886)

Don''t you need to zero page if PG_ZERO is not set in flags ?

[br]

• Add assert to ensure buffer deallocation happens in the same thread as allocation
• Zero the pages if required. vm_page_alloc_contig does not guarantee that page will be zeroed with VM_ALLOC_ZERO flag set
• Iterate over pages properly using vm_page_next

[br]

sys/dev/hwpmc/hwpmc_cs.c

148 ↗

(On Diff #39886)

Wanted to ensure that object will be destroyed after we stop tracing operation.

154 ↗

(On Diff #39886)

Thanks I moved taking a reference few lines later. Not sure however yet if we need to take additional reference. It works fine with and without taking additional reference.

157 ↗

(On Diff #39886)

Thanks I switched to vm_page_next() for iteration.

158 ↗

(On Diff #39886)

Yes, thanks.

190 ↗

(On Diff #39886)

Added.

[kib]

sys/dev/hwpmc/hwpmc_cs.c

154 ↗

(On Diff #39886)

You might just leak the object with the additional reference.

[br]

• Don't take additional reference for VM object.
• Suppress ETF buffer fill readings. ETF is a small FIFO hardware buffer we are not accessing in software

[br]

sys/dev/hwpmc/hwpmc_cs.c

154 ↗

(On Diff #39886)

I removed taking additional reference. In works fine. Thanks!

[andrew]

sys/arm64/coresight/coresight-dynamic-replicator.c

99–102 ↗

(On Diff #39999)

Why are you using function pointers and not a new device interface?

sys/arm64/coresight/coresight-etm4x.c

80 ↗

(On Diff #39999)

Why do we need the isb here & below?

139 ↗

(On Diff #39999)

Is this comment > 80 characters? It's difficult to tell in the phabricator interface.

179 ↗

(On Diff #39999)

What are these magic values?

181 ↗

(On Diff #39999)

This should be at the top of the function

183 ↗

(On Diff #39999)

Should this be under bootverbose?

206 ↗

(On Diff #39999)

What's 1 << 0?

[br]

• Add coresight device interface. Use it instead of function pointers
• Wrap long lines
• Unmagic values
• Print debugging information if bootverbose set
• Add ETM_DEBUG macro

[br]

• Include coresight_if.m to patch

[br]

sys/arm64/coresight/coresight-dynamic-replicator.c

99–102 ↗

(On Diff #39999)

I switched to new device interface. Thanks

sys/arm64/coresight/coresight-etm4x.c

80 ↗

(On Diff #39999)

I think we don't need this. I took it from linux during early debugging. They use it to ensure everyone sees the write or to ensure order

139 ↗

(On Diff #39999)

Fixed

179 ↗

(On Diff #39999)

Fixed

181 ↗

(On Diff #39999)

Thanks. Fixed

183 ↗

(On Diff #39999)

I added ETM_DEBUG macro for this. Thanks

206 ↗

(On Diff #39999)

Fixed. Thanks

[br]

Support for Coresight devices splitted-out to a separate review https://reviews.freebsd.org/D14618

[br]

Regenerate patch

[br]

Initialize ev to NULL before processing command line. This fixes operation using Intel PT

[br]

Regenerate patch

[br]

Regenerate patch

[br]

Regenerate patch (opencsd committed to HEAD)

[br]

Regenerate patch

[br]

split out changes

[br]

add context

[br]

check return value for pmc_vm_initialize(md);

[br]

Regenerate patch

[br]

merge changes from upstream

[br]

regenerate

[br]

merge changes from upstream

[br]

merge from upstream

[br]

regenerate

[br]

fix style

[br]

Add a comment on why we pause the thread

[br]

regenerate patch

[markj]

sys/dev/hwpmc/hwpmc_mod.c

1789

Use bool literals, "true"/"false".

1813

Isn't this somewhat fragile? If pmctrace crashes before unsuspending the thread, won't the thread stay stuck? ptrace(), for instance, will send SIGKILL to the target process if the tracing process dies.

5991

This should come earlier. This case is only meant to be executed if initialization completes successfully, but the pmc_vm_initialize() call introduces a new error case.

[br]

• Use bool literals
• Move pmc_vm_initialize() just after pmc_md_initialize()

[br]

sys/dev/hwpmc/hwpmc_mod.c

1813

Yes it is fragile. Any crash of pmctrace leaves the tracing pid on a pause. I am not sure how to avoid this.
At least a warning to user must be added

[markj]

sys/dev/hwpmc/hwpmc_mod.c

1813

Why is it necessary to suspend all threads in the process? At this point we have not yet returned from mmap(), so other application threads should not be accessing the newly mapped region.

[br]

Use thread_suspend_one() instead of thread_suspend_switch()

[br]

sys/dev/hwpmc/hwpmc_mod.c

1813

It is not necessary, so I changed to thread_suspend_one()

[br]

use sleep(9) instead of thread_suspend_one

[br]

Regenerate patch.