Re: [PATCH v13 00/14] fix several inconsistencies with sysfs configuration in etmX

[Yeoreum Yun]

Sashiko's review found one typo but others are not valid and
I've commented for them.

Therefore, I'll resend again with typo fix only keeping the version.

Thanks!
> The current ETMx configuration via sysfs can lead to the following
> inconsistencies:
> 
>   - If a configuration is modified via sysfs while a perf session is
>     active, the running configuration may differ between before
>     a sched-out and after a subsequent sched-in.
> 
>   - If a perf session and sysfs session tries to enable concurrently,
>     configuration from configfs could be corrupted (etm4).
> 
>   - There is chance to corrupt drvdata->config if perf session tries
>     to enabled among handling cscfg_csdev_disable_active_config()
>     in etm4_disable_sysfs() (etm4).
> 
> To resolve these inconsistencies, the configuration should be separated into:
> 
>   - curr_config, which is applied configuration for the current session
>   - config, which stores the settings configured via sysfs.
> 
> and apply configuration from configfs after taking a mode.
> 
> Patch History
> =============
> from v12 to v13:
>   - add r-b tags
>   - Link to v12: https://lore.kernel.org/r/20260924-separate_etm_cfg_v2-v12-0-a2b41d2f7c2b@xxxxxxx
> 
> from v11 to v12:
>   - rename active_config to curr_config.
>   - rename config to sysfs_config.
>   - remove redundant config argument in etmX_args.
>   - add read-back and copy from curr_config to sysfs_config TRCSEQSTR
>     (seq_state).
>   - add r-b and t-b tags.
>   - Link to v11: https://lore.kernel.org/r/20260915-separate_etm_cfg_v2-v11-0-d2b258d51747@xxxxxxx
> 
> from v10 to v11:
>   - replace direct register read from etm3 sysfs with IPI.
>   - fix issue cntr_val and others field which required to be shown after
>     sysfs session disable.
>   - prohibit the write for some etm3 sysfs while sysfs session is
>     enabled.
>   - drop locktype change in etm3
>   - Link to v10: https://lore.kernel.org/r/20260911-separate_etm_cfg_v2-v10-0-1b715d95927a@xxxxxxx
> 
> from v9 to v10:
>   - rebase to coresight/next
>   - https://lore.kernel.org/all/20260725113645.57519-1-yeoreum.yun@xxxxxxx/
> 
> from v8 to v9:
>   - add feat_csdev_lock guard interface.
>   - set feat_csdev->drv_spinlock as NULL for etmv4_drvdata.
>   - https://lore.kernel.org/all/20260629090007.1718746-1-yeoreum.yun@xxxxxxx/
> 
> from v7 to v8:
>   - accept @Leo Yan' suggestion to handle error.
>   - small minor fixes following @Suzuki' suggestion.
>   - https://lore.kernel.org/all/20260519154812.254884-1-yeoreum.yun@xxxxxxx/
> 
> from v6 to v7:
>   - rebase on coresight/next
>   - add ETM_MAX_SEQ_TRANSITIONS define
>   - remove redundant patch relavent cpu-hotplug as coresight-pm patch
>     merged.
>   - https://lore.kernel.org/all/20260422132203.977549-1-yeoreum.yun@xxxxxxx/
> 
> from v5 to v6:
>   - fix missing of calling cscfg_csdev_disable_active_config()
>   - add rb & fixes tags.
>   - add ss_status field in etm4x_drvdata to expose STATUS and PENDING bits.
>   - https://lore.kernel.org/all/20260415165528.3369607-1-yeoreum.yun@xxxxxxx/
> 
> from v4 to v5:
>   - add rb-tag.
>   - fix underflow issue for nrseqstate.
>   - fix wrong check in etm4_sspcicrn_present().
>   - remove redundant fields on etmv4_save_state.
>   - rename caps->ss_status to ss_cmp.
>   - fix wrong location of etm4_release_trace_id.
>   - https://lore.kernel.org/all/20260413142003.3549310-1-yeoreum.yun@xxxxxxx/
> 
> from v3 to v4:
>   - change etm_drvdata->spinlock type to raw_spin_lock_t
>   - remove redundant call etmX_enable_hw() with starting_cpu() callsback.
>   - fix missing trace id release.
>   - add missing docs.
>   - https://lore.kernel.org/all/20260412175506.412301-1-yeoreum.yun@xxxxxxx/
> 
> from v2 to v3:
>   - fix build error for etm3x.
>   - fix checkpatch warning.
>   - https://lore.kernel.org/all/20260410074310.2693385-1-yeoreum.yun@xxxxxxx/
> 
> from v1 to v2
>   - rebased to v7.0-rc7.
>   - introduce etmX_caps structure to save etmX's capabilities.
>   - remove ss_status from etmv4_config.
>   - modify active_config after taking a mode (perf/sysfs).
>   - https://lore.kernel.org/all/20260317181705.2456271-1-yeoreum.yun@xxxxxxx/
> 
> ---
> Yeoreum Yun (14):
>       coresight: etm4x: read-back TRCSEQSTR at disabling and prohibit modifying seq_state while enabling.
>       coresight: etm4x: prohibit modifying cntr_val while session is enabled
>       coresight: etm3x: prohibit modifying cntr_val and reset while session is enabled
>       coresight: etm4x: fix inconsistencies with sysfs configuration
>       coresight: etm3x: fix inconsistencies with sysfs configuration
>       coresight: etm3x: remove redundant cpu online check on etm_enable_sysfs()
>       coresight: etm4x: introduce struct etm4_caps
>       coresight: etm4x: exclude ss_status from drvdata->config
>       coresight: etm4x: remove s_ex_level from config
>       coresight: etm4x: rename local config as curr_config referring drvdata->curr_config
>       coresight: etm4x: rename drvdata->config to sysfs_config
>       coresight: etm3x: introduce struct etm_caps
>       coresight: etm3x: rename local config as curr_config referring drvdata->curr_config
>       coresight: etm3x: rename drvdata->config to sysfs_config
> 
>  drivers/hwtracing/coresight/coresight-config.c     |  18 +-
>  drivers/hwtracing/coresight/coresight-config.h     |  22 +
>  drivers/hwtracing/coresight/coresight-etm.h        |  48 +-
>  drivers/hwtracing/coresight/coresight-etm3x-core.c | 196 +++--
>  .../hwtracing/coresight/coresight-etm3x-sysfs.c    | 461 +++++-----
>  drivers/hwtracing/coresight/coresight-etm4x-cfg.c  |  14 +-
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 567 ++++++------
>  .../hwtracing/coresight/coresight-etm4x-sysfs.c    | 961 +++++++++++----------
>  drivers/hwtracing/coresight/coresight-etm4x.h      | 199 +++--
>  9 files changed, 1340 insertions(+), 1146 deletions(-)
> ---
> base-commit: a3281934612608db9e1d24aaf913c846e913de89
> change-id: 20260911-separate_etm_cfg_v2-3518a168cbff
> 
> Best regards,
> -- 
> Sincerely,
> Yeoreum Yun
> 

-- 
Sincerely,
Yeoreum Yun