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

[Steve Clevenger]

On 2/2/2023 9:27 AM, Suzuki K Poulose wrote:
> 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
> 
I'll need to determine which of the 3 coresight_timeout calls which read
the TRCSTATR idle bit times out. In each case a number of sysreg
accesses have occurred prior to any of these 3 calls.

> 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
>>>>>
>>>>>
>>>
>