Re: [PATCH v2 10/17] arm64: Enable workaround for TRBE overwrite in FILL mode

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 22/09/2021 08:23, Anshuman Khandual wrote:
> 
> 
> On 9/21/21 7:11 PM, Suzuki K Poulose wrote:
>> Now that we have the work around implmented in the TRBE
>> driver, add the Kconfig entries and document the errata.
>>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: Leo Yan <leo.yan@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   Documentation/arm64/silicon-errata.rst |  4 +++
>>   arch/arm64/Kconfig                     | 39 ++++++++++++++++++++++++++
>>   2 files changed, 43 insertions(+)
>>
>> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
>> index d410a47ffa57..2f99229d993c 100644
>> --- a/Documentation/arm64/silicon-errata.rst
>> +++ b/Documentation/arm64/silicon-errata.rst
>> @@ -92,12 +92,16 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A710     | #2119858        | ARM64_ERRATUM_2119858       |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N1     | #1188873,1418040| ARM64_ERRATUM_1418040       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | MMU-500         | #841119,826419  | N/A                         |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   +----------------+-----------------+-----------------+-----------------------------+
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 077f2ec4eeb2..eac4030322df 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -666,6 +666,45 @@ config ARM64_ERRATUM_1508412
>>   
>>   	  If unsure, say Y.
>>   
>> +config ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>> +	bool
>> +
>> +config ARM64_ERRATUM_2119858
>> +	bool "Cortex-A710: 2119858: workaround TRBE overwriting trace data in FILL mode"
>> +	default y
>> +	depends on CORESIGHT_TRBE
>> +	select ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>> +	help
>> +	  This option adds the workaround for ARM Cortex-A710 erratum 2119858.
>> +
>> +	  Affected Cortex-A710 cores could overwrite upto 3 cache lines of trace
>> +	  data at the base of the buffer (ponited by TRBASER_EL1) in FILL mode in
>> +	  the event of a WRAP event.
>> +
>> +	  Work around the issue by always making sure we move the TRBPTR_EL1 by
>> +	  256bytes before enabling the buffer and filling the first 256bytes of
>> +	  the buffer with ETM ignore packets upon disabling.
>> +
>> +	  If unsure, say Y.
>> +
>> +config ARM64_ERRATUM_2139208
>> +	bool "Neoverse-N2: 2139208: workaround TRBE overwriting trace data in FILL mode"
>> +	default y
>> +	depends on CORESIGHT_TRBE
>> +	select ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>> +	help
>> +	  This option adds the workaround for ARM Neoverse-N2 erratum 2139208.
>> +
>> +	  Affected Neoverse-N2 cores could overwrite upto 3 cache lines of trace
>> +	  data at the base of the buffer (ponited by TRBASER_EL1) in FILL mode in
> 
> s/ponited/pointed
> 
>> +	  the event of a WRAP event.
>> +
>> +	  Work around the issue by always making sure we move the TRBPTR_EL1 by
>> +	  256bytes before enabling the buffer and filling the first 256bytes of
>> +	  the buffer with ETM ignore packets upon disabling.
>> +
>> +	  If unsure, say Y.
>> +
>>   config CAVIUM_ERRATUM_22375
>>   	bool "Cavium erratum 22375, 24313"
>>   	default y
>>
> 
> The real errata problem description for both these erratums are exactly
> the same. Rather a more generalized description should be included for
> the ARM64_WORKAROUND_TRBE_OVERWRITE_FILL_MODE, which abstracts out the
> problem and a corresponding solution that is implemented in the driver.
> This should also help us reduce current redundancy.
> 

The issue is what a user wants to see. A user who wants to configure the
kernel specifically for a given CPU (think embedded systems), they would
want to hand pick the errata for the particular CPU. So, moving the help
text to an implicitly selected Kconfig symbol. I would rather keep this
as it is to keep it user friendly. This doesn't affect the code size
anyways.

The other option is to remove all the CPU specific Kconfig symbols and
update the "title" to reflect both the CPU/erratum numbers.

Kind regards
Suzuki