CoreSight

coresight@lists.linaro.org

9 participants
2378 discussions

Re: [RFC V2 PATCH] coresight: change tmc from misc device to cdev device

by Mathieu Poirier

Good day Jay, On Fri, Dec 17, 2021 at 11:42:08AM +0800, Jay Chen wrote: > From: root <root(a)localhost.localdomain> > > For coresight in the server scenario, if there are too many > funnel (about 130) and tmc (about 130) peripherals, the error > of insufficient device numbers will occur by misc device > Why is the changelog mention funnels when they have nothing to do with misc devices? > Signed-off-by: root <root(a)localhost.localdomain> > --- > .../hwtracing/coresight/coresight-tmc-core.c | 84 +++++++++++++++---- > drivers/hwtracing/coresight/coresight-tmc.h | 10 ++- > 2 files changed, 75 insertions(+), 19 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index d0276af82494..03b114c0616a 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -31,6 +31,11 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); > DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); > DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); > > +static dev_t tmc_major; > +static struct class *tmc_class; > + > +#define TMC_DEV_MAX (MINORMASK + 1) > + > void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) > { > struct coresight_device *csdev = drvdata->csdev; > @@ -147,7 +152,7 @@ static int tmc_open(struct inode *inode, struct file *file) > { > int ret; > struct tmc_drvdata *drvdata = container_of(file->private_data, > - struct tmc_drvdata, miscdev); > + struct tmc_drvdata, cdev); > > ret = tmc_read_prepare(drvdata); > if (ret) > @@ -179,7 +184,7 @@ static ssize_t tmc_read(struct file *file, char __user *data, size_t len, > char *bufp; > ssize_t actual; > struct tmc_drvdata *drvdata = container_of(file->private_data, > - struct tmc_drvdata, miscdev); > + struct tmc_drvdata, cdev); > actual = tmc_get_sysfs_trace(drvdata, *ppos, len, &bufp); > if (actual <= 0) > return 0; > @@ -200,7 +205,7 @@ static int tmc_release(struct inode *inode, struct file *file) > { > int ret; > struct tmc_drvdata *drvdata = container_of(file->private_data, > - struct tmc_drvdata, miscdev); > + struct tmc_drvdata, cdev); > > ret = tmc_read_unprepare(drvdata); > if (ret) > @@ -449,8 +454,9 @@ static u32 tmc_etr_get_max_burst_size(struct device *dev) > > static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > { > - int ret = 0; > + int ret = 0, tmc_num = 0; > u32 devid; > + dev_t devt; > void __iomem *base; > struct device *dev = &adev->dev; > struct coresight_platform_data *pdata = NULL; > @@ -546,14 +552,26 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > goto out; > } > > - drvdata->miscdev.name = desc.name; > - drvdata->miscdev.minor = MISC_DYNAMIC_MINOR; > - drvdata->miscdev.fops = &tmc_fops; > - ret = misc_register(&drvdata->miscdev); > - if (ret) > + tmc_num = etb_devs.nr_idx + etf_devs.nr_idx + > + etr_devs.nr_idx; Humm... This is turning out to be a lot worse than expected. Please use an IDA, the same way the rpmsg_char driver does. > + > + cdev_init(&drvdata->cdev.cdev, &tmc_fops); > + drvdata->cdev.cdev.owner = THIS_MODULE; > + devt = MKDEV(MAJOR(tmc_major), tmc_num - 1); > + ret = cdev_add(&drvdata->cdev.cdev, devt, 1); > + if (ret) { > coresight_unregister(drvdata->csdev); > - else > + goto out; > + } > + > + drvdata->cdev.dev = device_create(tmc_class, NULL, devt, &drvdata->cdev, desc.name); > + if (IS_ERR(drvdata->cdev.dev)) { > + ret = PTR_ERR(drvdata->cdev.dev); > + cdev_del(&drvdata->cdev.cdev); > + coresight_unregister(drvdata->csdev); > + } else > pm_runtime_put(&adev->dev); > + > out: > return ret; > } > @@ -584,12 +602,8 @@ static void tmc_remove(struct amba_device *adev) > { > struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev); > > - /* > - * Since misc_open() holds a refcount on the f_ops, which is > - * etb fops in this case, device is there until last file > - * handler to this device is closed. > - */ > - misc_deregister(&drvdata->miscdev); > + device_destroy(tmc_class, drvdata->cdev.dev->devt); > + cdev_del(&drvdata->cdev.cdev); > coresight_unregister(drvdata->csdev); > } > > @@ -618,7 +632,43 @@ static struct amba_driver tmc_driver = { > .id_table = tmc_ids, > }; > > -module_amba_driver(tmc_driver); > +static int __init tmc_init(void) > +{ > + int ret; > + > + ret = alloc_chrdev_region(&tmc_major, 0, TMC_DEV_MAX, "tmc"); > + if (ret < 0) { > + pr_err("tmc: failed to allocate char dev region\n"); > + return ret; > + } > + > + tmc_class = class_create(THIS_MODULE, "tmc"); > + if (IS_ERR(tmc_class)) { > + pr_err("failed to create tmc class\n"); > + ret = PTR_ERR(tmc_class); > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > + return ret; > + } > + > + ret = amba_driver_register(&tmc_driver); > + if (ret) { > + pr_err("tmc: error registering amba driver\n"); > + class_destroy(tmc_class); > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > + } > + > + return ret; > +} > + > +static void __exit tmc_exit(void) > +{ > + amba_driver_unregister(&tmc_driver); > + class_destroy(tmc_class); > + unregister_chrdev_region(tmc_major, TMC_DEV_MAX); > +} > + > +module_init(tmc_init); > +module_exit(tmc_exit); > > MODULE_AUTHOR("Pratik Patel <pratikp(a)codeaurora.org>"); > MODULE_DESCRIPTION("Arm CoreSight Trace Memory Controller driver"); > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > index 6bec20a392b3..b65ac363f9e4 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -7,6 +7,7 @@ > #ifndef _CORESIGHT_TMC_H > #define _CORESIGHT_TMC_H > > +#include <linux/cdev.h> > #include <linux/dma-mapping.h> > #include <linux/idr.h> > #include <linux/miscdevice.h> > @@ -163,11 +164,16 @@ struct etr_buf { > void *private; > }; > > +struct tmc_cdev { > + struct cdev cdev; > + struct device *dev; > +}; > + > /** > * struct tmc_drvdata - specifics associated to an TMC component > * @base: memory mapped base address for this component. > * @csdev: component vitals needed by the framework. > - * @miscdev: specifics to handle "/dev/xyz.tmc" entry. > + * @tmc_cdev: specifics to handle "/dev/xyz.tmc" entry. > * @spinlock: only one at a time pls. > * @pid: Process ID of the process being monitored by the session > * that is using this component. > @@ -191,7 +197,7 @@ struct etr_buf { > struct tmc_drvdata { > void __iomem *base; > struct coresight_device *csdev; > - struct miscdevice miscdev; > + struct tmc_cdev cdev; > spinlock_t spinlock; > pid_t pid; > bool reading; > -- > 2.27.0 >

