CoreSight

coresight@lists.linaro.org

9 participants
2375 discussions

Re: [PATCH] coresight-tpdm: Correct the property name of MSR number

by Tao Zhang

On 10/20/2023 2:55 PM, Krzysztof Kozlowski wrote: > On 20/10/2023 04:51, Tao Zhang wrote: >> Correct the property name of the DSB MSR number that needs to be >> read in TPDM driver. The right property name is >> "qcom,dsb-msrs-num". > Missing Fixes tag. I will add it in the next patch. > >> This patch depends on patch series "Add support to configure TPDM DSB >> subunit" >> https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=788353 > This is not suitable for commit msg. Dependencies are noted under ---. > > And how is this depending on that patch? Your buggy code was applied > long time ago! Yes, no need to depend on the patch series and it has been applied. I will remove this comments in the next patch. >> Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> >> --- >> drivers/hwtracing/coresight/coresight-tpdm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c >> index b25284e..97654aa 100644 >> --- a/drivers/hwtracing/coresight/coresight-tpdm.c >> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c >> @@ -892,7 +892,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) >> >> if (drvdata && tpdm_has_dsb_dataset(drvdata)) >> of_property_read_u32(drvdata->dev->of_node, >> - "qcom,dsb_msr_num", &drvdata->dsb_msr_num); >> + "qcom,dsb-msrs-num", &drvdata->dsb_msr_num); > So you never tested your DTS... We can keep asking about this but still > testing does not happen :/ Since this new property has not been applied on the exist upstream DTS, I tested this driver with the local DTS. Unfortunately, the property name in the local DTS is not updated, this is why it is not found in the tests. Best, Tao > > Best regards, > Krzysztof >

1 year, 8 months

[PATCH] coresight-tpdm: Correct the property name of MSR number

by Tao Zhang

Correct the property name of the DSB MSR number that needs to be read in TPDM driver. The right property name is "qcom,dsb-msrs-num". This patch depends on patch series "Add support to configure TPDM DSB subunit" https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=788353 Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> --- drivers/hwtracing/coresight/coresight-tpdm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c index b25284e..97654aa 100644 --- a/drivers/hwtracing/coresight/coresight-tpdm.c +++ b/drivers/hwtracing/coresight/coresight-tpdm.c @@ -892,7 +892,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) if (drvdata && tpdm_has_dsb_dataset(drvdata)) of_property_read_u32(drvdata->dev->of_node, - "qcom,dsb_msr_num", &drvdata->dsb_msr_num); + "qcom,dsb-msrs-num", &drvdata->dsb_msr_num); /* Set up coresight component description */ desc.name = coresight_alloc_device_name(&tpdm_devs, dev); -- 2.7.4

1 year, 8 months

Re: [PATCH 2/3] coresight: ultrasoc-smb: simplify the code for check to_copy valid

by hejunhao

On 2023/10/19 21:35, Jonathan Cameron wrote: > On Thu, 12 Oct 2023 17:47:05 +0800 > Junhao He <hejunhao3(a)huawei.com> wrote: > >> We only need to check once when before using the to_copy variable >> to simplify the code. >> >> Signed-off-by: Junhao He <hejunhao3(a)huawei.com> > I'm not convinced this one is an improvement. Sometimes it's easier to just > see the individual conditions checked even if we could combine them. > It's easy to understand we don't copy data if: > a) We ask for 0 data. > b) There is 0 data > > Less easy to establish that with the extra wrap around code in there > (even though that has no impact on to_copy if it is 0) > > Jonathan > Thanks, I will drop this patch. Best regards, Junhao. >> --- >> drivers/hwtracing/coresight/ultrasoc-smb.c | 13 ++++--------- >> 1 file changed, 4 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c >> index b08a619d1116..e78edc3480ce 100644 >> --- a/drivers/hwtracing/coresight/ultrasoc-smb.c >> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c >> @@ -127,20 +127,15 @@ static ssize_t smb_read(struct file *file, char __user *data, size_t len, >> struct smb_drv_data, miscdev); >> struct smb_data_buffer *sdb = &drvdata->sdb; >> struct device *dev = &drvdata->csdev->dev; >> - ssize_t to_copy = 0; >> - >> - if (!len) >> - return 0; >> - >> - if (!sdb->data_size) >> - return 0; >> - >> - to_copy = min(sdb->data_size, len); >> + ssize_t to_copy = min(sdb->data_size, len); >> >> /* Copy parts of trace data when read pointer wrap around SMB buffer */ >> if (sdb->buf_rdptr + to_copy > sdb->buf_size) >> to_copy = sdb->buf_size - sdb->buf_rdptr; >> >> + if (!to_copy) >> + return 0; >> + >> if (copy_to_user(data, sdb->buf_base + sdb->buf_rdptr, to_copy)) { >> dev_dbg(dev, "Failed to copy data to user\n"); >> return -EFAULT; > . >

1 year, 8 months

[PATCH v2 0/6] kvm/coresight: Support exclude guest and exclude host

by James Clark

This is a combination of the RFC for nVHE here [1] and v3 of VHE version here [2]. After a few of the review comments it seemed much simpler for both versions to use the same interface and be in the same patchset. FEAT_TRF is a Coresight feature that allows trace capture to be completely filtered at different exception levels, unlike the existing TRCVICTLR controls which may still emit target addresses of branches, even if the following trace is filtered. Without FEAT_TRF, it was possible to start a trace session on a host and also collect trace from the guest as TRCVICTLR was never programmed to exclude guests (and it could still emit target addresses even if it was). With FEAT_TRF, the current behavior of trace in guests exists depends on whether nVHE or VHE are being used. Both of the examples below are from the host's point of view, as Coresight isn't accessible from guests. This patchset is only relevant to when FEAT_TRF exists, otherwise there is no change. nVHE: Because the host and the guest are both using TRFCR_EL1, trace will be generated in guests depending on the same filter rules the host is using. For example if the host is tracing userspace only, then guest userspace trace will also be collected. (This is further limited by whether TRBE is used because an issue with TRBE means that it's completely disabled in nVHE guests, but it's possible to have other tracing components.) VHE: With VHE, the host filters will be in TRFCR_EL2, but the filters in TRFCR_EL1 will be active when the guest is running. Because we don't write to TRFCR_EL1, guest trace will be completely disabled. With this change, the guest filtering rules from the Perf session are honored for both nVHE and VHE modes. This is done by either writing to TRFCR_EL12 at the start of the Perf session and doing nothing else further, or caching the guest value and writing it at guest switch for nVHE. The first commit moves the register to sysreg because I add the EL12 version in a later commit. --- Changes since V1: * Squashed all the arm64/tools/sysreg changes into the first commit * Add a new commit to move SPE and TRBE regs into the kvm sysreg array * Add a comment above the TRFCR global that it's per host CPU rather than vcpu Changes since nVHE RFC [1]: * Re-write just in terms of the register value to be written for the host and the guest. This removes some logic from the hyp code and a value of kvm_vcpu_arch:trfcr_el1 = 0 no longer means "don't restore". * Remove all the conditional compilation and new files. * Change the kvm_etm_update_vcpu_events macro to a function. * Re-use DEBUG_STATE_SAVE_TRFCR so iflags don't need to be expanded anymore. * Expand the cover letter. Changes since VHE v3 [2]: * Use the same interface as nVHE mode so TRFCR_EL12 is now written by kvm. [1]: https://lore.kernel.org/kvmarm/20230804101317.460697-1-james.clark@arm.com/ [2]: https://lore.kernel.org/kvmarm/20230905102117.2011094-1-james.clark@arm.com/ James Clark (6): arm64/sysreg: Move TRFCR definitions to sysreg arm64: KVM: Rename DEBUG_STATE_SAVE_TRBE to DEBUG_STATE_SAVE_TRFCR arm64: KVM: Move SPE and trace registers to the sysreg array arm64: KVM: Add interface to set guest value for TRFCR register arm64: KVM: Write TRFCR value on guest switch with nVHE coresight: Pass guest TRFCR value to KVM arch/arm64/include/asm/kvm_host.h | 13 +-- arch/arm64/include/asm/kvm_hyp.h | 6 +- arch/arm64/include/asm/sysreg.h | 12 --- arch/arm64/kvm/arm.c | 1 + arch/arm64/kvm/debug.c | 43 ++++++++- arch/arm64/kvm/hyp/nvhe/debug-sr.c | 87 +++++++++++-------- arch/arm64/kvm/hyp/nvhe/switch.c | 4 +- arch/arm64/tools/sysreg | 41 +++++++++ .../coresight/coresight-etm4x-core.c | 42 +++++++-- drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- drivers/hwtracing/coresight/coresight-priv.h | 3 + 11 files changed, 186 insertions(+), 68 deletions(-) -- 2.34.1

1 year, 8 months

Re: [PATCH 3/3] coresight: ultrasoc-smb: fix uninitialized before use buf_hw_base

by hejunhao

On 2023/10/19 10:34, Yicong Yang wrote: > On 2023/10/12 17:47, Junhao He wrote: >> In smb_reset_buffer, the sdb->buf_hw_base variable is uninitialized >> before use, which initializes it in smb_init_data_buffer. And the SMB >> regiester are set in smb_config_inport. >> So move the call after smb_config_inport. >> >> Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver") >> >> Signed-off-by: Junhao He <hejunhao3(a)huawei.com> >> --- >> drivers/hwtracing/coresight/ultrasoc-smb.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c >> index e78edc3480ce..21ba473786e5 100644 >> --- a/drivers/hwtracing/coresight/ultrasoc-smb.c >> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c >> @@ -475,7 +475,6 @@ static int smb_init_data_buffer(struct platform_device *pdev, >> static void smb_init_hw(struct smb_drv_data *drvdata) >> { >> smb_disable_hw(drvdata); >> - smb_reset_buffer(drvdata); >> >> writel(SMB_LB_CFG_LO_DEFAULT, drvdata->base + SMB_LB_CFG_LO_REG); >> writel(SMB_LB_CFG_HI_DEFAULT, drvdata->base + SMB_LB_CFG_HI_REG); >> @@ -597,6 +596,7 @@ static int smb_probe(struct platform_device *pdev) >> } >> >> platform_set_drvdata(pdev, drvdata); >> + smb_reset_buffer(drvdata); > Shouldn't we reset the buffer before registering the sink? Otherwise it'll > be possible for user to access an unreset one. > Hi Yicong, Thanks for the comments! The code after the smb_register_sink() function also needs to moved. I will fix all in next version. Best regards, Junhao.

1 year, 8 months

Re: [PATCH 1/3] coresight: ultrasoc-smb: fix sleep while close preempt in enable_smb

by hejunhao

Hi Yicong, On 2023/10/19 11:05, Yicong Yang wrote: > On 2023/10/12 17:47, Junhao He wrote: > > > Use spinlock replace mutex to control driver data access to one at a > time. But the function copy_to_user() may sleep so spinlock do not to > lock it. > > Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver") > Signed-off-by: Junhao He <hejunhao3(a)huawei.com> > --- > drivers/hwtracing/coresight/ultrasoc-smb.c | 36 ++++++++++------------ > drivers/hwtracing/coresight/ultrasoc-smb.h | 6 ++-- > 2 files changed, 19 insertions(+), 23 deletions(-) > > diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c > index e9a32a97fbee..b08a619d1116 100644 > --- a/drivers/hwtracing/coresight/ultrasoc-smb.c > +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c > @@ -99,7 +99,7 @@ static int smb_open(struct inode *inode, struct file *file) > struct smb_drv_data, miscdev); > int ret = 0; > > - mutex_lock(&drvdata->mutex); > + spin_lock(&drvdata->spinlock); > > if (drvdata->reading) { > ret = -EBUSY; > @@ -115,7 +115,7 @@ static int smb_open(struct inode *inode, struct file *file) > > drvdata->reading = true; > out: > - mutex_unlock(&drvdata->mutex); > + spin_unlock(&drvdata->spinlock); > > return ret; > } > @@ -132,10 +132,8 @@ static ssize_t smb_read(struct file *file, char __user *data, size_t len, > if (!len) > return 0; > > - mutex_lock(&drvdata->mutex); > - > if (!sdb->data_size) > - goto out; > + return 0; > > to_copy = min(sdb->data_size, len); > > @@ -145,20 +143,18 @@ static ssize_t smb_read(struct file *file, char __user *data, size_t len, > > if (copy_to_user(data, sdb->buf_base + sdb->buf_rdptr, to_copy)) { > dev_dbg(dev, "Failed to copy data to user\n"); > - to_copy = -EFAULT; > - goto out; > + return -EFAULT; > } > > + spin_lock(&drvdata->spinlock); > *ppos += to_copy; > - > smb_update_read_ptr(drvdata, to_copy); > > - dev_dbg(dev, "%zu bytes copied\n", to_copy); > -out: > if (!sdb->data_size) > smb_reset_buffer(drvdata); > - mutex_unlock(&drvdata->mutex); > + spin_unlock(&drvdata->spinlock); > Do we still need the lock here? If we get here, we should have the exclusive > access to the file, which is protected in open(). Or any other cases? This is something I've also considered. If someone opens an SMB device and reads it using multithreading, The SMB device will got out of sync. I've seen other coresight sink drivers such as etb do not use locks in the read function. Maybe it's not necessary here, the buffer synchronization is guaranteed by the user. Best regards, Junhao.

1 year, 8 months

[PATCH v1 0/2] perf cs-etm: Add support for itrace option 'T'

by Leo Yan

This patch set is to introduce a new itrace option 'T' for forcily use timestamp trace for kernel time and support this option in cs-etm. Some Arm platforms (either Arm CPUs prior to Armv8 or miss the FEAT_TRF feature, currently the ETM driver cannot decide if the timestamp trace is same with kernel time. This is why we introduce the itrace option 'T', we leave decision to users so users can specify this option to forcily use the timestamp trace as the kernel time. This patch set is tested on Arm Juno board. Leo Yan (2): perf auxtrace: Add 'T' itrace option for timestamp trace perf cs-etm: Enable itrace option 'T' tools/perf/Documentation/itrace.txt | 1 + tools/perf/util/auxtrace.c | 3 +++ tools/perf/util/auxtrace.h | 3 +++ tools/perf/util/cs-etm.c | 21 ++++++++++++++++++--- 4 files changed, 25 insertions(+), 3 deletions(-) -- 2.34.1

1 year, 8 months

Re: [PATCH v1 1/2] perf auxtrace: Add 'T' itrace option for timestamp trace

by James Clark

On 19/10/2023 11:47, Adrian Hunter wrote: > On 14/10/23 10:45, Leo Yan wrote: >> An AUX trace can contain timestamp, but in some situations, the hardware >> trace module (e.g. Arm CoreSight) cannot decide the traced timestamp is >> the same source with CPU's time, thus the decoder can not use the >> timestamp trace for samples. >> >> This patch introduces 'T' itrace option. If users know the platforms > > "If users know" <- how would users know? Could the kernel > or tools also figure it out? > The problem is this was only made into a discoverable feature with Feat_TRF in Armv8.4. So this workaround is to support devices that already had the right kind of timestamps before that feature. It might be possible to list every device in the driver, but maybe there could even be some firmware that disconnects the clock source on a device that we thought would have it. IMO adding this option is the best way to workaround it. >> they are working on have the same time counter with CPUs, users can >> use this new option to tell a decoder for using timestamp trace as >> kernel time. >> >> Signed-off-by: Leo Yan <leo.yan(a)linaro.org> >> --- >> tools/perf/Documentation/itrace.txt | 1 + >> tools/perf/util/auxtrace.c | 3 +++ >> tools/perf/util/auxtrace.h | 3 +++ >> 3 files changed, 7 insertions(+) >> >> diff --git a/tools/perf/Documentation/itrace.txt b/tools/perf/Documentation/itrace.txt >> index a97f95825b14..19cc179be9a7 100644 >> --- a/tools/perf/Documentation/itrace.txt >> +++ b/tools/perf/Documentation/itrace.txt >> @@ -25,6 +25,7 @@ >> q quicker (less detailed) decoding >> A approximate IPC >> Z prefer to ignore timestamps (so-called "timeless" decoding) >> + T use the timestamp trace as kernel time >> >> The default is all events i.e. the same as --itrace=iybxwpe, >> except for perf script where it is --itrace=ce >> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c >> index a0368202a746..f528c4364d23 100644 >> --- a/tools/perf/util/auxtrace.c >> +++ b/tools/perf/util/auxtrace.c >> @@ -1638,6 +1638,9 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts, >> case 'Z': >> synth_opts->timeless_decoding = true; >> break; >> + case 'T': >> + synth_opts->use_timestamp = true; >> + break; >> case ' ': >> case ',': >> break; >> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h >> index 29eb82dff574..55702215a82d 100644 >> --- a/tools/perf/util/auxtrace.h >> +++ b/tools/perf/util/auxtrace.h >> @@ -99,6 +99,7 @@ enum itrace_period_type { >> * @remote_access: whether to synthesize remote access events >> * @mem: whether to synthesize memory events >> * @timeless_decoding: prefer "timeless" decoding i.e. ignore timestamps >> + * @use_timestamp: use the timestamp trace as kernel time >> * @vm_time_correlation: perform VM Time Correlation >> * @vm_tm_corr_dry_run: VM Time Correlation dry-run >> * @vm_tm_corr_args: VM Time Correlation implementation-specific arguments >> @@ -146,6 +147,7 @@ struct itrace_synth_opts { >> bool remote_access; >> bool mem; >> bool timeless_decoding; >> + bool use_timestamp; >> bool vm_time_correlation; >> bool vm_tm_corr_dry_run; >> char *vm_tm_corr_args; >> @@ -678,6 +680,7 @@ bool auxtrace__evsel_is_auxtrace(struct perf_session *session, >> " q: quicker (less detailed) decoding\n" \ >> " A: approximate IPC\n" \ >> " Z: prefer to ignore timestamps (so-called \"timeless\" decoding)\n" \ >> +" T: use the timestamp trace as kernel time\n" \ >> " PERIOD[ns|us|ms|i|t]: specify period to sample stream\n" \ >> " concatenate multiple options. Default is iybxwpe or cewp\n" >> > >

1 year, 8 months

[PATCH] coresight: etm4x: Enable ETE device accessed via MMIO

by Ruidong Tian

The ETM4X driver now assume that all ETE as CPU system instructions accessed device, in fact the ETE device on some machines also accessed via MMIO. Signed-off-by: Ruidong Tian <tianruidong(a)linux.alibaba.com> --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 285539104bcc..ad298c9cc87e 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1103,8 +1103,9 @@ static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata, * with MMIO. But we cannot touch the OSLK until we are * sure this is an ETM. So rely only on the TRCDEVARCH. */ - if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH) { - pr_warn_once("TRCDEVARCH doesn't match ETMv4 architecture\n"); + if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH && + (devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETE_ARCH) { + pr_warn_once("TRCDEVARCH doesn't match ETMv4/ETE architecture\n"); return false; } -- 2.33.1

1 year, 8 months

RE: [EXT] Re: [PATCH 1/7] dt-bindings: arm: coresight-tmc: Add "memory-region" property

by Linu Cherian

Hi Conor, > -----Original Message----- > From: Conor Dooley <conor(a)kernel.org> > Sent: Saturday, October 14, 2023 6:56 PM > To: Linu Cherian <lcherian(a)marvell.com> > Cc: Mike Leach <mike.leach(a)linaro.org>; suzuki.poulose(a)arm.com; > james.clark(a)arm.com; leo.yan(a)linaro.org; linux-arm- > kernel(a)lists.infradead.org; coresight(a)lists.linaro.org; linux- > kernel(a)vger.kernel.org; robh+dt(a)kernel.org; > krzysztof.kozlowski+dt(a)linaro.org; conor+dt(a)kernel.org; > devicetree(a)vger.kernel.org; Sunil Kovvuri Goutham > <sgoutham(a)marvell.com>; George Cherian <gcherian(a)marvell.com> > Subject: Re: [EXT] Re: [PATCH 1/7] dt-bindings: arm: coresight-tmc: Add > "memory-region" property > > On Sat, Oct 14, 2023 at 11:36:37AM +0000, Linu Cherian wrote: > > > > + - description: Reserved meta data memory. Used for ETR and ETF > sinks. > > > > + > > > > + memory-region-names: > > > > + items: > > > > + - const: trace-mem > > > > + - const: metadata-mem > > > > + > > > > > > Is there a constraint required here? If we are using the memory area > > > for trace in a panic situation, then we must have the meta data > > > memory area defined? > > > Perhaps a set of names such as "etr-trace-mem", "panic-trace-mem" , > > > "panic-metadata-mem", were the first is for general ETR trace in > > > non-panic situation and then constrain the "panic-" areas to appear > together. > > > The "etr-trace-mem", "panic-trace-mem" could easily point to the > > > same area. > > > > > As noted above, we do not have other generic use case for these reserved > regions now. > > Hence two regions/names, panic-trace-mem and panic-metadata-mem > with > > constraints kept as > > minItems: 2 and maxItems: 2 would suffice ? > > Whatever you do, please delete the -mem suffix. Ack.

1 year, 8 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight