Re: [PATCH v4 25/25] coresight: Add support for v8.4 SelfHosted tracing

From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, mathieu.poirier@linaro.org,
	mike.leach@linaro.org, linux-kernel@vger.kernel.org,
	anshuman.khandual@arm.com, jonathan.zhouwen@huawei.com,
	coresight@lists.linaro.org, Will Deacon <will@kernel.org>
Subject: Re: [PATCH v4 25/25] coresight: Add support for v8.4 SelfHosted tracing
Date: Fri, 20 Nov 2020 10:03:00 +0000	[thread overview]
Message-ID: <cac71aef-e596-2e66-1fa0-2a5336cb270e@arm.com> (raw)
In-Reply-To: <20201119172243.GF4376@gaia>

On 11/19/20 5:22 PM, Catalin Marinas wrote:
> On Thu, Nov 19, 2020 at 04:45:47PM +0000, Suzuki K Poulose wrote:
>> From: Jonathan Zhou <jonathan.zhouwen@huawei.com>
>>
>> v8.4 tracing extensions added support for trace filtering controlled
>> by TRFCR_ELx. This must be programmed to allow tracing at EL1/EL2 and
>> EL0. The timestamp used is the virtual time. Also enable CONTEXIDR_EL2
>> tracing if we are running the kernel at EL2.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: Will Deacon <will@kernel.org>
>> Signed-off-by: Jonathan Zhou <jonathan.zhouwen@huawei.com>
>> [ Move the trace filtering setup etm_init_arch_data() and
>>   clean ups]
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   .../coresight/coresight-etm4x-core.c          | 25 +++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 1d054d2ab2a0..647685736134 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -760,6 +760,30 @@ static bool etm4_init_csdev_access(struct etmv4_drvdata *drvdata,
>>   	return false;
>>   }
>>   
>> +static void cpu_enable_tracing(void)
>> +{
>> +	u64 dfr0 = read_sysreg(id_aa64dfr0_el1);
>> +	u64 trfcr;
>> +
>> +	if (!(dfr0 >> ID_AA64DFR0_TRACE_FILT_SHIFT))
>> +		return;
> 
> What if we get a new field at position 44 while the FILT one at 40 is 0?
> We should use cpuid_feature_extract_field() here.
> 

Bah, you're right. Sorry for that silly mistake. Thanks for spotting.

> BTW, can this function not go in the cpufeature.c code?

Yes, this could go there. The only reason for keeping this here
was that the ETM is the only Trace component we support.

Ideally, this should be performed at enable/disable of the ETM,
based on the "config" selected for the session. That way we can
be sure that nobody (read, a VM) traces the host kernel/el0 behind our back
(even though we would trap the access for now).

Suzuki