CoreSight

coresight@lists.linaro.org

5 participants
2517 discussions

Re: [PATCH v6 3/3] coresight: prevent deactivate active config while enabling the config

by Leo Yan

On Wed, May 14, 2025 at 05:19:51PM +0100, Yeoreum Yun wrote: > While enable active config via cscfg_csdev_enable_active_config(), > active config could be deactivated via configfs' sysfs interface. > This could make UAF issue in below scenario: > > CPU0 CPU1 > (sysfs enable) load module > cscfg_load_config_sets() > activate config. // sysfs > (sys_active_cnt == 1) > ... > cscfg_csdev_enable_active_config() > lock(csdev->cscfg_csdev_lock) > // here load config activate by CPU1 > unlock(csdev->cscfg_csdev_lock) > > deactivate config // sysfs > (sys_activec_cnt == 0) > cscfg_unload_config_sets() > unload module > > // access to config_desc which freed > // while unloading module. > cscfg_csdev_enable_config > > To address this, use cscfg_config_desc's active_cnt as a reference count > which will be holded when > - activate the config. > - enable the activated config. > and put the module reference when config_active_cnt == 0. > > Fixes: f8cce2ff3c04 ("coresight: syscfg: Add API to activate and enable configurations") > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> Reviewed-by: Leo Yan <leo.yan(a)arm.com> > --- > .../hwtracing/coresight/coresight-config.h | 2 +- > .../hwtracing/coresight/coresight-syscfg.c | 49 +++++++++++++------ > 2 files changed, 35 insertions(+), 16 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h > index b9ebc9fcfb7f..90fd937d3bd8 100644 > --- a/drivers/hwtracing/coresight/coresight-config.h > +++ b/drivers/hwtracing/coresight/coresight-config.h > @@ -228,7 +228,7 @@ struct cscfg_feature_csdev { > * @feats_csdev:references to the device features to enable. > */ > struct cscfg_config_csdev { > - const struct cscfg_config_desc *config_desc; > + struct cscfg_config_desc *config_desc; > struct coresight_device *csdev; > bool enabled; > struct list_head node; > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c > index 23017612f2ea..83dad24e0116 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg.c > +++ b/drivers/hwtracing/coresight/coresight-syscfg.c > @@ -869,6 +869,25 @@ void cscfg_csdev_reset_feats(struct coresight_device *csdev) > } > EXPORT_SYMBOL_GPL(cscfg_csdev_reset_feats); > > +static bool cscfg_config_desc_get(struct cscfg_config_desc *config_desc) > +{ > + if (!atomic_fetch_inc(&config_desc->active_cnt)) { > + /* must ensure that config cannot be unloaded in use */ > + if (unlikely(cscfg_owner_get(config_desc->load_owner))) { > + atomic_dec(&config_desc->active_cnt); > + return false; > + } > + } > + > + return true; > +} > + > +static void cscfg_config_desc_put(struct cscfg_config_desc *config_desc) > +{ > + if (!atomic_dec_return(&config_desc->active_cnt)) > + cscfg_owner_put(config_desc->load_owner); > +} > + > /* > * This activate configuration for either perf or sysfs. Perf can have multiple > * active configs, selected per event, sysfs is limited to one. > @@ -892,22 +911,17 @@ static int _cscfg_activate_config(unsigned long cfg_hash) > if (config_desc->available == false) > return -EBUSY; > > - /* must ensure that config cannot be unloaded in use */ > - err = cscfg_owner_get(config_desc->load_owner); > - if (err) > + if (!cscfg_config_desc_get(config_desc)) { > + err = -EINVAL; > break; > + } > + > /* > * increment the global active count - control changes to > * active configurations > */ > atomic_inc(&cscfg_mgr->sys_active_cnt); > > - /* > - * mark the descriptor as active so enable config on a > - * device instance will use it > - */ > - atomic_inc(&config_desc->active_cnt); > - > err = 0; > dev_dbg(cscfg_device(), "Activate config %s.\n", config_desc->name); > break; > @@ -922,9 +936,8 @@ static void _cscfg_deactivate_config(unsigned long cfg_hash) > > list_for_each_entry(config_desc, &cscfg_mgr->config_desc_list, item) { > if ((unsigned long)config_desc->event_ea->var == cfg_hash) { > - atomic_dec(&config_desc->active_cnt); > atomic_dec(&cscfg_mgr->sys_active_cnt); > - cscfg_owner_put(config_desc->load_owner); > + cscfg_config_desc_put(config_desc); > dev_dbg(cscfg_device(), "Deactivate config %s.\n", config_desc->name); > break; > } > @@ -1049,7 +1062,7 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev, > unsigned long cfg_hash, int preset) > { > struct cscfg_config_csdev *config_csdev_active = NULL, *config_csdev_item; > - const struct cscfg_config_desc *config_desc; > + struct cscfg_config_desc *config_desc; > unsigned long flags; > int err = 0; > > @@ -1064,8 +1077,8 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev, > raw_spin_lock_irqsave(&csdev->cscfg_csdev_lock, flags); > list_for_each_entry(config_csdev_item, &csdev->config_csdev_list, node) { > config_desc = config_csdev_item->config_desc; > - if ((atomic_read(&config_desc->active_cnt)) && > - ((unsigned long)config_desc->event_ea->var == cfg_hash)) { > + if (((unsigned long)config_desc->event_ea->var == cfg_hash) && > + cscfg_config_desc_get(config_desc)) { > config_csdev_active = config_csdev_item; > csdev->active_cscfg_ctxt = (void *)config_csdev_active; > break; > @@ -1099,7 +1112,11 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev, > err = -EBUSY; > raw_spin_unlock_irqrestore(&csdev->cscfg_csdev_lock, flags); > } > + > + if (err) > + cscfg_config_desc_put(config_desc); > } > + > return err; > } > EXPORT_SYMBOL_GPL(cscfg_csdev_enable_active_config); > @@ -1138,8 +1155,10 @@ void cscfg_csdev_disable_active_config(struct coresight_device *csdev) > raw_spin_unlock_irqrestore(&csdev->cscfg_csdev_lock, flags); > > /* true if there was an enabled active config */ > - if (config_csdev) > + if (config_csdev) { > cscfg_csdev_disable_config(config_csdev); > + cscfg_config_desc_put(config_csdev->config_desc); > + } > } > EXPORT_SYMBOL_GPL(cscfg_csdev_disable_active_config); > > -- > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7} >

5 months, 2 weeks

Re: [PATCH v5 3/3] coresight: prevent deactivate active config while enabling the config

by Leo Yan

Hi Levi, On Wed, May 14, 2025 at 12:04:39PM +0100, Yeoreum Yun wrote: [...] > > > +static bool cscfg_config_desc_get(struct cscfg_config_desc *config_desc) > > > > I would like to change the return type to int, so the error is handled > > within the function. As a result, the caller _cscfg_activate_config() > > does not need to explicitly return an error value. > > I think it's not good since cscfg_config_desc_get() failed only when > try_module_get() failed and its return type is "bool". Understood. I thought it would be easier for later maintenance if we encapsulate error handling in the function, but I don't have strong opinion. It is fine for me to return bool type. Thanks, Leo

5 months, 2 weeks

Re: [PATCH v5 3/3] coresight: prevent deactivate active config while enabling the config

by Leo Yan

On Tue, May 13, 2025 at 06:06:22PM +0100, Yeoreum Yun wrote: > While enable active config via cscfg_csdev_enable_active_config(), > active config could be deactivated via configfs' sysfs interface. > This could make UAF issue in below scenario: > > CPU0 CPU1 > (sysfs enable) load module > cscfg_load_config_sets() > activate config. // sysfs > (sys_active_cnt == 1) > ... > cscfg_csdev_enable_active_config() > lock(csdev->cscfg_csdev_lock) > // here load config activate by CPU1 > unlock(csdev->cscfg_csdev_lock) > > deactivate config // sysfs > (sys_activec_cnt == 0) > cscfg_unload_config_sets() > unload module > > // access to config_desc which freed > // while unloading module. > cscfg_csdev_enable_config > > To address this, use cscfg_config_desc's active_cnt as a reference count > which will be holded when > - activate the config. > - enable the activated config. > and put the module reference when config_active_cnt == 0. > > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > --- > .../hwtracing/coresight/coresight-config.h | 2 +- > .../hwtracing/coresight/coresight-syscfg.c | 49 +++++++++++++------ > 2 files changed, 35 insertions(+), 16 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h > index b9ebc9fcfb7f..90fd937d3bd8 100644 > --- a/drivers/hwtracing/coresight/coresight-config.h > +++ b/drivers/hwtracing/coresight/coresight-config.h > @@ -228,7 +228,7 @@ struct cscfg_feature_csdev { > * @feats_csdev:references to the device features to enable. > */ > struct cscfg_config_csdev { > - const struct cscfg_config_desc *config_desc; > + struct cscfg_config_desc *config_desc; > struct coresight_device *csdev; > bool enabled; > struct list_head node; > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c > index 5d194b9269f5..6d8c212ad434 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg.c > +++ b/drivers/hwtracing/coresight/coresight-syscfg.c > @@ -870,6 +870,25 @@ void cscfg_csdev_reset_feats(struct coresight_device *csdev) > } > EXPORT_SYMBOL_GPL(cscfg_csdev_reset_feats); > > +static bool cscfg_config_desc_get(struct cscfg_config_desc *config_desc) I would like to change the return type to int, so the error is handled within the function. As a result, the caller _cscfg_activate_config() does not need to explicitly return an error value. Otherwise, the patch looks good to me. Thanks, Leo > +{ > + if (!atomic_fetch_inc(&config_desc->active_cnt)) { > + /* must ensure that config cannot be unloaded in use */ > + if (unlikely(cscfg_owner_get(config_desc->load_owner))) { > + atomic_dec(&config_desc->active_cnt); > + return false; > + } > + } > + > + return true; > +} > + > +static void cscfg_config_desc_put(struct cscfg_config_desc *config_desc) > +{ > + if (!atomic_dec_return(&config_desc->active_cnt)) > + cscfg_owner_put(config_desc->load_owner); > +} > + > /* > * This activate configuration for either perf or sysfs. Perf can have multiple > * active configs, selected per event, sysfs is limited to one. > @@ -893,22 +912,17 @@ static int _cscfg_activate_config(unsigned long cfg_hash) > if (config_desc->available == false) > return -EBUSY; > > - /* must ensure that config cannot be unloaded in use */ > - err = cscfg_owner_get(config_desc->load_owner); > - if (err) > + if (!cscfg_config_desc_get(config_desc)) { > + err = -EINVAL; > break; > + } > + > /* > * increment the global active count - control changes to > * active configurations > */ > atomic_inc(&cscfg_mgr->sys_active_cnt); > > - /* > - * mark the descriptor as active so enable config on a > - * device instance will use it > - */ > - atomic_inc(&config_desc->active_cnt); > - > err = 0; > dev_dbg(cscfg_device(), "Activate config %s.\n", config_desc->name); > break; > @@ -923,9 +937,8 @@ static void _cscfg_deactivate_config(unsigned long cfg_hash) > > list_for_each_entry(config_desc, &cscfg_mgr->config_desc_list, item) { > if ((unsigned long)config_desc->event_ea->var == cfg_hash) { > - atomic_dec(&config_desc->active_cnt); > atomic_dec(&cscfg_mgr->sys_active_cnt); > - cscfg_owner_put(config_desc->load_owner); > + cscfg_config_desc_put(config_desc); > dev_dbg(cscfg_device(), "Deactivate config %s.\n", config_desc->name); > break; > } > @@ -1050,7 +1063,7 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev, > unsigned long cfg_hash, int preset) > { > struct cscfg_config_csdev *config_csdev_active = NULL, *config_csdev_item; > - const struct cscfg_config_desc *config_desc; > + struct cscfg_config_desc *config_desc; > unsigned long flags; > int err = 0; > > @@ -1065,8 +1078,8 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev, > raw_spin_lock_irqsave(&csdev->cscfg_csdev_lock, flags); > list_for_each_entry(config_csdev_item, &csdev->config_csdev_list, node) { > config_desc = config_csdev_item->config_desc; > - if ((atomic_read(&config_desc->active_cnt)) && > - ((unsigned long)config_desc->event_ea->var == cfg_hash)) { > + if (((unsigned long)config_desc->event_ea->var == cfg_hash) && > + cscfg_config_desc_get(config_desc)) { > config_csdev_active = config_csdev_item; > csdev->active_cscfg_ctxt = (void *)config_csdev_active; > break; > @@ -1100,7 +1113,11 @@ int cscfg_csdev_enable_active_config(struct coresight_device *csdev, > err = -EBUSY; > raw_spin_unlock_irqrestore(&csdev->cscfg_csdev_lock, flags); > } > + > + if (err) > + cscfg_config_desc_put(config_desc); > } > + > return err; > } > EXPORT_SYMBOL_GPL(cscfg_csdev_enable_active_config); > @@ -1139,8 +1156,10 @@ void cscfg_csdev_disable_active_config(struct coresight_device *csdev) > raw_spin_unlock_irqrestore(&csdev->cscfg_csdev_lock, flags); > > /* true if there was an enabled active config */ > - if (config_csdev) > + if (config_csdev) { > cscfg_csdev_disable_config(config_csdev); > + cscfg_config_desc_put(config_csdev->config_desc); > + } > } > EXPORT_SYMBOL_GPL(cscfg_csdev_disable_active_config); > > -- > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7} >

5 months, 2 weeks

Re: [PATCH v5 2/3] coresight: holding cscfg_csdev_lock while removing cscfg from csdev

by Leo Yan

On Tue, May 13, 2025 at 06:06:21PM +0100, Yeoreum Yun wrote: > There'll be possible race scenario for coresight config: > > CPU0 CPU1 > (perf enable) load module > cscfg_load_config_sets() > activate config. // sysfs > (sys_active_cnt == 1) > ... > cscfg_csdev_enable_active_config() > lock(csdev->cscfg_csdev_lock) > deactivate config // sysfs > (sys_activec_cnt == 0) > cscfg_unload_config_sets() > <iterating config_csdev_list> cscfg_remove_owned_csdev_configs() > // here load config activate by CPU1 > unlock(csdev->cscfg_csdev_lock) > > iterating config_csdev_list could be raced with config_csdev_list's > entry delete. > > To resolve this race , hold csdev->cscfg_csdev_lock() while > cscfg_remove_owned_csdev_configs() > > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> > Fixes: 02bd588e12df ("coresight: configuration: Update API to permit dynamic load/unload") > --- > drivers/hwtracing/coresight/coresight-syscfg.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c > index a70c1454b410..5d194b9269f5 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg.c > +++ b/drivers/hwtracing/coresight/coresight-syscfg.c > @@ -391,14 +391,17 @@ static void cscfg_owner_put(struct cscfg_load_owner_info *owner_info) > static void cscfg_remove_owned_csdev_configs(struct coresight_device *csdev, void *load_owner) > { > struct cscfg_config_csdev *config_csdev, *tmp; > + unsigned long flags; > > if (list_empty(&csdev->config_csdev_list)) > return; > > + raw_spin_lock_irqsave(&csdev->cscfg_csdev_lock, flags); Could we use the format: guard(raw_spinlock_irqsave)(&csdev->cscfg_csdev_lock); Sorry I did not mention this in the earlier review. Otherwise: Reviewed-by: Leo Yan <leo.yan(a)arm.com> > list_for_each_entry_safe(config_csdev, tmp, &csdev->config_csdev_list, node) { > if (config_csdev->config_desc->load_owner == load_owner) > list_del(&config_csdev->node); > } > + raw_spin_unlock_irqrestore(&csdev->cscfg_csdev_lock, flags); > } > > static void cscfg_remove_owned_csdev_features(struct coresight_device *csdev, void *load_owner) > -- > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7} >

