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 X-Spam-Level: X-Spam-Status: No, score=-14.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A2AB6C07E99 for ; Fri, 9 Jul 2021 09:34:09 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 6A5126105A for ; Fri, 9 Jul 2021 09:34:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6A5126105A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=hZ0Jsmx/NhTjZu+BOxELEqWwyWGtnm4X8u1mVS+8KBU=; b=T0arFiIvaw0thS aE8Kfne2leIfS5jgweGPqBNc8D5EGKDSHbO/pLRWYVOKooP9bcJK1J2od9EYEDohP0a4mbqqzvK/R fpao5DZksYyX38mDM0oyXkA1+fEJUJMmIA3b92EQ+GWx0xIKh4Lktc9XAFMBx/p6cdZ/foJLSHLzs Zyizi9w2bLd4jwvmBMlP+eqeLtnkcKP0zlNbFIsAvpJA2Ka0uSvU3EdzMHajiehVC9iCQxI8d78P3 oZiwWhd0XdDhlWhDPlJxMGjAMi+hz6pzYUaNJC0u4+13znF8En7jG1IW1XjWD0mf65DN60zywXtTu CnpQnXhDZCFXHPoWXihg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m1ms9-001L2n-PX; Fri, 09 Jul 2021 09:32:37 +0000 Received: from mail-wm1-x32b.google.com ([2a00:1450:4864:20::32b]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m1ms2-001L1Q-8U for linux-arm-kernel@lists.infradead.org; Fri, 09 Jul 2021 09:32:33 +0000 Received: by mail-wm1-x32b.google.com with SMTP id j16-20020a05600c1c10b0290204b096b0caso5844659wms.1 for ; Fri, 09 Jul 2021 02:32:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=wOEcE7jdB5CR8vG66N+pDBDqnCPYwjT896W5Pl44ev0=; b=gDqPsnjzdwpUfhkZwnUCNtAtwjL/DlGtQBLQiympjIsP2GajgVG9IEw1zFYr8ejYK1 /z+pG54Cy2sbZNK811xQ8PKHMGlG4Gc92g3hHXPW5bO35LM3b5G61TToaxH1OmHzOtTx /5lLyWxfr7kc6RXP9acldbVbls0gIjTRPLm2/TtSOOZI3Rw2vslR+Ul7vu1cOmHc0wd2 9uT7Oc3uDGaAVPde6xjtVBtUZXodEh5DhBgSdEAhF3MjEoIbQ7/Xk7fDiPSLa5tNWLJ/ iFo7xP4ZPNRb1AHZfy+Z0PdKgauPta8a8WJu2ao6at4GUa8ZNeZEuoa4rvQES4ZzAoKc KnfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=wOEcE7jdB5CR8vG66N+pDBDqnCPYwjT896W5Pl44ev0=; b=DG+N7Awj0Z63O5VEKNtPQvrRaS3ba6GnegqtX1vNT+I+x2S+6oZzDMVgHjSMj+UZ5a 5Pm7L3XUk3joj/NkXn9no6sY7/kCFE0aoApaUrwkCbAdvkEN7Ob9HeJZ240BO4kmDp2I SJGPzU67G8Sc7dNYtyhETWcmRWmEdcLvBRUA3UAXB+liWNm91mhF/cE++ZodIJ2oB+gT RjiX/aWMD86FgfvzqVbSbr1JNib9ZepX9As0YO0BHlzyhTrwqBr4WmDlcJ+CV42/AUsk 6Kx+yFT3oHjAYhC6vXCPLySafDbew+PgRHPKd+B9LBpPMkhBSD0/SrYO4pYqn82ayeTN dJ/g== X-Gm-Message-State: AOAM531DSjaFsxnv84bC6OG+3Uc+vy3moC3Bf6ZMZFDMj+lG+6PFJXea GDH008ZSgC/BVJPekYix5MBQWXzDY2lrXOGg0wl7HQ== X-Google-Smtp-Source: ABdhPJxXpwgVOMv7zCo5UCnFxyD0T3P97dbRkoStg7pM81gGOBl3a1xGQO3qEhVUQpw+aiY3/MRtdV+EPqA6HByeyf0= X-Received: by 2002:a7b:c402:: with SMTP id k2mr39561006wmi.5.1625823148546; Fri, 09 Jul 2021 02:32:28 -0700 (PDT) MIME-Version: 1.0 References: <20210512211752.4103-1-mike.leach@linaro.org> <20210512211752.4103-7-mike.leach@linaro.org> <20210528161704.GA1355502@xps15> In-Reply-To: <20210528161704.GA1355502@xps15> From: Mike Leach Date: Fri, 9 Jul 2021 10:32:17 +0100 Message-ID: Subject: Re: [RFC PATCH 6/8] coresight: etm4x: syscfg: Add resource management to etm4x. To: Mathieu Poirier Cc: Coresight ML , linux-arm-kernel , "Suzuki K. Poulose" , Leo Yan X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210709_023230_386641_1ED8F25F X-CRM114-Status: GOOD ( 49.97 ) 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 HI Mathieu, On Fri, 28 May 2021 at 17:17, Mathieu Poirier wrote: > > On Wed, May 12, 2021 at 10:17:50PM +0100, Mike Leach wrote: > > Adds resource management to configuration and feature handling for ETM4 > > using the system configuration resource management API. > > > > Allows specification of ETM4 resources when creating configurations > > and features. > > > > Adds in checking and validation of resources used by features to > > prevent over allocation when multiple features used in a configuration. > > > > Signed-off-by: Mike Leach > > --- > > .../hwtracing/coresight/coresight-etm4x-cfg.c | 533 ++++++++++++++++++ > > .../hwtracing/coresight/coresight-etm4x-cfg.h | 196 ++++++- > > include/linux/coresight.h | 2 + > > 3 files changed, 724 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > > index d2ea903231b2..ba6d20b58a9a 100644 > > --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > > +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.c > > @@ -28,6 +28,11 @@ > > } \ > > } > > > > +/* check for a match to ts rate */ > > +static bool etm4_cfg_feat_is_ts_rate(const struct cscfg_feature_desc *feat_desc); > > + > > +#define TS_RATE_REG_VAL_IDX 0 > > + > > /** > > * etm4_cfg_map_reg_offset - validate and map the register offset into a > > * location in the driver config struct. > > @@ -128,6 +133,66 @@ static int etm4_cfg_map_reg_offset(struct etmv4_drvdata *drvdata, > > return err; > > } > > > > +static void etm4_cfg_dump_res_mask(const char *name, struct etm4_cfg_resources *res) > > +{ > > + pr_debug("Mask %s\n", name); > > + pr_debug("selectors %08x; addr_cmp %04x\n", res->selectors, res->addr_cmp); > > + pr_debug("cid_cmp %02x; vmid_cmp %02x; counters %02x\n", res->cid_cmp, > > + res->vmid_cmp, res->counters); > > + pr_debug("misc bits %08x\n", res->misc); > > +} > > + > > +/* > > + * generate an address offset from a resource type and index > > + * Bit selected resources will return a ETM4_RES_OFFSET_SKIP value > > + * as these require special handling on enable / disable. > > + */ > > +static u32 etm4_cfg_get_res_offset(u16 res_type, u32 res_idx) > > +{ > > + u32 offset = ETM4_RES_OFFSET_ERR; > > + > > + switch (res_type & ETM4_CFG_RES_MASK) { > > + case ETM4_CFG_RES_CTR: > > + if (res_type & ETM4_CTR_VAL) > > + offset = TRCCNTVRn(res_idx); > > + else if (res_type & ETM4_CTR_RLD) > > + offset = TRCCNTRLDVRn(res_idx); > > + else if (res_type & ETM4_CTR_CTRL) > > + offset = TRCCNTCTLRn(res_idx); > > + break; > > + case ETM4_CFG_RES_CMP: > > + if (res_type & ETM4_CMP_VAL) > > + offset = TRCACVRn(res_idx); > > + else if (res_type & ETM4_CMP_CTL) > > + offset = TRCACATRn(res_idx); > > + break; > > + case ETM4_CFG_RES_SEL: > > + offset = TRCRSCTLRn(res_idx); > > + break; > > + > > + case ETM4_CFG_RES_SEQ: > > + if (res_type & ETM4_SEQ_STATE_R) > > + offset = TRCSEQEVRn(res_idx); > > + else if (res_type & ETM4_SEQ_RESET_R) > > + offset = TRCSEQRSTEVR; > > + break; > > + case ETM4_CFG_RES_CID_CMP: > > + offset = TRCCIDCVRn(res_idx); > > + break; > > + > > + case ETM4_CFG_RES_VID_CMP: > > + offset = TRCVMIDCVRn(res_idx); > > + break; > > + > > + /* these two have dedicated enable functions, no address needed */ > > + case ETM4_CFG_RES_BITCTRL: > > fallthrough; > OK > > + case ETM4_CFG_RES_TS: > > + offset = ETM4_RES_OFFSET_SKIP; > > + break; > > + } > > + return offset; > > +} > > + > > /** > > * etm4_cfg_load_feature - load a feature into a device instance. > > * > > @@ -163,11 +228,349 @@ static int etm4_cfg_load_feature(struct coresight_device *csdev, > > /* process the register descriptions */ > > for (i = 0; i < feat_csdev->nr_regs && !err; i++) { > > offset = feat_desc->regs_desc[i].offset; > > + > > + /* resource needs conversion to a register access value */ > > + if (feat_desc->regs_desc[i].type & CS_CFG_REG_TYPE_RESOURCE) { > > + offset = etm4_cfg_get_res_offset(feat_desc->regs_desc[i].hw_info, > > + offset); > > + if (offset == ETM4_RES_OFFSET_ERR) { > > + err = -ENODEV; > > + break; > > + } else if (offset == ETM4_RES_OFFSET_SKIP) > > + continue; > > + } > > err = etm4_cfg_map_reg_offset(drvdata, &feat_csdev->regs_csdev[i], offset); > > } > > return err; > > } > > > > +/* > > + * ts rate - set a counter to emit timestamp requests at a set interval. > > + * if we have sufficient resources then we use a counter and resource > > + * selector to achieve this. > > + * > > + * However, if not then do the best possible - which prevents the perf > > + * event timestamp request from failing if any configuration selection > > + * is using resources. e.g. when profiling, timestamps do not really matter. > > To me it seems like we are adding too much intelligence, but we can have this > conversation at a later time. > As noted below - this tries to balance the background perf resource requirements with the explicit user requirements. > > + */ > > +void etm4_cfg_set_ts_rate(struct coresight_device *csdev, u32 ts_rate_val) > > +{ > > + struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > > + struct etmv4_config *drvcfg = &drvdata->config; > > + struct cscfg_res_impl_used *res_impl_used; > > + int counter_idx, res_sel_idx; > > + u32 tsctlr_val = 0; > > + > > + res_impl_used = (struct cscfg_res_impl_used *)csdev->cscfg_res_mask; > > + > > + /* look for resources */ > > + counter_idx = etm4_res_find_counter(res_impl_used); > > + res_sel_idx = etm4_res_find_selector(res_impl_used); > > + if (counter_idx >= 0 && res_sel_idx >= 0) { > > + /* counter and selector - can set up ts rate normally */ > > + /* > > + * counter @ 1 and reload @ rate supplied - > > + * immediate timestamp then every rate > > + */ > > + drvcfg->cntr_val[counter_idx] = 1; > > + drvcfg->cntrldvr[counter_idx] = ts_rate_val; > > + /* > > + * counter ctrl - bit 16: 1 for reload self, > > + * bit 7: 0 single event, > > + * bit 6:0 res sel 1 - true > > + */ > > + drvcfg->cntr_ctrl[counter_idx] = 0x1 << 16 | 0x1; > > + > > + /* > > + * set up resource selector for the counter. > > + * bits 19:16 - group 0b0010 counter > > + * bits 15:0 - bit select for counter idx > > + */ > > + drvcfg->res_ctrl[res_sel_idx] = (0x2 << 16) | (0x1 << counter_idx); > > + > > + /* single selector bit 7 == 0, bit 6:0 - selector index */ > > + tsctlr_val = res_sel_idx; > > + > > + } else if (ts_rate_val == 1) { > > + /* > > + * perf always tries to use a min value - > > + * emulate by setting the ts event to true > > + */ > > + /* single selector bit 7 == 0, bit 6:0 - selector 1 - always true */ > > + tsctlr_val = 0x1; > > + } > > + > > + /* set the configr reg to enable TS, and the ts control reg */ > > + drvcfg->ts_ctrl = tsctlr_val; > > + drvcfg->cfg |= BIT(11); > > +} > > + > > +/* > > + * on enable a feature - called after generic routine has programmed other registers. > > + * handle bit selects and custom elements > > + */ > > +static int etm4_cfg_on_enable_feat(struct cscfg_feature_csdev *feat_csdev) > > +{ > > + int err = 0; > > + struct etm4_cfg_resources *res_feat; > > + struct device *dev = feat_csdev->csdev->dev.parent; > > + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); > > + struct etmv4_config *drvcfg = &drvdata->config; > > + u32 ts_rate_val; > > + > > + /* > > + * look for the bit selected resources in this feature and set driver > > + * values to be programmed when enabling hardware. > > + */ > > + res_feat = (struct etm4_cfg_resources *)feat_csdev->res_used; > > + > > + /* if none of the bit selected resources in use, exit early */ > > + if (!res_feat->misc) > > + return 0; > > + > > + /* otherwise check each and set as required */ > > + if (res_feat->ctxt_id) > > + drvcfg->cfg |= BIT(6); > > + > > + if (res_feat->vm_id) > > + drvcfg->cfg |= BIT(7); > > + > > + /* return stack is bit 12 in config register */ > > + if (res_feat->return_stack) > > + drvcfg->cfg |= BIT(12); > > + > > + /* branch broadcast - feature using this must program the bbctlr */ > > + if (res_feat->branch_broadcast) > > + drvcfg->cfg |= BIT(3); > > + > > + /* cycle count */ > > + if (res_feat->cycle_cnt) { > > + drvcfg->cfg |= BIT(4); > > + /* TRM: Must program this for cycacc to work - ensure mun permitted */ > > + if (drvcfg->ccctlr < drvdata->ccitmin) > > + drvcfg->ccctlr = drvdata->ccitmin; > > + } > > + > > + /* > > + * timestamps - if not ts-rate just set to on, otherwise > > + * set using reload counter according to requested rate > > + */ > > + if (res_feat->timestamp) { > > + /* the current feature is the ts-rate feature */ > > + if (res_feat->ts_rate) { > > + ts_rate_val = feat_csdev->regs_csdev[TS_RATE_REG_VAL_IDX].reg_desc.val32; > > + etm4_cfg_set_ts_rate(feat_csdev->csdev, ts_rate_val); > > I would like to see a better way to do this. Setting the timestamp rate > should be treated like any other feature. If the current framework doesn't > allow for it then it should be extended. > see the comment below. > > + } else > > + drvcfg->cfg |= BIT(11); > > + } > > + return err; > > +} > > + > > +/* set the overall available resource masks for the device */ > > +static int etm4_cfg_set_res_mask(struct coresight_device *csdev) > > +{ > > + struct device *dev = csdev->dev.parent; > > + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); > > + struct etm4_cfg_resources *res; > > + struct cscfg_res_impl_used *res_impl_used; > > + > > + res_impl_used = devm_kzalloc(dev, sizeof(*res_impl_used), GFP_KERNEL); > > + if (!res_impl_used) > > + return -ENOMEM; > > + res = &res_impl_used->impl; > > + > > + /* selectors */ > > + if (drvdata->nr_resource) > > + res->selectors = GENMASK((drvdata->nr_resource * 2) - 1, 0); > > + > > + /* comparators */ > > + if (drvdata->nr_addr_cmp) > > + res->addr_cmp = GENMASK(drvdata->nr_addr_cmp - 1, 0); > > + > > + if (drvdata->numvmidc) > > + res->vmid_cmp = GENMASK(drvdata->numvmidc - 1, 0); > > + > > + if (drvdata->numcidc) > > + res->cid_cmp = GENMASK(drvdata->numcidc - 1, 0); > > + > > + /* misc resources */ > > + if (drvdata->nr_cntr) > > + res->counters = GENMASK(drvdata->nr_cntr - 1, 0); > > + > > + if (drvdata->trccci) > > + res->cycle_cnt = 1; > > + > > + if (drvdata->trcbb) > > + res->branch_broadcast = 1; > > + > > + if (drvdata->ctxid_size) > > + res->ctxt_id = 1; > > + > > + if (drvdata->vmid_size) > > + res->vm_id = 1; > > + > > + if (drvdata->nrseqstate) > > + res->sequencer = 1; > > + > > + if (drvdata->retstack) > > + res->return_stack = 1; > > + > > + if (drvdata->ts_size) { > > + res->timestamp = 1; > > + if (drvdata->nr_cntr && drvdata->nr_resource) > > + res->ts_rate = 1; > > + } > > + etm4_cfg_dump_res_mask("device impl resources", &res_impl_used->impl); > > + csdev->cscfg_res_mask = res_impl_used; > > + return 0; > > +} > > + > > +/* > > + * reads a descriptor and updates the resource mask structure > > + * checks resource indexes are valid. > > + */ > > +static int etm4_cfg_update_res_from_desc(const struct cscfg_feature_desc *feat_desc, > > + struct etm4_cfg_resources *res) > > +{ > > + struct cscfg_regval_desc *regs_desc = &feat_desc->regs_desc[0]; > > + u32 res_idx, hw_info; > > + int i; > > + > > + for (i = 0; i < feat_desc->nr_regs; i++) { > > + if (regs_desc[i].type & CS_CFG_REG_TYPE_RESOURCE) { > > + res_idx = regs_desc[i].offset; > > + hw_info = regs_desc[i].hw_info; > > + switch (hw_info & ETM4_CFG_RES_MASK) { > > + case ETM4_CFG_RES_CTR: > > + if (res_idx >= ETMv4_MAX_CNTR) > > + goto invalid_resource_err; > > + res->counters |= BIT(res_idx); > > + break; > > + > > + case ETM4_CFG_RES_CMP: > > + if (res_idx >= ETM_MAX_SINGLE_ADDR_CMP) > > + goto invalid_resource_err; > > + res->addr_cmp |= BIT(res_idx); > > + break; > > + > > + case ETM4_CFG_RES_SEL: > > + if (res_idx >= ETM_MAX_RES_SEL) > > + goto invalid_resource_err; > > + res->selectors |= BIT(res_idx); > > + break; > > + > > + case ETM4_CFG_RES_SEQ: > > + res->sequencer = 1; > > + break; > > + > > + case ETM4_CFG_RES_TS: > > + res->timestamp = 1; > > + if (etm4_cfg_feat_is_ts_rate(feat_desc)) > > + res->ts_rate = 1; > > + break; > > + > > + case ETM4_CFG_RES_BITCTRL: > > + if (hw_info & ETM4_BITCTRL_BRANCH_BROADCAST) > > + res->branch_broadcast = 1; > > + if (hw_info & ETM4_BITCTRL_CYCLE_COUNT) > > + res->cycle_cnt = 1; > > + if (hw_info & ETM4_BITCTRL_CTXTID) > > + res->ctxt_id = 1; > > + if (hw_info & ETM4_BITCTRL_VMID) > > + res->vm_id = 1; > > + if (hw_info & ETM4_BITCTRL_RETSTACK) > > + res->return_stack = 1; > > + break; > > + > > + case ETM4_CFG_RES_CID_CMP: > > + if (res_idx >= ETMv4_MAX_CTXID_CMP) > > + goto invalid_resource_err; > > + res->cid_cmp |= BIT(res_idx); > > + break; > > + > > + case ETM4_CFG_RES_VID_CMP: > > + if (res_idx >= ETM_MAX_VMID_CMP) > > + goto invalid_resource_err; > > + res->vmid_cmp |= BIT(res_idx); > > + break; > > + } > > + } > > + } > > + return 0; > > + > > +invalid_resource_err: > > + pr_err("Error: Invalid resource values in feature %s\n", feat_desc->name); > > + return -EINVAL; > > +} > > +/* > > + * Check that the device contains the minimum resources required to support the > > + * described @feat_desc. Return -ENODEV if missing required resources. > > + */ > > +static int etm4_cfg_check_feat_res(struct coresight_device *csdev, > > + struct cscfg_feature_desc *feat_desc) > > +{ > > + struct etm4_cfg_resources req_res; > > + struct cscfg_res_impl_used *dev_res; > > + int err; > > + > > + /* create a resource mask from descriptor and validate */ > > + memset(&req_res, 0, sizeof(req_res)); > > + err = etm4_cfg_update_res_from_desc(feat_desc, &req_res); > > + etm4_cfg_dump_res_mask("check_feat_res", &req_res); > > + if (!err) { > > + dev_res = (struct cscfg_res_impl_used *)csdev->cscfg_res_mask; > > + if (!etm4_cfg_check_impl(&dev_res->impl, &req_res)) > > + return -ENODEV; > > + } > > + return err; > > +} > > + > > +/* > > + * Allocate resource requirements for the feature before > > + * it is programmed into the system. Ensures that two or more features in a > > + * configuration do not try to use the same resources on the device. > > + * > > + * At this point we use the absolute programmed resources - we do not attempt > > + * to find alternate available resources. (e.g. if 2 features use selector 3, > > + * fail the 2nd feature - do not look for an alternative free selector). > > + */ > > +static int etm4_cfg_alloc_feat_res(struct cscfg_feature_csdev *feat_csdev) > > +{ > > + struct coresight_device *csdev = feat_csdev->csdev; > > + struct device *dev = csdev->dev.parent; > > + struct etm4_cfg_resources *res_feat, *res_inuse; > > + int err = 0; > > + > > + /* one off initialisation of resources required for this feature */ > > + if (!feat_csdev->res_used) { > > + res_feat = devm_kzalloc(dev, sizeof(*res_feat), GFP_KERNEL); > > + if (!res_feat) > > + return -ENOMEM; > > + err = etm4_cfg_update_res_from_desc(feat_csdev->feat_desc, res_feat); > > + if (err) > > + return err; > > + feat_csdev->res_used = res_feat; > > + } else > > + res_feat = (struct etm4_cfg_resources *)feat_csdev->res_used; > > + > > + /* check that the device resources reqiured are not in use */ > > + res_inuse = &((struct cscfg_res_impl_used *)csdev->cscfg_res_mask)->used; > > + if (!etm4_cfg_check_set_inuse(res_inuse, res_feat)) > > + err = -ENOSPC; > > + > > + return err; > > +} > > + > > +static void etm4_cfg_clear_feat_res(struct cscfg_feature_csdev *feat_csdev) > > +{ > > + struct coresight_device *csdev = feat_csdev->csdev; > > + struct etm4_cfg_resources *res_feat, *res_inuse; > > + > > + res_feat = (struct etm4_cfg_resources *)feat_csdev->res_used; > > + res_inuse = &((struct cscfg_res_impl_used *)csdev->cscfg_res_mask)->used; > > + etm4_cfg_clear_inuse(res_inuse, res_feat); > > +} > > + > > /* match information when loading configurations */ > > #define CS_CFG_ETM4_MATCH_FLAGS (CS_CFG_MATCH_CLASS_SRC_ALL | \ > > CS_CFG_MATCH_CLASS_SRC_ETM4) > > @@ -175,8 +578,138 @@ static int etm4_cfg_load_feature(struct coresight_device *csdev, > > int etm4_cscfg_register(struct coresight_device *csdev) > > { > > struct cscfg_csdev_feat_ops ops; > > + int err = 0; > > + > > + err = etm4_cfg_set_res_mask(csdev); > > + if (err) > > + return err; > > > > ops.load_feat = &etm4_cfg_load_feature; > > + ops.check_feat_res = &etm4_cfg_check_feat_res; > > + ops.alloc_feat_res = &etm4_cfg_alloc_feat_res; > > + ops.clear_feat_res = &etm4_cfg_clear_feat_res; > > + ops.set_on_enable = &etm4_cfg_on_enable_feat; > > + ops.clear_on_disable = 0; > > > > return cscfg_register_csdev(csdev, CS_CFG_ETM4_MATCH_FLAGS, &ops); > > } > > + > > +/* > > + * find first available bit in implemented mask @impl, that is not set in @used mask. > > + * set bit in @used and return. Return -ENOSPC if no available bits. > > + */ > > +int etm4_cfg_find_unused_idx(unsigned long *impl, unsigned long *used, int size) > > +{ > > + unsigned long end_idx, unused_idx; > > + > > + end_idx = find_first_zero_bit(impl, size); > > + unused_idx = find_first_zero_bit(used, size); > > + if (unused_idx < end_idx) { > > + *used |= BIT(unused_idx); > > + return (int)unused_idx; > > + } > > + return -ENOSPC; > > +} > > + > > +/* > > + * find first available pair of bits in implemented mask @impl, that are not set in > > + * @used mask. First bit of pair will always be an even index. > > + * Set bits in @used and return. Return -ENOSPC if no available bits. > > + */ > > +int etm4_cfg_find_unused_idx_pair(unsigned long *impl, unsigned long *used, int size) > > +{ > > + unsigned long end_idx, first_unused_idx, next_unused_idx; > > + > > + end_idx = find_first_zero_bit(impl, size); > > + first_unused_idx = find_first_zero_bit(used, size); > > + > > + /* > > + * even indexes are the 1st in a pair, look through the comparators > > + * till a pair found or we are at the end of the list. > > + */ > > + while (first_unused_idx < end_idx) { > > + /* first is an even number, if the next is free we have a pair */ > > + if (!(first_unused_idx % 2)) { > > + next_unused_idx = find_next_zero_bit(used, size, first_unused_idx); > > + if (next_unused_idx == (first_unused_idx + 1)) { > > + *used |= BIT(first_unused_idx); > > + *used |= BIT(next_unused_idx); > > + return (int)first_unused_idx; > > + } > > + first_unused_idx = next_unused_idx; > > + } else > > + first_unused_idx = find_next_zero_bit(used, size, first_unused_idx); > > + } > > + return -ENOSPC; > > +} > > + > > + > > +/* built in timestamp rate for etm4x */ > > +static struct cscfg_parameter_desc ts_rate_param[] = { > > + { > > + .name = "ts_rate_cycles", > > + .value = 100. > > + }, > > +}; > > + > > +static struct cscfg_regval_desc ts_rate_regs[] = { > > + { > > + .type = CS_CFG_REG_TYPE_RESOURCE | CS_CFG_REG_TYPE_VAL_PARAM, > > + .offset = 0, > > + .hw_info = ETM4_CFG_RES_TS, > > + .param_idx = 0, > > + }, > > +}; > > + > > +static struct cscfg_feature_desc ts_rate_etm4x = { > > + .name = "timestamp-rate", > > + .description = "Enable timestamps and set rate they appear in the trace.\n" > > + "Rate value is number of cycles between timestamp requests. Min value 1.\n", > > + .match_flags = CS_CFG_MATCH_CLASS_SRC_ETM4, > > + .nr_params = ARRAY_SIZE(ts_rate_param), > > + .params_desc = ts_rate_param, > > + .nr_regs = ARRAY_SIZE(ts_rate_regs), > > + .regs_desc = ts_rate_regs, > > +}; > > I vote for leaving the timestamp rate feature out of this set. It is > introducing a fair amount of complexity to a patchset that is already complex. > When we move forward with it, it should be in its own file like we did for > autoFDO. > ts rate was introduced as a way of managing the built-in requirement in perf enable that it would insist on a counted timestamp when in cpu wide mode for correlation, or fail to run. To manage these expectations with that of any configuration competing for the resource, ts_rate is treated in a way that it will degrade gracefully it all the resources are not available. This would mean that the perf enable event could try to set it, but configurations would take priority. Looking at this now, it may be better to remove this as a feature as you suggest, and manage the perf requirements as part of the normal resource managment - with the caveat that if a user selects a configuration, then perf will not refuse to start the event if a counted timestamp cannot be implemented. This will avoid unexplained unavailable resource errors were the perf event takes resources in the background. (this is in fact the behaviour of the baseline set - though in this case a config simply overrides perfs counter usage). Thanks Mike > I am out of time for this set and as such will stop here. We can tackle the > rest in future revisions. > > Thanks, > Mathieu > > > + > > +static struct cscfg_feature_desc *etm4x_feats[] = { > > + &ts_rate_etm4x, > > + NULL, > > +}; > > + > > +static struct cscfg_config_desc *etm4x_cfgs[] = { > > + NULL, > > +}; > > + > > +static struct cscfg_load_owner_info etm4x_mod_owner = { > > + .type = CSCFG_OWNER_MODULE, > > + .owner_handle = THIS_MODULE, > > +}; > > + > > +/* > > + * check if incoming feature is ts-rate > > + */ > > +static bool etm4_cfg_feat_is_ts_rate(const struct cscfg_feature_desc *feat_desc) > > +{ > > + if (!strcmp(feat_desc->name, ts_rate_etm4x.name)) > > + return true; > > + return false; > > +} > > + > > +/* load the etm4 builtin ts_rate feature into the system */ > > +int etm4_cscfg_load_builtin_cfg(void) > > +{ > > + int err; > > + > > + err = cscfg_load_config_sets(etm4x_cfgs, etm4x_feats, &etm4x_mod_owner); > > + > > + /* if currently loaded matching devs ts_rate, still allow to load */ > > + if (err == -ENODEV) > > + err = 0; > > + return err; > > +} > > + > > +void etm4_cscfg_unload_builtin_cfg(void) > > +{ > > + cscfg_unload_config_sets(&etm4x_mod_owner); > > +} > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-cfg.h b/drivers/hwtracing/coresight/coresight-etm4x-cfg.h > > index 32dab34c1dac..dd69a8ef522d 100644 > > --- a/drivers/hwtracing/coresight/coresight-etm4x-cfg.h > > +++ b/drivers/hwtracing/coresight/coresight-etm4x-cfg.h > > @@ -13,18 +13,200 @@ > > > > /* resource IDs */ > > > > +/* > > + * 12 bit resource ID: > > + * 3:0 = resource type in use. > > + * 11:4 = additional resource specific information. > > + */ > > #define ETM4_CFG_RES_CTR 0x001 > > #define ETM4_CFG_RES_CMP 0x002 > > -#define ETM4_CFG_RES_CMP_PAIR0 0x003 > > -#define ETM4_CFG_RES_CMP_PAIR1 0x004 > > -#define ETM4_CFG_RES_SEL 0x005 > > -#define ETM4_CFG_RES_SEL_PAIR0 0x006 > > -#define ETM4_CFG_RES_SEL_PAIR1 0x007 > > -#define ETM4_CFG_RES_SEQ 0x008 > > -#define ETM4_CFG_RES_TS 0x009 > > +#define ETM4_CFG_RES_SEL 0x003 > > +#define ETM4_CFG_RES_SEQ 0x004 > > +#define ETM4_CFG_RES_TS 0x005 > > +#define ETM4_CFG_RES_BITCTRL 0x006 > > +#define ETM4_CFG_RES_CID_CMP 0x007 > > +#define ETM4_CFG_RES_VID_CMP 0x008 > > #define ETM4_CFG_RES_MASK 0x00F > > > > +/* additional bits to supplement _CFG_RES_CTR */ > > +#define ETM4_CTR_VAL 0x010 > > +#define ETM4_CTR_RLD 0x020 > > +#define ETM4_CTR_CTRL 0x040 > > + > > +/* additional bits for address comparators _CFG_RES_CMP */ > > +#define ETM4_CMP_PAIR0 0x010 > > +#define ETM4_CMP_PAIR1 0x020 > > +#define ETM4_CMP_VAL 0x040 > > +#define ETM4_CMP_CTL 0x080 > > + > > +/* additional bits for resource selectors _CFG_RES_SEL */ > > +#define ETM4_SEL_PAIR0 0x010 > > +#define ETM4_SEL_PAIR1 0x020 > > + > > +/* addtional bits for sequencer _CFG_RES_SEQ */ > > +#define ETM4_SEQ_STATE_R 0x010 > > +#define ETM4_SEQ_RESET_R 0x020 > > + > > +/* additional bits to supplement _CFG_RES_BITCTRL */ > > +#define ETM4_BITCTRL_BRANCH_BROADCAST 0x010 > > +#define ETM4_BITCTRL_CYCLE_COUNT 0x020 > > +#define ETM4_BITCTRL_CTXTID 0x040 > > +#define ETM4_BITCTRL_VMID 0x080 > > +#define ETM4_BITCTRL_RETSTACK 0x100 > > + > > +/* error value when calculating resource register offset (max offset = 0xFFC) */ > > +#define ETM4_RES_OFFSET_ERR 0xFFF > > + > > +/* skip value if a bit control that is resolved later */ > > +#define ETM4_RES_OFFSET_SKIP 0xFFE > > + > > +/** > > + * Masks to indicate resource usage. > > + * @selectors: The resource selector regs - max 32 off > > + * @comparators: Comparators - address (16 max), context ID (8 max), VMID (8 max). > > + * @misc:- bitselected features, sequencer etc. > > + */ > > +struct etm4_cfg_resources { > > + u32 selectors; > > + u16 addr_cmp; > > + u8 cid_cmp; > > + u8 vmid_cmp; > > + u8 counters; > > + union { > > + u32 misc; > > + struct { > > + u32 cycle_cnt:1; > > + u32 branch_broadcast:1; > > + u32 ctxt_id:1; > > + u32 vm_id:1; > > + u32 sequencer:1; > > + u32 return_stack:1; > > + u32 timestamp:1; > > + u32 ts_rate:1; > > + }; > > + }; > > +}; > > + > > +/* structure to hold implemented & used resources for the coresight device */ > > +struct cscfg_res_impl_used { > > + struct etm4_cfg_resources impl; > > + struct etm4_cfg_resources used; > > +}; > > + > > +/* resource mask tests */ > > +/* check implmented - ensure that all bits in @req exist in @impl */ > > +static inline bool etm4_cfg_check_impl(struct etm4_cfg_resources *impl, > > + struct etm4_cfg_resources *req) > > +{ > > + /* invert impl then and req - anything set is outside impl mask */ > > + if ((~impl->selectors & req->selectors) || > > + (~impl->addr_cmp & req->addr_cmp) || > > + (~impl->cid_cmp & req->cid_cmp) || > > + (~impl->vmid_cmp & req->vmid_cmp) || > > + (~impl->counters & req->counters) || > > + (~impl->misc & req->misc)) > > + return false; > > + return true; > > +} > > + > > +/* check @req not @inuse, & set @inuse if free (assumes @req passed the impl check) */ > > +static inline bool etm4_cfg_check_set_inuse(struct etm4_cfg_resources *inuse, > > + struct etm4_cfg_resources *req) > > +{ > > + /* first check for hits between inuse and requested bits */ > > + if ((inuse->selectors & req->selectors) || > > + (inuse->addr_cmp & req->addr_cmp) || > > + (inuse->cid_cmp & req->cid_cmp) || > > + (inuse->vmid_cmp & req->vmid_cmp) || > > + (inuse->counters & req->counters) || > > + (inuse->misc & req->misc)) > > + return false; > > + > > + /* set all requested bits as inuse */ > > + inuse->selectors |= req->selectors; > > + inuse->addr_cmp |= req->addr_cmp; > > + inuse->cid_cmp |= req->cid_cmp; > > + inuse->vmid_cmp |= req->vmid_cmp; > > + inuse->counters |= req->counters; > > + inuse->misc |= req->misc; > > + return true; > > +} > > + > > +static inline void etm4_cfg_clear_inuse(struct etm4_cfg_resources *inuse, > > + struct etm4_cfg_resources *req) > > +{ > > + /* clear requested bits from inuse */ > > + inuse->selectors &= ~req->selectors; > > + inuse->addr_cmp &= ~req->addr_cmp; > > + inuse->cid_cmp &= ~req->cid_cmp; > > + inuse->vmid_cmp &= ~req->vmid_cmp; > > + inuse->counters &= ~req->counters; > > + inuse->misc &= ~req->misc; > > +} > > + > > /* ETMv4 specific config functions */ > > int etm4_cscfg_register(struct coresight_device *csdev); > > +int etm4_cfg_find_unused_idx(unsigned long *impl, unsigned long *used, int size); > > +int etm4_cfg_find_unused_idx_pair(unsigned long *impl, unsigned long *used, int size); > > +void etm4_cfg_set_ts_rate(struct coresight_device *csdev, u32 ts_rate_val); > > + > > +/* register etm4x builtins with cscfg on module load */ > > +int etm4_cscfg_load_builtin_cfg(void); > > +void etm4_cscfg_unload_builtin_cfg(void); > > + > > +/* > > + * Set of functions to find an available resource from @res->impl, not already marked as used > > + * in @res->used. > > + * return index and mark as used in @res->used. return -ENOSPC if nothing available. > > + */ > > + > > +static inline int etm4_res_find_selector(struct cscfg_res_impl_used *res) > > +{ > > + unsigned long *impl, *used; > > + > > + if (!res->impl.selectors) > > + return -ENOSPC; > > + > > + impl = (unsigned long *)&res->impl.selectors; > > + used = (unsigned long *)&res->used.selectors; > > + return etm4_cfg_find_unused_idx(impl, used, ETM_MAX_RES_SEL); > > +} > > + > > +static inline int etm4_res_find_counter(struct cscfg_res_impl_used *res) > > +{ > > + unsigned long *impl, *used; > > + > > + if (!res->impl.counters) > > + return -ENOSPC; > > + > > + impl = (unsigned long *)&res->impl.counters; > > + used = (unsigned long *)&res->used.counters; > > + return etm4_cfg_find_unused_idx(impl, used, ETMv4_MAX_CNTR); > > +} > > + > > +static inline int etm4_res_find_addr_comparator(struct cscfg_res_impl_used *res) > > +{ > > + unsigned long *impl, *used; > > + > > + if (!res->impl.addr_cmp) > > + return -ENOSPC; > > + > > + impl = (unsigned long *)&res->impl.addr_cmp; > > + used = (unsigned long *)&res->used.addr_cmp; > > + return etm4_cfg_find_unused_idx(impl, used, ETM_MAX_SINGLE_ADDR_CMP); > > +} > > + > > + > > +static inline int etm4_res_find_addr_comp_pair(struct cscfg_res_impl_used *res) > > +{ > > + unsigned long *impl, *used; > > + > > + if (!res->impl.addr_cmp) > > + return -ENOSPC; > > + > > + impl = (unsigned long *)&res->impl.addr_cmp; > > + used = (unsigned long *)&res->used.addr_cmp; > > + return etm4_cfg_find_unused_idx_pair(impl, used, ETM_MAX_SINGLE_ADDR_CMP); > > +} > > > > #endif /* CORESIGHT_ETM4X_CFG_H */ > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > > index a348049ee08b..b513964b9305 100644 > > --- a/include/linux/coresight.h > > +++ b/include/linux/coresight.h > > @@ -223,6 +223,7 @@ struct coresight_sysfs_link { > > * @feature_csdev_list: List of complex feature programming added to the device. > > * @config_csdev_list: List of system configurations added to the device. > > * @active_cscfg_ctxt: Context information for current active system configuration. > > + * @cscfg_res_mask: Available device specific resources usable in features. > > */ > > struct coresight_device { > > struct coresight_platform_data *pdata; > > @@ -248,6 +249,7 @@ struct coresight_device { > > struct list_head feature_csdev_list; > > struct list_head config_csdev_list; > > void *active_cscfg_ctxt; > > + void *cscfg_res_mask; > > }; > > > > /* > > -- > > 2.17.1 > > -- 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