[PATCH v2 4/6] coresight: samples: Add an example config writer for configfs load
Mike Leach
mike.leach at linaro.org
Tue Jan 18 08:38:40 PST 2022
HI Mathieu,
On Thu, 13 Jan 2022 at 17:56, Mathieu Poirier
<mathieu.poirier at linaro.org> wrote:
>
> Good morning Mike,
>
> On Tue, Nov 30, 2021 at 10:00:58PM +0000, Mike Leach wrote:
> > 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.
> >
>
> Despite trying quite hard I haven't found a single bug in this patchset, which
> is a feat for user space code.
>
> Please see comments below...
>
> > Signed-off-by: Mike Leach <mike.leach at linaro.org>
> > ---
> > samples/coresight/Makefile | 7 +
> > samples/coresight/coresight-cfg-bufw.c | 302 ++++++++++++++++++++++
> > samples/coresight/coresight-cfg-bufw.h | 24 ++
> > samples/coresight/coresight-cfg-filegen.c | 89 +++++++
> > 4 files changed, 422 insertions(+)
> > create mode 100644 samples/coresight/coresight-cfg-bufw.c
> > create mode 100644 samples/coresight/coresight-cfg-bufw.h
> > create mode 100644 samples/coresight/coresight-cfg-filegen.c
> >
> > diff --git a/samples/coresight/Makefile b/samples/coresight/Makefile
> > index b3fce4af2347..07bfd99d7a68 100644
> > --- a/samples/coresight/Makefile
> > +++ b/samples/coresight/Makefile
> > @@ -1,4 +1,11 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> >
> > +# coresight config - loadable module configuration.
> > obj-$(CONFIG_SAMPLE_CORESIGHT_SYSCFG) += coresight-cfg-sample.o
> > ccflags-y += -I$(srctree)/drivers/hwtracing/coresight
> > +
> > +# coresight config - configfs loadable binary config generator
> > +userprogs-always-y += coresight-cfg-filegen
> > +
> > +coresight-cfg-filegen-objs := coresight-cfg-filegen.o coresight-cfg-bufw.o
> > +userccflags += -I$(srctree)/drivers/hwtracing/coresight
> > diff --git a/samples/coresight/coresight-cfg-bufw.c b/samples/coresight/coresight-cfg-bufw.c
> > new file mode 100644
> > index 000000000000..8c32a8509eef
> > --- /dev/null
> > +++ b/samples/coresight/coresight-cfg-bufw.c
> > @@ -0,0 +1,302 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2020 Linaro Limited, All rights reserved.
>
> Are you sure you want to keep this to 2020? Here and in all other files.
>
Given the lenght of legal copyright - not a major issue - but if
respinning will address.
> > + * Author: Mike Leach <mike.leach at linaro.org>
> > + */
> > +
> > +#include <string.h>
> > +
> > +#include "coresight-cfg-bufw.h"
> > +
> > +/*
> > + * Set of macros to make writing the buffer code easier.
> > + *.
>
> Extra '.'
>
> > + * 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_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;
> > +}
> > +
> > +
>
> Extra newline
>
> > +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
> > + *
> > + *
>
> Extra newlines
>
Will sort these.
> > + */
> > +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_desc,
> > + 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_features = 0;
> > +
> > + /* 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 ((!config_desc && !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 a single config */
> > + if (config_desc) {
> > + bytes_w = cscfg_file_write_config(buffer + used, buflen - used,
> > + config_desc);
> > + 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/samples/coresight/coresight-cfg-bufw.h b/samples/coresight/coresight-cfg-bufw.h
> > new file mode 100644
> > index 000000000000..00b16c583cad
> > --- /dev/null
> > +++ b/samples/coresight/coresight-cfg-bufw.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2020 Linaro Limited, All rights reserved.
> > + * Author: Mike Leach <mike.leach at linaro.org>
> > + */
> > +
> > +#ifndef _CORESIGHT_CFG_BUFW_H
> > +#define _CORESIGHT_CFG_BUFW_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_desc,
> > + struct cscfg_feature_desc **feat_descs);
> > +
> > +#endif /* _CORESIGHT_CFG_BUFW_H */
> > diff --git a/samples/coresight/coresight-cfg-filegen.c b/samples/coresight/coresight-cfg-filegen.c
> > new file mode 100644
> > index 000000000000..c8be18ee97b6
> > --- /dev/null
> > +++ b/samples/coresight/coresight-cfg-filegen.c
> > @@ -0,0 +1,89 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2020 Linaro Limited, All rights reserved.
> > + * Author: Mike Leach <mike.leach at linaro.org>
> > + */
> > +
> > +#include <linux/types.h>
> > +#include <linux/unistd.h>
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +
> > +#include "coresight-cfg-bufw.h"
> > +
> > +/*
> > + * generate example binary coresight configuration files for loading
> > + * into the coresight subsystem via configfs
> > + */
> > +
> > +/* create a similar configuration example as the coresight-cfg-sample.c file. */
> > +
> > +/* we will provide 4 sets of preset parameter values */
> > +#define AFDO2_NR_PRESETS 4
> > +/* the total number of parameters in used features - strobing has 2 */
> > +#define AFDO2_NR_PARAM_SUM 2
> > +
> > +static const char *afdo2_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 afdo2_presets[AFDO2_NR_PRESETS][AFDO2_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(afdo2_ref_names),
> > + .feat_ref_names = afdo2_ref_names,
> > + .nr_presets = AFDO2_NR_PRESETS,
> > + .nr_total_params = AFDO2_NR_PARAM_SUM,
> > + .presets = &afdo2_presets[0][0],
> > +};
> > +
> > +static struct cscfg_feature_desc *sample_feats[] = {
> > + NULL
> > +};
>
> For completeness there would be value in providing an example that requires
> features.
>
Agreed but given this patchset has been split into multiple ones this
has dropped som eof the context between patches.
The next set - which adds ETMv4 resource management also contains
multiple additional examples showing features.
These cannot appear here as they use the new resource IDs introduced
in the next set.
> > +
> > +static struct cscfg_config_desc *sample_cfgs[] = {
> > + &afdo3,
> > + NULL
> > +};
>
> Any reason to declare sample_cfgs[] when there can only be one configuration?
>
The API defined requires a NULL terminated list.
> Lastly I get the following [1] when compiling this set - I have not investigated
> further.
>
Looks like there is a failure to copy the coresight_config_file.c
source from the kernel into the sample working directory.
This is compiled into the user space reader tool. Not sure why it
would be copying in my environment and not yours.
It seems like we have been here before in a previous patchset that
contained the samples - I think then it was something to do with
directtory definitions - but I never really understood why the fix we
found worked.
The sampel can be compiled natively (x86). In the samples directory run:-
make -f Makefile.host
This will do the same copy and build, but create x86 versions of the
tools. Perhaps this will highlight the differences.
As an aside, the output files from coresight-cfg-filegen can be
subsequently transferred across to the ARM base target and used.
Thanks
Mike
> Thanks,
> Mathieu
>
> [1]. https://pastebin.com/Nw2DTqTC
>
> > +
> > +
> > +#define CSCFG_BIN_FILENAME "example1.cscfg"
> > +
> > +int main(int argc, char **argv)
> > +{
> > + u8 buffer[CSCFG_FILE_MAXSIZE];
> > + int used;
> > + FILE *fp;
> > +
> > + printf("Coresight Configuration file Generator\n\n");
> > +
> > + used = cscfg_file_write_buffer(buffer, CSCFG_FILE_MAXSIZE,
> > + sample_cfgs[0], sample_feats);
> > +
> > + if (used < 0) {
> > + printf("Error %d writing configuration %s into buffer\n",
> > + used, sample_cfgs[0]->name);
> > + return used;
> > + }
> > +
> > + fp = fopen(CSCFG_BIN_FILENAME, "wb");
> > + if (fp == NULL) {
> > + printf("Error opening file %s\n", CSCFG_BIN_FILENAME);
> > + return -1;
> > + }
> > + fwrite(buffer, used, sizeof(u8), fp);
> > + fclose(fp);
> > + return 0;
> > +}
> > --
> > 2.17.1
> >
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
More information about the linux-arm-kernel
mailing list