[PATCH 1/3] coresight etm4x: Early clear TRCOSLAR.OSLK prior to TRCIDR1 read

[Suzuki K Poulose]

Missed the reference, see below.

On 23/01/2023 22:18, Suzuki K Poulose wrote:
> On 23/01/2023 19:48, Steve Clevenger wrote:
>>
>>
>> On 1/23/2023 9:33 AM, Suzuki K Poulose wrote:
>>> On 23/01/2023 17:22, Steve Clevenger wrote:
>>>>
>>>>
>>>> On 1/23/2023 2:54 AM, Suzuki K Poulose wrote:
>>>>> On 21/01/2023 07:30, Steve Clevenger wrote:
>>>>>>
>>>>>> Hi Suzuki,
>>>>>>
>>>>>> Comments in-line. Please note the approach I attempted while 
>>>>>> adding in
>>>>>> the Ampere support was to otherwise not disturb existing driver code
>>>>>> for
>>>>>> non-Ampere parts.
>>>>>>
>>>>>> Steve
>>>>>>
>>>>>> On 1/20/2023 3:12 AM, Suzuki K Poulose wrote:
>>>>>>> Hi Steve
>>>>>>>
>>>>>>> Thanks for the patches. Have a few comments below.
>>>>>>>
>>>>>>> On 20/01/2023 00:51, Steve Clevenger wrote:
>>>>>>>> Add Ampere early clear of ETM TRCOSLAR.OSLK prior to TRCIDR1 
>>>>>>>> access.
>>>>>>>> Ampere Computing erratum AC03_DEBUG_06 describes an Ampere
>>>>>>>> Computing design decision MMIO reads are considered the same as an
>>>>>>>> external debug access. If TRCOSLAR.OSLK is set, the TRCIDR1 access
>>>>>>>> results in a bus fault followed by a kernel panic.  A TRCIDR1 read
>>>>>>>> is valid regardless of TRCOSLAR.OSLK provided MMIO access
>>>>>>>> (now deprecated) is supported.
>>>>>>>> AC03_DEBUG_06 is described in the AmpereOne Developer Errata:
>>>>>>>> https://solutions.amperecomputing.com/customer-connect/products/AmpereOne-device-documentation
>>>>>>>
>>>>>>> Please could you add this erratum to the :
>>>>>>>
>>>>>>> Documentation/arm64/silicon-errata.rst ?
>>>>>>>
>>>>>>> Given the ETM is v4.6, doesn't it support system instructions and
>>>>>>> that is causing this issue of "MMIO access is considered external" ?
>>>>>>> If it does, I think we should drop all of this and simply wire the
>>>>>>> system instruction access support.
>>>>>
>>>>>> That's not the issue in this case. This MMIO access should've been
>>>>>> allowed by the Ampere ETMv4.6 implementation.  Based on comments I've
>>>>>
>>>>> That doesn't answe the question. Please could you confirm the value of
>>>>> ID_AA64DFR0_EL1 on your system ?
>>>> This ID_AA64DFR0_EL1 value came from a TRACE32 debug session connected
>>>> to this part. The ID_AA64DFR0_EL1 value is 0x000F01F210307619. So,
>>>> TraceVer, bits [7:4] are b0001. My understanding is the system register
>>>> interface must be implemented on all ETMv4.6 parts.
>>>
>>> So, I don't understand why we are pushing towards enabling the
>>> "obsolete" MMIO interface ? Is this because "ACPI" mandates it ?
>>> Then, please don't. The spec needs an update to reflect the ETMs
>>> with sysreg access and ETEs.
>>>
>>> Why not stick to the system register access* ?
>>>
>>> * PS: The ACPI support for the ETM/ETE needs additional changes to the
>>> CoreSight driver, *not* the CoreSight ACPI spec. @Anshuman is working on
>>> this at the moment and will be available soon.
>>>
>>> The hack patch below should be sufficient to give it a try and if it 
>>> works.
> 
>> I don't understand your postscript. Certainly there's driver work to be
>> done, but I also think the DEN0067 CoreSight ACPI specification needs
> 
> The issue is having a single HID for ETMs (which from a spec point of
> view makes sense for) with and without MMIO access. That brings two
> different components in Linux (AMBA hook for ACPI and a platform driver)
> compete for the said HID. There are other reasons to disconnect the
> CoreSight from AMBA framework and manage them directly [0].

[0] https://lkml.kernel.org/r/e37e12ab-9701-2883-724a-2a281ad35df2@arm.com