Re: [PATCH v3 2/5] coresight: configfs: Add in binary attributes to load files

[Mathieu Poirier <mathieu.poirier@linaro.org>]

On Thu, Apr 14, 2022 at 07:44:54AM +0100, 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 <mike.leach@linaro.org>
> ---
>  .../coresight/coresight-config-file.h         |  9 ++
>  .../coresight/coresight-syscfg-configfs.c     | 91 +++++++++++++++++++
>  .../hwtracing/coresight/coresight-syscfg.c    | 37 ++++++++
>  .../hwtracing/coresight/coresight-syscfg.h    |  1 +
>  4 files changed, 138 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-config-file.h b/drivers/hwtracing/coresight/coresight-config-file.h
> index 591f4c2c4be9..f85cf1aabf56 100644
> --- a/drivers/hwtracing/coresight/coresight-config-file.h
> +++ b/drivers/hwtracing/coresight/coresight-config-file.h
> @@ -7,6 +7,8 @@
>  #ifndef _CORESIGHT_CORESIGHT_CONFIG_FILE_H
>  #define _CORESIGHT_CORESIGHT_CONFIG_FILE_H
>  
> +#include "coresight-config.h"
> +
>  /*
>   * Structures to represent configuration descriptors in a memory buffer
>   * to serialise to and from files
> @@ -114,4 +116,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..d1ddcba10ad8 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,10 +381,100 @@ 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_load_descs *load_descs = 0;
> +	struct cscfg_load_owner_info *owner_info = 0;
> +	int err = 0;
> +
> +	if (size > CSCFG_FILE_MAXSIZE) {
> +		pr_err("cscfg: 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;
> +	}
> +
> +	owner_info->owner_handle = load_descs;
> +	owner_info->type = CSCFG_OWNER_CONFIGFS;
> +
> +	err = cscfg_file_read_buffer(buffer, size, load_descs);
> +	if (err) {
> +		pr_err("cscfg: 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) {
> +		pr_err("cscfg: Load error - Failed to load configuaration file.\n");
> +		goto exit_memfree;
> +	}
> +
> +	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_load_owner_info *owner_info = 0;
> +	const char *name;
> +	int err;
> +
> +	if (size > CSCFG_FILE_MAXSIZE) {
> +		pr_err("cscfg: Unload error - Input file too large\n");
> +		return -EINVAL;
> +	}
> +
> +	err = cscfg_file_read_buffer_first_name(buffer, size, &name);
> +	if (err) {
> +		pr_err("cscfg: Unload error - Failed to read input file\n");
> +		return err;
> +	}
> +
> +	owner_info = cscfg_find_fs_owned_cfg_by_name(name);
> +	if (!owner_info) {
> +		pr_err("cscfg: Unload error: Failed to find configuration %s from input file\n",
> +		       name);
> +		return err;

Since cscfg_find_fs_owned_cfg_by_name() reads a configuration of feature name, I
think the error message should be:

        pr_err("cscfg: Unload error: Failed to find %s from input file\n",
                name);

> +	}
> +	err = cscfg_unload_config_sets(owner_info);
> +	if (err) {
> +		pr_err("cscfg: Unload error: Cannot unload configuration %s\n",
> +		       name);
> +		return err;
> +	}
> +
> +	kfree(owner_info);
> +	kfree((struct cscfg_fs_load_descs *)(owner_info->owner_handle));

I think the internal memory for the arrays contained in the cscfg_fs_load_descs
structure should be freed explicitly here, especially if they can be up to 16K.

More comments to come later or tomorrow.

Thanks,
Mathieu

> +	return size;
> +}
> +CONFIGFS_BIN_ATTR_WO(cscfg_cfg_, unload, NULL, CSCFG_FILE_MAXSIZE);
> +
> +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,
>  };
>  
> +/* group for configurations dir, with load and unload attribs */
>  static struct config_group cscfg_configs_grp = {
>  	.cg_item = {
>  		.ci_namebuf = "configurations",
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
> index 11850fd8c3b5..2e478b3e8c8d 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.c
> @@ -587,6 +587,43 @@ 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_load_owner_info *cscfg_find_fs_owned_cfg_by_name(const char *name)
> +{
> +	struct cscfg_load_owner_info *owner_info = NULL;
> +	struct cscfg_fs_load_descs *fs_load_cfg;
> +	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;
> +			}
> +		}
> +	}
> +
> +	/* not found */
> +	owner_info = NULL;
> +
> +exit_unlock:
> +	mutex_unlock(&cscfg_mutex);
> +	return owner_info;
> +}
> +
>  /* 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..487d872d931a 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_load_owner_info *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
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel