CoreSight April 2023

coresight@lists.linaro.org

9 participants
28 discussions

Re: [PATCH] coresight: Fix signedness bug in tmc_etr_buf_insert_barrier_packet()

by Suzuki K Poulose

Hi Dan On 21/04/2023 11:42, Dan Carpenter wrote: > This code generates a Smatch warning: > > drivers/hwtracing/coresight/coresight-tmc-etr.c:947 tmc_etr_buf_insert_barrier_packet() > error: uninitialized symbol 'bufp'. > > The problem is that if tmc_sg_table_get_data() returns -EINVAL, then > when we test if "len < CORESIGHT_BARRIER_PKT_SIZE", the negative "len" > value is type promoted to a high unsigned long value which is greater > than CORESIGHT_BARRIER_PKT_SIZE. Fix this bug by adding an explicit > check for error codes. > > Fixes: 75f4e3619fe2 ("coresight: tmc-etr: Add transparent buffer management") > Signed-off-by: Dan Carpenter <dan.carpenter(a)linaro.org> > --- > drivers/hwtracing/coresight/coresight-tmc-etr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 918d461fcf4a..eaa296ced167 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -942,7 +942,7 @@ tmc_etr_buf_insert_barrier_packet(struct etr_buf *etr_buf, u64 offset) > > len = tmc_etr_buf_get_data(etr_buf, offset, > CORESIGHT_BARRIER_PKT_SIZE, &bufp); > - if (WARN_ON(len < CORESIGHT_BARRIER_PKT_SIZE)) > + if (WARN_ON(len < 0 || len < CORESIGHT_BARRIER_PKT_SIZE)) > return -EINVAL; > coresight_insert_barrier_packet(bufp); > return offset + CORESIGHT_BARRIER_PKT_SIZE; Thanks for the fix, I will send this as fixes at 6.4-rc1, as I have already sent the PULL request for 6.4 Suzuki

1 year, 8 months

Re: [PATCH] perf cs-etm: Add support for coresight trace for any range of CPUs

by Suzuki K Poulose

On 20/04/2023 13:37, Ganapatrao Kulkarni wrote: > > > On 20-04-2023 06:00 pm, James Clark wrote: >> >> >> On 20/04/2023 12:47, Ganapatrao Kulkarni wrote: >>> ... >>> My patch is rebased on 6.3-RC7 codebase with Mike's 3 perf patches >>> related to dynamic id [1] support(queued for 6.4). >>> >>> "perf report -D" works for me. >> >> I was referring to sparse CPU lists, which I think you mentioned above >> doesn't work even with this patch. >> >>> >>> [1] https://www.spinics.net/lists/linux-perf-users/msg27452.html >>> >> >> It should be based on the next branch here: >> git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git > > OK. It need not be. Since this patch is purely perf tools patch and has nothing to do with the kernel drivers, it should be beased on whatever the tip of the perf tool tree is. Otherwise we risk rebasing to that eventually. Cheers Suzuki

1 year, 8 months

Re: [PATCH] perf cs-etm: Add support for coresight trace for any range of CPUs

by James Clark

On 20/04/2023 12:47, Ganapatrao Kulkarni wrote: > > Hi James, > > On 20-04-2023 03:13 pm, James Clark wrote: >> >> >> On 19/04/2023 18:21, Ganapatrao Kulkarni wrote: >>> The current implementation supports coresight trace for a range of >>> CPUs, if the first CPU is CPU0. >>> >>> Adding changes to enable coresight trace for any range of CPUs by >>> decoding the first CPU also from the header. >>> Later, first CPU id is used instead of CPU0 across the decoder >>> functions. >>> >>> Signed-off-by: Ganapatrao Kulkarni <gankulkarni(a)os.amperecomputing.com> >>> --- >>> .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 4 +- >>> .../perf/util/cs-etm-decoder/cs-etm-decoder.h | 3 +- >>> tools/perf/util/cs-etm.c | 62 ++++++++++++------- >>> 3 files changed, 42 insertions(+), 27 deletions(-) >>> >>> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >>> b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >>> index 82a27ab90c8b..41ab299b643b 100644 >>> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >>> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >>> @@ -724,7 +724,7 @@ cs_etm_decoder__create_etm_decoder(struct >>> cs_etm_decoder_params *d_params, >>> } >>> struct cs_etm_decoder * >>> -cs_etm_decoder__new(int decoders, struct cs_etm_decoder_params >>> *d_params, >>> +cs_etm_decoder__new(int first_decoder, int decoders, struct >>> cs_etm_decoder_params *d_params, >>> struct cs_etm_trace_params t_params[]) >>> { >>> struct cs_etm_decoder *decoder; >>> @@ -769,7 +769,7 @@ cs_etm_decoder__new(int decoders, struct >>> cs_etm_decoder_params *d_params, >>> /* init raw frame logging if required */ >>> cs_etm_decoder__init_raw_frame_logging(d_params, decoder); >>> - for (i = 0; i < decoders; i++) { >>> + for (i = first_decoder; i < decoders; i++) { >>> ret = cs_etm_decoder__create_etm_decoder(d_params, >>> &t_params[i], >>> decoder); >>> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h >>> b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h >>> index 92a855fbe5b8..b06193fc75b4 100644 >>> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h >>> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h >>> @@ -90,7 +90,8 @@ int cs_etm_decoder__process_data_block(struct >>> cs_etm_decoder *decoder, >>> size_t len, size_t *consumed); >>> struct cs_etm_decoder * >>> -cs_etm_decoder__new(int num_cpu, >>> +cs_etm_decoder__new(int first_decoder, >>> + int decoders, >>> struct cs_etm_decoder_params *d_params, >>> struct cs_etm_trace_params t_params[]); >>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c >>> index 94e2d02009eb..2619513ae088 100644 >>> --- a/tools/perf/util/cs-etm.c >>> +++ b/tools/perf/util/cs-etm.c >>> @@ -55,6 +55,8 @@ struct cs_etm_auxtrace { >>> u8 has_virtual_ts; /* Virtual/Kernel timestamps in the trace. */ >>> int num_cpu; >>> + int first_cpu; >>> + int last_cpu; >>> u64 latest_kernel_timestamp; >>> u32 auxtrace_type; >>> u64 branches_sample_type; >>> @@ -638,14 +640,13 @@ static void cs_etm__set_trace_param_ete(struct >>> cs_etm_trace_params *t_params, >>> } >>> static int cs_etm__init_trace_params(struct cs_etm_trace_params >>> *t_params, >>> - struct cs_etm_auxtrace *etm, >>> - int decoders) >>> + struct cs_etm_auxtrace *etm) >>> { >>> int i; >>> u32 etmidr; >>> u64 architecture; >>> - for (i = 0; i < decoders; i++) { >>> + for (i = etm->first_cpu; i < etm->last_cpu; i++) { >>> architecture = etm->metadata[i][CS_ETM_MAGIC]; >>> switch (architecture) { >>> @@ -817,7 +818,7 @@ static void cs_etm__free(struct perf_session >>> *session) >>> /* Then the RB tree itself */ >>> intlist__delete(traceid_list); >>> - for (i = 0; i < aux->num_cpu; i++) >>> + for (i = aux->first_cpu; i < aux->last_cpu; i++) >>> zfree(&aux->metadata[i]); >>> thread__zput(aux->unknown_thread); >>> @@ -921,7 +922,8 @@ static struct cs_etm_queue >>> *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, >>> * Each queue can only contain data from one CPU when >>> unformatted, so only one decoder is >>> * needed. >>> */ >>> - int decoders = formatted ? etm->num_cpu : 1; >>> + int first_decoder = formatted ? etm->first_cpu : 0; >>> + int decoders = first_decoder + (formatted ? etm->num_cpu : 1); >>> etmq = zalloc(sizeof(*etmq)); >>> if (!etmq) >>> @@ -937,7 +939,7 @@ static struct cs_etm_queue >>> *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, >>> if (!t_params) >>> goto out_free; >>> - if (cs_etm__init_trace_params(t_params, etm, decoders)) >>> + if (cs_etm__init_trace_params(t_params, etm)) >>> goto out_free; >>> /* Set decoder parameters to decode trace packets */ >>> @@ -947,8 +949,7 @@ static struct cs_etm_queue >>> *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, >>> formatted)) >>> goto out_free; >>> - etmq->decoder = cs_etm_decoder__new(decoders, &d_params, >>> - t_params); >>> + etmq->decoder = cs_etm_decoder__new(first_decoder, decoders, >>> &d_params, t_params); >>> if (!etmq->decoder) >>> goto out_free; >>> @@ -2959,11 +2960,11 @@ static int cs_etm__queue_aux_records(struct >>> perf_session *session) >>> * Loop through the ETMs and complain if we find at least one where >>> ts_source != 1 (virtual >>> * timestamps). >>> */ >>> -static bool cs_etm__has_virtual_ts(u64 **metadata, int num_cpu) >>> +static bool cs_etm__has_virtual_ts(u64 **metadata, struct >>> cs_etm_auxtrace *etm) >>> { >>> int j; >>> - for (j = 0; j < num_cpu; j++) { >>> + for (j = etm->first_cpu; j < etm->last_cpu; j++) { >>> switch (metadata[j][CS_ETM_MAGIC]) { >>> case __perf_cs_etmv4_magic: >>> if (HAS_PARAM(j, ETMV4, TS_SOURCE) || >>> metadata[j][CS_ETMV4_TS_SOURCE] != 1) >>> @@ -2982,13 +2983,14 @@ static bool cs_etm__has_virtual_ts(u64 >>> **metadata, int num_cpu) >>> } >>> /* map trace ids to correct metadata block, from information in >>> metadata */ >>> -static int cs_etm__map_trace_ids_metadata(int num_cpu, u64 **metadata) >>> +static int cs_etm__map_trace_ids_metadata(struct cs_etm_auxtrace *etm) >>> { >>> u64 cs_etm_magic; >>> + u64 **metadata = etm->metadata; >>> u8 trace_chan_id; >>> int i, err; >>> - for (i = 0; i < num_cpu; i++) { >>> + for (i = etm->first_cpu; i < etm->last_cpu; i++) { >>> cs_etm_magic = metadata[i][CS_ETM_MAGIC]; >>> switch (cs_etm_magic) { >>> case __perf_cs_etmv3_magic: >>> @@ -3015,12 +3017,13 @@ static int cs_etm__map_trace_ids_metadata(int >>> num_cpu, u64 **metadata) >>> * If we found AUX_HW_ID packets, then set any metadata marked as >>> unused to the >>> * unused value to reduce the number of unneeded decoders created. >>> */ >>> -static int cs_etm__clear_unused_trace_ids_metadata(int num_cpu, u64 >>> **metadata) >>> +static int cs_etm__clear_unused_trace_ids_metadata(struct >>> cs_etm_auxtrace *etm) >>> { >>> u64 cs_etm_magic; >>> + u64 **metadata = etm->metadata; >>> int i; >>> - for (i = 0; i < num_cpu; i++) { >>> + for (i = etm->first_cpu; i < etm->last_cpu; i++) { >>> cs_etm_magic = metadata[i][CS_ETM_MAGIC]; >>> switch (cs_etm_magic) { >>> case __perf_cs_etmv3_magic: >>> @@ -3049,7 +3052,7 @@ int cs_etm__process_auxtrace_info_full(union >>> perf_event *event, >>> int event_header_size = sizeof(struct perf_event_header); >>> int total_size = auxtrace_info->header.size; >>> int priv_size = 0; >>> - int num_cpu; >>> + int num_cpu, first_cpu = 0, last_cpu; >>> int err = 0; >>> int aux_hw_id_found; >>> int i, j; >>> @@ -3068,22 +3071,31 @@ int cs_etm__process_auxtrace_info_full(union >>> perf_event *event, >>> /* First the global part */ >>> ptr = (u64 *) auxtrace_info->priv; >>> num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff; >>> - metadata = zalloc(sizeof(*metadata) * num_cpu); >>> + >>> + /* Start parsing after the common part of the header */ >>> + i = CS_HEADER_VERSION_MAX; >>> + >>> + /*Get CPU id of first event */ >>> + first_cpu = ptr[i + CS_ETM_CPU]; >>> + last_cpu = first_cpu + num_cpu; >>> + >>> + if (first_cpu > cpu__max_cpu().cpu || >>> + last_cpu > cpu__max_cpu().cpu) >>> + return -EINVAL; >>> + >>> + metadata = zalloc(sizeof(*metadata) * last_cpu); >> >> Hi Ganapatrao, >> >> I think I see what the problem is, but I'm wondering if a better fix >> would be to further decouple the CPU ID from the index in the array. >> >> With your change it's not clear what happens with sparse recordings, for >> example 'perf record -e cs_etm// -C 1,3,5'. And it seems like there is > > This patch fixes for any range of CPUs. > Record with sparse list of CPUs will not work with current code still. > Is there a major issue that means sparse can't be done? I'm thinking it would be best to fix both issues with one change while we understand this part rather than a half fix that might have do be completely re-understood and re-done later anyway. Unless there is some big blocker but I can't see it? >> some wastage in the zalloc here for example if only CPU 256 is traced >> then we'd still make 256 decoders but 255 of them would be unused? >> >> I tried to test sparse recordings, but your change doesn't apply to the >> latest coresight/next branch. I did notice that 'perf report -D' doesn't >> work with them on coresight/next (it just quits), but I couldn't see if >> that's fixed with your change. > > My patch is rebased on 6.3-RC7 codebase with Mike's 3 perf patches > related to dynamic id [1] support(queued for 6.4). > > "perf report -D" works for me. I was referring to sparse CPU lists, which I think you mentioned above doesn't work even with this patch. > > [1] https://www.spinics.net/lists/linux-perf-users/msg27452.html > It should be based on the next branch here: git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git >> >> Would a better fix not be to keep the metadata loops from 0-N and >> instead save the CPU ID in cs_etm_decoder_params or the decoder. That >> way it would support both sparse and not starting from 0 cases? I think > > Yep, I though this initially, it got complicated due to for more > for-loops. I will try again and post V2. > I can't imagine it would need any extra for loops off the top of my head. Just saving the CPU ID in a few structs and using it wherever it's needed instead of the loop index. I imagine most of the loops would actually stay the same rather than be changed like you have in V1. >> the code would be better if it's worded like "i < recorded_cpus" rather >> than "i < cpu" to make it clear that i isn't actually the CPU ID it's >> just an index. > > Yes, makes sense to call it "recorded_cpus". > >> >> Also a new test for this scenario would probably be a good idea. >> >> Thanks >> James >> > Thanks, > Ganapat

1 year, 8 months

Re: [PATCH] perf cs-etm: Add support for coresight trace for any range of CPUs

by James Clark

On 19/04/2023 18:21, Ganapatrao Kulkarni wrote: > The current implementation supports coresight trace for a range of > CPUs, if the first CPU is CPU0. > > Adding changes to enable coresight trace for any range of CPUs by > decoding the first CPU also from the header. > Later, first CPU id is used instead of CPU0 across the decoder functions. > > Signed-off-by: Ganapatrao Kulkarni <gankulkarni(a)os.amperecomputing.com> > --- > .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 4 +- > .../perf/util/cs-etm-decoder/cs-etm-decoder.h | 3 +- > tools/perf/util/cs-etm.c | 62 ++++++++++++------- > 3 files changed, 42 insertions(+), 27 deletions(-) > > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > index 82a27ab90c8b..41ab299b643b 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > @@ -724,7 +724,7 @@ cs_etm_decoder__create_etm_decoder(struct cs_etm_decoder_params *d_params, > } > > struct cs_etm_decoder * > -cs_etm_decoder__new(int decoders, struct cs_etm_decoder_params *d_params, > +cs_etm_decoder__new(int first_decoder, int decoders, struct cs_etm_decoder_params *d_params, > struct cs_etm_trace_params t_params[]) > { > struct cs_etm_decoder *decoder; > @@ -769,7 +769,7 @@ cs_etm_decoder__new(int decoders, struct cs_etm_decoder_params *d_params, > /* init raw frame logging if required */ > cs_etm_decoder__init_raw_frame_logging(d_params, decoder); > > - for (i = 0; i < decoders; i++) { > + for (i = first_decoder; i < decoders; i++) { > ret = cs_etm_decoder__create_etm_decoder(d_params, > &t_params[i], > decoder); > diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h > index 92a855fbe5b8..b06193fc75b4 100644 > --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h > +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h > @@ -90,7 +90,8 @@ int cs_etm_decoder__process_data_block(struct cs_etm_decoder *decoder, > size_t len, size_t *consumed); > > struct cs_etm_decoder * > -cs_etm_decoder__new(int num_cpu, > +cs_etm_decoder__new(int first_decoder, > + int decoders, > struct cs_etm_decoder_params *d_params, > struct cs_etm_trace_params t_params[]); > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index 94e2d02009eb..2619513ae088 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -55,6 +55,8 @@ struct cs_etm_auxtrace { > u8 has_virtual_ts; /* Virtual/Kernel timestamps in the trace. */ > > int num_cpu; > + int first_cpu; > + int last_cpu; > u64 latest_kernel_timestamp; > u32 auxtrace_type; > u64 branches_sample_type; > @@ -638,14 +640,13 @@ static void cs_etm__set_trace_param_ete(struct cs_etm_trace_params *t_params, > } > > static int cs_etm__init_trace_params(struct cs_etm_trace_params *t_params, > - struct cs_etm_auxtrace *etm, > - int decoders) > + struct cs_etm_auxtrace *etm) > { > int i; > u32 etmidr; > u64 architecture; > > - for (i = 0; i < decoders; i++) { > + for (i = etm->first_cpu; i < etm->last_cpu; i++) { > architecture = etm->metadata[i][CS_ETM_MAGIC]; > > switch (architecture) { > @@ -817,7 +818,7 @@ static void cs_etm__free(struct perf_session *session) > /* Then the RB tree itself */ > intlist__delete(traceid_list); > > - for (i = 0; i < aux->num_cpu; i++) > + for (i = aux->first_cpu; i < aux->last_cpu; i++) > zfree(&aux->metadata[i]); > > thread__zput(aux->unknown_thread); > @@ -921,7 +922,8 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, > * Each queue can only contain data from one CPU when unformatted, so only one decoder is > * needed. > */ > - int decoders = formatted ? etm->num_cpu : 1; > + int first_decoder = formatted ? etm->first_cpu : 0; > + int decoders = first_decoder + (formatted ? etm->num_cpu : 1); > > etmq = zalloc(sizeof(*etmq)); > if (!etmq) > @@ -937,7 +939,7 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, > if (!t_params) > goto out_free; > > - if (cs_etm__init_trace_params(t_params, etm, decoders)) > + if (cs_etm__init_trace_params(t_params, etm)) > goto out_free; > > /* Set decoder parameters to decode trace packets */ > @@ -947,8 +949,7 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, > formatted)) > goto out_free; > > - etmq->decoder = cs_etm_decoder__new(decoders, &d_params, > - t_params); > + etmq->decoder = cs_etm_decoder__new(first_decoder, decoders, &d_params, t_params); > > if (!etmq->decoder) > goto out_free; > @@ -2959,11 +2960,11 @@ static int cs_etm__queue_aux_records(struct perf_session *session) > * Loop through the ETMs and complain if we find at least one where ts_source != 1 (virtual > * timestamps). > */ > -static bool cs_etm__has_virtual_ts(u64 **metadata, int num_cpu) > +static bool cs_etm__has_virtual_ts(u64 **metadata, struct cs_etm_auxtrace *etm) > { > int j; > > - for (j = 0; j < num_cpu; j++) { > + for (j = etm->first_cpu; j < etm->last_cpu; j++) { > switch (metadata[j][CS_ETM_MAGIC]) { > case __perf_cs_etmv4_magic: > if (HAS_PARAM(j, ETMV4, TS_SOURCE) || metadata[j][CS_ETMV4_TS_SOURCE] != 1) > @@ -2982,13 +2983,14 @@ static bool cs_etm__has_virtual_ts(u64 **metadata, int num_cpu) > } > > /* map trace ids to correct metadata block, from information in metadata */ > -static int cs_etm__map_trace_ids_metadata(int num_cpu, u64 **metadata) > +static int cs_etm__map_trace_ids_metadata(struct cs_etm_auxtrace *etm) > { > u64 cs_etm_magic; > + u64 **metadata = etm->metadata; > u8 trace_chan_id; > int i, err; > > - for (i = 0; i < num_cpu; i++) { > + for (i = etm->first_cpu; i < etm->last_cpu; i++) { > cs_etm_magic = metadata[i][CS_ETM_MAGIC]; > switch (cs_etm_magic) { > case __perf_cs_etmv3_magic: > @@ -3015,12 +3017,13 @@ static int cs_etm__map_trace_ids_metadata(int num_cpu, u64 **metadata) > * If we found AUX_HW_ID packets, then set any metadata marked as unused to the > * unused value to reduce the number of unneeded decoders created. > */ > -static int cs_etm__clear_unused_trace_ids_metadata(int num_cpu, u64 **metadata) > +static int cs_etm__clear_unused_trace_ids_metadata(struct cs_etm_auxtrace *etm) > { > u64 cs_etm_magic; > + u64 **metadata = etm->metadata; > int i; > > - for (i = 0; i < num_cpu; i++) { > + for (i = etm->first_cpu; i < etm->last_cpu; i++) { > cs_etm_magic = metadata[i][CS_ETM_MAGIC]; > switch (cs_etm_magic) { > case __perf_cs_etmv3_magic: > @@ -3049,7 +3052,7 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, > int event_header_size = sizeof(struct perf_event_header); > int total_size = auxtrace_info->header.size; > int priv_size = 0; > - int num_cpu; > + int num_cpu, first_cpu = 0, last_cpu; > int err = 0; > int aux_hw_id_found; > int i, j; > @@ -3068,22 +3071,31 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event, > /* First the global part */ > ptr = (u64 *) auxtrace_info->priv; > num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff; > - metadata = zalloc(sizeof(*metadata) * num_cpu); > + > + /* Start parsing after the common part of the header */ > + i = CS_HEADER_VERSION_MAX; > + > + /*Get CPU id of first event */ > + first_cpu = ptr[i + CS_ETM_CPU]; > + last_cpu = first_cpu + num_cpu; > + > + if (first_cpu > cpu__max_cpu().cpu || > + last_cpu > cpu__max_cpu().cpu) > + return -EINVAL; > + > + metadata = zalloc(sizeof(*metadata) * last_cpu); Hi Ganapatrao, I think I see what the problem is, but I'm wondering if a better fix would be to further decouple the CPU ID from the index in the array. With your change it's not clear what happens with sparse recordings, for example 'perf record -e cs_etm// -C 1,3,5'. And it seems like there is some wastage in the zalloc here for example if only CPU 256 is traced then we'd still make 256 decoders but 255 of them would be unused? I tried to test sparse recordings, but your change doesn't apply to the latest coresight/next branch. I did notice that 'perf report -D' doesn't work with them on coresight/next (it just quits), but I couldn't see if that's fixed with your change. Would a better fix not be to keep the metadata loops from 0-N and instead save the CPU ID in cs_etm_decoder_params or the decoder. That way it would support both sparse and not starting from 0 cases? I think the code would be better if it's worded like "i < recorded_cpus" rather than "i < cpu" to make it clear that i isn't actually the CPU ID it's just an index. Also a new test for this scenario would probably be a good idea. Thanks James

1 year, 8 months

[GIT PULL] coresight: Updates for v6.4

by Suzuki K Poulose

Hi Greg, Please find the updates for coresight self hosted tracing targeting v6.4 Kindly pull. Suzuki The following changes since commit 197b6b60ae7bc51dd0814953c562833143b292aa: Linux 6.3-rc4 (2023-03-26 14:40:20 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git tags/coresight-next-v6.4 for you to fetch changes up to 18996a113f2567aef3057e300e3193ce2df1684c: coresight: etm_pmu: Set the module field (2023-04-14 12:14:09 +0100) ---------------------------------------------------------------- coresight: Updates for v6.4 This is a relatively smaller update for CoreSight tracing subsystem targeting v6.4, with the following changes: - Removing Mathieu Poirier as MAINTAINER for the subsystem, with updates to CREDITS for his contributions. - Fix CoreSight ETM PMU to set the module field Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> ---------------------------------------------------------------- Suzuki K Poulose (2): MAINTAINERS: Remove Mathieu Poirier as coresight maintainer coresight: etm_pmu: Set the module field CREDITS | 5 +++++ MAINTAINERS | 1 - drivers/hwtracing/coresight/coresight-etm-perf.c | 1 + 3 files changed, 6 insertions(+), 1 deletion(-)

1 year, 8 months

[PATCH] perf tools: Fix use before NULL check introduced by map__dso() introduction

by Arnaldo Carvalho de Melo

James Clark noticed that the recent 63df0e4bc368adbd ("perf map: Add accessor for dso") patch accessed map->dso before the 'map' variable was NULL checked, which is a change in logic that leads to segmentation faults, so comb thru that patch to fix similar cases. Fixes: 63df0e4bc368adbd ("perf map: Add accessor for dso") Cc: Adrian Hunter <adrian.hunter(a)intel.com> Cc: Alexander Shishkin <alexander.shishkin(a)linux.intel.com> Cc: Ian Rogers <irogers(a)google.com> Cc: Ingo Molnar <mingo(a)redhat.com> Cc: Jiri Olsa <jolsa(a)kernel.org> Cc: John Garry <john.g.garry(a)oracle.com> Cc: Leo Yan <leo.yan(a)linaro.org> Cc: Mark Rutland <mark.rutland(a)arm.com> Cc: Mathieu Poirier <mathieu.poirier(a)linaro.org> Cc: Mike Leach <mike.leach(a)linaro.org> Cc: Namhyung Kim <namhyung(a)kernel.org> Cc: Peter Zijlstra <peterz(a)infradead.org Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com> Cc: Will Deacon <will(a)kernel.org> To: James Clark <james.clark(a)arm.com> Link: https://lore.kernel.org/lkml/ Signed-off-by: Arnaldo Carvalho de Melo <acme(a)redhat.com> --- tools/perf/builtin-script.c | 7 +++---- tools/perf/ui/browsers/hists.c | 4 ++-- tools/perf/util/sort.c | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index 8fba247b798ca307..006f522d0e7f6a18 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -1075,8 +1075,7 @@ static int grab_bb(u8 *buffer, u64 start, u64 end, return 0; } - dso = map__dso(al.map); - if (!thread__find_map(thread, *cpumode, start, &al) || !dso) { + if (!thread__find_map(thread, *cpumode, start, &al) || (dso = map__dso(al.map)) == NULL) { pr_debug("\tcannot resolve %" PRIx64 "-%" PRIx64 "\n", start, end); return 0; } @@ -1106,9 +1105,9 @@ static int map__fprintf_srccode(struct map *map, u64 addr, FILE *fp, struct srcc unsigned line; int len; char *srccode; - struct dso *dso = map__dso(map); + struct dso *dso; - if (!map || !dso) + if (!map || (dso = map__dso(map)) == NULL) return 0; srcfile = get_srcline_split(dso, map__rip_2objdump(map, addr), diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c index ab70e5f5fad236e5..69c81759a64f938f 100644 --- a/tools/perf/ui/browsers/hists.c +++ b/tools/perf/ui/browsers/hists.c @@ -2499,9 +2499,9 @@ add_annotate_opt(struct hist_browser *browser __maybe_unused, struct map_symbol *ms, u64 addr) { - struct dso *dso = map__dso(ms->map); + struct dso *dso; - if (!ms->map || !dso || dso->annotate_warned) + if (!ms->map || (dso = map__dso(ms->map)) == NULL || dso->annotate_warned) return 0; if (!ms->sym) diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index f2ffaf90648e469e..31b1cd0935e277ba 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -1568,7 +1568,7 @@ static int hist_entry__dcacheline_snprintf(struct hist_entry *he, char *bf, if (he->mem_info) { struct map *map = he->mem_info->daddr.ms.map; - struct dso *dso = map__dso(map); + struct dso *dso = map ? map__dso(map) : NULL; addr = cl_address(he->mem_info->daddr.al_addr, chk_double_cl); ms = &he->mem_info->daddr.ms; -- 2.39.2

1 year, 8 months

[PATCH] perf: cs-etm: Fix segfault in dso lookup

by James Clark

map__dso() is called before thread__find_map() which always results in a null pointer dereference. Fix it by finding first, then checking if it exists. Fixes: 63df0e4bc368 ("perf map: Add accessor for dso") Signed-off-by: James Clark <james.clark(a)arm.com> --- tools/perf/util/cs-etm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 103865968700..8dd81ddd9e4e 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -885,9 +885,11 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id, thread = etmq->etm->unknown_thread; } - dso = map__dso(al.map); + if (!thread__find_map(thread, cpumode, address, &al)) + return 0; - if (!thread__find_map(thread, cpumode, address, &al) || !dso) + dso = map__dso(al.map); + if (!dso) return 0; if (dso->data.status == DSO_DATA_STATUS_ERROR && -- 2.34.1

1 year, 8 months

[PATCH] coresight: etm_pmu: Set the module field

by Suzuki K Poulose

struct pmu::module must be set to the module owning the PMU driver. Set this for the coresight etm_pmu. Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> --- drivers/hwtracing/coresight/coresight-etm-perf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index a48c97da8165..711f451b6946 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -901,6 +901,7 @@ int __init etm_perf_init(void) etm_pmu.addr_filters_sync = etm_addr_filters_sync; etm_pmu.addr_filters_validate = etm_addr_filters_validate; etm_pmu.nr_addr_filters = ETM_ADDR_CMP_MAX; + etm_pmu.module = THIS_MODULE; ret = perf_pmu_register(&etm_pmu, CORESIGHT_ETM_PMU_NAME, -1); if (ret == 0) -- 2.34.1

1 year, 8 months

[PATCH] coresight: Add support of setting trace id

by Mao Jinlong

With a growing number of trace sources, the number of source could be greater than 127 in future. There will be lack of ids. There could be also trace id reserved for some trace HW. With this change, trace id will be only configured when enable the source. Trace id will be dynamically allocated when traceid of driver data is not set when enable source. Once traceid of driver data is set when enable source, the traceid will be set as the specific value. Signed-off-by: Mao Jinlong <quic_jinlmao(a)quicinc.com> --- .../coresight/coresight-etm4x-core.c | 35 +++--- .../coresight/coresight-etm4x-sysfs.c | 30 +++++ drivers/hwtracing/coresight/coresight-stm.c | 67 ++++++++--- drivers/hwtracing/coresight/coresight-tpda.c | 107 ++++++++++++++---- drivers/hwtracing/coresight/coresight-tpda.h | 4 +- .../hwtracing/coresight/coresight-trace-id.c | 56 +++++++++ .../hwtracing/coresight/coresight-trace-id.h | 24 ++++ 7 files changed, 274 insertions(+), 49 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 1ea8f173cca0..8f2e4d2b0fc2 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -233,25 +233,32 @@ static int etm4_cpu_id(struct coresight_device *csdev) int etm4_read_alloc_trace_id(struct etmv4_drvdata *drvdata) { - int trace_id; + int trace_id, ret = 0; - /* - * This will allocate a trace ID to the cpu, - * or return the one currently allocated. - * The trace id function has its own lock - */ - trace_id = coresight_trace_id_get_cpu_id(drvdata->cpu); - if (IS_VALID_CS_TRACE_ID(trace_id)) - drvdata->trcid = (u8)trace_id; - else - dev_err(&drvdata->csdev->dev, - "Failed to allocate trace ID for %s on CPU%d\n", - dev_name(&drvdata->csdev->dev), drvdata->cpu); - return trace_id; + if (!drvdata->trcid) { + /* + * This will allocate a trace ID to the cpu, + * or return the one currently allocated. + * The trace id function has its own lock + */ + trace_id = coresight_trace_id_get_cpu_id(drvdata->cpu); + if (IS_VALID_CS_TRACE_ID(trace_id)) + drvdata->trcid = (u8)trace_id; + else { + ret = -EINVAL; + dev_err(&drvdata->csdev->dev, + "Failed to allocate trace ID for %s on CPU%d\n", + dev_name(&drvdata->csdev->dev), drvdata->cpu); + } + } else + ret = coresight_trace_id_set_cpu_id(drvdata->cpu, drvdata->trcid); + + return ret; } void etm4_release_trace_id(struct etmv4_drvdata *drvdata) { + drvdata->trcid = 0; coresight_trace_id_put_cpu_id(drvdata->cpu); } diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index 5e62aa40ecd0..bd342e63868c 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -2335,6 +2335,35 @@ static ssize_t ts_source_show(struct device *dev, } static DEVICE_ATTR_RO(ts_source); +static ssize_t traceid_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int val; + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); + + val = drvdata->trcid; + return sysfs_emit(buf, "0x%x\n", val); +} + +static ssize_t traceid_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + int ret; + unsigned long val; + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); + + ret = kstrtoul(buf, 16, &val); + if (ret) + return ret; + + if (!drvdata->csdev->enable) + drvdata->trcid = val; + + return size; +} +static DEVICE_ATTR_RW(traceid); + static struct attribute *coresight_etmv4_attrs[] = { &dev_attr_nr_pe_cmp.attr, &dev_attr_nr_addr_cmp.attr, @@ -2390,6 +2419,7 @@ static struct attribute *coresight_etmv4_attrs[] = { &dev_attr_vmid_masks.attr, &dev_attr_cpu.attr, &dev_attr_ts_source.attr, + &dev_attr_traceid.attr, NULL, }; diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index 66a614c5492c..1291f5f39ab1 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -192,10 +192,29 @@ static void stm_enable_hw(struct stm_drvdata *drvdata) CS_LOCK(drvdata->base); } +static int stm_configure_trace_id(struct stm_drvdata *drvdata) +{ + int traceid, ret = 0; + + if (!drvdata->traceid) { + traceid = coresight_trace_id_get_system_id(); + if (traceid < 0) + return traceid; + + drvdata->traceid = traceid; + } else + ret = coresight_trace_id_set_system_id(drvdata->traceid); + + return ret; + +} + static int stm_enable(struct coresight_device *csdev, struct perf_event *event, u32 mode) { u32 val; + int ret; + struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); if (mode != CS_MODE_SYSFS) @@ -207,6 +226,10 @@ static int stm_enable(struct coresight_device *csdev, if (val) return -EBUSY; + ret = stm_configure_trace_id(drvdata); + if (ret) + return ret; + pm_runtime_get_sync(csdev->dev.parent); spin_lock(&drvdata->spinlock); @@ -261,6 +284,8 @@ static void stm_disable(struct coresight_device *csdev, struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct csdev_access *csa = &csdev->access; + + coresight_trace_id_put_system_id(drvdata->traceid); /* * For as long as the tracer isn't disabled another entity can't * change its status. As such we can read the status here without @@ -268,6 +293,7 @@ static void stm_disable(struct coresight_device *csdev, */ if (local_read(&drvdata->mode) == CS_MODE_SYSFS) { spin_lock(&drvdata->spinlock); + drvdata->traceid = 0; stm_disable_hw(drvdata); spin_unlock(&drvdata->spinlock); @@ -608,7 +634,33 @@ static ssize_t traceid_show(struct device *dev, val = drvdata->traceid; return sprintf(buf, "%#lx\n", val); } -static DEVICE_ATTR_RO(traceid); + +static ssize_t traceid_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + int ret; + unsigned long val; + struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent); + + ret = kstrtoul(buf, 16, &val); + if (ret) + return ret; + + if (!IS_VALID_CS_TRACE_ID(val)) { + dev_err(&drvdata->csdev->dev, "Invalid trace id\n"); + return -EINVAL; + } + + if (!drvdata->csdev->enable) + drvdata->traceid = val; + else + dev_err(&drvdata->csdev->dev, "Device must be enabled! %s\n", + __func__); + + return size; +} +static DEVICE_ATTR_RW(traceid); static struct attribute *coresight_stm_attrs[] = { &dev_attr_hwevent_enable.attr, @@ -806,7 +858,7 @@ static void stm_init_generic_data(struct stm_drvdata *drvdata, static int stm_probe(struct amba_device *adev, const struct amba_id *id) { - int ret, trace_id; + int ret; void __iomem *base; struct device *dev = &adev->dev; struct coresight_platform_data *pdata = NULL; @@ -890,22 +942,12 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id) goto stm_unregister; } - trace_id = coresight_trace_id_get_system_id(); - if (trace_id < 0) { - ret = trace_id; - goto cs_unregister; - } - drvdata->traceid = (u8)trace_id; - pm_runtime_put(&adev->dev); dev_info(&drvdata->csdev->dev, "%s initialized\n", (char *)coresight_get_uci_data(id)); return 0; -cs_unregister: - coresight_unregister(drvdata->csdev); - stm_unregister: stm_unregister_device(&drvdata->stm); return ret; @@ -915,7 +957,6 @@ static void stm_remove(struct amba_device *adev) { struct stm_drvdata *drvdata = dev_get_drvdata(&adev->dev); - coresight_trace_id_put_system_id(drvdata->traceid); coresight_unregister(drvdata->csdev); stm_unregister_device(&drvdata->stm); diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c index f712e112ecff..41f83a5de3f2 100644 --- a/drivers/hwtracing/coresight/coresight-tpda.c +++ b/drivers/hwtracing/coresight/coresight-tpda.c @@ -21,6 +21,28 @@ DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda"); +static int tpda_configure_trace_id(struct tpda_drvdata *drvdata) +{ + int traceid, ret; + /* + * TPDA must has a unique atid. This atid can uniquely + * identify the TPDM trace source connected to the TPDA. + * The TPDMs which are connected to same TPDA share the + * same trace-id. When TPDA does packetization, different + * port will have unique channel number for decoding. + */ + if (!drvdata->traceid) { + traceid = coresight_trace_id_get_system_id(); + if (traceid < 0) + return traceid; + + drvdata->traceid = traceid; + } else + ret = coresight_trace_id_set_system_id(drvdata->traceid); + + return ret; +} + /* Settings pre enabling port control register */ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata) { @@ -28,8 +50,9 @@ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata) val = readl_relaxed(drvdata->base + TPDA_CR); val &= ~TPDA_CR_ATID; - val |= FIELD_PREP(TPDA_CR_ATID, drvdata->atid); + val |= FIELD_PREP(TPDA_CR_ATID, drvdata->traceid); writel_relaxed(val, drvdata->base + TPDA_CR); + } static void tpda_enable_port(struct tpda_drvdata *drvdata, int port) @@ -52,11 +75,17 @@ static void __tpda_enable(struct tpda_drvdata *drvdata, int port) tpda_enable_port(drvdata, port); CS_LOCK(drvdata->base); + } static int tpda_enable(struct coresight_device *csdev, int inport, int outport) { struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + int ret; + + ret = tpda_configure_trace_id(drvdata); + if (ret) + return ret; spin_lock(&drvdata->spinlock); if (atomic_read(&csdev->refcnt[inport]) == 0) @@ -87,7 +116,11 @@ static void tpda_disable(struct coresight_device *csdev, int inport, { struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + coresight_trace_id_put_system_id(drvdata->traceid); spin_lock(&drvdata->spinlock); + + drvdata->traceid = 0; + if (atomic_dec_return(&csdev->refcnt[inport]) == 0) __tpda_disable(drvdata, inport); @@ -105,27 +138,63 @@ static const struct coresight_ops tpda_cs_ops = { .link_ops = &tpda_link_ops, }; -static int tpda_init_default_data(struct tpda_drvdata *drvdata) +static ssize_t traceid_show(struct device *dev, + struct device_attribute *attr, char *buf) { - int atid; - /* - * TPDA must has a unique atid. This atid can uniquely - * identify the TPDM trace source connected to the TPDA. - * The TPDMs which are connected to same TPDA share the - * same trace-id. When TPDA does packetization, different - * port will have unique channel number for decoding. - */ - atid = coresight_trace_id_get_system_id(); - if (atid < 0) - return atid; + int val; + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); - drvdata->atid = atid; - return 0; + val = drvdata->traceid; + return sysfs_emit(buf, "0x%x\n", val); } -static int tpda_probe(struct amba_device *adev, const struct amba_id *id) +static ssize_t traceid_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t size) { int ret; + unsigned long val; + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); + + ret = kstrtoul(buf, 16, &val); + if (ret) + return ret; + + if (!IS_VALID_CS_TRACE_ID(val)) { + dev_err(drvdata->dev, "Invalid trace id\n"); + return -EINVAL; + } + + if (!drvdata->csdev->enable) + drvdata->traceid = val; + else + dev_err(drvdata->dev, "Device must be enabled! %s\n", __func__); + + return size; +} +static DEVICE_ATTR_RW(traceid); + +static struct attribute *coresight_tpda_attrs[] = { + &dev_attr_traceid.attr, + NULL, +}; + +static const struct attribute_group coresight_tpda_group = { + .attrs = coresight_tpda_attrs, +}; + +static const struct attribute_group *coresight_tpda_groups[] = { + &coresight_tpda_group, + NULL, +}; + +static void tpda_init_default_data(struct tpda_drvdata *drvdata) +{ + drvdata->traceid = 0; +} + +static int tpda_probe(struct amba_device *adev, const struct amba_id *id) +{ struct device *dev = &adev->dev; struct coresight_platform_data *pdata; struct tpda_drvdata *drvdata; @@ -151,9 +220,7 @@ static int tpda_probe(struct amba_device *adev, const struct amba_id *id) spin_lock_init(&drvdata->spinlock); - ret = tpda_init_default_data(drvdata); - if (ret) - return ret; + tpda_init_default_data(drvdata); desc.name = coresight_alloc_device_name(&tpda_devs, dev); if (!desc.name) @@ -164,6 +231,7 @@ static int tpda_probe(struct amba_device *adev, const struct amba_id *id) desc.pdata = adev->dev.platform_data; desc.dev = &adev->dev; desc.access = CSDEV_ACCESS_IOMEM(base); + desc.groups = coresight_tpda_groups; drvdata->csdev = coresight_register(&desc); if (IS_ERR(drvdata->csdev)) return PTR_ERR(drvdata->csdev); @@ -178,7 +246,6 @@ static void tpda_remove(struct amba_device *adev) { struct tpda_drvdata *drvdata = dev_get_drvdata(&adev->dev); - coresight_trace_id_put_system_id(drvdata->atid); coresight_unregister(drvdata->csdev); } diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h index 0399678df312..5de6dc92f450 100644 --- a/drivers/hwtracing/coresight/coresight-tpda.h +++ b/drivers/hwtracing/coresight/coresight-tpda.h @@ -22,14 +22,14 @@ * @dev: The device entity associated to this component. * @csdev: component vitals needed by the framework. * @spinlock: lock for the drvdata value. - * @enable: enable status of the component. + * @traceid: the atid value of TPDA. */ struct tpda_drvdata { void __iomem *base; struct device *dev; struct coresight_device *csdev; spinlock_t spinlock; - u8 atid; + u8 traceid; }; #endif /* _CORESIGHT_CORESIGHT_TPDA_H */ diff --git a/drivers/hwtracing/coresight/coresight-trace-id.c b/drivers/hwtracing/coresight/coresight-trace-id.c index af5b4ef59cea..192626efcb01 100644 --- a/drivers/hwtracing/coresight/coresight-trace-id.c +++ b/drivers/hwtracing/coresight/coresight-trace-id.c @@ -110,6 +110,19 @@ static int coresight_trace_id_alloc_new_id(struct coresight_trace_id_map *id_map return id; } +static int coresight_trace_id_set(int id, struct coresight_trace_id_map *id_map) +{ + if (WARN(!IS_VALID_CS_TRACE_ID(id), "Invalid Trace ID %d\n", id)) + return -EINVAL; + if (WARN(test_bit(id, id_map->used_ids), "ID is already used: %d\n", id)) + return -EINVAL; + set_bit(id, id_map->used_ids); + + DUMP_ID_MAP(id_map); + + return 0; +} + static void coresight_trace_id_free(int id, struct coresight_trace_id_map *id_map) { if (WARN(!IS_VALID_CS_TRACE_ID(id), "Invalid Trace ID %d\n", id)) @@ -195,6 +208,37 @@ static int coresight_trace_id_map_get_cpu_id(int cpu, struct coresight_trace_id_ return id; } +static int coresight_trace_id_map_set_cpu_id(int cpu, int id, struct coresight_trace_id_map *id_map) +{ + unsigned long flags; + + spin_lock_irqsave(&id_map_lock, flags); + + if (WARN(!IS_VALID_CS_TRACE_ID(id), "Invalid Trace ID %d\n", id)) { + spin_unlock_irqrestore(&id_map_lock, flags); + return -EINVAL; + } + + if (WARN(test_bit(id, id_map->used_ids), "ID is already used: %d\n", id)) { + spin_unlock_irqrestore(&id_map_lock, flags); + return -EINVAL; + } + + set_bit(id, id_map->used_ids); + + /* allocate the new id to the cpu */ + atomic_set(&per_cpu(cpu_id, cpu), id); + + cpumask_clear_cpu(cpu, &cpu_id_release_pending); + clear_bit(id, id_map->pend_rel_ids); + + spin_unlock_irqrestore(&id_map_lock, flags); + DUMP_ID_CPU(cpu, id); + DUMP_ID_MAP(id_map); + + return 0; +} + static void coresight_trace_id_map_put_cpu_id(int cpu, struct coresight_trace_id_map *id_map) { unsigned long flags; @@ -251,6 +295,12 @@ static void coresight_trace_id_map_put_system_id(struct coresight_trace_id_map * /* API functions */ +int coresight_trace_id_set_cpu_id(int cpu, int id) +{ + return coresight_trace_id_map_set_cpu_id(cpu, id, &id_map_default); +} +EXPORT_SYMBOL_GPL(coresight_trace_id_set_cpu_id); + int coresight_trace_id_get_cpu_id(int cpu) { return coresight_trace_id_map_get_cpu_id(cpu, &id_map_default); @@ -269,6 +319,12 @@ int coresight_trace_id_read_cpu_id(int cpu) } EXPORT_SYMBOL_GPL(coresight_trace_id_read_cpu_id); +int coresight_trace_id_set_system_id(int id) +{ + return coresight_trace_id_set(id, &id_map_default); +} +EXPORT_SYMBOL_GPL(coresight_trace_id_set_system_id); + int coresight_trace_id_get_system_id(void) { return coresight_trace_id_map_get_system_id(&id_map_default); diff --git a/drivers/hwtracing/coresight/coresight-trace-id.h b/drivers/hwtracing/coresight/coresight-trace-id.h index 3797777d367e..5dab9a473266 100644 --- a/drivers/hwtracing/coresight/coresight-trace-id.h +++ b/drivers/hwtracing/coresight/coresight-trace-id.h @@ -61,8 +61,21 @@ struct coresight_trace_id_map { DECLARE_BITMAP(pend_rel_ids, CORESIGHT_TRACE_IDS_MAX); }; + /* Allocate and release IDs for a single default trace ID map */ +/** + * Set the CoreSight Trace Id for the CPU. + * + * Set CoreSight Trace Id associated with the CPU. + * + * @cpu: The CPU index for the id. + * @id: Coresight Trace ID value. + * + * return: 0 if set successfully or -EINVAL if fail to set. + */ +int coresight_trace_id_set_cpu_id(int cpu, int id); + /** * Read and optionally allocate a CoreSight trace ID and associate with a CPU. * @@ -111,6 +124,17 @@ void coresight_trace_id_put_cpu_id(int cpu); */ int coresight_trace_id_read_cpu_id(int cpu); +/** + * Set trace id for a system component. + * + * Set the trace id if system component needs a static id for the trace. + * + * @id: value of trace ID. + * + * return: 0 if set successfully or -EINVAL if fail to set. + */ +int coresight_trace_id_set_system_id(int id); + /** * Allocate a CoreSight trace ID for a system component. * -- 2.39.0

1 year, 8 months

Re: [PATCH v2 0/6] Refactor definitions out of perf.h

by Arnaldo Carvalho de Melo

Em Mon, Apr 10, 2023 at 09:25:05AM -0700, Ian Rogers escreveu: > Move definitions and declarations for greater consistency. > > The original motivation was, when fuzzing something like parse-events, > having the main function in perf.c alongside global variables like > input_name means that input_name must be redeclared with the fuzzer > function's main. However, as the fuzzer is using the tools/perf code > as a library this causes backward linking reference that the linker > may warn about. > > v2. Remove references from commit messages about linking, as Adrian > points out we could solve that problem with #ifdefs. Thanks, applied. - Arnaldo > Ian Rogers (6): > perf ui: Move window resize signal functions > perf usage: Move usage strings > perf header: Move perf_version_string declaration > perf version: Use regular verbose flag > perf util: Move input_name to util > perf util: Move perf_guest/host declarations > > tools/perf/builtin-annotate.c | 2 +- > tools/perf/builtin-buildid-list.c | 2 +- > tools/perf/builtin-c2c.c | 2 +- > tools/perf/builtin-data.c | 2 +- > tools/perf/builtin-diff.c | 2 +- > tools/perf/builtin-evlist.c | 2 +- > tools/perf/builtin-help.c | 1 + > tools/perf/builtin-kmem.c | 2 +- > tools/perf/builtin-kvm.c | 1 + > tools/perf/builtin-kwork.c | 2 +- > tools/perf/builtin-mem.c | 2 +- > tools/perf/builtin-sched.c | 2 +- > tools/perf/builtin-stat.c | 2 +- > tools/perf/builtin-timechart.c | 2 +- > tools/perf/builtin-version.c | 7 +++---- > tools/perf/builtin.h | 3 --- > tools/perf/perf.c | 27 +-------------------------- > tools/perf/perf.h | 9 --------- > tools/perf/ui/browsers/hists.c | 2 +- > tools/perf/ui/hist.c | 2 +- > tools/perf/ui/setup.c | 19 +++++++++++++++++++ > tools/perf/ui/tui/setup.c | 1 - > tools/perf/ui/ui.h | 3 +++ > tools/perf/util/cs-etm.c | 1 + > tools/perf/util/event.c | 2 +- > tools/perf/util/evlist.c | 1 + > tools/perf/util/header.h | 2 ++ > tools/perf/util/parse-events.c | 2 +- > tools/perf/util/session.c | 2 +- > tools/perf/util/top.c | 2 +- > tools/perf/util/usage.c | 6 ++++++ > tools/perf/util/util.c | 2 ++ > tools/perf/util/util.h | 8 ++++++++ > 33 files changed, 66 insertions(+), 61 deletions(-) > > -- > 2.40.0.577.gac1e443424-goog > -- - Arnaldo

1 year, 8 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight April 2023