| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <065b4ad2-db39-42ed-91d1-d818f82ece79@arm.com>
Date: Tue, 17 Dec 2024 14:52:19 +0000
From: Suzuki K Poulose <suzuki.poulose@....com>
To: Mao Jinlong <quic_jinlmao@...cinc.com>, Mike Leach
<mike.leach@...aro.org>, James Clark <james.clark@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>
Cc: coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
Tao Zhang <quic_taozha@...cinc.com>
Subject: Re: [PATCH v2 1/3] coresight-tpdm: Add MCMB dataset support
On 05/11/2024 12:39, Mao Jinlong wrote:
> MCMB (Multi-lane CMB) is a special form of CMB dataset type. MCMB
> subunit TPDM has the same number and usage of registers as CMB
> subunit TPDM. MCMB subunit can be enabled for data collection by
> writing 1 to the first bit of CMB_CR register. The difference is
> that MCMB subunit TPDM needs to select the lane and enable it in
> using it.
>
> Signed-off-by: Tao Zhang <quic_taozha@...cinc.com>
> Signed-off-by: Mao Jinlong <quic_jinlmao@...cinc.com>
> ---
> drivers/hwtracing/coresight/coresight-tpda.c | 7 ++--
> drivers/hwtracing/coresight/coresight-tpdm.c | 44 +++++++++++++++++---
> drivers/hwtracing/coresight/coresight-tpdm.h | 27 ++++++------
> 3 files changed, 57 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
> index bfca103f9f84..4b61b9840740 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.c
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> #include <linux/amba/bus.h>
> @@ -68,11 +68,12 @@ static int tpdm_read_element_size(struct tpda_drvdata *drvdata,
> int rc = -EINVAL;
> struct tpdm_drvdata *tpdm_data = dev_get_drvdata(csdev->dev.parent);
>
> - if (tpdm_has_dsb_dataset(tpdm_data)) {
> + if (tpdm_data->dsb) {
> rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent),
> "qcom,dsb-element-bits", &drvdata->dsb_esize);
> }
> - if (tpdm_has_cmb_dataset(tpdm_data)) {
> +
> + if (tpdm_data->cmb) {
> rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent),
> "qcom,cmb-element-bits", &drvdata->cmb_esize);
> }
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index b7d99e91ab84..0529858586c1 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> #include <linux/amba/bus.h>
> @@ -21,6 +21,21 @@
>
> DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
>
> +static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
> +{
> + return (drvdata->datasets & TPDM_PIDR0_DS_DSB);
> +}
> +
> +static bool tpdm_has_cmb_dataset(struct tpdm_drvdata *drvdata)
> +{
> + return (drvdata->datasets & TPDM_PIDR0_DS_CMB);
> +}
> +
> +static bool tpdm_has_mcmb_dataset(struct tpdm_drvdata *drvdata)
> +{
> + return (drvdata->datasets & TPDM_PIDR0_DS_MCMB);
> +}
> +
> /* Read dataset array member with the index number */
> static ssize_t tpdm_simple_dataset_show(struct device *dev,
> struct device_attribute *attr,
> @@ -198,7 +213,7 @@ static umode_t tpdm_cmb_is_visible(struct kobject *kobj,
> struct device *dev = kobj_to_dev(kobj);
> struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>
> - if (drvdata && tpdm_has_cmb_dataset(drvdata))
> + if (drvdata && drvdata->cmb)
> return attr->mode;
>
> return 0;
> @@ -246,8 +261,10 @@ static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
> drvdata->dsb->trig_type = false;
> }
>
> - if (drvdata->cmb)
> + if (drvdata->cmb) {
> memset(drvdata->cmb, 0, sizeof(struct cmb_dataset));
> + drvdata->cmb->trig_ts = true;
This looks like an unrelated change from the MCMB dataset. Is there an
explanation ? Is this a mistake ? If it is intended, please make it a
separate patch.
> + }
> }
>
> static void set_dsb_mode(struct tpdm_drvdata *drvdata, u32 *val)
> @@ -388,7 +405,7 @@ static void tpdm_enable_cmb(struct tpdm_drvdata *drvdata)
> {
> u32 val, i;
>
> - if (!tpdm_has_cmb_dataset(drvdata))
> + if (!drvdata->cmb)
> return;
>
> /* Configure pattern registers */
> @@ -415,6 +432,19 @@ static void tpdm_enable_cmb(struct tpdm_drvdata *drvdata)
> val |= TPDM_CMB_CR_MODE;
> else
> val &= ~TPDM_CMB_CR_MODE;
> +
> + if (tpdm_has_mcmb_dataset(drvdata)) {
> + val &= ~TPDM_CMB_CR_XTRIG_LNSEL;
> + /* Set the lane participates in tghe output pattern */
minor nit: typo "tghe" => "the"
> + val |= FIELD_PREP(TPDM_CMB_CR_XTRIG_LNSEL,
> + drvdata->cmb->mcmb.trig_lane);
> +
> + /* Set the enablement of the lane */
> + val &= ~TPDM_CMB_CR_E_LN;
> + val |= FIELD_PREP(TPDM_CMB_CR_E_LN,
> + drvdata->cmb->mcmb.lane_select);
> + }
> +
> /* Set the enable bit of CMB control register to 1 */
> val |= TPDM_CMB_CR_ENA;
> writel_relaxed(val, drvdata->base + TPDM_CMB_CR);
> @@ -480,7 +510,7 @@ static void tpdm_disable_cmb(struct tpdm_drvdata *drvdata)
> {
> u32 val;
>
> - if (!tpdm_has_cmb_dataset(drvdata))
> + if (!drvdata->cmb)
> return;
>
> val = readl_relaxed(drvdata->base + TPDM_CMB_CR);
> @@ -542,12 +572,14 @@ static int tpdm_datasets_setup(struct tpdm_drvdata *drvdata)
> if (!drvdata->dsb)
> return -ENOMEM;
> }
> - if (tpdm_has_cmb_dataset(drvdata) && (!drvdata->cmb)) {
> + if ((tpdm_has_cmb_dataset(drvdata) || tpdm_has_mcmb_dataset(drvdata))
> + && (!drvdata->cmb)) {
> drvdata->cmb = devm_kzalloc(drvdata->dev,
> sizeof(*drvdata->cmb), GFP_KERNEL);
> if (!drvdata->cmb)
> return -ENOMEM;
> }
> +
> tpdm_reset_datasets(drvdata);
>
> return 0;
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
> index e08d212642e3..fd9153b92335 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> /*
> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> #ifndef _CORESIGHT_CORESIGHT_TPDM_H
> @@ -9,7 +9,7 @@
> /* The max number of the datasets that TPDM supports */
> #define TPDM_DATASETS 7
>
> -/* CMB Subunit Registers */
> +/* CMB/MCMB Subunit Registers */
> #define TPDM_CMB_CR (0xA00)
> /* CMB subunit timestamp insertion enable register */
> #define TPDM_CMB_TIER (0xA04)
> @@ -34,6 +34,10 @@
> #define TPDM_CMB_TIER_XTRIG_TSENAB BIT(1)
> /* For timestamp fo all trace */
> #define TPDM_CMB_TIER_TS_ALL BIT(2)
> +/* MCMB trigger lane select */
> +#define TPDM_CMB_CR_XTRIG_LNSEL GENMASK(20, 18)
> +/* MCMB lane enablement */
> +#define TPDM_CMB_CR_E_LN GENMASK(17, 10)
Please could you move the CMB_CR related defintions together ?
i.e., closer to TPDM_CMB_CR_MODE
Suzuki
>
> /* Patten register number */
> #define TPDM_CMB_MAX_PATT 2
> @@ -112,11 +116,13 @@
> * PERIPHIDR0[0] : Fix to 1 if ImplDef subunit present, else 0
> * PERIPHIDR0[1] : Fix to 1 if DSB subunit present, else 0
> * PERIPHIDR0[2] : Fix to 1 if CMB subunit present, else 0
> + * PERIPHIDR0[6] : Fix to 1 if MCMB subunit present, else 0
> */
>
> #define TPDM_PIDR0_DS_IMPDEF BIT(0)
> #define TPDM_PIDR0_DS_DSB BIT(1)
> #define TPDM_PIDR0_DS_CMB BIT(2)
> +#define TPDM_PIDR0_DS_MCMB BIT(6)
>
> #define TPDM_DSB_MAX_LINES 256
> /* MAX number of EDCR registers */
> @@ -256,6 +262,9 @@ struct dsb_dataset {
> * @patt_ts: Indicates if pattern match for timestamp is enabled.
> * @trig_ts: Indicates if CTI trigger for timestamp is enabled.
> * @ts_all: Indicates if timestamp is enabled for all packets.
> + * struct mcmb_dataset
> + * @mcmb_trig_lane: Save data for trigger lane
> + * @mcmb_lane_select: Save data for lane enablement
> */
> struct cmb_dataset {
> u32 trace_mode;
> @@ -267,6 +276,10 @@ struct cmb_dataset {
> bool patt_ts;
> bool trig_ts;
> bool ts_all;
> + struct {
> + u8 trig_lane;
> + u8 lane_select;
> + } mcmb;
> };
>
> /**
> @@ -324,14 +337,4 @@ struct tpdm_dataset_attribute {
> enum dataset_mem mem;
> u32 idx;
> };
> -
> -static bool tpdm_has_dsb_dataset(struct tpdm_drvdata *drvdata)
> -{
> - return (drvdata->datasets & TPDM_PIDR0_DS_DSB);
> -}
> -
> -static bool tpdm_has_cmb_dataset(struct tpdm_drvdata *drvdata)
> -{
> - return (drvdata->datasets & TPDM_PIDR0_DS_CMB);
> -}
> #endif /* _CORESIGHT_CORESIGHT_TPDM_H */
Powered by blists - more mailing lists