RE: [EXT] Re: [PATCH v6 7/7] coresight: config: Add preloaded configuration

[Linu Cherian]

Hi Suzuki,

> -----Original Message-----
> From: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> Sent: Friday, January 12, 2024 7:34 PM
> To: Linu Cherian <lcherian@xxxxxxxxxxx>; mike.leach@xxxxxxxxxx;
> james.clark@xxxxxxx; leo.yan@xxxxxxxxxx
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; coresight@xxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; Sunil Kovvuri Goutham
> <sgoutham@xxxxxxxxxxx>; George Cherian <gcherian@xxxxxxxxxxx>
> Subject: [EXT] Re: [PATCH v6 7/7] coresight: config: Add preloaded
> configuration
> 
> External Email
> 
> ----------------------------------------------------------------------
> On 05/01/2024 05:58, 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 to be programmed as below.
> >
> > $cd /config/cs-syscfg/features/gen_etrig/params
> > $echo <panic_address> > address/value
> >
> > Signed-off-by: Linu Cherian <lcherian@xxxxxxxxxxx>
> > ---
> > Changelog from v5:
> > * 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 995d3b2c76df..68b15c8d9462 100644
> > --- a/drivers/hwtracing/coresight/Makefile
> > +++ b/drivers/hwtracing/coresight/Makefile
> > @@ -5,7 +5,7 @@
> >   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
> > +\
> >   		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..037d6773fab8
> > --- /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)
> > +#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 = 0x0,
> > +	},
> > +};
> > +
> > +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",
> 
> Since this is actually generic, i.e., Stop trace on a Kernel address, we could
> rename this ?  Or why don't we pre-populate the address of "panic"
> at load time. That way the user doesn't have to figure out the kernel address
> (e.g., if KASLR is enabled)
> 

Yeah, will hardcode that parameter value to panic symbol. That will avoid the hassle of user figuring out the kernel address for panic.

diff --git a/drivers/hwtracing/coresight/coresight-cfg-pstop.c b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
index 037d6773fab8..c2bfbd07bfaf 100644
--- a/drivers/hwtracing/coresight/coresight-cfg-pstop.c
+++ b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
@@ -18,7 +18,7 @@
 static struct cscfg_parameter_desc gen_etrig_params[] = {
        {
                .name = "address",
-               .value = 0x0,
+               .value = (u64)panic,
        },
 };


Linu Cherian.