Good day Mike,
On Thu, Dec 24, 2020 at 07:20:57PM +0000, Mike Leach wrote:
Hi MAthieu,
Thanks for the review as ever. Sorry its taken me a bit of time to respond, but there has been a bit of reworking to do.
Ok
I certainly agree with the _desc / _csdev convention, and the follow up set should address that.
Ok
The locking is also reconfigured as mentioned - so that it is simpler.
Ok
The general rules have been tightened. I was trying to allow features to be enabled independently of configurations when driving the system from configfs / sysfs rather than perf. But in the end there is little value in that approach.
Ok
As mentioned I have been working on follow up patches - which should be able to be split into smaller changesets. One is the dynamic loading of configurations, which will be either by loadable module, or configfs. The latter will allow the user to mkdir in the configs directory, and then load a binary blob via a "load" file. I do not believe that it is practicable to define an entire set of features with registers and configs via configfs - given that it is relatively inflexible in terms of adding files dynamically from a kernel perspective.
My view of the changes you are making is likely incomplete and as such it is probably a good idea to wait for another revision before providing further comments.
Let me know if there are things you need clarifications on that I have missed in your replies.
Thanks, Mathieu
That said, it may be possible later to add in a way to defne a configuration via configfs using previously defined named features only.
Regards
Mike
On Thu, 26 Nov 2020 at 18:52, Mathieu Poirier mathieu.poirier@linaro.org wrote:
Hi Mike,
On Fri, Oct 30, 2020 at 05:56:46PM +0000, Mike Leach wrote:
This patchset introduces initial concepts in CoreSight complex system configuration support.
Configurations consist of 2 elements:-
- 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/cs_config. 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 configuation 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 coresight/next (5.10-rc1 base)
I am done reviewing this third revision. I commented on a fair amount of things but that isn't unusual with this much code, especially when it is new code that doesn't have a precedent.
As I stated before I think the general architecture of the feature is sound. The concepts of XYZ_desc and XYZ_csdev have now sunk in and I quite like it. It will be really good when the naming convention is straighten out.
I spent a long time thinking about creating configuration and features on the fly from configfs. It is definitely not something that needs to be part of this set, nor have to be implemented any time soon, but is this something that is on your radar?
Lastly the "RFC" should be dropped for the fourth iteration, we are way passed that point now.
Thanks, Mathieu
Changes since v2:
- Added documentation file.
- 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:
- Moved preloaded configurations and features out of individual drivers.
- 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 (9): 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: etm4x: Add complex configuration handlers to etmv4 coresight: config: Add preloaded configurations coresight: syscfg: Add initial configfs support. coresight: syscfg: Allow update of feature params from configfs coresight: docs: Add documentation for CoreSight config.
.../trace/coresight/coresight-config.rst | 230 ++++++ Documentation/trace/coresight/coresight.rst | 16 + drivers/hwtracing/coresight/Makefile | 6 +- .../coresight/coresight-cfg-preload.c | 160 +++++ .../hwtracing/coresight/coresight-config.c | 392 +++++++++++ .../hwtracing/coresight/coresight-config.h | 311 +++++++++ drivers/hwtracing/coresight/coresight-core.c | 18 +- .../hwtracing/coresight/coresight-etm-perf.c | 166 ++++- .../hwtracing/coresight/coresight-etm-perf.h | 10 +- .../hwtracing/coresight/coresight-etm4x-cfg.c | 181 +++++ .../hwtracing/coresight/coresight-etm4x-cfg.h | 29 + .../coresight/coresight-etm4x-core.c | 36 +- .../coresight/coresight-etm4x-sysfs.c | 3 + .../coresight/coresight-syscfg-configfs.c | 421 +++++++++++ .../coresight/coresight-syscfg-configfs.h | 47 ++ .../hwtracing/coresight/coresight-syscfg.c | 656 ++++++++++++++++++ .../hwtracing/coresight/coresight-syscfg.h | 83 +++ include/linux/coresight.h | 7 + 18 files changed, 2743 insertions(+), 29 deletions(-) create mode 100644 Documentation/trace/coresight/coresight-config.rst create mode 100644 drivers/hwtracing/coresight/coresight-cfg-preload.c 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
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK