Message-ID: <a80b9a03-01ba-3752-9e6e-ee2d194ba2a1@arm.com>
Date:   Tue, 13 Jul 2021 10:42:53 +0100
From:   Branislav Rankov <branislav.rankov@....com>
To:     Mathieu Poirier <mathieu.poirier@...aro.org>,
        Mike Leach <mike.leach@...aro.org>
Cc:     alexander.shishkin@...ux.intel.com, gregkh@...uxfoundation.org,
        coresight@...ts.linaro.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v8 00/10] CoreSight configuration management; ETM strobing

Hi Mathieu,

On 7/12/21 5:44 PM, Mathieu Poirier wrote:
> Good morning Mike,
> 
> On Wed, Jul 07, 2021 at 02:29:53PM +0100, Mike Leach wrote:
>> This patchset introduces initial concepts in CoreSight system
>> configuration management support. to allow more detailed and complex
>> programming to be applied to CoreSight systems during trace capture.
>>
>> Configurations consist of 2 elements:-
>> 1) Features - programming combinations for devices, applied to a class of
>> device on the system (all ETMv4), or individual devices.
>> 2) Configurations - a set of programmed features used when the named
>> configuration is selected.
>>
>> Features and configurations are declared as a data table, a set of register,
>> resource and parameter requirements. Features and configurations are loaded
>> into the system by the virtual cs_syscfg device. This then matches features
>> to any registered devices and loads the feature into them.
>>
>> Individual device classes that support feature and configuration register
>> with cs_syscfg.
>>
>> Once loaded a configuration can be enabled for a specific trace run.
>> Configurations are registered with the perf cs_etm event as entries in
>> cs_etm/events. These can be selected on the perf command line as follows:-
>>
>> perf record -e cs_etm/<config_name>/ ...
>>
>> This patch set has one pre-loaded configuration and feature.
>> A named "strobing" feature is provided for ETMv4.
>> A named "autofdo" configuration is provided. This configuration enables
>> strobing on any ETM in used.
>>
>> Thus the command:
>> perf record -e cs_etm/autofdo/ ...
>>
>> will trace the supplied application while enabling the "autofdo" configuation
>> on each ETM as it is enabled by perf. This in turn will enable strobing for
>> the ETM - with default parameters. Parameters can be adjusted using configfs.
>>
>> The sink used in the trace run will be automatically selected.
>>
>> A configuration can supply up to 15 of preset parameter values, which will
>> subsitute in parameter values for any feature used in the configuration.
>>
>> Selection of preset values as follows
>> perf record -e cs_etm/autofdo,preset=1/ ...
>>
>> (valid presets 1-N, where N is the number supplied in the configuration, not
>> exceeding 15. preset=0 is the same as not selecting a preset.)
>>
>> Applies to & tested against coresight/next (5.13-rc6 base)
>>
>> Changes since v7:
>>
>> Fixed kernel test robot issue - config with CORESIGHT=y & CONFIGFS_FS=m causes
>> build error. Altered CORESIGHT config to select CONFIGFS_FS.
>> Reported-by: kernel test robot <lkp@...el.com>
>>
>> Replaced mutex use to protect loaded config lists in coresight devices with per
>> device spinlock to remove issue when disable called in interrupt context.
>> Reported-by: Branislav Rankov <branislav.rankov@....com>
>>
> 
> Can you indicate which patches have changed so I don't have to review the whole
> thing again?   It is also common practice to remove the RB tag when patches
> have changed enough to mandate another review.  In this case all patches still
> bare my RB tags.
> 
> Branislav reported the problem but he is not a recipient.  I would like to have
> a confirmation from him that this set fixes the problem he observed before I
> start looking at it.

I have tested this series and the issue I reported is fixed.
> 
> Thanks,
> Mathieu
> 
>>
>> Changes since v6:
>> Fixed kernel test robot issues-
>> Reported-by: kernel test robot <lkp@...el.com>
>>
>> Changes since v5:
>>
>> 1) Fix code style issues from auto-build reports, as
>> Reported-by: kernel test robot <lkp@...el.com>
>> 2) Update comments to get consistent docs for API functions.
>> 3) remove unused #define from autofdo example.
>> 4) fix perf code style issues from patch 4 (Mathieu)
>> 5) fix configfs code style issues from patch 9. (Mathieu)
>>
>> Changes since v4: (based on comments from Matthieu and Suzuki).
>> No large functional changes - primarily code improvements and naming schema.
>> 1) Updated entire set to ensure a consistent naming scheme was used for
>> variables and struct members that refer to the key objects in the system.
>> Suffixes _desc used for all references to feature and configuraion descriptors,
>> suffix _csdev used for all references to load feature and configs in the csdev
>> instances. (Mathieu & Suzuki).
>> 2) Dropped the 'configurations' sub dir in cs_etm perf directories as superfluous
>> with the configfs containing the same information. (Mathieu).
>> 3) Simplified perf handling code (suzuki)
>> 4) Multiple simplifications and improvements for code readability (Matthieu
>> and Suzuki)
>>
>> Changes since v3: (Primarily based on comments from Matthieu)
>> 1) Locking mechanisms simplified.
>> 2) Removed the possibility to enable features independently from
>> configurations.Only configurations can be enabled now. Simplifies programming
>> logic.
>> 3) Configuration now uses an activate->enable mechanism. This means that perf
>> will activate a selected configuration at the start of a session (during
>> setup_aux), and disable at the end of a session (around free_aux)
>> The active configuration and associated features will be programmed into the
>> CoreSight device instances when they are enabled. This locks the configuration
>> into the system while in use. Parameters cannot be altered while this is
>> in place. This mechanism will be extended in future for dynamic load / unload
>> of configurations to prevent removal while in use.
>> 4) Removed the custom bus / driver as un-necessary. A single device is
>> registered to own perf fs elements and configfs.
>> 5) Various other minor issues addressed.
>>
>> Changes since v2:
>> 1) Added documentation file.
>> 2) Altered cs_syscfg driver to no longer be coresight_device based, and moved
>> to its own custom bus to remove it from the main coresight bus. (Mathieu)
>> 3) Added configfs support to inspect and control loaded configurations and
>> features. Allows listing of preset values (Yabin Cui)
>> 4) Dropped sysfs support for adjusting feature parameters on the per device
>> basis, in favour of a single point adjustment in configfs that is pushed to all
>> device instances.
>> 5) Altered how the config and preset command line options are handled in perf
>> and the drivers. (Mathieu and Suzuki).
>> 6) Fixes for various issues and technical points (Mathieu, Yabin)
>>
>> Changes since v1:
>> 1) Moved preloaded configurations and features out of individual drivers.
>> 2) Added cs_syscfg driver to manage configurations and features. Individual
>> drivers register with cs_syscfg indicating support for config, and provide
>> matching information that the system uses to load features into the drivers.
>> This allows individual drivers to be updated on an as needed basis - and
>> removes the need to consider devices that cannot benefit from configuration -
>> static replicators, funnels, tpiu.
>> 3) Added perf selection of configuarations.
>> 4) Rebased onto the coresight module loading set. 
>>
>> To follow in future revisions / sets:-
>> a) load of additional config and features by loadable module.
>> b) load of additional config and features by configfs
>> c) enhanced resource management for ETMv4 and checking features have sufficient
>> resources to be enabled.
>> d) ECT and CTI support for configuration and features.
>>
>> Mike Leach (10):
>>   coresight: syscfg: Initial coresight system configuration
>>   coresight: syscfg: Add registration and feature loading for cs devices
>>   coresight: config: Add configuration and feature generic functions
>>   coresight: etm-perf: update to handle configuration selection
>>   coresight: syscfg: Add API to activate and enable configurations
>>   coresight: etm-perf: Update to activate selected configuration
>>   coresight: etm4x: Add complex configuration handlers to etmv4
>>   coresight: config: Add preloaded configurations
>>   coresight: syscfg: Add initial configfs support
>>   Documentation: coresight: Add documentation for CoreSight config
>>
>>  .../trace/coresight/coresight-config.rst      | 244 ++++++
>>  Documentation/trace/coresight/coresight.rst   |  16 +
>>  drivers/hwtracing/coresight/Kconfig           |   1 +
>>  drivers/hwtracing/coresight/Makefile          |   7 +-
>>  .../hwtracing/coresight/coresight-cfg-afdo.c  | 153 ++++
>>  .../coresight/coresight-cfg-preload.c         |  31 +
>>  .../coresight/coresight-cfg-preload.h         |  13 +
>>  .../hwtracing/coresight/coresight-config.c    | 275 ++++++
>>  .../hwtracing/coresight/coresight-config.h    | 253 ++++++
>>  drivers/hwtracing/coresight/coresight-core.c  |  12 +-
>>  .../hwtracing/coresight/coresight-etm-perf.c  | 150 +++-
>>  .../hwtracing/coresight/coresight-etm-perf.h  |  12 +-
>>  .../hwtracing/coresight/coresight-etm4x-cfg.c | 182 ++++
>>  .../hwtracing/coresight/coresight-etm4x-cfg.h |  30 +
>>  .../coresight/coresight-etm4x-core.c          |  38 +-
>>  .../coresight/coresight-etm4x-sysfs.c         |   3 +
>>  .../coresight/coresight-syscfg-configfs.c     | 396 +++++++++
>>  .../coresight/coresight-syscfg-configfs.h     |  45 +
>>  .../hwtracing/coresight/coresight-syscfg.c    | 829 ++++++++++++++++++
>>  .../hwtracing/coresight/coresight-syscfg.h    |  81 ++
>>  include/linux/coresight.h                     |  11 +
>>  21 files changed, 2746 insertions(+), 36 deletions(-)
>>  create mode 100644 Documentation/trace/coresight/coresight-config.rst
>>  create mode 100644 drivers/hwtracing/coresight/coresight-cfg-afdo.c
>>  create mode 100644 drivers/hwtracing/coresight/coresight-cfg-preload.c
>>  create mode 100644 drivers/hwtracing/coresight/coresight-cfg-preload.h
>>  create mode 100644 drivers/hwtracing/coresight/coresight-config.c
>>  create mode 100644 drivers/hwtracing/coresight/coresight-config.h
>>  create mode 100644 drivers/hwtracing/coresight/coresight-etm4x-cfg.c
>>  create mode 100644 drivers/hwtracing/coresight/coresight-etm4x-cfg.h
>>  create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.c
>>  create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.h
>>  create mode 100644 drivers/hwtracing/coresight/coresight-syscfg.c
>>  create mode 100644 drivers/hwtracing/coresight/coresight-syscfg.h
>>
>> -- 
>> 2.17.1
>>
> _______________________________________________
> CoreSight mailing list
> CoreSight@...ts.linaro.org
> https://lists.linaro.org/mailman/listinfo/coresight
> 

Powered by blists - more mailing lists