linux-kernel - Re: [PATCH v3 23/26] coresight: etm4x: Detect system instructions support

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: <b4a4bd73-00c5-a921-3f01-7100d2a0edf8@arm.com>
Date:   Tue, 10 Nov 2020 09:31:42 +0000
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     Mathieu Poirier <mathieu.poirier@...aro.org>
Cc:     linux-arm-kernel@...ts.infradead.org, mike.leach@...aro.org,
        coresight@...ts.linaro.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 23/26] coresight: etm4x: Detect system instructions
 support

On 11/9/20 8:22 PM, Mathieu Poirier wrote:
> On Wed, Oct 28, 2020 at 10:09:42PM +0000, Suzuki K Poulose wrote:
>> ETM v4.4 onwards adds support for system instruction access
>> to the ETM. Detect the support on an ETM and switch to using the
>> mode when available.
>>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@....com>
>> ---
>>   .../coresight/coresight-etm4x-core.c          | 39 +++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 4bc2f15b6332..dc537b5612eb 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -675,6 +675,37 @@ static const struct coresight_ops etm4_cs_ops = {
>>   	.source_ops	= &etm4_source_ops,
>>   };
>>   
>> +static inline bool cpu_supports_sysreg_trace(void)
>> +{
>> +	u64 dfr0 = read_sysreg_s(SYS_ID_AA64DFR0_EL1);
>> +
>> +	return ((dfr0 >> ID_AA64DFR0_TRACEVER_SHIFT) & 0xfUL) > 0;
> 
> I would do:
> 
>          return ((dfr0 >> ID_AA64DFR0_TRACEVER_SHIFT) & 0xfUL) == 1;
> 
> Because any other value than '1' are reserved.

Correct. However, this is something we follow for all ID features
in the arm64 kernel and is clarified in the Arm ARM (ARM DDI 0487F.a) :

"D13.1.3 Principles of the ID scheme for fields in ID registers"

Which guarantees that a (field  > n) implies, everything that field == n
is implied. (Well there are exceptions listed in the section, but
TRACEVER is not one of those). So this should cover an old kernel
running on a future CPU, using the features that it understands.
(See feature_matches() in arch/arm64/kernel/cpufeature.c, which is
the fundamental logic to detect a feature).

Please let me know if you're OK with the justification.

Thanks for the review.

Suzuki