[PATCH v2 4/6] coresight: samples: Add an example config writer for configfs load

Mike Leach mike.leach at linaro.org
Wed Feb 2 12:54:35 PST 2022


Hi Mathieu,

On Fri, 28 Jan 2022 at 18:43, Mathieu Poirier
<mathieu.poirier at linaro.org> wrote:
>
> 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.
>
> The user space program to read and write configurations should go under
> tools/coresight rather than sample/coresight.  Have a look at tools/perf and tools/bpf.
> There is a lot of examples in there that deals with code shared between kernel
> and user space.  There is even a nifty scripts that checks for diverging files
> between kernel and tools when compiling the perf tools.
>

OK - hadn't thought of that but seems like a good idea,

> You should also find how to properly deal with error codes and standard
> types in a generic way rather than redefining them as in the previous patch.
>

Agreed - having a quick look, there appears to be suitable headers in
tools/include/uapi/...

> I think this patchset is holding together but needs some refactoring to conform
> to what others have done when dealing with shared kernel/user space code.
>

Agreed.

Thanks

Mike

> I am done reviewing this set.
>
> Thanks,
> Mathieu
>
> >
> > 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.
> > + * 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.
> > + *.
> > + * 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;
> > +}
> > +
> > +
> > +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_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
> > +};
> > +
> > +static struct cscfg_config_desc *sample_cfgs[] = {
> > +     &afdo3,
> > +     NULL
> > +};
> > +
> > +
> > +#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