CoreSight

coresight@lists.linaro.org

6 participants
2351 discussions

Re: [PATCH 3/3] coresight etm4x: Add 32-bit read/write option to split 64-bit words

by Leo Yan

On Thu, Mar 09, 2023 at 12:46:55PM -0800, Steve Clevenger wrote: [...] > > #include <linux/io-64-nonatomic-hi-lo.h> > > > > static inline u64 etm4x_relaxed_read64(struct csdev_access *csa, unsigned int offset) > > { > > if (csa->io_mem) { > > if (csa->no_quad_mmio) > > return hi_lo_readq_relaxed(csa->base + offset); > > else > > return readq_relaxed(csa->base + offset); > > } else { > > return read_etm4x_sysreg_offset(offset, true); > > } > > } > > > > I'm not saying to play magic on {writeq|readq}_relaxed(), would it be > > fine to directly call hi_lo_readq_relaxed()? > > I can do this, and it would work fine. Without compile protection, one > concern is the silent inclusion of io-64-nonatomic-hi-lo.h definitions > for readq_relaxed/writeq_relaxed if the include hierarchy gets changed. > The only (minor) risk I see is to performance. Yeah, we could include io.h ahead io-64-nonatomic-hi-lo.h: #include <linux/io.h> #include <linux/io-64-nonatomic-lo-hi.h> Thus we can get the definitions for {readq|writeq}_relaxed() from "io.h". > Second, it seems to me the hi_lo and lo_hi macros were intended to deal > with endianness. IMO, the implementation I offered is agnostic without a > hint to endianness or atomicity. Seems to me, kernel functions with explict endianness naming is not bad thing :) > The CoreSight maintainer audience is > limited and already aware of these issues, so I'll make the change for you. Thanks! Suzuki, Mike, I think it's good to get your opinion before Steve can proceed for updating patch. Leo

2 years, 2 months

Re: [PATCH 3/3] coresight etm4x: Add 32-bit read/write option to split 64-bit words

by Leo Yan

Hi Steve, On Wed, Mar 08, 2023 at 11:08:34AM -0800, Steve Clevenger wrote: [...] > I'm familiar with this interface. The reason I chose not to use it is > this solution is configured at compile time. The writeq_relaxed (used by > etm4x_relaxed_write64) and readq_relaxed (used by etm4x_relaxed_read64) > otherwise default to 64-bit access. I was uncertain how a compile time > change would go over with the maintainers. Correct me, but compile time > configurations in the kernel seem to be related to ARM64 erratum, versus > manufacturer specific? > > My take (based on limited knowledge) of the changes necessary to support > a compile time decision seemed to exceed the changes compared with the > implementation I submitted upstream. If this becomes a sticking point, > let me know. I am suggesting something like below: #include <linux/io-64-nonatomic-hi-lo.h> static inline u64 etm4x_relaxed_read64(struct csdev_access *csa, unsigned int offset) { if (csa->io_mem) { if (csa->no_quad_mmio) return hi_lo_readq_relaxed(csa->base + offset); else return readq_relaxed(csa->base + offset); } else { return read_etm4x_sysreg_offset(offset, true); } } I'm not saying to play magic on {writeq|readq}_relaxed(), would it be fine to directly call hi_lo_readq_relaxed()? Thanks, Leo

2 years, 2 months

Re: [BUG] perf: No samples found when using kcore + coresight

by Leo Yan

