Hi Christoph
On Tue, 27 Dec 2022 at 17:09, Christoph Hellwig hch@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