| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <291150c1-8bbb-ece5-c21f-0f6f37ca2119@arm.com>
Date: Mon, 12 Jun 2023 12:13:40 +0100
From: James Clark <james.clark@....com>
To: Leo Yan <leo.yan@...aro.org>
Cc: coresight@...ts.linaro.org,
Suzuki K Poulose <suzuki.poulose@....com>,
Mike Leach <mike.leach@...aro.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
John Garry <john.g.garry@...cle.com>,
Will Deacon <will@...nel.org>,
linux-arm-kernel@...ts.infradead.org,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/5] perf cs-etm: Track exception level
On 10/06/2023 03:33, Leo Yan wrote:
> 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@....com>
>
> Reviewed-by: Leo Yan <leo.yan@...aro.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
>
I think even if we did that we wouldn't be able to distinguish between
different guests where processes have the same PID. For IntelPT it looks
like there is a way to merge two recordings from host and guest and it
uses the timestamps to attribute samples to particular guests. I don't
see why this can't work for Coresight too and it doesn't require any
changes to the driver or hardware.
James
>> ---
>> .../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
>>
Powered by blists - more mailing lists