Hi James
On Tue, 3 Dec 2024 at 14:25, James Clark <james.clark@xxxxxxxxxx> wrote:
On 27/11/2024 1:42 pm, Mike Leach wrote:
Add an example config table generator to test loading configuration
tables.
Provides a table buffer writer function that can be re-used in other
userspace programs.
Table write format matches that expected by the corresponding reader
in the configfs driver code.
Generates tables and outputs in form of binary files.
Add a config table file reader and printer. Takes in config table files
and prints the contents. Uses table reader source from kernel driver.
Signed-off-by: Mike Leach <mike.leach@xxxxxxxxxx>
---
MAINTAINERS | 1 +
.../coresight/coresight-config-table.h | 5 +
Hi Mike,
Isn't there some convention about maintaining a copy of kernel headers
in the tools? Especially as you wouldn't rebuild the tools after
updating the kernel headers so breakages might go unnoticed.
Not sure - perf keeps a copy and has a check script on build to ensure
the copy matches the kernel version.
Keeping two copies of the same thing always strikes me as poor
practice, so I went for copying over.
Both methods risk breakages when something in the kernel changes.
[...]
+
+/*
+ * sets of presets leaves strobing window constant while varying period to allow
+ * experimentation with mark / space ratios for various workloads
+ */
+static u64 afdo_set_a_presets[AFDO_NR_PRESETS][AFDO_NR_PARAM_SUM] = {
+ { 2000, 100 },
+ { 2000, 1000 },
+ { 2000, 5000 },
+ { 2000, 10000 },
+ { 4000, 100 },
The comment above here looks like its for example1, this one does vary
the window size.
Probably only example2 is enough, I assumed they were different but
example2 is basically the same as example1 with an extra preset list. We
could comment that the second preset list is optional and delete
example1. Saves people reading more and wondering what the difference is.
Yes - perhaps both examples were not necessary - but the point was you
can have two configs in a single loadable table.
I tried to make an example that doesn't use an existing feature by
reacreating afdo from scratch which I thought would be a good example.
It's pasted at the end. I had to copy paste the ETMv4 macros and
constants though, I couldn't include them in the userspace generator
because of this error:
The updated ETM config set that is to follow addresses this issue -
the macros are split off into a separate file (and adds in a whole
lot of validation - ensuring a configuration cannot specify and
allocate more resources than are available.)
This is also the reason that the examples provided were very simple.
More complex ones are to follow!
This set was focused on loading tables so that the next patchsets
dealing with resource validating ETM configs and CTI configs had an
easy to use test platform.
coresight-config.h:10:10: fatal error: linux/coresight.h: No such
file
or directory
10 | #include <linux/coresight.h>
I also got this error when loading it:
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000008
cscfg_reset_feat (drivers/hwtracing/coresight/coresight-config.c:64
coresight-config.c:124) coresight
cscfg_load_config_sets
(drivers/hwtracing/coresight/coresight-syscfg.c:217
coresight-syscfg.c:262 coresight-syscfg.c:492 coresight-syscfg.c:610)
coresight
cscfg_dyn_load_cfg_table
(drivers/hwtracing/coresight/coresight-syscfg-configfs.c:294) coresight
cscfg_cfg_load_table_write
(drivers/hwtracing/coresight/coresight-syscfg-configfs.c:799) coresight
configfs_release_bin_file (fs/configfs/file.c:415)
__fput (fs/file_table.c:432)
__fput_sync (fs/file_table.c:517)
__arm64_sys_close (fs/open.c:1568 fs/open.c:1550 fs/open.c:1550)
invoke_syscall (arch/arm64/kernel/syscall.c:?
arch/arm64/kernel/syscall.c:49)
el0_svc_common (include/linux/thread_info.h:127
arch/arm64/kernel/syscall.c:140)
do_el0_svc (arch/arm64/kernel/syscall.c:152)
I'll go back to my tests to check I was testing what I thought I was -
and didn't accidentally change something after testing and before
sending the patchset.
So I'm wondering if we can do the same thing by setting values via
individual files rather than one binary blob which would avoid some of
these issues. From what I understand the feature params can already be
set directly this way via
/sys/kernel/config/cs-syscfg/features/strobing/params/period/value
We'd have to add a way to add new configs with a mkdir, then the same
things can be configured without needing an additional tool. Links
between features and configs can be done with symlinks which is also
mentioned in the configfs docs.
Something like this would be a bit like the current version:
# ls /config/cs-syscfg
configurations
features
# mkdir /config/cs-syscfg/features/my_config
# ls /config/cs-syscfg/features/my_config
description
matches
regs_desc
params
# mkdir /config/cs-syscfg/features/my_config/regs_desc/TRCRSCTLRn0
# ls /config/cs-syscfg/features/my_config/TRCRSCTLRn0
type
offset
val
mask
This is precisely what I wanted to avoid. Doing it this way is both
time consuming for the user and ends up reproducing most of the sysfs
files in configfs.
Parameters are there to allow a limited amount of relevent runtime> The resource approach allows us to define certain bitfields e.g.> Branch broadcast - as user configurable - but omit anything users are
adjustment. Parameter values may also only populate part of a hw
register depending on use case.
not allowed to meddle with.
The table load is atomic - it is validated and succeeds or fails
completely. The load mechanism prevents the new configuration from
becoming visible until it is loaded. The configfs per file method has
an issue in deciding when the configuration is "complete". If you look
in the configfs docs/source there was at one time a "commit"
methodology proposed, but this never came to fruition.
Table load re-uses the same mechanisms as the built-in and loadable> not the kernel version.
module methods for adding configurations, but without the need for
re-compiling the kernel / compiling against a specific kernel -
meaning loadable tables are dependent only on the hardware available,
Adding a per file option means that the new
configurations would be different from the current load methods
upstream
But another way could be to enumerate all possible regs for each device.
This would remove the need to have all the #defines in whatever tool is
making the config (avoiding the #include issue from above):
# mkdir /config/cs-syscfg/features/my_config
# ls /config/cs-syscfg/features/my_config/regs_desc
regs_desc is initially empty, but specify what device it's for to make
all the properties appear (or the mkdir could be done in an etmv4
folder):
# echo "SRC_ETM4" > matches
# ls /config/cs-syscfg/features/my_config/regs_desc
TRCRSCTLRn0
TRCRSCTLRn1
TRCRSCTLRn2
... etc ...
Now type and offset don't need to be set:
# ls /config/cs-syscfg/features/my_config/regs_desc/TRCRSCTLRn0
val
mask
save
Don't we already have the full list of parameters in
etm4_cfg_map_reg_offset(), so we can expose this to users via configfs
directly rather than needing any tooling. And doesn't any new device
that's supported by the config mechanism need to know about all its
parameters, so it wouldn't remove any flexibility?
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK