Messages in this thread | | | Date | Sat, 10 Jun 2023 10:33:35 +0800 | From | Leo Yan <> | Subject | Re: [PATCH v2 4/5] perf cs-etm: Track exception level |
| |
On Thu, Jun 08, 2023 at 11:59:28AM +0100, James Clark wrote: > Currently we assume all trace belongs to the host machine so when > the decoder should be looking at the guest kernel maps it can crash > because it looks at the host ones instead. > > Avoid one scenario (guest kernel running at EL1) by assigning the > default guest machine to this trace. For userspace trace it's still not > possible to determine guest vs host, but the PIDs should help in this > case. > > Signed-off-by: James Clark <james.clark@arm.com>
Reviewed-by: Leo Yan <leo.yan@linaro.org>
Just input a thought for support both host and guest machines, seems to me, a feasibe solution is CoreSight can extend its capability to trace below system registers together:
- VTTBR_EL2.vmid - CONTEXTIDR_EL2 - CONTEXTIDR_EL1
So:
VTTBR_EL2.vmid == 0: host machine, CONTEXTIDR_EL2 is for tracing PID VTTBR_EL2.vmid != 0: guest machine, CONTEXTIDR_EL1 is for tracing PID
So far we only can trace either CONTEXTIDR_EL2 (and it's even a bit mess that we save CONTEXTIDR_EL2 into 'elem->context.vmid') or CONTEXTIDR_EL1, this is the main reason it's hard for perf to distinguish host and guest. I know you might have discussed this yet, here I just record some ideas in case later we can review it.
Thanks, Leo
> --- > .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 7 +- > tools/perf/util/cs-etm.c | 75 +++++++++++++++---- > tools/perf/util/cs-etm.h | 7 +- > 3 files changed, 67 insertions(+), 22 deletions(-) > > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > index 2af641d26866..44c49acd6bff 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > @@ -561,12 +561,13 @@ cs_etm_decoder__set_tid(struct cs_etm_queue *etmq, > break; > } > > + if (cs_etm__etmq_set_tid_el(etmq, tid, trace_chan_id, > + elem->context.exception_level)) > + return OCSD_RESP_FATAL_SYS_ERR; > + > if (tid == -1) > return OCSD_RESP_CONT; > > - if (cs_etm__etmq_set_tid(etmq, tid, trace_chan_id)) > - return OCSD_RESP_FATAL_SYS_ERR; > - > /* > * A timestamp is generated after a PE_CONTEXT element so make sure > * to rely on that coming one. > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index e0904f276e89..916e86f003f4 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -14,7 +14,6 @@ > #include <linux/types.h> > #include <linux/zalloc.h> > > -#include <opencsd/ocsd_if_types.h> > #include <stdlib.h> > > #include "auxtrace.h" > @@ -88,6 +87,8 @@ struct cs_etm_traceid_queue { > union perf_event *event_buf; > struct thread *thread; > struct thread *prev_thread; > + ocsd_ex_level prev_el; > + ocsd_ex_level el; > struct branch_stack *last_branch; > struct branch_stack *last_branch_rb; > struct cs_etm_packet *prev_packet; > @@ -482,6 +483,7 @@ static int cs_etm__init_traceid_queue(struct cs_etm_queue *etmq, > > queue = &etmq->etm->queues.queue_array[etmq->queue_nr]; > tidq->trace_chan_id = trace_chan_id; > + tidq->el = tidq->prev_el = ocsd_EL_unknown; > tidq->thread = machine__findnew_thread(&etm->session->machines.host, -1, > queue->tid); > tidq->prev_thread = machine__idle_thread(&etm->session->machines.host); > @@ -621,6 +623,7 @@ static void cs_etm__packet_swap(struct cs_etm_auxtrace *etm, > tmp = tidq->packet; > tidq->packet = tidq->prev_packet; > tidq->prev_packet = tmp; > + tidq->prev_el = tidq->el; > thread__put(tidq->prev_thread); > tidq->prev_thread = thread__get(tidq->thread); > } > @@ -882,11 +885,43 @@ static bool cs_etm__evsel_is_auxtrace(struct perf_session *session, > return evsel->core.attr.type == aux->pmu_type; > } > > -static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address) > +static struct machine *cs_etm__get_machine(struct cs_etm_queue *etmq, > + ocsd_ex_level el) > { > - struct machine *machine; > + enum cs_etm_pid_fmt pid_fmt = cs_etm__get_pid_fmt(etmq); > > - machine = &etmq->etm->session->machines.host; > + /* > + * For any virtualisation based on nVHE (e.g. pKVM), or host kernels > + * running at EL1 assume everything is the host. > + */ > + if (pid_fmt == CS_ETM_PIDFMT_CTXTID) > + return &etmq->etm->session->machines.host; > + > + /* > + * Not perfect, but otherwise assume anything in EL1 is the default > + * guest, and everything else is the host. Distinguishing between guest > + * and host userspaces isn't currently supported either. Neither is > + * multiple guest support. All this does is reduce the likeliness of > + * decode errors where we look into the host kernel maps when it should > + * have been the guest maps. > + */ > + switch (el) { > + case ocsd_EL1: > + return machines__find_guest(&etmq->etm->session->machines, > + DEFAULT_GUEST_KERNEL_ID); > + case ocsd_EL3: > + case ocsd_EL2: > + case ocsd_EL0: > + case ocsd_EL_unknown: > + default: > + return &etmq->etm->session->machines.host; > + } > +} > + > +static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address, > + ocsd_ex_level el) > +{ > + struct machine *machine = cs_etm__get_machine(etmq, el); > > if (address >= machine__kernel_start(machine)) { > if (machine__is_host(machine)) > @@ -896,10 +931,14 @@ static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address) > } else { > if (machine__is_host(machine)) > return PERF_RECORD_MISC_USER; > - else if (perf_guest) > + else { > + /* > + * Can't really happen at the moment because > + * cs_etm__get_machine() will always return > + * machines.host for any non EL1 trace. > + */ > return PERF_RECORD_MISC_GUEST_USER; > - else > - return PERF_RECORD_MISC_HYPERVISOR; > + } > } > } > > @@ -916,11 +955,12 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id, > if (!etmq) > return 0; > > - cpumode = cs_etm__cpu_mode(etmq, address); > tidq = cs_etm__etmq_get_traceid_queue(etmq, trace_chan_id); > if (!tidq) > return 0; > > + cpumode = cs_etm__cpu_mode(etmq, address, tidq->el); > + > if (!thread__find_map(tidq->thread, cpumode, address, &al)) > return 0; > > @@ -1298,10 +1338,11 @@ cs_etm__get_trace(struct cs_etm_queue *etmq) > return etmq->buf_len; > } > > -static void cs_etm__set_thread(struct cs_etm_auxtrace *etm, > - struct cs_etm_traceid_queue *tidq, pid_t tid) > +static void cs_etm__set_thread(struct cs_etm_queue *etmq, > + struct cs_etm_traceid_queue *tidq, pid_t tid, > + ocsd_ex_level el) > { > - struct machine *machine = &etm->session->machines.host; > + struct machine *machine = cs_etm__get_machine(etmq, el); > > if (tid != -1) { > thread__zput(tidq->thread); > @@ -1311,10 +1352,12 @@ static void cs_etm__set_thread(struct cs_etm_auxtrace *etm, > /* Couldn't find a known thread */ > if (!tidq->thread) > tidq->thread = machine__idle_thread(machine); > + > + tidq->el = el; > } > > -int cs_etm__etmq_set_tid(struct cs_etm_queue *etmq, > - pid_t tid, u8 trace_chan_id) > +int cs_etm__etmq_set_tid_el(struct cs_etm_queue *etmq, pid_t tid, > + u8 trace_chan_id, ocsd_ex_level el) > { > struct cs_etm_traceid_queue *tidq; > > @@ -1322,7 +1365,7 @@ int cs_etm__etmq_set_tid(struct cs_etm_queue *etmq, > if (!tidq) > return -EINVAL; > > - cs_etm__set_thread(etmq->etm, tidq, tid); > + cs_etm__set_thread(etmq, tidq, tid, el); > return 0; > } > > @@ -1392,7 +1435,7 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq, > struct perf_sample sample = {.ip = 0,}; > > event->sample.header.type = PERF_RECORD_SAMPLE; > - event->sample.header.misc = cs_etm__cpu_mode(etmq, addr); > + event->sample.header.misc = cs_etm__cpu_mode(etmq, addr, tidq->el); > event->sample.header.size = sizeof(struct perf_event_header); > > /* Set time field based on etm auxtrace config. */ > @@ -1451,7 +1494,7 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq, > ip = cs_etm__last_executed_instr(tidq->prev_packet); > > event->sample.header.type = PERF_RECORD_SAMPLE; > - event->sample.header.misc = cs_etm__cpu_mode(etmq, ip); > + event->sample.header.misc = cs_etm__cpu_mode(etmq, ip, tidq->prev_el); > event->sample.header.size = sizeof(struct perf_event_header); > > /* Set time field based on etm auxtrace config. */ > diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h > index 2f47f4ec5b27..7cca37887917 100644 > --- a/tools/perf/util/cs-etm.h > +++ b/tools/perf/util/cs-etm.h > @@ -251,10 +251,11 @@ enum cs_etm_pid_fmt { > }; > > #ifdef HAVE_CSTRACE_SUPPORT > +#include <opencsd/ocsd_if_types.h> > int cs_etm__get_cpu(u8 trace_chan_id, int *cpu); > -enum pid_fmt cs_etm__get_pid_fmt(struct cs_etm_queue *etmq); > -int cs_etm__etmq_set_tid(struct cs_etm_queue *etmq, > - pid_t tid, u8 trace_chan_id); > +enum cs_etm_pid_fmt cs_etm__get_pid_fmt(struct cs_etm_queue *etmq); > +int cs_etm__etmq_set_tid_el(struct cs_etm_queue *etmq, pid_t tid, > + u8 trace_chan_id, ocsd_ex_level el); > bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq); > void cs_etm__etmq_set_traceid_queue_timestamp(struct cs_etm_queue *etmq, > u8 trace_chan_id); > -- > 2.34.1 >
| |