Hi Yang, On Wed, Mar 08, 2023 at 11:56:38AM -0800, Yang Shi wrote: [...] > > Dumping raw events could show the events from the bad data file. But > > it has zero samples after event collapse. > > > > The only difference is --kcore inserted a new text_poke dummy event. > > It seems coresight also inserted a dummy event with my command but > > your command didn't. So it seems like the two dummy events confused > > event collapse. > > > > The text_poke dummy event is added by commit > > f42c0ce573df79d1b8bd169008c994dcdd43585a ("perf record: Always get > > text_poke events with --kcore option"). If I reverted this commit, > > then it works. But I'm not sure whether this is the right fix or real > > root cause or not. Or coresight shouldn't insert its own dummy event? > > It seems like coresight needs to insert the dummy event if > full_auxtrace is on IIUC. So it sounds like event collapse can't > handle such a case? I am struggling to understand the meaning "event collapse" :) I reviewed your shared dump, the bad and good perf data both contain the dummy event with 'text_poke = 1'. Could you confirm the shared dump in your previous email is correct or not? > I also tried v5.19 (before "perf record: Always > get text_poke events with --kcore option", which was merged in v6.0), > it works. So it seems like a regression. Yeah, we need to fix it. I am not sure the Linux kernel for Arm64 supports text poke or not (kernel needs some specific handling when alter instructions), the kernel change is the prerequisites. On the other hand, in the current code cs-etm misses to handle the event PERF_RECORD_TEXT_POKE in the function cs_etm__process_event(). This might be the cause for the failure. Do you mind to share the bad perf.data file with James and me? Thanks, Leo

2 years, 2 months

[PATCH 0/8] coresight: Fix CTI module refcount leak by making it a helper device

by James Clark

Currently there is a refcount leak in CTI when using system wide mode or tracing multithreaded applications. See the last commit for a reproducer. This prevents the module from being unloaded. Historically there have been a few issues and fixes attempted around here which have resulted in some extra logic and a member to keep track of CTI being enabled 'struct coresight_device->ect_enabled'. The fix in commit 665c157e0204 ("coresight: cti: Fix hang in cti_disable_hw()") was also related to CTI having its own enable/disable path which came later than other devices. If we make CTI a helper device and enable helper devices adjacent to the path we get very similar enable/disable behavior to now, but with more reuse of the existing reference counting logic in the coresight core code. This also affects CATU which can have a little bit of its hard coded enable/disable code removed. Enabling CATU on the generic path does require that input connections are tracked so that it can get its associated ETR buffer. Applies to coresight/next (669c4614236a7) but also requires the realloc_array patch here [1]. Also available in full here [2]. [1]: https://lore.kernel.org/linux-arm-kernel/20230306152723.3090195-1-james.cla… [2]: https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-cti-module-refcou… James Clark (8): coresight: Use enum type for cs_mode wherever possible coresight: Change name of pdata->conns coresight: Rename nr_outports to nr_outconns coresight: Dynamically add connections coresight: Store in-connections as well as out-connections coresight: Refactor out buffer allocation function for ETR coresight: Enable and disable helper devices adjacent to the path coresight: Fix CTI module refcount leak by making it a helper device drivers/hwtracing/coresight/coresight-catu.c | 34 ++- drivers/hwtracing/coresight/coresight-core.c | 258 +++++++++++------- .../hwtracing/coresight/coresight-cti-core.c | 56 ++-- .../hwtracing/coresight/coresight-cti-sysfs.c | 4 +- drivers/hwtracing/coresight/coresight-cti.h | 4 +- drivers/hwtracing/coresight/coresight-etb10.c | 3 +- .../coresight/coresight-etm3x-core.c | 6 +- .../coresight/coresight-etm4x-core.c | 6 +- .../hwtracing/coresight/coresight-platform.c | 168 +++++++++--- drivers/hwtracing/coresight/coresight-priv.h | 9 +- drivers/hwtracing/coresight/coresight-stm.c | 6 +- drivers/hwtracing/coresight/coresight-sysfs.c | 1 - .../hwtracing/coresight/coresight-tmc-etf.c | 2 +- .../hwtracing/coresight/coresight-tmc-etr.c | 88 +++--- drivers/hwtracing/coresight/coresight-tmc.h | 2 + drivers/hwtracing/coresight/coresight-tpdm.c | 4 +- drivers/hwtracing/coresight/coresight-tpiu.c | 3 +- drivers/hwtracing/coresight/coresight-trbe.c | 3 +- drivers/hwtracing/coresight/ultrasoc-smb.c | 3 +- drivers/hwtracing/coresight/ultrasoc-smb.h | 2 +- include/linux/coresight.h | 92 ++++--- 21 files changed, 483 insertions(+), 271 deletions(-) -- 2.34.1

2 years, 2 months

Re: [PATCH 3/3] coresight etm4x: Add 32-bit read/write option to split 64-bit words

