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

[Suzuki K Poulose]

On 02/02/2023 17:12, Steve Clevenger wrote:
> Hi Suzuki,
> 
> Commented in-line.
> 
> Steve C.
> 
> On 2/2/2023 3:16 AM, Suzuki K Poulose wrote:
>> On 02/02/2023 05:20, Steve Clevenger wrote:
>>>
>>> Hi Suzuki,
>>>
>>> I've split out the bug fix (i.e. nr_addr_cmp use) to a separate patch
>>
>> Thanks for that.
>>
>>> and added references to the Ampere erratum in silicon-errata.rst.
>>> These will be submitted as separate patches.
>>>
>>> The ETM4.x TRCOSLAR.OSLK early clear has moved to etm4_init_csdev_iomem
>>> for all manufacturers. I think this is what you asked for.
>>> The no_quad_mmio flag has moved to struct csdev_access, and the split
>>> 64-bit read/write logic has been implemented entirely in the header file
>>> coresight-etm4x.h is the existing calls.
>>> I'd like to retire this patch thread, and submit these as a new thread.
>>> Is there an objection?
>>
>> I would still like to use the system instructions method for the ETM,
>> than hacking the MMIO access for something that is obsolete.
>> The ACPI document for CoreSight will be published soon for review to
>> accommodate the description for ETMs without MMIO and it no longer
>> mandates the MemoryResource.
>>
>> What is the objection to using system instruction access here ?
> No objection going forward. For our initial release, we're not in a
> position to change the CoreSight DSDT based on a specification which is
> incomplete.

There is on change to the CoreSight DSDT specification as such. The only
change to the "spec" is along the lines of :

"MMIO interface is mandatory only if not accessible via system 
instruction access "


> Based on a quick sysreg only build, I didn't collect trace samples. I
> haven't had time to chase this, but the reported error is "timeout while
> waiting for Trace Idle Status" on a TRCSTATR read. More testing is

Are you able to access the other registers ?

e.g,

$ cat /sys/bus/coresight/devices/etm0/mgmt/trcpidr0

Suzuki


> required. If this isn't related to an Ampere sysreg access problem
> (doubtful), the next place I'd look is as a synchronization issue.
> 
>>
>> Thanks
>> Suzuki
>>
>>
>>
>>>
>>> Thanks,
>>> Steve
>>>
>>> On 1/23/2023 2:51 PM, Suzuki K Poulose wrote:
>>>>
>>>> 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
>>>>
>>>>
>>