linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/6]  coresight: syscfg: Extend configfs for config load
@ 2022-12-19 23:46 Mike Leach
  2022-12-19 23:46 ` [PATCH v5 1/6] coresight: configfs: Update memory allocation / free for configfs elements Mike Leach
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Mike Leach @ 2022-12-19 23:46 UTC (permalink / raw)
  To: linux-arm-kernel, coresight, linux-kernel
  Cc: mathieu.poirier, suzuki.poulose, acme, james.clark, Mike Leach

This set extends the configfs support to allow loading and unloading of
configurations as binary files via configfs.

Additional attributes - load and unload are provided to in the
/config/cs-syscfg subsytem base group to implement the load functionality.

Routines to generate binary configuration files are supplied in
./tools/coresight.

Example generator and reader applications are provided.

Tools may be cross compiled or built for use on host system.

Documentation is updated to describe feature usage.

Changes since v4:
1) Update coresight/next - 6.1-rc3
2) Update to lockdep fixes to avoid read lock race in configfs.

Changes since v3:
1) Rebase & tested on coresight/next - 5.19-rc3 - which includes the
fix patch for earlier configfs works.
2) Lockdep investigations resulted in re-design of some of the code
accessing configfs.
3) moved load and unload attributes to root of cs-syscfg. (Mathieu)
4) Additional minor fixes suggested by Mathieu.
5) Memory for configfs loaded and unloaded configurations is now
explicitly freed.
6) LOCKDEP nesting fix for configfs base code (fs/configfs/dir.c)

Changes since v2:
1) Rebased & tested on coresight/next - 5.18-rc2
2) Moved coresight config generator and reader programs from samples to
tools/coresight. Docs updated to match. (suggested by Mathieu)
3) userspace builds now use userspace headers from tools/...
4) Other minor fixes from Mathieu's review.

Changes since v1:
1) Rebased to coresight/next - 5.16-rc1 with previous coresight config
set applied.
2) Makefile.host fixed to default to all target.

Mike Leach (6):
  coresight: configfs: Update memory allocation / free for configfs
    elements
  coresight: configfs: Add in functionality for load via configfs
  coresight: configfs: Add in binary attributes to load files
  coresight: configfs: Modify config files to allow userspace use
  coresight: tools: Add config file write and reader tools
  Documentation: coresight: docs for config load via configfs

 .../trace/coresight/coresight-config.rst      | 202 +++++-
 MAINTAINERS                                   |   1 +
 drivers/hwtracing/coresight/Makefile          |   2 +-
 .../coresight/coresight-config-file.c         | 583 ++++++++++++++++++
 .../coresight/coresight-config-file.h         | 139 +++++
 .../hwtracing/coresight/coresight-config.h    |  27 +
 .../coresight/coresight-syscfg-configfs.c     | 361 +++++++++--
 .../coresight/coresight-syscfg-configfs.h     |   4 +
 .../hwtracing/coresight/coresight-syscfg.c    | 137 +++-
 .../hwtracing/coresight/coresight-syscfg.h    |   6 +-
 tools/coresight/Makefile                      |  52 ++
 tools/coresight/coresight-cfg-bufw.c          | 309 ++++++++++
 tools/coresight/coresight-cfg-bufw.h          |  26 +
 tools/coresight/coresight-cfg-example1.c      |  62 ++
 tools/coresight/coresight-cfg-example2.c      |  95 +++
 tools/coresight/coresight-cfg-examples.h      |  22 +
 tools/coresight/coresight-cfg-file-gen.c      |  61 ++
 tools/coresight/coresight-cfg-file-read.c     | 227 +++++++
 tools/coresight/coresight-config-uapi.h       |  76 +++
 19 files changed, 2339 insertions(+), 53 deletions(-)
 create mode 100644 drivers/hwtracing/coresight/coresight-config-file.c
 create mode 100644 drivers/hwtracing/coresight/coresight-config-file.h
 create mode 100644 tools/coresight/Makefile
 create mode 100644 tools/coresight/coresight-cfg-bufw.c
 create mode 100644 tools/coresight/coresight-cfg-bufw.h
 create mode 100644 tools/coresight/coresight-cfg-example1.c
 create mode 100644 tools/coresight/coresight-cfg-example2.c
 create mode 100644 tools/coresight/coresight-cfg-examples.h
 create mode 100644 tools/coresight/coresight-cfg-file-gen.c
 create mode 100644 tools/coresight/coresight-cfg-file-read.c
 create mode 100644 tools/coresight/coresight-config-uapi.h

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v5 1/6] coresight: configfs: Update memory allocation / free for configfs elements
  2022-12-19 23:46 [PATCH v5 0/6] coresight: syscfg: Extend configfs for config load Mike Leach
@ 2022-12-19 23:46 ` Mike Leach
  2022-12-19 23:46 ` [PATCH v5 2/6] coresight: configfs: Add in functionality for load via configfs Mike Leach
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Mike Leach @ 2022-12-19 23:46 UTC (permalink / raw)
  To: linux-arm-kernel, coresight, linux-kernel
  Cc: mathieu.poirier, suzuki.poulose, acme, james.clark, Mike Leach

Previously, the objects backing the configfs directories and files were
created using devm managed memory on the coresight device.

Now we are adding configfs load/unload, configurations can be loaded
many times over the lifetime of the device, so it is more appropriate to
use normally allocated and freed memory.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 .../coresight/coresight-syscfg-configfs.c     | 119 +++++++++++++-----
 1 file changed, 88 insertions(+), 31 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
index 433ede94dd63..6e8c8db52d39 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
@@ -14,7 +14,7 @@ static inline struct config_item_type *cscfg_create_ci_type(void)
 {
 	struct config_item_type *ci_type;
 
-	ci_type = devm_kzalloc(cscfg_device(), sizeof(*ci_type), GFP_KERNEL);
+	ci_type = kzalloc(sizeof(*ci_type), GFP_KERNEL);
 	if (ci_type)
 		ci_type->ct_owner = THIS_MODULE;
 
@@ -175,6 +175,19 @@ static struct config_item_type cscfg_config_preset_type = {
 	.ct_attrs = cscfg_config_preset_attrs,
 };
 
+
+/* walk list of presets and free the previously allocated memory */
+static void cscfg_destroy_preset_groups(struct config_group *cfg_view_group)
+{
+	struct cscfg_fs_preset *cfg_fs_preset;
+	struct config_group *p_group;
+
+	list_for_each_entry(p_group, &cfg_view_group->default_groups, default_groups) {
+		cfg_fs_preset = container_of(p_group, struct cscfg_fs_preset, group);
+		kfree(cfg_fs_preset);
+	}
+}
+
 static int cscfg_add_preset_groups(struct cscfg_fs_config *cfg_view)
 {
 	int preset_num;
@@ -186,11 +199,12 @@ static int cscfg_add_preset_groups(struct cscfg_fs_config *cfg_view)
 		return 0;
 
 	for (preset_num = 1; preset_num <= config_desc->nr_presets; preset_num++) {
-		cfg_fs_preset = devm_kzalloc(cscfg_device(),
-					     sizeof(struct cscfg_fs_preset), GFP_KERNEL);
+		cfg_fs_preset = kzalloc(sizeof(struct cscfg_fs_preset), GFP_KERNEL);
 
-		if (!cfg_fs_preset)
+		if (!cfg_fs_preset) {
+			cscfg_destroy_preset_groups(&cfg_view->group);
 			return -ENOMEM;
+		}
 
 		snprintf(name, CONFIGFS_ITEM_NAME_LEN, "preset%d", preset_num);
 		cfg_fs_preset->preset_num = preset_num;
@@ -204,14 +218,10 @@ static int cscfg_add_preset_groups(struct cscfg_fs_config *cfg_view)
 
 static struct config_group *cscfg_create_config_group(struct cscfg_config_desc *config_desc)
 {
-	struct cscfg_fs_config *cfg_view;
-	struct device *dev = cscfg_device();
+	struct cscfg_fs_config *cfg_view = NULL;
 	int err;
 
-	if (!dev)
-		return ERR_PTR(-EINVAL);
-
-	cfg_view = devm_kzalloc(dev, sizeof(struct cscfg_fs_config), GFP_KERNEL);
+	cfg_view = kzalloc(sizeof(struct cscfg_fs_config), GFP_KERNEL);
 	if (!cfg_view)
 		return ERR_PTR(-ENOMEM);
 
@@ -220,12 +230,21 @@ static struct config_group *cscfg_create_config_group(struct cscfg_config_desc *
 
 	/* add in a preset<n> dir for each preset */
 	err = cscfg_add_preset_groups(cfg_view);
-	if (err)
+	if (err) {
+		kfree(cfg_view);
 		return ERR_PTR(err);
-
+	}
 	return &cfg_view->group;
 }
 
+static void cscfg_destroy_config_group(struct config_group *group)
+{
+	struct cscfg_fs_config *cfg_view = container_of(group, struct cscfg_fs_config, group);
+
+	cscfg_destroy_preset_groups(&cfg_view->group);
+	kfree(cfg_view);
+}
+
 /* attributes for features view */
 
 static ssize_t cscfg_feat_description_show(struct config_item *item, char *page)
@@ -314,6 +333,17 @@ static struct config_item_type cscfg_param_view_type = {
 	.ct_attrs = cscfg_param_view_attrs,
 };
 
+/* walk the list of default groups - which were set as param items and remove */
+static void cscfg_destroy_params_group_items(struct config_group *params_group)
+{
+	struct cscfg_fs_param *param_item;
+	struct config_group *p_group;
+
+	list_for_each_entry(p_group, &params_group->default_groups, default_groups) {
+		param_item = container_of(p_group, struct cscfg_fs_param, group);
+		kfree(param_item);
+	}
+}
 /*
  * configfs has far less functionality provided to add attributes dynamically than sysfs,
  * and the show and store fns pass the enclosing config_item so the actual attribute cannot
@@ -322,15 +352,16 @@ static struct config_item_type cscfg_param_view_type = {
 static int cscfg_create_params_group_items(struct cscfg_feature_desc *feat_desc,
 					   struct config_group *params_group)
 {
-	struct device *dev = cscfg_device();
 	struct cscfg_fs_param *param_item;
 	int i;
 
 	/* parameter items - as groups with default_value attribute */
 	for (i = 0; i < feat_desc->nr_params; i++) {
-		param_item = devm_kzalloc(dev, sizeof(struct cscfg_fs_param), GFP_KERNEL);
-		if (!param_item)
+		param_item = kzalloc(sizeof(struct cscfg_fs_param), GFP_KERNEL);
+		if (!param_item) {
+			cscfg_destroy_params_group_items(params_group);
 			return -ENOMEM;
+		}
 		param_item->feat_desc = feat_desc;
 		param_item->param_idx = i;
 		config_group_init_type_name(&param_item->group,
@@ -343,27 +374,22 @@ static int cscfg_create_params_group_items(struct cscfg_feature_desc *feat_desc,
 
 static struct config_group *cscfg_create_feature_group(struct cscfg_feature_desc *feat_desc)
 {
-	struct cscfg_fs_feature *feat_view;
-	struct config_item_type *params_group_type;
+	struct cscfg_fs_feature *feat_view = NULL;
+	struct config_item_type *params_group_type = NULL;
 	struct config_group *params_group = NULL;
-	struct device *dev = cscfg_device();
-	int item_err;
-
-	if (!dev)
-		return ERR_PTR(-EINVAL);
+	int err = -ENOMEM;
 
-	feat_view = devm_kzalloc(dev, sizeof(struct cscfg_fs_feature), GFP_KERNEL);
+	feat_view = kzalloc(sizeof(struct cscfg_fs_feature), GFP_KERNEL);
 	if (!feat_view)
 		return ERR_PTR(-ENOMEM);
 
 	if (feat_desc->nr_params) {
-		params_group = devm_kzalloc(dev, sizeof(struct config_group), GFP_KERNEL);
+		params_group = kzalloc(sizeof(struct config_group), GFP_KERNEL);
 		if (!params_group)
-			return ERR_PTR(-ENOMEM);
-
+			goto exit_err_free_mem;
 		params_group_type = cscfg_create_ci_type();
 		if (!params_group_type)
-			return ERR_PTR(-ENOMEM);
+			goto exit_err_free_mem;
 	}
 
 	feat_view->feat_desc = feat_desc;
@@ -373,11 +399,36 @@ static struct config_group *cscfg_create_feature_group(struct cscfg_feature_desc
 	if (params_group) {
 		config_group_init_type_name(params_group, "params", params_group_type);
 		configfs_add_default_group(params_group, &feat_view->group);
-		item_err = cscfg_create_params_group_items(feat_desc, params_group);
-		if (item_err)
-			return ERR_PTR(item_err);
+		err = cscfg_create_params_group_items(feat_desc, params_group);
+		if (err)
+			goto exit_err_free_mem;
 	}
 	return &feat_view->group;
+
+exit_err_free_mem:
+	kfree(feat_view);
+	kfree(params_group_type);
+	kfree(params_group);
+	return ERR_PTR(err);
+}
+
+static void cscfg_destroy_feature_group(struct config_group *feat_group)
+{
+	struct cscfg_fs_feature *feat_view;
+	struct config_group *params_group = NULL;
+
+	feat_view = container_of(feat_group, struct cscfg_fs_feature, group);
+
+	/* params group is the first item on the default group list */
+	if (!list_empty(&feat_group->default_groups)) {
+		params_group = list_first_entry(&feat_group->default_groups,
+						struct config_group, default_groups);
+		cscfg_destroy_params_group_items(params_group);
+		/* free the item type, then the group */
+		kfree(params_group->cg_item.ci_type);
+		kfree(params_group);
+	}
+	kfree(feat_view);
 }
 
 static struct config_item_type cscfg_configs_type = {
@@ -403,6 +454,8 @@ int cscfg_configfs_add_config(struct cscfg_config_desc *config_desc)
 	err =  configfs_register_group(&cscfg_configs_grp, new_group);
 	if (!err)
 		config_desc->fs_group = new_group;
+	else
+		cscfg_destroy_config_group(new_group);
 	return err;
 }
 
@@ -410,6 +463,7 @@ void cscfg_configfs_del_config(struct cscfg_config_desc *config_desc)
 {
 	if (config_desc->fs_group) {
 		configfs_unregister_group(config_desc->fs_group);
+		cscfg_destroy_config_group(config_desc->fs_group);
 		config_desc->fs_group = NULL;
 	}
 }
@@ -434,9 +488,11 @@ int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc)
 	new_group = cscfg_create_feature_group(feat_desc);
 	if (IS_ERR(new_group))
 		return PTR_ERR(new_group);
-	err =  configfs_register_group(&cscfg_features_grp, new_group);
+	err = configfs_register_group(&cscfg_features_grp, new_group);
 	if (!err)
 		feat_desc->fs_group = new_group;
+	else
+		cscfg_destroy_feature_group(new_group);
 	return err;
 }
 
@@ -444,6 +500,7 @@ void cscfg_configfs_del_feature(struct cscfg_feature_desc *feat_desc)
 {
 	if (feat_desc->fs_group) {
 		configfs_unregister_group(feat_desc->fs_group);
+		cscfg_destroy_feature_group(feat_desc->fs_group);
 		feat_desc->fs_group = NULL;
 	}
 }
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 2/6] coresight: configfs: Add in functionality for load via configfs
  2022-12-19 23:46 [PATCH v5 0/6] coresight: syscfg: Extend configfs for config load Mike Leach
  2022-12-19 23:46 ` [PATCH v5 1/6] coresight: configfs: Update memory allocation / free for configfs elements Mike Leach
@ 2022-12-19 23:46 ` Mike Leach
  2022-12-19 23:46 ` [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files Mike Leach
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Mike Leach @ 2022-12-19 23:46 UTC (permalink / raw)
  To: linux-arm-kernel, coresight, linux-kernel
  Cc: mathieu.poirier, suzuki.poulose, acme, james.clark, Mike Leach

Add in functionality to allow load via configfs.

Define a binary file format and provide a reader for that format
that will create and populate configuration and feature structures
use by the driver infrastructure.

Adds API to access new functionality.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 drivers/hwtracing/coresight/Makefile          |   2 +-
 .../coresight/coresight-config-file.c         | 520 ++++++++++++++++++
 .../coresight/coresight-config-file.h         | 139 +++++
 .../hwtracing/coresight/coresight-config.h    |  15 +
 .../hwtracing/coresight/coresight-syscfg.h    |   1 +
 5 files changed, 676 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hwtracing/coresight/coresight-config-file.c
 create mode 100644 drivers/hwtracing/coresight/coresight-config-file.h

diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index b6c4a48140ec..5de2bb79f4ac 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -6,7 +6,7 @@ obj-$(CONFIG_CORESIGHT) += coresight.o
 coresight-y := coresight-core.o  coresight-etm-perf.o coresight-platform.o \
 		coresight-sysfs.o coresight-syscfg.o coresight-config.o \
 		coresight-cfg-preload.o coresight-cfg-afdo.o \
-		coresight-syscfg-configfs.o
+		coresight-syscfg-configfs.o coresight-config-file.o
 obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o
 coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \
 		      coresight-tmc-etr.o
diff --git a/drivers/hwtracing/coresight/coresight-config-file.c b/drivers/hwtracing/coresight/coresight-config-file.c
new file mode 100644
index 000000000000..be11261e3a14
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-config-file.c
@@ -0,0 +1,520 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+
+#include "coresight-config.h"
+#include "coresight-config-file.h"
+
+#define cscfg_extract_u64(val64) { \
+	val64 = *(u64 *)(buffer + used); \
+	used += sizeof(u64); \
+	}
+
+#define cscfg_extract_u32(val32) { \
+	val32 = *(u32 *)(buffer + used); \
+	used += sizeof(u32); \
+	}
+
+#define cscfg_extract_u16(val16) { \
+	val16 = *(u16 *)(buffer + used); \
+	used += sizeof(u16); \
+	}
+
+#define cscfg_extract_u8(val8) { \
+	val8 = *(buffer + used); \
+	used++; \
+	}
+
+static int cscfg_file_read_hdr(const u8 *buffer, const int buflen, int *buf_used,
+			       struct cscfg_file_header *hdr)
+{
+	/* file header always at the start of the buffer */
+	int used = 0;
+
+	if (buflen < sizeof(struct cscfg_file_header))
+		return -EINVAL;
+
+	cscfg_extract_u32(hdr->magic_version);
+	if (hdr->magic_version != CSCFG_FILE_MAGIC_VERSION)
+		return -EINVAL;
+
+	cscfg_extract_u16(hdr->length);
+	if (hdr->length > buflen)
+		return -EINVAL;
+
+	cscfg_extract_u16(hdr->nr_configs);
+	cscfg_extract_u16(hdr->nr_features);
+
+	*buf_used = used;
+	return 0;
+}
+
+static int cscfg_file_read_elem_hdr(const u8 *buffer, const int buflen, int *buf_used,
+				    struct cscfg_file_elem_header *elem_hdr)
+{
+	int used = *buf_used;
+
+	if ((buflen - used) < (sizeof(u16) + sizeof(u8)))
+		return -EINVAL;
+
+	/* read length and check enough buffer remains for this element */
+	elem_hdr->elem_length = *(u16 *)(buffer + used);
+	if ((buflen - used) < elem_hdr->elem_length)
+		return -EINVAL;
+	/* don't use extract fn as we update used _after_ the comparison */
+	used += sizeof(u16);
+
+	/* read type and validate */
+	cscfg_extract_u8(elem_hdr->elem_type);
+	if ((elem_hdr->elem_type < CSCFG_FILE_ELEM_TYPE_FEAT) ||
+	    (elem_hdr->elem_type > CSCFG_FILE_ELEM_TYPE_CFG))
+		return -EINVAL;
+
+	*buf_used = used;
+	return 0;
+}
+
+static int cscfg_file_read_elem_str(const u8 *buffer, const int buflen, int *buf_used,
+				    struct cscfg_file_elem_str *elem_str)
+{
+	int used = *buf_used;
+
+	if ((buflen - used) < sizeof(u16))
+		return -EINVAL;
+
+	cscfg_extract_u16(elem_str->str_len);
+
+	if ((buflen - used) < elem_str->str_len)
+		return -EINVAL;
+
+	/* check for 0 termination */
+	if (buffer[used + (elem_str->str_len - 1)] != 0)
+		return -EINVAL;
+
+	elem_str->str = kstrdup((char *)(buffer + used),  GFP_KERNEL);
+	used += elem_str->str_len;
+
+	*buf_used = used;
+	return 0;
+}
+
+static int cscfg_file_alloc_desc_arrays(struct cscfg_fs_load_descs *desc_arrays,
+					int nr_features, int nr_configs)
+{
+	/* arrays are 0 terminated - nr_configs & nr_features elements */
+	desc_arrays->config_descs = kcalloc(nr_configs + 1,  sizeof(struct cscfg_config_desc *),
+					    GFP_KERNEL);
+	if (!desc_arrays->config_descs)
+		return -ENOMEM;
+	desc_arrays->feat_descs = kcalloc(nr_features + 1, sizeof(struct cscfg_feature_desc *),
+					  GFP_KERNEL);
+	if (!desc_arrays->feat_descs)
+		return -ENOMEM;
+	return 0;
+}
+
+/* free up the data allocated to a config desc */
+static void cscfg_file_free_config_desc(struct cscfg_config_desc *config_desc)
+{
+	int i;
+
+	if (!config_desc)
+		return;
+
+	/* free presets */
+	kfree(config_desc->presets);
+
+	/* free feat ref strings */
+	if (config_desc->nr_feat_refs) {
+		/* each string */
+		for (i = 0; i < config_desc->nr_feat_refs; i++)
+			kfree(config_desc->feat_ref_names[i]);
+
+		/* and the char * array */
+		kfree(config_desc->feat_ref_names);
+	}
+
+	/* next the strings */
+	kfree(config_desc->name);
+	kfree(config_desc->description);
+
+	/* finally the struct itself */
+	kfree(config_desc);
+}
+
+static int cscfg_file_read_elem_config(const u8 *buffer, const int buflen, int *buf_used,
+				       struct cscfg_fs_load_descs *desc_arrays,
+				       const int cfg_index)
+{
+	struct cscfg_file_elem_header elem_hdr;
+	struct cscfg_file_elem_str elem_str;
+	struct cscfg_config_desc *config_desc;
+	int used = *buf_used, nr_preset_vals, nr_preset_bytes, i;
+	int err = 0;
+	u64 *presets;
+
+	/*
+	 * read the header - if not config, then don't update buf_used
+	 * pointer on return
+	 */
+	err = cscfg_file_read_elem_hdr(buffer, buflen, &used, &elem_hdr);
+	if (err)
+		return err;
+	if (elem_hdr.elem_type != CSCFG_FILE_ELEM_TYPE_CFG)
+		return 0;
+
+	/* we have a config - allocate the descriptor */
+	config_desc = kzalloc(sizeof(struct cscfg_config_desc), GFP_KERNEL);
+	if (!config_desc)
+		return -ENOMEM;
+
+	/* read the name string */
+	err = cscfg_file_read_elem_str(buffer, buflen, &used, &elem_str);
+	if (err)
+		return err;
+	config_desc->name = elem_str.str;
+
+	/* read the description string */
+	err = cscfg_file_read_elem_str(buffer, buflen, &used, &elem_str);
+	if (err)
+		return err;
+	config_desc->description = elem_str.str;
+
+	/* read in some values */
+	if ((buflen - used) < sizeof(u64))
+		return -EINVAL;
+	cscfg_extract_u16(config_desc->nr_presets);
+	cscfg_extract_u32(config_desc->nr_total_params);
+	cscfg_extract_u16(config_desc->nr_feat_refs);
+
+	/* read the array of 64bit presets if present */
+	nr_preset_vals = config_desc->nr_total_params * config_desc->nr_presets;
+	if (nr_preset_vals) {
+		presets = kcalloc(nr_preset_vals, sizeof(u64), GFP_KERNEL);
+		if (!presets)
+			return -ENOMEM;
+
+		nr_preset_bytes = sizeof(u64) * nr_preset_vals;
+		if ((buflen - used) < nr_preset_bytes)
+			return -EINVAL;
+
+		memcpy(presets, (buffer + used), nr_preset_bytes);
+		config_desc->presets = presets;
+		used += nr_preset_bytes;
+	}
+
+	/* read the array of feature names referenced by the config */
+	if (config_desc->nr_feat_refs) {
+		config_desc->feat_ref_names = kcalloc(config_desc->nr_feat_refs,
+						      sizeof(char *), GFP_KERNEL);
+		if (!config_desc->feat_ref_names)
+			return -ENOMEM;
+
+		for (i = 0; i < config_desc->nr_feat_refs; i++) {
+			err = cscfg_file_read_elem_str(buffer, buflen, &used, &elem_str);
+			if (err)
+				return err;
+			config_desc->feat_ref_names[i] = elem_str.str;
+		}
+	}
+
+	desc_arrays->config_descs[cfg_index] = config_desc;
+	*buf_used = used;
+	return 0;
+}
+
+/*
+ * Read a config name - if there is a config at this position.
+ *
+ * Reads the element header at the current position.
+ * If it is a configuration header (type = CSCFG_FILE_ELEM_TYPE_CFG),
+ * then continue and read the configuration name into @elem_str and update the
+ * @buf_used pointers.
+ *
+ * Otherwise return 0, without updating the used pointers.
+ */
+static int cscfg_file_read_elem_config_name(const u8 *buffer, const int buflen, int *buf_used,
+					    struct cscfg_file_elem_str *elem_str)
+{
+	struct cscfg_file_elem_header elem_hdr;
+	int used = *buf_used;
+	int err;
+
+	elem_str->str_len = 0;
+	/*
+	 * read the header - if not config, then don't update buf_used
+	 * pointer on return
+	 */
+	err = cscfg_file_read_elem_hdr(buffer, buflen, &used, &elem_hdr);
+	if (err)
+		return err;
+	if (elem_hdr.elem_type != CSCFG_FILE_ELEM_TYPE_CFG)
+		return 0;
+
+	/* read the name string */
+	err = cscfg_file_read_elem_str(buffer, buflen, &used, elem_str);
+	if (err)
+		return err;
+	*buf_used = used;
+
+	return 0;
+}
+
+static int cscfg_file_read_elem_param(const u8 *buffer, const int buflen, int *buf_used,
+				      struct cscfg_parameter_desc *param_desc)
+{
+	struct cscfg_file_elem_str elem_str;
+	int err = 0, used = *buf_used;
+
+	/* parameter name */
+	err = cscfg_file_read_elem_str(buffer, buflen, &used, &elem_str);
+	if (err)
+		return err;
+	param_desc->name = elem_str.str;
+
+	/* parameter value */
+	if ((buflen - used) < sizeof(u64))
+		return -EINVAL;
+	cscfg_extract_u64(param_desc->value);
+
+	*buf_used = used;
+	return err;
+}
+
+static void cscfg_file_free_feat_desc(struct cscfg_feature_desc *feat_desc)
+{
+	if (!feat_desc)
+		return;
+
+	/* free up the register descriptor array */
+	kfree(feat_desc->regs_desc);
+
+	/* free up the parameters array */
+	kfree(feat_desc->params_desc);
+
+	/* name and description strings */
+	kfree(feat_desc->name);
+	kfree(feat_desc->description);
+
+	/* finally the struct itself */
+	kfree(feat_desc);
+}
+
+static int cscfg_file_read_elem_feature(const u8 *buffer, const int buflen, int *buf_used,
+					struct cscfg_fs_load_descs *desc_arrays,
+					const int feat_idx)
+{
+	struct cscfg_file_elem_header elem_hdr;
+	struct cscfg_file_elem_str elem_str;
+	struct cscfg_feature_desc *feat_desc;
+	struct cscfg_regval_desc *p_reg_desc;
+	int used = *buf_used, err, i, nr_regs_bytes;
+	u32 val32;
+
+	/* allocate the feature descriptor object */
+	feat_desc = kzalloc(sizeof(struct cscfg_feature_desc), GFP_KERNEL);
+	if (!feat_desc)
+		return -ENOMEM;
+
+	/* read and check the element header */
+	err = cscfg_file_read_elem_hdr(buffer, buflen, &used, &elem_hdr);
+	if (err)
+		return err;
+
+	if (elem_hdr.elem_type != CSCFG_FILE_ELEM_TYPE_FEAT)
+		return -EINVAL;
+
+	/* read the feature name */
+	err = cscfg_file_read_elem_str(buffer, buflen, &used, &elem_str);
+	if (err)
+		return err;
+	feat_desc->name = elem_str.str;
+
+	/* read the description string */
+	err = cscfg_file_read_elem_str(buffer, buflen, &used, &elem_str);
+	if (err)
+		return err;
+	feat_desc->description = elem_str.str;
+
+	/*
+	 * read in some values
+	 * [u32 value: match_flags]
+	 * [u16 value: nr_regs]	    - number of registers.
+	 * [u16 value: nr_params]   - number of parameters.
+	 */
+	cscfg_extract_u32(feat_desc->match_flags);
+	cscfg_extract_u16(feat_desc->nr_regs);
+	cscfg_extract_u16(feat_desc->nr_params);
+
+	/* register descriptors  - 32 bit + 64 bit value */
+	if (feat_desc->nr_regs) {
+		nr_regs_bytes = ((sizeof(u32) + sizeof(u64)) * feat_desc->nr_regs);
+		if ((buflen - used) < nr_regs_bytes)
+			return -EINVAL;
+		feat_desc->regs_desc = kcalloc(feat_desc->nr_regs,
+					       sizeof(struct cscfg_regval_desc), GFP_KERNEL);
+		if (!feat_desc->regs_desc)
+			return -ENOMEM;
+
+		for (i = 0; i < feat_desc->nr_regs; i++) {
+			cscfg_extract_u32(val32);
+			p_reg_desc = (struct cscfg_regval_desc *)&feat_desc->regs_desc[i];
+			CSCFG_FILE_U32_TO_REG_DESC_INFO(val32, p_reg_desc);
+			cscfg_extract_u64(feat_desc->regs_desc[i].val64);
+		}
+	}
+
+	/* parameter descriptors - string + 64 bit value */
+	if (feat_desc->nr_params) {
+		feat_desc->params_desc = kcalloc(feat_desc->nr_params,
+						 sizeof(struct cscfg_parameter_desc), GFP_KERNEL);
+		if (!feat_desc->params_desc)
+			return -ENOMEM;
+		for (i = 0; i < feat_desc->nr_params; i++) {
+			err = cscfg_file_read_elem_param(buffer, buflen, &used,
+							 &feat_desc->params_desc[i]);
+			if (err)
+				return err;
+		}
+	}
+
+	desc_arrays->feat_descs[feat_idx] = feat_desc;
+	*buf_used = used;
+	return 0;
+}
+
+/*
+ * Read a feature name.
+ *
+ * Check the element header to ensure that a feature (type = CSCFG_FILE_ELEM_TYPE_FEAT)
+ * is at this position and read the following string & update @buf_used.
+ *
+ * If not a feature then return -EINVAL.
+ */
+static int cscfg_file_read_elem_feat_name(const u8 *buffer, const int buflen, int *buf_used,
+					  struct cscfg_file_elem_str *elem_str)
+{
+	struct cscfg_file_elem_header elem_hdr;
+	int used = *buf_used;
+	int err;
+
+	elem_str->str_len = 0;
+	/*
+	 * read the header - if not feature, then don't update buf_used
+	 * pointer on return
+	 */
+	err = cscfg_file_read_elem_hdr(buffer, buflen, &used, &elem_hdr);
+	if (err)
+		return err;
+	if (elem_hdr.elem_type != CSCFG_FILE_ELEM_TYPE_FEAT)
+		return -EINVAL;
+
+	/* read the feature name */
+	err = cscfg_file_read_elem_str(buffer, buflen, &used, elem_str);
+	if (err)
+		return err;
+	*buf_used = used;
+
+	return 0;
+}
+
+/*
+ * Read a buffer and create the configuration and feature
+ * descriptors to load into the cscfg system
+ */
+int cscfg_file_read_buffer(const u8 *buffer, const int buflen,
+			   struct cscfg_fs_load_descs *desc_arrays)
+{
+	struct cscfg_file_header hdr;
+	int used = 0, err, i;
+
+	/* read in the file header */
+	err = cscfg_file_read_hdr(buffer, buflen, &used, &hdr);
+	if (err)
+		return err;
+
+	/* allocate the memory for the descriptor pointer arrays */
+	err = cscfg_file_alloc_desc_arrays(desc_arrays, hdr.nr_features, hdr.nr_configs);
+	if (err)
+		return err;
+
+	/* read elements */
+
+	/* first elements are configurations */
+	for (i = 0; i < hdr.nr_configs; i++) {
+		err = cscfg_file_read_elem_config(buffer, buflen, &used, desc_arrays, i);
+		if (err)
+			return err;
+	}
+
+	/* now read and populate all the feature descriptors */
+	for (i = 0; i < hdr.nr_features; i++) {
+		err = cscfg_file_read_elem_feature(buffer, buflen, &used, desc_arrays, i);
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
+int cscfg_file_read_buffer_first_name(const u8 *buffer, const int buflen,
+				      const char **name)
+{
+	struct cscfg_file_header hdr;
+	struct cscfg_file_elem_str elem_str;
+	int used = 0, err = 0;
+
+	*name = NULL;
+
+	/* read in the file header */
+	err = cscfg_file_read_hdr(buffer, buflen, &used, &hdr);
+	if (err)
+		return err;
+
+	err = cscfg_file_read_elem_config_name(buffer, buflen, &used, &elem_str);
+	if (err)
+		return err;
+
+	/* no config string - get first feature name */
+	if (!elem_str.str_len) {
+		err = cscfg_file_read_elem_feat_name(buffer, buflen, &used, &elem_str);
+		if (err)
+			return err;
+	}
+	if (elem_str.str_len)
+		*name = elem_str.str;
+	return err;
+}
+
+/*
+ * Need to free up the dynamically allocated descriptor arrays on unload
+ * as the memory used could be significant if many configurations are loaded
+ * and unloaded while the machine is operational.
+ *
+ * This frees up all the memory allocated by this file during the load process.
+ */
+void cscfg_file_free_load_descs(struct cscfg_fs_load_descs *desc_arrays)
+{
+	int i = 0;
+
+	if (!desc_arrays)
+		return;
+
+	/* free up each of the config descriptors */
+	while (desc_arrays->config_descs[i]) {
+		cscfg_file_free_config_desc(desc_arrays->config_descs[i]);
+		i++;
+	}
+
+	/* free up each of the feature descriptors */
+	i = 0;
+	while (desc_arrays->feat_descs[i]) {
+		cscfg_file_free_feat_desc(desc_arrays->feat_descs[i]);
+		i++;
+	}
+
+	/* finally free up the load descs pointer arrays */
+	kfree(desc_arrays->config_descs);
+	kfree(desc_arrays->feat_descs);
+}
diff --git a/drivers/hwtracing/coresight/coresight-config-file.h b/drivers/hwtracing/coresight/coresight-config-file.h
new file mode 100644
index 000000000000..6e8d259dfaaa
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-config-file.h
@@ -0,0 +1,139 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020-2022 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+
+#ifndef _CORESIGHT_CORESIGHT_CONFIG_FILE_H
+#define _CORESIGHT_CORESIGHT_CONFIG_FILE_H
+
+#include <linux/sizes.h>
+
+#include "coresight-config.h"
+
+/*
+ * Structures to represent configuration descriptors in a memory buffer
+ * to serialise to and from files
+ *
+ * File structure - for loading configuration(s) + feature(s)
+ * from configfs.
+ *
+ * [cscfg_file_header]	- mandatory
+ * [CONFIG_ELEM] * [cscfg_file_header.nr_configs] - optional.
+ * [FEATURE_ELEM] * [cscfg_file_header.nr_features] - optional.
+ *
+ * File valid if it has both config(s) and feature(s), only config(s)
+ * or only feature(s).
+ * Invalid file if no config or features.
+ *
+ * File structure for [CONFIG_ELEM]:
+ *
+ * [cscfg_file_elem_header] - header length value to end of feature strings.
+ * [cscfg_file_elem_str]    - name of the configuration
+ * [cscfg_file_elem_str]    - description of configuration
+ * [u16 value - nr_presets] - number of sets of presets supplied
+ * [u32 value - nr_total_params] - total of all params in referenced features
+ * [u16 value - nr_feat_refs] - number of features selected by this configuration
+ * [u64 values] * (nr_presets * nr_total_params)
+ * [cscfg_file_elem_str] * nr_feat_refs - names of features selected by configuration.
+ *
+ *  A configuration must reference at least one feature.
+ *  Referenced features may be in this file, or have been loaded previously.
+ *
+ * File structure for a [FEATURE_ELEM]
+ *
+ * [cscfg_file_elem_header] - header length is total bytes to end of param structures.
+ * [cscfg_file_elem_str]    - feature name.
+ * [cscfg_file_elem_str]    - feature description.
+ * [u32 value: match_flags]
+ * [u16 value: nr_regs]	    - number of registers.
+ * [u16 value: nr_params]   - number of parameters.
+ * [cscfg_regval_desc struct] * nr_regs
+ * [PARAM_ELEM] * nr_params
+ *
+ * File structure for [PARAM_ELEM]
+ *
+ * [cscfg_file_elem_str]    - parameter name.
+ * [u64 value: param_value] - initial value.
+ */
+
+/* major element types - configurations and features */
+
+#define CSCFG_FILE_ELEM_TYPE_FEAT	0x1
+#define CSCFG_FILE_ELEM_TYPE_CFG	0x2
+
+#define CSCFG_FILE_MAGIC_VERSION	0xC5CF0001
+
+#define CSCFG_FILE_U32_TO_REG_DESC_INFO(val32, p_desc) \
+	{ \
+	p_desc->type = (val32 >> 24) & 0xFF; \
+	p_desc->offset = (val32 >> 12) & 0xFFF; \
+	p_desc->hw_info = val32 & 0xFFF; \
+	}
+
+#define CSCFG_FILE_REG_DESC_INFO_TO_U32(val32, p_desc) \
+	{ \
+	val32 = p_desc->hw_info & 0xFFF; \
+	val32 |= ((p_desc->offset & 0xFFF) << 12); \
+	val32 |= ((p_desc->type & 0xFF) << 24); \
+	}
+
+/*
+ * Binary attributes in configfs need a max size, as an internal buffer is declared,
+ * and will not be exceeded to prevent kernel OOM errors / attacks.
+ *
+ * Use a value that will reasonably cover all the usable & programmable registers in
+ * an ETM, the most complex device we have.
+ */
+#define CSCFG_FILE_MAXSIZE	SZ_16K
+
+/* limit string sizes - used for descriptions and names. */
+#define CSCFG_FILE_STR_MAXSIZE	SZ_1K
+
+/**
+ * file header.
+ *
+ * @magic_version: magic number / version for file/buffer format.
+ * @length       : total length of all data in the buffer.
+ * @nr_configs	 : total number of configs in the buffer.
+ * @nr_features  : total number of features in the buffer.
+ */
+struct cscfg_file_header {
+	u32 magic_version;
+	u16 length;
+	u16 nr_configs;
+	u16 nr_features;
+};
+
+/**
+ * element header
+ *
+ * @elem_length: total length of this element
+ * @elem_type  : type of this element - one of CSCFG_FILE_ELEM_TYPE.. defines.
+ */
+struct cscfg_file_elem_header {
+	u16 elem_length;
+	u8 elem_type;
+};
+
+/**
+ * string file element.
+ *
+ * @str_len: length of string buffer including 0 terminator
+ * @str    : string buffer - 0 terminated.
+ */
+struct cscfg_file_elem_str {
+	u16 str_len;
+	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);
+/* on unload we need to free up memory allocated on read */
+void cscfg_file_free_load_descs(struct cscfg_fs_load_descs *desc_arrays);
+
+#endif /* _CORESIGHT_CORESIGHT_CONFIG_FILE_H */
diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
index 6ba013975741..ad212954f99e 100644
--- a/drivers/hwtracing/coresight/coresight-config.h
+++ b/drivers/hwtracing/coresight/coresight-config.h
@@ -85,6 +85,21 @@ struct cscfg_regval_desc {
 	};
 };
 
+/**
+ * Dynamically loaded descriptor arrays loaded via configfs.
+ *
+ * For builtin or module loaded configurations / features these are
+ * statically defined at compile time. For configfs we create the arrays
+ * dynamically so need a structure to handle this.
+ *
+ * @config_descs:	array of config descriptor pointers.
+ * @feat_descs:		array of feature descriptor pointers.
+ */
+struct cscfg_fs_load_descs {
+	struct cscfg_config_desc **config_descs;
+	struct cscfg_feature_desc **feat_descs;
+};
+
 /**
  * Device feature descriptor - combination of registers and parameters to
  * program a device to implement a specific complex function.
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
index 66e2db890d82..d0c9543850c0 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.h
+++ b/drivers/hwtracing/coresight/coresight-syscfg.h
@@ -79,6 +79,7 @@ struct cscfg_registered_csdev {
 enum cscfg_load_owner_type {
 	CSCFG_OWNER_PRELOAD,
 	CSCFG_OWNER_MODULE,
+	CSCFG_OWNER_CONFIGFS,
 };
 
 /**
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files
  2022-12-19 23:46 [PATCH v5 0/6] coresight: syscfg: Extend configfs for config load Mike Leach
  2022-12-19 23:46 ` [PATCH v5 1/6] coresight: configfs: Update memory allocation / free for configfs elements Mike Leach
  2022-12-19 23:46 ` [PATCH v5 2/6] coresight: configfs: Add in functionality for load via configfs Mike Leach
@ 2022-12-19 23:46 ` Mike Leach
  2022-12-24  7:16   ` Dan Carpenter
  2022-12-27 17:09   ` Christoph Hellwig
  2022-12-19 23:46 ` [PATCH v5 4/6] coresight: configfs: Modify config files to allow userspace use Mike Leach
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 15+ messages in thread
From: Mike Leach @ 2022-12-19 23:46 UTC (permalink / raw)
  To: linux-arm-kernel, coresight, linux-kernel
  Cc: mathieu.poirier, suzuki.poulose, acme, james.clark, Mike Leach

Add in functionality and binary attribute to load and unload
configurations as binary data.

Files are loaded via the 'load' binary attribute. System reads the incoming
file, which must be formatted correctly as defined in the file reader code.
This will create configuration(s) and/or feature(s) and load them
into the system.

These will then appear in configfs, in the 'configurations' and 'features'
directories, ready for use.

A mutex is used to prevent load and unload operations from happening
simultaneously. Further, a flag enabling configfs load/unload is
provided, along with API calls to allow this functionality to be
controlled during start-up and shut-down, and when configurations
are loaded via loadable modules.

This ensures that load/unload operations are strictly serialised.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 .../coresight/coresight-syscfg-configfs.c     | 242 +++++++++++++++++-
 .../coresight/coresight-syscfg-configfs.h     |   4 +
 .../hwtracing/coresight/coresight-syscfg.c    | 137 +++++++++-
 .../hwtracing/coresight/coresight-syscfg.h    |   5 +-
 4 files changed, 374 insertions(+), 14 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
index 6e8c8db52d39..47b3638d8f23 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
@@ -5,10 +5,96 @@
  */
 
 #include <linux/configfs.h>
+#include <linux/module.h>
+#include <linux/workqueue.h>
 
 #include "coresight-config.h"
+#include "coresight-config-file.h"
 #include "coresight-syscfg-configfs.h"
 
+/* prevent race in load / unload operations */
+static DEFINE_MUTEX(cfs_mutex);
+
+/*
+ * need to enable / disable load via configfs when
+ * initialising / shutting down the subsystem, or
+ * loading / unloading module.
+ */
+static bool cscfg_fs_load_enabled;
+
+/*
+ * Lockdep issues occur if deleting the config directory as part
+ * of the unload write operation. Therefore we schedule the main
+ * part of the unload to be completed as a work item
+ */
+struct cscfg_load_owner_info *unload_owner_info;
+
+/* complete the unload operation */
+static void cscfg_complete_unload(struct work_struct *work)
+{
+	mutex_lock(&cfs_mutex);
+
+	if (!cscfg_fs_load_enabled || !unload_owner_info) {
+		pr_warn("cscfg: skipping unload completion\n");
+		goto exit_unlock;
+	}
+
+	if (!cscfg_unload_config_sets(unload_owner_info))
+		cscfg_configfs_free_owner_info(unload_owner_info);
+	else
+		pr_err("cscfg: configfs configuration unload error\n");
+	unload_owner_info = NULL;
+
+exit_unlock:
+	mutex_unlock(&cfs_mutex);
+	kfree(work);
+}
+
+static int cscfg_schedule_unload(struct cscfg_load_owner_info *owner_info, const char *name)
+{
+	struct work_struct *work;
+	int err;
+
+	work = kzalloc(sizeof(struct work_struct), GFP_KERNEL);
+	if (!work)
+		return -ENOMEM;
+
+	/* set cscfg state as starting an unload operation */
+	err = cscfg_set_unload_start();
+	if (err) {
+		pr_err("Config unload %s: failed to set unload start flag\n", name);
+		kfree(work);
+		return err;
+	}
+
+	INIT_WORK(work, cscfg_complete_unload);
+	unload_owner_info = owner_info;
+	schedule_work(work);
+	return 0;
+}
+
+void cscfg_configfs_enable_fs_load(void)
+{
+	mutex_lock(&cfs_mutex);
+	cscfg_fs_load_enabled = true;
+	mutex_unlock(&cfs_mutex);
+}
+
+void cscfg_configfs_disable_fs_load(void)
+{
+	mutex_lock(&cfs_mutex);
+	cscfg_fs_load_enabled = false;
+	mutex_unlock(&cfs_mutex);
+}
+
+void cscfg_configfs_at_exit(void)
+{
+	mutex_lock(&cfs_mutex);
+	cscfg_fs_load_enabled = false;
+	unload_owner_info = NULL;
+	mutex_unlock(&cfs_mutex);
+}
+
 /* create a default ci_type. */
 static inline struct config_item_type *cscfg_create_ci_type(void)
 {
@@ -431,14 +517,154 @@ static void cscfg_destroy_feature_group(struct config_group *feat_group)
 	kfree(feat_view);
 }
 
-static struct config_item_type cscfg_configs_type = {
+/* Attributes in configfs that allow load and unload of configuration binary files */
+
+/* free memory associated with a configfs loaded configuration file & descriptors */
+void cscfg_configfs_free_owner_info(struct cscfg_load_owner_info *owner_info)
+{
+	struct cscfg_fs_load_descs *load_descs = 0;
+
+	if (!owner_info)
+		return;
+
+	load_descs = (struct cscfg_fs_load_descs *)(owner_info->owner_handle);
+
+	if (load_descs) {
+		/* free the data allocated on file load, pointed to by load_descs */
+		cscfg_file_free_load_descs(load_descs);
+		kfree(load_descs);
+	}
+
+	kfree(owner_info);
+}
+
+
+/* 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;
+
+	/* ensure we cannot simultaneously load and unload */
+	if (!mutex_trylock(&cfs_mutex))
+		return -EBUSY;
+
+	/* check configfs load / unload ops are permitted */
+	if (!cscfg_fs_load_enabled || unload_owner_info) {
+		err = -EBUSY;
+		goto exit_unlock;
+	}
+
+	if (size > CSCFG_FILE_MAXSIZE) {
+		pr_err("cscfg: Load error - Input file too large.\n");
+		err = -EINVAL;
+		goto exit_unlock;
+	}
+
+	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;
+	}
+
+	mutex_unlock(&cfs_mutex);
+	return size;
+
+exit_memfree:
+	cscfg_configfs_free_owner_info(owner_info);
+
+exit_unlock:
+	mutex_unlock(&cfs_mutex);
+	return err;
+}
+CONFIGFS_BIN_ATTR_WO(cscfg_cfg_, load, NULL, CSCFG_FILE_MAXSIZE);
+
+/* read "buffer" and schedule named configuration for unload */
+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 = 0;
+
+	/* ensure we cannot simultaneously load and unload */
+	if (!mutex_trylock(&cfs_mutex))
+		return -EBUSY;
+
+	/* check configfs load / unload ops are permitted & no ongoing unload */
+	if (!cscfg_fs_load_enabled || unload_owner_info) {
+		err = -EBUSY;
+		goto exit_unlock;
+	}
+
+	if (size > CSCFG_FILE_MAXSIZE) {
+		pr_err("cscfg: Unload error - Input file too large\n");
+		err = -EINVAL;
+		goto exit_unlock;
+	}
+
+	err = cscfg_file_read_buffer_first_name(buffer, size, &name);
+	if (err) {
+		pr_err("cscfg: Unload error - Failed to read input file\n");
+		goto exit_unlock;
+	}
+
+	owner_info = cscfg_find_fs_owned_cfg_by_name(name);
+	if (!owner_info) {
+		pr_err("cscfg: Unload error: Failed to find %s from input file\n",
+		       name);
+		goto exit_unlock;
+	}
+
+	/* actual unload is scheduled as a work item */
+	err = cscfg_schedule_unload(owner_info, name);
+	if (err)
+		goto exit_unlock;
+
+	err = size;
+
+exit_unlock:
+	mutex_unlock(&cfs_mutex);
+	return err;
+}
+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_load_type = {
 	.ct_owner = THIS_MODULE,
+	.ct_bin_attrs = cscfg_config_configfs_bin_attrs,
 };
 
+static struct config_item_type cscfg_configs_grp_type = {
+	.ct_owner = THIS_MODULE,
+};
+
+/* group for configurations dir */
 static struct config_group cscfg_configs_grp = {
 	.cg_item = {
 		.ci_namebuf = "configurations",
-		.ci_type = &cscfg_configs_type,
+		.ci_type = &cscfg_configs_grp_type,
 	},
 };
 
@@ -508,18 +734,20 @@ void cscfg_configfs_del_feature(struct cscfg_feature_desc *feat_desc)
 int cscfg_configfs_init(struct cscfg_manager *cscfg_mgr)
 {
 	struct configfs_subsystem *subsys;
-	struct config_item_type *ci_type;
 
 	if (!cscfg_mgr)
 		return -EINVAL;
 
-	ci_type = cscfg_create_ci_type();
-	if (!ci_type)
-		return -ENOMEM;
+	/* load and unload by configfs initially disabled */
+	cscfg_fs_load_enabled = false;
+
+	/* no current unload operation in progress */
+	unload_owner_info = NULL;
 
+	/* init subsystem group - with load and unload attributes */
 	subsys = &cscfg_mgr->cfgfs_subsys;
 	config_item_set_name(&subsys->su_group.cg_item, CSCFG_FS_SUBSYS_NAME);
-	subsys->su_group.cg_item.ci_type = ci_type;
+	subsys->su_group.cg_item.ci_type = &cscfg_configs_load_type;
 
 	config_group_init(&subsys->su_group);
 	mutex_init(&subsys->su_mutex);
diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
index 373d84d43268..7f5610c1895e 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
+++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
@@ -45,5 +45,9 @@ int cscfg_configfs_add_config(struct cscfg_config_desc *config_desc);
 int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc);
 void cscfg_configfs_del_config(struct cscfg_config_desc *config_desc);
 void cscfg_configfs_del_feature(struct cscfg_feature_desc *feat_desc);
+void cscfg_configfs_free_owner_info(struct cscfg_load_owner_info *owner_info);
+void cscfg_configfs_enable_fs_load(void);
+void cscfg_configfs_disable_fs_load(void);
+void cscfg_configfs_at_exit(void);
 
 #endif /* CORESIGHT_SYSCFG_CONFIGFS_H */
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
index 11138a9762b0..aa03e991cd9d 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg.c
@@ -554,6 +554,22 @@ static int cscfg_fs_register_cfgs_feats(struct cscfg_config_desc **config_descs,
 	return 0;
 }
 
+/*
+ * check owner info and if module owner, disable / enable
+ * configfs load ops.
+ */
+static void cscfg_check_disable_fs_load(struct cscfg_load_owner_info *owner_info)
+{
+	if (owner_info->type == CSCFG_OWNER_MODULE)
+		cscfg_configfs_disable_fs_load();
+}
+
+static void cscfg_check_enable_fs_load(struct cscfg_load_owner_info *owner_info)
+{
+	if (owner_info->type == CSCFG_OWNER_MODULE)
+		cscfg_configfs_enable_fs_load();
+}
+
 /**
  * cscfg_load_config_sets - API function to load feature and config sets.
  *
@@ -578,10 +594,13 @@ int cscfg_load_config_sets(struct cscfg_config_desc **config_descs,
 {
 	int err = 0;
 
+	/* if this load is by module owner, need to disable configfs load/unload */
+	cscfg_check_disable_fs_load(owner_info);
+
 	mutex_lock(&cscfg_mutex);
 	if (cscfg_mgr->load_state != CSCFG_NONE) {
-		mutex_unlock(&cscfg_mutex);
-		return -EBUSY;
+		err = -EBUSY;
+		goto exit_unlock;
 	}
 	cscfg_mgr->load_state = CSCFG_LOAD;
 
@@ -616,7 +635,7 @@ int cscfg_load_config_sets(struct cscfg_config_desc **config_descs,
 
 	/* mark any new configs as available for activation */
 	cscfg_set_configs_available(config_descs);
-	goto exit_unlock;
+	goto exit_clear_state;
 
 err_clean_cfs:
 	/* cleanup after error registering with configfs */
@@ -631,9 +650,13 @@ int cscfg_load_config_sets(struct cscfg_config_desc **config_descs,
 err_clean_load:
 	cscfg_unload_owned_cfgs_feats(owner_info);
 
-exit_unlock:
+exit_clear_state:
 	cscfg_mgr->load_state = CSCFG_NONE;
+
+exit_unlock:
 	mutex_unlock(&cscfg_mutex);
+
+	cscfg_check_enable_fs_load(owner_info);
 	return err;
 }
 EXPORT_SYMBOL_GPL(cscfg_load_config_sets);
@@ -659,8 +682,12 @@ int cscfg_unload_config_sets(struct cscfg_load_owner_info *owner_info)
 	int err = 0;
 	struct cscfg_load_owner_info *load_list_item = NULL;
 
+	/* if this unload is by module owner, need to disable configfs load/unload */
+	cscfg_check_disable_fs_load(owner_info);
+
 	mutex_lock(&cscfg_mutex);
-	if (cscfg_mgr->load_state != CSCFG_NONE) {
+	if ((cscfg_mgr->load_state != CSCFG_NONE) &&
+	    (cscfg_mgr->load_state != CSCFG_UNLOAD_START)) {
 		mutex_unlock(&cscfg_mutex);
 		return -EBUSY;
 	}
@@ -705,10 +732,80 @@ int cscfg_unload_config_sets(struct cscfg_load_owner_info *owner_info)
 exit_unlock:
 	cscfg_mgr->load_state = CSCFG_NONE;
 	mutex_unlock(&cscfg_mutex);
+
+	cscfg_check_enable_fs_load(owner_info);
 	return err;
 }
 EXPORT_SYMBOL_GPL(cscfg_unload_config_sets);
 
+int cscfg_set_unload_start(void)
+{
+	int ret = 0;
+
+	mutex_lock(&cscfg_mutex);
+	if (cscfg_mgr->load_state != CSCFG_NONE)
+		ret = -EBUSY;
+	else
+		cscfg_mgr->load_state = CSCFG_UNLOAD_START;
+	mutex_unlock(&cscfg_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cscfg_set_unload_start);
+
+/* 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;
+}
+
+/* get configfs config load name from configfs type owner */
+static const char *cscfg_find_load_name_fs_owned_cfg(struct cscfg_load_owner_info *owner_info)
+{
+	struct cscfg_fs_load_descs *fs_load_cfg;
+	const char *name = "unknown";
+
+	if (owner_info->type == CSCFG_OWNER_CONFIGFS) {
+		fs_load_cfg = (struct cscfg_fs_load_descs *)owner_info->owner_handle;
+		if (fs_load_cfg->config_descs[0])
+			name = fs_load_cfg->config_descs[0]->name;
+		else if (fs_load_cfg->feat_descs[0])
+			name = fs_load_cfg->feat_descs[0]->name;
+	}
+	return name;
+}
+
 /* Handle coresight device registration and add configs and features to devices */
 
 /* iterate through config lists and load matching configs to device */
@@ -881,7 +978,7 @@ static int _cscfg_activate_config(unsigned long cfg_hash)
 	struct cscfg_config_desc *config_desc;
 	int err = -EINVAL;
 
-	if (cscfg_mgr->load_state == CSCFG_UNLOAD)
+	if (cscfg_mgr->load_state >= CSCFG_UNLOAD)
 		return -EBUSY;
 
 	list_for_each_entry(config_desc, &cscfg_mgr->config_desc_list, item) {
@@ -1206,6 +1303,7 @@ static int cscfg_create_device(void)
 static void cscfg_unload_cfgs_on_exit(void)
 {
 	struct cscfg_load_owner_info *owner_info = NULL;
+	bool free_configfs_owner = false;
 
 	/*
 	 * grab the mutex - even though we are exiting, some configfs files
@@ -1240,6 +1338,23 @@ static void cscfg_unload_cfgs_on_exit(void)
 			 */
 			pr_err("cscfg: ERROR: prior module failed to unload configuration\n");
 			goto list_remove;
+
+		case CSCFG_OWNER_CONFIGFS:
+			/*
+			 * configfs loaded items may still be present if the user did not
+			 * unload them during the session. These have dynamically allocated
+			 * descriptor tables (unlike the two types above that are statically
+			 * allocated at compile time)
+			 */
+			pr_info("cscfg: unloading configfs loaded configuration %s\n",
+				cscfg_find_load_name_fs_owned_cfg(owner_info));
+
+			/*
+			 * as this is not being unloaded by configfs, need to flag the
+			 * requirement to free up the descriptors.
+			 */
+			free_configfs_owner = true;
+			break;
 		}
 
 		/* remove from configfs - outside the scope of the list mutex */
@@ -1253,6 +1368,12 @@ static void cscfg_unload_cfgs_on_exit(void)
 list_remove:
 		/* remove from load order list */
 		list_del(&owner_info->item);
+
+		/* configfs owned dynamic loaded config, free memory now */
+		if (free_configfs_owner) {
+			cscfg_configfs_free_owner_info(owner_info);
+			free_configfs_owner = false;
+		}
 	}
 	mutex_unlock(&cscfg_mutex);
 }
@@ -1284,6 +1405,9 @@ int __init cscfg_init(void)
 	if (err)
 		goto exit_err;
 
+	/* can now allow load / unload from configfs */
+	cscfg_configfs_enable_fs_load();
+
 	dev_info(cscfg_device(), "CoreSight Configuration manager initialised");
 	return 0;
 
@@ -1294,5 +1418,6 @@ int __init cscfg_init(void)
 
 void cscfg_exit(void)
 {
+	cscfg_configfs_at_exit();
 	cscfg_clear_device();
 }
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
index d0c9543850c0..73353080810d 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.h
+++ b/drivers/hwtracing/coresight/coresight-syscfg.h
@@ -20,7 +20,8 @@
 enum cscfg_load_ops {
 	CSCFG_NONE,
 	CSCFG_LOAD,
-	CSCFG_UNLOAD
+	CSCFG_UNLOAD,
+	CSCFG_UNLOAD_START, /* unload started by fs, will be completed later */
 };
 
 /**
@@ -108,6 +109,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,
@@ -124,5 +126,6 @@ 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);
+int cscfg_set_unload_start(void);
 
 #endif /* CORESIGHT_SYSCFG_H */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 4/6] coresight: configfs: Modify config files to allow userspace use
  2022-12-19 23:46 [PATCH v5 0/6] coresight: syscfg: Extend configfs for config load Mike Leach
                   ` (2 preceding siblings ...)
  2022-12-19 23:46 ` [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files Mike Leach
@ 2022-12-19 23:46 ` Mike Leach
  2022-12-27 17:10   ` Christoph Hellwig
  2022-12-19 23:46 ` [PATCH v5 5/6] coresight: tools: Add config file write and reader tools Mike Leach
  2022-12-19 23:46 ` [PATCH v5 6/6] Documentation: coresight: docs for config load via configfs Mike Leach
  5 siblings, 1 reply; 15+ messages in thread
From: Mike Leach @ 2022-12-19 23:46 UTC (permalink / raw)
  To: linux-arm-kernel, coresight, linux-kernel
  Cc: mathieu.poirier, suzuki.poulose, acme, james.clark, Mike Leach

Update coresight-config.h and the coresight-config-file.c & .h
to allow use in userspace programs.

Use __KERNEL__ defines to filter out driver only structures and
elements so that user space programs can use the descriptor structures.

Abstract memory allocation in coresight-config-file.c to allow read
file functions to be run in userspace and kernel drivers.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 .../coresight/coresight-config-file.c         | 117 +++++++++++++-----
 .../hwtracing/coresight/coresight-config.h    |  12 ++
 2 files changed, 101 insertions(+), 28 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-config-file.c b/drivers/hwtracing/coresight/coresight-config-file.c
index be11261e3a14..cf9f178eef97 100644
--- a/drivers/hwtracing/coresight/coresight-config-file.c
+++ b/drivers/hwtracing/coresight/coresight-config-file.c
@@ -7,6 +7,63 @@
 #include "coresight-config.h"
 #include "coresight-config-file.h"
 
+/*
+ * To allow reuse of this source in tools, define memory allocation fns according
+ * to build environment.
+ */
+
+#ifdef __KERNEL__
+
+static void *cscfg_calloc(size_t num, size_t size)
+{
+	return kcalloc(num, size, GFP_KERNEL);
+}
+
+static char *cscfg_strdup(const char *str)
+{
+	return kstrdup(str, GFP_KERNEL);
+}
+
+static void *cscfg_zalloc(size_t size)
+{
+	return kzalloc(size, GFP_KERNEL);
+}
+
+static void cscfg_free(void *mem)
+{
+	kfree(mem);
+}
+#else
+
+#include <stddef.h>
+#include <string.h>
+#include <stdlib.h>
+
+static void *cscfg_calloc(size_t num, size_t size)
+{
+	return calloc(num, size);
+}
+
+static char *cscfg_strdup(const char *str)
+{
+	return strdup(str);
+}
+
+static void *cscfg_zalloc(size_t size)
+{
+	void *ptr = malloc(size);
+
+	if (ptr)
+		memset(ptr, 0, size);
+	return ptr;
+}
+
+static void cscfg_free(void *mem)
+{
+	free(mem);
+}
+#endif
+
 #define cscfg_extract_u64(val64) { \
 	val64 = *(u64 *)(buffer + used); \
 	used += sizeof(u64); \
@@ -80,6 +137,7 @@ static int cscfg_file_read_elem_str(const u8 *buffer, const int buflen, int *buf
 				    struct cscfg_file_elem_str *elem_str)
 {
 	int used = *buf_used;
+	const u8 *str;
 
 	if ((buflen - used) < sizeof(u16))
 		return -EINVAL;
@@ -89,11 +147,13 @@ static int cscfg_file_read_elem_str(const u8 *buffer, const int buflen, int *buf
 	if ((buflen - used) < elem_str->str_len)
 		return -EINVAL;
 
+	str = buffer + used;
+
 	/* check for 0 termination */
-	if (buffer[used + (elem_str->str_len - 1)] != 0)
+	if (str[elem_str->str_len - 1] != 0)
 		return -EINVAL;
 
-	elem_str->str = kstrdup((char *)(buffer + used),  GFP_KERNEL);
+	elem_str->str = cscfg_strdup((char *)str);
 	used += elem_str->str_len;
 
 	*buf_used = used;
@@ -104,12 +164,13 @@ static int cscfg_file_alloc_desc_arrays(struct cscfg_fs_load_descs *desc_arrays,
 					int nr_features, int nr_configs)
 {
 	/* arrays are 0 terminated - nr_configs & nr_features elements */
-	desc_arrays->config_descs = kcalloc(nr_configs + 1,  sizeof(struct cscfg_config_desc *),
-					    GFP_KERNEL);
+	desc_arrays->config_descs = cscfg_calloc(nr_configs + 1,
+						 sizeof(struct cscfg_config_desc *));
 	if (!desc_arrays->config_descs)
 		return -ENOMEM;
-	desc_arrays->feat_descs = kcalloc(nr_features + 1, sizeof(struct cscfg_feature_desc *),
-					  GFP_KERNEL);
+
+	desc_arrays->feat_descs = cscfg_calloc(nr_features + 1,
+					       sizeof(struct cscfg_feature_desc *));
 	if (!desc_arrays->feat_descs)
 		return -ENOMEM;
 	return 0;
@@ -124,24 +185,24 @@ static void cscfg_file_free_config_desc(struct cscfg_config_desc *config_desc)
 		return;
 
 	/* free presets */
-	kfree(config_desc->presets);
+	cscfg_free((void *)config_desc->presets);
 
 	/* free feat ref strings */
 	if (config_desc->nr_feat_refs) {
 		/* each string */
 		for (i = 0; i < config_desc->nr_feat_refs; i++)
-			kfree(config_desc->feat_ref_names[i]);
+			cscfg_free((void *)config_desc->feat_ref_names[i]);
 
 		/* and the char * array */
-		kfree(config_desc->feat_ref_names);
+		cscfg_free((void *)config_desc->feat_ref_names);
 	}
 
 	/* next the strings */
-	kfree(config_desc->name);
-	kfree(config_desc->description);
+	cscfg_free((void *)config_desc->name);
+	cscfg_free((void *)config_desc->description);
 
 	/* finally the struct itself */
-	kfree(config_desc);
+	cscfg_free((void *)config_desc);
 }
 
 static int cscfg_file_read_elem_config(const u8 *buffer, const int buflen, int *buf_used,
@@ -166,7 +227,7 @@ static int cscfg_file_read_elem_config(const u8 *buffer, const int buflen, int *
 		return 0;
 
 	/* we have a config - allocate the descriptor */
-	config_desc = kzalloc(sizeof(struct cscfg_config_desc), GFP_KERNEL);
+	config_desc = cscfg_zalloc(sizeof(struct cscfg_config_desc));
 	if (!config_desc)
 		return -ENOMEM;
 
@@ -192,7 +253,7 @@ static int cscfg_file_read_elem_config(const u8 *buffer, const int buflen, int *
 	/* read the array of 64bit presets if present */
 	nr_preset_vals = config_desc->nr_total_params * config_desc->nr_presets;
 	if (nr_preset_vals) {
-		presets = kcalloc(nr_preset_vals, sizeof(u64), GFP_KERNEL);
+		presets = cscfg_calloc(nr_preset_vals, sizeof(u64));
 		if (!presets)
 			return -ENOMEM;
 
@@ -207,8 +268,8 @@ static int cscfg_file_read_elem_config(const u8 *buffer, const int buflen, int *
 
 	/* read the array of feature names referenced by the config */
 	if (config_desc->nr_feat_refs) {
-		config_desc->feat_ref_names = kcalloc(config_desc->nr_feat_refs,
-						      sizeof(char *), GFP_KERNEL);
+		config_desc->feat_ref_names = cscfg_calloc(config_desc->nr_feat_refs,
+							   sizeof(char *));
 		if (!config_desc->feat_ref_names)
 			return -ENOMEM;
 
@@ -289,17 +350,17 @@ static void cscfg_file_free_feat_desc(struct cscfg_feature_desc *feat_desc)
 		return;
 
 	/* free up the register descriptor array */
-	kfree(feat_desc->regs_desc);
+	cscfg_free((void *)feat_desc->regs_desc);
 
 	/* free up the parameters array */
-	kfree(feat_desc->params_desc);
+	cscfg_free((void *)feat_desc->params_desc);
 
 	/* name and description strings */
-	kfree(feat_desc->name);
-	kfree(feat_desc->description);
+	cscfg_free((void *)feat_desc->name);
+	cscfg_free((void *)feat_desc->description);
 
 	/* finally the struct itself */
-	kfree(feat_desc);
+	cscfg_free((void *)feat_desc);
 }
 
 static int cscfg_file_read_elem_feature(const u8 *buffer, const int buflen, int *buf_used,
@@ -314,7 +375,7 @@ static int cscfg_file_read_elem_feature(const u8 *buffer, const int buflen, int
 	u32 val32;
 
 	/* allocate the feature descriptor object */
-	feat_desc = kzalloc(sizeof(struct cscfg_feature_desc), GFP_KERNEL);
+	feat_desc = cscfg_zalloc(sizeof(struct cscfg_feature_desc));
 	if (!feat_desc)
 		return -ENOMEM;
 
@@ -353,8 +414,8 @@ static int cscfg_file_read_elem_feature(const u8 *buffer, const int buflen, int
 		nr_regs_bytes = ((sizeof(u32) + sizeof(u64)) * feat_desc->nr_regs);
 		if ((buflen - used) < nr_regs_bytes)
 			return -EINVAL;
-		feat_desc->regs_desc = kcalloc(feat_desc->nr_regs,
-					       sizeof(struct cscfg_regval_desc), GFP_KERNEL);
+		feat_desc->regs_desc = cscfg_calloc(feat_desc->nr_regs,
+						    sizeof(struct cscfg_regval_desc));
 		if (!feat_desc->regs_desc)
 			return -ENOMEM;
 
@@ -368,8 +429,8 @@ static int cscfg_file_read_elem_feature(const u8 *buffer, const int buflen, int
 
 	/* parameter descriptors - string + 64 bit value */
 	if (feat_desc->nr_params) {
-		feat_desc->params_desc = kcalloc(feat_desc->nr_params,
-						 sizeof(struct cscfg_parameter_desc), GFP_KERNEL);
+		feat_desc->params_desc = cscfg_calloc(feat_desc->nr_params,
+						      sizeof(struct cscfg_parameter_desc));
 		if (!feat_desc->params_desc)
 			return -ENOMEM;
 		for (i = 0; i < feat_desc->nr_params; i++) {
@@ -515,6 +576,6 @@ void cscfg_file_free_load_descs(struct cscfg_fs_load_descs *desc_arrays)
 	}
 
 	/* finally free up the load descs pointer arrays */
-	kfree(desc_arrays->config_descs);
-	kfree(desc_arrays->feat_descs);
+	cscfg_free(desc_arrays->config_descs);
+	cscfg_free(desc_arrays->feat_descs);
 }
diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
index ad212954f99e..58d4f179ff0d 100644
--- a/drivers/hwtracing/coresight/coresight-config.h
+++ b/drivers/hwtracing/coresight/coresight-config.h
@@ -7,7 +7,14 @@
 #ifndef _CORESIGHT_CORESIGHT_CONFIG_H
 #define _CORESIGHT_CORESIGHT_CONFIG_H
 
+/*
+ * Filter out kernel only portions of the file to allow user space programs
+ * to use the descriptor definitions.
+ */
+#ifdef __KERNEL__
 #include <linux/coresight.h>
+#endif
+
 #include <linux/types.h>
 
 /* CoreSight Configuration Management - component and system wide configuration */
@@ -100,6 +107,10 @@ struct cscfg_fs_load_descs {
 	struct cscfg_feature_desc **feat_descs;
 };
 
+
+/* remainder of header is used by the kernel drivers only */
+#ifdef __KERNEL__
+
 /**
  * Device feature descriptor - combination of registers and parameters to
  * program a device to implement a specific complex function.
@@ -274,4 +285,5 @@ void cscfg_csdev_disable_config(struct cscfg_config_csdev *config_csdev);
 /* reset a feature to default values */
 void cscfg_reset_feat(struct cscfg_feature_csdev *feat_csdev);
 
+#endif /* __KERNEL__ */
 #endif /* _CORESIGHT_CORESIGHT_CONFIG_H */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 5/6] coresight: tools: Add config file write and reader tools
  2022-12-19 23:46 [PATCH v5 0/6] coresight: syscfg: Extend configfs for config load Mike Leach
                   ` (3 preceding siblings ...)
  2022-12-19 23:46 ` [PATCH v5 4/6] coresight: configfs: Modify config files to allow userspace use Mike Leach
@ 2022-12-19 23:46 ` Mike Leach
  2022-12-19 23:46 ` [PATCH v5 6/6] Documentation: coresight: docs for config load via configfs Mike Leach
  5 siblings, 0 replies; 15+ messages in thread
From: Mike Leach @ 2022-12-19 23:46 UTC (permalink / raw)
  To: linux-arm-kernel, coresight, linux-kernel
  Cc: mathieu.poirier, suzuki.poulose, acme, james.clark, Mike Leach

Add an example file generator to test loading configurations via a
binary attribute in configfs.

Provides a file buffer writer function that can be re-used in other
userspace programs.

Buffer write format matches that expected by the corresponding reader
in the configfs driver code.

Add a config file reader and printer. Takes in config files and prints
the contents. Uses file reader source from kernel driver.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 MAINTAINERS                                   |   1 +
 .../coresight/coresight-config-file.c         |   2 +
 tools/coresight/Makefile                      |  52 +++
 tools/coresight/coresight-cfg-bufw.c          | 309 ++++++++++++++++++
 tools/coresight/coresight-cfg-bufw.h          |  26 ++
 tools/coresight/coresight-cfg-example1.c      |  62 ++++
 tools/coresight/coresight-cfg-example2.c      |  95 ++++++
 tools/coresight/coresight-cfg-examples.h      |  22 ++
 tools/coresight/coresight-cfg-file-gen.c      |  61 ++++
 tools/coresight/coresight-cfg-file-read.c     | 227 +++++++++++++
 tools/coresight/coresight-config-uapi.h       |  76 +++++
 11 files changed, 933 insertions(+)
 create mode 100644 tools/coresight/Makefile
 create mode 100644 tools/coresight/coresight-cfg-bufw.c
 create mode 100644 tools/coresight/coresight-cfg-bufw.h
 create mode 100644 tools/coresight/coresight-cfg-example1.c
 create mode 100644 tools/coresight/coresight-cfg-example2.c
 create mode 100644 tools/coresight/coresight-cfg-examples.h
 create mode 100644 tools/coresight/coresight-cfg-file-gen.c
 create mode 100644 tools/coresight/coresight-cfg-file-read.c
 create mode 100644 tools/coresight/coresight-config-uapi.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 379945f82a64..ed90b86dd5c5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2067,6 +2067,7 @@ F:	drivers/hwtracing/coresight/*
 F:	include/dt-bindings/arm/coresight-cti-dt.h
 F:	include/linux/coresight*
 F:	samples/coresight/*
+F:	tools/coresight/*
 F:	tools/perf/tests/shell/coresight/*
 F:	tools/perf/arch/arm/util/auxtrace.c
 F:	tools/perf/arch/arm/util/cs-etm.c
diff --git a/drivers/hwtracing/coresight/coresight-config-file.c b/drivers/hwtracing/coresight/coresight-config-file.c
index cf9f178eef97..43aaa44e86d5 100644
--- a/drivers/hwtracing/coresight/coresight-config-file.c
+++ b/drivers/hwtracing/coresight/coresight-config-file.c
@@ -39,6 +39,8 @@ static void cscfg_free(void *mem)
 #include <string.h>
 #include <stdlib.h>
 
+#include "coresight-config-uapi.h"
+
 static void *cscfg_calloc(size_t num, size_t size)
 {
 	return calloc(num, size);
diff --git a/tools/coresight/Makefile b/tools/coresight/Makefile
new file mode 100644
index 000000000000..584e2271b0f5
--- /dev/null
+++ b/tools/coresight/Makefile
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: GPL-2.0-only
+include ../scripts/Makefile.include
+include ../scripts/Makefile.arch
+
+# Makefile to build the coresight configuration file reader and generator tools
+
+this-makefile := $(lastword $(MAKEFILE_LIST))
+tools-src := $(realpath $(dir $(this-makefile)))
+srctree := $(realpath $(dir $(tools-src)/../../.))
+
+# ensure we use all as the default - skip anything in included Makefile
+.DEFAULT_GOAL = all
+# MAKECMDGOALS isn't set if there's no explicit goal in the
+# command line, so set the default.
+MAKECMDGOALS ?= $(.DEFAULT_GOAL)
+
+# compile flags
+CFLAGS += $(CPPFLAGS) -c -Wall -DLINUX -Wno-switch -Wlogical-op -fPIC -I$(srctree)/drivers/hwtracing/coresight -I$(srctree)/tools/include/  -I$(srctree)/tools/include/uapi
+
+# object files
+coresight-cfg-file-gen-objs := coresight-cfg-file-gen.o coresight-cfg-bufw.o \
+				coresight-cfg-example1.o coresight-cfg-example2.o
+coresight-cfg-file-read-objs := coresight-cfg-file-read.o coresight-config-file.o
+
+# debug variant
+ifdef DEBUG
+CFLAGS += -g -O0 -DDEBUG
+else
+CFLAGS += -O2 -DNDEBUG
+endif
+
+.PHONY: all
+all:  coresight-cfg-file-gen coresight-cfg-file-read
+
+coresight-config-file.o: src_copy
+	$(CC) $(CFLAGS) coresight-config-file.c -o coresight-config-file.o
+
+.PHONY: src_copy
+src_copy:
+	@cp $(srctree)/drivers/hwtracing/coresight/coresight-config-file.c $(srctree)/tools/coresight/.
+
+coresight-cfg-file-gen: $(coresight-cfg-file-gen-objs)
+	$(CC) $(LDFLAGS) $(coresight-cfg-file-gen-objs) -o coresight-cfg-file-gen
+
+coresight-cfg-file-read: $(coresight-cfg-file-read-objs)
+	$(CC) $(LDFLAGS) $(coresight-cfg-file-read-objs) -o coresight-cfg-file-read
+
+clean:
+	rm -f coresight-cfg-file-gen coresight-cfg-file-read
+	rm -f *.o
+	rm -f coresight-config-file.c
+	rm -f *.cscfg
diff --git a/tools/coresight/coresight-cfg-bufw.c b/tools/coresight/coresight-cfg-bufw.c
new file mode 100644
index 000000000000..891fff4e3264
--- /dev/null
+++ b/tools/coresight/coresight-cfg-bufw.c
@@ -0,0 +1,309 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020-2022 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+
+#include <string.h>
+
+#include "coresight-cfg-bufw.h"
+#include "coresight-config-uapi.h"
+
+/*
+ * Set of macros to make writing the buffer code easier.
+ *.
+ * Uses naming convention as 'buffer' for the buffer pointer and
+ * 'used' as the current bytes used by the encosing function.
+ */
+#define cscfg_write_u64(val64) { \
+	*(u64 *)(buffer + used) = val64; \
+	used += sizeof(u64); \
+	}
+
+#define cscfg_write_u32(val32) { \
+	*(u32 *)(buffer + used) = val32; \
+	used += sizeof(u32); \
+	}
+
+#define cscfg_write_u16(val16) { \
+	*(u16 *)(buffer + used) = val16; \
+	used += sizeof(u16); \
+	}
+
+#define cscfg_write_u8(val8) { \
+	*(buffer + used) = val8; \
+	used++;	\
+	}
+
+#define CHECK_WRET(rval) { \
+	if (rval < 0) \
+		return rval; \
+	used += rval; \
+	}
+
+/* write the header at the start of the buffer */
+static int cscfg_file_write_fhdr(u8 *buffer, const int buflen,
+				 const struct cscfg_file_header *fhdr)
+{
+	int used = 0;
+
+	cscfg_write_u32(fhdr->magic_version);
+	cscfg_write_u16(fhdr->length);
+	cscfg_write_u16(fhdr->nr_configs);
+	cscfg_write_u16(fhdr->nr_features);
+	return used;
+}
+
+static int cscfg_file_write_string(u8 *buffer, const int buflen, const char *string)
+{
+	int len, used = 0;
+
+	len = strlen(string);
+	if (len > CSCFG_FILE_STR_MAXSIZE)
+		return -EINVAL;
+
+	if (buflen < (len + 1 + sizeof(u16)))
+		return -EINVAL;
+
+	cscfg_write_u16((u16)(len + 1));
+	strcpy((char *)(buffer + used), string);
+	used += (len + 1);
+
+	return used;
+}
+
+static int cscfg_file_write_elem_hdr(u8 *buffer, const int buflen,
+				     struct cscfg_file_elem_header *ehdr)
+{
+	int used = 0;
+
+	if (buflen < (sizeof(u16) + sizeof(u8)))
+		return -EINVAL;
+
+	cscfg_write_u16(ehdr->elem_length);
+	cscfg_write_u8(ehdr->elem_type);
+
+	return used;
+}
+
+static int cscfg_file_write_config(u8 *buffer, const int buflen,
+				   struct cscfg_config_desc *config_desc)
+{
+	int used = 0, bytes_w, space_req, preset_bytes, i;
+	struct cscfg_file_elem_header ehdr;
+
+	ehdr.elem_length = 0;
+	ehdr.elem_type = CSCFG_FILE_ELEM_TYPE_CFG;
+
+	/* write element header at current buffer location */
+	bytes_w = cscfg_file_write_elem_hdr(buffer, buflen, &ehdr);
+	CHECK_WRET(bytes_w);
+
+	/* write out the configuration name */
+	bytes_w = cscfg_file_write_string(buffer + used, buflen - used,
+					  config_desc->name);
+	CHECK_WRET(bytes_w);
+
+	/* write out the description string */
+	bytes_w = cscfg_file_write_string(buffer + used, buflen - used,
+					  config_desc->description);
+	CHECK_WRET(bytes_w);
+
+	/*
+	 * calculate the space needed for variables + presets
+	 * [u16 value - nr_presets]
+	 * [u32 value - nr_total_params]
+	 * [u16 value - nr_feat_refs]
+	 * [u64 values] * (nr_presets * nr_total_params)
+	 */
+	preset_bytes = sizeof(u64) * config_desc->nr_presets * config_desc->nr_total_params;
+	space_req = (sizeof(u16) * 2) + sizeof(u32) + preset_bytes;
+
+	if ((buflen - used) < space_req)
+		return -EINVAL;
+
+	cscfg_write_u16((u16)config_desc->nr_presets);
+	cscfg_write_u32((u32)config_desc->nr_total_params);
+	cscfg_write_u16((u16)config_desc->nr_feat_refs);
+	if (preset_bytes) {
+		memcpy(buffer + used, (u8 *)config_desc->presets, preset_bytes);
+		used += preset_bytes;
+	}
+
+	/* now write the feature ref names */
+	for (i = 0; i < config_desc->nr_feat_refs; i++) {
+		bytes_w = cscfg_file_write_string(buffer + used, buflen - used,
+						  config_desc->feat_ref_names[i]);
+		CHECK_WRET(bytes_w);
+	}
+
+	/* rewrite the element header with the correct length */
+	ehdr.elem_length = used;
+	bytes_w = cscfg_file_write_elem_hdr(buffer, buflen, &ehdr);
+	/* no CHECK_WRET as used must not be updated */
+	if (bytes_w < 0)
+		return bytes_w;
+
+	return used;
+}
+
+/*
+ * write a parameter structure into the buffer in following format:
+ * [cscfg_file_elem_str]    - parameter name.
+ * [u64 value: param_value] - initial value.
+ */
+static int cscfg_file_write_param(u8 *buffer, const int buflen,
+				  struct cscfg_parameter_desc *param_desc)
+{
+	int used = 0, bytes_w;
+
+	bytes_w = cscfg_file_write_string(buffer + used, buflen - used,
+					  param_desc->name);
+	CHECK_WRET(bytes_w);
+
+	if ((buflen - used) < sizeof(u64))
+		return -EINVAL;
+
+	cscfg_write_u64(param_desc->value);
+	return used;
+}
+
+/*
+ * Write a feature element from cscfg_feature_desc in following format:
+ *
+ * [cscfg_file_elem_header] - header length is total bytes to end of param structures.
+ * [cscfg_file_elem_str]    - feature name.
+ * [cscfg_file_elem_str]    - feature description.
+ * [u32 value: match_flags]
+ * [u16 value: nr_regs]	    - number of registers.
+ * [u16 value: nr_params]   - number of parameters.
+ * [cscfg_regval_desc struct] * nr_regs
+ * [PARAM_ELEM] * nr_params
+ */
+static int cscfg_file_write_feat(u8 *buffer, const int buflen,
+				 struct cscfg_feature_desc *feat_desc)
+{
+	struct cscfg_file_elem_header ehdr;
+	struct cscfg_regval_desc *p_reg_desc;
+	int used = 0, bytes_w, i, space_req;
+	u32 val32;
+
+	ehdr.elem_length = 0;
+	ehdr.elem_type = CSCFG_FILE_ELEM_TYPE_FEAT;
+
+	/* write element header at current buffer location */
+	bytes_w = cscfg_file_write_elem_hdr(buffer, buflen, &ehdr);
+	CHECK_WRET(bytes_w);
+
+	/* write out the name string */
+	bytes_w = cscfg_file_write_string(buffer + used, buflen - used,
+					  feat_desc->name);
+	CHECK_WRET(bytes_w)
+
+	/* write out the description string */
+	bytes_w = cscfg_file_write_string(buffer + used, buflen - used,
+					  feat_desc->description);
+	CHECK_WRET(bytes_w);
+
+	/* check for space for variables and register structures */
+	space_req = (sizeof(u16) * 2) + sizeof(u32) +
+		(sizeof(struct cscfg_regval_desc) * feat_desc->nr_regs);
+	if ((buflen - used) < space_req)
+		return -EINVAL;
+
+	/* write the variables */
+	cscfg_write_u32((u32)feat_desc->match_flags);
+	cscfg_write_u16((u16)feat_desc->nr_regs);
+	cscfg_write_u16((u16)feat_desc->nr_params);
+
+	/*write the registers */
+	for (i = 0; i < feat_desc->nr_regs; i++) {
+		p_reg_desc = (struct cscfg_regval_desc *)&feat_desc->regs_desc[i];
+		CSCFG_FILE_REG_DESC_INFO_TO_U32(val32, p_reg_desc);
+		cscfg_write_u32(val32);
+		cscfg_write_u64(feat_desc->regs_desc[i].val64);
+	}
+
+	/* write any parameters */
+	for (i = 0; i < feat_desc->nr_params; i++) {
+		bytes_w = cscfg_file_write_param(buffer + used, buflen - used,
+						 &feat_desc->params_desc[i]);
+		CHECK_WRET(bytes_w);
+	}
+
+	/*
+	 * rewrite the element header at the start of the buffer block
+	 * with the correct length
+	 */
+	ehdr.elem_length = used;
+	bytes_w = cscfg_file_write_elem_hdr(buffer, buflen, &ehdr);
+	/* no CHECK_WRET as used must not be updated */
+	if (bytes_w < 0)
+		return bytes_w;
+
+	return used;
+}
+
+/*
+ * write a buffer from the configuration and feature
+ * descriptors to write into a file for configfs.
+ *
+ * Will only write one config, and/or a number of features,
+ * per the file standard.
+ */
+int cscfg_file_write_buffer(u8 *buffer, const int buflen,
+			    struct cscfg_config_desc **config_descs,
+			    struct cscfg_feature_desc **feat_descs)
+{
+	struct cscfg_file_header fhdr;
+	int used = 0,  bytes_w, i;
+
+	/* init the file header */
+	fhdr.magic_version = CSCFG_FILE_MAGIC_VERSION;
+	fhdr.length = 0;
+	fhdr.nr_configs = 0;
+	fhdr.nr_features = 0;
+
+	/* count the configs */
+	if (config_descs) {
+		while (config_descs[fhdr.nr_configs])
+			fhdr.nr_configs++;
+	}
+
+	/* count the features */
+	if (feat_descs) {
+		while (feat_descs[fhdr.nr_features])
+			fhdr.nr_features++;
+	}
+
+	/* need a buffer and at least one config or feature */
+	if ((!fhdr.nr_configs && !fhdr.nr_features) ||
+	    !buffer || (buflen > CSCFG_FILE_MAXSIZE))
+		return -EINVAL;
+
+	/* write a header at the start to get the length of the header */
+	bytes_w = cscfg_file_write_fhdr(buffer, buflen, &fhdr);
+	CHECK_WRET(bytes_w);
+
+	/* write configs */
+	for (i = 0; i < fhdr.nr_configs; i++) {
+		bytes_w = cscfg_file_write_config(buffer + used, buflen - used,
+						  config_descs[i]);
+		CHECK_WRET(bytes_w);
+	}
+
+	/* write any features */
+	for (i = 0; i < fhdr.nr_features; i++) {
+		bytes_w = cscfg_file_write_feat(buffer + used, buflen - used,
+						feat_descs[i]);
+		CHECK_WRET(bytes_w);
+	}
+
+	/* finally re-write the header at the buffer start with the correct length */
+	fhdr.length = (u16)used;
+	bytes_w = cscfg_file_write_fhdr(buffer, buflen, &fhdr);
+	/* no CHECK_WRET as used must not be updated */
+	if (bytes_w < 0)
+		return bytes_w;
+	return used;
+}
diff --git a/tools/coresight/coresight-cfg-bufw.h b/tools/coresight/coresight-cfg-bufw.h
new file mode 100644
index 000000000000..08e0e796bf2e
--- /dev/null
+++ b/tools/coresight/coresight-cfg-bufw.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020-2022 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+
+#ifndef _CORESIGHT_CFG_BUFW_H
+#define _CORESIGHT_CFG_BUFW_H
+
+#include <linux/types.h>
+
+#include "coresight-config-file.h"
+
+/*
+ * Function to take coresight configurations and features and
+ * write them into a supplied memory buffer for serialisation
+ * into a file.
+ *
+ * Resulting file can then be loaded into the coresight
+ * infrastructure via configfs.
+ */
+int cscfg_file_write_buffer(u8 *buffer, const int buflen,
+			    struct cscfg_config_desc **config_descs,
+			    struct cscfg_feature_desc **feat_descs);
+
+#endif /* _CORESIGHT_CFG_BUFW_H */
diff --git a/tools/coresight/coresight-cfg-example1.c b/tools/coresight/coresight-cfg-example1.c
new file mode 100644
index 000000000000..c562116ffc94
--- /dev/null
+++ b/tools/coresight/coresight-cfg-example1.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020-2022 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+#include <linux/types.h>
+#include <linux/unistd.h>
+#include <stdio.h>
+#include <unistd.h>
+
+#include "coresight-cfg-examples.h"
+
+/*
+ * create a configuration only example using the strobing feature
+ */
+
+/* we will provide 4 sets of preset parameter values */
+#define AFDO3_NR_PRESETS	4
+/* the total number of parameters in used features - strobing has 2 */
+#define AFDO3_NR_PARAM_SUM	2
+
+static const char *afdo3_ref_names[] = {
+	"strobing",
+};
+
+/*
+ * set of presets leaves strobing window constant while varying period to allow
+ * experimentation with mark / space ratios for various workloads
+ */
+static u64 afdo3_presets[AFDO3_NR_PRESETS][AFDO3_NR_PARAM_SUM] = {
+	{ 2000, 100 },
+	{ 2000, 1000 },
+	{ 2000, 5000 },
+	{ 2000, 10000 },
+};
+
+struct cscfg_config_desc afdo3 = {
+	.name = "autofdo3",
+	.description = "Setup ETMs with strobing for autofdo\n"
+	"Supplied presets allow experimentation with mark-space ratio for various loads\n",
+	.nr_feat_refs = ARRAY_SIZE(afdo3_ref_names),
+	.feat_ref_names = afdo3_ref_names,
+	.nr_presets = AFDO3_NR_PRESETS,
+	.nr_total_params = AFDO3_NR_PARAM_SUM,
+	.presets = &afdo3_presets[0][0],
+};
+
+static struct cscfg_feature_desc *sample_feats[] = {
+	NULL
+};
+
+static struct cscfg_config_desc *sample_cfgs[] = {
+	&afdo3,
+	NULL
+};
+
+struct cscfg_file_eg_info file_info_eg1 = {
+	.example_name = "example1",
+	.filename = "example1.cscfg",
+	.config_descs = sample_cfgs,
+	.feat_descs = sample_feats,
+};
diff --git a/tools/coresight/coresight-cfg-example2.c b/tools/coresight/coresight-cfg-example2.c
new file mode 100644
index 000000000000..6312a185bd46
--- /dev/null
+++ b/tools/coresight/coresight-cfg-example2.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020-2022 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+#include <linux/types.h>
+#include <linux/unistd.h>
+#include <stdio.h>
+#include <unistd.h>
+
+#include "coresight-cfg-examples.h"
+
+/*
+ * create a dual configuration only example using the strobing feature
+ */
+
+/* we will provide 10 sets of preset parameter values */
+#define AFDO_NR_PRESETS		10
+/* the total number of parameters in used features - strobing has 2 */
+#define AFDO_NR_PARAM_SUM	2
+
+static const char *afdo_ref_names[] = {
+	"strobing",
+};
+
+/*
+ * sets of presets leaves strobing window constant while varying period to allow
+ * experimentation with mark / space ratios for various workloads
+ */
+static u64 afdo_set_a_presets[AFDO_NR_PRESETS][AFDO_NR_PARAM_SUM] = {
+	{ 2000, 100 },
+	{ 2000, 1000 },
+	{ 2000, 5000 },
+	{ 2000, 10000 },
+	{ 4000, 100 },
+	{ 4000, 1000 },
+	{ 4000, 5000 },
+	{ 4000, 10000 },
+	{ 6000, 100 },
+	{ 6000, 1000 },
+};
+
+
+static u64 afdo_set_b_presets[AFDO_NR_PRESETS][AFDO_NR_PARAM_SUM] = {
+	{ 6000, 5000 },
+	{ 6000, 10000 },
+	{ 8000, 100 },
+	{ 8000, 1000 },
+	{ 8000, 5000 },
+	{ 8000, 10000 },
+	{ 12000, 100 },
+	{ 12000, 1000 },
+	{ 12000, 5000 },
+	{ 12000, 10000 },
+};
+/* two configurations with differing preset tables */
+struct cscfg_config_desc afdo_seta = {
+	.name = "autofdo_set_a",
+	.description = "Setup ETMs with strobing for autofdo\n"
+	"Supplied presets allow experimentation with mark-space ratio for various loads\n",
+	.nr_feat_refs = ARRAY_SIZE(afdo_ref_names),
+	.feat_ref_names = afdo_ref_names,
+	.nr_presets = AFDO_NR_PRESETS,
+	.nr_total_params = AFDO_NR_PARAM_SUM,
+	.presets = &afdo_set_a_presets[0][0],
+};
+
+struct cscfg_config_desc afdo_setb = {
+	.name = "autofdo_set_b",
+	.description = "Setup ETMs with strobing for autofdo\n"
+	"Supplied presets allow experimentation with mark-space ratio for various loads\n",
+	.nr_feat_refs = ARRAY_SIZE(afdo_ref_names),
+	.feat_ref_names = afdo_ref_names,
+	.nr_presets = AFDO_NR_PRESETS,
+	.nr_total_params = AFDO_NR_PARAM_SUM,
+	.presets = &afdo_set_b_presets[0][0],
+};
+
+
+static struct cscfg_feature_desc *sample_feats[] = {
+	NULL
+};
+
+static struct cscfg_config_desc *sample_cfgs[] = {
+	&afdo_seta,
+	&afdo_setb,
+	NULL
+};
+
+struct cscfg_file_eg_info file_info_eg2 = {
+	.example_name = "example2",
+	.filename = "example2.cscfg",
+	.config_descs = sample_cfgs,
+	.feat_descs = sample_feats,
+};
diff --git a/tools/coresight/coresight-cfg-examples.h b/tools/coresight/coresight-cfg-examples.h
new file mode 100644
index 000000000000..a13e1553b271
--- /dev/null
+++ b/tools/coresight/coresight-cfg-examples.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020-2022 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+
+#ifndef _CORESIGHT_CFG_EXAMPLES_H
+#define _CORESIGHT_CFG_EXAMPLES_H
+
+#include <linux/kernel.h>
+
+#include "coresight-config-uapi.h"
+
+/* structure to pass configuraiton information to generator program */
+struct cscfg_file_eg_info {
+	const char *example_name;
+	const char *filename;
+	struct cscfg_config_desc **config_descs;
+	struct cscfg_feature_desc **feat_descs;
+};
+
+#endif /* _CORESIGHT_CFG_EXAMPLES_H */
diff --git a/tools/coresight/coresight-cfg-file-gen.c b/tools/coresight/coresight-cfg-file-gen.c
new file mode 100644
index 000000000000..0840732dbd50
--- /dev/null
+++ b/tools/coresight/coresight-cfg-file-gen.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020-2022 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+
+#include <linux/types.h>
+#include <linux/unistd.h>
+#include <stdio.h>
+#include <unistd.h>
+
+#include "coresight-cfg-examples.h"
+#include "coresight-cfg-bufw.h"
+
+/* references to the configuration and feature example structures */
+extern struct cscfg_file_eg_info file_info_eg1;
+extern struct cscfg_file_eg_info file_info_eg2;
+
+/* array of example files to generate */
+struct cscfg_file_eg_info *info_ptrs[] = {
+	&file_info_eg1,
+	&file_info_eg2,
+	NULL,
+};
+
+int main(int argc, char **argv)
+{
+	struct cscfg_config_desc **config_descs;
+	struct cscfg_feature_desc **feat_descs;
+	u8 buffer[CSCFG_FILE_MAXSIZE];
+	int used, idx = 0;
+	FILE *fp;
+	const char *filename;
+
+	printf("Coresight Configuration file Generator\n\n");
+
+	while (info_ptrs[idx]) {
+		printf("Generating %s example\n", info_ptrs[idx]->example_name);
+		config_descs = info_ptrs[idx]->config_descs;
+		feat_descs = info_ptrs[idx]->feat_descs;
+		filename = info_ptrs[idx]->filename;
+		used = cscfg_file_write_buffer(buffer, CSCFG_FILE_MAXSIZE,
+					       config_descs, feat_descs);
+
+		if (used < 0) {
+			printf("Error %d writing configuration %s into buffer\n",
+			       used, info_ptrs[idx]->example_name);
+			return used;
+		}
+
+		fp = fopen(filename, "wb");
+		if (fp == NULL) {
+			printf("Error opening file %s\n", filename);
+			return -1;
+		}
+		fwrite(buffer, used, sizeof(u8), fp);
+		fclose(fp);
+		idx++;
+	}
+	return 0;
+}
diff --git a/tools/coresight/coresight-cfg-file-read.c b/tools/coresight/coresight-cfg-file-read.c
new file mode 100644
index 000000000000..c808fb2747f9
--- /dev/null
+++ b/tools/coresight/coresight-cfg-file-read.c
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020-2022 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+
+#include <linux/types.h>
+#include <linux/unistd.h>
+#include <ctype.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "coresight-config-file.h"
+#include "coresight-config-uapi.h"
+
+/*
+ * tool to read and print a generated configuration
+ * re-uses the read code source from the driver.
+ */
+
+static void validate_config_name(const char *name)
+{
+	int i, len = strlen(name);
+
+	for (i = 0; i < len; i++) {
+		if (!isalnum(name[i]) && !(name[i] == '_')) {
+			printf("\n************************************************\n");
+			printf("ERROR: Configuration name %s invalid character(s)\n", name);
+			printf("     : must contain only alphanumeric and _ only\n");
+			printf("************************************************\n");
+		}
+	}
+}
+
+static void print_configs(struct cscfg_fs_load_descs *load_descs)
+{
+	struct cscfg_config_desc *config_desc;
+	int i, j, p, cfg_idx = 0;
+
+	config_desc = load_descs->config_descs[cfg_idx];
+	if (!config_desc) {
+		printf("File contains no configurations.\n\n");
+		return;
+	}
+
+	while (config_desc) {
+		printf("Configuration %d\nName:- %s\n", cfg_idx + 1, config_desc->name);
+		validate_config_name(config_desc->name);
+		printf("Description:-\n%s\n", config_desc->description);
+		printf("Uses %d features:-\n", config_desc->nr_feat_refs);
+		for (i = 0; i < config_desc->nr_feat_refs; i++)
+			printf("Feature-%d: %s\n", i + 1, config_desc->feat_ref_names[i]);
+
+		printf("\nProvides %d sets of preset values, %d presets per set\n",
+		       config_desc->nr_presets, config_desc->nr_total_params);
+		if (config_desc->nr_presets) {
+			for (i = 0; i < config_desc->nr_presets; i++) {
+				printf("set[%d]: ", i);
+				for (j = 0; j < config_desc->nr_total_params; j++) {
+					p = (i * config_desc->nr_total_params) + j;
+					printf("0x%lx, ",  config_desc->presets[p]);
+				}
+				printf("\n");
+			}
+		}
+		printf("\n============================================\n");
+		cfg_idx++;
+		config_desc = load_descs->config_descs[cfg_idx];
+	}
+}
+
+static void print_reg_type_info(u8 type)
+{
+	if (type & CS_CFG_REG_TYPE_STD)
+		printf("std_reg ");
+	if (type & CS_CFG_REG_TYPE_RESOURCE)
+		printf("resource ");
+	if (type & CS_CFG_REG_TYPE_VAL_PARAM)
+		printf("param_index ");
+	if (type & CS_CFG_REG_TYPE_VAL_64BIT)
+		printf("64_bit ");
+	else
+		printf("32_bit ");
+	if (type & CS_CFG_REG_TYPE_VAL_MASK)
+		printf("masked ");
+	if (type & CS_CFG_REG_TYPE_VAL_SAVE)
+		printf("save_on_disable ");
+
+}
+
+static void print_regs(int nr, struct cscfg_regval_desc *regs_desc_array)
+{
+	int i;
+	struct cscfg_regval_desc *reg_desc;
+	u8 type;
+	u16 offset;
+	u16 info;
+
+	for (i = 0; i < nr; i++) {
+		reg_desc = &regs_desc_array[i];
+		type = (u8)reg_desc->type;
+		offset = (u16)reg_desc->offset;
+		info = (u16)reg_desc->hw_info;
+
+		printf("Reg(%d): Type 0x%x: ", i, type);
+		print_reg_type_info(type);
+		printf("\nOffset: 0x%03x; HW Info: 0x%03x\n", offset, info);
+		printf("Value: ");
+		if (type & CS_CFG_REG_TYPE_VAL_64BIT)
+			printf("0x%lx\n", reg_desc->val64);
+		else if (type & CS_CFG_REG_TYPE_VAL_PARAM) {
+			printf("param(%d) ", reg_desc->param_idx);
+			if (type & (CS_CFG_REG_TYPE_VAL_MASK))
+				printf(" mask: 0x%x", reg_desc->mask32);
+			printf("\n");
+		} else {
+			printf("0x%x ", reg_desc->val32);
+			if (type & (CS_CFG_REG_TYPE_VAL_MASK))
+				printf(" mask: 0x%x", reg_desc->mask32);
+			printf("\n");
+		}
+	}
+}
+
+static void print_params(int nr, struct cscfg_parameter_desc *params_desc)
+{
+	int i;
+
+	for (i = 0; i < nr; i++)
+		printf("Param(%d) : %s; Init value 0x%lx\n", i,
+		       params_desc[i].name, params_desc[i].value);
+}
+
+static void print_features(struct cscfg_fs_load_descs *load_descs)
+{
+	struct cscfg_feature_desc *feat_desc = 0;
+	int idx = 0;
+
+	feat_desc = load_descs->feat_descs[idx];
+	if (!feat_desc) {
+		printf("File contains no features\n\n");
+		return;
+	}
+
+	while (feat_desc) {
+		printf("Feature %d\nName:- %s\n\n", idx + 1, feat_desc->name);
+		printf("Description:- %s\n", feat_desc->description);
+		printf("Match flags: 0x%x\n", feat_desc->match_flags);
+		printf("\nNumber of Paraneters: %d\n", feat_desc->nr_params);
+		if (feat_desc->nr_params)
+			print_params(feat_desc->nr_params, feat_desc->params_desc);
+		printf("\nNumber of Registers: %d\n", feat_desc->nr_regs);
+		if (feat_desc->nr_regs)
+			print_regs(feat_desc->nr_regs, feat_desc->regs_desc);
+		printf("\n============================================\n");
+
+		/* next feature */
+		idx++;
+		feat_desc = load_descs->feat_descs[idx];
+	}
+}
+
+int main(int argc, char **argv)
+{
+	FILE *fp;
+	struct cscfg_fs_load_descs *load_descs;
+	int err, fsize;
+	u8 buffer[CSCFG_FILE_MAXSIZE];
+
+	printf("CoreSight Configuration file reader");
+	printf("\n============================================\n\n");
+
+
+	/* need a filename */
+	if (argc <= 1) {
+		printf("Please provide filename on command line\n");
+		return -EINVAL;
+	}
+
+	/* open file and read into the buffer. */
+	fp = fopen(argv[1], "rb");
+	if (fp == NULL) {
+		printf("Error opening file %s\n", argv[1]);
+		return -EINVAL;
+	}
+
+	fseek(fp, 0, SEEK_END);
+	fsize = ftell(fp);
+	rewind(fp);
+	if (fsize > CSCFG_FILE_MAXSIZE) {
+		printf("Error: Input file too large.");
+		fclose(fp);
+		return -EINVAL;
+	}
+	err = fread(buffer, sizeof(u8), fsize, fp);
+	fclose(fp);
+
+	if (err < fsize) {
+		printf("Error reading file %s\n", argv[1]);
+		return -EINVAL;
+	}
+
+	/* allocate the descriptor structures to be populated by read operation */
+	load_descs = malloc(sizeof(struct cscfg_fs_load_descs));
+	if (!load_descs) {
+		printf("Error allocating load descs structure.\n");
+		return -ENOMEM;
+	}
+
+	/* read the buffer and create the configuration and feature structures */
+	err = cscfg_file_read_buffer(buffer, fsize, load_descs);
+	if (err) {
+		printf("Error reading configuration file\n");
+		goto exit_free_mem;
+	}
+
+	/* print the contents of the structures */
+	print_configs(load_descs);
+	print_features(load_descs);
+
+exit_free_mem:
+	cscfg_file_free_load_descs(load_descs);
+	free(load_descs);
+	return err;
+}
diff --git a/tools/coresight/coresight-config-uapi.h b/tools/coresight/coresight-config-uapi.h
new file mode 100644
index 000000000000..d051c01ea982
--- /dev/null
+++ b/tools/coresight/coresight-config-uapi.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+
+#ifndef _CORESIGHT_CORESIGHT_CONFIG_UAPI_H
+#define _CORESIGHT_CORESIGHT_CONFIG_UAPI_H
+
+#include <linux/types.h>
+#include <asm-generic/errno-base.h>
+
+#include "coresight-config.h"
+
+/*
+ * Userspace versions of the configuration and feature descriptors.
+ * Used in the tools/coresight programs.
+ *
+ * Compatible with structures in coresight-config.h for use in
+ * coresight-config-file.c common reader source file.
+ */
+
+/**
+ * Device feature descriptor - combination of registers and parameters to
+ * program a device to implement a specific complex function.
+ *
+ * UAPI version - removed kernel constructs.
+ *
+ * @name:	 feature name.
+ * @description: brief description of the feature.
+ * @match_flags: matching information if loading into a device
+ * @nr_params:   number of parameters used.
+ * @params_desc: array of parameters used.
+ * @nr_regs:	 number of registers used.
+ * @regs_desc:	 array of registers used.
+ */
+struct cscfg_feature_desc {
+	const char *name;
+	const char *description;
+	u32 match_flags;
+	int nr_params;
+	struct cscfg_parameter_desc *params_desc;
+	int nr_regs;
+	struct cscfg_regval_desc *regs_desc;
+};
+
+/**
+ * Configuration descriptor - describes selectable system configuration.
+ *
+ * A configuration describes device features in use, and may provide preset
+ * values for the parameters in those features.
+ *
+ * A single set of presets is the sum of the parameters declared by
+ * all the features in use - this value is @nr_total_params.
+ *
+ * UAPI version - removed kernel constructs.
+ *
+ * @name:		name of the configuration - used for selection.
+ * @description:	description of the purpose of the configuration.
+ * @nr_feat_refs:	Number of features used in this configuration.
+ * @feat_ref_names:	references to features used in this configuration.
+ * @nr_presets:		Number of sets of presets supplied by this configuration.
+ * @nr_total_params:	Sum of all parameters declared by used features
+ * @presets:		Array of preset values.
+ */
+struct cscfg_config_desc {
+	const char *name;
+	const char *description;
+	int nr_feat_refs;
+	const char **feat_ref_names;
+	int nr_presets;
+	int nr_total_params;
+	const u64 *presets; /* nr_presets * nr_total_params */
+};
+
+#endif /* _CORESIGHT_CORESIGHT_CONFIG_UAPI_H */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 6/6] Documentation: coresight: docs for config load via configfs
  2022-12-19 23:46 [PATCH v5 0/6] coresight: syscfg: Extend configfs for config load Mike Leach
                   ` (4 preceding siblings ...)
  2022-12-19 23:46 ` [PATCH v5 5/6] coresight: tools: Add config file write and reader tools Mike Leach
@ 2022-12-19 23:46 ` Mike Leach
  2022-12-21  3:55   ` Bagas Sanjaya
  5 siblings, 1 reply; 15+ messages in thread
From: Mike Leach @ 2022-12-19 23:46 UTC (permalink / raw)
  To: linux-arm-kernel, coresight, linux-kernel
  Cc: mathieu.poirier, suzuki.poulose, acme, james.clark, Mike Leach,
	Jonathan Corbet, linux-doc

Add documentation covering the configfs updates that allow
binary configuration files to be loaded and unloaded via configfs,
along with the demonstration programs in samples.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 .../trace/coresight/coresight-config.rst      | 202 +++++++++++++++++-
 1 file changed, 195 insertions(+), 7 deletions(-)

diff --git a/Documentation/trace/coresight/coresight-config.rst b/Documentation/trace/coresight/coresight-config.rst
index 6d5ffa6f7347..109053eb1b93 100644
--- a/Documentation/trace/coresight/coresight-config.rst
+++ b/Documentation/trace/coresight/coresight-config.rst
@@ -141,11 +141,11 @@ Mount configfs as normal and the 'cs-syscfg' subsystem will appear::
     $ ls /config
     cs-syscfg  stp-policy
 
-This has two sub-directories::
+This has two sub-directories, with the load and unload attribute files::
 
     $ cd cs-syscfg/
     $ ls
-    configurations  features
+    configurations features load  unload
 
 The system has the configuration 'autofdo' built in. It may be examined as
 follows::
@@ -278,9 +278,16 @@ Creating and Loading Custom Configurations
 ==========================================
 
 Custom configurations and / or features can be dynamically loaded into the
-system by using a loadable module.
+system by using a loadable module, or by loading a binary configuration
+file in configfs.
 
-An example of a custom configuration is found in ./samples/coresight.
+Loaded configurations can use previously loaded features. The system will
+ensure that it is not possible to unload a feature that is currently in
+use, by enforcing the unload order as the strict reverse of the load order.
+
+
+Using a Loadable Module
+-----------------------
 
 This creates a new configuration that uses the existing built in
 strobing feature, but provides a different set of presets.
@@ -289,6 +296,187 @@ When the module is loaded, then the configuration appears in the configfs
 file system and is selectable in the same way as the built in configuration
 described above.
 
-Configurations can use previously loaded features. The system will ensure
-that it is not possible to unload a feature that is currently in use, by
-enforcing the unload order as the strict reverse of the load order.
+The file 'coresight-cfg-sample.c' contains the configuration and module
+initialisation code needed to create the loadable module.
+
+This will be built alongside the kernel modules if select in KConfig.
+
+An example of a custom configuration module is found in './samples/coresight'.
+
+Using a Binary Configuration File
+---------------------------------
+
+The './tools/coresight' directory contains example programs to generate and
+read and print binary configuration files.
+
+Building the tools creates the 'coresight-cfg-file-gen' program that will
+generate a configuration binary 'example1.cscfg' that can be loaded into the
+system using configfs. The configuration declared in the source file
+'coresight-cfg-example1.c' is named 'autofdo3' - the name that will be used
+once loaded.
+
+The source files 'coresight-cfg-bufw.h' and 'coresight-cfg-bufw.c' provide a
+standard function to convert a configuration declared in 'C' into the correct
+binary buffer format. These files can be re-used to create new custom
+configurations. Alternatively, addition examples can be added to the
+'coresight-cfg-file-gen' program::
+
+    $ ./coresight-cfg-file-gen
+    Coresight Configuration file Generator
+
+    Generating example1 example
+    Generating example2 example
+
+The program 'coresight-cfg-file-read' can read back and print a configuration
+binary. This is built using the file reader from the driver code
+(coresight-config-file.c), which is copied over into './tools/coresight' at
+build time.::
+
+    ./coresight-cfg-file-read example1.cscfg
+    CoreSight Configuration file reader
+    ============================================
+
+    Configuration 1
+    Name:- autofdo3
+    Description:-
+    Setup ETMs with strobing for autofdo
+    Supplied presets allow experimentation with mark-space ratio for various loads
+
+    Uses 1 features:-
+    Feature-1: strobing
+
+    Provides 4 sets of preset values, 2 presets per set
+    set[0]: 0x7d0, 0x64,
+    set[1]: 0x7d0, 0x3e8,
+    set[2]: 0x7d0, 0x1388,
+    set[3]: 0x7d0, 0x2710,
+
+    ============================================
+    File contains no features
+
+There are additional attributes in the cs-syscfg directory - load and
+unload that can be used to load and unload configuration binary files. To
+load, 'cat' the binary config into the load attribute::
+
+    $ ls /config/cs-syscfg
+    configurations features  load  unload
+    $ cat example1.cscfg > /config/cs-syscfg/load
+    $ ls /config/cs-syscfg/configurations/
+    autofdo  autofdo3
+
+To unload, use the same file in the unload attribute::
+
+    $ cat example1.cscfg > /config/cs-syscfg/unload
+    ls /config/cs-syscfg/configurations/
+    autofdo
+
+
+
+Binary Configuration File Format
+--------------------------------
+
+The file format is defined in the source file **coresight-config-file.h**
+
+The source reader and generator examples produce a binary of this format.
+
+This arrangement is reproduced below:-
+
+Overall File structure
+~~~~~~~~~~~~~~~~~~~~~~
+
+::
+
+   [cscfg_file_header]   // Mandatory
+   [CONFIG_ELEM]*        // Optional - multiple, defined by cscfg_file_header.nr_configs
+   [FEATURE_ELEM]*       // Optional - multiple, defined by cscfg_file_header.nr_features
+
+File is invalid if both [CONFIG_ELEM] and [FEATURE_ELEM] are omitted.
+
+A file that contains only [FEATURE_ELEM] may be loaded, and the features used
+by subsequently loaded files with [CONFIG_ELEM] elements.
+
+Element Name Strings
+~~~~~~~~~~~~~~~~~~~~
+
+Configuration name strings are required to consist of alphanumeric characters and '_' only. Other special characters are not permitted.
+
+::
+   my_config_2          // is a valid name.
+   this-bad-config#5    // this will not work
+
+This is in order to comply with the requirements of the perf command line.
+
+It is recommended that Feature and Parameter names use the same convention to allow for future enhancements to the command line syntax.
+
+CONFIG_ELEM element
+~~~~~~~~~~~~~~~~~~~
+
+::
+
+   [cscfg_file_elem_header]                // header length value to end of feature strings.
+   [cscfg_file_elem_str]                   // name of the configuration.
+                                           // (see element string name requirements)
+   [cscfg_file_elem_str]                   // description of configuration.
+   [u16 value](nr_presets)                 // number of defined sets presets values.
+   [u32 value](nr_total_params)            // total parameters defined by all used features.
+   [u16 value](nr_feat_refs)               // number of features referenced by the configuration
+   [u64 values] * (nr_presets * nr_total_params)     // the preset values.
+   [cscfg_file_elem_str] * (nr_feat_refs)  // names of features used in the configurations.
+
+FEATURE_ELEM element
+~~~~~~~~~~~~~~~~~~~~
+
+::
+
+   [cscfg_file_elem_header]                // header length is total bytes to end of param structures.
+   [cscfg_file_elem_str]                   // feature name.
+   [cscfg_file_elem_str]                   // feature description.
+   [u32 value](match_flags)                // flags to associate the feature with a device.
+   [u16 value](nr_regs)                    // number of registers.
+   [u16 value](nr_params)                  // number of parameters.
+   [cscfg_regval_desc struct] * (nr_regs)  // register definitions
+   [PARAM_ELEM] * (nr_params)              // parameters definitions
+
+PARAM_ELEM element
+~~~~~~~~~~~~~~~~~~
+
+::
+
+   [cscfg_file_elem_str]         // parameter name.
+   [u64 value](param_value)      // initial value.
+
+Additional definitions.
+~~~~~~~~~~~~~~~~~~~~~~~
+
+The following structures are defined in **coresight-config-file.h**
+
+ * **struct cscfg_file_header** : This structure contains an initial magic number, the total
+   length of the file, and the number of configurations and features in the file.
+ * **struct cscfg_file_elem_header**: This defines the total length and type of a CONFIG_ELEM
+   or a FEATURE_ELEM.
+ * **struct cscfg_file_elem_str**: This defines a string and its length.
+
+The magic number in cscfg_file_header is defined as two bitfields::
+
+   [31:8] Fixed magic number to identify file type.
+   [7:0]  Current file format version.
+
+The following defines determine the maximum overall file size and maximum individual
+string size::
+
+   CSCFG_FILE_MAXSIZE       // maximum overall file size.
+   CSCFG_FILE_STR_MAXSIZE   // maximum individual string size.
+
+Load Dependencies.
+~~~~~~~~~~~~~~~~~~
+
+Files may be unloaded only in the strict reverse order of loading. This is enforced by the
+configuration system.
+
+This is to ensure that any load dependencies are maintained.
+
+A configuration file that contains a CONFIG_ELEM that references named features "feat_A" and "feat_B" will load only if either:-
+a) "feat_A" and/or "feat_B" has been loaded previously, or are present as built-in / module loaded features.
+b) "feat_A" and/or "feat_B" are declared as FEAT_ELEM in the same file as the CONFIG_ELEM.
+
+Files that contain features or configurations with the same names as those already loaded will fail to load.
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 6/6] Documentation: coresight: docs for config load via configfs
  2022-12-19 23:46 ` [PATCH v5 6/6] Documentation: coresight: docs for config load via configfs Mike Leach
@ 2022-12-21  3:55   ` Bagas Sanjaya
  2023-01-16 12:29     ` Mike Leach
  0 siblings, 1 reply; 15+ messages in thread
From: Bagas Sanjaya @ 2022-12-21  3:55 UTC (permalink / raw)
  To: Mike Leach, linux-arm-kernel, coresight, linux-kernel
  Cc: mathieu.poirier, suzuki.poulose, acme, james.clark,
	Jonathan Corbet, linux-doc


[-- Attachment #1.1: Type: text/plain, Size: 10412 bytes --]

On Mon, Dec 19, 2022 at 11:46:38PM +0000, Mike Leach wrote:
> diff --git a/Documentation/trace/coresight/coresight-config.rst b/Documentation/trace/coresight/coresight-config.rst
> index 6d5ffa6f7347..109053eb1b93 100644
> --- a/Documentation/trace/coresight/coresight-config.rst
> +++ b/Documentation/trace/coresight/coresight-config.rst
> @@ -141,11 +141,11 @@ Mount configfs as normal and the 'cs-syscfg' subsystem will appear::
>      $ ls /config
>      cs-syscfg  stp-policy
>  
> -This has two sub-directories::
> +This has two sub-directories, with the load and unload attribute files::
>  
>      $ cd cs-syscfg/
>      $ ls
> -    configurations  features
> +    configurations features load  unload
>  
>  The system has the configuration 'autofdo' built in. It may be examined as
>  follows::
> @@ -278,9 +278,16 @@ Creating and Loading Custom Configurations
>  ==========================================
>  
>  Custom configurations and / or features can be dynamically loaded into the
> -system by using a loadable module.
> +system by using a loadable module, or by loading a binary configuration
> +file in configfs.
>  
> -An example of a custom configuration is found in ./samples/coresight.
> +Loaded configurations can use previously loaded features. The system will
> +ensure that it is not possible to unload a feature that is currently in
> +use, by enforcing the unload order as the strict reverse of the load order.
> +
> +
> +Using a Loadable Module
> +-----------------------
>  
>  This creates a new configuration that uses the existing built in
>  strobing feature, but provides a different set of presets.
> @@ -289,6 +296,187 @@ When the module is loaded, then the configuration appears in the configfs
>  file system and is selectable in the same way as the built in configuration
>  described above.
>  
> -Configurations can use previously loaded features. The system will ensure
> -that it is not possible to unload a feature that is currently in use, by
> -enforcing the unload order as the strict reverse of the load order.
> +The file 'coresight-cfg-sample.c' contains the configuration and module
> +initialisation code needed to create the loadable module.
> +
> +This will be built alongside the kernel modules if select in KConfig.

What config options (CONFIG_) are required for above to work?

> +
> +An example of a custom configuration module is found in './samples/coresight'.
> +
> +Using a Binary Configuration File
> +---------------------------------
> +
> +The './tools/coresight' directory contains example programs to generate and
> +read and print binary configuration files.
> +
> +Building the tools creates the 'coresight-cfg-file-gen' program that will
> +generate a configuration binary 'example1.cscfg' that can be loaded into the
> +system using configfs. The configuration declared in the source file
> +'coresight-cfg-example1.c' is named 'autofdo3' - the name that will be used
> +once loaded.
> +
> +The source files 'coresight-cfg-bufw.h' and 'coresight-cfg-bufw.c' provide a
> +standard function to convert a configuration declared in 'C' into the correct
> +binary buffer format. These files can be re-used to create new custom
> +configurations. Alternatively, addition examples can be added to the

s/addition/additional/

> +'coresight-cfg-file-gen' program::
> +
> +    $ ./coresight-cfg-file-gen
> +    Coresight Configuration file Generator
> +
> +    Generating example1 example
> +    Generating example2 example
> +
> +The program 'coresight-cfg-file-read' can read back and print a configuration
> +binary. This is built using the file reader from the driver code
> +(coresight-config-file.c), which is copied over into './tools/coresight' at
> +build time.::
> +
> +    ./coresight-cfg-file-read example1.cscfg
> +    CoreSight Configuration file reader
> +    ============================================
> +
> +    Configuration 1
> +    Name:- autofdo3
> +    Description:-
> +    Setup ETMs with strobing for autofdo
> +    Supplied presets allow experimentation with mark-space ratio for various loads
> +
> +    Uses 1 features:-
> +    Feature-1: strobing
> +
> +    Provides 4 sets of preset values, 2 presets per set
> +    set[0]: 0x7d0, 0x64,
> +    set[1]: 0x7d0, 0x3e8,
> +    set[2]: 0x7d0, 0x1388,
> +    set[3]: 0x7d0, 0x2710,
> +
> +    ============================================
> +    File contains no features
> +
> +There are additional attributes in the cs-syscfg directory - load and
> +unload that can be used to load and unload configuration binary files. To
> +load, 'cat' the binary config into the load attribute::
> +
> +    $ ls /config/cs-syscfg
> +    configurations features  load  unload
> +    $ cat example1.cscfg > /config/cs-syscfg/load
> +    $ ls /config/cs-syscfg/configurations/
> +    autofdo  autofdo3
> +
> +To unload, use the same file in the unload attribute::
> +
> +    $ cat example1.cscfg > /config/cs-syscfg/unload
> +    ls /config/cs-syscfg/configurations/
> +    autofdo
> +
> +
> +
> +Binary Configuration File Format
> +--------------------------------
> +
> +The file format is defined in the source file **coresight-config-file.h**

Use single-quote for identifier names for consistency here.

> +
> +The source reader and generator examples produce a binary of this format.
> +
> +This arrangement is reproduced below:-
> +
> +Overall File structure
> +~~~~~~~~~~~~~~~~~~~~~~
> +
> +::
> +
> +   [cscfg_file_header]   // Mandatory
> +   [CONFIG_ELEM]*        // Optional - multiple, defined by cscfg_file_header.nr_configs
> +   [FEATURE_ELEM]*       // Optional - multiple, defined by cscfg_file_header.nr_features
> +
> +File is invalid if both [CONFIG_ELEM] and [FEATURE_ELEM] are omitted.
> +
> +A file that contains only [FEATURE_ELEM] may be loaded, and the features used
> +by subsequently loaded files with [CONFIG_ELEM] elements.
> +
> +Element Name Strings
> +~~~~~~~~~~~~~~~~~~~~
> +
> +Configuration name strings are required to consist of alphanumeric characters and '_' only. Other special characters are not permitted.
> +
> +::
> +   my_config_2          // is a valid name.
> +   this-bad-config#5    // this will not work

Instead of using code-block for name examples, what about "... For
example, foo_bar is a valid name where as foo-bar# is not."?

> +
> +This is in order to comply with the requirements of the perf command line.
> +
> +It is recommended that Feature and Parameter names use the same convention to allow for future enhancements to the command line syntax.
> +
> +CONFIG_ELEM element
> +~~~~~~~~~~~~~~~~~~~
> +
> +::
> +
> +   [cscfg_file_elem_header]                // header length value to end of feature strings.
> +   [cscfg_file_elem_str]                   // name of the configuration.
> +                                           // (see element string name requirements)
> +   [cscfg_file_elem_str]                   // description of configuration.
> +   [u16 value](nr_presets)                 // number of defined sets presets values.
> +   [u32 value](nr_total_params)            // total parameters defined by all used features.
> +   [u16 value](nr_feat_refs)               // number of features referenced by the configuration
> +   [u64 values] * (nr_presets * nr_total_params)     // the preset values.
> +   [cscfg_file_elem_str] * (nr_feat_refs)  // names of features used in the configurations.
> +
> +FEATURE_ELEM element
> +~~~~~~~~~~~~~~~~~~~~
> +
> +::
> +
> +   [cscfg_file_elem_header]                // header length is total bytes to end of param structures.
> +   [cscfg_file_elem_str]                   // feature name.
> +   [cscfg_file_elem_str]                   // feature description.
> +   [u32 value](match_flags)                // flags to associate the feature with a device.
> +   [u16 value](nr_regs)                    // number of registers.
> +   [u16 value](nr_params)                  // number of parameters.
> +   [cscfg_regval_desc struct] * (nr_regs)  // register definitions
> +   [PARAM_ELEM] * (nr_params)              // parameters definitions
> +
> +PARAM_ELEM element
> +~~~~~~~~~~~~~~~~~~
> +
> +::
> +
> +   [cscfg_file_elem_str]         // parameter name.
> +   [u64 value](param_value)      // initial value.
> +
> +Additional definitions.
> +~~~~~~~~~~~~~~~~~~~~~~~

Trim trailing period for section names

> +
> +The following structures are defined in **coresight-config-file.h**
> +
> + * **struct cscfg_file_header** : This structure contains an initial magic number, the total
> +   length of the file, and the number of configurations and features in the file.
> + * **struct cscfg_file_elem_header**: This defines the total length and type of a CONFIG_ELEM
> +   or a FEATURE_ELEM.
> + * **struct cscfg_file_elem_str**: This defines a string and its length.

Again, for consistency, wrap identifier names in single-quotes.

> +
> +The magic number in cscfg_file_header is defined as two bitfields::
> +
> +   [31:8] Fixed magic number to identify file type.
> +   [7:0]  Current file format version.
> +
> +The following defines determine the maximum overall file size and maximum individual
> +string size::
> +
> +   CSCFG_FILE_MAXSIZE       // maximum overall file size.
> +   CSCFG_FILE_STR_MAXSIZE   // maximum individual string size.

For parameter lists in elements, use bullet lists instead.

> +
> +Load Dependencies.
> +~~~~~~~~~~~~~~~~~~

Trim trailing period for section names.
> +
> +Files may be unloaded only in the strict reverse order of loading. This is enforced by the
> +configuration system.
> +
> +This is to ensure that any load dependencies are maintained.
> +
> +A configuration file that contains a CONFIG_ELEM that references named features "feat_A" and "feat_B" will load only if either:-
> +a) "feat_A" and/or "feat_B" has been loaded previously, or are present as built-in / module loaded features.
> +b) "feat_A" and/or "feat_B" are declared as FEAT_ELEM in the same file as the CONFIG_ELEM.

Separate the preceding paragraph and the list with a blank line in order
for the list to be rendered as list.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files
  2022-12-19 23:46 ` [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files Mike Leach
@ 2022-12-24  7:16   ` Dan Carpenter
  2023-01-16 12:32     ` Mike Leach
  2022-12-27 17:09   ` Christoph Hellwig
  1 sibling, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2022-12-24  7:16 UTC (permalink / raw)
  To: oe-kbuild, Mike Leach, linux-arm-kernel, coresight, linux-kernel
  Cc: lkp, oe-kbuild-all, mathieu.poirier, suzuki.poulose, acme,
	james.clark, Mike Leach

Hi Mike,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mike-Leach/coresight-syscfg-Extend-configfs-for-config-load/20221220-074850
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20221219234638.3661-4-mike.leach%40linaro.org
patch subject: [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files
config: arm-randconfig-m041-20221218
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>

smatch warnings:
drivers/hwtracing/coresight/coresight-syscfg-configfs.c:595 cscfg_cfg_load_write() warn: possible memory leak of 'load_descs'

vim +/load_descs +595 drivers/hwtracing/coresight/coresight-syscfg-configfs.c

97b8fd654556b1 Mike Leach 2022-12-19  543  static ssize_t cscfg_cfg_load_write(struct config_item *item, const void *buffer, size_t size)
97b8fd654556b1 Mike Leach 2022-12-19  544  {
97b8fd654556b1 Mike Leach 2022-12-19  545  	struct cscfg_fs_load_descs *load_descs = 0;
97b8fd654556b1 Mike Leach 2022-12-19  546  	struct cscfg_load_owner_info *owner_info = 0;
97b8fd654556b1 Mike Leach 2022-12-19  547  	int err = 0;
97b8fd654556b1 Mike Leach 2022-12-19  548  
97b8fd654556b1 Mike Leach 2022-12-19  549  	/* ensure we cannot simultaneously load and unload */
97b8fd654556b1 Mike Leach 2022-12-19  550  	if (!mutex_trylock(&cfs_mutex))
97b8fd654556b1 Mike Leach 2022-12-19  551  		return -EBUSY;
97b8fd654556b1 Mike Leach 2022-12-19  552  
97b8fd654556b1 Mike Leach 2022-12-19  553  	/* check configfs load / unload ops are permitted */
97b8fd654556b1 Mike Leach 2022-12-19  554  	if (!cscfg_fs_load_enabled || unload_owner_info) {
97b8fd654556b1 Mike Leach 2022-12-19  555  		err = -EBUSY;
97b8fd654556b1 Mike Leach 2022-12-19  556  		goto exit_unlock;
97b8fd654556b1 Mike Leach 2022-12-19  557  	}
97b8fd654556b1 Mike Leach 2022-12-19  558  
97b8fd654556b1 Mike Leach 2022-12-19  559  	if (size > CSCFG_FILE_MAXSIZE) {
97b8fd654556b1 Mike Leach 2022-12-19  560  		pr_err("cscfg: Load error - Input file too large.\n");
97b8fd654556b1 Mike Leach 2022-12-19  561  		err = -EINVAL;
97b8fd654556b1 Mike Leach 2022-12-19  562  		goto exit_unlock;
97b8fd654556b1 Mike Leach 2022-12-19  563  	}
97b8fd654556b1 Mike Leach 2022-12-19  564  
97b8fd654556b1 Mike Leach 2022-12-19  565  	load_descs = kzalloc(sizeof(struct cscfg_fs_load_descs), GFP_KERNEL);
97b8fd654556b1 Mike Leach 2022-12-19  566  	owner_info = kzalloc(sizeof(struct cscfg_load_owner_info), GFP_KERNEL);
97b8fd654556b1 Mike Leach 2022-12-19  567  	if (!load_descs || !owner_info) {
97b8fd654556b1 Mike Leach 2022-12-19  568  		err = -ENOMEM;
97b8fd654556b1 Mike Leach 2022-12-19  569  		goto exit_memfree;

This exit leaks (will never happen in real life though).

97b8fd654556b1 Mike Leach 2022-12-19  570  	}
97b8fd654556b1 Mike Leach 2022-12-19  571  
97b8fd654556b1 Mike Leach 2022-12-19  572  	owner_info->owner_handle = load_descs;
97b8fd654556b1 Mike Leach 2022-12-19  573  	owner_info->type = CSCFG_OWNER_CONFIGFS;
97b8fd654556b1 Mike Leach 2022-12-19  574  
97b8fd654556b1 Mike Leach 2022-12-19  575  	err = cscfg_file_read_buffer(buffer, size, load_descs);
97b8fd654556b1 Mike Leach 2022-12-19  576  	if (err) {
97b8fd654556b1 Mike Leach 2022-12-19  577  		pr_err("cscfg: Load error - Failed to read input file.\n");
97b8fd654556b1 Mike Leach 2022-12-19  578  		goto exit_memfree;
97b8fd654556b1 Mike Leach 2022-12-19  579  	}
97b8fd654556b1 Mike Leach 2022-12-19  580  
97b8fd654556b1 Mike Leach 2022-12-19  581  	err = cscfg_load_config_sets(load_descs->config_descs, load_descs->feat_descs, owner_info);
97b8fd654556b1 Mike Leach 2022-12-19  582  	if (err) {
97b8fd654556b1 Mike Leach 2022-12-19  583  		pr_err("cscfg: Load error - Failed to load configuaration file.\n");
97b8fd654556b1 Mike Leach 2022-12-19  584  		goto exit_memfree;
97b8fd654556b1 Mike Leach 2022-12-19  585  	}
97b8fd654556b1 Mike Leach 2022-12-19  586  
97b8fd654556b1 Mike Leach 2022-12-19  587  	mutex_unlock(&cfs_mutex);
97b8fd654556b1 Mike Leach 2022-12-19  588  	return size;
97b8fd654556b1 Mike Leach 2022-12-19  589  
97b8fd654556b1 Mike Leach 2022-12-19  590  exit_memfree:
97b8fd654556b1 Mike Leach 2022-12-19  591  	cscfg_configfs_free_owner_info(owner_info);
97b8fd654556b1 Mike Leach 2022-12-19  592  
97b8fd654556b1 Mike Leach 2022-12-19  593  exit_unlock:
97b8fd654556b1 Mike Leach 2022-12-19  594  	mutex_unlock(&cfs_mutex);
97b8fd654556b1 Mike Leach 2022-12-19 @595  	return err;
97b8fd654556b1 Mike Leach 2022-12-19  596  }

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files
  2022-12-19 23:46 ` [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files Mike Leach
  2022-12-24  7:16   ` Dan Carpenter
@ 2022-12-27 17:09   ` Christoph Hellwig
  2023-01-16 12:36     ` Mike Leach
  1 sibling, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2022-12-27 17:09 UTC (permalink / raw)
  To: Mike Leach
  Cc: linux-arm-kernel, coresight, linux-kernel, mathieu.poirier,
	suzuki.poulose, acme, james.clark

On Mon, Dec 19, 2022 at 11:46:35PM +0000, Mike Leach wrote:
> Add in functionality and binary attribute to load and unload
> configurations as binary data.
> 
> Files are loaded via the 'load' binary attribute. System reads the incoming
> file, which must be formatted correctly as defined in the file reader code.
> This will create configuration(s) and/or feature(s) and load them
> into the system.

Binary attributes are intended to pass things such as firmware
through.  Defining your own structured file format seems like a
major abuse of the configfs design.  What's the advantage of this
over simply using an ioctl?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 4/6] coresight: configfs: Modify config files to allow userspace use
  2022-12-19 23:46 ` [PATCH v5 4/6] coresight: configfs: Modify config files to allow userspace use Mike Leach
@ 2022-12-27 17:10   ` Christoph Hellwig
  2023-01-16 12:29     ` Mike Leach
  0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2022-12-27 17:10 UTC (permalink / raw)
  To: Mike Leach
  Cc: linux-arm-kernel, coresight, linux-kernel, mathieu.poirier,
	suzuki.poulose, acme, james.clark

On Mon, Dec 19, 2022 at 11:46:36PM +0000, Mike Leach wrote:
> Update coresight-config.h and the coresight-config-file.c & .h
> to allow use in userspace programs.
> 
> Use __KERNEL__ defines to filter out driver only structures and
> elements so that user space programs can use the descriptor structures.
> 
> Abstract memory allocation in coresight-config-file.c to allow read
> file functions to be run in userspace and kernel drivers.

That's now how kernel code is written.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 4/6] coresight: configfs: Modify config files to allow userspace use
  2022-12-27 17:10   ` Christoph Hellwig
@ 2023-01-16 12:29     ` Mike Leach
  0 siblings, 0 replies; 15+ messages in thread
From: Mike Leach @ 2023-01-16 12:29 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arm-kernel, coresight, linux-kernel, mathieu.poirier,
	suzuki.poulose, acme, james.clark

I'll rework this in the next set.

Thanks for the review

Mike


On Tue, 27 Dec 2022 at 17:10, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Dec 19, 2022 at 11:46:36PM +0000, Mike Leach wrote:
> > Update coresight-config.h and the coresight-config-file.c & .h
> > to allow use in userspace programs.
> >
> > Use __KERNEL__ defines to filter out driver only structures and
> > elements so that user space programs can use the descriptor structures.
> >
> > Abstract memory allocation in coresight-config-file.c to allow read
> > file functions to be run in userspace and kernel drivers.
>
> That's now how kernel code is written.



--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 6/6] Documentation: coresight: docs for config load via configfs
  2022-12-21  3:55   ` Bagas Sanjaya
@ 2023-01-16 12:29     ` Mike Leach
  0 siblings, 0 replies; 15+ messages in thread
From: Mike Leach @ 2023-01-16 12:29 UTC (permalink / raw)
  To: Bagas Sanjaya
  Cc: linux-arm-kernel, coresight, linux-kernel, mathieu.poirier,
	suzuki.poulose, acme, james.clark, Jonathan Corbet, linux-doc

Hi Bagas,

On Wed, 21 Dec 2022 at 03:55, Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>
> On Mon, Dec 19, 2022 at 11:46:38PM +0000, Mike Leach wrote:
> > diff --git a/Documentation/trace/coresight/coresight-config.rst b/Documentation/trace/coresight/coresight-config.rst
> > index 6d5ffa6f7347..109053eb1b93 100644
> > --- a/Documentation/trace/coresight/coresight-config.rst
> > +++ b/Documentation/trace/coresight/coresight-config.rst
> > @@ -141,11 +141,11 @@ Mount configfs as normal and the 'cs-syscfg' subsystem will appear::
> >      $ ls /config
> >      cs-syscfg  stp-policy
> >
> > -This has two sub-directories::
> > +This has two sub-directories, with the load and unload attribute files::
> >
> >      $ cd cs-syscfg/
> >      $ ls
> > -    configurations  features
> > +    configurations features load  unload
> >
> >  The system has the configuration 'autofdo' built in. It may be examined as
> >  follows::
> > @@ -278,9 +278,16 @@ Creating and Loading Custom Configurations
> >  ==========================================
> >
> >  Custom configurations and / or features can be dynamically loaded into the
> > -system by using a loadable module.
> > +system by using a loadable module, or by loading a binary configuration
> > +file in configfs.
> >
> > -An example of a custom configuration is found in ./samples/coresight.
> > +Loaded configurations can use previously loaded features. The system will
> > +ensure that it is not possible to unload a feature that is currently in
> > +use, by enforcing the unload order as the strict reverse of the load order.
> > +
> > +
> > +Using a Loadable Module
> > +-----------------------
> >
> >  This creates a new configuration that uses the existing built in
> >  strobing feature, but provides a different set of presets.
> > @@ -289,6 +296,187 @@ When the module is loaded, then the configuration appears in the configfs
> >  file system and is selectable in the same way as the built in configuration
> >  described above.
> >
> > -Configurations can use previously loaded features. The system will ensure
> > -that it is not possible to unload a feature that is currently in use, by
> > -enforcing the unload order as the strict reverse of the load order.
> > +The file 'coresight-cfg-sample.c' contains the configuration and module
> > +initialisation code needed to create the loadable module.
> > +
> > +This will be built alongside the kernel modules if select in KConfig.
>
> What config options (CONFIG_) are required for above to work?
>
> > +
> > +An example of a custom configuration module is found in './samples/coresight'.
> > +
> > +Using a Binary Configuration File
> > +---------------------------------
> > +
> > +The './tools/coresight' directory contains example programs to generate and
> > +read and print binary configuration files.
> > +
> > +Building the tools creates the 'coresight-cfg-file-gen' program that will
> > +generate a configuration binary 'example1.cscfg' that can be loaded into the
> > +system using configfs. The configuration declared in the source file
> > +'coresight-cfg-example1.c' is named 'autofdo3' - the name that will be used
> > +once loaded.
> > +
> > +The source files 'coresight-cfg-bufw.h' and 'coresight-cfg-bufw.c' provide a
> > +standard function to convert a configuration declared in 'C' into the correct
> > +binary buffer format. These files can be re-used to create new custom
> > +configurations. Alternatively, addition examples can be added to the
>
> s/addition/additional/
>
> > +'coresight-cfg-file-gen' program::
> > +
> > +    $ ./coresight-cfg-file-gen
> > +    Coresight Configuration file Generator
> > +
> > +    Generating example1 example
> > +    Generating example2 example
> > +
> > +The program 'coresight-cfg-file-read' can read back and print a configuration
> > +binary. This is built using the file reader from the driver code
> > +(coresight-config-file.c), which is copied over into './tools/coresight' at
> > +build time.::
> > +
> > +    ./coresight-cfg-file-read example1.cscfg
> > +    CoreSight Configuration file reader
> > +    ============================================
> > +
> > +    Configuration 1
> > +    Name:- autofdo3
> > +    Description:-
> > +    Setup ETMs with strobing for autofdo
> > +    Supplied presets allow experimentation with mark-space ratio for various loads
> > +
> > +    Uses 1 features:-
> > +    Feature-1: strobing
> > +
> > +    Provides 4 sets of preset values, 2 presets per set
> > +    set[0]: 0x7d0, 0x64,
> > +    set[1]: 0x7d0, 0x3e8,
> > +    set[2]: 0x7d0, 0x1388,
> > +    set[3]: 0x7d0, 0x2710,
> > +
> > +    ============================================
> > +    File contains no features
> > +
> > +There are additional attributes in the cs-syscfg directory - load and
> > +unload that can be used to load and unload configuration binary files. To
> > +load, 'cat' the binary config into the load attribute::
> > +
> > +    $ ls /config/cs-syscfg
> > +    configurations features  load  unload
> > +    $ cat example1.cscfg > /config/cs-syscfg/load
> > +    $ ls /config/cs-syscfg/configurations/
> > +    autofdo  autofdo3
> > +
> > +To unload, use the same file in the unload attribute::
> > +
> > +    $ cat example1.cscfg > /config/cs-syscfg/unload
> > +    ls /config/cs-syscfg/configurations/
> > +    autofdo
> > +
> > +
> > +
> > +Binary Configuration File Format
> > +--------------------------------
> > +
> > +The file format is defined in the source file **coresight-config-file.h**
>
> Use single-quote for identifier names for consistency here.
>
> > +
> > +The source reader and generator examples produce a binary of this format.
> > +
> > +This arrangement is reproduced below:-
> > +
> > +Overall File structure
> > +~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +::
> > +
> > +   [cscfg_file_header]   // Mandatory
> > +   [CONFIG_ELEM]*        // Optional - multiple, defined by cscfg_file_header.nr_configs
> > +   [FEATURE_ELEM]*       // Optional - multiple, defined by cscfg_file_header.nr_features
> > +
> > +File is invalid if both [CONFIG_ELEM] and [FEATURE_ELEM] are omitted.
> > +
> > +A file that contains only [FEATURE_ELEM] may be loaded, and the features used
> > +by subsequently loaded files with [CONFIG_ELEM] elements.
> > +
> > +Element Name Strings
> > +~~~~~~~~~~~~~~~~~~~~
> > +
> > +Configuration name strings are required to consist of alphanumeric characters and '_' only. Other special characters are not permitted.
> > +
> > +::
> > +   my_config_2          // is a valid name.
> > +   this-bad-config#5    // this will not work
>
> Instead of using code-block for name examples, what about "... For
> example, foo_bar is a valid name where as foo-bar# is not."?
>
> > +
> > +This is in order to comply with the requirements of the perf command line.
> > +
> > +It is recommended that Feature and Parameter names use the same convention to allow for future enhancements to the command line syntax.
> > +
> > +CONFIG_ELEM element
> > +~~~~~~~~~~~~~~~~~~~
> > +
> > +::
> > +
> > +   [cscfg_file_elem_header]                // header length value to end of feature strings.
> > +   [cscfg_file_elem_str]                   // name of the configuration.
> > +                                           // (see element string name requirements)
> > +   [cscfg_file_elem_str]                   // description of configuration.
> > +   [u16 value](nr_presets)                 // number of defined sets presets values.
> > +   [u32 value](nr_total_params)            // total parameters defined by all used features.
> > +   [u16 value](nr_feat_refs)               // number of features referenced by the configuration
> > +   [u64 values] * (nr_presets * nr_total_params)     // the preset values.
> > +   [cscfg_file_elem_str] * (nr_feat_refs)  // names of features used in the configurations.
> > +
> > +FEATURE_ELEM element
> > +~~~~~~~~~~~~~~~~~~~~
> > +
> > +::
> > +
> > +   [cscfg_file_elem_header]                // header length is total bytes to end of param structures.
> > +   [cscfg_file_elem_str]                   // feature name.
> > +   [cscfg_file_elem_str]                   // feature description.
> > +   [u32 value](match_flags)                // flags to associate the feature with a device.
> > +   [u16 value](nr_regs)                    // number of registers.
> > +   [u16 value](nr_params)                  // number of parameters.
> > +   [cscfg_regval_desc struct] * (nr_regs)  // register definitions
> > +   [PARAM_ELEM] * (nr_params)              // parameters definitions
> > +
> > +PARAM_ELEM element
> > +~~~~~~~~~~~~~~~~~~
> > +
> > +::
> > +
> > +   [cscfg_file_elem_str]         // parameter name.
> > +   [u64 value](param_value)      // initial value.
> > +
> > +Additional definitions.
> > +~~~~~~~~~~~~~~~~~~~~~~~
>
> Trim trailing period for section names
>
> > +
> > +The following structures are defined in **coresight-config-file.h**
> > +
> > + * **struct cscfg_file_header** : This structure contains an initial magic number, the total
> > +   length of the file, and the number of configurations and features in the file.
> > + * **struct cscfg_file_elem_header**: This defines the total length and type of a CONFIG_ELEM
> > +   or a FEATURE_ELEM.
> > + * **struct cscfg_file_elem_str**: This defines a string and its length.
>
> Again, for consistency, wrap identifier names in single-quotes.
>
> > +
> > +The magic number in cscfg_file_header is defined as two bitfields::
> > +
> > +   [31:8] Fixed magic number to identify file type.
> > +   [7:0]  Current file format version.
> > +
> > +The following defines determine the maximum overall file size and maximum individual
> > +string size::
> > +
> > +   CSCFG_FILE_MAXSIZE       // maximum overall file size.
> > +   CSCFG_FILE_STR_MAXSIZE   // maximum individual string size.
>
> For parameter lists in elements, use bullet lists instead.
>
> > +
> > +Load Dependencies.
> > +~~~~~~~~~~~~~~~~~~
>
> Trim trailing period for section names.
> > +
> > +Files may be unloaded only in the strict reverse order of loading. This is enforced by the
> > +configuration system.
> > +
> > +This is to ensure that any load dependencies are maintained.
> > +
> > +A configuration file that contains a CONFIG_ELEM that references named features "feat_A" and "feat_B" will load only if either:-
> > +a) "feat_A" and/or "feat_B" has been loaded previously, or are present as built-in / module loaded features.
> > +b) "feat_A" and/or "feat_B" are declared as FEAT_ELEM in the same file as the CONFIG_ELEM.
>
> Separate the preceding paragraph and the list with a blank line in order
> for the list to be rendered as list.
>

The next version will contain all the adjustments that you suggested,
though rather than using single quotes, I have changed all the
occurrences to consistently the double back tick to make the filenames
etc fixed width font.

Thanks for the review

Mike


> Thanks.
>
> --
> An old man doll... just what I always wanted! - Clara



--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files
  2022-12-24  7:16   ` Dan Carpenter
@ 2023-01-16 12:32     ` Mike Leach
  0 siblings, 0 replies; 15+ messages in thread
From: Mike Leach @ 2023-01-16 12:32 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, linux-arm-kernel, coresight, linux-kernel, lkp,
	oe-kbuild-all, mathieu.poirier, suzuki.poulose, acme,
	james.clark

Hi,

Thanks for this - I'll fix in next set

Mike

On Sat, 24 Dec 2022 at 07:16, Dan Carpenter <error27@gmail.com> wrote:
>
> Hi Mike,
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Mike-Leach/coresight-syscfg-Extend-configfs-for-config-load/20221220-074850
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
> patch link:    https://lore.kernel.org/r/20221219234638.3661-4-mike.leach%40linaro.org
> patch subject: [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files
> config: arm-randconfig-m041-20221218
> compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
>
> smatch warnings:
> drivers/hwtracing/coresight/coresight-syscfg-configfs.c:595 cscfg_cfg_load_write() warn: possible memory leak of 'load_descs'
>
> vim +/load_descs +595 drivers/hwtracing/coresight/coresight-syscfg-configfs.c
>
> 97b8fd654556b1 Mike Leach 2022-12-19  543  static ssize_t cscfg_cfg_load_write(struct config_item *item, const void *buffer, size_t size)
> 97b8fd654556b1 Mike Leach 2022-12-19  544  {
> 97b8fd654556b1 Mike Leach 2022-12-19  545       struct cscfg_fs_load_descs *load_descs = 0;
> 97b8fd654556b1 Mike Leach 2022-12-19  546       struct cscfg_load_owner_info *owner_info = 0;
> 97b8fd654556b1 Mike Leach 2022-12-19  547       int err = 0;
> 97b8fd654556b1 Mike Leach 2022-12-19  548
> 97b8fd654556b1 Mike Leach 2022-12-19  549       /* ensure we cannot simultaneously load and unload */
> 97b8fd654556b1 Mike Leach 2022-12-19  550       if (!mutex_trylock(&cfs_mutex))
> 97b8fd654556b1 Mike Leach 2022-12-19  551               return -EBUSY;
> 97b8fd654556b1 Mike Leach 2022-12-19  552
> 97b8fd654556b1 Mike Leach 2022-12-19  553       /* check configfs load / unload ops are permitted */
> 97b8fd654556b1 Mike Leach 2022-12-19  554       if (!cscfg_fs_load_enabled || unload_owner_info) {
> 97b8fd654556b1 Mike Leach 2022-12-19  555               err = -EBUSY;
> 97b8fd654556b1 Mike Leach 2022-12-19  556               goto exit_unlock;
> 97b8fd654556b1 Mike Leach 2022-12-19  557       }
> 97b8fd654556b1 Mike Leach 2022-12-19  558
> 97b8fd654556b1 Mike Leach 2022-12-19  559       if (size > CSCFG_FILE_MAXSIZE) {
> 97b8fd654556b1 Mike Leach 2022-12-19  560               pr_err("cscfg: Load error - Input file too large.\n");
> 97b8fd654556b1 Mike Leach 2022-12-19  561               err = -EINVAL;
> 97b8fd654556b1 Mike Leach 2022-12-19  562               goto exit_unlock;
> 97b8fd654556b1 Mike Leach 2022-12-19  563       }
> 97b8fd654556b1 Mike Leach 2022-12-19  564
> 97b8fd654556b1 Mike Leach 2022-12-19  565       load_descs = kzalloc(sizeof(struct cscfg_fs_load_descs), GFP_KERNEL);
> 97b8fd654556b1 Mike Leach 2022-12-19  566       owner_info = kzalloc(sizeof(struct cscfg_load_owner_info), GFP_KERNEL);
> 97b8fd654556b1 Mike Leach 2022-12-19  567       if (!load_descs || !owner_info) {
> 97b8fd654556b1 Mike Leach 2022-12-19  568               err = -ENOMEM;
> 97b8fd654556b1 Mike Leach 2022-12-19  569               goto exit_memfree;
>
> This exit leaks (will never happen in real life though).
>
> 97b8fd654556b1 Mike Leach 2022-12-19  570       }
> 97b8fd654556b1 Mike Leach 2022-12-19  571
> 97b8fd654556b1 Mike Leach 2022-12-19  572       owner_info->owner_handle = load_descs;
> 97b8fd654556b1 Mike Leach 2022-12-19  573       owner_info->type = CSCFG_OWNER_CONFIGFS;
> 97b8fd654556b1 Mike Leach 2022-12-19  574
> 97b8fd654556b1 Mike Leach 2022-12-19  575       err = cscfg_file_read_buffer(buffer, size, load_descs);
> 97b8fd654556b1 Mike Leach 2022-12-19  576       if (err) {
> 97b8fd654556b1 Mike Leach 2022-12-19  577               pr_err("cscfg: Load error - Failed to read input file.\n");
> 97b8fd654556b1 Mike Leach 2022-12-19  578               goto exit_memfree;
> 97b8fd654556b1 Mike Leach 2022-12-19  579       }
> 97b8fd654556b1 Mike Leach 2022-12-19  580
> 97b8fd654556b1 Mike Leach 2022-12-19  581       err = cscfg_load_config_sets(load_descs->config_descs, load_descs->feat_descs, owner_info);
> 97b8fd654556b1 Mike Leach 2022-12-19  582       if (err) {
> 97b8fd654556b1 Mike Leach 2022-12-19  583               pr_err("cscfg: Load error - Failed to load configuaration file.\n");
> 97b8fd654556b1 Mike Leach 2022-12-19  584               goto exit_memfree;
> 97b8fd654556b1 Mike Leach 2022-12-19  585       }
> 97b8fd654556b1 Mike Leach 2022-12-19  586
> 97b8fd654556b1 Mike Leach 2022-12-19  587       mutex_unlock(&cfs_mutex);
> 97b8fd654556b1 Mike Leach 2022-12-19  588       return size;
> 97b8fd654556b1 Mike Leach 2022-12-19  589
> 97b8fd654556b1 Mike Leach 2022-12-19  590  exit_memfree:
> 97b8fd654556b1 Mike Leach 2022-12-19  591       cscfg_configfs_free_owner_info(owner_info);
> 97b8fd654556b1 Mike Leach 2022-12-19  592
> 97b8fd654556b1 Mike Leach 2022-12-19  593  exit_unlock:
> 97b8fd654556b1 Mike Leach 2022-12-19  594       mutex_unlock(&cfs_mutex);
> 97b8fd654556b1 Mike Leach 2022-12-19 @595       return err;
> 97b8fd654556b1 Mike Leach 2022-12-19  596  }
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files
  2022-12-27 17:09   ` Christoph Hellwig
@ 2023-01-16 12:36     ` Mike Leach
  0 siblings, 0 replies; 15+ messages in thread
From: Mike Leach @ 2023-01-16 12:36 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arm-kernel, coresight, linux-kernel, mathieu.poirier,
	suzuki.poulose, acme, james.clark

Hi Christoph

On Tue, 27 Dec 2022 at 17:09, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Dec 19, 2022 at 11:46:35PM +0000, Mike Leach wrote:
> > Add in functionality and binary attribute to load and unload
> > configurations as binary data.
> >
> > Files are loaded via the 'load' binary attribute. System reads the incoming
> > file, which must be formatted correctly as defined in the file reader code.
> > This will create configuration(s) and/or feature(s) and load them
> > into the system.
>
> Binary attributes are intended to pass things such as firmware
> through.  Defining your own structured file format seems like a
> major abuse of the configfs design.  What's the advantage of this
> over simply using an ioctl?

The coresight configurations loaded here, represent programming of the
entire coresight subsystem - possibly tens of registers (especially on
the ETM), across multiple devices, in ways that are not possible using
the limited parameters of the perf command line.

The ETM can be programmed in ways that use counters. sequencers, and
optionally interact with other components such as CTI / CTM to send
conditional hardware triggers, all of which control the when and how
the trace is collected. Additionally there are system trace components
that might need to run at the same time - such as ELA, and other
system monitors that output data on the trace bus currently being
introduce by some chip designers.

As such the configuration must be loaded into the coresight system as
a single operation - with the individual drivers validating the
requested programming, where any error will fail the configuration
load. The individual drivers are also responsible for defining which
device registers user configurations can use - these being limited to
those that affect the scope of trace collected, with other operational
functions being reserved to the drivers themselves.

To achieve this a variable sized table of programming descriptors is
defined, that are transferred into the individual devices. The very
limited set of built in configurations - where the programming
descriptors are compiled into the driver modules themselves use the
same set of descriptors. however, recompiling kernel modules to
program new configurations is neither scaleable, flexible or desirable
- so we need a way of loading configurations at runtime. So the file
structure is simple a serialisation of these descriptor tables - with
a header defining the input type and overall size..

The advantage of these runtime configurations is that they can be
portable - and dependent on the hardware in the system, not the kernel
build version. Moreover, there are trace scenarios when we want to
trace what is present, not recompile a module / kernel to achieve
this, especially investigating issues on production systems.

1) using configfs to load these configurations keeps all the coresight
configuration ABI in a single file system - configfs. The current
builtin configurations can be viewed, enabled,  and parameters
configrured in the current configfs that we have upstreamed. Adding
load / unload here is a logical extension of this.

2) because of the nature of configurations described above - we would
need a separate device to represent the whole subsystem - in order to
provide the ioctl support for loading. This device approach to
managing configurations has been previously rejected by the Coresight
maintainers, who suggested that configfs was the correct way to
configure a complex sub-system.

3) configurations are variable in size, An ioctl command would provide
a pointer to the configuration data - but the kernel would have to
trust that the underlying data is correctly formed. With a configfs
file write we get the buffer _and_ its size which is a good deal safer
from an input perspective.

4) ioctl use would require a loader program - configfs allows load
directly from the command line.

I agree that ioctls certainly have there uses, especially with small,
fixed size data types - but configfs is far better suited to this
complex use case.
Indeed the ioctl documentation suggests using configfs for
configuration cases that are too complex for sysfs, when an ioctl may
not be suitable.

This use of binary attributes is based on the existing use of a
configfs binary attribute is for the ACPI tables - the ACPI driver
here takes the buffer, does some initial validation of the file size
etc, then calls the inslall ./ validate ACPI routines where the table
is added to an internal list of tables maintained by the kernel. These
tables may well be shared by firmware - but are also used by the
kernel.

There appears to be nothing in the configfs documentation limiting the
use of binary attributes for passing firmware, Even the sysfs docs
suggest that this is an expected use but it is not a hard and fast
rule if there are no alternatives.
Granted in the vast majority of cases there are better alternatives.

I believe that loading via configfs is the best and safest engineering
solution for this particular use case.

Regards

Mike

--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-01-16 12:37 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-19 23:46 [PATCH v5 0/6] coresight: syscfg: Extend configfs for config load Mike Leach
2022-12-19 23:46 ` [PATCH v5 1/6] coresight: configfs: Update memory allocation / free for configfs elements Mike Leach
2022-12-19 23:46 ` [PATCH v5 2/6] coresight: configfs: Add in functionality for load via configfs Mike Leach
2022-12-19 23:46 ` [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files Mike Leach
2022-12-24  7:16   ` Dan Carpenter
2023-01-16 12:32     ` Mike Leach
2022-12-27 17:09   ` Christoph Hellwig
2023-01-16 12:36     ` Mike Leach
2022-12-19 23:46 ` [PATCH v5 4/6] coresight: configfs: Modify config files to allow userspace use Mike Leach
2022-12-27 17:10   ` Christoph Hellwig
2023-01-16 12:29     ` Mike Leach
2022-12-19 23:46 ` [PATCH v5 5/6] coresight: tools: Add config file write and reader tools Mike Leach
2022-12-19 23:46 ` [PATCH v5 6/6] Documentation: coresight: docs for config load via configfs Mike Leach
2022-12-21  3:55   ` Bagas Sanjaya
2023-01-16 12:29     ` Mike Leach

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).