From: James Clark <james.clark@arm.com>
To: Suzuki K Poulose <suzuki.poulose@arm.com>, coresight@lists.linaro.org
Cc: Mike Leach <mike.leach@linaro.org>, Leo Yan <leo.yan@linaro.org>,
John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Mathieu Poirier <mathieu.poirier@linaro.org>
Subject: Re: [PATCH 2/3] coresight: Fail to open with return stacks if they are unavailable
Date: Thu, 9 Dec 2021 11:13:55 +0000 [thread overview]
Message-ID: <b61ef2e3-e573-4867-af5d-fd5fabece4b1@arm.com> (raw)
In-Reply-To: <b52ef2f3-9e30-59a6-2aea-e46c93915868@arm.com>
On 09/12/2021 11:00, Suzuki K Poulose wrote:
> On 08/12/2021 16:09, James Clark wrote:
>> Maintain consistency with the other options by failing to open when they
>> aren't supported. For example ETM_OPT_TS, ETM_OPT_CTXTID2 and the newly
>> added ETM_OPT_BRANCH_BROADCAST all return with -EINVAL if they are
>> requested but not supported by hardware.
>>
>> The consequence of not doing this is that the user may not be
>> aware that they are not enabling the feature as it is silently disabled.
>>
>> Signed-off-by: James Clark <james.clark@arm.com>
>> ---
>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index d2bafb50c66a..0a9bb943a5e5 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -674,10 +674,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>> }
>> /* return stack - enable if selected and supported */
>> - if ((attr->config & BIT(ETM_OPT_RETSTK)) && drvdata->retstack)
>> - /* bit[12], Return stack enable bit */
>> - config->cfg |= BIT(12);
>> -
>> + if (attr->config & BIT(ETM_OPT_RETSTK)) {
>> + if (!drvdata->retstack) {
>> + ret = -EINVAL;
>> + goto out;
>> + } else {
>> + /* bit[12], Return stack enable bit */
>> + config->cfg |= BIT(12);
>> + }
>
> nit: While at this, please could you change the hard coded value
> to ETM4_CFG_BIT_RETSTK ?
>
I started changing them all because I had trouble searching for bits by name but then
I thought it would snowball into a bigger change so I undid it.
I think I'll just go and do it now if it's an issue here.
> Otherwise, looks good to me
>
> Suzuki
WARNING: multiple messages have this Message-ID (diff)
From: James Clark <james.clark@arm.com>
To: Suzuki K Poulose <suzuki.poulose@arm.com>, coresight@lists.linaro.org
Cc: Mike Leach <mike.leach@linaro.org>, Leo Yan <leo.yan@linaro.org>,
John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Mathieu Poirier <mathieu.poirier@linaro.org>
Subject: Re: [PATCH 2/3] coresight: Fail to open with return stacks if they are unavailable
Date: Thu, 9 Dec 2021 11:13:55 +0000 [thread overview]
Message-ID: <b61ef2e3-e573-4867-af5d-fd5fabece4b1@arm.com> (raw)
In-Reply-To: <b52ef2f3-9e30-59a6-2aea-e46c93915868@arm.com>
On 09/12/2021 11:00, Suzuki K Poulose wrote:
> On 08/12/2021 16:09, James Clark wrote:
>> Maintain consistency with the other options by failing to open when they
>> aren't supported. For example ETM_OPT_TS, ETM_OPT_CTXTID2 and the newly
>> added ETM_OPT_BRANCH_BROADCAST all return with -EINVAL if they are
>> requested but not supported by hardware.
>>
>> The consequence of not doing this is that the user may not be
>> aware that they are not enabling the feature as it is silently disabled.
>>
>> Signed-off-by: James Clark <james.clark@arm.com>
>> ---
>> drivers/hwtracing/coresight/coresight-etm4x-core.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index d2bafb50c66a..0a9bb943a5e5 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -674,10 +674,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>> }
>> /* return stack - enable if selected and supported */
>> - if ((attr->config & BIT(ETM_OPT_RETSTK)) && drvdata->retstack)
>> - /* bit[12], Return stack enable bit */
>> - config->cfg |= BIT(12);
>> -
>> + if (attr->config & BIT(ETM_OPT_RETSTK)) {
>> + if (!drvdata->retstack) {
>> + ret = -EINVAL;
>> + goto out;
>> + } else {
>> + /* bit[12], Return stack enable bit */
>> + config->cfg |= BIT(12);
>> + }
>
> nit: While at this, please could you change the hard coded value
> to ETM4_CFG_BIT_RETSTK ?
>
I started changing them all because I had trouble searching for bits by name but then
I thought it would snowball into a bigger change so I undid it.
I think I'll just go and do it now if it's an issue here.
> Otherwise, looks good to me
>
> Suzuki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-12-09 11:14 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-08 16:09 [PATCH 1/3] coresight: Add config flag to enable branch broadcast James Clark
2021-12-08 16:09 ` James Clark
2021-12-08 16:09 ` [PATCH 2/3] coresight: Fail to open with return stacks if they are unavailable James Clark
2021-12-08 16:09 ` James Clark
2021-12-09 11:00 ` Suzuki K Poulose
2021-12-09 11:00 ` Suzuki K Poulose
2021-12-09 11:13 ` James Clark [this message]
2021-12-09 11:13 ` James Clark
2021-12-10 17:22 ` Mathieu Poirier
2021-12-10 17:22 ` Mathieu Poirier
2021-12-13 9:48 ` Mike Leach
2021-12-13 9:48 ` Mike Leach
2021-12-17 9:40 ` James Clark
2021-12-17 9:40 ` James Clark
2021-12-17 11:52 ` Mike Leach
2021-12-17 11:52 ` Mike Leach
2022-01-13 9:08 ` James Clark
2022-01-13 9:08 ` James Clark
2021-12-13 14:23 ` James Clark
2021-12-13 14:23 ` James Clark
2022-01-07 15:10 ` James Clark
2022-01-07 15:10 ` James Clark
2022-01-12 9:46 ` Suzuki K Poulose
2022-01-12 9:46 ` Suzuki K Poulose
2021-12-08 16:09 ` [PATCH 3/3] perf cs-etm: Update deduction of TRCCONFIGR register for branch broadcast James Clark
2021-12-08 16:09 ` James Clark
2021-12-10 2:41 ` Leo Yan
2021-12-10 2:41 ` Leo Yan
2022-01-13 9:09 ` James Clark
2022-01-13 9:09 ` James Clark
2021-12-09 9:21 ` [PATCH 1/3] coresight: Add config flag to enable " Suzuki K Poulose
2021-12-09 9:21 ` Suzuki K Poulose
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b61ef2e3-e573-4867-af5d-fd5fabece4b1@arm.com \
--to=james.clark@arm.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=coresight@lists.linaro.org \
--cc=john.garry@huawei.com \
--cc=jolsa@redhat.com \
--cc=leo.yan@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.poirier@linaro.org \
--cc=mike.leach@linaro.org \
--cc=namhyung@kernel.org \
--cc=suzuki.poulose@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.