Subject: Re: [PATCH v2 2/6] coresight: configfs: Add in binary attributes to load files

From: Mathieu Poirier <[email-masked]>
Date: Fri, 28 Jan 2022 11:17:23 -0700

On Tue, Nov 30, 2021 at 10:00:56PM +0000, Mike Leach wrote:
> Add in functionality and binary attribute to load configurations
> as binary data.
> 
> Reads the incoming attribute, which must be formatted correctly
> as defined in the file reader code - and will create a configuration
> and/or features and load them into the system.
> 
> These will then appear in configfs ready for use.
> 
> Unload functionality is also provided.
> 
> Signed-off-by: Mike Leach <[email-masked]>
> ---
>  .../coresight/coresight-config-file.h         |   7 +
>  .../coresight/coresight-syscfg-configfs.c     | 148 +++++++++++++++++-
>  .../coresight/coresight-syscfg-configfs.h     |   8 +
>  .../hwtracing/coresight/coresight-syscfg.c    |  36 +++++
>  .../hwtracing/coresight/coresight-syscfg.h    |   1 +
>  5 files changed, 193 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-config-file.h b/drivers/hwtracing/coresight/coresight-config-file.h
> index 6c8c5af0a614..03899f7d94c9 100644
> --- a/drivers/hwtracing/coresight/coresight-config-file.h
> +++ b/drivers/hwtracing/coresight/coresight-config-file.h
> @@ -115,4 +115,11 @@ struct cscfg_file_elem_str {
>  	char *str;
>  };
>  
> +/* kernel configfs needs to read the incoming file buffers to load. */
> +int cscfg_file_read_buffer(const u8 *buffer, const int buflen,
> +			   struct cscfg_fs_load_descs *desc_arrays);
> +/* to unload we just need the first name - config or first feature */
> +int cscfg_file_read_buffer_first_name(const u8 *buffer, const int buflen,
> +				      const char **name);
> +
>  #endif /* _CORESIGHT_CORESIGHT_CONFIG_FILE_H */
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> index 433ede94dd63..50abdb5aa6b6 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> @@ -7,6 +7,7 @@
>  #include <linux/configfs.h>
>  
>  #include "coresight-config.h"
> +#include "coresight-config-file.h"
>  #include "coresight-syscfg-configfs.h"
>  
>  /* create a default ci_type. */
> @@ -380,14 +381,147 @@ static struct config_group *cscfg_create_feature_group(struct cscfg_feature_desc
>  	return &feat_view->group;
>  }
>  
> +/* Attributes in configfs that allow load and unload of configuration binary files */
> +
> +/* load "buffer" as a configuration binary file */
> +static ssize_t cscfg_cfg_load_write(struct config_item *item, const void *buffer, size_t size)
> +{
> +	struct cscfg_fs_configs_grp *configs_grp;
> +	struct cscfg_fs_load_descs *load_descs = 0;
> +	struct cscfg_load_owner_info *owner_info = 0;
> +	int err = 0;
> +	const char *name;
> +
> +	configs_grp = container_of(to_config_group(item), struct cscfg_fs_configs_grp, group);
> +	if (size > CSCFG_FILE_MAXSIZE) {
> +		scnprintf(configs_grp->status, CSCFG_FS_STATUS_STRLEN,
> +			  "Load error: Input file too large.\n");
> +		return -EINVAL;
> +	}
> +
> +	load_descs = kzalloc(sizeof(struct cscfg_fs_load_descs), GFP_KERNEL);
> +	owner_info = kzalloc(sizeof(struct cscfg_load_owner_info), GFP_KERNEL);
> +	if (!load_descs || !owner_info) {
> +		err = -ENOMEM;
> +		goto exit_memfree;
> +	}
> +
> +	load_descs->owner_info = owner_info;
> +	owner_info->owner_handle = load_descs;
> +	owner_info->type = CSCFG_OWNER_CONFIGFS;
> +
> +	err = cscfg_file_read_buffer(buffer, size, load_descs);
> +	if (err) {
> +		scnprintf(configs_grp->status, CSCFG_FS_STATUS_STRLEN,
> +			  "Load error: Failed to read input file.\n");
> +		goto exit_memfree;
> +	}
> +
> +	err = cscfg_load_config_sets(load_descs->config_descs, load_descs->feat_descs, owner_info);
> +	if (err) {
> +		scnprintf(configs_grp->status, CSCFG_FS_STATUS_STRLEN,
> +			  "Load error: Failed to load configuaration file.\n");
> +		goto exit_memfree;
> +	}
> +
> +	/* name of config if there is one, otherwise first feature */
> +	if (load_descs->config_descs[0])
> +		name = load_descs->config_descs[0]->name;
> +	else
> +		name = load_descs->feat_descs[0]->name;
> +	scnprintf(configs_grp->status, CSCFG_FS_STATUS_STRLEN,
> +		  "OK: configuration file loaded (%s).\n", name);
> +
> +	return size;
> +
> +exit_memfree:
> +	kfree(load_descs);
> +	kfree(owner_info);
> +	return err;
> +}
> +CONFIGFS_BIN_ATTR_WO(cscfg_cfg_, load, NULL, CSCFG_FILE_MAXSIZE);
> +
> +/* read "buffer" and unload configuration */
> +static ssize_t cscfg_cfg_unload_write(struct config_item *item, const void *buffer, size_t size)
> +{
> +	struct cscfg_fs_configs_grp *configs_grp;
> +	struct cscfg_fs_load_descs *load_descs;
> +	const char *name;
> +	int err;
> +
> +	configs_grp = container_of(to_config_group(item), struct cscfg_fs_configs_grp, group);
> +	if (size > CSCFG_FILE_MAXSIZE) {
> +		scnprintf(configs_grp->status, CSCFG_FS_STATUS_STRLEN,
> +			  "Unload error: Input file too large\n");
> +		return -EINVAL;
> +	}
> +
> +	err = cscfg_file_read_buffer_first_name(buffer, size, &name);
> +	if (err) {
> +		scnprintf(configs_grp->status, CSCFG_FS_STATUS_STRLEN,
> +			  "Unload error: Failed to read input file\n");
> +		return err;
> +	}
> +
> +	load_descs = cscfg_find_fs_owned_cfg_by_name(name);
> +	if (!load_descs) {
> +		scnprintf(configs_grp->status, CSCFG_FS_STATUS_STRLEN,
> +			  "Unload error: Failed to find configuration %s from input file\n",
> +			  name);
> +		return err;
> +	}
> +	err = cscfg_unload_config_sets(load_descs->owner_info);
> +	if (err) {
> +		scnprintf(configs_grp->status, CSCFG_FS_STATUS_STRLEN,
> +			  "Unload error: Cannot unload configuration %s\n",
> +			  name);
> +		return err;
> +	}
> +
> +	scnprintf(configs_grp->status, CSCFG_FS_STATUS_STRLEN,
> +		  "OK: configuration file unloaded (%s).\n", name);
> +
> +	kfree((struct cscfg_load_owner_info *)load_descs->owner_info);
> +	kfree(load_descs);
> +	return size;
> +}
> +CONFIGFS_BIN_ATTR_WO(cscfg_cfg_, unload, NULL, CSCFG_FILE_MAXSIZE);
> +
> +/* show the status of the last load / unload operation */
> +static ssize_t cscfg_cfg_last_load_status_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_configs_grp *configs_grp;
> +
> +	configs_grp = container_of(to_config_group(item), struct cscfg_fs_configs_grp, group);
> +
> +	return scnprintf(page, PAGE_SIZE, "%s\n", configs_grp->status);

