Re: [PATCH v2 4/5] coresight: configfs: Allow configfs to activate configuration

Good day Mike,

On Tue, Oct 19, 2021 at 08:13:50PM +0100, Mike Leach wrote:

Adds configfs attributes to allow a configuration to be enabled for use when sysfs is used to control CoreSight.

perf retains independent enabling of configurations.

Signed-off-by: Mike Leach mike.leach@linaro.org

.../coresight/coresight-etm4x-core.c | 11 +- .../coresight/coresight-syscfg-configfs.c | 67 ++++++++++ .../coresight/coresight-syscfg-configfs.h | 2 + .../hwtracing/coresight/coresight-syscfg.c | 126 ++++++++++++++---- .../hwtracing/coresight/coresight-syscfg.h | 8 +- 5 files changed, 186 insertions(+), 28 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 86a313857b58..bf18128cf5de 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -722,7 +722,16 @@ static int etm4_enable_sysfs(struct coresight_device *csdev) { struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct etm4_enable_arg arg = { };

• int ret;

• unsigned long cfg_hash;
• int ret, preset;
• /* enable any config activated by configfs */
• cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset);
• if (cfg_hash) {

• ret = cscfg_csdev_enable_active_config(csdev, cfg_hash, preset);
  

• if (ret)
  

• 	return ret;
  

• }

spin_lock(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c index 345a62f1b728..c388a77a2683 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c @@ -6,6 +6,7 @@ #include <linux/configfs.h> +#include "coresight-config.h" #include "coresight-syscfg-configfs.h" /* create a default ci_type. */ @@ -87,9 +88,75 @@ static ssize_t cscfg_cfg_values_show(struct config_item *item, char *page) } CONFIGFS_ATTR_RO(cscfg_cfg_, values); +static ssize_t cscfg_cfg_activate_show(struct config_item *item, char *page) +{

• struct cscfg_fs_config *fs_config = container_of(to_config_group(item),

• 					 struct cscfg_fs_config, group);
  

• return scnprintf(page, PAGE_SIZE, "%d\n", fs_config->active);

+}

+static ssize_t cscfg_cfg_activate_store(struct config_item *item,

• 			const char *page, size_t count)
  

+{

• struct cscfg_fs_config *fs_config = container_of(to_config_group(item),

• 					 struct cscfg_fs_config, group);
  

• int err;
• bool val;
• err = kstrtobool(page, &val);
• if (!err)

• err = cscfg_config_sysfs_activate(fs_config->config_desc, val);
  

• if (!err) {

• fs_config->active = val;
  

• if (val)
  

• 	cscfg_config_sysfs_set_preset(fs_config->preset);
  

• }
• return err ? err : count;

+} +CONFIGFS_ATTR(cscfg_cfg_, activate);

I would call this "enable" to follow what is already done for coresight in sysfs.

+static ssize_t cscfg_cfg_active_preset_show(struct config_item *item, char *page) +{

• struct cscfg_fs_config *fs_config = container_of(to_config_group(item),

• 					 struct cscfg_fs_config, group);
  

• return scnprintf(page, PAGE_SIZE, "%d\n", fs_config->preset);

+}

+static ssize_t cscfg_cfg_active_preset_store(struct config_item *item,

• 			     const char *page, size_t count)
  

+{

• struct cscfg_fs_config *fs_config = container_of(to_config_group(item),

• 					 struct cscfg_fs_config, group);
  

• int preset, err;
• err = kstrtoint(page, 0, &preset);
• if (!err) {

• /*
  

•  * presets start at 1, and go up to max (15),
  

•  * but the config may provide fewer.
  

•  */
  

• if ((preset < 1) || (preset > fs_config->config_desc->nr_presets))
  

• 	err = -EINVAL;
  

• }
• if (!err) {

• /* set new value */
  

• fs_config->preset = preset;
  

• /* set on system if active */
  

• if (fs_config->active)
  

• 	cscfg_config_sysfs_set_preset(fs_config->preset);
  

• }
• return err ? err : count;

+} +CONFIGFS_ATTR(cscfg_cfg_, active_preset);

