| 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: <20241016101114.GB896339@hyd1403.caveonetworks.com>
Date: Wed, 16 Oct 2024 15:41:14 +0530
From: Linu Cherian <lcherian@...vell.com>
To: Suzuki K Poulose <suzuki.poulose@....com>
CC: <mike.leach@...aro.org>, <james.clark@....com>,
<linux-arm-kernel@...ts.infradead.org>, <coresight@...ts.linaro.org>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
<corbet@....net>, <devicetree@...r.kernel.org>, <sgoutham@...vell.com>,
<gcherian@...vell.com>
Subject: Re: [PATCH v10 7/8] coresight: config: Add preloaded configuration
On 2024-10-03 at 18:59:30, Suzuki K Poulose (suzuki.poulose@....com) wrote:
> On 16/09/2024 11:34, Linu Cherian wrote:
> > Add a preloaded configuration for generating
> > external trigger on address match. This can be
> > used by CTI and ETR blocks to stop trace capture
> > on kernel panic.
> >
> > Kernel address for "panic" function is used as the
> > default trigger address.
> >
> > This new configuration is available as,
> > /sys/kernel/config/cs-syscfg/configurations/panicstop
> >
> > Signed-off-by: Linu Cherian <lcherian@...vell.com>
> > Reviewed-by: James Clark <james.clark@....com>
> > ---
> > Changelog from v9:
> > No changes
> >
> > drivers/hwtracing/coresight/Makefile | 2 +-
> > .../coresight/coresight-cfg-preload.c | 2 +
> > .../coresight/coresight-cfg-preload.h | 2 +
> > .../hwtracing/coresight/coresight-cfg-pstop.c | 83 +++++++++++++++++++
> > 4 files changed, 88 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/hwtracing/coresight/coresight-cfg-pstop.c
> >
> > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> > index 4ba478211b31..46ce7f39d05f 100644
> > --- a/drivers/hwtracing/coresight/Makefile
> > +++ b/drivers/hwtracing/coresight/Makefile
> > @@ -25,7 +25,7 @@ subdir-ccflags-y += $(condflags)
> > obj-$(CONFIG_CORESIGHT) += coresight.o
> > coresight-y := coresight-core.o coresight-etm-perf.o coresight-platform.o \
> > coresight-sysfs.o coresight-syscfg.o coresight-config.o \
> > - coresight-cfg-preload.o coresight-cfg-afdo.o \
> > + coresight-cfg-preload.o coresight-cfg-afdo.o coresight-cfg-pstop.o \
>
> Please could you only build it when ETM4X is selected ? That way you
> could drop the "CONFIG" check in the code ?
coresight-cfg-pstop feature follows the approach taken in existing
features like preload, afdo etc. Hence, shall we keep the same format ?
Please let me know if you think otherwise.
> > coresight-syscfg-configfs.o coresight-trace-id.o
> > obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o
> > coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \
> > diff --git a/drivers/hwtracing/coresight/coresight-cfg-preload.c b/drivers/hwtracing/coresight/coresight-cfg-preload.c
> > index e237a4edfa09..4980e68483c5 100644
> > --- a/drivers/hwtracing/coresight/coresight-cfg-preload.c
> > +++ b/drivers/hwtracing/coresight/coresight-cfg-preload.c
> > @@ -13,6 +13,7 @@
> > static struct cscfg_feature_desc *preload_feats[] = {
> > #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> > &strobe_etm4x,
> > + &gen_etrig_etm4x,
> > #endif
> > NULL
> > };
> > @@ -20,6 +21,7 @@ static struct cscfg_feature_desc *preload_feats[] = {
> > static struct cscfg_config_desc *preload_cfgs[] = {
> > #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> > &afdo_etm4x,
> > + &pstop_etm4x,
> > #endif
> > NULL
> > };
> > diff --git a/drivers/hwtracing/coresight/coresight-cfg-preload.h b/drivers/hwtracing/coresight/coresight-cfg-preload.h
> > index 21299e175477..291ba530a6a5 100644
> > --- a/drivers/hwtracing/coresight/coresight-cfg-preload.h
> > +++ b/drivers/hwtracing/coresight/coresight-cfg-preload.h
> > @@ -10,4 +10,6 @@
> > #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> > extern struct cscfg_feature_desc strobe_etm4x;
> > extern struct cscfg_config_desc afdo_etm4x;
> > +extern struct cscfg_feature_desc gen_etrig_etm4x;
> > +extern struct cscfg_config_desc pstop_etm4x;
> > #endif
> > diff --git a/drivers/hwtracing/coresight/coresight-cfg-pstop.c b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
> > new file mode 100644
> > index 000000000000..c2bfbd07bfaf
> > --- /dev/null
> > +++ b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
> > @@ -0,0 +1,83 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright(C) 2023 Marvell.
> > + * Based on coresight-cfg-afdo.c
> > + */
> > +
> > +#include "coresight-config.h"
> > +
> > +/* ETMv4 includes and features */
> > +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
>
> Could we not drop this check, if we only build it when
> CONFIG_CORESIGHT_SOURCE_ETM4X is selected. It is not useful
> otherwise anyways.
IIUC, this arrangement might be helpful to add register
configurations for future ETM versions in the same file.
>
> Rest looks fine to me
>
> Suzuki
>
>
> > +#include "coresight-etm4x-cfg.h"
> > +
> > +/* preload configurations and features */
> > +
> > +/* preload in features for ETMv4 */
> > +
> > +/* panic_stop feature */
> > +static struct cscfg_parameter_desc gen_etrig_params[] = {
> > + {
> > + .name = "address",
> > + .value = (u64)panic,
> > + },
> > +};
> > +
> > +static struct cscfg_regval_desc gen_etrig_regs[] = {
> > + /* resource selector */
> > + {
> > + .type = CS_CFG_REG_TYPE_RESOURCE,
> > + .offset = TRCRSCTLRn(2),
> > + .hw_info = ETM4_CFG_RES_SEL,
> > + .val32 = 0x40001,
> > + },
> > + /* single address comparator */
> > + {
> > + .type = CS_CFG_REG_TYPE_RESOURCE | CS_CFG_REG_TYPE_VAL_64BIT |
> > + CS_CFG_REG_TYPE_VAL_PARAM,
> > + .offset = TRCACVRn(0),
> > + .val32 = 0x0,
> > + },
> > + {
> > + .type = CS_CFG_REG_TYPE_RESOURCE,
> > + .offset = TRCACATRn(0),
> > + .val64 = 0xf00,
> > + },
> > + /* Driver external output[0] with comparator out */
> > + {
> > + .type = CS_CFG_REG_TYPE_RESOURCE,
> > + .offset = TRCEVENTCTL0R,
> > + .val32 = 0x2,
> > + },
> > + /* end of regs */
> > +};
> > +
> > +struct cscfg_feature_desc gen_etrig_etm4x = {
> > + .name = "gen_etrig",
> > + .description = "Generate external trigger on address match\n"
> > + "parameter \'address\': address of kernel address\n",
> > + .match_flags = CS_CFG_MATCH_CLASS_SRC_ETM4,
> > + .nr_params = ARRAY_SIZE(gen_etrig_params),
> > + .params_desc = gen_etrig_params,
> > + .nr_regs = ARRAY_SIZE(gen_etrig_regs),
> > + .regs_desc = gen_etrig_regs,
> > +};
> > +
> > +/* create a panic stop configuration */
> > +
> > +/* the total number of parameters in used features */
> > +#define PSTOP_NR_PARAMS ARRAY_SIZE(gen_etrig_params)
> > +
> > +static const char *pstop_ref_names[] = {
> > + "gen_etrig",
> > +};
> > +
> > +struct cscfg_config_desc pstop_etm4x = {
> > + .name = "panicstop",
> > + .description = "Stop ETM on kernel panic\n",
> > + .nr_feat_refs = ARRAY_SIZE(pstop_ref_names),
> > + .feat_ref_names = pstop_ref_names,
> > + .nr_total_params = PSTOP_NR_PARAMS,
> > +};
> > +
> > +/* end of ETM4x configurations */
> > +#endif /* IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X) */
>
>
Powered by blists - more mailing lists