linux-kernel - Re: [RFC] perf arm-spe: Track task context switch for cpu-mode events

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <be937a2e-311b-2a8b-1094-39c203c6d9f3@arm.com>
Date:   Fri, 8 Oct 2021 12:07:55 +0100
From:   German Gomez <german.gomez@....com>
To:     Namhyung Kim <namhyung@...nel.org>, Leo Yan <leo.yan@...aro.org>
Cc:     James Clark <james.clark@....com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>, Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Ian Rogers <irogers@...gle.com>,
        Stephane Eranian <eranian@...gle.com>,
        Adrian Hunter <adrian.hunter@...el.com>
Subject: Re: [RFC] perf arm-spe: Track task context switch for cpu-mode events

Hi Leo, Namhyung,

On 06/10/2021 17:09, Namhyung Kim wrote:
> Hi Leo and German,
>
> [...]
>
> I think it'd be better to check it in perf record and not set
> evsel->core.attr.context_switch if possible.
>
> Or it can ignore the context switch once it sees a context packet.
>
>> Here should note one thing is the perf tool needs to have knowledge to
>> decide if the bit 3 'CX' (macro 'SYS_PMSCR_EL1_CX_SHIFT' in kernel) has
>> been set in register PMSCR or not.  AFAIK, Arm SPE driver doesn't
>> expose any interface (or config) to userspace for the context tracing,
>> so one method is to add an extra config in Arm SPE driver for this
>> bit, e.g. 'ATTR_CFG_FLD_cx_enable_CFG' can be added in Arm SPE driver.
>>
>> Alternatively, rather than adding new config, I am just wandering we
>> simply use two flags in perf's decoding: 'use_switch_event_for_pid' and
>> 'use_ctx_packet_for_pid', the first variable will be set if detects
>> the tracing is userspace only, the second varaible will be set when
>> detects the hardware tracing containing context packet.  So if the
>> variable 'use_ctx_packet_for_pid' has been set, then the decoder will
>> always use context packet for sample's PID, otherwise, it falls back
>> to check 'use_switch_event_for_pid' and set sample PID based on switch
>> events.
>>
>> If have any other idea, please feel free bring up.
> If it's just kernel config, we can check /proc/config.gz or
> /boot/config-$(uname -r).  When it knows for sure it can just use
> the context packet, otherwise it needs the context switch.
>
> Thanks,
> Namhyung

Please correct me if I'm wrong, after disabling the PID_IN_CONTEXTIDR
feature in the kernel, I don't see any context packets in the auxtraces.
I think after applying the patch from [1], it should be sufficient to
determine if pid tracing should fall back to use --switch-events when
context_id from that patch has a value of -1.

If the patch at the end of this message is applied on top of Namhyuna's
and [1], I think it can work. Also, if the pmu driver is patched to
disable the 'CX' bit when the pid is not in the root namespace [2]
(unfortunately I haven't been able to set up an environment to properly
test Leo's patch yet) tracing could also fall back to context-switch for
userspace tracing. What do you think?

Thanks,
German

[1] https://www.spinics.net/lists/linux-perf-users/msg12543.html
[2] https://lore.kernel.org/lkml/20210916135418.GA383600@leoy-ThinkPad-X240s/

diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index 708323d..e224665 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -71,6 +71,12 @@
     u64                kernel_start;
 
     unsigned long            num_events;
+
+    /*
+     * Used for PID tracing.
+     */
+    u8                use_context_id_pkt;
+    u8                use_context_switch_event;
 };
 
 struct arm_spe_queue {
@@ -586,13 +592,30 @@
     return timeless_decoding;
 }
 
+static bool arm_spe__is_exclude_kernel(struct arm_spe *spe) {
+    struct evsel *evsel;
+    struct evlist *evlist = spe->session->evlist;
+
+    evlist__for_each_entry(evlist, evsel) {
+        if (evsel->core.attr.type == spe->pmu_type && evsel->core.attr.exclude_kernel)
+            return true;
+    }
+
+    return false;
+}
+
 static void arm_spe_set_pid_tid_cpu(struct arm_spe *spe,
                     struct auxtrace_queue *queue)
 {
     struct arm_spe_queue *speq = queue->priv;
-    pid_t tid;
+    pid_t tid = -1;
 
-    tid = machine__get_current_tid(spe->machine, speq->cpu);
+    if (spe->use_context_id_pkt)
+        tid = speq->decoder->record.context_id;
+
+    if (tid == -1 && spe->use_context_switch_event)
+        tid = machine__get_current_tid(spe->machine, speq->cpu);
+
     if (tid != -1) {
         speq->tid = tid;
         thread__zput(speq->thread);
@@ -1084,6 +1107,15 @@
     spe->timeless_decoding = arm_spe__is_timeless_decoding(spe);
 
     /*
+     * Always try to use context packet by default for pid tracing.
+     *
+     * If it's not enabled in the pmu driver, it will always have a value of -1 and we can try
+     * to fall back to using context-switch events instead.
+     */
+    spe->use_context_id_pkt = true;
+    spe->use_context_switch_event = arm_spe__is_exclude_kernel(spe);
+
+    /*
      * The synthesized event PERF_RECORD_TIME_CONV has been handled ahead
      * and the parameters for hardware clock are stored in the session
      * context.  Passes these parameters to the struct perf_tsc_conversion
@@ -1141,4 +1173,4 @@
 err_free:
     free(spe);
     return err;
-}
+}
\ No newline at end of file