Re: [PATCH 08/14] tools/perf: process auxtrace events and display in perf report -D

[Athira Rajeev <atrajeev@linux.ibm.com>]

> On 27 Aug 2025, at 10:58 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> 
> On 15/08/2025 11:34, Athira Rajeev wrote:
>> Add vpa dtl pmu auxtrace process function for "perf report -D".
>> The auxtrace event processing functions are defined in file
>> "util/powerpc-vpadtl.c". Data structures used includes "struct
>> powerpc_vpadtl_queue", "struct powerpc_vpadtl" to store the auxtrace
>> buffers in queue. Different PERF_RECORD_XXX are generated
>> during recording. PERF_RECORD_AUXTRACE_INFO is processed first
>> since it is of type perf_user_event_type and perf session event
>> delivers perf_session__process_user_event() first. Define function
>> powerpc_vpadtl_process_auxtrace_info() to handle the processing of
>> PERF_RECORD_AUXTRACE_INFO records. In this function, initialize
>> the aux buffer queues using auxtrace_queues__init(). Setup the
>> required infrastructure for aux data processing. The data is collected
>> per CPU and auxtrace_queue is created for each CPU.
>> 
>> Define powerpc_vpadtl_process_event() function to process
>> PERF_RECORD_AUXTRACE records. In this, add the event to queue using
>> auxtrace_queues__add_event() and process the buffer in
>> powerpc_vpadtl_dump_event(). The first entry in the buffer with
>> timebase as zero has boot timebase and frequency. Remaining data is of
>> format for "struct dtl_entry". Define the translation for
>> dispatch_reasons and preempt_reasons, report this when dump trace is
>> invoked via powerpc_vpadtl_dump()
>> 
>> Sample output:
>> 
>>   ./perf record -a -e sched:*,vpa_dtl/dtl_all/ -c 1000000000 sleep 1
>>   [ perf record: Woken up 1 times to write data ]
>>   [ perf record: Captured and wrote 0.300 MB perf.data ]
>> 
>>   ./perf report -D
>> 
>>   0 0 0x39b10 [0x30]: PERF_RECORD_AUXTRACE size: 0x690  offset: 0  ref: 0  idx: 0  tid: -1  cpu: 0
>>   .
>>   . ... VPA DTL PMU data: size 1680 bytes, entries is 35
>>   .  00000000: boot_tb: 21349649546353231, tb_freq: 512000000
>>   .  00000030: dispatch_reason:decrementer interrupt, preempt_reason:H_CEDE, enqueue_to_dispatch_time:7064, ready_to_enqueue_time:187, waiting_to_ready_time:6611773
>>   .  00000060: dispatch_reason:priv doorbell, preempt_reason:H_CEDE, enqueue_to_dispatch_time:146, ready_to_enqueue_time:0, waiting_to_ready_time:15359437
>>   .  00000090: dispatch_reason:decrementer interrupt, preempt_reason:H_CEDE, enqueue_to_dispatch_time:4868, ready_to_enqueue_time:232, waiting_to_ready_time:5100709
>>   .  000000c0: dispatch_reason:priv doorbell, preempt_reason:H_CEDE, enqueue_to_dispatch_time:179, ready_to_enqueue_time:0, waiting_to_ready_time:30714243
>>   .  000000f0: dispatch_reason:priv doorbell, preempt_reason:H_CEDE, enqueue_to_dispatch_time:197, ready_to_enqueue_time:0, waiting_to_ready_time:15350648
>>   .  00000120: dispatch_reason:priv doorbell, preempt_reason:H_CEDE, enqueue_to_dispatch_time:213, ready_to_enqueue_time:0, waiting_to_ready_time:15353446
>>   .  00000150: dispatch_reason:priv doorbell, preempt_reason:H_CEDE, enqueue_to_dispatch_time:212, ready_to_enqueue_time:0, waiting_to_ready_time:15355126
>>   .  00000180: dispatch_reason:decrementer interrupt, preempt_reason:H_CEDE, enqueue_to_dispatch_time:6368, ready_to_enqueue_time:164, waiting_to_ready_time:5104665
>> 
>> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
>> ---
>> tools/perf/util/Build            |   1 +
>> tools/perf/util/auxtrace.c       |   2 +
>> tools/perf/util/powerpc-vpadtl.c | 299 +++++++++++++++++++++++++++++++
>> 3 files changed, 302 insertions(+)
>> create mode 100644 tools/perf/util/powerpc-vpadtl.c
>> 
>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
>> index 4959e7a990e4..5ead46dc98e7 100644
>> --- a/tools/perf/util/Build
>> +++ b/tools/perf/util/Build
>> @@ -136,6 +136,7 @@ perf-util-$(CONFIG_AUXTRACE) += arm-spe-decoder/
>> perf-util-$(CONFIG_AUXTRACE) += hisi-ptt.o
>> perf-util-$(CONFIG_AUXTRACE) += hisi-ptt-decoder/
>> perf-util-$(CONFIG_AUXTRACE) += s390-cpumsf.o
>> +perf-util-$(CONFIG_AUXTRACE) += powerpc-vpadtl.o
>> 
>> ifdef CONFIG_LIBOPENCSD
>> perf-util-$(CONFIG_AUXTRACE) += cs-etm.o
>> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
>> index f587d386c5ef..bd1404f26bb7 100644
>> --- a/tools/perf/util/auxtrace.c
>> +++ b/tools/perf/util/auxtrace.c
>> @@ -1395,6 +1395,8 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
>> err = hisi_ptt_process_auxtrace_info(event, session);
>> break;
>> case PERF_AUXTRACE_VPA_PMU:
>> + err = powerpc_vpadtl_process_auxtrace_info(event, session);
>> + break;
>> case PERF_AUXTRACE_UNKNOWN:
>> default:
>> return -EINVAL;
>> diff --git a/tools/perf/util/powerpc-vpadtl.c b/tools/perf/util/powerpc-vpadtl.c
>> new file mode 100644
>> index 000000000000..ea7b59c45f4a
>> --- /dev/null
>> +++ b/tools/perf/util/powerpc-vpadtl.c
>> @@ -0,0 +1,299 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * VPA DTL PMU support
>> + */
>> +
>> +#include <endian.h>
>> +#include <errno.h>
>> +#include <byteswap.h>
>> +#include <inttypes.h>
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <linux/bitops.h>
>> +#include <linux/log2.h>
>> +#include <elf.h>
>> +#include <limits.h>
>> +
>> +#include "cpumap.h"
>> +#include "color.h"
>> +#include "evsel.h"
>> +#include "evlist.h"
>> +#include "machine.h"
>> +#include "session.h"
>> +#include "util.h"
>> +#include "thread.h"
>> +#include "debug.h"
>> +#include "auxtrace.h"
>> +#include "powerpc-vpadtl.h"
>> +#include "map.h"
>> +#include "symbol_conf.h"
>> +#include "symbol.h"
> 
> Are all these #includes really needed
> 
>> +
>> +/*
>> + * The DTL entries are of below format
>> + */
>> +struct dtl_entry {
>> + u8      dispatch_reason;
>> + u8      preempt_reason;
>> + u16     processor_id;
>> + u32     enqueue_to_dispatch_time;
>> + u32     ready_to_enqueue_time;
>> + u32     waiting_to_ready_time;
>> + u64     timebase;
>> + u64     fault_addr;
>> + u64     srr0;
>> + u64     srr1;
>> +};
> 
> struct dtl_entry is moved in a later patch.
> Maybe call it vpadtl_entry or powerpc_vpadtl_entry and
> put it in perf/util/event.h since it is eventually needed
> in perf/builtin-script.c
Sure