3 years, 6 months

Re: [PATCH 02/12] perf test: Shell - only run .sh shell files to skip other files

by Carsten Haitzler

On 12/16/21 10:22, Daniel Thompson wrote: > On Wed, Dec 15, 2021 at 04:03:53PM +0000, carsten.haitzler(a)foss.arm.com wrote: >> From: Carsten Haitzler <carsten.haitzler(a)arm.com> >> >> You edit your scripts in the tests and end up with your usual shell >> backup files with ~ or .bak or something else at the end, but then your >> next perf test run wants to run the backups too. You might also have perf >> .data files in the directory or something else undesireable as well. You end >> up chasing which test is the one you edited and the backup and have to keep >> removing all the backup files, so automatically skip any files that are >> not plain *.sh scripts to limit the time wasted in chasing ghosts. >> >> Signed-off-by: Carsten Haitzler <carsten.haitzler(a)arm.com> > > Why require both executable *and* endswith('.sh')? Paranoia. :) Making sure we really only run things that are meant to be run and avoid other junk/tmp/whatever files. >> --- >> tools/perf/tests/builtin-test.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c >> index ece272b55587..849737ead9fd 100644 >> --- a/tools/perf/tests/builtin-test.c >> +++ b/tools/perf/tests/builtin-test.c >> @@ -297,7 +297,20 @@ static const char *shell_test__description(char *description, size_t size, >> for (int __i = 0; __i < nr && (ent = entlist[__i]); __i++) \ >> if (!is_directory(base, ent) && \ >> is_executable_file(base, ent) && \ >> - ent->d_name[0] != '.') >> + ent->d_name[0] != '.' && \ >> + (shell_file_is_sh(ent->d_name) == 0)) >> + >> +static int shell_file_is_sh(const char *file) >> +{ >> + const char *ext; >> + >> + ext = strchr(file, '.'); > > Shouldn't this be strrchr()? Oh indeed probably should be. My bad. Nothing uses a dot inside the filename yet. I can fix that - will wait for the rest to come in before doing an update

3 years, 6 months

Re: [PATCH v2 2/9] Coresight: Add coresight TPDM source driver

by Mike Leach

Hi, I have a couple of comments relating to Coresight trace IDs On Wed, 15 Dec 2021 at 16:10, Jinlong Mao <quic_jinlmao(a)quicinc.com> wrote: > > Hi Mathieu, > > On 12/15/2021 2:57 AM, Mathieu Poirier wrote: > > On Thu, Dec 09, 2021 at 10:15:36PM +0800, Mao Jinlong wrote: > >> Add driver to support Coresight device TPDM (Trace, Profiling and > >> Diagnostics Monitor). TPDM is a monitor to collect data from > >> different datasets. This change is to add probe/enable/disable > >> functions for tpdm source. > >> > >> Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> > >> Signed-off-by: Mao Jinlong <quic_jinlmao(a)quicinc.com> > >> --- > >> MAINTAINERS | 8 + > >> drivers/hwtracing/coresight/Kconfig | 13 ++ > >> drivers/hwtracing/coresight/Makefile | 1 + > >> drivers/hwtracing/coresight/coresight-core.c | 3 +- > >> drivers/hwtracing/coresight/coresight-tpdm.c | 152 +++++++++++++++++++ > >> drivers/hwtracing/coresight/coresight-tpdm.h | 31 ++++ > >> include/linux/coresight.h | 1 + > >> 7 files changed, 208 insertions(+), 1 deletion(-) > >> create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c > >> create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h > >> > >> diff --git a/MAINTAINERS b/MAINTAINERS > >> index 7a2345ce8521..59f39b3194f6 100644 > >> --- a/MAINTAINERS > >> +++ b/MAINTAINERS > >> @@ -15560,6 +15560,14 @@ L: netdev(a)vger.kernel.org > >> S: Supported > >> F: drivers/net/ipa/ > >> > >> +QCOM CORESIGHT COMPONENTS DRIVER > >> +M: Tao Zhang <quic_taozha(a)quicinc.com> > >> +M: Jinlong Mao <quic_jinlmao(a)quicinc.com> > >> +M: Mathieu Poirier <mathieu.poirier(a)linaro.org> > >> +M: Suzuki K Poulose <suzuki.poulose(a)arm.com> > >> +S: Maintained > >> +F: drivers/hwtracing/coresight/coresight-tpdm.c > >> + > > There is no need for an extra entry in the MAINTAINERS file. The checkpatch.pl > > script is smart enough to know when to CC you and Tao every time the TPDM/TPDA > > drivers are modified. Suzuki and I will simply wait for you guys to add your RB > > tags before reviewing the patches. I have explained this in the previous revision. > > Hi Mathieu, > > I will remove this entry. > > >> QEMU MACHINE EMULATOR AND VIRTUALIZER SUPPORT > >> M: Gabriel Somlo <somlo(a)cmu.edu> > >> M: "Michael S. Tsirkin" <mst(a)redhat.com> > >> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig > >> index 514a9b8086e3..5c506a1cd08f 100644 > >> --- a/drivers/hwtracing/coresight/Kconfig > >> +++ b/drivers/hwtracing/coresight/Kconfig > >> @@ -201,4 +201,17 @@ config CORESIGHT_TRBE > >> > >> To compile this driver as a module, choose M here: the module will be > >> called coresight-trbe. > >> + > >> +config CORESIGHT_TPDM > >> + tristate "CoreSight Trace, Profiling & Diagnostics Monitor driver" > >> + select CORESIGHT_LINKS_AND_SINKS > > Is this available on 32bit HW as well? If not please make it dependent on ARM64 > > as it for ETMv4x devices. > This is available on 32bit HW as well. > >> + help > >> + This driver provides support for configuring monitor. Monitors are > >> + primarily responsible for data set collection and support the > >> + ability to collect any permutation of data set types. Monitors are > >> + also responsible for interaction with system cross triggering. > >> + > >> + To compile this driver as a module, choose M here: the module will be > >> + called coresight-tpdm. > >> + > >> endif > >> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile > >> index b6c4a48140ec..e7392a0dddeb 100644 > >> --- a/drivers/hwtracing/coresight/Makefile > >> +++ b/drivers/hwtracing/coresight/Makefile > >> @@ -25,5 +25,6 @@ obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o > >> obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o > >> obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o > >> obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o > >> +obj-$(CONFIG_CORESIGHT_TPDM) += coresight-tpdm.o > >> coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ > >> coresight-cti-sysfs.o > >> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > >> index cc6b6cabf85f..a7f1a6f09cfb 100644 > >> --- a/drivers/hwtracing/coresight/coresight-core.c > >> +++ b/drivers/hwtracing/coresight/coresight-core.c > >> @@ -1071,7 +1071,8 @@ static int coresight_validate_source(struct coresight_device *csdev, > >> } > >> > >> if (subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_PROC && > >> - subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE) { > >> + subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE && > >> + subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SYS) { > >> dev_err(&csdev->dev, "wrong device subtype in %s\n", function); > >> return -EINVAL; > >> } > >> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c > >> new file mode 100644 > >> index 000000000000..f494cef4fb24 > >> --- /dev/null > >> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c > >> @@ -0,0 +1,152 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved. > >> + */ > >> + > >> +#include <linux/amba/bus.h> > >> +#include <linux/bitmap.h> > >> +#include <linux/coresight.h> > >> +#include <linux/device.h> > >> +#include <linux/err.h> > >> +#include <linux/fs.h> > >> +#include <linux/io.h> > >> +#include <linux/kernel.h> > >> +#include <linux/module.h> > >> +#include <linux/of.h> > >> +#include <linux/regulator/consumer.h> > >> + > >> +#include "coresight-priv.h" > >> +#include "coresight-tpdm.h" > >> + > >> +DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm"); > >> + > >> +static int tpdm_enable(struct coresight_device *csdev, > >> + struct perf_event *event, u32 mode) > >> +{ > >> + struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > >> + > >> + mutex_lock(&drvdata->lock); > >> + if (drvdata->enable) { > >> + mutex_unlock(&drvdata->lock); > >> + return -EBUSY; > >> + } > >> + > >> + drvdata->enable = true; > >> + mutex_unlock(&drvdata->lock); > >> + > >> + dev_info(drvdata->dev, "TPDM tracing enabled\n"); > >> + return 0; > >> +} > >> + > >> +static void tpdm_disable(struct coresight_device *csdev, > >> + struct perf_event *event) > >> +{ > >> + struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > >> + > >> + mutex_lock(&drvdata->lock); > >> + if (!drvdata->enable) { > >> + mutex_unlock(&drvdata->lock); > >> + return; > >> + } > >> + > >> + drvdata->enable = false; > >> + mutex_unlock(&drvdata->lock); > >> + > >> + dev_info(drvdata->dev, "TPDM tracing disabled\n"); > >> +} > >> + > >> +static int tpdm_trace_id(struct coresight_device *csdev) > >> +{ > >> + struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > >> + > >> + return drvdata->traceid; > >> +} > >> + > >> +static const struct coresight_ops_source tpdm_source_ops = { > >> + .trace_id = tpdm_trace_id, > >> + .enable = tpdm_enable, > >> + .disable = tpdm_disable, > >> +}; > >> + > >> +static const struct coresight_ops tpdm_cs_ops = { > >> + .source_ops = &tpdm_source_ops, > >> +}; > >> + > >> +static void tpdm_init_default_data(struct tpdm_drvdata *drvdata) > >> +{ > >> + static int traceid = TPDM_TRACE_ID_START; > >> + > >> + drvdata->traceid = traceid++; > >> +} > > I have been specific on how to properly do this in the last revision. Given the > > above about the MAINTAINERS file, I am not sure that I will continue reviewing this set. > > > > There is also no need to rush another revision as I won't have the bandwidth to > > process it before the holidays. > > > > Thanks, > > Mathieu > > Hi Mathieu, > > Sorry, not addressed your previous comments here. > > For the trace id, each coresight component has 7 bits to store the trace > id. So the trace id should be from 1 to 127 as 0 is invalid. IDs 0x70 - 0x7F (`112 - 127 ) are reserved - see the ARM Coresight Architecture specification v3.0 > > Apart from TPDMs/STM/ETMs, we also have other coresight components in > our internal device. About 80 ids are already used. > > Some components have fixed trace id in HW. If we use functions below to > count the trace id, there will be conflict to other components. > > Can we use 1-15 for etm trace ids and 16 - 127 for other coresight > components ? And handle trace ids in its' own driver ? > This will limit systems to 15 cores - some have more! > > static inline int coresight_get_system_trace_id(int id) > { > /* Start system IDs above the highest per CPU trace ID. */ > return coresigth_get_trace_id(cpumask_last(cpu_possible_mask) + 1); > } > > static inline int coresight_get_trace_id(int cpu) > { > /* > * A trace ID of value 0 is invalid, so let's start at some > * random value that fits in 7 bits and go from there. Since > * the common convention is to have data trace IDs be I(N) + 1, > * set instruction trace IDs as a function of the CPU number. > */ > return (CORESIGHT_ETM_PMU_SEED + (cpu * 2)); > } > This fixed relationship between cpu and trace ID is used in the perf tooling to populate the elements in the perf.data file to correctly allow association between CPU and trace data, and thus allow correct trace decode. It should be possible to create another more dynamic mapping scheme - but this must include a way to support the perf requirements too. Regards Mike > Thanks > > Jinlong Mao > > >> + > >> +static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) > >> +{ > >> + struct device *dev = &adev->dev; > >> + struct coresight_platform_data *pdata; > >> + struct tpdm_drvdata *drvdata; > >> + struct coresight_desc desc = { 0 }; > >> + > >> + desc.name = coresight_alloc_device_name(&tpdm_devs, dev); > >> + if (!desc.name) > >> + return -ENOMEM; > >> + pdata = coresight_get_platform_data(dev); > >> + if (IS_ERR(pdata)) > >> + return PTR_ERR(pdata); > >> + adev->dev.platform_data = pdata; > >> + > >> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > >> + if (!drvdata) > >> + return -ENOMEM; > >> + drvdata->dev = &adev->dev; > >> + dev_set_drvdata(dev, drvdata); > >> + > >> + drvdata->base = devm_ioremap_resource(dev, &adev->res); > >> + if (!drvdata->base) > >> + return -ENOMEM; > >> + > >> + mutex_init(&drvdata->lock); > >> + > >> + desc.type = CORESIGHT_DEV_TYPE_SOURCE; > >> + desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SYS; > >> + desc.ops = &tpdm_cs_ops; > >> + desc.pdata = adev->dev.platform_data; > >> + desc.dev = &adev->dev; > >> + drvdata->csdev = coresight_register(&desc); > >> + if (IS_ERR(drvdata->csdev)) > >> + return PTR_ERR(drvdata->csdev); > >> + > >> + tpdm_init_default_data(drvdata); > >> + pm_runtime_put(&adev->dev); > >> + > >> + return 0; > >> +} > >> + > >> +static void __exit tpdm_remove(struct amba_device *adev) > >> +{ > >> + struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev); > >> + > >> + coresight_unregister(drvdata->csdev); > >> +} > >> + > >> +static struct amba_id tpdm_ids[] = { > >> + { > >> + .id = 0x000f0e00, > >> + .mask = 0x000fff00, > >> + }, > >> + { 0, 0}, > >> +}; > >> + > >> +static struct amba_driver tpdm_driver = { > >> + .drv = { > >> + .name = "coresight-tpdm", > >> + .owner = THIS_MODULE, > >> + .suppress_bind_attrs = true, > >> + }, > >> + .probe = tpdm_probe, > >> + .id_table = tpdm_ids, > >> +}; > >> + > >> +module_amba_driver(tpdm_driver); > >> + > >> +MODULE_LICENSE("GPL v2"); > >> +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Monitor driver"); > >> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h > >> new file mode 100644 > >> index 000000000000..980ae90ff1c8 > >> --- /dev/null > >> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h > >> @@ -0,0 +1,31 @@ > >> +/* SPDX-License-Identifier: GPL-2.0 */ > >> +/* > >> + * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved. > >> + */ > >> + > >> +#ifndef _CORESIGHT_CORESIGHT_TPDM_H > >> +#define _CORESIGHT_CORESIGHT_TPDM_H > >> + > >> +/* Default value of the traceid */ > >> +#define TPDM_TRACE_ID_START 128 > >> + > >> +/** > >> + * struct tpdm_drvdata - specifics associated to an TPDM component > >> + * @base: memory mapped base address for this component. > >> + * @dev: The device entity associated to this component. > >> + * @csdev: component vitals needed by the framework. > >> + * @lock: lock for the enable value. > >> + * @enable: enable status of the component. > >> + * @traceid: value of the current ID for this component. > >> + */ > >> + > >> +struct tpdm_drvdata { > >> + void __iomem *base; > >> + struct device *dev; > >> + struct coresight_device *csdev; > >> + struct mutex lock; > >> + bool enable; > >> + int traceid; > >> +}; > >> + > >> +#endif /* _CORESIGHT_CORESIGHT_TPDM_H */ > >> diff --git a/include/linux/coresight.h b/include/linux/coresight.h > >> index 93a2922b7653..e48d463be63b 100644 > >> --- a/include/linux/coresight.h > >> +++ b/include/linux/coresight.h > >> @@ -65,6 +65,7 @@ enum coresight_dev_subtype_source { > >> CORESIGHT_DEV_SUBTYPE_SOURCE_PROC, > >> CORESIGHT_DEV_SUBTYPE_SOURCE_BUS, > >> CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE, > >> + CORESIGHT_DEV_SUBTYPE_SOURCE_SYS, > >> }; > >> > >> enum coresight_dev_subtype_helper { > >> -- > >> 2.17.1 > >> -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

3 years, 6 months

Re: [PATCH v2 6/7] audit: Use task_is_in_init_pid_ns()

by Leo Yan

On Wed, Dec 15, 2021 at 02:09:12PM -0500, Richard Guy Briggs wrote: > On 2021-12-14 17:35, Paul Moore wrote: > > On Wed, Dec 8, 2021 at 3:33 AM Leo Yan <leo.yan(a)linaro.org> wrote: > > > > > > Replace open code with task_is_in_init_pid_ns() for checking root PID > > > namespace. > > > > > > Signed-off-by: Leo Yan <leo.yan(a)linaro.org> > > > --- > > > kernel/audit.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > I'm not sure how necessary this is, but it looks correct to me. > > I had the same thought. I looks correct to me. I could see the value > if it permitted init_pid_ns to not be global. Just for a background info, we need to check root PID namespace in some drivers [1], to avoid introducing more open codes, we decided to refactor with helper task_is_in_init_pid_ns(). [1] https://lore.kernel.org/lkml/20211213121323.1887180-1-leo.yan@linaro.org/ > > Acked-by: Paul Moore <paul(a)paul-moore.com> > > Reviewed-by: Richard Guy Briggs <rgb(a)redhat.com> Thanks for review, Paul and Richard. Leo

3 years, 6 months

[PATCH 01/12] perf test: Shell - Limit to only run executable scripts in tests

by carsten.haitzler＠foss.arm.com

From: Carsten Haitzler <carsten.haitzler(a)arm.com> Perf test's shell runner will just run everything in the tests directory (as long as it's not another directory or does not begin with a dot), but sometimes you find files in there that are not shell scripts - perf.data output for example if you do some testing and then the next time you run perf test it tries to run these. Check the files are executable so they are actually intended to be test scripts and not just some "random junk" files there. Signed-off-by: Carsten Haitzler <carsten.haitzler(a)arm.com> --- tools/perf/tests/builtin-test.c | 2 +- tools/perf/util/path.c | 12 ++++++++++++ tools/perf/util/path.h | 1 + 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index c4b888f18e9c..1a7e21e5acf1 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -512,7 +512,7 @@ static const char *shell_test__description(char *description, size_t size, #define for_each_shell_test(dir, base, ent) \ while ((ent = readdir(dir)) != NULL) \ - if (!is_directory(base, ent) && ent->d_name[0] != '.') + if (!is_directory(base, ent) && is_executable_file(base, ent) && ent->d_name[0] != '.') static const char *shell_tests__dir(char *path, size_t size) { diff --git a/tools/perf/util/path.c b/tools/perf/util/path.c index caed0336429f..7dde8c230ae8 100644 --- a/tools/perf/util/path.c +++ b/tools/perf/util/path.c @@ -92,3 +92,15 @@ bool is_directory(const char *base_path, const struct dirent *dent) return S_ISDIR(st.st_mode); } + +bool is_executable_file(const char *base_path, const struct dirent *dent) +{ + char path[PATH_MAX]; + struct stat st; + + sprintf(path, "%s/%s", base_path, dent->d_name); + if (stat(path, &st)) + return false; + + return !S_ISDIR(st.st_mode) && (st.st_mode & S_IXUSR); +} diff --git a/tools/perf/util/path.h b/tools/perf/util/path.h index 083429b7efa3..d94902c22222 100644 --- a/tools/perf/util/path.h +++ b/tools/perf/util/path.h @@ -12,5 +12,6 @@ int path__join3(char *bf, size_t size, const char *path1, const char *path2, con bool is_regular_file(const char *file); bool is_directory(const char *base_path, const struct dirent *dent); +bool is_executable_file(const char *base_path, const struct dirent *dent); #endif /* _PERF_PATH_H */ -- 2.32.0

3 years, 6 months

Re: [PATCH v2 2/9] Coresight: Add coresight TPDM source driver

by Mathieu Poirier

On Thu, Dec 09, 2021 at 10:15:36PM +0800, Mao Jinlong wrote: > Add driver to support Coresight device TPDM (Trace, Profiling and > Diagnostics Monitor). TPDM is a monitor to collect data from > different datasets. This change is to add probe/enable/disable > functions for tpdm source. > > Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> > Signed-off-by: Mao Jinlong <quic_jinlmao(a)quicinc.com> > --- > MAINTAINERS | 8 + > drivers/hwtracing/coresight/Kconfig | 13 ++ > drivers/hwtracing/coresight/Makefile | 1 + > drivers/hwtracing/coresight/coresight-core.c | 3 +- > drivers/hwtracing/coresight/coresight-tpdm.c | 152 +++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tpdm.h | 31 ++++ > include/linux/coresight.h | 1 + > 7 files changed, 208 insertions(+), 1 deletion(-) > create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c > create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7a2345ce8521..59f39b3194f6 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15560,6 +15560,14 @@ L: netdev(a)vger.kernel.org > S: Supported > F: drivers/net/ipa/ > > +QCOM CORESIGHT COMPONENTS DRIVER > +M: Tao Zhang <quic_taozha(a)quicinc.com> > +M: Jinlong Mao <quic_jinlmao(a)quicinc.com> > +M: Mathieu Poirier <mathieu.poirier(a)linaro.org> > +M: Suzuki K Poulose <suzuki.poulose(a)arm.com> > +S: Maintained > +F: drivers/hwtracing/coresight/coresight-tpdm.c > + There is no need for an extra entry in the MAINTAINERS file. The checkpatch.pl script is smart enough to know when to CC you and Tao every time the TPDM/TPDA drivers are modified. Suzuki and I will simply wait for you guys to add your RB tags before reviewing the patches. I have explained this in the previous revision. > QEMU MACHINE EMULATOR AND VIRTUALIZER SUPPORT > M: Gabriel Somlo <somlo(a)cmu.edu> > M: "Michael S. Tsirkin" <mst(a)redhat.com> > diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig > index 514a9b8086e3..5c506a1cd08f 100644 > --- a/drivers/hwtracing/coresight/Kconfig > +++ b/drivers/hwtracing/coresight/Kconfig > @@ -201,4 +201,17 @@ config CORESIGHT_TRBE > > To compile this driver as a module, choose M here: the module will be > called coresight-trbe. > + > +config CORESIGHT_TPDM > + tristate "CoreSight Trace, Profiling & Diagnostics Monitor driver" > + select CORESIGHT_LINKS_AND_SINKS Is this available on 32bit HW as well? If not please make it dependent on ARM64 as it for ETMv4x devices. > + help > + This driver provides support for configuring monitor. Monitors are > + primarily responsible for data set collection and support the > + ability to collect any permutation of data set types. Monitors are > + also responsible for interaction with system cross triggering. > + > + To compile this driver as a module, choose M here: the module will be > + called coresight-tpdm. > + > endif > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile > index b6c4a48140ec..e7392a0dddeb 100644 > --- a/drivers/hwtracing/coresight/Makefile > +++ b/drivers/hwtracing/coresight/Makefile > @@ -25,5 +25,6 @@ obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o > obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o > obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o > obj-$(CONFIG_CORESIGHT_TRBE) += coresight-trbe.o > +obj-$(CONFIG_CORESIGHT_TPDM) += coresight-tpdm.o > coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ > coresight-cti-sysfs.o > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index cc6b6cabf85f..a7f1a6f09cfb 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -1071,7 +1071,8 @@ static int coresight_validate_source(struct coresight_device *csdev, > } > > if (subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_PROC && > - subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE) { > + subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE && > + subtype != CORESIGHT_DEV_SUBTYPE_SOURCE_SYS) { > dev_err(&csdev->dev, "wrong device subtype in %s\n", function); > return -EINVAL; > } > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c > new file mode 100644 > index 000000000000..f494cef4fb24 > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-tpdm.c > @@ -0,0 +1,152 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/amba/bus.h> > +#include <linux/bitmap.h> > +#include <linux/coresight.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/fs.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/regulator/consumer.h> > + > +#include "coresight-priv.h" > +#include "coresight-tpdm.h" > + > +DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm"); > + > +static int tpdm_enable(struct coresight_device *csdev, > + struct perf_event *event, u32 mode) > +{ > + struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + mutex_lock(&drvdata->lock); > + if (drvdata->enable) { > + mutex_unlock(&drvdata->lock); > + return -EBUSY; > + } > + > + drvdata->enable = true; > + mutex_unlock(&drvdata->lock); > + > + dev_info(drvdata->dev, "TPDM tracing enabled\n"); > + return 0; > +} > + > +static void tpdm_disable(struct coresight_device *csdev, > + struct perf_event *event) > +{ > + struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + mutex_lock(&drvdata->lock); > + if (!drvdata->enable) { > + mutex_unlock(&drvdata->lock); > + return; > + } > + > + drvdata->enable = false; > + mutex_unlock(&drvdata->lock); > + > + dev_info(drvdata->dev, "TPDM tracing disabled\n"); > +} > + > +static int tpdm_trace_id(struct coresight_device *csdev) > +{ > + struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + return drvdata->traceid; > +} > + > +static const struct coresight_ops_source tpdm_source_ops = { > + .trace_id = tpdm_trace_id, > + .enable = tpdm_enable, > + .disable = tpdm_disable, > +}; > + > +static const struct coresight_ops tpdm_cs_ops = { > + .source_ops = &tpdm_source_ops, > +}; > + > +static void tpdm_init_default_data(struct tpdm_drvdata *drvdata) > +{ > + static int traceid = TPDM_TRACE_ID_START; > + > + drvdata->traceid = traceid++; > +} I have been specific on how to properly do this in the last revision. Given the above about the MAINTAINERS file, I am not sure that I will continue reviewing this set. There is also no need to rush another revision as I won't have the bandwidth to process it before the holidays. Thanks, Mathieu > + > +static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) > +{ > + struct device *dev = &adev->dev; > + struct coresight_platform_data *pdata; > + struct tpdm_drvdata *drvdata; > + struct coresight_desc desc = { 0 }; > + > + desc.name = coresight_alloc_device_name(&tpdm_devs, dev); > + if (!desc.name) > + return -ENOMEM; > + pdata = coresight_get_platform_data(dev); > + if (IS_ERR(pdata)) > + return PTR_ERR(pdata); > + adev->dev.platform_data = pdata; > + > + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > + if (!drvdata) > + return -ENOMEM; > + drvdata->dev = &adev->dev; > + dev_set_drvdata(dev, drvdata); > + > + drvdata->base = devm_ioremap_resource(dev, &adev->res); > + if (!drvdata->base) > + return -ENOMEM; > + > + mutex_init(&drvdata->lock); > + > + desc.type = CORESIGHT_DEV_TYPE_SOURCE; > + desc.subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SYS; > + desc.ops = &tpdm_cs_ops; > + desc.pdata = adev->dev.platform_data; > + desc.dev = &adev->dev; > + drvdata->csdev = coresight_register(&desc); > + if (IS_ERR(drvdata->csdev)) > + return PTR_ERR(drvdata->csdev); > + > + tpdm_init_default_data(drvdata); > + pm_runtime_put(&adev->dev); > + > + return 0; > +} > + > +static void __exit tpdm_remove(struct amba_device *adev) > +{ > + struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev); > + > + coresight_unregister(drvdata->csdev); > +} > + > +static struct amba_id tpdm_ids[] = { > + { > + .id = 0x000f0e00, > + .mask = 0x000fff00, > + }, > + { 0, 0}, > +}; > + > +static struct amba_driver tpdm_driver = { > + .drv = { > + .name = "coresight-tpdm", > + .owner = THIS_MODULE, > + .suppress_bind_attrs = true, > + }, > + .probe = tpdm_probe, > + .id_table = tpdm_ids, > +}; > + > +module_amba_driver(tpdm_driver); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("Trace, Profiling & Diagnostic Monitor driver"); > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h > new file mode 100644 > index 000000000000..980ae90ff1c8 > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-tpdm.h > @@ -0,0 +1,31 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2021 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#ifndef _CORESIGHT_CORESIGHT_TPDM_H > +#define _CORESIGHT_CORESIGHT_TPDM_H > + > +/* Default value of the traceid */ > +#define TPDM_TRACE_ID_START 128 > + > +/** > + * struct tpdm_drvdata - specifics associated to an TPDM component > + * @base: memory mapped base address for this component. > + * @dev: The device entity associated to this component. > + * @csdev: component vitals needed by the framework. > + * @lock: lock for the enable value. > + * @enable: enable status of the component. > + * @traceid: value of the current ID for this component. > + */ > + > +struct tpdm_drvdata { > + void __iomem *base; > + struct device *dev; > + struct coresight_device *csdev; > + struct mutex lock; > + bool enable; > + int traceid; > +}; > + > +#endif /* _CORESIGHT_CORESIGHT_TPDM_H */ > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index 93a2922b7653..e48d463be63b 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -65,6 +65,7 @@ enum coresight_dev_subtype_source { > CORESIGHT_DEV_SUBTYPE_SOURCE_PROC, > CORESIGHT_DEV_SUBTYPE_SOURCE_BUS, > CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE, > + CORESIGHT_DEV_SUBTYPE_SOURCE_SYS, > }; > > enum coresight_dev_subtype_helper { > -- > 2.17.1 >

3 years, 6 months

Re: [PATCH v2 1/9] Use IDR to maintain all the enabled sources' paths.

by Mathieu Poirier

Hi Mao, On Thu, Dec 09, 2021 at 10:15:35PM +0800, Mao Jinlong wrote: > Use hash length of the source's device name to map to the pointer > of the enabled path. Using IDR will be more efficient than using > the list. And there could be other sources except STM and CPU etms > in the new HWs. It is better to maintain all the paths together. > > Signed-off-by: Mao Jinlong <quic_jinlmao(a)quicinc.com> > --- > drivers/hwtracing/coresight/coresight-core.c | 76 +++++++------------- > 1 file changed, 26 insertions(+), 50 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index 8a18c71df37a..cc6b6cabf85f 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -7,6 +7,7 @@ > #include <linux/init.h> > #include <linux/types.h> > #include <linux/device.h> > +#include <linux/idr.h> > #include <linux/io.h> > #include <linux/err.h> > #include <linux/export.h> > @@ -26,6 +27,12 @@ > static DEFINE_MUTEX(coresight_mutex); > static DEFINE_PER_CPU(struct coresight_device *, csdev_sink); > > +/* > + * Use IDR to map the hash length of the source's device name > + * to the pointer of path for the source > + */ > +static DEFINE_IDR(path_idr); > + > /** > * struct coresight_node - elements of a path, from source to sink > * @csdev: Address of an element. > @@ -36,20 +43,6 @@ struct coresight_node { > struct list_head link; > }; > > -/* > - * When operating Coresight drivers from the sysFS interface, only a single > - * path can exist from a tracer (associated to a CPU) to a sink. > - */ > -static DEFINE_PER_CPU(struct list_head *, tracer_path); > - > -/* > - * As of this writing only a single STM can be found in CS topologies. Since > - * there is no way to know if we'll ever see more and what kind of > - * configuration they will enact, for the time being only define a single path > - * for STM. > - */ > -static struct list_head *stm_path; > - > /* > * When losing synchronisation a new barrier packet needs to be inserted at the > * beginning of the data collected in a buffer. That way the decoder knows that > @@ -1088,10 +1081,11 @@ static int coresight_validate_source(struct coresight_device *csdev, > > int coresight_enable(struct coresight_device *csdev) > { > - int cpu, ret = 0; > + int ret = 0; > struct coresight_device *sink; > struct list_head *path; > enum coresight_dev_subtype_source subtype; > + u32 hash; > > subtype = csdev->subtype.source_subtype; > > @@ -1133,26 +1127,14 @@ int coresight_enable(struct coresight_device *csdev) > if (ret) > goto err_source; > > - switch (subtype) { > - case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC: > - /* > - * When working from sysFS it is important to keep track > - * of the paths that were created so that they can be > - * undone in 'coresight_disable()'. Since there can only > - * be a single session per tracer (when working from sysFS) > - * a per-cpu variable will do just fine. > - */ > - cpu = source_ops(csdev)->cpu_id(csdev); > - per_cpu(tracer_path, cpu) = path; > - break; > - case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE: > - stm_path = path; > - break; > - default: > - /* We can't be here */ > - break; > - } > - > + /* > + * Use the hash length of source's device name as ID > + * and map the ID to the pointer of the path. > + */ > + hash = hashlen_hash(hashlen_string(NULL, dev_name(&csdev->dev))); > + ret = idr_alloc_u32(&path_idr, path, &hash, hash, GFP_KERNEL); > + if (ret) > + goto err_source; > out: > mutex_unlock(&coresight_mutex); > return ret; > @@ -1168,8 +1150,9 @@ EXPORT_SYMBOL_GPL(coresight_enable); > > void coresight_disable(struct coresight_device *csdev) > { > - int cpu, ret; > + int ret; > struct list_head *path = NULL; > + u32 hash; > > mutex_lock(&coresight_mutex); > > @@ -1180,21 +1163,13 @@ void coresight_disable(struct coresight_device *csdev) > if (!csdev->enable || !coresight_disable_source(csdev)) > goto out; > > - switch (csdev->subtype.source_subtype) { > - case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC: > - cpu = source_ops(csdev)->cpu_id(csdev); > - path = per_cpu(tracer_path, cpu); > - per_cpu(tracer_path, cpu) = NULL; > - break; > - case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE: > - path = stm_path; > - stm_path = NULL; > - break; > - default: > - /* We can't be here */ > - break; > - } > + hash = hashlen_hash(hashlen_string(NULL, dev_name(&csdev->dev))); > + /* Find the path by the hash length. */ > + path = idr_find(&path_idr, hash); > + if (path == NULL) > + return; Please add a dev_err() here as this should really not be happening. > > + idr_remove(&path_idr, hash); > coresight_disable_path(path); > coresight_release_path(path); > > @@ -1779,6 +1754,7 @@ static int __init coresight_init(void) > > static void __exit coresight_exit(void) > { > + idr_destroy(&path_idr); As far as I can tell this isn't needed. > cscfg_exit(); > etm_perf_exit(); > bus_unregister(&coresight_bustype); > -- > 2.17.1 >

3 years, 6 months

[PATCH v2 0/4] coresight: etm: Correct PID tracing for non-root namespace

by Leo Yan

If a profiling program runs in a non-root PID namespace, if CoreSight driver enables PID tracing (with contextID), it can lead to mismatching issue between the context ID traced in hardware (from the root namespace) and the PIDs gathered by profiling tool (e.g. perf) in its non-root namespace. CoreSight driver has tried to address this issue for the contextID related interfaces under sysfs, but it misses to prevent user to set VMID (virtual contextID) for kernel runs in EL2 with VHE; furthermore, it misses to handle the case when the profiling tool runs in the non-root PID namespace. For this reason, this patch series is to correct contextID tracing for non-root namespace. After applied this patchset, patch 02 doesn't permit users to access virtual contextID via sysfs nodes in the non-root PID namespace, patch 03 and 04 stop to trace PID packet for non-root PID namespace. This patch is dependent on the patchset "pid: Introduce helper task_is_in_root_ns()" [1]. [1] https://lore.kernel.org/lkml/20211208083320.472503-1-leo.yan@linaro.org/ Leo Yan (4): coresight: etm4x: Add lock for reading virtual context ID comparator coresight: etm4x: Don't use virtual contextID for non-root PID namespace coresight: etm4x: Don't trace PID for non-root PID namespace coresight: etm3x: Don't trace PID for non-root PID namespace .../coresight/coresight-etm3x-core.c | 4 +++ .../coresight/coresight-etm4x-core.c | 10 +++++-- .../coresight/coresight-etm4x-sysfs.c | 30 +++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) -- 2.25.1

3 years, 6 months

Re: [PATCH v2 4/7] connector/cn_proc: Use task_is_in_init_pid_ns()

by Leo Yan

On Tue, Dec 14, 2021 at 04:58:32PM +1100, Balbir Singh wrote: > On Wed, Dec 08, 2021 at 04:33:17PM +0800, Leo Yan wrote: > > This patch replaces open code with task_is_in_init_pid_ns() to check if > > a task is in root PID namespace. > > > > Signed-off-by: Leo Yan <leo.yan(a)linaro.org> > > --- > > drivers/connector/cn_proc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c > > index 646ad385e490..ccac1c453080 100644 > > --- a/drivers/connector/cn_proc.c > > +++ b/drivers/connector/cn_proc.c > > @@ -358,7 +358,7 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg, > > * other namespaces. > > */ > > if ((current_user_ns() != &init_user_ns) || > > - (task_active_pid_ns(current) != &init_pid_ns)) > > + !task_is_in_init_pid_ns(current)) > > return; > > > > Sounds like there might scope for other wrappers - is_current_in_user_init_ns() Indeed, for new wrapper is_current_in_user_init_ns(), I searched kernel and located there have multiple places use open code. If no one works on this refactoring, I will send a new patchset for it separately. > Acked-by: Balbir Singh <bsingharora(a)gmail.com> Thank you for review, Balbir! Also thanks review from Leon and Suzuki. Leo

3 years, 6 months

Re: [PATCH v2 4/4] coresight: etm3x: Don't trace PID for non-root PID namespace

by Leo Yan

Hi, On Tue, Dec 14, 2021 at 03:16:42AM +0800, kernel test robot wrote: > Hi Leo, > > I love your patch! Yet something to improve: > > [auto build test ERROR on linus/master] > [also build test ERROR on v5.16-rc5] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: https://github.com/0day-ci/linux/commits/Leo-Yan/coresight-etm-Correct-PID-… > base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2585cf9dfaaddf00b069673f27bb3f8530e2039c > config: arm-buildonly-randconfig-r003-20211213 (https://download.01.org/0day-ci/archive/20211214/202112140344.viPmOWp6-lkp@…) > compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b6a2ddb6c8ac29412b1361810972e15221fa021c) > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # install arm cross compiling tool for clang build > # apt-get install binutils-arm-linux-gnueabi > # https://github.com/0day-ci/linux/commit/81d5f47788c40d34c8159d64d4505eb4852… > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Leo-Yan/coresight-etm-Correct-PID-tracing-for-non-root-namespace/20211213-201632 > git checkout 81d5f47788c40d34c8159d64d4505eb485254e8f > # save the config file to linux build tree > mkdir build_dir > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/hwtracing/coresight/ > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp(a)intel.com> > > All errors (new ones prefixed by >>): > > >> drivers/hwtracing/coresight/coresight-etm3x-core.c:344:7: error: implicit declaration of function 'task_is_in_init_pid_ns' [-Werror,-Wimplicit-function-declaration] > if (!task_is_in_init_pid_ns(current)) > ^ > 1 error generated. > > > vim +/task_is_in_init_pid_ns +344 drivers/hwtracing/coresight/coresight-etm3x-core.c > > 301 > 302 #define ETM3X_SUPPORTED_OPTIONS (ETMCR_CYC_ACC | \ > 303 ETMCR_TIMESTAMP_EN | \ > 304 ETMCR_RETURN_STACK) > 305 > 306 static int etm_parse_event_config(struct etm_drvdata *drvdata, > 307 struct perf_event *event) > 308 { > 309 struct etm_config *config = &drvdata->config; > 310 struct perf_event_attr *attr = &event->attr; > 311 > 312 if (!attr) > 313 return -EINVAL; > 314 > 315 /* Clear configuration from previous run */ > 316 memset(config, 0, sizeof(struct etm_config)); > 317 > 318 if (attr->exclude_kernel) > 319 config->mode = ETM_MODE_EXCL_KERN; > 320 > 321 if (attr->exclude_user) > 322 config->mode = ETM_MODE_EXCL_USER; > 323 > 324 /* Always start from the default config */ > 325 etm_set_default(config); > 326 > 327 /* > 328 * By default the tracers are configured to trace the whole address > 329 * range. Narrow the field only if requested by user space. > 330 */ > 331 if (config->mode) > 332 etm_config_trace_mode(config); > 333 > 334 /* > 335 * At this time only cycle accurate, return stack and timestamp > 336 * options are available. > 337 */ > 338 if (attr->config & ~ETM3X_SUPPORTED_OPTIONS) > 339 return -EINVAL; > 340 > 341 config->ctrl = attr->config; > 342 > 343 /* Don't trace contextID when runs in non-root PID namespace */ > > 344 if (!task_is_in_init_pid_ns(current)) This patchset is based on another patchset [1], as described on the cover letter patch. This is why here reports for building failure. To avoid the false positive reporting, if any better practice I can follow up to resolve the dependency between two patchsets, please let me know and I will do in next time. Thanks, Leo [1] https://lore.kernel.org/lkml/20211208083320.472503-1-leo.yan@linaro.org/ > 345 config->ctrl &= ~ETMCR_CTXID_SIZE; > 346 > 347 /* > 348 * Possible to have cores with PTM (supports ret stack) and ETM > 349 * (never has ret stack) on the same SoC. So if we have a request > 350 * for return stack that can't be honoured on this core then > 351 * clear the bit - trace will still continue normally > 352 */ > 353 if ((config->ctrl & ETMCR_RETURN_STACK) && > 354 !(drvdata->etmccer & ETMCCER_RETSTACK)) > 355 config->ctrl &= ~ETMCR_RETURN_STACK; > 356 > 357 return 0; > 358 } > 359 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

3 years, 6 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight