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

Hi Mathieu,

On Fri, 28 Jan 2022 at 18:17, Mathieu Poirier mathieu.poirier@linaro.org wrote:

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 mike.leach@linaro.org

.../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.

I guess the priciple use is to give feedback to the user if a load error occurs. An alternative could be pr_err at point of error. What I am trying to avoid is the case where someone attempts to load a configuration and it fails but there is no feedback. (Once the resource management patchset set for ETM is used, tnen loading can fail if the resources required do not match those avaialble - this is important given that amount of ETM resource is a hardware design implementation detail).

Regards

Mike

...

+} +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