5 months, 2 weeks

Re: [PATCH v5 1/3] coresight/etm4: fix missing disable active config

by Leo Yan

On Tue, May 13, 2025 at 06:06:20PM +0100, Yeoreum Yun wrote: > When etm4 device is disabled via sysfs, it should disable its active > count. > > Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com> With adding fix tag: Fixes: 7ebd0ec6cf94 ("coresight: configfs: Allow configfs to activate configuration") Reviewed-by: Leo Yan <leo.yan(a)arm.com> > --- > drivers/hwtracing/coresight/coresight-etm4x-core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 2b8f10463840..f008f87f0801 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -1020,6 +1020,9 @@ static void etm4_disable_sysfs(struct coresight_device *csdev) > smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1); > > raw_spin_unlock(&drvdata->spinlock); > + > + cscfg_csdev_disable_active_config(csdev); > + > cpus_read_unlock(); > > /* > -- > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7} >

5 months, 2 weeks

Re: [PATCH v4] coresight: prevent deactivate active config while enabling the config

by Leo Yan

Hi Levi, On Fri, May 02, 2025 at 11:34:17AM +0100, Yeoreum Yun wrote: [...] > > > > > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > > > > > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > > > > > @@ -1020,6 +1020,9 @@ static void etm4_disable_sysfs(struct coresight_device *csdev) > > > > > smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1); > > > > > > > > > > raw_spin_unlock(&drvdata->spinlock); > > > > > + > > > > > + cscfg_csdev_disable_active_config(csdev); > > > > > + > > > > > > > > In general, we need to split changes into several patches if each > > > > addresses a different issue. From my understanding, the change above is > > > > to fix missing to disable config when disable Sysfs mode. > > > > > > > > If so, could we use a seperate patch for this change? > > > > > > > > > > It's not a differnt issue. Without this line, the active count wouldn't > > > decrese and it raise another issue -- unloadable moudle for active_cnt :( > > > So I think it should be included in this patch. > > > > I read the code again and concluded the change above is not related to > > locking and would be a separate issue: when we close a Sysfs session, > > we need to disable a config on a CoreSight device. > > Could you clarify what is meaning "unloadable moudle for active_cnt"? > > I only saw "cscfg_mgr->sys_active_cnt" is used for module unloading, > > but have no clue why "active_cnt" impacts module unloading. > > Yes. but it also related "by this patch". > When the load config and "activate" configuration via sysfs, > not only the cscfg_mgr->sys_active_cnt is increase but also > "individual cscfg->active_cnt". > This patch extends the meaning of "cscfg->active_cnt" includes > "enable of configuraiton". so that cscfg_config_desc_put() prevent > decrease "module reference" while holding individual active_cnt. > That's why without this change, the "module reference" couldn't be > decreased. The problem before this change is deactivation doesn't > chekc cscfg->active_cnt but put module reference whenever it calls. Thanks for explanation and it makes sense to me. As we discussed, given this patch is relative big, let us divide into three small patches for easier review. - The first patch is to address that the sysfs interface misses to call cscfg_csdev_disable_active_config() for disabling config. - The second patch fixes the locking issue for "config_csdev_list". As the "config_csdev_list" is protected by cscfg_csdev_lock, the cscfg_remove_owned_csdev_configs() function should use lock for exclusive operations. - The third patch is to fix reference counter of a config module. The patch adds cscfg_config_desc_get() and cscfg_config_desc_put() in the config enabling / disabling flow for acquiring module reference counter. Thanks, Leo

5 months, 3 weeks

[PATCH v4 0/7] Arm CoreSight: Support AUX pause and resume

by Leo Yan

This series is to enable AUX pause and resume on Arm CoreSight. The first patch extracts the trace unit controlling operations to two functions. These two functions will be used by AUX pause and resume. Patches 02 and 03 change the ETMv4 driver to prepare callback functions for AUX pause and resume. Patch 04 changes the ETM perf layer to support AUX pause and resume in a perf session. The patch 05 re-enables sinks after buffer update, based on it, the patch 06 updates buffer on AUX pause occasion, which can mitigate the trace data lose issue. Patch 07 documents the AUX pause usages with Arm CoreSight. This patch set has been verified on the Hikey960 board. It is suggested to disable CPUIdle (add `nohlt` option in Linux command line) when verifying this series. ETM and funnel drivers are found issues during CPU suspend and resume which will be addressed separately. Changes from v3: - Re-enabled sink in buffer update callbacks (Suzuki). Changes from v2: - Rebased on CoreSight next branch. - Dropped the uAPI 'update_buf_on_pause' and updated document respectively (Suzuki). - Renamed ETM callbacks to .pause_perf() and .resume_perf() (Suzuki). - Minor improvement for error handling in the AUX resume flow. Changes from v1: - Added validation function pointers in pause and resume APIs (Mike). Leo Yan (7): coresight: etm4x: Extract the trace unit controlling coresight: Introduce pause and resume APIs for source coresight: etm4x: Hook pause and resume callbacks coresight: perf: Support AUX trace pause and resume coresight: tmc: Re-enable sink after buffer update coresight: perf: Update buffer on AUX pause Documentation: coresight: Document AUX pause and resume Documentation/trace/coresight/coresight-perf.rst | 31 +++++++++ drivers/hwtracing/coresight/coresight-core.c | 22 +++++++ drivers/hwtracing/coresight/coresight-etm-perf.c | 84 +++++++++++++++++++++++- drivers/hwtracing/coresight/coresight-etm4x-core.c | 143 +++++++++++++++++++++++++++++------------ drivers/hwtracing/coresight/coresight-etm4x.h | 2 + drivers/hwtracing/coresight/coresight-priv.h | 2 + drivers/hwtracing/coresight/coresight-tmc-etf.c | 9 +++ drivers/hwtracing/coresight/coresight-tmc-etr.c | 10 +++ include/linux/coresight.h | 4 ++ 9 files changed, 265 insertions(+), 42 deletions(-) -- 2.34.1

5 months, 3 weeks

[PATCH v5 0/2] coresight: Add Coresight Trace Network On Chip driver

by Yuanfang Zhang

The Trace Network On Chip (TNOC) is an integration hierarchy which is a hardware component that integrates the functionalities of TPDA and funnels. It collects trace from subsystems and transfers it to coresight sink. In addition to the generic TNOC mentioned above, there is also a special type of TNOC called Interconnect TNOC. Unlike the generic TNOC, the Interconnect TNOC doesn't need ATID. Its primary function is to connect the source of subsystems to the Aggregator TNOC. Its driver is different from this patch and will describe it and upstream its driver separately. Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com> --- Changes in v5: - update cover-letter to describe the Interconnect TNOC. - Link to v4: https://lore.kernel.org/r/20250415-trace-noc-v4-0-979938fedfd8@quicinc.com Changes in v4: - Fix dt_binding warning. - update mask of trace_noc amba_id. - Modify driver comments. - rename TRACE_NOC_SYN_VAL to TRACE_NOC_SYNC_INTERVAL. - Link to v3: https://lore.kernel.org/r/20250411-trace-noc-v3-0-1f19ddf7699b@quicinc.com Changes in v3: - Remove unnecessary sysfs nodes. - update commit messages. - Use 'writel' instead of 'write_relaxed' when writing to the register for the last time. - Add trace_id ops. - Link to v2: https://lore.kernel.org/r/20250226-trace-noc-driver-v2-0-8afc6584afc5@quici… Changes in v2: - Modified the format of DT binging file. - Fix compile warnings. - Link to v1: https://lore.kernel.org/r/46643089-b88d-49dc-be05-7bf0bb21f847@quicinc.com --- Yuanfang Zhang (2): dt-bindings: arm: Add device Trace Network On Chip definition coresight: add coresight Trace Network On Chip driver .../bindings/arm/qcom,coresight-tnoc.yaml | 111 ++++++++++++ drivers/hwtracing/coresight/Kconfig | 13 ++ drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/coresight-tnoc.c | 191 +++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tnoc.h | 34 ++++ 5 files changed, 350 insertions(+) --- base-commit: a2cc6ff5ec8f91bc463fd3b0c26b61166a07eb11 change-id: 20250403-trace-noc-f8286b30408e Best regards, -- Yuanfang Zhang <quic_yuanfang(a)quicinc.com>

5 months, 3 weeks

Re: [PATCH v1 1/1] coresight: cti: Replace inclusion by struct fwnode_handle forward declaration

by Suzuki K Poulose

On Mon, 31 Mar 2025 10:14:53 +0300, Andy Shevchenko wrote: > The fwnode.h is not supposed to be used by the drivers as it > has the definitions for the core parts for different device > property provider implementations. Drop it. > > Since the code wants to use the pointer to the struct fwnode_handle > the forward declaration is provided. > > [...] Applied, thanks! [1/1] coresight: cti: Replace inclusion by struct fwnode_handle forward declaration https://git.kernel.org/coresight/c/aad548a9 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

5 months, 3 weeks

Re: [PATCH v4] perf: Allocate non-contiguous AUX pages by default

by Anshuman Khandual

On 5/7/25 23:43, Yabin Cui wrote: > perf always allocates contiguous AUX pages based on aux_watermark. > However, this contiguous allocation doesn't benefit all PMUs. For > instance, ARM SPE and TRBE operate with virtual pages, and Coresight > ETR allocates a separate buffer. For these PMUs, allocating contiguous > AUX pages unnecessarily exacerbates memory fragmentation. This > fragmentation can prevent their use on long-running devices. > > This patch modifies the perf driver to be memory-friendly by default, > by allocating non-contiguous AUX pages. For PMUs requiring contiguous > pages (Intel BTS and some Intel PT), the existing > PERF_PMU_CAP_AUX_NO_SG capability can be used. For PMUs that don't > require but can benefit from contiguous pages (some Intel PT), a new > capability, PERF_PMU_CAP_AUX_PREFER_LARGE, is added to maintain their > existing behavior. > > Signed-off-by: Yabin Cui <yabinc(a)google.com> > --- > Changes since v3: > Add comments and a local variable to explain max_order value > changes in rb_alloc_aux(). > > Changes since v2: > Let NO_SG imply PREFER_LARGE. So PMUs don't need to set both flags. > Then the only place needing PREFER_LARGE is intel/pt.c. > > Changes since v1: > In v1, default is preferring contiguous pages, and add a flag to > allocate non-contiguous pages. In v2, default is allocating > non-contiguous pages, and add a flag to prefer contiguous pages. > > v1 patchset: > perf,coresight: Reduce fragmentation with non-contiguous AUX pages for > cs_etm > > arch/x86/events/intel/pt.c | 2 ++ > include/linux/perf_event.h | 1 + > kernel/events/ring_buffer.c | 33 ++++++++++++++++++++++++--------- > 3 files changed, 27 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c > index fa37565f6418..25ead919fc48 100644 > --- a/arch/x86/events/intel/pt.c > +++ b/arch/x86/events/intel/pt.c > @@ -1863,6 +1863,8 @@ static __init int pt_init(void) > > if (!intel_pt_validate_hw_cap(PT_CAP_topa_multiple_entries)) > pt_pmu.pmu.capabilities = PERF_PMU_CAP_AUX_NO_SG; > + else > + pt_pmu.pmu.capabilities = PERF_PMU_CAP_AUX_PREFER_LARGE; > > pt_pmu.pmu.capabilities |= PERF_PMU_CAP_EXCLUSIVE | > PERF_PMU_CAP_ITRACE | > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 0069ba6866a4..56d77348c511 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -301,6 +301,7 @@ struct perf_event_pmu_context; > #define PERF_PMU_CAP_AUX_OUTPUT 0x0080 > #define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0100 > #define PERF_PMU_CAP_AUX_PAUSE 0x0200 > +#define PERF_PMU_CAP_AUX_PREFER_LARGE 0x0400 > > /** > * pmu::scope > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c > index 5130b119d0ae..69c90ea1b79a 100644 > --- a/kernel/events/ring_buffer.c > +++ b/kernel/events/ring_buffer.c > @@ -679,7 +679,19 @@ int rb_alloc_aux(struct perf_buffer *rb, struct perf_event *event, > { > bool overwrite = !(flags & RING_BUFFER_WRITABLE); > int node = (event->cpu == -1) ? -1 : cpu_to_node(event->cpu); > - int ret = -ENOMEM, max_order; > + /* > + * True if the PMU needs a contiguous AUX buffer (CAP_AUX_NO_SG) or > + * prefers large contiguous pages (CAP_AUX_PREFER_LARGE). > + */ > + bool use_contiguous_pages = event->pmu->capabilities & ( > + PERF_PMU_CAP_AUX_NO_SG | PERF_PMU_CAP_AUX_PREFER_LARGE); > + /* > + * Initialize max_order to 0 for page allocation. This allocates single > + * pages to minimize memory fragmentation. This is overriden if Small nit typo -- s/overriden/overridden ^^^^ > + * use_contiguous_pages is true. > + */ > + int max_order = 0; > + int ret = -ENOMEM; > > if (!has_aux(event)) > return -EOPNOTSUPP; > @@ -689,8 +701,8 @@ int rb_alloc_aux(struct perf_buffer *rb, struct perf_event *event, > > if (!overwrite) { > /* > - * Watermark defaults to half the buffer, and so does the > - * max_order, to aid PMU drivers in double buffering. > + * Watermark defaults to half the buffer, to aid PMU drivers > + * in double buffering. > */ > if (!watermark) > watermark = min_t(unsigned long, > @@ -698,16 +710,19 @@ int rb_alloc_aux(struct perf_buffer *rb, struct perf_event *event, > (unsigned long)nr_pages << (PAGE_SHIFT - 1)); > > /* > - * Use aux_watermark as the basis for chunking to > - * help PMU drivers honor the watermark. > + * If using contiguous pages, use aux_watermark as the basis > + * for chunking to help PMU drivers honor the watermark. > */ > - max_order = get_order(watermark); > + if (use_contiguous_pages) > + max_order = get_order(watermark); > } else { > /* > - * We need to start with the max_order that fits in nr_pages, > - * not the other way around, hence ilog2() and not get_order. > + * If using contiguous pages, we need to start with the > + * max_order that fits in nr_pages, not the other way around, > + * hence ilog2() and not get_order. > */ > - max_order = ilog2(nr_pages); > + if (use_contiguous_pages) > + max_order = ilog2(nr_pages); > watermark = 0; > } > Reviewed-by: Anshuman Khandual <anshuman.khandual(a)arm.com>

5 months, 3 weeks

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight