CoreSight

coresight@lists.linaro.org

1 participants
2095 discussions

Re: [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files

by Mike Leach

Hi Christoph On Tue, 27 Dec 2022 at 17:09, Christoph Hellwig <hch(a)infradead.org> wrote: > > On Mon, Dec 19, 2022 at 11:46:35PM +0000, Mike Leach wrote: > > Add in functionality and binary attribute to load and unload > > configurations as binary data. > > > > Files are loaded via the 'load' binary attribute. System reads the incoming > > file, which must be formatted correctly as defined in the file reader code. > > This will create configuration(s) and/or feature(s) and load them > > into the system. > > Binary attributes are intended to pass things such as firmware > through. Defining your own structured file format seems like a > major abuse of the configfs design. What's the advantage of this > over simply using an ioctl? The coresight configurations loaded here, represent programming of the entire coresight subsystem - possibly tens of registers (especially on the ETM), across multiple devices, in ways that are not possible using the limited parameters of the perf command line. The ETM can be programmed in ways that use counters. sequencers, and optionally interact with other components such as CTI / CTM to send conditional hardware triggers, all of which control the when and how the trace is collected. Additionally there are system trace components that might need to run at the same time - such as ELA, and other system monitors that output data on the trace bus currently being introduce by some chip designers. As such the configuration must be loaded into the coresight system as a single operation - with the individual drivers validating the requested programming, where any error will fail the configuration load. The individual drivers are also responsible for defining which device registers user configurations can use - these being limited to those that affect the scope of trace collected, with other operational functions being reserved to the drivers themselves. To achieve this a variable sized table of programming descriptors is defined, that are transferred into the individual devices. The very limited set of built in configurations - where the programming descriptors are compiled into the driver modules themselves use the same set of descriptors. however, recompiling kernel modules to program new configurations is neither scaleable, flexible or desirable - so we need a way of loading configurations at runtime. So the file structure is simple a serialisation of these descriptor tables - with a header defining the input type and overall size.. The advantage of these runtime configurations is that they can be portable - and dependent on the hardware in the system, not the kernel build version. Moreover, there are trace scenarios when we want to trace what is present, not recompile a module / kernel to achieve this, especially investigating issues on production systems. 1) using configfs to load these configurations keeps all the coresight configuration ABI in a single file system - configfs. The current builtin configurations can be viewed, enabled, and parameters configrured in the current configfs that we have upstreamed. Adding load / unload here is a logical extension of this. 2) because of the nature of configurations described above - we would need a separate device to represent the whole subsystem - in order to provide the ioctl support for loading. This device approach to managing configurations has been previously rejected by the Coresight maintainers, who suggested that configfs was the correct way to configure a complex sub-system. 3) configurations are variable in size, An ioctl command would provide a pointer to the configuration data - but the kernel would have to trust that the underlying data is correctly formed. With a configfs file write we get the buffer _and_ its size which is a good deal safer from an input perspective. 4) ioctl use would require a loader program - configfs allows load directly from the command line. I agree that ioctls certainly have there uses, especially with small, fixed size data types - but configfs is far better suited to this complex use case. Indeed the ioctl documentation suggests using configfs for configuration cases that are too complex for sysfs, when an ioctl may not be suitable. This use of binary attributes is based on the existing use of a configfs binary attribute is for the ACPI tables - the ACPI driver here takes the buffer, does some initial validation of the file size etc, then calls the inslall ./ validate ACPI routines where the table is added to an internal list of tables maintained by the kernel. These tables may well be shared by firmware - but are also used by the kernel. There appears to be nothing in the configfs documentation limiting the use of binary attributes for passing firmware, Even the sysfs docs suggest that this is an expected use but it is not a hard and fast rule if there are no alternatives. Granted in the vast majority of cases there are better alternatives. I believe that loading via configfs is the best and safest engineering solution for this particular use case. Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

1 year, 9 months

Re: [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files

by Mike Leach

Hi, Thanks for this - I'll fix in next set Mike On Sat, 24 Dec 2022 at 07:16, Dan Carpenter <error27(a)gmail.com> wrote: > > Hi Mike, > > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Mike-Leach/coresight-syscfg-… > base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next > patch link: https://lore.kernel.org/r/20221219234638.3661-4-mike.leach%40linaro.org > patch subject: [PATCH v5 3/6] coresight: configfs: Add in binary attributes to load files > config: arm-randconfig-m041-20221218 > compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0 > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot <lkp(a)intel.com> > | Reported-by: Dan Carpenter <error27(a)gmail.com> > > smatch warnings: > drivers/hwtracing/coresight/coresight-syscfg-configfs.c:595 cscfg_cfg_load_write() warn: possible memory leak of 'load_descs' > > vim +/load_descs +595 drivers/hwtracing/coresight/coresight-syscfg-configfs.c > > 97b8fd654556b1 Mike Leach 2022-12-19 543 static ssize_t cscfg_cfg_load_write(struct config_item *item, const void *buffer, size_t size) > 97b8fd654556b1 Mike Leach 2022-12-19 544 { > 97b8fd654556b1 Mike Leach 2022-12-19 545 struct cscfg_fs_load_descs *load_descs = 0; > 97b8fd654556b1 Mike Leach 2022-12-19 546 struct cscfg_load_owner_info *owner_info = 0; > 97b8fd654556b1 Mike Leach 2022-12-19 547 int err = 0; > 97b8fd654556b1 Mike Leach 2022-12-19 548 > 97b8fd654556b1 Mike Leach 2022-12-19 549 /* ensure we cannot simultaneously load and unload */ > 97b8fd654556b1 Mike Leach 2022-12-19 550 if (!mutex_trylock(&cfs_mutex)) > 97b8fd654556b1 Mike Leach 2022-12-19 551 return -EBUSY; > 97b8fd654556b1 Mike Leach 2022-12-19 552 > 97b8fd654556b1 Mike Leach 2022-12-19 553 /* check configfs load / unload ops are permitted */ > 97b8fd654556b1 Mike Leach 2022-12-19 554 if (!cscfg_fs_load_enabled || unload_owner_info) { > 97b8fd654556b1 Mike Leach 2022-12-19 555 err = -EBUSY; > 97b8fd654556b1 Mike Leach 2022-12-19 556 goto exit_unlock; > 97b8fd654556b1 Mike Leach 2022-12-19 557 } > 97b8fd654556b1 Mike Leach 2022-12-19 558 > 97b8fd654556b1 Mike Leach 2022-12-19 559 if (size > CSCFG_FILE_MAXSIZE) { > 97b8fd654556b1 Mike Leach 2022-12-19 560 pr_err("cscfg: Load error - Input file too large.\n"); > 97b8fd654556b1 Mike Leach 2022-12-19 561 err = -EINVAL; > 97b8fd654556b1 Mike Leach 2022-12-19 562 goto exit_unlock; > 97b8fd654556b1 Mike Leach 2022-12-19 563 } > 97b8fd654556b1 Mike Leach 2022-12-19 564 > 97b8fd654556b1 Mike Leach 2022-12-19 565 load_descs = kzalloc(sizeof(struct cscfg_fs_load_descs), GFP_KERNEL); > 97b8fd654556b1 Mike Leach 2022-12-19 566 owner_info = kzalloc(sizeof(struct cscfg_load_owner_info), GFP_KERNEL); > 97b8fd654556b1 Mike Leach 2022-12-19 567 if (!load_descs || !owner_info) { > 97b8fd654556b1 Mike Leach 2022-12-19 568 err = -ENOMEM; > 97b8fd654556b1 Mike Leach 2022-12-19 569 goto exit_memfree; > > This exit leaks (will never happen in real life though). > > 97b8fd654556b1 Mike Leach 2022-12-19 570 } > 97b8fd654556b1 Mike Leach 2022-12-19 571 > 97b8fd654556b1 Mike Leach 2022-12-19 572 owner_info->owner_handle = load_descs; > 97b8fd654556b1 Mike Leach 2022-12-19 573 owner_info->type = CSCFG_OWNER_CONFIGFS; > 97b8fd654556b1 Mike Leach 2022-12-19 574 > 97b8fd654556b1 Mike Leach 2022-12-19 575 err = cscfg_file_read_buffer(buffer, size, load_descs); > 97b8fd654556b1 Mike Leach 2022-12-19 576 if (err) { > 97b8fd654556b1 Mike Leach 2022-12-19 577 pr_err("cscfg: Load error - Failed to read input file.\n"); > 97b8fd654556b1 Mike Leach 2022-12-19 578 goto exit_memfree; > 97b8fd654556b1 Mike Leach 2022-12-19 579 } > 97b8fd654556b1 Mike Leach 2022-12-19 580 > 97b8fd654556b1 Mike Leach 2022-12-19 581 err = cscfg_load_config_sets(load_descs->config_descs, load_descs->feat_descs, owner_info); > 97b8fd654556b1 Mike Leach 2022-12-19 582 if (err) { > 97b8fd654556b1 Mike Leach 2022-12-19 583 pr_err("cscfg: Load error - Failed to load configuaration file.\n"); > 97b8fd654556b1 Mike Leach 2022-12-19 584 goto exit_memfree; > 97b8fd654556b1 Mike Leach 2022-12-19 585 } > 97b8fd654556b1 Mike Leach 2022-12-19 586 > 97b8fd654556b1 Mike Leach 2022-12-19 587 mutex_unlock(&cfs_mutex); > 97b8fd654556b1 Mike Leach 2022-12-19 588 return size; > 97b8fd654556b1 Mike Leach 2022-12-19 589 > 97b8fd654556b1 Mike Leach 2022-12-19 590 exit_memfree: > 97b8fd654556b1 Mike Leach 2022-12-19 591 cscfg_configfs_free_owner_info(owner_info); > 97b8fd654556b1 Mike Leach 2022-12-19 592 > 97b8fd654556b1 Mike Leach 2022-12-19 593 exit_unlock: > 97b8fd654556b1 Mike Leach 2022-12-19 594 mutex_unlock(&cfs_mutex); > 97b8fd654556b1 Mike Leach 2022-12-19 @595 return err; > 97b8fd654556b1 Mike Leach 2022-12-19 596 } > > -- > 0-DAY CI Kernel Test Service > https://01.org/lkp > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

1 year, 9 months

Re: [PATCH v5 6/6] Documentation: coresight: docs for config load via configfs

by Mike Leach

Hi Bagas, On Wed, 21 Dec 2022 at 03:55, Bagas Sanjaya <bagasdotme(a)gmail.com> wrote: > > On Mon, Dec 19, 2022 at 11:46:38PM +0000, Mike Leach wrote: > > diff --git a/Documentation/trace/coresight/coresight-config.rst b/Documentation/trace/coresight/coresight-config.rst > > index 6d5ffa6f7347..109053eb1b93 100644 > > --- a/Documentation/trace/coresight/coresight-config.rst > > +++ b/Documentation/trace/coresight/coresight-config.rst > > @@ -141,11 +141,11 @@ Mount configfs as normal and the 'cs-syscfg' subsystem will appear:: > > $ ls /config > > cs-syscfg stp-policy > > > > -This has two sub-directories:: > > +This has two sub-directories, with the load and unload attribute files:: > > > > $ cd cs-syscfg/ > > $ ls > > - configurations features > > + configurations features load unload > > > > The system has the configuration 'autofdo' built in. It may be examined as > > follows:: > > @@ -278,9 +278,16 @@ Creating and Loading Custom Configurations > > ========================================== > > > > Custom configurations and / or features can be dynamically loaded into the > > -system by using a loadable module. > > +system by using a loadable module, or by loading a binary configuration > > +file in configfs. > > > > -An example of a custom configuration is found in ./samples/coresight. > > +Loaded configurations can use previously loaded features. The system will > > +ensure that it is not possible to unload a feature that is currently in > > +use, by enforcing the unload order as the strict reverse of the load order. > > + > > + > > +Using a Loadable Module > > +----------------------- > > > > This creates a new configuration that uses the existing built in > > strobing feature, but provides a different set of presets. > > @@ -289,6 +296,187 @@ When the module is loaded, then the configuration appears in the configfs > > file system and is selectable in the same way as the built in configuration > > described above. > > > > -Configurations can use previously loaded features. The system will ensure > > -that it is not possible to unload a feature that is currently in use, by > > -enforcing the unload order as the strict reverse of the load order. > > +The file 'coresight-cfg-sample.c' contains the configuration and module > > +initialisation code needed to create the loadable module. > > + > > +This will be built alongside the kernel modules if select in KConfig. > > What config options (CONFIG_) are required for above to work? > > > + > > +An example of a custom configuration module is found in './samples/coresight'. > > + > > +Using a Binary Configuration File > > +--------------------------------- > > + > > +The './tools/coresight' directory contains example programs to generate and > > +read and print binary configuration files. > > + > > +Building the tools creates the 'coresight-cfg-file-gen' program that will > > +generate a configuration binary 'example1.cscfg' that can be loaded into the > > +system using configfs. The configuration declared in the source file > > +'coresight-cfg-example1.c' is named 'autofdo3' - the name that will be used > > +once loaded. > > + > > +The source files 'coresight-cfg-bufw.h' and 'coresight-cfg-bufw.c' provide a > > +standard function to convert a configuration declared in 'C' into the correct > > +binary buffer format. These files can be re-used to create new custom > > +configurations. Alternatively, addition examples can be added to the > > s/addition/additional/ > > > +'coresight-cfg-file-gen' program:: > > + > > + $ ./coresight-cfg-file-gen > > + Coresight Configuration file Generator > > + > > + Generating example1 example > > + Generating example2 example > > + > > +The program 'coresight-cfg-file-read' can read back and print a configuration > > +binary. This is built using the file reader from the driver code > > +(coresight-config-file.c), which is copied over into './tools/coresight' at > > +build time.:: > > + > > + ./coresight-cfg-file-read example1.cscfg > > + CoreSight Configuration file reader > > + ============================================ > > + > > + Configuration 1 > > + Name:- autofdo3 > > + Description:- > > + Setup ETMs with strobing for autofdo > > + Supplied presets allow experimentation with mark-space ratio for various loads > > + > > + Uses 1 features:- > > + Feature-1: strobing > > + > > + Provides 4 sets of preset values, 2 presets per set > > + set[0]: 0x7d0, 0x64, > > + set[1]: 0x7d0, 0x3e8, > > + set[2]: 0x7d0, 0x1388, > > + set[3]: 0x7d0, 0x2710, > > + > > + ============================================ > > + File contains no features > > + > > +There are additional attributes in the cs-syscfg directory - load and > > +unload that can be used to load and unload configuration binary files. To > > +load, 'cat' the binary config into the load attribute:: > > + > > + $ ls /config/cs-syscfg > > + configurations features load unload > > + $ cat example1.cscfg > /config/cs-syscfg/load > > + $ ls /config/cs-syscfg/configurations/ > > + autofdo autofdo3 > > + > > +To unload, use the same file in the unload attribute:: > > + > > + $ cat example1.cscfg > /config/cs-syscfg/unload > > + ls /config/cs-syscfg/configurations/ > > + autofdo > > + > > + > > + > > +Binary Configuration File Format > > +-------------------------------- > > + > > +The file format is defined in the source file **coresight-config-file.h** > > Use single-quote for identifier names for consistency here. > > > + > > +The source reader and generator examples produce a binary of this format. > > + > > +This arrangement is reproduced below:- > > + > > +Overall File structure > > +~~~~~~~~~~~~~~~~~~~~~~ > > + > > +:: > > + > > + [cscfg_file_header] // Mandatory > > + [CONFIG_ELEM]* // Optional - multiple, defined by cscfg_file_header.nr_configs > > + [FEATURE_ELEM]* // Optional - multiple, defined by cscfg_file_header.nr_features > > + > > +File is invalid if both [CONFIG_ELEM] and [FEATURE_ELEM] are omitted. > > + > > +A file that contains only [FEATURE_ELEM] may be loaded, and the features used > > +by subsequently loaded files with [CONFIG_ELEM] elements. > > + > > +Element Name Strings > > +~~~~~~~~~~~~~~~~~~~~ > > + > > +Configuration name strings are required to consist of alphanumeric characters and '_' only. Other special characters are not permitted. > > + > > +:: > > + my_config_2 // is a valid name. > > + this-bad-config#5 // this will not work > > Instead of using code-block for name examples, what about "... For > example, foo_bar is a valid name where as foo-bar# is not."? > > > + > > +This is in order to comply with the requirements of the perf command line. > > + > > +It is recommended that Feature and Parameter names use the same convention to allow for future enhancements to the command line syntax. > > + > > +CONFIG_ELEM element > > +~~~~~~~~~~~~~~~~~~~ > > + > > +:: > > + > > + [cscfg_file_elem_header] // header length value to end of feature strings. > > + [cscfg_file_elem_str] // name of the configuration. > > + // (see element string name requirements) > > + [cscfg_file_elem_str] // description of configuration. > > + [u16 value](nr_presets) // number of defined sets presets values. > > + [u32 value](nr_total_params) // total parameters defined by all used features. > > + [u16 value](nr_feat_refs) // number of features referenced by the configuration > > + [u64 values] * (nr_presets * nr_total_params) // the preset values. > > + [cscfg_file_elem_str] * (nr_feat_refs) // names of features used in the configurations. > > + > > +FEATURE_ELEM element > > +~~~~~~~~~~~~~~~~~~~~ > > + > > +:: > > + > > + [cscfg_file_elem_header] // header length is total bytes to end of param structures. > > + [cscfg_file_elem_str] // feature name. > > + [cscfg_file_elem_str] // feature description. > > + [u32 value](match_flags) // flags to associate the feature with a device. > > + [u16 value](nr_regs) // number of registers. > > + [u16 value](nr_params) // number of parameters. > > + [cscfg_regval_desc struct] * (nr_regs) // register definitions > > + [PARAM_ELEM] * (nr_params) // parameters definitions > > + > > +PARAM_ELEM element > > +~~~~~~~~~~~~~~~~~~ > > + > > +:: > > + > > + [cscfg_file_elem_str] // parameter name. > > + [u64 value](param_value) // initial value. > > + > > +Additional definitions. > > +~~~~~~~~~~~~~~~~~~~~~~~ > > Trim trailing period for section names > > > + > > +The following structures are defined in **coresight-config-file.h** > > + > > + * **struct cscfg_file_header** : This structure contains an initial magic number, the total > > + length of the file, and the number of configurations and features in the file. > > + * **struct cscfg_file_elem_header**: This defines the total length and type of a CONFIG_ELEM > > + or a FEATURE_ELEM. > > + * **struct cscfg_file_elem_str**: This defines a string and its length. > > Again, for consistency, wrap identifier names in single-quotes. > > > + > > +The magic number in cscfg_file_header is defined as two bitfields:: > > + > > + [31:8] Fixed magic number to identify file type. > > + [7:0] Current file format version. > > + > > +The following defines determine the maximum overall file size and maximum individual > > +string size:: > > + > > + CSCFG_FILE_MAXSIZE // maximum overall file size. > > + CSCFG_FILE_STR_MAXSIZE // maximum individual string size. > > For parameter lists in elements, use bullet lists instead. > > > + > > +Load Dependencies. > > +~~~~~~~~~~~~~~~~~~ > > Trim trailing period for section names. > > + > > +Files may be unloaded only in the strict reverse order of loading. This is enforced by the > > +configuration system. > > + > > +This is to ensure that any load dependencies are maintained. > > + > > +A configuration file that contains a CONFIG_ELEM that references named features "feat_A" and "feat_B" will load only if either:- > > +a) "feat_A" and/or "feat_B" has been loaded previously, or are present as built-in / module loaded features. > > +b) "feat_A" and/or "feat_B" are declared as FEAT_ELEM in the same file as the CONFIG_ELEM. > > Separate the preceding paragraph and the list with a blank line in order > for the list to be rendered as list. > The next version will contain all the adjustments that you suggested, though rather than using single quotes, I have changed all the occurrences to consistently the double back tick to make the filenames etc fixed width font. Thanks for the review Mike > Thanks. > > -- > An old man doll... just what I always wanted! - Clara -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

1 year, 9 months

Re: [PATCH v5 4/6] coresight: configfs: Modify config files to allow userspace use

by Mike Leach

I'll rework this in the next set. Thanks for the review Mike On Tue, 27 Dec 2022 at 17:10, Christoph Hellwig <hch(a)infradead.org> wrote: > > On Mon, Dec 19, 2022 at 11:46:36PM +0000, Mike Leach wrote: > > Update coresight-config.h and the coresight-config-file.c & .h > > to allow use in userspace programs. > > > > Use __KERNEL__ defines to filter out driver only structures and > > elements so that user space programs can use the descriptor structures. > > > > Abstract memory allocation in coresight-config-file.c to allow read > > file functions to be run in userspace and kernel drivers. > > That's now how kernel code is written. -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

1 year, 9 months

[PATCH v16 0/2] Add support for UltraSoc System Memory Buffer

by Junhao He

Add support for UltraSoc System Memory Buffer. Change since v15: - Modify document based on Bagas and Suzuki's comments. - Link: https://lore.kernel.org/lkml/20221220092945.30722-1-hejunhao3@huawei.com/ Change since v14: - Add some helpers to the buffer and simplify dumping data according to Jonathan's comment. - Link: https://lore.kernel.org/lkml/20221123123823.27973-1-hejunhao3@huawei.com/ Change since v13: - Modify document and the patches description according to Bagas's comment. - Add dependency on config ACPI, drop redundant enable hw in smb_update_buffer(), Modify smb_purge_data() description according to Suzuki's comment. - Link: https://lore.kernel.org/lkml/20221114090316.63157-1-hejunhao3@huawei.com/ Change since v12: - Modify the code style and add "#ifdef CONFIG_ACPI" according to Jonathan's comment. - Address the comments from Yicong, included drop "buf_base"__iomem attribute, modify the "reading" type to bool and fix FIELD_PREP. - Link: https://lore.kernel.org/lkml/20221109135008.9485-1-hejunhao3@huawei.com/ Change since v11: - Modify the code style and rename the macro according to Jonathan's comment. - Link: https://lore.kernel.org/lkml/20221107130624.59886-1-hejunhao3@huawei.com/ Change since v10: - Rebase onto v6.1-rc4, included similar sysfs register accessors (as same as James's patch) - Link: https://lore.kernel.org/lkml/20221022115929.7503-1-hejunhao3@huawei.com/ Change since v9: - Update the Contact tag in SMB document. - Replace the spinlock with mutex. - Do some clean-ups in "smb_enable()" and "smb_release()". - Use classic memory mapped interface. - Link: https://lore.kernel.org/linux-arm-kernel/20220818132231.28240-1-hejunhao3@h… Change since v8: - Insert a blank line at the end of the config tag in Kconfig according to Randy's comment. - Link: https://lore.kernel.org/linux-arm-kernel/20220816131634.38195-1-hejunhao3@h… Change since v7: - Use the macros for register bit flags and numbers of resource. - Cleanup punctuation. - Update the Date tag and the KernelVersion tag in the document. - Link: https://lore.kernel.org/lkml/20220712091353.34540-1-hejunhao3@huawei.com/ Change since v6: - Modify the code style and driver description according to Suzuki's comment. - Modify configuration of "drvdata->reading", to void problems in open/read concurrency scenario. - Rename the macro of "SMB_FLOW_MASK". - Use the "handle->head" to determine the page number and offset. - Link: https://lore.kernel.org/linux-arm-kernel/20220606130223.57354-1-liuqi115@hu… Change since v5: - Address the comments from Suzuki, add some comments in SMB document, and modify configuration of "drvdata->reading", to void problems in multi-core concurrency scenario - Link: https://lore.kernel.org/linux-arm-kernel/20220416083953.52610-1-liuqi115@hu… Change since v4: - Add a simple document of SMB driver according to Suzuki's comment. - Address the comments from Suzuki. - Link: https://lore.kernel.org/linux-arm-kernel/20220128061755.31909-1-liuqi115@hu… Change since v3: - Modify the file header according to community specifications. - Address the comments from Mathieu. - Link: https://lore.kernel.org/linux-arm-kernel/20211118110016.40398-1-liuqi115@hu… Change since v2: - Move ultrasoc driver to drivers/hwtracing/coresight by Mathieu's comment. - Link: https://lists.linaro.org/pipermail/coresight/2021-November/007310.html Change since v1: - Drop the document of UltraSoc according to Mathieu's comment. - Add comments to explain some private hardware settings. - Address the comments from Mathieu. - Link: https://lists.linaro.org/pipermail/coresight/2021-August/006842.html Change since RFC: - Move driver to drivers/hwtracing/coresight/ultrasoc. - Remove ultrasoc-axi-com.c, as AXI-COM doesn't need to be configured in basic tracing function. - Remove ultrasoc.c as SMB does not need to register with the ultrasoc core. - Address the comments from Mathieu and Suzuki. - Link: https://lists.linaro.org/pipermail/coresight/2021-June/006535.html Qi Liu (2): drivers/coresight: Add UltraSoc System Memory Buffer driver Documentation: Add document for UltraSoc SMB driver .../sysfs-bus-coresight-devices-ultra_smb | 31 + .../trace/coresight/ultrasoc-smb.rst | 83 +++ drivers/hwtracing/coresight/Kconfig | 12 + drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/ultrasoc-smb.c | 648 ++++++++++++++++++ drivers/hwtracing/coresight/ultrasoc-smb.h | 125 ++++ 6 files changed, 900 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb create mode 100644 Documentation/trace/coresight/ultrasoc-smb.rst create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.c create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.h -- 2.33.0

1 year, 9 months

[PATCH v2] coresight: etm4x: Fix accesses to TRCSEQRSTEVR and TRCSEQSTR

by Junhao He

The TRCSEQRSTEVR and TRCSEQSTR registers are not implemented if the TRCIDR5.NUMSEQSTATE == 0. Skip accessing the registers in such cases. Fixes: 2e1cdfe184b5 ("coresight-etm4x: Adding CoreSight ETM4x driver") Signed-off-by: Junhao He <hejunhao3(a)huawei.com> Reviewed-by: Mike Leach <mike.leach(a)linaro.org> Reviewed-by: Anshuman Khandual <anshuman.khandual(a)arm.com> --- .../hwtracing/coresight/coresight-etm4x-core.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 1cc052979e01..77bca6932f01 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -427,8 +427,10 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) etm4x_relaxed_write32(csa, config->vipcssctlr, TRCVIPCSSCTLR); for (i = 0; i < drvdata->nrseqstate - 1; i++) etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i)); - etm4x_relaxed_write32(csa, config->seq_rst, TRCSEQRSTEVR); - etm4x_relaxed_write32(csa, config->seq_state, TRCSEQSTR); + if (drvdata->nrseqstate) { + etm4x_relaxed_write32(csa, config->seq_rst, TRCSEQRSTEVR); + etm4x_relaxed_write32(csa, config->seq_state, TRCSEQSTR); + } etm4x_relaxed_write32(csa, config->ext_inp, TRCEXTINSELR); for (i = 0; i < drvdata->nr_cntr; i++) { etm4x_relaxed_write32(csa, config->cntrldvr[i], TRCCNTRLDVRn(i)); @@ -1634,8 +1636,10 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) for (i = 0; i < drvdata->nrseqstate - 1; i++) state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i)); - state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR); - state->trcseqstr = etm4x_read32(csa, TRCSEQSTR); + if (drvdata->nrseqstate) { + state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR); + state->trcseqstr = etm4x_read32(csa, TRCSEQSTR); + } state->trcextinselr = etm4x_read32(csa, TRCEXTINSELR); for (i = 0; i < drvdata->nr_cntr; i++) { @@ -1763,8 +1767,10 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) for (i = 0; i < drvdata->nrseqstate - 1; i++) etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i)); - etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR); - etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR); + if (drvdata->nrseqstate) { + etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR); + etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR); + } etm4x_relaxed_write32(csa, state->trcextinselr, TRCEXTINSELR); for (i = 0; i < drvdata->nr_cntr; i++) { -- 2.33.0

1 year, 9 months

[PATCH v3 0/3] coresight: cti: Add PM runtime call in enable_store

by James Clark

Changes since v2: * Reword first commit message and add fixes tag * Pickup Jinlong's tested-by tags ---- This should be a slight improvement on Jinlong's previous version. Now it's not possible to trigger the error message from pm_runtime_put() by calling disable twice. It's also similar to the original pre-breaking change version where pm_runtime_put() was only called if the device was actually disabled, but with one difference: Previously pm_runtime_put() was only called once for the last disable call, but because of the reference counting in pm_runtime, it should have been called once for each enable call. This meant that the clock would have never been disabled if there were ever multiple enable calls. This is now fixed. The third commit is a refactor and doesn't need to be backported. I removed one of the atomic types because it didn't appear to be required. Maybe it was added for a reason which I'm not aware of, if so it should be pretty easy to drop that change. James Clark (2): coresight: cti: Prevent negative values of enable count coresight: cti: Remove atomic type from enable_req_count Mao Jinlong (1): coresight: cti: Add PM runtime call in enable_store .../hwtracing/coresight/coresight-cti-core.c | 23 ++++++++++++------- .../hwtracing/coresight/coresight-cti-sysfs.c | 15 +++++++++--- drivers/hwtracing/coresight/coresight-cti.h | 2 +- 3 files changed, 28 insertions(+), 12 deletions(-) base-commit: 88603b6dc419445847923fcb7fe5080067a30f98 -- 2.25.1

1 year, 9 months

[PATCH] coresight: etm4x: Fix accesses to TRCSEQRSTEVR and TRCSEQSTR

by Junhao He

The TRCSEQRSTEVR and TRCSEQSTR register is not implemented if the TRCIDR5.NUMSEQSTATE == 0. Skip accessing the register in such cases. Signed-off-by: Junhao He <hejunhao3(a)huawei.com> --- .../hwtracing/coresight/coresight-etm4x-core.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 80fefaba58ee..c7a65d1524fc 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -424,8 +424,10 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) etm4x_relaxed_write32(csa, config->vipcssctlr, TRCVIPCSSCTLR); for (i = 0; i < drvdata->nrseqstate - 1; i++) etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i)); - etm4x_relaxed_write32(csa, config->seq_rst, TRCSEQRSTEVR); - etm4x_relaxed_write32(csa, config->seq_state, TRCSEQSTR); + if (drvdata->nrseqstate) { + etm4x_relaxed_write32(csa, config->seq_rst, TRCSEQRSTEVR); + etm4x_relaxed_write32(csa, config->seq_state, TRCSEQSTR); + } etm4x_relaxed_write32(csa, config->ext_inp, TRCEXTINSELR); for (i = 0; i < drvdata->nr_cntr; i++) { etm4x_relaxed_write32(csa, config->cntrldvr[i], TRCCNTRLDVRn(i)); @@ -1631,8 +1633,10 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) for (i = 0; i < drvdata->nrseqstate - 1; i++) state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i)); - state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR); - state->trcseqstr = etm4x_read32(csa, TRCSEQSTR); + if (drvdata->nrseqstate) { + state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR); + state->trcseqstr = etm4x_read32(csa, TRCSEQSTR); + } state->trcextinselr = etm4x_read32(csa, TRCEXTINSELR); for (i = 0; i < drvdata->nr_cntr; i++) { @@ -1760,8 +1764,10 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) for (i = 0; i < drvdata->nrseqstate - 1; i++) etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i)); - etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR); - etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR); + if (drvdata->nrseqstate) { + etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR); + etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR); + } etm4x_relaxed_write32(csa, state->trcextinselr, TRCEXTINSELR); for (i = 0; i < drvdata->nr_cntr; i++) { -- 2.33.0

1 year, 9 months

Re: [PATCH] coresight: tmc-etr: Don't enable ETR when it's not ready

by James Clark

On 09/01/2023 23:43, Yabin Cui wrote: > Otherwise, it may cause error in AXI bus and result in a kernel panic. Hi Yabin, Thanks for the fix. Do you have a reproducer for this or some more info? For example is it a regression or has it always been there? And on which platform. Thanks James > > Signed-off-by: Yabin Cui <yabinc(a)google.com> > --- > .../hwtracing/coresight/coresight-tmc-core.c | 4 +++- > .../hwtracing/coresight/coresight-tmc-etr.c | 18 +++++++++++++++--- > drivers/hwtracing/coresight/coresight-tmc.h | 2 +- > 3 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index 07abf28ad725..c106d142e632 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -31,7 +31,7 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); > DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); > DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); > > -void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) > +int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) > { > struct coresight_device *csdev = drvdata->csdev; > struct csdev_access *csa = &csdev->access; > @@ -40,7 +40,9 @@ void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) > if (coresight_timeout(csa, TMC_STS, TMC_STS_TMCREADY_BIT, 1)) { > dev_err(&csdev->dev, > "timeout while waiting for TMC to be Ready\n"); > + return -EBUSY; > } > + return 0; > } > > void tmc_flush_and_stop(struct tmc_drvdata *drvdata) > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 867ad8bb9b0c..2da99dd41ed6 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -983,15 +983,21 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata) > etr_buf->ops->sync(etr_buf, rrp, rwp); > } > > -static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) > +static int __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) > { > u32 axictl, sts; > struct etr_buf *etr_buf = drvdata->etr_buf; > + int rc = 0; > > CS_UNLOCK(drvdata->base); > > /* Wait for TMCSReady bit to be set */ > - tmc_wait_for_tmcready(drvdata); > + rc = tmc_wait_for_tmcready(drvdata); > + if (rc) { > + dev_err(&drvdata->csdev->dev, "not ready ETR isn't enabled\n"); > + CS_LOCK(drvdata->base); > + return rc; > + } > > writel_relaxed(etr_buf->size / 4, drvdata->base + TMC_RSZ); > writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE); > @@ -1032,6 +1038,7 @@ static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) > tmc_enable_hw(drvdata); > > CS_LOCK(drvdata->base); > + return rc; > } > > static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, > @@ -1060,7 +1067,12 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, > rc = coresight_claim_device(drvdata->csdev); > if (!rc) { > drvdata->etr_buf = etr_buf; > - __tmc_etr_enable_hw(drvdata); > + rc = __tmc_etr_enable_hw(drvdata); > + if (rc) { > + drvdata->etr_buf = NULL; > + coresight_disclaim_device(drvdata->csdev); > + tmc_etr_disable_catu(drvdata); > + } > } > > return rc; > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > index 66959557cf39..01c0382a29c0 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -255,7 +255,7 @@ struct tmc_sg_table { > }; > > /* Generic functions */ > -void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata); > +int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata); > void tmc_flush_and_stop(struct tmc_drvdata *drvdata); > void tmc_enable_hw(struct tmc_drvdata *drvdata); > void tmc_disable_hw(struct tmc_drvdata *drvdata);

1 year, 10 months

[PATCH v2 0/3] coresight: cti: Add PM runtime call in enable_store

by James Clark

This should be a slight improvement on Jinlong's previous version. Now it's not possible to trigger the error message from pm_runtime_put() by calling disable twice. It's also similar to the original pre-breaking change version where pm_runtime_put() was only called if the device was actually disabled, but with one difference: Previously pm_runtime_put() was only called once for the last disable call, but because of the reference counting in pm_runtime, it should have been called once for each enable call. This meant that the clock would have never been disabled if there were ever multiple enable calls. This is now fixed. The third commit is a refactor and doesn't need to be backported. I removed one of the atomic types because it didn't appear to be required. Maybe it was added for a reason which I'm not aware of, if so it should be pretty easy to drop that change. James Clark (2): coresight: cti: Prevent negative values of enable count coresight: cti: Remove atomic type from enable_req_count Mao Jinlong (1): coresight: cti: Add PM runtime call in enable_store .../hwtracing/coresight/coresight-cti-core.c | 23 ++++++++++++------- .../hwtracing/coresight/coresight-cti-sysfs.c | 15 +++++++++--- drivers/hwtracing/coresight/coresight-cti.h | 2 +- 3 files changed, 28 insertions(+), 12 deletions(-) base-commit: c767c34740132ffc478226864a7461493cdc2413 -- 2.25.1

1 year, 10 months

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight