* [PATCH] coresight: fix the wrong type of the trace_id in coresight_path
@ 2025-04-01 1:42 Jie Gan
2025-04-01 6:11 ` Anshuman Khandual
0 siblings, 1 reply; 6+ messages in thread
From: Jie Gan @ 2025-04-01 1:42 UTC (permalink / raw)
To: Suzuki K Poulose, Mike Leach, James Clark, Jie Gan
Cc: Tingwei Zhang, Jinlong Mao, coresight, linux-arm-kernel,
linux-kernel, linux-arm-msm, Dan Carpenter
The trace_id in coresight_path may contain an error number which means a
negative integer, but the current type of the trace_id is u8. Change the
type to int to fix it.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Fixes: 3c03c49b2fa5 ("Coresight: Introduce a new struct coresight_path")
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
include/linux/coresight.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index d79a242b271d..c2bf10c43e7c 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -337,7 +337,7 @@ static struct coresight_dev_list (var) = { \
*/
struct coresight_path {
struct list_head path_list;
- u8 trace_id;
+ int trace_id;
};
enum cs_mode {
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] coresight: fix the wrong type of the trace_id in coresight_path 2025-04-01 1:42 [PATCH] coresight: fix the wrong type of the trace_id in coresight_path Jie Gan @ 2025-04-01 6:11 ` Anshuman Khandual 2025-04-01 9:56 ` Mike Leach 0 siblings, 1 reply; 6+ messages in thread From: Anshuman Khandual @ 2025-04-01 6:11 UTC (permalink / raw) To: Jie Gan, Suzuki K Poulose, Mike Leach, James Clark, Jie Gan Cc: Tingwei Zhang, Jinlong Mao, coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, Dan Carpenter On 4/1/25 07:12, Jie Gan wrote: > The trace_id in coresight_path may contain an error number which means a > negative integer, but the current type of the trace_id is u8. Change the > type to int to fix it. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Fixes: 3c03c49b2fa5 ("Coresight: Introduce a new struct coresight_path") > Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> LGTM Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com> > --- > include/linux/coresight.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index d79a242b271d..c2bf10c43e7c 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -337,7 +337,7 @@ static struct coresight_dev_list (var) = { \ > */ > struct coresight_path { > struct list_head path_list; > - u8 trace_id; > + int trace_id; > }; > > enum cs_mode { ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] coresight: fix the wrong type of the trace_id in coresight_path 2025-04-01 6:11 ` Anshuman Khandual @ 2025-04-01 9:56 ` Mike Leach 2025-04-02 0:50 ` Jie Gan 0 siblings, 1 reply; 6+ messages in thread From: Mike Leach @ 2025-04-01 9:56 UTC (permalink / raw) To: Anshuman Khandual Cc: Jie Gan, Suzuki K Poulose, James Clark, Jie Gan, Tingwei Zhang, Jinlong Mao, coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, Dan Carpenter Hi, On Tue, 1 Apr 2025 at 07:11, Anshuman Khandual <anshuman.khandual@arm.com> wrote: > > On 4/1/25 07:12, Jie Gan wrote: > > The trace_id in coresight_path may contain an error number which means a > > negative integer, but the current type of the trace_id is u8. Change the > > type to int to fix it. > > > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > > Fixes: 3c03c49b2fa5 ("Coresight: Introduce a new struct coresight_path") > > Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> > > LGTM > > Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com> > > > --- > > include/linux/coresight.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > > index d79a242b271d..c2bf10c43e7c 100644 > > --- a/include/linux/coresight.h > > +++ b/include/linux/coresight.h > > @@ -337,7 +337,7 @@ static struct coresight_dev_list (var) = { \ > > */ > > struct coresight_path { > > struct list_head path_list; > > - u8 trace_id; > > + int trace_id; > > }; > > > > enum cs_mode { There are many places in the Coresight drivers that assign a u8 traceid from the path trace ID. e.g. In coresight-etm4x-core.c : etm4_enable_perf() drvdata->trcid = path->trace_id; drvdata->trcid is defined as a u8 - the reason being trace IDs are 128 bits wide with some reserved values. Will this not just trigger the same issue if path->trace_id is changed to an int? Even if not it is inconsistent handling of the trace ID values. Trace ID errors should be handled by returning an invalid trace ID value - were the trace ID value will fail the macro IS_VALID_CS_TRACE_ID(), or separate the return of a trace ID from an error return in a function. Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] coresight: fix the wrong type of the trace_id in coresight_path 2025-04-01 9:56 ` Mike Leach @ 2025-04-02 0:50 ` Jie Gan 2025-04-02 8:17 ` Mike Leach 0 siblings, 1 reply; 6+ messages in thread From: Jie Gan @ 2025-04-02 0:50 UTC (permalink / raw) To: Mike Leach, Anshuman Khandual Cc: Jie Gan, Suzuki K Poulose, James Clark, Tingwei Zhang, Jinlong Mao, coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, Dan Carpenter On 4/1/2025 5:56 PM, Mike Leach wrote: > Hi, > > On Tue, 1 Apr 2025 at 07:11, Anshuman Khandual > <anshuman.khandual@arm.com> wrote: >> >> On 4/1/25 07:12, Jie Gan wrote: >>> The trace_id in coresight_path may contain an error number which means a >>> negative integer, but the current type of the trace_id is u8. Change the >>> type to int to fix it. >>> >>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org> >>> Fixes: 3c03c49b2fa5 ("Coresight: Introduce a new struct coresight_path") >>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> >> >> LGTM >> >> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com> >> >>> --- >>> include/linux/coresight.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h >>> index d79a242b271d..c2bf10c43e7c 100644 >>> --- a/include/linux/coresight.h >>> +++ b/include/linux/coresight.h >>> @@ -337,7 +337,7 @@ static struct coresight_dev_list (var) = { \ >>> */ >>> struct coresight_path { >>> struct list_head path_list; >>> - u8 trace_id; >>> + int trace_id; >>> }; >>> >>> enum cs_mode { > > There are many places in the Coresight drivers that assign a u8 > traceid from the path trace ID. > > e.g. > In coresight-etm4x-core.c : etm4_enable_perf() > > drvdata->trcid = path->trace_id; > > drvdata->trcid is defined as a u8 - the reason being trace IDs are > 128 bits wide with some reserved values. > > Will this not just trigger the same issue if path->trace_id is changed > to an int? Even if not it is inconsistent handling of the trace ID > values. > > Trace ID errors should be handled by returning an invalid trace ID > value - were the trace ID value will fail the macro > IS_VALID_CS_TRACE_ID(), or separate the return of a trace ID from an > error return in a function. > Hi Mike, The path->trace_id is verified after it has been assigned with the logic you mentioned: if (!IS_VALID_CS_TRACE_ID(path->trace_id)) goto err_path; So it should be safe to assign to another u8 parameter, like you mentioned: In coresight-etm4x-core.c : etm4_enable_perf() drvdata->trcid = path->trace_id; Thanks, Jie > Regards > > Mike > > > > -- > Mike Leach > Principal Engineer, ARM Ltd. > Manchester Design Centre. UK ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] coresight: fix the wrong type of the trace_id in coresight_path 2025-04-02 0:50 ` Jie Gan @ 2025-04-02 8:17 ` Mike Leach 2025-04-02 8:36 ` Jie Gan 0 siblings, 1 reply; 6+ messages in thread From: Mike Leach @ 2025-04-02 8:17 UTC (permalink / raw) To: Jie Gan Cc: Anshuman Khandual, Jie Gan, Suzuki K Poulose, James Clark, Tingwei Zhang, Jinlong Mao, coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, Dan Carpenter Hi, On Wed, 2 Apr 2025 at 01:50, Jie Gan <quic_jiegan@quicinc.com> wrote: > > > > On 4/1/2025 5:56 PM, Mike Leach wrote: > > Hi, > > > > On Tue, 1 Apr 2025 at 07:11, Anshuman Khandual > > <anshuman.khandual@arm.com> wrote: > >> > >> On 4/1/25 07:12, Jie Gan wrote: > >>> The trace_id in coresight_path may contain an error number which means a > >>> negative integer, but the current type of the trace_id is u8. Change the > >>> type to int to fix it. > >>> > >>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > >>> Fixes: 3c03c49b2fa5 ("Coresight: Introduce a new struct coresight_path") > >>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> > >> > >> LGTM > >> > >> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com> > >> > >>> --- > >>> include/linux/coresight.h | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h > >>> index d79a242b271d..c2bf10c43e7c 100644 > >>> --- a/include/linux/coresight.h > >>> +++ b/include/linux/coresight.h > >>> @@ -337,7 +337,7 @@ static struct coresight_dev_list (var) = { \ > >>> */ > >>> struct coresight_path { > >>> struct list_head path_list; > >>> - u8 trace_id; > >>> + int trace_id; > >>> }; > >>> > >>> enum cs_mode { > > > > There are many places in the Coresight drivers that assign a u8 > > traceid from the path trace ID. > > > > e.g. > > In coresight-etm4x-core.c : etm4_enable_perf() > > > > drvdata->trcid = path->trace_id; > > > > drvdata->trcid is defined as a u8 - the reason being trace IDs are > > 128 bits wide with some reserved values. > > > > Will this not just trigger the same issue if path->trace_id is changed > > to an int? Even if not it is inconsistent handling of the trace ID > > values. > > > > Trace ID errors should be handled by returning an invalid trace ID > > value - were the trace ID value will fail the macro > > IS_VALID_CS_TRACE_ID(), or separate the return of a trace ID from an > > error return in a function. > > > > Hi Mike, > > The path->trace_id is verified after it has been assigned with the logic > you mentioned: > > if (!IS_VALID_CS_TRACE_ID(path->trace_id)) > goto err_path; > > So it should be safe to assign to another u8 parameter, like you mentioned: > > In coresight-etm4x-core.c : etm4_enable_perf() > > drvdata->trcid = path->trace_id; > It is safe but will it not trigger a warning just like the one you are trying to fix as the types are mismatched? Mike > Thanks, > Jie > > > > Regards > > > > Mike > > > > > > > > -- > > Mike Leach > > Principal Engineer, ARM Ltd. > > Manchester Design Centre. UK > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] coresight: fix the wrong type of the trace_id in coresight_path 2025-04-02 8:17 ` Mike Leach @ 2025-04-02 8:36 ` Jie Gan 0 siblings, 0 replies; 6+ messages in thread From: Jie Gan @ 2025-04-02 8:36 UTC (permalink / raw) To: Mike Leach Cc: Anshuman Khandual, Jie Gan, Suzuki K Poulose, James Clark, Tingwei Zhang, Jinlong Mao, coresight, linux-arm-kernel, linux-kernel, linux-arm-msm, Dan Carpenter On 4/2/2025 4:17 PM, Mike Leach wrote: > Hi, > > On Wed, 2 Apr 2025 at 01:50, Jie Gan <quic_jiegan@quicinc.com> wrote: >> >> >> >> On 4/1/2025 5:56 PM, Mike Leach wrote: >>> Hi, >>> >>> On Tue, 1 Apr 2025 at 07:11, Anshuman Khandual >>> <anshuman.khandual@arm.com> wrote: >>>> >>>> On 4/1/25 07:12, Jie Gan wrote: >>>>> The trace_id in coresight_path may contain an error number which means a >>>>> negative integer, but the current type of the trace_id is u8. Change the >>>>> type to int to fix it. >>>>> >>>>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org> >>>>> Fixes: 3c03c49b2fa5 ("Coresight: Introduce a new struct coresight_path") >>>>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com> >>>> >>>> LGTM >>>> >>>> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com> >>>> >>>>> --- >>>>> include/linux/coresight.h | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h >>>>> index d79a242b271d..c2bf10c43e7c 100644 >>>>> --- a/include/linux/coresight.h >>>>> +++ b/include/linux/coresight.h >>>>> @@ -337,7 +337,7 @@ static struct coresight_dev_list (var) = { \ >>>>> */ >>>>> struct coresight_path { >>>>> struct list_head path_list; >>>>> - u8 trace_id; >>>>> + int trace_id; >>>>> }; >>>>> >>>>> enum cs_mode { >>> >>> There are many places in the Coresight drivers that assign a u8 >>> traceid from the path trace ID. >>> >>> e.g. >>> In coresight-etm4x-core.c : etm4_enable_perf() >>> >>> drvdata->trcid = path->trace_id; >>> >>> drvdata->trcid is defined as a u8 - the reason being trace IDs are >>> 128 bits wide with some reserved values. >>> >>> Will this not just trigger the same issue if path->trace_id is changed >>> to an int? Even if not it is inconsistent handling of the trace ID >>> values. >>> >>> Trace ID errors should be handled by returning an invalid trace ID >>> value - were the trace ID value will fail the macro >>> IS_VALID_CS_TRACE_ID(), or separate the return of a trace ID from an >>> error return in a function. >>> >> >> Hi Mike, >> >> The path->trace_id is verified after it has been assigned with the logic >> you mentioned: >> >> if (!IS_VALID_CS_TRACE_ID(path->trace_id)) >> goto err_path; >> >> So it should be safe to assign to another u8 parameter, like you mentioned: >> >> In coresight-etm4x-core.c : etm4_enable_perf() >> >> drvdata->trcid = path->trace_id; >> > > It is safe but will it not trigger a warning just like the one you are > trying to fix as the types are mismatched? Hi Mike, It should trigger another type mismatch warning. This patch aims to fix the situation like assign a negative value to u8 will cause a integer toggle(it may happen and I think I should fix it), e.g. -22 for 234. Thanks, Jie > > Mike > >> Thanks, >> Jie >> >> >>> Regards >>> >>> Mike >>> >>> >>> >>> -- >>> Mike Leach >>> Principal Engineer, ARM Ltd. >>> Manchester Design Centre. UK >> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-02 8:36 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-04-01 1:42 [PATCH] coresight: fix the wrong type of the trace_id in coresight_path Jie Gan 2025-04-01 6:11 ` Anshuman Khandual 2025-04-01 9:56 ` Mike Leach 2025-04-02 0:50 ` Jie Gan 2025-04-02 8:17 ` Mike Leach 2025-04-02 8:36 ` Jie Gan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®