by Leo Yan

Hi Steve, On Sun, Mar 05, 2023 at 10:54:08PM -0700, Steve Clevenger wrote: > An Ampere Computing design decision to not support 64-bit read/write > access in the ETMv4.6 implementation. > > The Ampere work around is to split ETMv4.6 64-bit register access > into 2 ea. 32-bit read/write accesses. > AC03_DEBUG_10 is described in the AmpereOne Developer Errata document: > > https://solutions.amperecomputing.com/customer-connect/products/AmpereOne-d… > > Signed-off-by: Steve Clevenger <scclevenger(a)os.amperecomputing.com> > --- > drivers/hwtracing/coresight/coresight-etm4x.h | 58 ++++++++++++++----- > 1 file changed, 44 insertions(+), 14 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h > index 434f4e95ee17..17457ec71876 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.h > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h > @@ -539,11 +539,6 @@ > readl_relaxed((csa)->base + (offset)) : \ > read_etm4x_sysreg_offset((offset), false))) > > -#define etm4x_relaxed_read64(csa, offset) \ > - ((u64)((csa)->io_mem ? \ > - readq_relaxed((csa)->base + (offset)) : \ > - read_etm4x_sysreg_offset((offset), true))) > - > #define etm4x_read32(csa, offset) \ > ({ \ > u32 __val = etm4x_relaxed_read32((csa), (offset)); \ > @@ -567,15 +562,6 @@ > false); \ > } while (0) > > -#define etm4x_relaxed_write64(csa, val, offset) \ > - do { \ > - if ((csa)->io_mem) \ > - writeq_relaxed((val), (csa)->base + (offset)); \ > - else \ > - write_etm4x_sysreg_offset((val), (offset), \ > - true); \ > - } while (0) > - > #define etm4x_write32(csa, val, offset) \ > do { \ > __io_bw(); \ > @@ -1091,6 +1077,50 @@ void etm4_config_trace_mode(struct etmv4_config *config); > u64 etm4x_sysreg_read(u32 offset, bool _relaxed, bool _64bit); > void etm4x_sysreg_write(u64 val, u32 offset, bool _relaxed, bool _64bit); > > +/* 64-bit aligned to convert 64-bit access to 2 ea. 32-bit access */ > +#pragma pack(push, 8) > + > +struct etm_quad_split { > + u32 lsw; > + u32 msw; > +}; > + > +#pragma pack(pop) > + > +static inline u64 etm4x_relaxed_read64(struct csdev_access *csa, unsigned int offset) > +{ > + if (csa->io_mem) { > + if (csa->no_quad_mmio) { > + /* split 64-bit reads into 2 consecutive 32-bit reads */ > + struct etm_quad_split container; > + > + container.lsw = etm4x_read32(csa, offset); > + container.msw = etm4x_read32(csa, offset + sizeof(u32)); > + > + return *(u64 *) &container; To be honest, I am not familiar with this part, just want to remind two things. Firstly, here you could use hi_lo_readq_relaxed(), seems to me, hi_lo_readq_relaxed() does the same thing with above section and you don't need to define the structure etm_quad_split(). Secondly, IIUC, a main problem with splitting 64-bit access into two 32-bit accesses is breaking atomicity. If here have race condition between reading and writing 64-bit registers, we need to consider to use spinlock for register accessing. I'd like to leave the second issue to Suzuki/Mike/James for comfirmation in case I introduced complexity. > + } else > + return readq_relaxed(csa->base + offset); > + } else > + return read_etm4x_sysreg_offset(offset, true); Here need to add brackets: } else { return read_etm4x_sysreg_offset(offset, true); } > +} > + > +static inline void etm4x_relaxed_write64(struct csdev_access *csa, u64 quad, unsigned int offset) > +{ > + if (csa->io_mem) { > + /* split 64-bit writes into 2 consecutive 32-bit writes */ > + if (csa->no_quad_mmio) { > + struct etm_quad_split container; > + > + *(u64 *) &container = quad; > + > + etm4x_relaxed_write32(csa, container.lsw, offset); > + etm4x_relaxed_write32(csa, container.msw, offset + sizeof(u32)); > + } else > + writeq_relaxed(quad, csa->base + offset); > + } else > + write_etm4x_sysreg_offset(quad, offset, true); \ Ditto. Please drop the character '\' at the end of the line. Thanks, Leo P.s. I have a ADLink AVA platform (Ampere Altra SoC with 32 CPUs), I am glad to give a test if you can confirm this patch set can apply on it (and please clarify if there have any prerequisite for firmwares). > +} > + > static inline bool etm4x_is_ete(struct etmv4_drvdata *drvdata) > { > return drvdata->arch >= ETM_ARCH_ETE; > -- > 2.25.1 >

