Re: [PATCH v5 07/12] coresight: etm4x: fix inconsistencies with sysfs configuration

[Yeoreum Yun]

Hi Leo,

> On Tue, Apr 21, 2026 at 12:14:20PM +0100, Yeoreum Yun wrote:
>
> [...]
>
> > > >   - There is a risk of corrupting drvdata->config if a perf session enables
> > > >     tracing while cscfg_csdev_disable_active_config() is being handled in
> > > >     etm4_disable_sysfs().
> > >
> > > Similiar to above, cscfg_csdev_disable_active_config() is not
> > > protected in etm4_disable_sysfs().
> >
> > This is not true.
> > at the time of etm4_disable_sysfs() "mode" is already taken
> > (whether sysfs or perf). In this situation, it's enough to
> > call cscfg_csdev_disable_active_config() before chaning
> > mode to DISABLED.
>
> To be clear, I am trying to understand issue _before_ your patch.
> Without this patch, cscfg_csdev_disable_active_config() is not
> protected by the mode.

Right.

[...]

> > > > @@ -1386,6 +1401,7 @@ static void etm4_init_arch_data(void *info)
> > > >  	drvdata = dev_get_drvdata(init_arg->dev);
> > > >  	caps = &drvdata->caps;
> > > >  	csa = init_arg->csa;
> > > > +	config = &drvdata->active_config;
> > >
> > > Should we init drvdata->config instead so make it has sane values ?
> > >
> > > In other words, drvdata->active_config are always set at the runtime,
> > > so don't need to init it at all, right?
> >
> > No. at least when the initialise, I think we should fill the its
> > contesnt with the "etm4_set_default()".
> >
> > That's why the consequence call etm4_set_default() called with
> > active_config and config is coped with the default configutation.
>
> I'm concerned that some config fields may be reused across sessions.
> For example, a sysfs session copies drvdata->config into
> drvdata->active_config, and later a perf session may reuse stale
> values. The same issue can happen in the reverse direction.
>
> A clean approach would be to treat drvdata->active_config as
> per-session state:
>
>   1) clear drvdata->active_config at session start
>   2) apply the session-specific config
>      2.1) sysfs: use drvdata->config
>      2.2) perf: use event config

>   3) then apply configfs configuration
This is not true.

In case of "perf", it always clear the "active_config" with 0x00
before enable. so it wouldn't.

In case of sysfs we don't need to worries about since
"config" field saves everything and this is applied into active_config.

So, your issue doesn't exist in this case.



--
Sincerely,
Yeoreum Yun