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