And I would call this "preset" since this is what is used on the perf cmd line.

Please add a new "Using Configurations from configfs" section in [1] to show how "enable" and "preset" should be used.

With that:

Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org

[1]. Documentation/trace/coresight/coresight-config.rst

static struct configfs_attribute *cscfg_config_view_attrs[] = { &cscfg_cfg_attr_description, &cscfg_cfg_attr_feature_refs,

• &cscfg_cfg_attr_activate,
• &cscfg_cfg_attr_active_preset, NULL,

}; diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h index ea1e54d29f7f..373d84d43268 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h @@ -15,6 +15,8 @@ struct cscfg_fs_config { struct cscfg_config_desc *config_desc; struct config_group group;

• bool active;
• int preset;

}; /* container for feature view */ diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c index 4b91d8d9bc37..759c32889efd 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg.c +++ b/drivers/hwtracing/coresight/coresight-syscfg.c @@ -744,30 +744,20 @@ void cscfg_csdev_reset_feats(struct coresight_device *csdev) } EXPORT_SYMBOL_GPL(cscfg_csdev_reset_feats); -/**

• • cscfg_activate_config - Mark a configuration descriptor as active.
• • This will be seen when csdev devices are enabled in the system.
• • Only activated configurations can be enabled on individual devices.
• • Activation protects the configuration from alteration or removal while
• • active.
• • Selection by hash value - generated from the configuration name when it
• • was loaded and added to the cs_etm/configurations file system for selection
• • by perf.

+/*

• • This activate configuration for either perf or sysfs. Perf can have multiple
• • active configs, selected per event, sysfs is limited to one.
  • Increments the configuration descriptor active count and the global active
  • count.
  • @cfg_hash: Hash value of the selected configuration name.
  */

-int cscfg_activate_config(unsigned long cfg_hash) +static int _cscfg_activate_config(unsigned long cfg_hash) { struct cscfg_config_desc *config_desc; int err = -EINVAL;

• mutex_lock(&cscfg_mutex);
• list_for_each_entry(config_desc, &cscfg_mgr->config_desc_list, item) { if ((unsigned long)config_desc->event_ea->var == cfg_hash) { /* must ensure that config cannot be unloaded in use */

@@ -791,6 +781,101 @@ int cscfg_activate_config(unsigned long cfg_hash) break; } }

• return err;

+}

+static void _cscfg_deactivate_config(unsigned long cfg_hash) +{

• struct cscfg_config_desc *config_desc;
• 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);
  

• 	dev_dbg(cscfg_device(), "Deactivate config %s.\n", config_desc->name);
  

• 	break;
  

• }
  

• }

+}

+/*

• • called from configfs to set/clear the active configuration for use when
• • using sysfs to control trace.
• */

+int cscfg_config_sysfs_activate(struct cscfg_config_desc *config_desc, bool activate) +{

• unsigned long cfg_hash;
• int err = 0;
• mutex_lock(&cscfg_mutex);
• cfg_hash = (unsigned long)config_desc->event_ea->var;
• if (activate) {

• /* cannot be a current active value to activate this */
  

• if (cscfg_mgr->sysfs_active_config) {
  

• 	err = -EBUSY;
  

• 	goto exit_unlock;
  

• }
  

• err = _cscfg_activate_config(cfg_hash);
  

• if (!err)
  

• 	cscfg_mgr->sysfs_active_config = cfg_hash;
  

• } else {

• /* disable if matching current value */
  

• if (cscfg_mgr->sysfs_active_config == cfg_hash) {
  

• 	_cscfg_deactivate_config(cfg_hash);
  

• 	cscfg_mgr->sysfs_active_config = 0;
  

• } else
  

• 	err = -EINVAL;
  

• }

+exit_unlock:

• mutex_unlock(&cscfg_mutex);
• return err;

+}

+/* set the sysfs preset value */ +void cscfg_config_sysfs_set_preset(int preset) +{

• mutex_lock(&cscfg_mutex);
• cscfg_mgr->sysfs_active_preset = preset;
• mutex_unlock(&cscfg_mutex);

+}

+/*

• • Used by a device to get the config and preset selected as active in configfs,
• • when using sysfs to control trace.
• */

+void cscfg_config_sysfs_get_active_cfg(unsigned long *cfg_hash, int *preset) +{

• mutex_lock(&cscfg_mutex);
• *preset = cscfg_mgr->sysfs_active_preset;
• *cfg_hash = cscfg_mgr->sysfs_active_config;
• mutex_unlock(&cscfg_mutex);

+} +EXPORT_SYMBOL_GPL(cscfg_config_sysfs_get_active_cfg);

+/**

• • cscfg_activate_config - Mark a configuration descriptor as active.
• • This will be seen when csdev devices are enabled in the system.
• • Only activated configurations can be enabled on individual devices.
• • Activation protects the configuration from alteration or removal while
• • active.
• • Selection by hash value - generated from the configuration name when it
• • was loaded and added to the cs_etm/configurations file system for selection
• • by perf.
• • @cfg_hash: Hash value of the selected configuration name.
• */

+int cscfg_activate_config(unsigned long cfg_hash) +{

• int err = 0;
• mutex_lock(&cscfg_mutex);
• err = _cscfg_activate_config(cfg_hash); mutex_unlock(&cscfg_mutex);

return err; @@ -806,19 +891,8 @@ EXPORT_SYMBOL_GPL(cscfg_activate_config); */ void cscfg_deactivate_config(unsigned long cfg_hash) {

• struct cscfg_config_desc *config_desc;
• mutex_lock(&cscfg_mutex);
• 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);
  

• 	dev_dbg(cscfg_device(), "Deactivate config %s.\n", config_desc->name);
  

• 	break;
  

• }
  

• }

• _cscfg_deactivate_config(cfg_hash); mutex_unlock(&cscfg_mutex);

} EXPORT_SYMBOL_GPL(cscfg_deactivate_config); diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h index 1da37874f70f..9106ffab4833 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg.h +++ b/drivers/hwtracing/coresight/coresight-syscfg.h @@ -28,6 +28,8 @@

• @load_order_list: Ordered list of owners for dynamically loaded configurations.
• @sys_active_cnt: Total number of active config descriptor references.
• @cfgfs_subsys: configfs subsystem used to manage configurations.

• • @sysfs_active_config:Active config hash used if CoreSight controlled from sysfs.
• • @sysfs_active_preset:Active preset index used if CoreSight controlled from sysfs.
  */

struct cscfg_manager { struct device dev; @@ -37,6 +39,8 @@ struct cscfg_manager { struct list_head load_order_list; atomic_t sys_active_cnt; struct configfs_subsystem cfgfs_subsys;

• u32 sysfs_active_config;
• int sysfs_active_preset;

}; /* get reference to dev in cscfg_manager */ @@ -88,7 +92,8 @@ int cscfg_preload(void *owner_handle); const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name); int cscfg_update_feat_param_val(struct cscfg_feature_desc *feat_desc, int param_idx, u64 value);

+int cscfg_config_sysfs_activate(struct cscfg_config_desc *cfg_desc, bool activate); +void cscfg_config_sysfs_set_preset(int preset); /* syscfg manager external API */ int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs, @@ -104,5 +109,6 @@ void cscfg_csdev_reset_feats(struct coresight_device *csdev); int cscfg_csdev_enable_active_config(struct coresight_device *csdev, unsigned long cfg_hash, int preset); void cscfg_csdev_disable_active_config(struct coresight_device *csdev); +void cscfg_config_sysfs_get_active_cfg(unsigned long *cfg_hash, int *preset);

#endif /* CORESIGHT_SYSCFG_H */

2.17.1