CoreSight September 2023

coresight@lists.linaro.org

10 participants
39 discussions

Re: [PATCH v1 2/3] perf tools: Revert enable indices setting syntax for BPF map

by James Clark

On 04/09/2023 15:56, Ian Rogers wrote: > On Mon, Sep 4, 2023 at 4:02 AM James Clark <james.clark(a)arm.com> wrote: >> >> >> >> On 28/07/2023 01:12, Ian Rogers wrote: >>> This reverts commit e571e029bdbf ("perf tools: Enable indices setting >>> syntax for BPF map"). >>> >>> The reverted commit added a notion of arrays that could be set as >>> event terms for BPF events. The parsing hasn't worked over multiple >>> Linux releases. Given the broken nature of the parsing it appears the >>> code isn't in use, nor could I find a way for it to be used to add a >>> test. >>> >>> The original commit contains a test in the commit message, >>> however, running it yields: >>> ``` >>> $ perf record -e './test_bpf_map_3.c/map:channel.value[0,1,2,3...5]=101/' usleep 2 >>> event syntax error: '..pf_map_3.c/map:channel.value[0,1,2,3...5]=101/' >>> \___ parser error >>> Run 'perf list' for a list of valid events >>> >>> Usage: perf record [<options>] [<command>] >>> or: perf record [<options>] -- <command> [<options>] >>> >>> -e, --event <event> event selector. use 'perf list' to list available events >>> ``` >>> >>> Given the code can't be used this commit reverts and removes it. >>> >> >> Hi Ian, >> >> Unfortunately this revert breaks Coresight sink argument parsing. >> >> Before: >> >> $ perf record -e cs_etm/@tmc_etr0/ -- true >> [ perf record: Woken up 1 times to write data ] >> [ perf record: Captured and wrote 4.008 MB perf.data ] >> >> After: >> >> $ perf record -e cs_etm/@tmc_etr0/ -- true >> event syntax error: 'cs_etm/@tmc_etr0/' >> \___ parser error >> >> I can't really see how it's related to the array syntax that the commit >> messages mention, but it could either be that the revert wasn't applied >> cleanly or just some unintended side effect. >> >> We should probably add a cross platform parsing test for Coresight >> arguments, but I don't know whether we should just blindly revert the >> revert for now, or work on a new change that explicitly fixes the >> Coresight case. > > Agreed, I'll take a look. Any chance you could post the full error > message? I suspect there's a first error hiding in there too. > > Thanks, > Ian > No that seems to be the whole thing, running with -vvv and pasting everything: $ perf record -e cs_etm/@tmc_etr0/ -vvv -- true event syntax error: 'cs_etm/@tmc_etr0/' \___ parser error Run 'perf list' for a list of valid events Usage: perf record [<options>] [<command>] or: perf record [<options>] -- <command> [<options>] -e, --event <event> event selector. use 'perf list' to list available events I previously mentioned that this is a Coresight thing, but I saw that it may actually be the more generic "drv_str". Unless nothing other than Coresight is using it, then it's just a Coresight thing. >> Thanks >> James >> >>> Signed-off-by: Ian Rogers <irogers(a)google.com> >>> --- >>> tools/perf/util/parse-events.c | 8 +-- >>> tools/perf/util/parse-events.l | 11 --- >>> tools/perf/util/parse-events.y | 122 --------------------------------- >>> 3 files changed, 1 insertion(+), 140 deletions(-) >>> >>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c >>> index 02647313c918..0e2004511cf5 100644 >>> --- a/tools/perf/util/parse-events.c >>> +++ b/tools/perf/util/parse-events.c >>> @@ -800,13 +800,7 @@ parse_events_config_bpf(struct parse_events_state *parse_state, >>> >>> parse_events_error__handle(parse_state->error, idx, >>> strdup(errbuf), >>> - strdup( >>> -"Hint:\tValid config terms:\n" >>> -" \tmap:[<arraymap>].value<indices>=[value]\n" >>> -" \tmap:[<eventmap>].event<indices>=[event]\n" >>> -"\n" >>> -" \twhere <indices> is something like [0,3...5] or [all]\n" >>> -" \t(add -v to see detail)")); >>> + NULL); >>> return err; >>> } >>> } >>> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l >>> index 99335ec586ae..d7d084cc4140 100644 >>> --- a/tools/perf/util/parse-events.l >>> +++ b/tools/perf/util/parse-events.l >>> @@ -175,7 +175,6 @@ do { \ >>> %x mem >>> %s config >>> %x event >>> -%x array >>> >>> group [^,{}/]*[{][^}]*[}][^,{}/]* >>> event_pmu [^,{}/]+[/][^/]*[/][^,{}/]* >>> @@ -251,14 +250,6 @@ non_digit [^0-9] >>> } >>> } >>> >>> -<array>{ >>> -"]" { BEGIN(config); return ']'; } >>> -{num_dec} { return value(yyscanner, 10); } >>> -{num_hex} { return value(yyscanner, 16); } >>> -, { return ','; } >>> -"\.\.\." { return PE_ARRAY_RANGE; } >>> -} >>> - >>> <config>{ >>> /* >>> * Please update config_term_names when new static term is added. >>> @@ -302,8 +293,6 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); } >>> {lc_type}-{lc_op_result} { return lc_str(yyscanner, _parse_state); } >>> {lc_type}-{lc_op_result}-{lc_op_result} { return lc_str(yyscanner, _parse_state); } >>> {name_minus} { return str(yyscanner, PE_NAME); } >>> -\[all\] { return PE_ARRAY_ALL; } >>> -"[" { BEGIN(array); return '['; } >>> @{drv_cfg_term} { return drv_str(yyscanner, PE_DRV_CFG_TERM); } >>> } >>> >>> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y >>> index 454577f7aff6..5a90e7874c59 100644 >>> --- a/tools/perf/util/parse-events.y >>> +++ b/tools/perf/util/parse-events.y >>> @@ -64,7 +64,6 @@ static void free_list_evsel(struct list_head* list_evsel) >>> %token PE_LEGACY_CACHE >>> %token PE_PREFIX_MEM >>> %token PE_ERROR >>> -%token PE_ARRAY_ALL PE_ARRAY_RANGE >>> %token PE_DRV_CFG_TERM >>> %token PE_TERM_HW >>> %type <num> PE_VALUE >>> @@ -108,11 +107,6 @@ static void free_list_evsel(struct list_head* list_evsel) >>> %type <list_evsel> groups >>> %destructor { free_list_evsel ($$); } <list_evsel> >>> %type <tracepoint_name> tracepoint_name >>> -%destructor { free ($$.sys); free ($$.event); } <tracepoint_name> >>> -%type <array> array >>> -%type <array> array_term >>> -%type <array> array_terms >>> -%destructor { free ($$.ranges); } <array> >>> %type <hardware_term> PE_TERM_HW >>> %destructor { free ($$.str); } <hardware_term> >>> >>> @@ -127,7 +121,6 @@ static void free_list_evsel(struct list_head* list_evsel) >>> char *sys; >>> char *event; >>> } tracepoint_name; >>> - struct parse_events_array array; >>> struct hardware_term { >>> char *str; >>> u64 num; >>> @@ -878,121 +871,6 @@ PE_TERM >>> >>> $$ = term; >>> } >>> -| >>> -name_or_raw array '=' name_or_legacy >>> -{ >>> - struct parse_events_term *term; >>> - int err = parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_USER, $1, $4, &@1, &@4); >>> - >>> - if (err) { >>> - free($1); >>> - free($4); >>> - free($2.ranges); >>> - PE_ABORT(err); >>> - } >>> - term->array = $2; >>> - $$ = term; >>> -} >>> -| >>> -name_or_raw array '=' PE_VALUE >>> -{ >>> - struct parse_events_term *term; >>> - int err = parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER, $1, $4, false, &@1, &@4); >>> - >>> - if (err) { >>> - free($1); >>> - free($2.ranges); >>> - PE_ABORT(err); >>> - } >>> - term->array = $2; >>> - $$ = term; >>> -} >>> -| >>> -PE_DRV_CFG_TERM >>> -{ >>> - struct parse_events_term *term; >>> - char *config = strdup($1); >>> - int err; >>> - >>> - if (!config) >>> - YYNOMEM; >>> - err = parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_DRV_CFG, config, $1, &@1, NULL); >>> - if (err) { >>> - free($1); >>> - free(config); >>> - PE_ABORT(err); >>> - } >>> - $$ = term; >>> -} >>> - >>> -array: >>> -'[' array_terms ']' >>> -{ >>> - $$ = $2; >>> -} >>> -| >>> -PE_ARRAY_ALL >>> -{ >>> - $$.nr_ranges = 0; >>> - $$.ranges = NULL; >>> -} >>> - >>> -array_terms: >>> -array_terms ',' array_term >>> -{ >>> - struct parse_events_array new_array; >>> - >>> - new_array.nr_ranges = $1.nr_ranges + $3.nr_ranges; >>> - new_array.ranges = realloc($1.ranges, >>> - sizeof(new_array.ranges[0]) * >>> - new_array.nr_ranges); >>> - if (!new_array.ranges) >>> - YYNOMEM; >>> - memcpy(&new_array.ranges[$1.nr_ranges], $3.ranges, >>> - $3.nr_ranges * sizeof(new_array.ranges[0])); >>> - free($3.ranges); >>> - $$ = new_array; >>> -} >>> -| >>> -array_term >>> - >>> -array_term: >>> -PE_VALUE >>> -{ >>> - struct parse_events_array array; >>> - >>> - array.nr_ranges = 1; >>> - array.ranges = malloc(sizeof(array.ranges[0])); >>> - if (!array.ranges) >>> - YYNOMEM; >>> - array.ranges[0].start = $1; >>> - array.ranges[0].length = 1; >>> - $$ = array; >>> -} >>> -| >>> -PE_VALUE PE_ARRAY_RANGE PE_VALUE >>> -{ >>> - struct parse_events_array array; >>> - >>> - if ($3 < $1) { >>> - struct parse_events_state *parse_state = _parse_state; >>> - struct parse_events_error *error = parse_state->error; >>> - char *err_str; >>> - >>> - if (asprintf(&err_str, "Expected '%ld' to be less-than '%ld'", $3, $1) < 0) >>> - err_str = NULL; >>> - >>> - parse_events_error__handle(error, @1.first_column, err_str, NULL); >>> - YYABORT; >>> - } >>> - array.nr_ranges = 1; >>> - array.ranges = malloc(sizeof(array.ranges[0])); >>> - if (!array.ranges) >>> - YYNOMEM; >>> - array.ranges[0].start = $1; >>> - array.ranges[0].length = $3 - $1 + 1; >>> - $$ = array; >>> -} >>> >>> sep_dc: ':' | >>>

8 months, 1 week

Re: [PATCH v1 2/3] perf tools: Revert enable indices setting syntax for BPF map

by James Clark

On 28/07/2023 01:12, Ian Rogers wrote: > This reverts commit e571e029bdbf ("perf tools: Enable indices setting > syntax for BPF map"). > > The reverted commit added a notion of arrays that could be set as > event terms for BPF events. The parsing hasn't worked over multiple > Linux releases. Given the broken nature of the parsing it appears the > code isn't in use, nor could I find a way for it to be used to add a > test. > > The original commit contains a test in the commit message, > however, running it yields: > ``` > $ perf record -e './test_bpf_map_3.c/map:channel.value[0,1,2,3...5]=101/' usleep 2 > event syntax error: '..pf_map_3.c/map:channel.value[0,1,2,3...5]=101/' > \___ parser error > Run 'perf list' for a list of valid events > > Usage: perf record [<options>] [<command>] > or: perf record [<options>] -- <command> [<options>] > > -e, --event <event> event selector. use 'perf list' to list available events > ``` > > Given the code can't be used this commit reverts and removes it. > Hi Ian, Unfortunately this revert breaks Coresight sink argument parsing. Before: $ perf record -e cs_etm/@tmc_etr0/ -- true [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 4.008 MB perf.data ] After: $ perf record -e cs_etm/@tmc_etr0/ -- true event syntax error: 'cs_etm/@tmc_etr0/' \___ parser error I can't really see how it's related to the array syntax that the commit messages mention, but it could either be that the revert wasn't applied cleanly or just some unintended side effect. We should probably add a cross platform parsing test for Coresight arguments, but I don't know whether we should just blindly revert the revert for now, or work on a new change that explicitly fixes the Coresight case. Thanks James > Signed-off-by: Ian Rogers <irogers(a)google.com> > --- > tools/perf/util/parse-events.c | 8 +-- > tools/perf/util/parse-events.l | 11 --- > tools/perf/util/parse-events.y | 122 --------------------------------- > 3 files changed, 1 insertion(+), 140 deletions(-) > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index 02647313c918..0e2004511cf5 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -800,13 +800,7 @@ parse_events_config_bpf(struct parse_events_state *parse_state, > > parse_events_error__handle(parse_state->error, idx, > strdup(errbuf), > - strdup( > -"Hint:\tValid config terms:\n" > -" \tmap:[<arraymap>].value<indices>=[value]\n" > -" \tmap:[<eventmap>].event<indices>=[event]\n" > -"\n" > -" \twhere <indices> is something like [0,3...5] or [all]\n" > -" \t(add -v to see detail)")); > + NULL); > return err; > } > } > diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l > index 99335ec586ae..d7d084cc4140 100644 > --- a/tools/perf/util/parse-events.l > +++ b/tools/perf/util/parse-events.l > @@ -175,7 +175,6 @@ do { \ > %x mem > %s config > %x event > -%x array > > group [^,{}/]*[{][^}]*[}][^,{}/]* > event_pmu [^,{}/]+[/][^/]*[/][^,{}/]* > @@ -251,14 +250,6 @@ non_digit [^0-9] > } > } > > -<array>{ > -"]" { BEGIN(config); return ']'; } > -{num_dec} { return value(yyscanner, 10); } > -{num_hex} { return value(yyscanner, 16); } > -, { return ','; } > -"\.\.\." { return PE_ARRAY_RANGE; } > -} > - > <config>{ > /* > * Please update config_term_names when new static term is added. > @@ -302,8 +293,6 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); } > {lc_type}-{lc_op_result} { return lc_str(yyscanner, _parse_state); } > {lc_type}-{lc_op_result}-{lc_op_result} { return lc_str(yyscanner, _parse_state); } > {name_minus} { return str(yyscanner, PE_NAME); } > -\[all\] { return PE_ARRAY_ALL; } > -"[" { BEGIN(array); return '['; } > @{drv_cfg_term} { return drv_str(yyscanner, PE_DRV_CFG_TERM); } > } > > diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y > index 454577f7aff6..5a90e7874c59 100644 > --- a/tools/perf/util/parse-events.y > +++ b/tools/perf/util/parse-events.y > @@ -64,7 +64,6 @@ static void free_list_evsel(struct list_head* list_evsel) > %token PE_LEGACY_CACHE > %token PE_PREFIX_MEM > %token PE_ERROR > -%token PE_ARRAY_ALL PE_ARRAY_RANGE > %token PE_DRV_CFG_TERM > %token PE_TERM_HW > %type <num> PE_VALUE > @@ -108,11 +107,6 @@ static void free_list_evsel(struct list_head* list_evsel) > %type <list_evsel> groups > %destructor { free_list_evsel ($$); } <list_evsel> > %type <tracepoint_name> tracepoint_name > -%destructor { free ($$.sys); free ($$.event); } <tracepoint_name> > -%type <array> array > -%type <array> array_term > -%type <array> array_terms > -%destructor { free ($$.ranges); } <array> > %type <hardware_term> PE_TERM_HW > %destructor { free ($$.str); } <hardware_term> > > @@ -127,7 +121,6 @@ static void free_list_evsel(struct list_head* list_evsel) > char *sys; > char *event; > } tracepoint_name; > - struct parse_events_array array; > struct hardware_term { > char *str; > u64 num; > @@ -878,121 +871,6 @@ PE_TERM > > $$ = term; > } > -| > -name_or_raw array '=' name_or_legacy > -{ > - struct parse_events_term *term; > - int err = parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_USER, $1, $4, &@1, &@4); > - > - if (err) { > - free($1); > - free($4); > - free($2.ranges); > - PE_ABORT(err); > - } > - term->array = $2; > - $$ = term; > -} > -| > -name_or_raw array '=' PE_VALUE > -{ > - struct parse_events_term *term; > - int err = parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER, $1, $4, false, &@1, &@4); > - > - if (err) { > - free($1); > - free($2.ranges); > - PE_ABORT(err); > - } > - term->array = $2; > - $$ = term; > -} > -| > -PE_DRV_CFG_TERM > -{ > - struct parse_events_term *term; > - char *config = strdup($1); > - int err; > - > - if (!config) > - YYNOMEM; > - err = parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_DRV_CFG, config, $1, &@1, NULL); > - if (err) { > - free($1); > - free(config); > - PE_ABORT(err); > - } > - $$ = term; > -} > - > -array: > -'[' array_terms ']' > -{ > - $$ = $2; > -} > -| > -PE_ARRAY_ALL > -{ > - $$.nr_ranges = 0; > - $$.ranges = NULL; > -} > - > -array_terms: > -array_terms ',' array_term > -{ > - struct parse_events_array new_array; > - > - new_array.nr_ranges = $1.nr_ranges + $3.nr_ranges; > - new_array.ranges = realloc($1.ranges, > - sizeof(new_array.ranges[0]) * > - new_array.nr_ranges); > - if (!new_array.ranges) > - YYNOMEM; > - memcpy(&new_array.ranges[$1.nr_ranges], $3.ranges, > - $3.nr_ranges * sizeof(new_array.ranges[0])); > - free($3.ranges); > - $$ = new_array; > -} > -| > -array_term > - > -array_term: > -PE_VALUE > -{ > - struct parse_events_array array; > - > - array.nr_ranges = 1; > - array.ranges = malloc(sizeof(array.ranges[0])); > - if (!array.ranges) > - YYNOMEM; > - array.ranges[0].start = $1; > - array.ranges[0].length = 1; > - $$ = array; > -} > -| > -PE_VALUE PE_ARRAY_RANGE PE_VALUE > -{ > - struct parse_events_array array; > - > - if ($3 < $1) { > - struct parse_events_state *parse_state = _parse_state; > - struct parse_events_error *error = parse_state->error; > - char *err_str; > - > - if (asprintf(&err_str, "Expected '%ld' to be less-than '%ld'", $3, $1) < 0) > - err_str = NULL; > - > - parse_events_error__handle(error, @1.first_column, err_str, NULL); > - YYABORT; > - } > - array.nr_ranges = 1; > - array.ranges = malloc(sizeof(array.ranges[0])); > - if (!array.ranges) > - YYNOMEM; > - array.ranges[0].start = $1; > - array.ranges[0].length = $3 - $1 + 1; > - $$ = array; > -} > > sep_dc: ':' | >

8 months, 1 week

[PATCH] MAINTAINERS: Remove myself as a Arm CoreSight reviewer

by Leo Yan

I haven't done any meaningful work for a long while on Arm CoreSight and it's unlikely I'll be able to do related work in the future. Remove myself from the Arm CoreSight "Reviewers" list. Signed-off-by: Leo Yan <leo.yan(a)linaro.org> --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index e0ad886d3163..342b8a3e19e3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2102,7 +2102,6 @@ N: digicolor ARM/CORESIGHT FRAMEWORK AND DRIVERS M: Suzuki K Poulose <suzuki.poulose(a)arm.com> R: Mike Leach <mike.leach(a)linaro.org> -R: Leo Yan <leo.yan(a)linaro.org> L: coresight(a)lists.linaro.org (moderated for non-subscribers) L: linux-arm-kernel(a)lists.infradead.org (moderated for non-subscribers) S: Maintained -- 2.39.2

8 months, 1 week

[RESEND PATCH] MAINTAINERS: Remove myself as a Arm CoreSight reviewer

by Leo Yan

I haven't done any meaningful work for a long while on Arm CoreSight and it's unlikely I'll be able to do related work in the future. Remove myself from the Arm CoreSight "Reviewers" list. Signed-off-by: Leo Yan <leo.yan(a)linaro.org> --- Rebased on Linux kernel 6.5. MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 1cc0be8ea75a..f72a275c8ea1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2044,7 +2044,6 @@ ARM/CORESIGHT FRAMEWORK AND DRIVERS M: Suzuki K Poulose <suzuki.poulose(a)arm.com> R: Mike Leach <mike.leach(a)linaro.org> R: James Clark <james.clark(a)arm.com> -R: Leo Yan <leo.yan(a)linaro.org> L: coresight(a)lists.linaro.org (moderated for non-subscribers) L: linux-arm-kernel(a)lists.infradead.org (moderated for non-subscribers) S: Maintained -- 2.34.1

8 months, 1 week

[RFC 0/7] coresight: Move remaining AMBA ACPI devices into platform driver

by Anshuman Khandual

This moves remaining AMBA ACPI devices into respective platform drivers for enabling ACPI based power management support. This might still require some further changes but presented here just for some initial review & feedback. This series applies on coresight/next coresight-next-v6.6 and has been built tested. This series has also been boot tested on a DT based coresight device platform. Although it still requires testing on ACPI platform. Cc: Suzuki Poulose <suzuki.poulose(a)arm.com> Cc: James Clark <james.clark(a)arm.com> Cc: Mike Leach <mike.leach(a)linaro.org> Cc: coresight(a)lists.linaro.org Cc: linux-arm-kernel(a)lists.infradead.org Cc: linux-kernel(a)vger.kernel.org Anshuman Khandual (7): coresight: replicator: Move ACPI support from AMBA driver to platform driver coresight: funnel: Move ACPI support from AMBA driver to platform driver coresight: catu: Move ACPI support from AMBA driver to platform driver coresight: tpiu: Move ACPI support from AMBA driver to platform driver coresight: tmc: Move ACPI support from AMBA driver to platform driver coresight: stm: Move ACPI support from AMBA driver to platform driver coresight: debug: Move ACPI support from AMBA driver to platform driver drivers/acpi/acpi_amba.c | 8 -- drivers/hwtracing/coresight/coresight-catu.c | 136 ++++++++++++++++-- drivers/hwtracing/coresight/coresight-catu.h | 1 + .../hwtracing/coresight/coresight-cpu-debug.c | 130 +++++++++++++++-- .../hwtracing/coresight/coresight-funnel.c | 49 ++++--- .../coresight/coresight-replicator.c | 44 +++--- drivers/hwtracing/coresight/coresight-stm.c | 80 +++++++++-- .../hwtracing/coresight/coresight-tmc-core.c | 127 ++++++++++++++-- drivers/hwtracing/coresight/coresight-tmc.h | 1 + drivers/hwtracing/coresight/coresight-tpiu.c | 76 +++++++++- 10 files changed, 549 insertions(+), 103 deletions(-) -- 2.25.1

8 months, 1 week

Re: Merge conflict resolution for CoreSight ACPI TRBE support

by Suzuki K Poulose

On 01/09/2023 20:23, Greg Kroah-Hartman wrote: > On Fri, Sep 01, 2023 at 06:38:14PM +0100, Suzuki K Poulose wrote: >> On 01/09/2023 16:47, Greg Kroah-Hartman wrote: >>> On Fri, Sep 01, 2023 at 12:15:51PM +0100, Suzuki K Poulose wrote: >>>> Hi Greg >>>> >>>> I have a question for you w.r.t pushing this series [0]. This >>>> involved some changes to the ARM PMU ACPI stub code, for parsing >>>> the ACPI tables to detect TRBE (managed via CoreSight drivers). >>>> >>>> The ARM PMU ACPI stub is maintained by Will and he tried to queue this >>>> in, but there was a conflict [1] with the CoreSight branch and >>>> hence he dropped the coresight changes, keeping the base changes. >>>> >>>> His questions in the thread above were clariried [2] and Anshuman has >>>> posted the "coresight" changes alone [3]. >>>> >>>> Now, I have already sent a pull request for v6.6. Do you think >>>> I can send a pull request with the remaining changes from Anshuman ? >>> >>> Has any of this been in linux-next yet? If not, then 6.6 is not going >>> to happen, sorry. >> >> As mentioned they were in shortly, but dropped due to the conflict with >> CoreSight tree. > > Then the code can't be merged into 6.6-rc1. > >>> If it has been in linux-next, then what's the issue? >>> >>>> If so, what do you recommend ? The dependent patches are already queued >>>> via Will's tree and merged by Linus [4] >>>> >>>> I am happy to wait for the next cycle, if this is inconvenient for you. >>> >>> I can't take anything new for my trees until after 6.6-rc1 is out, >>> neither can anyone else. >>> >>> And if the dependancies are going to go in for 6.6-rc1, then what's the >>> issue? >> >> It is just that the functionality was split and there is something in >> in 6.6-rc1, which nobody is consuming, unless these two patches come in. >> Do you recommend sending them for v6.6 ? Or queue them for v6.7 ? > > Queue for 6.7 please. Thanks, will do. Suzuki

8 months, 1 week

Re: Merge conflict resolution for CoreSight ACPI TRBE support

by Suzuki K Poulose

On 01/09/2023 16:47, Greg Kroah-Hartman wrote: > On Fri, Sep 01, 2023 at 12:15:51PM +0100, Suzuki K Poulose wrote: >> Hi Greg >> >> I have a question for you w.r.t pushing this series [0]. This >> involved some changes to the ARM PMU ACPI stub code, for parsing >> the ACPI tables to detect TRBE (managed via CoreSight drivers). >> >> The ARM PMU ACPI stub is maintained by Will and he tried to queue this >> in, but there was a conflict [1] with the CoreSight branch and >> hence he dropped the coresight changes, keeping the base changes. >> >> His questions in the thread above were clariried [2] and Anshuman has >> posted the "coresight" changes alone [3]. >> >> Now, I have already sent a pull request for v6.6. Do you think >> I can send a pull request with the remaining changes from Anshuman ? > > Has any of this been in linux-next yet? If not, then 6.6 is not going > to happen, sorry. As mentioned they were in shortly, but dropped due to the conflict with CoreSight tree. > > If it has been in linux-next, then what's the issue? > >> If so, what do you recommend ? The dependent patches are already queued >> via Will's tree and merged by Linus [4] >> >> I am happy to wait for the next cycle, if this is inconvenient for you. > > I can't take anything new for my trees until after 6.6-rc1 is out, > neither can anyone else. > > And if the dependancies are going to go in for 6.6-rc1, then what's the > issue? It is just that the functionality was split and there is something in in 6.6-rc1, which nobody is consuming, unless these two patches come in. Do you recommend sending them for v6.6 ? Or queue them for v6.7 ? Kind regards Suzuki > > confused, > > greg k-h

8 months, 1 week

Re: [PATCH V2] coresight: tmc: Make etr buffer mode user configurable from sysfs

by Suzuki K Poulose

Cc: Sudeep Hi Steve On 28/08/2023 17:35, Steve Clevenger wrote: > > Hi Suzuki, > > On 8/27/2023 2:35 PM, Suzuki K Poulose wrote: >> Hi Steve >> >> On 26/08/2023 01:14, Steve Clevenger wrote: >>> >>> Unfortunately, I tested with the original patch not [PATCH V2]. I've >>> remedied this. My results below: >>> >>> [root@sut01sys-b212 linux]# cat >>> /sys/devices/system/cpu/cpu123/ARMHC501\:23/tmc_etr35/buf_modes_available >>> auto flat catu >>> [root@sut01sys-b212 linux]# cat >>> /sys/devices/system/cpu/cpu123/ARMHC501\:23/tmc_etr35/buf_mode_preferred >>> auto >>> [root@sut01sys-b212 linux]# echo "catu" > >>> /sys/devices/system/cpu/cpu123/ARMHC501\:23/tmc_etr35/buf_mode_preferred >>> [root@sut01sys-b212 linux]# cat >>> /sys/devices/system/cpu/cpu123/ARMHC501\:23/tmc_etr35/buf_mode_preferred >>> catu >>> >>> As with the V1 patch, auto defaults to catu. >>> >>> I expected to see tmc-sg (former default) as an available mode, but do >>> not. As I recall, the buffer mode defaulted to ETR scatter-gather prior >>> to this patch. Must this capability now be explicitly advertised? I've >>> seen this done as "arm,scatter-gather" in device trees, but not used by >>> Ampere. Perhaps someone can enlighten me. >> >> Yes, you must add that property to the TMC-ETR node (for both DT and >> ACPI). In the past, almost all of the TMC-ETRs (except Juno board) >> locked up the system while using the SG mode (due to the interconnect >> issues, something to do with the transaction). Thus, we decided to >> add a property explicitly enabling this for a given platform. >> >> When you mentioned, it was using TMC-ETR SG mode, how did you verify >> this ? Please be aware that the table allocation code etc are shared >> by both TMC-SG and CATU. >> > > You might recall how this started. I had no way to test the CATU due to > the order the ETR modes defaulted (Flat, ETR-SG, CATU). For test > purposes, I programmatically swapped the ETR-SG/CATU order and could > then verify CATU operation by the driver calling into CATU code. This So, were you using the DT based boot for the above runs ? > suggests Flat mode was bypassed, and the driver defaulted to ETR-SG > prior to this hack. This didn't offer the user any control, hence my > feature request. Note that most of the early Ampere self-hosted trace > collection used ETR-SG. Now I can't select it. > > How is this property described in the ACPI? The "ACPI for CoreSight™ 1.1 > Platform Design Document" (DEN0067) doesn't describe this. This is not specified in the ACPI platform design document. I can get it fixed. Ideally we need a property describing that the scatter-gather mode is safe to use. DT uses "arm,scatter-gather" property [0] and this is what we now expect in the ACPI based systems too. https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bin… Does it sound fine ? Suzuki > > Thanks, > Steve > > >> Kind regards >> Suzuki >> >>> >>> Steve C. >>> >>> On 8/23/2023 4:10 PM, Steve Clevenger wrote: >>>> >>>> Here's some quick feedback. My system shows two modes available; auto >>>> catu >>>> >>>> etr_buf_mode_current is writable. I expected to see tmc-sg (former >>>> default) listed in etr_buf_modes_available but it doesn't show up. >>>> >>>> Note that both the auto and catu etr_buf_mode_current settings default >>>> to catu. My understanding is auto should revert to the default behavior. >>>> On my system the default was tmc-sg. >>>> >>>> More later. >>>> >>>> [root@sut01sys-b212 kernel]# cat >>>> /sys/devices/system/cpu/cpu20/ARMHC501\:60/tmc_etr96/etr_buf_modes_available >>>> >>>> auto catu >>>> [root@sut01sys-b212 kernel]# cat >>>> /sys/devices/system/cpu/cpu20/ARMHC501\:60/tmc_etr96/etr_buf_mode_current >>>> catu >>>> [root@sut01sys-b212 kernel]# echo "catu" > >>>> /sys/devices/system/cpu/cpu20/ARMHC501\:60/tmc_etr96/etr_buf_mode_current >>>> [root@sut01sys-b212 kernel]# cat >>>> /sys/devices/system/cpu/cpu20/ARMHC501\:60/tmc_etr96/etr_buf_mode_current >>>> catu >>>> >>>> Steve C. >>>> >>>> >>>> On 8/21/2023 12:40 PM, Steve Clevenger wrote: >>>>> >>>>> Hi Suzuki, >>>>> >>>>> I may be able to test it this week. You've already pointed me at the >>>>> patch thread(s). The main holdup is I need to merge the 6.6 pending >>>>> platform work in order to use the Ampere ACPI. I couldn't get these >>>>> patches to apply directly to 6.4 last I tried. >>>>> >>>>> Steve C. >>>>> >>>>> On 8/18/2023 2:39 AM, Suzuki K Poulose wrote: >>>>>> Cc: Steve >>>>>> >>>>>> Steve, >>>>>> >>>>>> Are you able to test this with CATU ? >>>>>> >>>>>> >>>>>> On 18/08/2023 09:21, Anshuman Khandual wrote: >>>>>>> Currently TMC-ETR automatically selects the buffer mode from all >>>>>>> available >>>>>>> methods in the following sequentially fallback manner - also in that >>>>>>> order. >>>>>>> >>>>>>> 1. FLAT mode with or without IOMMU >>>>>>> 2. TMC-ETR-SG (scatter gather) mode when available >>>>>>> 3. CATU mode when available >>>>>>> >>>>>>> But this order might not be ideal for all situations. For example if >>>>>>> there >>>>>>> is a CATU connected to ETR, it may be better to use TMC-ETR scatter >>>>>>> gather >>>>>>> method, rather than CATU. But hard coding such order changes will >>>>>>> prevent >>>>>>> us from testing or using a particular mode. This change provides >>>>>>> following >>>>>>> new sysfs tunables for the user to control TMC-ETR buffer mode >>>>>>> explicitly, >>>>>>> if required. This adds following new sysfs files for buffer mode >>>>>>> selection >>>>>>> purpose explicitly in the user space. >>>>>>> >>>>>>> /sys/bus/coresight/devices/tmc_etr<N>/buf_modes_available >>>>>>> /sys/bus/coresight/devices/tmc_etr<N>/buf_mode_preferred >>>>>>> >>>>>>> $ cat buf_modes_available >>>>>>> auto flat tmc-sg catu ------------------> Supported TMC-ETR buffer >>>>>>> modes >>>>>>> >>>>>>> $ echo catu > buf_mode_preferred -------> Explicit buffer mode >>>>>>> request >>>>>>> >>>>>>> But explicit user request has to be within supported ETR buffer modes >>>>>>> only. >>>>>>> These sysfs interface files are exclussive to ETR, and hence these >>>>>>> are >>>>>>> not >>>>>>> available for other TMC devices such as ETB or ETF etc. >>>>>>> >>>>>>> A new auto' mode (i.e ETR_MODE_AUTO) has been added to help fallback >>>>>>> to the >>>>>>> existing default behaviour, when user provided preferred buffer mode >>>>>>> fails. >>>>>>> ETR_MODE_FLAT and ETR_MODE_AUTO are always available as preferred >>>>>>> modes. >>>>>>> >>>>>>> Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com> >>>>>>> Cc: Mike Leach <mike.leach(a)linaro.org> >>>>>>> Cc: James Clark <james.clark(a)arm.com> >>>>>>> Cc: Leo Yan <leo.yan(a)linaro.org> >>>>>>> Cc: Alexander Shishkin <alexander.shishkin(a)linux.intel.com> >>>>>>> Cc: coresight(a)lists.linaro.org >>>>>>> Cc: linux-arm-kernel(a)lists.infradead.org >>>>>>> Cc: linux-kernel(a)vger.kernel.org >>>>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual(a)arm.com> >>>>>>> --- >>>>>>> This applies on v6.5-rc6 >>>>>>> >>>>>>> Changes in V2: >>>>>>> >>>>>>> - Renamed sysfs file etr_buf_modes_available as buf_modes_available >>>>>>> - Renamed sysfs file buf_mode_current as buf_mode_preferred >>>>>>> - Renamed etr_supports_flat_mode() as etr_can_use_flat_mode() >>>>>>> - Renamed coresight_tmc_groups[] as coresight_etf_groups[] >>>>>>> - Reused coresight_tmc_group[] for trigger_cntr and buffer_size >>>>>>> - Fallback trying ETR_MODE_AUTO when user preferred mode fails >>>>>>> - Moved ETR sysfs details into coresight-tmc-etr.c >>>>>>> - Dropped etr_can_use_flat_mode() check while offering ETR_MODE_FLAT >>>>>>> in sysfs >>>>>>> - Moved struct etr_buf_hw inside coresight-tmc-etr.c >>>>>>> - Moved get_etr_buf_hw() and etr_can_use_flat_mode() inside >>>>>>> coresight-tmc-etr.c >>>>>>> - Updated month in >>>>>>> Documentation/ABI/testing/sysfs-bus-coresight-devices-tmc >>>>>>> >>>>>>> Changes in V1: >>>>>>> >>>>>>> https://lore.kernel.org/all/20230728084837.276551-1-anshuman.khandual@arm.c… >>>>>>> >>>>>>> .../testing/sysfs-bus-coresight-devices-tmc | 16 +++ >>>>>>> .../hwtracing/coresight/coresight-tmc-core.c | 15 ++- >>>>>>> .../hwtracing/coresight/coresight-tmc-etr.c | 111 >>>>>>> ++++++++++++++++-- >>>>>>> drivers/hwtracing/coresight/coresight-tmc.h | 3 + >>>>>>> 4 files changed, 131 insertions(+), 14 deletions(-) >>>>>> >>>>>> >>>>>> Looks good to me. >>>>>> >>>>>> Suzuki >>>>>> >>>>>> >>

8 months, 1 week

Merge conflict resolution for CoreSight ACPI TRBE support

by Suzuki K Poulose

Hi Greg I have a question for you w.r.t pushing this series [0]. This involved some changes to the ARM PMU ACPI stub code, for parsing the ACPI tables to detect TRBE (managed via CoreSight drivers). The ARM PMU ACPI stub is maintained by Will and he tried to queue this in, but there was a conflict [1] with the CoreSight branch and hence he dropped the coresight changes, keeping the base changes. His questions in the thread above were clariried [2] and Anshuman has posted the "coresight" changes alone [3]. Now, I have already sent a pull request for v6.6. Do you think I can send a pull request with the remaining changes from Anshuman ? If so, what do you recommend ? The dependent patches are already queued via Will's tree and merged by Linus [4] I am happy to wait for the next cycle, if this is inconvenient for you. Kind regards Suzuki [0] https://lkml.kernel.org/r/20230817055405.249630-1-anshuman.khandual@arm.com [1] https://lkml.kernel.org/r/20230821144030.52ab1357@canb.auug.org.au [2] https://lore.kernel.org/all/96d41d86-bfa2-f4d4-49ea-449c3471d036@arm.com/ [3] https://lkml.kernel.org/r/20230829135405.1159449-1-anshuman.khandual@arm.com [4] https://git.kernel.org/torvalds/c/1aa3d0274a4a

8 months, 1 week

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight September 2023