2 years, 2 months

Re: [BUG] perf: No samples found when using kcore + coresight

by James Clark

On 06/03/2023 21:13, Yang Shi wrote: > Hi, > > I'm seeking some help regarding perf record --kcore + coresight. When > I tracing with perf record --kcore -e cs_etm ... , perf report shows > "The perf.data/data data has no samples!". > > I tried other combinations: > 1. perf record --kcore (w/o using coresight): works > 2. perf record -e cs_etm ... : works > 3. Manually copy kcore with perf buildid-cache, then run perf record > -e cs_etm... : works > > So just "perf record --kcore -e cs_etm ..." doesn't work. > > I'm using v6.2 kernel and the perf is built from the same kernel > source with OpenCSD 1.4. Also attached the sample file generated by > perf. > > Any suggestion is appreciated. > > Thanks, > Yang Hi Yang, I don't see any issue with this command and I still get samples: perf record -e cs_etm// --kcore -- true You might want to try running both record and report in verbose and stdio mode (-vvv --stdio) to see if you see any warnings. I can't think of any way --kcore would cause an issue because all it does is save kcore into the .debug cache rather than affecting the actual perf.data file. Are you running perf report in the same place the recording was made? Or on another system? James

2 years, 2 months

Re: [PATCH 0/3] Ampere Computing ETMv4.x Support

by Suzuki K Poulose

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

2 years, 2 months

Re: [PATCH 0/3] Ampere Computing ETMv4.x Support

by Suzuki K Poulose

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

2 years, 2 months

Re: [PATCH 2/3] coresight etm4x: Add 32-bit read/write option to split 64-bit words

by Suzuki K Poulose

