| Message ID | cover.1677881753.git.scclevenger@os.amperecomputing.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Ampere Computing ETMv4.x Support | expand |
Hi Steve, On 06/03/2023 05:54, Steve Clevenger wrote: > Ampere ETMv4.x support. Added Ampere ETM ID, and changes required by > the Ampere ETMv4.x hardware implementation. > I don't see any mention of the access via system instructions. Where did that end up ? What is the conclusion on that front ? Kind regards Suzuki > Steve Clevenger (3): > Add known list of Ampere ETMv4 errata > coresight etm4x: Early clear TRCOSLAR.OSLK prior to TRCIDR1 read > coresight etm4x: Add 32-bit read/write option to split 64-bit words > > Documentation/arm64/silicon-errata.rst | 6 +- > .../coresight/coresight-etm4x-core.c | 50 +++++++++++----- > drivers/hwtracing/coresight/coresight-etm4x.h | 58 ++++++++++++++----- > include/linux/coresight.h | 3 + > 4 files changed, 89 insertions(+), 28 deletions(-) >
On 06/03/2023 05:54, Steve Clevenger wrote: > Ampere ETMv4.x support. Added Ampere ETM ID, and changes required by > the Ampere ETMv4.x hardware implementation. > nit: Subject should be : [PATCH <version> 0/X] <subsystem>: <component>: <Title> in this case case: <version> = NULL if version == 1 = version, otherwise Helps us to keep track of different versions of the postings. <subsystem> == coresight This helps the reviewers to "notice" or "ignore" a given series depending on their interest. <component> == etm4x This further helps the reviewers to filter the mails within a subsystem. [PATCH v2 0/3] coresight: etm4x: Support for Ampere computing Also, please include a changelog from the previous version to indicate what has changed. e.g, "Changes since v1: - Modified xyz - Dropped abc - Addressed comments on ijk (<name of the requester> " That helps the reviewers to get a picture of what they should be looking at and better spend their time. Kind regards Suzuki > Steve Clevenger (3): > Add known list of Ampere ETMv4 errata > coresight etm4x: Early clear TRCOSLAR.OSLK prior to TRCIDR1 read > coresight etm4x: Add 32-bit read/write option to split 64-bit words > > Documentation/arm64/silicon-errata.rst | 6 +- > .../coresight/coresight-etm4x-core.c | 50 +++++++++++----- > drivers/hwtracing/coresight/coresight-etm4x.h | 58 ++++++++++++++----- > include/linux/coresight.h | 3 + > 4 files changed, 89 insertions(+), 28 deletions(-) >
Hi Suzuki, To answer your question, Ampere plans to use the existing MMIO implementation to introduce CoreSight HW Assisted Trace since we're preparing for release. As a minimum, we know this would require a respin of our CoreSight DSDT. Also, if I didn't misunderstand, it sounded like you planned supporting work (e.g. ETMv4 not handled as an AMBA device). Since our ETMv4 sink components (ETF, ETR, +CATU) remain memory mapped, do these remain AMBA? We understand ETMv4/ETE MMIO is going away. As a sysreg quick test, I bypassed the code which checks for an ETMv4 base address in order to default to sysreg access. Trace collection failed with an error. I don't have the time to chase after this right now, but I intend to budget the time in the near future. Thanks and regards, Steve On 3/6/2023 2:29 AM, Suzuki K Poulose wrote: > Hi Steve, > > On 06/03/2023 05:54, Steve Clevenger wrote: >> Ampere ETMv4.x support. Added Ampere ETM ID, and changes required by >> the Ampere ETMv4.x hardware implementation. >> > > I don't see any mention of the access via system instructions. Where did > that end up ? What is the conclusion on that front ? > > Kind regards > Suzuki > > >> Steve Clevenger (3): >> Add known list of Ampere ETMv4 errata >> coresight etm4x: Early clear TRCOSLAR.OSLK prior to TRCIDR1 read >> coresight etm4x: Add 32-bit read/write option to split 64-bit words >> >> Documentation/arm64/silicon-errata.rst | 6 +- >> .../coresight/coresight-etm4x-core.c | 50 +++++++++++----- >> drivers/hwtracing/coresight/coresight-etm4x.h | 58 ++++++++++++++----- >> include/linux/coresight.h | 3 + >> 4 files changed, 89 insertions(+), 28 deletions(-) >> >
Hi Suzuki, I submitted the Ampere patches on a new thread to forego the delta. But, here's a summary of the changes from Version 1. Please allow some nits (title, etc.) to remain in place so I don't have to resubmit all the patch files. Thanks, Steve Version 2 changes: Addressed Suzuki Poulose, Mike Leach, and James Clark V1 comments: 1. Moved early clear of TRCOSLAR.OSLK code from etm4_init_arch_data to etm4_init_iomem_access. The early clear is now done for all manufacturers, instead of based on drvdata mmio_external flag (set for Ampere). The flag is deprecated. 2. Moved et4x_split_read64 implementation into etm4x_relaxed_read64, and etm4x_split_write64 into etm4x_relaxed_write64. This cleared up code changes in coresight-etm4x-core.c where ever these were used. 3. etm4x_relaxed_read64 and etm4x_relaxed_write64 are implemented as static inline instead of macro definitions. 4. Removed struct etm4_drvdata mmio_external flag, and moved no_quad_mmio flag to struct csdev_access. Then ensure pre-initialized csdev_access fields get preserved. 5. Dropped /linux/drivers/amba/bus.c debug statement patch to provide visibility to CoreSight PID/CID. On 3/6/2023 2:54 AM, Suzuki K Poulose wrote: > On 06/03/2023 05:54, Steve Clevenger wrote: >> Ampere ETMv4.x support. Added Ampere ETM ID, and changes required by >> the Ampere ETMv4.x hardware implementation. >> > > nit: Subject should be : > > [PATCH <version> 0/X] <subsystem>: <component>: <Title> > > in this case case: > > <version> = NULL if version == 1 > = version, otherwise > Helps us to keep track of different versions of the postings. > > <subsystem> == coresight > > This helps the reviewers to "notice" or "ignore" a given series > depending on their interest. > > <component> == etm4x > This further helps the reviewers to filter the mails within a subsystem. > > [PATCH v2 0/3] coresight: etm4x: Support for Ampere computing > > Also, please include a changelog from the previous version to indicate > what has changed. e.g, > > "Changes since v1: > - Modified xyz > - Dropped abc > - Addressed comments on ijk (<name of the requester> > " > > That helps the reviewers to get a picture of what they should be looking > at and better spend their time. > > Kind regards > Suzuki >> Steve Clevenger (3): >> Add known list of Ampere ETMv4 errata >> coresight etm4x: Early clear TRCOSLAR.OSLK prior to TRCIDR1 read >> coresight etm4x: Add 32-bit read/write option to split 64-bit words >> >> Documentation/arm64/silicon-errata.rst | 6 +- >> .../coresight/coresight-etm4x-core.c | 50 +++++++++++----- >> drivers/hwtracing/coresight/coresight-etm4x.h | 58 ++++++++++++++----- >> include/linux/coresight.h | 3 + >> 4 files changed, 89 insertions(+), 28 deletions(-) >> >
Hi Steve On 07/03/2023 01:23, Steve Clevenger wrote: > > Hi Suzuki, > > To answer your question, Ampere plans to use the existing MMIO > implementation to introduce CoreSight HW Assisted Trace since we're > preparing for release. As a minimum, we know this would require a respin Please note that MMIO interface is for "external debug" not for self-hosted usecase, with system instruction support. This is why you need to work around the driver "as an errata". > of our CoreSight DSDT. Also, if I didn't misunderstand, it sounded like > you planned supporting work (e.g. ETMv4 not handled as an AMBA > device). Since our ETMv4 sink components (ETF, ETR, +CATU) remain memory > mapped, do these remain AMBA? Just to be clear, all of these components will be MMIO. We are simply moving the "Linux" driver framework from AMBA driver to platform device. This will be done in stages. ETMv4x would be moved in the first pass to allow us to support system instruction based devices seemlessly with ACPI. > > We understand ETMv4/ETE MMIO is going away. As a sysreg quick test, I As mentioned above it is not going away. MMIO is the way to access if that is *the only* access mode. For ETMv4.4+ and ETE MMIO system instructions is *the mode* of access for self-hosted. Please be aware that, going down this route may prevent them being virtualised (when we add the support in the near future). > bypassed the code which checks for an ETMv4 base address in order to > default to sysreg access. Trace collection failed with an error. I don't > have the time to chase after this right now, but I intend to budget the > time in the near future. If we can get to the bottom of this, we should be able to support the platform in a future proof way than adding this ugly work around of accessing via the *external MMIO* interface. Suzuki > > Thanks and regards, > Steve > > On 3/6/2023 2:29 AM, Suzuki K Poulose wrote: >> Hi Steve, >> >> On 06/03/2023 05:54, Steve Clevenger wrote: >>> Ampere ETMv4.x support. Added Ampere ETM ID, and changes required by >>> the Ampere ETMv4.x hardware implementation. >>> >> >> I don't see any mention of the access via system instructions. Where did >> that end up ? What is the conclusion on that front ? >> >> Kind regards >> Suzuki >> >> >>> Steve Clevenger (3): >>> Add known list of Ampere ETMv4 errata >>> coresight etm4x: Early clear TRCOSLAR.OSLK prior to TRCIDR1 read >>> coresight etm4x: Add 32-bit read/write option to split 64-bit words >>> >>> Documentation/arm64/silicon-errata.rst | 6 +- >>> .../coresight/coresight-etm4x-core.c | 50 +++++++++++----- >>> drivers/hwtracing/coresight/coresight-etm4x.h | 58 ++++++++++++++----- >>> include/linux/coresight.h | 3 + >>> 4 files changed, 89 insertions(+), 28 deletions(-) >>> >>
Hi Suzuki, Comments inline. Steve On 3/7/2023 2:21 AM, Suzuki K Poulose wrote: > Hi Steve > > On 07/03/2023 01:23, Steve Clevenger wrote: >> >> Hi Suzuki, >> >> To answer your question, Ampere plans to use the existing MMIO >> implementation to introduce CoreSight HW Assisted Trace since we're >> preparing for release. As a minimum, we know this would require a respin > > Please note that MMIO interface is for "external debug" not for > self-hosted usecase, with system instruction support. This is why you need > to work around the driver "as an errata". > Sorry, my earlier response was poorly worded. > >> of our CoreSight DSDT. Also, if I didn't misunderstand, it sounded like >> you planned supporting work (e.g. ETMv4 not handled as an AMBA >> device). Since our ETMv4 sink components (ETF, ETR, +CATU) remain memory >> mapped, do these remain AMBA? > > Just to be clear, all of these components will be MMIO. We are simply > moving the "Linux" driver framework from AMBA driver to platform device. > This will be done in stages. ETMv4x would be moved in the first pass to > allow us to support system instruction based devices seemlessly with > ACPI. > >> >> We understand ETMv4/ETE MMIO is going away. As a sysreg quick test, I > > As mentioned above it is not going away. MMIO is the way to access if > that is *the only* access mode. For ETMv4.4+ and ETE MMIO system > instructions is *the mode* of access for self-hosted. Please be aware > that, going down this route may prevent them being virtualised (when > we add the support in the near future). Ampere intends to support sysreg access going forward. What we're lacking in this moment is time to work out the kinks. > >> bypassed the code which checks for an ETMv4 base address in order to >> default to sysreg access. Trace collection failed with an error. I don't >> have the time to chase after this right now, but I intend to budget the >> time in the near future. > > If we can get to the bottom of this, we should be able to support > the platform in a future proof way than adding this ugly work around > of accessing via the *external MMIO* interface. As I mention above, my assessment is it's too far along in our product cycle to work through unforeseen sysreg issues which might surface. I plan to devote time to this shortly. At present, a subset of our cores have working self-hosted trace using MMIO. Our priority right now is to scale the solution for our SoC. There are challenges here which span OpenCSD, perf, and possibly the CoreSight driver. In this patch submission, I've addressed the maintainer comments brought to my attention. Nothing else has surfaced. The Ampere Linux team meets every week or biweekly with ARM. I'll ask they add this topic to the agenda if you believe it's warranted. Otherwise, can I ask your support to move these upstream? Thank you > > Suzuki > >> >> Thanks and regards, >> Steve >> >> On 3/6/2023 2:29 AM, Suzuki K Poulose wrote: >>> Hi Steve, >>> >>> On 06/03/2023 05:54, Steve Clevenger wrote: >>>> Ampere ETMv4.x support. Added Ampere ETM ID, and changes required by >>>> the Ampere ETMv4.x hardware implementation. >>>> >>> >>> I don't see any mention of the access via system instructions. Where did >>> that end up ? What is the conclusion on that front ? >>> >>> Kind regards >>> Suzuki >>> >>> >>>> Steve Clevenger (3): >>>> Add known list of Ampere ETMv4 errata >>>> coresight etm4x: Early clear TRCOSLAR.OSLK prior to TRCIDR1 read >>>> coresight etm4x: Add 32-bit read/write option to split 64-bit words >>>> >>>> Documentation/arm64/silicon-errata.rst | 6 +- >>>> .../coresight/coresight-etm4x-core.c | 50 +++++++++++----- >>>> drivers/hwtracing/coresight/coresight-etm4x.h | 58 >>>> ++++++++++++++----- >>>> include/linux/coresight.h | 3 + >>>> 4 files changed, 89 insertions(+), 28 deletions(-) >>>> >>> >