[PATCH v3 2/5] coresight: configfs: Add in binary attributes to load files
Mike Leach
mike.leach at linaro.org
Wed Jun 1 01:33:01 PDT 2022
Hi,
previous comments re: freeing up memory actioned.
On Wed, 25 May 2022 at 20:30, Mathieu Poirier
<mathieu.poirier at linaro.org> wrote:
>
> 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 at 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;
> > + }
> > + 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));
> > + 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,
>
> I was thinking... Because it is possible to have a configuration file that only
> has features, wouldn't it make more sense to add the load and unload entries
> under cs-syscfg rather than cs-syscfg/configuration?
>
That is a possibility - though the docs refer to "configuration files"
irrespective of what they actually contain, so I think having these in
configurations
is not too bad.
cs-syscfg is regarded as our subsystem root so treated slightly
differently in terms of registration etc in the configfs API.
We know we can hang groups (directories) off it, so presumably
attributes (files) would be fine too.
Shouldn't be too difficult to try out, and I can easily back out if
issues arise.
Thanks
Mike
> > };
> >
> > +/* 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
> >
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
More information about the linux-arm-kernel
mailing list