I am still very ambivalent about this status thing...  Especially since it only
reports on the last item that was loaded/unloaded.

> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, last_load_status);
> +
> +static struct configfs_attribute *cscfg_config_configs_attrs[] = {
> +	&cscfg_cfg_attr_last_load_status,
> +	NULL,
> +};
> +
> +static struct configfs_bin_attribute *cscfg_config_configfs_bin_attrs[] = {
> +	&cscfg_cfg_attr_load,
> +	&cscfg_cfg_attr_unload,
> +	NULL,
> +};
> +
>  static struct config_item_type cscfg_configs_type = {
>  	.ct_owner = THIS_MODULE,
> +	.ct_bin_attrs = cscfg_config_configfs_bin_attrs,
> +	.ct_attrs = cscfg_config_configs_attrs,
>  };
>  
> -static struct config_group cscfg_configs_grp = {
> -	.cg_item = {
> -		.ci_namebuf = "configurations",
> -		.ci_type = &cscfg_configs_type,
> +/* group for configurations dir, with load, unload and status attribs */
> +static struct cscfg_fs_configs_grp cscfg_configs_grp = {
> +	.group = {
> +		.cg_item = {
> +			.ci_namebuf = "configurations",
> +			.ci_type = &cscfg_configs_type,
> +		},
>  	},
>  };
>  
> @@ -400,7 +534,7 @@ int cscfg_configfs_add_config(struct cscfg_config_desc *config_desc)
>  	new_group = cscfg_create_config_group(config_desc);
>  	if (IS_ERR(new_group))
>  		return PTR_ERR(new_group);
> -	err =  configfs_register_group(&cscfg_configs_grp, new_group);
> +	err =  configfs_register_group(&cscfg_configs_grp.group, new_group);
>  	if (!err)
>  		config_desc->fs_group = new_group;
>  	return err;
> @@ -468,8 +602,8 @@ int cscfg_configfs_init(struct cscfg_manager *cscfg_mgr)
>  	mutex_init(&subsys->su_mutex);
>  
>  	/* Add default groups to subsystem */
> -	config_group_init(&cscfg_configs_grp);
> -	configfs_add_default_group(&cscfg_configs_grp, &subsys->su_group);
> +	config_group_init(&cscfg_configs_grp.group);
> +	configfs_add_default_group(&cscfg_configs_grp.group, &subsys->su_group);
>  
>  	config_group_init(&cscfg_features_grp);
>  	configfs_add_default_group(&cscfg_features_grp, &subsys->su_group);
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> index 373d84d43268..8d6900e8c1ea 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> @@ -11,6 +11,14 @@
>  
>  #define CSCFG_FS_SUBSYS_NAME "cs-syscfg"
>  
> +#define CSCFG_FS_STATUS_STRLEN 256
> +
> +/* container for configs group */
> +struct cscfg_fs_configs_grp {
> +	struct config_group group;
> +	char status[CSCFG_FS_STATUS_STRLEN];
> +};
> +
>  /* container for configuration view */
>  struct cscfg_fs_config {
>  	struct cscfg_config_desc *config_desc;
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
> index 098fc34c4829..b5804cc2af9c 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.c
> @@ -587,6 +587,42 @@ int cscfg_unload_config_sets(struct cscfg_load_owner_info *owner_info)
>  }
>  EXPORT_SYMBOL_GPL(cscfg_unload_config_sets);
>  
> +/* find a configuration owned by configfs by name of config / first feature */
> +struct cscfg_fs_load_descs *cscfg_find_fs_owned_cfg_by_name(const char *name)
> +{
> +	struct cscfg_load_owner_info *owner_info;
> +	struct cscfg_fs_load_descs *fs_load_cfg = NULL;
> +	struct cscfg_config_desc *config_desc;
> +	struct cscfg_feature_desc *feat_desc;
> +
> +	mutex_lock(&cscfg_mutex);
> +
> +	/* search the load_owner list for CONFIGFS loaded types */
> +	list_for_each_entry(owner_info, &cscfg_mgr->load_order_list, item) {
> +		/* if this is a config fs owned item, then try to match */
> +		if (owner_info->type == CSCFG_OWNER_CONFIGFS) {
> +			fs_load_cfg = owner_info->owner_handle;
> +			/* first try to match the name against the config if it exists */
> +			if (fs_load_cfg->config_descs[0]) {
> +				config_desc = fs_load_cfg->config_descs[0];
> +				if (!strcmp(config_desc->name, name))
> +					goto exit_unlock;
> +			/* no config - match against first feature name */
> +			} else {
> +				feat_desc = fs_load_cfg->feat_descs[0];
> +				if (!strcmp(feat_desc->name, name))
> +					goto exit_unlock;
> +			}
> +			/* no match - move on */
> +			fs_load_cfg = NULL;
> +		}
> +	}
> +
> +exit_unlock:
> +	mutex_unlock(&cscfg_mutex);
> +	return fs_load_cfg;
> +}
> +
>  /* Handle coresight device registration and add configs and features to devices */
>  
>  /* iterate through config lists and load matching configs to device */
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
> index 6a6e33585be9..6bc29abe0650 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.h
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.h
> @@ -95,6 +95,7 @@ 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);
> +struct cscfg_fs_load_descs *cscfg_find_fs_owned_cfg_by_name(const char *name);
>  
>  /* syscfg manager external API */
>  int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel