CoreSight

coresight@lists.linaro.org

9 participants
2378 discussions

Re: A question about CTI code, need help to answer

by Mike Leach

Hi, On Mon, 27 Feb 2023 at 06:20, Tao Zhang <quic_taozha(a)quicinc.com> wrote: > > Hi Mike, > > 在 2/24/2023 11:43 PM, Tao Zhang 写道: > > Hi Mike, > > > > > > I have a question about the following code you promoted. > > > > Patch - coresight: cti: Add device tree support for v8 arch CTI > > > > You set the following mask variables in the function > > "cti_plat_create_v8_connections". > > > > tc->con_in->used_mask = 0x3; /* sigs <0 1> */ > > > > tc->con_out->used_mask = 0x7; /* sigs <0 1 2 > */ > > > > Could you tell us why they should be set to 0x3 and 0x7 here? > > > > Is there any particular reason for this design? > > > Add more background on this question below. > > The CPU CTIs we use based on the CTI v8 architecture that support more > trigger number for the trigger in and trigger out. > > If we configured "arm,coresight-cti-v8-arch" for CPU CTIs in the device > tree, it will causes the trigger number to be unavailable. > CTI's in v8 attached to the CPU have an architecturally mandated set of triggers - this is the set that is defined with this compatible. This is designed to save effort in describing the elements in the device tree. > Is there any way to solve the problem we have now? > > Is it possible to change the trigger number supported by CPU CTIs by > configuring device tree? For custom CTIs use the arm,coresight-cti compatible, and describe your CTI as shown in ./Documentation/devicetree/bindings/arm/arm.coresight-cti.yaml This has an example of custom CTI bindings Regards Mike > > > > > Best, > > > > Tao > > > Best, > > Tao > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

2 years, 4 months

Re: [RFC] Add trig_ts function in STM to generate TRIG_TS packet periodically

by Jinlong Mao

On 2/23/2023 9:38 PM, Alexander Shishkin wrote: > Jinlong Mao <quic_jinlmao(a)quicinc.com> writes: > >> On PC tool, it can show the logs from ETR in real time. >> >> When one small packet send from STM to ETR, it can be stuck between STM >> and ETR. >> When the packet stuck happens, it will be flushed to ETR only when some >> other packets >> generated from STM source or CTI flush commands are sent. If the time is >> too long to wait >> for next packets coming, user will consider that issue happens with >> previous small packet. >> And user's requirement is that packet from STM can be flushed to ETR >> automatically instead >> of sending commands manually. >> >> Is it appropriate to add a sysfs node for STM to generate the trigger >> packet periodically for such case ? > There's stm_heartbeat that will send a string via STM on a timer. It > will come on its own channel, so your PC tool should be able to ignore > it. Would that help? Hi Alex, It helps with my case. How to implement it ? Thanks Jinlong Mao > > Regards, > -- > Alex

2 years, 4 months

[RFC] Add trig_ts function in STM to generate TRIG_TS packet periodically

by Jinlong Mao

Hi all, When there is some small packet sent from STM to ETR, the small packet could be stuck between source and sink even if manual flush is set when disable ETR. So there is requirement that flush the STM trace periodically after enabling STM to ETR. STM can generate TRIG_TS packet by writing to offset 0xF0 of the driver STM stimulus port. ETR has ability to initiate a flush on seeing a TRIG_TS packet. For this requirement, I want to create a sysfs node like trig_ts for STM. When writing 1 to this sysfs node, a timer with 1 second periodicity in STM will start to generate the trig_ts packet to ETR. Once ETR receive the TRIG_TS packet, it will initiate a flush. Could you please help to provide your comments on this requirement ? Thanks Jinlong Mao

2 years, 4 months

Re: [PATCH] coresight: tmc-etr: Handle enable failure in tmc_read_unprepare_etr

by Suzuki K Poulose

On 21/02/2023 18:38, Yabin Cui wrote: > Ping for review? > > On Fri, Feb 10, 2023 at 11:43 PM Yabin Cui <yabinc(a)google.com> wrote: >> >> It's similar to what we did in tmc_read_unprepare_etb. >> >> Signed-off-by: Yabin Cui <yabinc(a)google.com> >> --- Thanks Yabin for the patch, will queue this at rc1 Suzuki >> drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c >> index 918d461fcf4a..b04f12079efd 100644 >> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c >> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c >> @@ -1763,6 +1763,7 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) >> { >> unsigned long flags; >> struct etr_buf *sysfs_buf = NULL; >> + int rc = 0; >> >> /* config types are set a boot time and never change */ >> if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR)) >> @@ -1777,7 +1778,11 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) >> * buffer. Since the tracer is still enabled drvdata::buf can't >> * be NULL. >> */ >> - __tmc_etr_enable_hw(drvdata); >> + rc = __tmc_etr_enable_hw(drvdata); >> + if (rc) { >> + spin_unlock_irqrestore(&drvdata->spinlock, flags); >> + return rc; >> + } >> } else { >> /* >> * The ETR is not tracing and the buffer was just read. >> -- >> 2.39.1.581.gbfd45094c4-goog >>

2 years, 4 months

[PATCH 0/3] Extend logging on TMC start / stop errors

by Mike Leach

Recent issues with trying to debug TMC timeouts and flush issues shows a general lack of logging and context around the possible errors Patchset addresses that. Signed-off-by: Mike Leach <mike.leach(a)linaro.org> Mike Leach (3): coresight: Update timeout functions to allow return of test register value coresight: tmc: Update error logging in tmc common functions coresight: etf: etr: Update logging around flush_and_stop() errors drivers/hwtracing/coresight/coresight-core.c | 50 +++++++++++++++---- .../hwtracing/coresight/coresight-tmc-core.c | 37 +++++++++++--- .../hwtracing/coresight/coresight-tmc-etf.c | 12 +++-- .../hwtracing/coresight/coresight-tmc-etr.c | 8 ++- drivers/hwtracing/coresight/coresight-tmc.h | 2 +- include/linux/coresight.h | 10 +++- 6 files changed, 93 insertions(+), 26 deletions(-) -- 2.17.1

2 years, 4 months

Re: [PATCH 21/24] Documentation: trace: correct spelling

by Mike Leach

Reviewed by: Mike Leach <mike.leach(a)linaro.org> On Thu, 9 Feb 2023 at 07:14, Randy Dunlap <rdunlap(a)infradead.org> wrote: > > Correct spelling problems for Documentation/trace/ as reported > by codespell. > > Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org> > Cc: Steven Rostedt <rostedt(a)goodmis.org> > Cc: Masami Hiramatsu <mhiramat(a)kernel.org> > Cc: Daniel Bristot de Oliveira <bristot(a)kernel.org> > Cc: linux-trace-kernel(a)vger.kernel.org > Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org> > Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com> > Cc: coresight(a)lists.linaro.org > Cc: linux-arm-kernel(a)lists.infradead.org > Cc: Jonathan Corbet <corbet(a)lwn.net> > Cc: linux-doc(a)vger.kernel.org > Reviewed-by: Mukesh Ojha <quic_mojha(a)quicinc.com> > Acked-by: Steven Rostedt (Google) <rostedt(a)goodmis.org> > Acked-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> # for coresight > --- > Documentation/trace/coresight/coresight-etm4x-reference.rst | 2 +- > Documentation/trace/events.rst | 6 +++--- > Documentation/trace/fprobe.rst | 2 +- > Documentation/trace/ftrace-uses.rst | 2 +- > Documentation/trace/hwlat_detector.rst | 2 +- > Documentation/trace/uprobetracer.rst | 2 +- > 7 files changed, 9 insertions(+), 9 deletions(-) > > diff -- a/Documentation/trace/coresight/coresight-etm4x-reference.rst b/Documentation/trace/coresight/coresight-etm4x-reference.rst > --- a/Documentation/trace/coresight/coresight-etm4x-reference.rst > +++ b/Documentation/trace/coresight/coresight-etm4x-reference.rst > @@ -675,7 +675,7 @@ Bit assignments shown below:- > reconstructed using only conditional branches. > > There is currently no support in Perf for supplying modified binaries to the decoder, so this > - feature is only inteded to be used for debugging purposes or with a 3rd party tool. > + feature is only intended to be used for debugging purposes or with a 3rd party tool. > > Choosing this option will result in a significant increase in the amount of trace generated - > possible danger of overflows, or fewer instructions covered. Note, that this option also > diff -- a/Documentation/trace/events.rst b/Documentation/trace/events.rst > --- a/Documentation/trace/events.rst > +++ b/Documentation/trace/events.rst > @@ -903,7 +903,7 @@ functions can be used. > > To create a kprobe event, an empty or partially empty kprobe event > should first be created using kprobe_event_gen_cmd_start(). The name > -of the event and the probe location should be specfied along with one > +of the event and the probe location should be specified along with one > or args each representing a probe field should be supplied to this > function. Before calling kprobe_event_gen_cmd_start(), the user > should create and initialize a dynevent_cmd object using > @@ -983,7 +983,7 @@ The basic idea is simple and amounts to > layer that can be used to generate trace event commands. The > generated command strings can then be passed to the command-parsing > and event creation code that already exists in the trace event > -subystem for creating the corresponding trace events. > +subsystem for creating the corresponding trace events. > > In a nutshell, the way it works is that the higher-level interface > code creates a struct dynevent_cmd object, then uses a couple > @@ -1056,7 +1056,7 @@ to add an operator between the pair (her > appended onto the end of the arg pair (here ';'). > > There's also a dynevent_str_add() function that can be used to simply > -add a string as-is, with no spaces, delimeters, or arg check. > +add a string as-is, with no spaces, delimiters, or arg check. > > Any number of dynevent_*_add() calls can be made to build up the string > (until its length surpasses cmd->maxlen). When all the arguments have > diff -- a/Documentation/trace/fprobe.rst b/Documentation/trace/fprobe.rst > --- a/Documentation/trace/fprobe.rst > +++ b/Documentation/trace/fprobe.rst > @@ -111,7 +111,7 @@ saved at function entry and passed to ex > the instruction pointer of @regs may be different from the @entry_ip > in the entry_handler. If you need traced instruction pointer, you need > to use @entry_ip. On the other hand, in the exit_handler, the instruction > - pointer of @regs is set to the currect return address. > + pointer of @regs is set to the correct return address. > > Share the callbacks with kprobes > ================================ > diff -- a/Documentation/trace/ftrace-uses.rst b/Documentation/trace/ftrace-uses.rst > --- a/Documentation/trace/ftrace-uses.rst > +++ b/Documentation/trace/ftrace-uses.rst > @@ -193,7 +193,7 @@ FTRACE_OPS_FL_RECURSION > Not, if this flag is set, then the callback will always be called > with preemption disabled. If it is not set, then it is possible > (but not guaranteed) that the callback will be called in > - preemptable context. > + preemptible context. > > FTRACE_OPS_FL_IPMODIFY > Requires FTRACE_OPS_FL_SAVE_REGS set. If the callback is to "hijack" > diff -- a/Documentation/trace/hwlat_detector.rst b/Documentation/trace/hwlat_detector.rst > --- a/Documentation/trace/hwlat_detector.rst > +++ b/Documentation/trace/hwlat_detector.rst > @@ -14,7 +14,7 @@ originally written for use by the "RT" p > kernel is highly latency sensitive. > > SMIs are not serviced by the Linux kernel, which means that it does not > -even know that they are occuring. SMIs are instead set up by BIOS code > +even know that they are occurring. SMIs are instead set up by BIOS code > and are serviced by BIOS code, usually for "critical" events such as > management of thermal sensors and fans. Sometimes though, SMIs are used for > other tasks and those tasks can spend an inordinate amount of time in the > diff -- a/Documentation/trace/uprobetracer.rst b/Documentation/trace/uprobetracer.rst > --- a/Documentation/trace/uprobetracer.rst > +++ b/Documentation/trace/uprobetracer.rst > @@ -55,7 +55,7 @@ Synopsis of uprobe_tracer > > (\*1) only for return probe. > (\*2) this is useful for fetching a field of data structures. > - (\*3) Unlike kprobe event, "u" prefix will just be ignored, becuse uprobe > + (\*3) Unlike kprobe event, "u" prefix will just be ignored, because uprobe > events can access only user-space memory. > > Types > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel(a)lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

2 years, 4 months

Re: [PATCH v3] coresight: tmc: Don't enable TMC when it's not ready.

by Suzuki K Poulose

On 09/02/2023 01:08, Yabin Cui wrote: > Friendly ping? > > On Thu, Feb 2, 2023 at 1:46 PM Yabin Cui <yabinc(a)google.com> wrote: >> >> If TMC ETR is enabled without being ready, in later use we may >> see AXI bus errors caused by accessing invalid addresses. >> >> Signed-off-by: Yabin Cui <yabinc(a)google.com> >> --- >> V1 -> V2: Make change to all TMCs instead of just ETR >> V2 -> V3: Handle etr enable failure in tmc_read_unprepare_etr As I mentioned, v2 was queued. Please could you update your changes on top of the coresight next branch and resend the patch ? Suzuki

2 years, 4 months

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

by Suzuki K Poulose

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-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, 4 months

Re: [PATCH v2] coresight: tmc: Don't enable TMC when it's not ready.

by Mike Leach

Hi, On Fri, 27 Jan 2023 at 23:10, Yabin Cui <yabinc(a)google.com> wrote: > > If TMC ETR is enabled without being ready, in later use we may > see AXI bus errors caused by accessing invalid addresses. > > Signed-off-by: Yabin Cui <yabinc(a)google.com> > --- > V1 -> V2: Make change to all TMCs instead of just ETR > > .../hwtracing/coresight/coresight-tmc-core.c | 4 +- > .../hwtracing/coresight/coresight-tmc-etf.c | 43 +++++++++++++++---- > .../hwtracing/coresight/coresight-tmc-etr.c | 18 ++++++-- > drivers/hwtracing/coresight/coresight-tmc.h | 2 +- > 4 files changed, 53 insertions(+), 14 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index 07abf28ad725..c106d142e632 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -31,7 +31,7 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); > DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); > DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); > > -void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) > +int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) > { > struct coresight_device *csdev = drvdata->csdev; > struct csdev_access *csa = &csdev->access; > @@ -40,7 +40,9 @@ void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) > if (coresight_timeout(csa, TMC_STS, TMC_STS_TMCREADY_BIT, 1)) { > dev_err(&csdev->dev, > "timeout while waiting for TMC to be Ready\n"); > + return -EBUSY; > } > + return 0; > } > > void tmc_flush_and_stop(struct tmc_drvdata *drvdata) > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c > index 4c4cbd1f7258..2840227e9135 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c > @@ -16,12 +16,19 @@ > static int tmc_set_etf_buffer(struct coresight_device *csdev, > struct perf_output_handle *handle); > > -static void __tmc_etb_enable_hw(struct tmc_drvdata *drvdata) > +static int __tmc_etb_enable_hw(struct tmc_drvdata *drvdata) > { > + int rc = 0; > + > CS_UNLOCK(drvdata->base); > > /* Wait for TMCSReady bit to be set */ > - tmc_wait_for_tmcready(drvdata); > + rc = tmc_wait_for_tmcready(drvdata); > + if (rc) { > + dev_err(&drvdata->csdev->dev, "fails to enable not ready TMC\n"); > + CS_LOCK(drvdata->base); > + return rc; > + } > > writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE); > writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI | > @@ -33,6 +40,7 @@ static void __tmc_etb_enable_hw(struct tmc_drvdata *drvdata) > tmc_enable_hw(drvdata); > > CS_LOCK(drvdata->base); > + return rc; > } > > static int tmc_etb_enable_hw(struct tmc_drvdata *drvdata) > @@ -42,8 +50,10 @@ static int tmc_etb_enable_hw(struct tmc_drvdata *drvdata) > if (rc) > return rc; > > - __tmc_etb_enable_hw(drvdata); > - return 0; > + rc = __tmc_etb_enable_hw(drvdata); > + if (rc) > + coresight_disclaim_device(drvdata->csdev); > + return rc; > } > > static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata) > @@ -91,12 +101,19 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata) > coresight_disclaim_device(drvdata->csdev); > } > > -static void __tmc_etf_enable_hw(struct tmc_drvdata *drvdata) > +static int __tmc_etf_enable_hw(struct tmc_drvdata *drvdata) > { > + int rc = 0; > + > CS_UNLOCK(drvdata->base); > > /* Wait for TMCSReady bit to be set */ > - tmc_wait_for_tmcready(drvdata); > + rc = tmc_wait_for_tmcready(drvdata); > + if (rc) { > + dev_err(&drvdata->csdev->dev, "fails to enable not ready TMC\n"); > + CS_LOCK(drvdata->base); > + return rc; > + } > > writel_relaxed(TMC_MODE_HARDWARE_FIFO, drvdata->base + TMC_MODE); > writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI, > @@ -105,6 +122,7 @@ static void __tmc_etf_enable_hw(struct tmc_drvdata *drvdata) > tmc_enable_hw(drvdata); > > CS_LOCK(drvdata->base); > + return rc; > } > > static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata) > @@ -114,8 +132,10 @@ static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata) > if (rc) > return rc; > > - __tmc_etf_enable_hw(drvdata); > - return 0; > + rc = __tmc_etf_enable_hw(drvdata); > + if (rc) > + coresight_disclaim_device(drvdata->csdev); > + return rc; > } > > static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata) > @@ -639,6 +659,7 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata) > char *buf = NULL; > enum tmc_mode mode; > unsigned long flags; > + int rc = 0; > > /* config types are set a boot time and never change */ > if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETB && > @@ -664,7 +685,11 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata) > * can't be NULL. > */ > memset(drvdata->buf, 0, drvdata->size); > - __tmc_etb_enable_hw(drvdata); > + rc = __tmc_etb_enable_hw(drvdata); > + if (rc) { > + spin_unlock_irqrestore(&drvdata->spinlock, flags); > + return rc; > + } There is a similar unprepare function in ETR - this should have similar updates. > } else { > /* > * The ETB/ETF is not tracing and the buffer was just read. > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 867ad8bb9b0c..0811cb44588b 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -983,15 +983,21 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata) > etr_buf->ops->sync(etr_buf, rrp, rwp); > } > > -static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) > +static int __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) > { > u32 axictl, sts; > struct etr_buf *etr_buf = drvdata->etr_buf; > + int rc = 0; > > CS_UNLOCK(drvdata->base); > > /* Wait for TMCSReady bit to be set */ > - tmc_wait_for_tmcready(drvdata); > + rc = tmc_wait_for_tmcready(drvdata); > + if (rc) { > + dev_err(&drvdata->csdev->dev, "fails to enable not ready TMC\n"); > + CS_LOCK(drvdata->base); > + return rc; > + } > > writel_relaxed(etr_buf->size / 4, drvdata->base + TMC_RSZ); > writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE); > @@ -1032,6 +1038,7 @@ static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) > tmc_enable_hw(drvdata); > > CS_LOCK(drvdata->base); > + return rc; > } > > static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, > @@ -1060,7 +1067,12 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, > rc = coresight_claim_device(drvdata->csdev); > if (!rc) { > drvdata->etr_buf = etr_buf; > - __tmc_etr_enable_hw(drvdata); > + rc = __tmc_etr_enable_hw(drvdata); > + if (rc) { > + drvdata->etr_buf = NULL; > + coresight_disclaim_device(drvdata->csdev); > + tmc_etr_disable_catu(drvdata); > + } > } > > return rc; > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > index 66959557cf39..01c0382a29c0 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -255,7 +255,7 @@ struct tmc_sg_table { > }; > > /* Generic functions */ > -void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata); > +int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata); > void tmc_flush_and_stop(struct tmc_drvdata *drvdata); > void tmc_enable_hw(struct tmc_drvdata *drvdata); > void tmc_disable_hw(struct tmc_drvdata *drvdata); > -- > 2.39.1.456.gfc5497dd1b-goog > The tmc_flush_and_stop() function also calls tmc_wait_for_tmcready(). The etb/etf_prepare_read operations disable the etb /etf before a read commences. For consistency if this fails due to TMC not being ready do we also need to fail the read prepare operations? Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

2 years, 4 months

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

by Suzuki K Poulose

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 ? 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, 4 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight