All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/1] coresight: Fix TRCCONFIGR.QE sysfs interface
@ 2022-01-20 11:30 ` James Clark
  0 siblings, 0 replies; 6+ messages in thread
From: James Clark @ 2022-01-20 11:30 UTC (permalink / raw)
  To: mathieu.poirier, coresight, mike.leach
  Cc: suzuki.poulose, leo.yan, James Clark, Leo Yan, linux-arm-kernel,
	linux-kernel

Changes since v1:
 * Add Mike's reviewed by tag
 * Also make it impossible to write the reserved value of 0b10, regardless
   of what is supplied by the user.

James Clark (1):
  coresight: Fix TRCCONFIGR.QE sysfs interface

 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 0/1] coresight: Fix TRCCONFIGR.QE sysfs interface
@ 2022-01-20 11:30 ` James Clark
  0 siblings, 0 replies; 6+ messages in thread
From: James Clark @ 2022-01-20 11:30 UTC (permalink / raw)
  To: mathieu.poirier, coresight, mike.leach
  Cc: suzuki.poulose, leo.yan, James Clark, Leo Yan, linux-arm-kernel,
	linux-kernel

Changes since v1:
 * Add Mike's reviewed by tag
 * Also make it impossible to write the reserved value of 0b10, regardless
   of what is supplied by the user.

James Clark (1):
  coresight: Fix TRCCONFIGR.QE sysfs interface

 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/1] coresight: Fix TRCCONFIGR.QE sysfs interface
  2022-01-20 11:30 ` James Clark
@ 2022-01-20 11:30   ` James Clark
  -1 siblings, 0 replies; 6+ messages in thread
From: James Clark @ 2022-01-20 11:30 UTC (permalink / raw)
  To: mathieu.poirier, coresight, mike.leach
  Cc: suzuki.poulose, leo.yan, James Clark, Leo Yan, linux-arm-kernel,
	linux-kernel

It's impossible to program a valid value for TRCCONFIGR.QE
when TRCIDR0.QSUPP==0b10. In that case the following is true:

  Q element support is implemented, and only supports Q elements without
  instruction counts. TRCCONFIGR.QE can only take the values 0b00 or 0b11.

Currently the low bit of QSUPP is checked to see if the low bit of QE can
be written to, but as you can see when QSUPP==0b10 the low bit is cleared
making it impossible to ever write the only valid value of 0b11 to QE.
0b10 would be written instead, which is a reserved QE value even for all
values of QSUPP.

The fix is to allow writing the low bit of QE for any non zero value of
QSUPP.

This change also ensures that the low bit is always set, even when the
user attempts to only set the high bit.

Signed-off-by: James Clark <james.clark@arm.com>
Reviewed-by: Mike Leach <mike.leach@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index a0640fa5c55b..57e94424a8d6 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -367,8 +367,12 @@ static ssize_t mode_store(struct device *dev,
 	mode = ETM_MODE_QELEM(config->mode);
 	/* start by clearing QE bits */
 	config->cfg &= ~(BIT(13) | BIT(14));
-	/* if supported, Q elements with instruction counts are enabled */
-	if ((mode & BIT(0)) && (drvdata->q_support & BIT(0)))
+	/*
+	 * if supported, Q elements with instruction counts are enabled.
+	 * Always set the low bit for any requested mode. Valid combos are
+	 * 0b00, 0b01 and 0b11.
+	 */
+	if (mode && drvdata->q_support)
 		config->cfg |= BIT(13);
 	/*
 	 * if supported, Q elements with and without instruction
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 1/1] coresight: Fix TRCCONFIGR.QE sysfs interface
@ 2022-01-20 11:30   ` James Clark
  0 siblings, 0 replies; 6+ messages in thread
From: James Clark @ 2022-01-20 11:30 UTC (permalink / raw)
  To: mathieu.poirier, coresight, mike.leach
  Cc: suzuki.poulose, leo.yan, James Clark, Leo Yan, linux-arm-kernel,
	linux-kernel

It's impossible to program a valid value for TRCCONFIGR.QE
when TRCIDR0.QSUPP==0b10. In that case the following is true:

  Q element support is implemented, and only supports Q elements without
  instruction counts. TRCCONFIGR.QE can only take the values 0b00 or 0b11.

Currently the low bit of QSUPP is checked to see if the low bit of QE can
be written to, but as you can see when QSUPP==0b10 the low bit is cleared
making it impossible to ever write the only valid value of 0b11 to QE.
0b10 would be written instead, which is a reserved QE value even for all
values of QSUPP.

The fix is to allow writing the low bit of QE for any non zero value of
QSUPP.

This change also ensures that the low bit is always set, even when the
user attempts to only set the high bit.

Signed-off-by: James Clark <james.clark@arm.com>
Reviewed-by: Mike Leach <mike.leach@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index a0640fa5c55b..57e94424a8d6 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -367,8 +367,12 @@ static ssize_t mode_store(struct device *dev,
 	mode = ETM_MODE_QELEM(config->mode);
 	/* start by clearing QE bits */
 	config->cfg &= ~(BIT(13) | BIT(14));
-	/* if supported, Q elements with instruction counts are enabled */
-	if ((mode & BIT(0)) && (drvdata->q_support & BIT(0)))
+	/*
+	 * if supported, Q elements with instruction counts are enabled.
+	 * Always set the low bit for any requested mode. Valid combos are
+	 * 0b00, 0b01 and 0b11.
+	 */
+	if (mode && drvdata->q_support)
 		config->cfg |= BIT(13);
 	/*
 	 * if supported, Q elements with and without instruction
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/1] coresight: Fix TRCCONFIGR.QE sysfs interface
  2022-01-20 11:30   ` James Clark
@ 2022-02-01 17:00     ` Mathieu Poirier
  -1 siblings, 0 replies; 6+ messages in thread
From: Mathieu Poirier @ 2022-02-01 17:00 UTC (permalink / raw)
  To: James Clark
  Cc: coresight, mike.leach, suzuki.poulose, leo.yan, Leo Yan,
	linux-arm-kernel, linux-kernel

On Thu, Jan 20, 2022 at 11:30:47AM +0000, James Clark wrote:
> It's impossible to program a valid value for TRCCONFIGR.QE
> when TRCIDR0.QSUPP==0b10. In that case the following is true:
> 
>   Q element support is implemented, and only supports Q elements without
>   instruction counts. TRCCONFIGR.QE can only take the values 0b00 or 0b11.
> 
> Currently the low bit of QSUPP is checked to see if the low bit of QE can
> be written to, but as you can see when QSUPP==0b10 the low bit is cleared
> making it impossible to ever write the only valid value of 0b11 to QE.
> 0b10 would be written instead, which is a reserved QE value even for all
> values of QSUPP.
> 
> The fix is to allow writing the low bit of QE for any non zero value of
> QSUPP.
> 
> This change also ensures that the low bit is always set, even when the
> user attempts to only set the high bit.
> 
> Signed-off-by: James Clark <james.clark@arm.com>
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index a0640fa5c55b..57e94424a8d6 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -367,8 +367,12 @@ static ssize_t mode_store(struct device *dev,
>  	mode = ETM_MODE_QELEM(config->mode);
>  	/* start by clearing QE bits */
>  	config->cfg &= ~(BIT(13) | BIT(14));
> -	/* if supported, Q elements with instruction counts are enabled */
> -	if ((mode & BIT(0)) && (drvdata->q_support & BIT(0)))
> +	/*
> +	 * if supported, Q elements with instruction counts are enabled.
> +	 * Always set the low bit for any requested mode. Valid combos are
> +	 * 0b00, 0b01 and 0b11.
> +	 */
> +	if (mode && drvdata->q_support)
>  		config->cfg |= BIT(13);

Interesting brain gymnastic - applied.

Thanks,
Mathieu

>  	/*
>  	 * if supported, Q elements with and without instruction
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/1] coresight: Fix TRCCONFIGR.QE sysfs interface
@ 2022-02-01 17:00     ` Mathieu Poirier
  0 siblings, 0 replies; 6+ messages in thread
From: Mathieu Poirier @ 2022-02-01 17:00 UTC (permalink / raw)
  To: James Clark
  Cc: coresight, mike.leach, suzuki.poulose, leo.yan, Leo Yan,
	linux-arm-kernel, linux-kernel

On Thu, Jan 20, 2022 at 11:30:47AM +0000, James Clark wrote:
> It's impossible to program a valid value for TRCCONFIGR.QE
> when TRCIDR0.QSUPP==0b10. In that case the following is true:
> 
>   Q element support is implemented, and only supports Q elements without
>   instruction counts. TRCCONFIGR.QE can only take the values 0b00 or 0b11.
> 
> Currently the low bit of QSUPP is checked to see if the low bit of QE can
> be written to, but as you can see when QSUPP==0b10 the low bit is cleared
> making it impossible to ever write the only valid value of 0b11 to QE.
> 0b10 would be written instead, which is a reserved QE value even for all
> values of QSUPP.
> 
> The fix is to allow writing the low bit of QE for any non zero value of
> QSUPP.
> 
> This change also ensures that the low bit is always set, even when the
> user attempts to only set the high bit.
> 
> Signed-off-by: James Clark <james.clark@arm.com>
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index a0640fa5c55b..57e94424a8d6 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -367,8 +367,12 @@ static ssize_t mode_store(struct device *dev,
>  	mode = ETM_MODE_QELEM(config->mode);
>  	/* start by clearing QE bits */
>  	config->cfg &= ~(BIT(13) | BIT(14));
> -	/* if supported, Q elements with instruction counts are enabled */
> -	if ((mode & BIT(0)) && (drvdata->q_support & BIT(0)))
> +	/*
> +	 * if supported, Q elements with instruction counts are enabled.
> +	 * Always set the low bit for any requested mode. Valid combos are
> +	 * 0b00, 0b01 and 0b11.
> +	 */
> +	if (mode && drvdata->q_support)
>  		config->cfg |= BIT(13);

Interesting brain gymnastic - applied.

Thanks,
Mathieu

>  	/*
>  	 * if supported, Q elements with and without instruction
> -- 
> 2.28.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-02-01 17:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-20 11:30 [PATCH v2 0/1] coresight: Fix TRCCONFIGR.QE sysfs interface James Clark
2022-01-20 11:30 ` James Clark
2022-01-20 11:30 ` [PATCH v2 1/1] " James Clark
2022-01-20 11:30   ` James Clark
2022-02-01 17:00   ` Mathieu Poirier
2022-02-01 17:00     ` Mathieu Poirier

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.