> 
>> +
>> +/*
>> + * Structure to save the auxtrace queue
>> + */
>> +struct powerpc_vpadtl {
>> + struct auxtrace auxtrace;
>> + struct auxtrace_queues queues;
>> + struct auxtrace_heap heap;
>> + u32 auxtrace_type;
>> + struct perf_session *session;
>> + struct machine *machine;
>> + u32 pmu_type;
>> +};
>> +
>> +struct boottb_freq {
>> + u64     boot_tb;
>> + u64     tb_freq;
>> + u64     timebase;
>> + u64     padded[3];
>> +};
>> +
>> +struct powerpc_vpadtl_queue {
>> + struct powerpc_vpadtl *vpa;
>> + unsigned int queue_nr;
>> + struct auxtrace_buffer *buffer;
>> + struct thread *thread;
>> + bool on_heap;
>> + bool done;
>> + pid_t pid;
>> + pid_t tid;
>> + int cpu;
>> +};
>> +
>> +const char *dispatch_reasons[11] = {
>> + "external_interrupt",
>> + "firmware_internal_event",
>> + "H_PROD",
>> + "decrementer_interrupt",
>> + "system_reset",
>> + "firmware_internal_event",
>> + "conferred_cycles",
>> + "time_slice",
>> + "virtual_memory_page_fault",
>> + "expropriated_adjunct",
>> + "priv_doorbell"};
>> +
>> +const char *preempt_reasons[10] = {
>> + "unused",
>> + "firmware_internal_event",
>> + "H_CEDE",
>> + "H_CONFER",
>> + "time_slice",
>> + "migration_hibernation_page_fault",
>> + "virtual_memory_page_fault",
>> + "H_CONFER_ADJUNCT",
>> + "hcall_adjunct",
>> + "HDEC_adjunct"};
>> +
>> +#define dtl_entry_size 48
> 
> sizeof(struct dtl_entry) ?
> 
>> +
>> +/*
>> + * Function to dump the dispatch trace data when perf report
>> + * is invoked with -D
>> + */
>> +static void powerpc_vpadtl_dump(struct powerpc_vpadtl *vpa __maybe_unused,
>> +  unsigned char *buf, size_t len)
>> +{
>> + struct dtl_entry *dtl;
>> + int pkt_len, pos = 0;
>> + const char *color = PERF_COLOR_BLUE;
>> +
>> + color_fprintf(stdout, color,
>> + ". ... VPA DTL PMU data: size %zu bytes, entries is %zu\n",
>> + len, len/dtl_entry_size);
>> +
>> + if (len % dtl_entry_size)
>> + len = len - (len % dtl_entry_size);
>> +
>> + while (len) {
>> + pkt_len = 48;
> 
> dtl_entry_size ?
Yes, thanks for pointing out. Will change in V2
> 
>> + printf(".");
>> + color_fprintf(stdout, color, "  %08x: ", pos);
>> + dtl = (struct dtl_entry *)buf;
>> + if (dtl->timebase != 0) {
>> + printf("dispatch_reason:%s, preempt_reason:%s, enqueue_to_dispatch_time:%d, ready_to_enqueue_time:%d, waiting_to_ready_time:%d\n",
>> + dispatch_reasons[dtl->dispatch_reason], preempt_reasons[dtl->preempt_reason], be32_to_cpu(dtl->enqueue_to_dispatch_time),
>> + be32_to_cpu(dtl->ready_to_enqueue_time), be32_to_cpu(dtl->waiting_to_ready_time));
> 
> Lines are getting a bit long

Will split these.
> 
>> + } else {
>> + struct boottb_freq *boot_tb = (struct boottb_freq *)buf;
>> +
>> + printf("boot_tb: %" PRIu64 ", tb_freq: %" PRIu64 "\n", boot_tb->boot_tb, boot_tb->tb_freq);
>> + }
>> +
>> + pos += pkt_len;
>> + buf += pkt_len;
>> + len -= pkt_len;
>> + }
>> +}
>> +
>> +static void powerpc_vpadtl_dump_event(struct powerpc_vpadtl *vpa, unsigned char *buf,
>> +        size_t len)
>> +{
>> + printf(".\n");
>> + powerpc_vpadtl_dump(vpa, buf, len);
>> +}
>> +
>> +static int powerpc_vpadtl_process_event(struct perf_session *session __maybe_unused,
>> +  union perf_event *event __maybe_unused,
>> +  struct perf_sample *sample __maybe_unused,
>> +  const struct perf_tool *tool __maybe_unused)
>> +{
>> + return 0;
>> +}
>> +
>> +/*
>> + * Process PERF_RECORD_AUXTRACE records
>> + */
>> +static int powerpc_vpadtl_process_auxtrace_event(struct perf_session *session,
>> +   union perf_event *event,
>> +   const struct perf_tool *tool __maybe_unused)
>> +{
>> + struct powerpc_vpadtl *vpa = container_of(session->auxtrace, struct powerpc_vpadtl,
>> +      auxtrace);
> 
> Might be worth adding a helper like
> 
> static struct powerpc_vpadtl *session_to_vpa(struct perf_session *session)
> {
> return container_of(session->auxtrace, struct powerpc_vpadtl, auxtrace);
> }
Ok Adrian
> 
>> + struct auxtrace_buffer *buffer;
>> + off_t data_offset;
>> + int fd = perf_data__fd(session->data);
>> + int err;
>> +
>> + if (perf_data__is_pipe(session->data)) {
>> + data_offset = 0;
>> + } else {
>> + data_offset = lseek(fd, 0, SEEK_CUR);
>> + if (data_offset == -1)
>> + return -errno;
>> + }
>> +
>> + err = auxtrace_queues__add_event(&vpa->queues, session, event,
>> +  data_offset, &buffer);
> 
> auxtrace_queues__add_event() is only needed here if there is no
> auxtrace index, however an auxtrace index is always written for
> new perf.data files.  The index gets processed and data queued
> by auxtrace_queues__process_index() which is added in patch 11.
> 
> Piped data, on the other hand, has no index and needs to be
> handled here.
> 
> So:
> 
> if (perf_data__is_pipe(session->data)) {
> err = auxtrace_queues__add_event(&vpa->queues, session, event, 0, &buffer);
> if (err)
> return err;
> }
> 
> 

Ok, will handle this change

>> + if (err)
>> + return err;
>> +
>> + /* Dump here now we have copied a piped trace out of the pipe */
>> + if (dump_trace) {
>> + if (auxtrace_buffer__get_data(buffer, fd)) {
>> + powerpc_vpadtl_dump_event(vpa, buffer->data,
>> +      buffer->size);
> 
> Unnecessary line wrap
> 
>> + auxtrace_buffer__put_data(buffer);
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int powerpc_vpadtl_flush(struct perf_session *session __maybe_unused,
>> +  const struct perf_tool *tool __maybe_unused)
>> +{
>> + return 0;
>> +}
>> +
>> +static void powerpc_vpadtl_free_queue(void *priv)
>> +{
>> + struct powerpc_vpadtl_queue *vpaq = priv;
>> +
>> + if (!vpaq)
>> + return;
>> +
>> + free(vpaq);
>> +}
>> +
>> +static void powerpc_vpadtl_free_events(struct perf_session *session)
>> +{
>> + struct powerpc_vpadtl *vpa = container_of(session->auxtrace, struct powerpc_vpadtl,
>> +      auxtrace);
>> + struct auxtrace_queues *queues = &vpa->queues;
>> + unsigned int i;
>> +
>> + for (i = 0; i < queues->nr_queues; i++) {
>> + powerpc_vpadtl_free_queue(queues->queue_array[i].priv);
> 
> This is the same as free(queues->queue_array[i].priv)
> 
>> + queues->queue_array[i].priv = NULL;
> 
> Could all be reduced to zfree(queues->queue_array[i].priv)
Sure
> 
>> + }
>> + auxtrace_queues__free(queues);
>> +}
>> +
>> +static void powerpc_vpadtl_free(struct perf_session *session)
>> +{
>> + struct powerpc_vpadtl *vpa = container_of(session->auxtrace, struct powerpc_vpadtl,
>> +      auxtrace);
>> +
>> + auxtrace_heap__free(&vpa->heap);
>> + powerpc_vpadtl_free_events(session);
>> + session->auxtrace = NULL;
>> + free(vpa);
>> +}
>> +
>> +static const char * const powerpc_vpadtl_info_fmts[] = {
>> + [POWERPC_VPADTL_TYPE] = "  PMU Type           %"PRId64"\n",
>> +};
>> +
>> +static void powerpc_vpadtl_print_info(__u64 *arr)
>> +{
>> + if (!dump_trace)
>> + return;
>> +
>> + fprintf(stdout, powerpc_vpadtl_info_fmts[POWERPC_VPADTL_TYPE], arr[POWERPC_VPADTL_TYPE]);
>> +}
>> +
>> +/*
>> + * Process the PERF_RECORD_AUXTRACE_INFO records and setup
>> + * the infrastructure to process auxtrace events. PERF_RECORD_AUXTRACE_INFO
>> + * is processed first since it is of type perf_user_event_type.
>> + * Initialise the aux buffer queues using auxtrace_queues__init().
>> + * auxtrace_queue is created for each CPU.
>> + */
>> +int powerpc_vpadtl_process_auxtrace_info(union perf_event *event,
>> +   struct perf_session *session)
>> +{
>> + struct perf_record_auxtrace_info *auxtrace_info = &event->auxtrace_info;
>> + size_t min_sz = sizeof(u64) * POWERPC_VPADTL_TYPE;
>> + struct powerpc_vpadtl *vpa;
>> + int err;
>> +
>> + if (auxtrace_info->header.size < sizeof(struct perf_record_auxtrace_info) +
>> + min_sz)
>> + return -EINVAL;
>> +
>> + vpa = zalloc(sizeof(struct powerpc_vpadtl));
>> + if (!vpa)
>> + return -ENOMEM;
>> +
>> + err = auxtrace_queues__init(&vpa->queues);
>> + if (err)
>> + goto err_free;
>> +
>> + vpa->session = session;
>> + vpa->machine = &session->machines.host; /* No kvm support */
>> + vpa->auxtrace_type = auxtrace_info->type;
>> + vpa->pmu_type = auxtrace_info->priv[POWERPC_VPADTL_TYPE];
>> +
>> + vpa->auxtrace.process_event = powerpc_vpadtl_process_event;
>> + vpa->auxtrace.process_auxtrace_event = powerpc_vpadtl_process_auxtrace_event;
>> + vpa->auxtrace.flush_events = powerpc_vpadtl_flush;
>> + vpa->auxtrace.free_events = powerpc_vpadtl_free_events;
>> + vpa->auxtrace.free = powerpc_vpadtl_free;
>> + session->auxtrace = &vpa->auxtrace;
>> +
>> + powerpc_vpadtl_print_info(&auxtrace_info->priv[0]);
>> +
>> + return 0;
>> +
>> +err_free:
>> + free(vpa);
>> + return err;
>> +}