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 B0701C433F5 for ; Fri, 28 Jan 2022 18:35:18 +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=2ZvS/c5FvrF/v2ch/a/shoVU9FZzi8o4PLijH5pcKrk=; b=bNt3YOf9Q/i3Vp Ky1pufAB1TxTcQoGWd47teq1KIEq7z9y5tWCYBgZ+1NA+wR4Aek6eezfCAJ+z+FTHdBXPTWyC8wfQ EPmx2bMOknE3dOa4u9sAiJRRfQ0cD2I+KIdDL3Z61bexMIKkNuatNCZmTLmyZqVRUOZ56xMYuoVYk NHn3ymF2lJHfVZkvWqvgO6ZxgFmsiF+UIkGYkCXrmJHQYOYyGQu+B0e5HbKvRQu7fTqFl7HtMihs9 Jt28TZSMnWHrqpmGhV9t/L1f2FDe8tuXRWuv6T0ph/BfdOm3b8W9/0aywPs0vEC4rbNBbdgKDIYpX cPpfbK8G/RuQk7hvWjiw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nDW4G-003Lme-VY; Fri, 28 Jan 2022 18:33:53 +0000 Received: from mail-pg1-x52c.google.com ([2607:f8b0:4864:20::52c]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nDW4C-003Llw-SI for linux-arm-kernel@lists.infradead.org; Fri, 28 Jan 2022 18:33:50 +0000 Received: by mail-pg1-x52c.google.com with SMTP id e16so5926671pgn.4 for ; Fri, 28 Jan 2022 10:33:48 -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=oYyTVm3P1XBWImP+p91s2HjmOTKW59K5UBvcb0C1VNc=; b=t0JJMLrp2hXM9nGNn+SHo5WAWPzrKgIDdIpEDB50FJ1PeK0WBMa8XLfNDFvsym3a7I qXVH+HZ4rcthSpIHqcd8qtjonJtRZlu9hKcfo0MBjwlJAuWg0AR8jU5OlXEphJ0WUB1M 96dgcAxgapzWczabPjuv/QhPoP1ZQT8h1kq6vmCqNrgF5SP1aFEZDvab+Zuwp+0ayZnK CYXd7XggDSPSmNq+OCRackWR/IVqLNwBk6gSTjyZgbNpFPI5ymUUo6H9Rn55gKAMqw6k 36CozigG9PMWFxiuUOk9DiTYWFnVcvA9IVDLuk4u3XmVJkm2hW12es4SdOEVJFWe1Mb9 caFg== 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=oYyTVm3P1XBWImP+p91s2HjmOTKW59K5UBvcb0C1VNc=; b=WTq8XykWwMP91IF20ORXVO4NHcl3hy/zKZ97X9Y133FyV8T5APNNt7y2By4k/f3k9w FULPUuC1UENMqr6FftG8AjH8+yI+fIxAR+uNjDXvx/jnQg6WzUmo5JrB0ABw6vvZIK/B /mVT4nkZr/PsYKedfuxTywgigq/ZgxS0otkssUv4GKQpCqch77Sh668Rn8a4TIUgxuuI hTbSsOc1yNHelTSt+Wn1FYuYgemHcccQnJEGl8XCR6YWKF3dLyIfmDfSPjbDxLawdVKM wfEvys5i49pqZ8Bd90joqmsJvnCJyinv5HEj+q5rZRfwWSGT97arpsGGrPHXj0BBcu6j UklQ== X-Gm-Message-State: AOAM533VdS+1wXM8v2+OHg+pIoniEn+M9rQjx2xVhEAEFw3W/J0PDXuL on+usXQhL6yq77LERjJ0gHMRXg== X-Google-Smtp-Source: ABdhPJwps7raaBjEw4SQ18QiHCghTZOV6tYh2PpYtx/UXmv7Haxefl6HckNQ/BrRCT5Gls25FP5cfg== X-Received: by 2002:a63:884a:: with SMTP id l71mr5766470pgd.589.1643394827813; Fri, 28 Jan 2022 10:33:47 -0800 (PST) Received: from p14s (S0106889e681aac74.cg.shawcable.net. [68.147.0.187]) by smtp.gmail.com with ESMTPSA id f22sm8746020pfj.206.2022.01.28.10.33.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 28 Jan 2022 10:33:46 -0800 (PST) Date: Fri, 28 Jan 2022 11:33:44 -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 3/6] coresight: configfs: Modify config files to allow userspace use Message-ID: <20220128183344.GC2240873@p14s> References: <20211130220100.25888-1-mike.leach@linaro.org> <20211130220100.25888-4-mike.leach@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20211130220100.25888-4-mike.leach@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220128_103348_962989_BD0CEEB5 X-CRM114-Status: GOOD ( 39.20 ) 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 On Tue, Nov 30, 2021 at 10:00:57PM +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. > > Signed-off-by: Mike Leach > --- > .../coresight/coresight-config-file.c | 96 +++++++++++++------ > .../coresight/coresight-config-file.h | 33 +++++++ > .../hwtracing/coresight/coresight-config.h | 21 ++++ > 3 files changed, 122 insertions(+), 28 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-config-file.c b/drivers/hwtracing/coresight/coresight-config-file.c > index 3fd001938324..77a64b54280d 100644 > --- a/drivers/hwtracing/coresight/coresight-config-file.c > +++ b/drivers/hwtracing/coresight/coresight-config-file.c > @@ -4,10 +4,58 @@ > * Author: Mike Leach > */ > > -#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__ > #include "coresight-syscfg.h" > > +static void *cscfg_calloc(size_t num, size_t size) > +{ > + return devm_kcalloc(cscfg_device(), num, size, GFP_KERNEL); > +} > + > +static char *cscfg_strdup(const char *str) > +{ > + return devm_kstrdup(cscfg_device(), str, GFP_KERNEL); > +} > + > +static void *cscfg_zalloc(size_t size) > +{ > + return devm_kzalloc(cscfg_device(), size, GFP_KERNEL); > +} > + > +#else > + > +#include > +#include > +#include > + > +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; > +} > + > +#endif > + > #define cscfg_extract_u64(val64) { \ > val64 = *(u64 *)(buffer + used); \ > used += sizeof(u64); \ > @@ -80,6 +128,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 +138,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[elem_str->str_len - 1] != 0) > + if (str[elem_str->str_len - 1] != 0) > return -EINVAL; > > - elem_str->str = devm_kstrdup(cscfg_device(), (char *)buffer, GFP_KERNEL); > + elem_str->str = cscfg_strdup((char *)str); > used += elem_str->str_len; > > *buf_used = used; > @@ -104,14 +155,12 @@ static int cscfg_file_alloc_desc_arrays(struct cscfg_fs_load_descs *desc_arrays, > int nr_features) > { > /* arrays are 0 terminated - max of 1 config & nr_features features */ > - desc_arrays->config_descs = devm_kcalloc(cscfg_device(), 2, > - sizeof(struct cscfg_config_desc *), > - GFP_KERNEL); > + desc_arrays->config_descs = cscfg_calloc(2, sizeof(struct cscfg_config_desc *)); > if (!desc_arrays->config_descs) > return -ENOMEM; > - desc_arrays->feat_descs = devm_kcalloc(cscfg_device(), 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; > @@ -138,8 +187,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 = devm_kzalloc(cscfg_device(), sizeof(struct cscfg_config_desc), > - GFP_KERNEL); > + config_desc = cscfg_zalloc(sizeof(struct cscfg_config_desc)); > if (!config_desc) > return -ENOMEM; > > @@ -165,8 +213,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 = devm_kcalloc(cscfg_device(), nr_preset_vals, > - sizeof(u64), GFP_KERNEL); > + presets = cscfg_calloc(nr_preset_vals, sizeof(u64)); > if (!presets) > return -ENOMEM; > > @@ -181,10 +228,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 = devm_kcalloc(cscfg_device(), > - 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; > > @@ -262,8 +307,7 @@ static int cscfg_file_read_elem_feature(const u8 *buffer, const int buflen, int > u32 val32; > > /* allocate the feature descriptor object */ > - feat_desc = devm_kzalloc(cscfg_device(), sizeof(struct cscfg_feature_desc), > - GFP_KERNEL); > + feat_desc = cscfg_zalloc(sizeof(struct cscfg_feature_desc)); > if (!feat_desc) > return -ENOMEM; > > @@ -302,10 +346,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 = devm_kcalloc(cscfg_device(), > - 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; > > @@ -319,10 +361,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 = devm_kcalloc(cscfg_device(), > - 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++) { > @@ -399,7 +439,7 @@ int cscfg_file_read_buffer(const u8 *buffer, const int buflen, > if (err) > return err; > } > - return used; > + return 0; > } > > int cscfg_file_read_buffer_first_name(const u8 *buffer, const int buflen, > diff --git a/drivers/hwtracing/coresight/coresight-config-file.h b/drivers/hwtracing/coresight/coresight-config-file.h > index 03899f7d94c9..04c365e5109b 100644 > --- a/drivers/hwtracing/coresight/coresight-config-file.h > +++ b/drivers/hwtracing/coresight/coresight-config-file.h > @@ -7,6 +7,39 @@ > #ifndef _CORESIGHT_CORESIGHT_CONFIG_FILE_H > #define _CORESIGHT_CORESIGHT_CONFIG_FILE_H > > +#include > + > +#ifndef __KERNEL__ > +/* > + * allow the user space programs to include the coresight config headers > + * to use the _desc structures, and reuse the read code > + */ > +#ifndef u8 > +typedef __u8 u8; > +typedef __u16 u16; > +typedef __u32 u32; > +typedef __u64 u64; > +#endif > + > +/* ARRAY SIZE is useful too */ > +#ifndef ARRAY_SIZE > +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > +#endif > + > +/* define EINVAL for user space */ > +#ifndef EINVAL > +#define EINVAL 22 > +#endif > + > +#ifndef ENOMEM > +#define ENOMEM 12 > +#endif > + > +#endif > + > +#include "coresight-config.h" > + > + > /* > * Structures to represent configuration descriptors in a memory buffer > * to serialise to and from files > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h > index 770986316bc2..2136393df710 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 > +#endif > + > #include > > /* CoreSight Configuration Management - component and system wide configuration */ > @@ -103,14 +110,18 @@ struct cscfg_regval_desc { > struct cscfg_feature_desc { > const char *name; > const char *description; > +#ifdef __KERNEL__ > struct list_head item; > +#endif > u32 match_flags; > int nr_params; > struct cscfg_parameter_desc *params_desc; > int nr_regs; > struct cscfg_regval_desc *regs_desc; > +#ifdef __KERNEL__ > void *load_owner; > struct config_group *fs_group; > +#endif > }; That will quickly become a maintenance nightmare. I suggest the following: struct cscfg_feature_desc { struct list_head item; void *load_owner; struct config_group *fs_group; struct cscfg_feature_udesc *udesc; }; struct cscfg_feature_udesc { u32 version; 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; }; Structure cscfg_feature_udesc should likely be under include/uapi/linux/coresight-config.h > > /** > @@ -138,16 +149,20 @@ struct cscfg_feature_desc { > struct cscfg_config_desc { > const char *name; > const char *description; > +#ifdef __KERNEL__ > struct list_head item; > +#endif > int nr_feat_refs; > const char **feat_ref_names; > int nr_presets; > int nr_total_params; > const u64 *presets; /* nr_presets * nr_total_params */ > +#ifdef __KERNEL__ > struct dev_ext_attribute *event_ea; > atomic_t active_cnt; > void *load_owner; > struct config_group *fs_group; > +#endif > }; > > /** > @@ -162,11 +177,16 @@ struct cscfg_config_desc { > * @feat_descs: array of feature descriptor pointers. > */ > struct cscfg_fs_load_descs { > +#ifdef __KERNEL__ > void *owner_info; > +#endif > struct cscfg_config_desc **config_descs; > struct cscfg_feature_desc **feat_descs; > }; Same comment as above. > > +/* remainder of header is used by the kernel drivers only */ > +#ifdef __KERNEL__ > + > /** > * config register instance - part of a loaded feature. > * maps register values to csdev driver structures > @@ -274,4 +294,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