Re: [PATCH 1/3] coresight: etm-perf: Add support for PID tracing for kernel at EL2

Suzuki K Poulose
2020-11-12T12:43:13Z

On 11/12/20 10:27 AM, Leo Yan wrote:
> On Tue, Nov 10, 2020 at 06:33:11PM +0000, Suzuki Kuruppassery Poulose wrote:
>> When the kernel is running at EL2, the PID is stored in CONTEXTIDR_EL2.
>> So, tracing CONTEXTIDR_EL1 doesn't give us the pid of the process.
>> Thus we should trace the VMID with VMIDOPT set to trace
>> CONTEXTIDR_EL2 instead of VMID. Given that we have an existing
>> config option "contextid" and this will be useful for tracing
>> virtual machines (when we get to support virtualization). So instead,
>> this patch adds a new option, contextid_in_vmid as a separate config.
>> Thus on an EL2 kernel, we will have two options available for
>> the perf tool. However, to make it easier for the user to
>> do pid tracing, we add a new format which will default to
>> "contextid" (on EL1 kernel) or "contextid_in_vmid" (on EL2
>> kernel). So that the user doesn't have to bother which EL the
>> kernel is running.
>>
>> i.e, perf record -e cs_etm/pid/u --
>>
>> will always do the "pid" tracing, independent of the kernel EL.
>>
>> Also, the perf tool will be updated to automatically select
>> "pid" config instead of the "contextid" for system wide/CPU wide
>> mode.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Al Grant <al.grant@arm.com>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: Leo Yan <leo.yan@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ++++++++++++++
>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 +++++++++
>> include/linux/coresight-pmu.h | 11 +++++++----
>> 3 files changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> index bdc34ca449f7..f763def145e4 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -30,14 +30,28 @@ static DEFINE_PER_CPU(struct coresight_device *, csdev_src);
>> /* ETMv3.5/PTM's ETMCR is 'config' */
>> PMU_FORMAT_ATTR(cycacc, "config:" __stringify(ETM_OPT_CYCACC));
>> PMU_FORMAT_ATTR(contextid, "config:" __stringify(ETM_OPT_CTXTID));
>> +PMU_FORMAT_ATTR(contextid_in_vmid, "config:" __stringify(ETM_OPT_CTXTID_IN_VMID));
>> PMU_FORMAT_ATTR(timestamp, "config:" __stringify(ETM_OPT_TS));
>> PMU_FORMAT_ATTR(retstack, "config:" __stringify(ETM_OPT_RETSTK));
>> /* Sink ID - same for all ETMs */
>> PMU_FORMAT_ATTR(sinkid, "config2:0-31");
>>
>> +static ssize_t format_attr_pid_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *page)
>> +{
>> + int pid_fmt = is_kernel_in_hyp_mode() ? ETM_OPT_CTXTID_IN_VMID : ETM_OPT_CTXTID;
>> +
>> + return sprintf(page, "config:%d\n", pid_fmt);
>> +}
>> +
>> +struct device_attribute format_attr_pid = __ATTR(pid, 0444, format_attr_pid_show, NULL);
>> +
>> static struct attribute *etm_config_formats_attr[] = {
>> &format_attr_cycacc.attr,
>> &format_attr_contextid.attr,
>> + &format_attr_contextid_in_vmid.attr,
>> + &format_attr_pid.attr,
>> &format_attr_timestamp.attr,
>> &format_attr_retstack.attr,
>> &format_attr_sinkid.attr,
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index d78a37b6592c..8cb2cb1febce 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -382,6 +382,15 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
>> /* bit[6], Context ID tracing bit */
>> config->cfg |= BIT(ETM4_CFG_BIT_CTXTID);
>>
>> + /* Do not enable VMID tracing if we are not running in EL2 */
>> + if (attr->config & BIT(ETM_OPT_CTXTID_IN_VMID)) {
>> + if (!is_kernel_in_hyp_mode()) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + config->cfg |= BIT(ETM4_CFG_BIT_VMID) | BIT(ETM4_CFG_BIT_VMID_OPT);
>> + }
>> +
>
> Nitpick: should the driver need to check the kernel running mode for the
> config ETM_OPT_CTXTID? I.e. if the BIT(ETM_OPT_CTXTID) is set in
> "attr->config", does it mean the kernel should run in EL1 mode?

That doesn't matter. Irrespective of the kernel EL, CTXTIDR_EL1 could
have some value in it. e.g, tracing a VM, CTXTIDR_El1 could have the
guest process PID. So, we should simply leave it for the user to decide.

>
> Just curious, does there have a scenario that we need to set BIT
> ETM4_CFG_BIT_VMID but the bit ETM4_CFG_BIT_VMID_OPT is cleared? I
> think a bit for this, but cannot find a situation for tracing context
> ID based on VTTBR_EL2.VMID. So this might be not a question for us.

Yes, it is possible to use to trace the VMID. e.g,tracing multiple VMs.
That way VMID could be used to separate the trace stream. This series
doesn't affect that. We could add a separate "config" bit for tracing
VMID, but make sure that selected configs don't conflict.

i.,e if (config & ETM_OPT_VMID) && (config & ETM_OPT_CTXTID_IN_VMID)
error !

Thanks
Suzuki