CoreSight March 2023

coresight@lists.linaro.org

10 participants
48 discussions

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, 1 month

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, 1 month

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, 1 month

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, 1 month

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, 1 month

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, 1 month

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

by Suzuki K Poulose

Hi Steve fyi, the ACPI spec for CoreSight has been updated to relax the MMIO region requirement for ETMv4x and ETE. Please find it here : https://developer.arm.com/documentation/den0067/latest Kind regards Suzuki On 02/02/2023 23:03, Steve Clevenger wrote: > > > 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-d… >>>>>>>>>>>>> >>>>>>>>>>>>> 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 >>>>>> >>>>>> >>>> >>

2 years, 1 month

[PATCH v2 0/9] Add support to configure TPDM DSB subunit

by Tao Zhang

From: Tao Zhang <taozha(a)qti.qualcomm.com> Introduction of TPDM DSB subunit DSB subunit is responsible for creating a dataset element, and is also optionally responsible for packing it to fit multiple elements on a single ATB transfer if possible in the configuration. The TPDM Core Datapath requests timestamps be stored by the TPDA and then delivering ATB sized data (depending on ATB width and element size, this could be smaller or larger than a dataset element) to the ATB Mast FSM. The DSB subunit must be configured prior to enablement. This series adds support for TPDM to configure the configure DSB subunit. Once this series patches are applied properly, the new tpdm nodes for should be observed at the tpdm path /sys/bus/coresight/devices/tpdm* which supports DSB subunit. e.g. /sys/devices/platform/soc(a)0/69d0000.tpdm/tpdm0#ls -l | grep dsb -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_edge_ctrl -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_edge_ctrl_mask -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_mode -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_patt_mask -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_patt_ts -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_patt_type -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_patt_val -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_trig_patt_mask -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_trig_patt_val -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_trig_ts -rw-r--r-- 1 root root 4096 Jan 1 00:01 dsb_trig_type We can use the commands are similar to the below to configure the TPDMs which support DSB subunit. Enable coresight sink first. echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink echo 1 > /sys/bus/coresight/devices/tpdm0/reset echo 0x3 0x3 0x1 > /sys/bus/coresight/devices/tpdm0/dsb_edge_ctrl_mask echo 0x6d 0x6d 0 > /sys/bus/coresight/devices/tpdm0/dsb_edge_ctrl echo 1 > /sys/bus/coresight/devices/tpdm0/dsb_patt_ts echo 1 > /sys/bus/coresight/devices/tpdm0/dsb_patt_type echo 0 > /sys/bus/coresight/devices/tpdm0/dsb_trig_ts echo 0 0xFFFFFFFF > /sys/bus/coresight/devices/tpdm0/dsb_patt_mask echo 0 0xFFFFFFFF > /sys/bus/coresight/devices/tpdm0/dsb_trig_patt_val This patch series depends on patch series "[v17,0/9] Coresight: Add support for TPDM and TPDA" https://patchwork.kernel.org/project/linux-arm-msm/cover/20230117145708.167… TPDM_DSB commit tree: https://git.codelinaro.org/clo/linux-kernel/coresight/-/tree/tpdm-dsb-v2 https://git.codelinaro.org/clo/linux-kernel/coresight/-/commits/tpdm-dsb-v2 Changes in V2: 1. Change the name of the property "qcom,dsb-elem-size" to "qcom,dsb-element-size" -- Suzuki K Poulose 2. Update the TPDA yaml file for the item "qcom,dsb-elem-size". -- Krzysztof Kozlowski 3. Add the full name of DSB in the description of the item "qcom,dsb-elem-size". -- Rob Herring Changes in V1: 1. Change the definition of the property "qcom,dsb-elem-size" from "uint32-array" to "uint32-matrix". -- Krzysztof Kozlowski 2. Add the full name of DSB. -- Rob Herring 3. Deal with 2 entries in an iteration in TPDA driver. -- Suzuki K Poulose 4. Divide the function "tpdm_datasets_alloc" into two functions, "tpdm_datasets_setup" and "tpdm_datasets_alloc". 5. Detecte the input string with the conventional semantics automatically, and constrain the size of the input value. -- Suzuki K Poulose 6. Use the hook function "is_visible()" to hide the DSB related knobs if the data sets are missing. -- Suzuki K Poulose 7. Use the macros "FIELD_GET" and "FIELD_PREP" to set the values. -- Suzuki K Poulose 8. Update the definition of the macros in TPDM driver. 9. Update the comments of the values for the nodes which are for DSB element creation and onfigure pattern match output. -- Suzuki K Poulose 10. Use API "sysfs_emit" to "replace scnprintf". -- Suzuki K Poulose Tao Zhang (9): dt-bindings: arm: Add support for DSB element coresight-tpda: Add DSB dataset support coresight-tpdm: Initialize DSB subunit configuration coresight-tpdm: Add reset node to TPDM node coresight-tpdm: Add nodes to set trigger timestamp and type coresight-tpdm: Add node to set dsb programming mode coresight-tpdm: Add nodes for dsb element creation coresight-tpdm: Add nodes to configure pattern match output coresight-tpdm: Add nodes for timestamp request .../bindings/arm/qcom,coresight-tpda.yaml | 22 + drivers/hwtracing/coresight/coresight-tpda.c | 62 ++ drivers/hwtracing/coresight/coresight-tpda.h | 4 + drivers/hwtracing/coresight/coresight-tpdm.c | 630 ++++++++++++++++++++- drivers/hwtracing/coresight/coresight-tpdm.h | 65 +++ 5 files changed, 778 insertions(+), 5 deletions(-) -- 2.7.4

2 years, 1 month

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight March 2023