On 20/01/2023 00:51, Steve Clevenger wrote: > Add 32-bit read/write access option for Ampere ETMv4.6 64-bit registers. > Ampere Computing erratum AC03_DEBUG_10 describes a design decision where > 64-bit read/write access is not supported for the ETMv4.6 implementation. > These 64-bit registers must be accessed as 2 ea. 32-bit registers. > AC03_DEBUG_10 is described in the AmpereOne Developer Errata: > https://solutions.amperecomputing.com/customer-connect/products/AmpereOne-d… As with the previous comment, please : a) If this is because of the system instruction access support b) Document the erratum > > Fix drvdata->nr_addr_cmp for() loop range bug to drvdata->nr_addr_cmp * 2 > in etm_enable_hw. Good catch ! Please separate this out and send it as a fix. I can queue this. > > Signed-off-by: Steve Clevenger <scclevenger(a)os.amperecomputing.com> > --- > .../coresight/coresight-etm4x-core.c | 81 ++++++++++++++----- > drivers/hwtracing/coresight/coresight-etm4x.h | 32 ++++++++ > 2 files changed, 93 insertions(+), 20 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 533be1928a09..bf4daa649cdf 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -452,18 +452,31 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > if (etm4x_sspcicrn_present(drvdata, i)) > etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i)); > } > - for (i = 0; i < drvdata->nr_addr_cmp; i++) { > - etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i)); > - etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i)); > + for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) { > + if (drvdata->no_quad_mmio) { > + etm4x_split_write64(csa, config->addr_val[i], TRCACVRn(i)); > + etm4x_split_write64(csa, config->addr_acc[i], TRCACATRn(i)); > + } else { > + etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i)); > + etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i)); > + } > + } Something like this can be leave some places out. I think we could hide it under the generic helpers and handle it there. May be "struct csdev_access" can cache this "no_quad_mmio" and do the right thing ? > + for (i = 0; i < drvdata->numcidc; i++) { > + if (drvdata->no_quad_mmio) > + etm4x_split_write64(csa, config->ctxid_pid[i], TRCCIDCVRn(i)); > + else > + etm4x_relaxed_write64(csa, config->ctxid_pid[i], TRCCIDCVRn(i)); > } > - for (i = 0; i < drvdata->numcidc; i++) > - etm4x_relaxed_write64(csa, config->ctxid_pid[i], TRCCIDCVRn(i)); > etm4x_relaxed_write32(csa, config->ctxid_mask0, TRCCIDCCTLR0); > if (drvdata->numcidc > 4) > etm4x_relaxed_write32(csa, config->ctxid_mask1, TRCCIDCCTLR1); > > - for (i = 0; i < drvdata->numvmidc; i++) > - etm4x_relaxed_write64(csa, config->vmid_val[i], TRCVMIDCVRn(i)); > + for (i = 0; i < drvdata->numvmidc; i++) { > + if (drvdata->no_quad_mmio) > + etm4x_split_write64(csa, config->vmid_val[i], TRCVMIDCVRn(i)); > + else > + etm4x_relaxed_write64(csa, config->vmid_val[i], TRCVMIDCVRn(i)); > + } > etm4x_relaxed_write32(csa, config->vmid_mask0, TRCVMIDCCTLR0); > if (drvdata->numvmidc > 4) > etm4x_relaxed_write32(csa, config->vmid_mask1, TRCVMIDCCTLR1); > @@ -1670,8 +1683,13 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) > } > > for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) { > - state->trcacvr[i] = etm4x_read64(csa, TRCACVRn(i)); > - state->trcacatr[i] = etm4x_read64(csa, TRCACATRn(i)); > + if (drvdata->no_quad_mmio) { > + state->trcacvr[i] = etm4x_split_read64(csa, TRCACVRn(i)); > + state->trcacatr[i] = etm4x_split_read64(csa, TRCACATRn(i)); > + } else { > + state->trcacvr[i] = etm4x_read64(csa, TRCACVRn(i)); > + state->trcacatr[i] = etm4x_read64(csa, TRCACATRn(i)); > + } > } > > /* > @@ -1681,11 +1699,19 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) > * unit") of ARM IHI 0064D. > */ > > - for (i = 0; i < drvdata->numcidc; i++) > - state->trccidcvr[i] = etm4x_read64(csa, TRCCIDCVRn(i)); > + for (i = 0; i < drvdata->numcidc; i++) { > + if (drvdata->no_quad_mmio) > + state->trccidcvr[i] = etm4x_split_read64(csa, TRCCIDCVRn(i)); > + else > + state->trccidcvr[i] = etm4x_read64(csa, TRCCIDCVRn(i)); > + } > > - for (i = 0; i < drvdata->numvmidc; i++) > - state->trcvmidcvr[i] = etm4x_read64(csa, TRCVMIDCVRn(i)); > + for (i = 0; i < drvdata->numvmidc; i++) { > + if (drvdata->no_quad_mmio) > + state->trcvmidcvr[i] = etm4x_split_read64(csa, TRCVMIDCVRn(i)); > + else > + state->trcvmidcvr[i] = etm4x_read64(csa, TRCVMIDCVRn(i)); > + } > > state->trccidcctlr0 = etm4x_read32(csa, TRCCIDCCTLR0); > if (drvdata->numcidc > 4) > @@ -1799,15 +1825,28 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) > } > > for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) { > - etm4x_relaxed_write64(csa, state->trcacvr[i], TRCACVRn(i)); > - etm4x_relaxed_write64(csa, state->trcacatr[i], TRCACATRn(i)); > + if (drvdata->no_quad_mmio) { > + etm4x_split_write64(csa, state->trcacvr[i], TRCACVRn(i)); > + etm4x_split_write64(csa, state->trcacatr[i], TRCACATRn(i)); > + } else { > + etm4x_relaxed_write64(csa, state->trcacvr[i], TRCACVRn(i)); > + etm4x_relaxed_write64(csa, state->trcacatr[i], TRCACATRn(i)); > + } > } > > - for (i = 0; i < drvdata->numcidc; i++) > - etm4x_relaxed_write64(csa, state->trccidcvr[i], TRCCIDCVRn(i)); > + for (i = 0; i < drvdata->numcidc; i++) { > + if (drvdata->no_quad_mmio) > + etm4x_split_write64(csa, state->trccidcvr[i], TRCCIDCVRn(i)); > + else > + etm4x_relaxed_write64(csa, state->trccidcvr[i], TRCCIDCVRn(i)); > + } > > - for (i = 0; i < drvdata->numvmidc; i++) > - etm4x_relaxed_write64(csa, state->trcvmidcvr[i], TRCVMIDCVRn(i)); > + for (i = 0; i < drvdata->numvmidc; i++) { > + if (drvdata->no_quad_mmio) > + etm4x_split_write64(csa, state->trcvmidcvr[i], TRCVMIDCVRn(i)); > + else > + etm4x_relaxed_write64(csa, state->trcvmidcvr[i], TRCVMIDCVRn(i)); > + } > > etm4x_relaxed_write32(csa, state->trccidcctlr0, TRCCIDCCTLR0); > if (drvdata->numcidc > 4) > @@ -2047,8 +2086,10 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) > * isolates the manufacturer JEP106 ID in the PID. > * TRCPIDR2 (JEDC|DES_1) << 16 | TRCPIDR1 (DES_0) << 8) > */ > - if ((init_arg.pid & 0x000FF000) == 0x00096000) > + if ((init_arg.pid & 0x000FF000) == 0x00096000) { > drvdata->mmio_external = true; > + drvdata->no_quad_mmio = true; > + } > > /* > * Serialize against CPUHP callbacks to avoid race condition > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h > index cf4f9f2e1807..0650bcdff410 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.h > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h > @@ -1016,6 +1016,7 @@ struct etmv4_save_state { > * the trace unit. > * @arch_features: Bitmap of arch features of etmv4 devices. > * @mmio_external: True if ETM considers MMIO an external access. > + * @no_quad_mmio: True if ETM does not support 64-bit (quad) access. > */ > struct etmv4_drvdata { > void __iomem *base; > @@ -1069,6 +1070,7 @@ struct etmv4_drvdata { > bool skip_power_up; > DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX); > bool mmio_external; > + bool no_quad_mmio; > }; > > /* Address comparator access types */ > @@ -1093,6 +1095,36 @@ void etm4_config_trace_mode(struct etmv4_config *config); > u64 etm4x_sysreg_read(u32 offset, bool _relaxed, bool _64bit); > void etm4x_sysreg_write(u64 val, u32 offset, bool _relaxed, bool _64bit); > > +/* 64-bit aligned to convert 64-bit access to 2 ea. 32-bit access */ > +#pragma pack(push, 8) > + > +struct etm_quad_split { > + u32 lsw; > + u32 msw; > +}; > + > +#pragma pack(pop) > + > +static inline u64 etm4x_split_read64(struct csdev_access *csa, unsigned int offset) > +{ > + struct etm_quad_split container; > + > + container.lsw = etm4x_read32(csa, offset); > + container.msw = etm4x_read32(csa, offset + sizeof(u32)); > + > + return *(u64 *) &container; Wouldn't this break with the "endianness" flip ? (Not that we have BE implementations). Could we not combine the two values to a 64bit value and pass that instead ? Similarly below. Suzuki > +} > + > +static inline void etm4x_split_write64(struct csdev_access *csa, u64 quad, unsigned int offset) > +{ > + struct etm_quad_split container; > + > + *(u64 *) &container = quad; > + > + etm4x_relaxed_write32(csa, container.lsw, offset); > + etm4x_relaxed_write32(csa, container.msw, offset + sizeof(u32)); > +} > + > static inline bool etm4x_is_ete(struct etmv4_drvdata *drvdata) > { > return drvdata->arch >= ETM_ARCH_ETE;

2 years, 2 months

Re: [PATCH 0/3] Ampere Computing ETMv4.x Support

by Suzuki K Poulose

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

2 years, 2 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight