From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 968CCC433EF for ; Thu, 13 Jan 2022 17:58:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=CtIym2kjqQY78wGDF6qPdNKUEWrhS3RhoW3EMzavUF0=; b=bkMlTpsUddyDOd s1Wd7YQvFlMwr02+4ToRADPkjJtqDDx3RQQUx2oH4M7n68P+xeEb5M/8okMqp6zvYwSdDFc6JzyTT DrK2ghXwp+PZFuIRPfHQSTUwZ8eOTuXvzdd6dBmzJwbR8hwdNNk6ojrDf16g/CrkgEv5Jf8Zbg+1w V6huCc+4kU97rOJ52plyhET3RmkUVXXQswzKjAo98KlHhRsn/wQb/IKMmlYcVzsXai8kuusXokFe/ +OUOuD9plUAPaG469+a8M+Ozvly7eWjcaEzBHREWrqPNthO8C230E2Ujw5nwW4gjIZbBZJ2qWerZG Px1odwDoAwwrXWGEyGlA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n84L7-006oUu-UL; Thu, 13 Jan 2022 17:56:46 +0000 Received: from mail-pf1-x431.google.com ([2607:f8b0:4864:20::431]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n84L3-006oTG-4D for linux-arm-kernel@lists.infradead.org; Thu, 13 Jan 2022 17:56:43 +0000 Received: by mail-pf1-x431.google.com with SMTP id a5so554811pfo.5 for ; Thu, 13 Jan 2022 09:56:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=kCb2Cd1ebYqQiST0ZDqk97bkARzfMxxyhAme8JjY0ZY=; b=FD5fy6H6Vo589kACEGAyGngZDyG0ZZOQ298vPbo2yeQAUSW/pgktsbjxQKPp5duIzn GiNy/h7XOTuKpKY8RjxjlTiZdpCUiWN92NEfhtJVZ5Cu1S4QnsR+SoICRAfd5Kz9IE9M Sp+wrYJx75S+cUx3cqPIqGUK8zd86Ws9kDVAi3wpMXeJXvycxCBSnTQC9n6SSidvhRPJ nbS6uvZ7deahO7bepsP8NU+eXfuzOmKCC86f2uk8tdUjwoZhQBySvLz3A45jNlN8j580 c2F+DNqe5J7hW1k3KJocmpmIPdj5tD5N1Kb3f/71ZvVt5eerSZYwNTFEQArVugr1OT9m awXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=kCb2Cd1ebYqQiST0ZDqk97bkARzfMxxyhAme8JjY0ZY=; b=agPUyhKY67h2seUwGPUiDZ+eshAjwmOZkGjNx0hjmhOq48KFhdBSOnjbAN1G4TLJQL w7HZ0EHXGoc+p6KtxpQ1As0A8mVHVFEibhtN5Xl/ZyXdtTLAluoZq+hA4Uj2gLUv7QsB gUjW0ayJQF1TMTcwSV2BVCm8Ec+k00ao4RfNNucEwH3MFBKCH5TCQFMAG5WXYnHuLGLD wOUAO8/FRGcoDFacwTtJGTU0Qf/swk8gmUJDtaWHnC32c1y9koO6QSu7GHtkmVpiQkaD G+8Tv/ZPsrXBMIOIaGY6vDvVsmeSOHYP6OcuIoJ319KmzW0XFM+ep3yEZ1YuOi2lT4EU dH8w== X-Gm-Message-State: AOAM533UOTdT7m6Fkf0UOGXc1nCnq7sRsXba9+1sbKOcNziWw1KN3Ctp WjVfqUY5te3oXbRVnXenZ7LuTK+PZPVahg== X-Google-Smtp-Source: ABdhPJypBEu2uOSDFPzsmibmutQ/k6CAtxreSSmXrCzTysVNJveuI3hi3GQd3Jwrs3bh+NKQa43mYA== X-Received: by 2002:a05:6a00:24d1:b0:4c1:f8f5:9f9c with SMTP id d17-20020a056a0024d100b004c1f8f59f9cmr2936787pfv.60.1642096599379; Thu, 13 Jan 2022 09:56:39 -0800 (PST) Received: from p14s (S0106889e681aac74.cg.shawcable.net. [68.147.0.187]) by smtp.gmail.com with ESMTPSA id v22sm3579836pfu.14.2022.01.13.09.56.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Jan 2022 09:56:38 -0800 (PST) Date: Thu, 13 Jan 2022 10:56:36 -0700 From: Mathieu Poirier To: Mike Leach Cc: linux-arm-kernel@lists.infradead.org, coresight@lists.linaro.org, suzuki.poulose@arm.com, leo.yan@linaro.org Subject: Re: [PATCH v2 4/6] coresight: samples: Add an example config writer for configfs load Message-ID: <20220113175636.GA967130@p14s> References: <20211130220100.25888-1-mike.leach@linaro.org> <20211130220100.25888-5-mike.leach@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20211130220100.25888-5-mike.leach@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220113_095641_222787_D55A539F X-CRM114-Status: GOOD ( 40.22 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 > --- > 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. > + * Author: Mike Leach > + */ > + > +#include > + > +#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 > + */ > +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 > + */ > + > +#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 > + */ > + > +#include > +#include > +#include > +#include > + > +#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. > + > +static struct cscfg_config_desc *sample_cfgs[] = { > + &afdo3, > + NULL > +}; Any reason to declare sample_cfgs[] when there can only be one configuration? Lastly I get the following [1] when compiling this set - I have not investigated further. 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel